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()?
>But we don't need it anywhere else. e.g. analyse_stripe
>shouldn't need the lock at all. Any change that could happen during the loop
>could equally happen after the lock was released so we don't lose by not
>having the lock.
>
>There is another current user of the lock, but I think that should be
>discarded as a false optimisation.
>We currently try to optimise out extra calls to bitmap_startwrite and
>bitmap_endwrite when we see back-to-back writes to the one stripe. However I
>suspect that is extremely unlikely and it just imposes and pointless need for
>synchronisation in raid5.
>
>We just simply call bitmap_startwrite whenever ->towrite changes from NULL to
>non-NULL, and call bitmap_endwrite whenever we clear ->written, reguardless
>what value ->towrite now has.
>
>Would you like to experiment with that? If I haven't described it well
>enough I can write a patch to show what I mean.
>
>Thanks,
>NeilBrown
>?韬{.n?????%??檩??w?{.n???{炳盯w???塄}?财??j:+v?? ? ???2??璀??摺?囤??z夸z罐?+?????w棹f
