Re: [PATCH v6 4/4] md: raid456 add nowait support

[Vishal Verma <vverma@digitalocean.com>]

On 12/25/21 5:07 PM, Song Liu wrote:
> On Sat, Dec 25, 2021 at 2:13 PM Vishal Verma <vverma@digitalocean.com> wrote:
>>
>> On 12/25/21 12:28 AM, Vishal Verma wrote:
>>>
>>> On 12/24/21 7:14 PM, Song Liu wrote:
>>>> On Tue, Dec 21, 2021 at 12:06 PM Vishal Verma<vverma@digitalocean.com>  wrote:
>>>>> Returns EAGAIN in case the raid456 driver would block
>>>>> waiting for situations like:
>>>>>
>>>>>     - Reshape operation,
>>>>>     - Discard operation.
>>>>>
>>>>> Signed-off-by: Vishal Verma<vverma@digitalocean.com>
>>>> I think we will need the following fix for raid456:
>>> Ack
>>>> ============================ 8< ============================
>>>>
>>>> diff --git i/drivers/md/raid5.c w/drivers/md/raid5.c
>>>> index 6ab22f29dacd..55d372ce3300 100644
>>>> --- i/drivers/md/raid5.c
>>>> +++ w/drivers/md/raid5.c
>>>> @@ -5717,6 +5717,7 @@ static void make_discard_request(struct mddev
>>>> *mddev, struct bio *bi)
>>>>                           raid5_release_stripe(sh);
>>>>                           /* Bail out if REQ_NOWAIT is set */
>>>>                           if (bi->bi_opf & REQ_NOWAIT) {
>>>> +                               finish_wait(&conf->wait_for_overlap, &w);
>>>>                                   bio_wouldblock_error(bi);
>>>>                                   return;
>>>>                           }
>>>> @@ -5734,6 +5735,7 @@ static void make_discard_request(struct mddev
>>>> *mddev, struct bio *bi)
>>>>                                   raid5_release_stripe(sh);
>>>>                                   /* Bail out if REQ_NOWAIT is set */
>>>>                                   if (bi->bi_opf & REQ_NOWAIT) {
>>>> +
>>>> finish_wait(&conf->wait_for_overlap, &w);
>>>>                                           bio_wouldblock_error(bi);
>>>>                                           return;
>>>>                                   }
>>>> @@ -5829,7 +5831,6 @@ static bool raid5_make_request(struct mddev
>>>> *mddev, struct bio * bi)
>>>>           last_sector = bio_end_sector(bi);
>>>>           bi->bi_next = NULL;
>>>>
>>>> -       md_account_bio(mddev, &bi);
>>>>           /* Bail out if REQ_NOWAIT is set */
>>>>           if ((bi->bi_opf & REQ_NOWAIT) &&
>>>>               (conf->reshape_progress != MaxSector) &&
>>>> @@ -5837,9 +5838,11 @@ static bool raid5_make_request(struct mddev
>>>> *mddev, struct bio * bi)
>>>>               ? (logical_sector > conf->reshape_progress &&
>>>> logical_sector <= conf->reshape_safe)
>>>>               : (logical_sector >= conf->reshape_safe && logical_sector
>>>> < conf->reshape_progress))) {
>>>>                   bio_wouldblock_error(bi);
>>>> +               if (rw == WRITE)
>>>> +                       md_write_end(mddev);
>>>>                   return true;
>>>>           }
>>>> -
>>>> +       md_account_bio(mddev, &bi);
>>>>           prepare_to_wait(&conf->wait_for_overlap, &w, TASK_UNINTERRUPTIBLE);
>>>>           for (; logical_sector < last_sector; logical_sector +=
>>>> RAID5_STRIPE_SECTORS(conf)) {
>>>>                   int previous;
>>>>
>>>> ============================ 8< ============================
>>>>
>>>> Vishal, please try to trigger all these conditions (including raid1,
>>>> raid10) and make sure
>>>> they work properly.
>>>>
>>>> For example, I triggered raid5 reshape and used something like the
>>>> following to make
>>>> sure the logic is triggered:
>>>>
>>>> diff --git a/drivers/md/raid5.c b/drivers/md/raid5.c
>>>> index 55d372ce3300..e79de48a0027 100644
>>>> --- a/drivers/md/raid5.c
>>>> +++ b/drivers/md/raid5.c
>>>> @@ -5840,6 +5840,11 @@ static bool raid5_make_request(struct mddev
>>>> *mddev, struct bio * bi)
>>>>                   bio_wouldblock_error(bi);
>>>>                   if (rw == WRITE)
>>>>                           md_write_end(mddev);
>>>> +               {
>>>> +                       static int count = 0;
>>>> +                       if (count++ < 10)
>>>> +                               pr_info("%s REQ_NOWAIT return\n", __func__);
>>>> +               }
>>>>                   return true;
>>>>           }
>>>>           md_account_bio(mddev, &bi);
>>>>
>>>> Thanks,
>>>> Song
>>>>
>>> Sure, will try this and verify for raid1/10.
> Please also try test raid5 with discard. I haven't tested those two
> conditions yet.
Ack.
>
>> I am running into an issue during raid10 reshape. I can see the nowait
>> code getting triggered during reshape, but it seems like the reshape
>> operation was stuck as soon as I issued write IO using FIO to the array
>> during reshape.
>> FIO also seem stuck i.e no IO went through...
> Maybe the following could fix it?
>
> Thanks,
> Song
Hmm no luck, still the same issue.
> diff --git i/drivers/md/raid10.c w/drivers/md/raid10.c
> index e2c524d50ec0..291eceaeb26c 100644
> --- i/drivers/md/raid10.c
> +++ w/drivers/md/raid10.c
> @@ -1402,14 +1402,14 @@ static void raid10_write_request(struct mddev
> *mddev, struct bio *bio,
>               : (bio->bi_iter.bi_sector + sectors > conf->reshape_safe &&
>                  bio->bi_iter.bi_sector < conf->reshape_progress))) {
>                  /* Need to update reshape_position in metadata */
> -               mddev->reshape_position = conf->reshape_progress;
> -               set_mask_bits(&mddev->sb_flags, 0,
> -                             BIT(MD_SB_CHANGE_DEVS) |
> BIT(MD_SB_CHANGE_PENDING));
> -               md_wakeup_thread(mddev->thread);
>                  if (bio->bi_opf & REQ_NOWAIT) {
>                          bio_wouldblock_error(bio);
>                          return;
>                  }
> +               mddev->reshape_position = conf->reshape_progress;
> +               set_mask_bits(&mddev->sb_flags, 0,
> +                             BIT(MD_SB_CHANGE_DEVS) |
> BIT(MD_SB_CHANGE_PENDING));
> +               md_wakeup_thread(mddev->thread);
>                  raid10_log(conf->mddev, "wait reshape metadata");
>                  wait_event(mddev->sb_wait,
>                             !test_bit(MD_SB_CHANGE_PENDING, &mddev->sb_flags));