On Tue, 3 Jul 2012 20:16:31 +0800 majianpeng <majianpeng@xxxxxxxxx> wrote:
> On 2012-07-02 15:39 NeilBrown <neilb@xxxxxxx> Wrote:
> >On Mon, 2 Jul 2012 11:16:26 +0800 Shaohua Li <shli@xxxxxxxxxx> wrote:
> >
> >
> >> > Then I could see what is being added and what is being removed all in the one
> >> > patch and I can be sure that they balance.
> >>
> >> reworked the patch 3-5 to two patches as you suggested, and sent to you. please check.
> >
> >Thanks. That's looking really good.
> >
> >However I think we can do better. I've been looking more closely at the code
> >and I think that the only things that we need stripe_lock to protect are
> >->toread and ->towrite, together with the following bios. e.g.
> >->toread->bi_next etc.
> >
> >->read and ->written don't need stripe_lock protection, as they are only
> >manipulated by the handle_stripe state machine which uses STRIPE_ACTIVE
> >and refcounts for exclusion.
> >
> >So add_stripe_bio need to take the lock while adding a bio to the
> >->toread and ->towrite lists, and ops_run_biodrain() and ops_run_biofill
> >need to take the lock while the move the list from ->to{read,write} to
> >->{read,written}.
> How about xchg()?
(it would help a little if you made your comments stand out more, and maybe
be a bit more verbose).
bio_add_stripe() can insert a new bio into a list that starts are ->towrite
or ->toread.
You cannot do that with a simple xchg. You really need a lock.
NeilBrown
Attachment:
signature.asc
Description: PGP signature
