Re: [md PATCH 02/14] md/raid5: simplfy delaying of writes while metadata is updated.

NeilBrown
2017-02-17T02:10:48Z

[-- Attachment #1: Type: text/plain, Size: 2134 bytes --]

On Thu, Feb 16 2017, Shaohua Li wrote:

> On Thu, Feb 16, 2017 at 03:39:01PM +1100, Neil Brown wrote:
>> If a device fails during a write, we must ensure the failure is
>> recorded in the metadata before the completion of the write is
>> acknowleged.
>>
>> Commit c3cce6cda162 ("md/raid5: ensure device failure recorded before
>> write request returns.") added code for this, but it was
>> unnecessarily complicated. We already had similar functionality for
>> handling updates to the bad-block-list, thanks to Commit de393cdea66c
>> ("md: make it easier to wait for bad blocks to be acknowledged.")
>>
>> So revert most of the former commit, and instead avoid collecting
>> completed writes if MD_CHANGE_PENDING is set. raid5d() will then flush
>> the metadata and retry the stripe_head.
>>
>> We check MD_CHANGE_PENDING *after* analyse_stripe() as it could be set
>> asynchronously. After analyse_stripe(), we have collected stable data
>> about the state of devices, which will be used to make decisions.
>>
>> Signed-off-by: NeilBrown <neilb@suse.com>
>> ---
>> drivers/md/raid5.c | 27 ++++-----------------------
>> drivers/md/raid5.h | 3 ---
>> 2 files changed, 4 insertions(+), 26 deletions(-)
>>
>> diff --git a/drivers/md/raid5.c b/drivers/md/raid5.c
>> index 760b726943c9..154593e0afbe 100644
>> --- a/drivers/md/raid5.c
>> +++ b/drivers/md/raid5.c
>> @@ -4492,7 +4492,8 @@ static void handle_stripe(struct stripe_head *sh)
>> if (test_bit(STRIPE_LOG_TRAPPED, &sh->state))
>> goto finish;
>>
>> - if (s.handle_bad_blocks) {
>> + if (s.handle_bad_blocks ||
>> + test_bit(MD_SB_CHANGE_PENDING, &conf->mddev->sb_flags)) {
>> set_bit(STRIPE_HANDLE, &sh->state);
>> goto finish;
>> }
>
> This part is fragile. We don't delay it and post it to handle list again. So
> the raid5d/worker could run this stripe infinitely.
>

Ah yes - I forget about the worker threads.

I think we need to get raid5_do_work() to block while MD_CHANGE_PENDING
is set.
I'll look over the interactions there and see if that really make sense.

Thanks,
NeilBrown

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 832 bytes --]