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

[Song Liu <song@kernel.org>]

On Wed, Nov 10, 2021 at 10:15 AM 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>
> ---
>  drivers/md/raid5.c | 14 ++++++++++++++
>  1 file changed, 14 insertions(+)
>
> diff --git a/drivers/md/raid5.c b/drivers/md/raid5.c
> index 9c1a5877cf9f..fa64ee315241 100644
> --- a/drivers/md/raid5.c
> +++ b/drivers/md/raid5.c
> @@ -5710,6 +5710,11 @@ static void make_discard_request(struct mddev *mddev, struct bio *bi)
>                 int d;
>         again:
>                 sh = raid5_get_active_stripe(conf, logical_sector, 0, 0, 0);
> +               /* Bail out if REQ_NOWAIT is set */
> +               if (bi->bi_opf & REQ_NOWAIT) {
> +                       bio_wouldblock_error(bi);
> +                       return;
> +               }

This is not right. raid5_get_active_stripe() gets refcount on the sh,
we cannot simply
return here. I think we need the logic after raid5_release_stripe()
and before schedule().

>                 prepare_to_wait(&conf->wait_for_overlap, &w,
>                                 TASK_UNINTERRUPTIBLE);
>                 set_bit(R5_Overlap, &sh->dev[sh->pd_idx].flags);
> @@ -5820,6 +5825,15 @@ static bool raid5_make_request(struct mddev *mddev, struct bio * 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 &&
> +           mddev->reshape_backwards
> +           ? logical_sector < conf->reshape_safe
> +           : logical_sector >= conf->reshape_safe) {
> +               bio_wouldblock_error(bi);
> +               return true;
> +       }

This is also problematic, and is the trigger of those error messages.
We only want to trigger -EAGAIN when logical_sector is between
reshape_progress and reshape_safe.

Please let me know if these make sense.

Thanks,
Song