>On Mon, 4 Mar 2013 10:24:16 +0800 majianpeng <majianpeng@xxxxxxxxx> wrote:
>
>> >On Thu, 28 Feb 2013 15:50:37 +0800 majianpeng <majianpeng@xxxxxxxxx> wrote:
>> >
>> >> Replacement is a fullsync which don't depent on bitmap.So regardless of
>> >> the presence and absence of bitmap, it do full resync.
>> >> If offset of normal io is larger than offset of resync,it will write
>> >> again when resync arrived this offset.
>> >
>> >This might be OK for RAID1 and RAID10 as recover is paused when writes
>> >happen, but that is not the case for RAID5, so it isn't safe to test against
>> >curr_resync - it gets updated a bit too later.
>> >
>> ->curr_resync + STRIPE_SECTORS is the next stripe which willbe replaced.
>> How about the ->curr_resync+STRIPE_SECTORS?
>
>If you aren't certain, then neither am I.
>
>As resync and normal writes can be intermingled you would need some guarantee
>that a write wouldn't be missed, and that almost certainly means a test under
>a lock against some value which is updated under a lock.
>
Yes as you said, we must not lost to write.
Because the normal io and replace io don't get the same stripe at the same time.
So there are two cases:
A: if (sh->sector < curr_resync)
For this case,withot doubt it must be write.
B: if (sh->sector >= curr_resync)
For sh->sector == curr_resync, this can't happen. Becase replace must wait this stripe.
For sh->sector > curr_resync, although the replace increate the ->curr_resync.But it can't
got the stripe which sh->sector == curr_resync.
So i think there isn't need some locks to protect.
For the above result, the base is the resync and normal io don't get the same stripe at the same time.
>>
>> >Also you messed up the formatting in raid10.c
>> >
>> Can you explain in detail?
>
>
>>> if (rrdev && (test_bit(Faulty, &rrdev->flags)
>>> - || test_bit(Unmerged, &rrdev->flags)))
>>> + || test_bit(Unmerged, &rrdev->flags) ||
>>> + (test_bit(Replacement, &rrdev->flags) &&
>>> + conf->mddev->curr_resync < r10_bio->sector)))
>
>Text that is inside parentheses (or other brackets) should never be to the
>left of the opening parenthesis unless that parenthesis is at the end of a
>line.
>In the original code, the "|| test_bit(Unmerged....." was to the right of the
>'('. In your version it starts to the left, and the lines you added also
>start to the left.
>
Thanks for the advise.
>> >I'm not convinced this optimisation is really worth it.
>> >
>> Maybe for HDD disk, it only improve speed by reducing some write operation.
>> But for ssd disk, it can reduce one write.
>
>Still doesn't sound convincing.
>
Maybe i do some test.But I still insist that write useless data is useless.
Thanks!
Jianpeng Ma
?韬{.n?????%??檩??w?{.n???{炳盯w???塄}?财??j:+v?? ? ???2??璀??摺?囤??z夸z罐?+?????w棹f
