Re: [PATCH] md: use BLK_STS_OK instead of hardcode

[Coly Li]

On 6/22/21 11:54 PM, Xianting Tian wrote:
> When setting io status, sometimes it uses BLK_STS_*, sometimes,
> it uses hardcode 0.
> Use the macro to replace hardcode in multiple places.
>
> Signed-off-by: Xianting Tian <xianting.tian@xxxxxxxxxxxxxxxxx>
> ---
>  drivers/md/bcache/request.c  | 2 +-
>  drivers/md/dm-clone-target.c | 2 +-
>  drivers/md/dm-integrity.c    | 2 +-
>  drivers/md/dm-mpath.c        | 2 +-
>  drivers/md/dm-raid1.c        | 2 +-
>  drivers/md/dm.c              | 2 +-
>  drivers/md/raid1.c           | 4 ++--
>  drivers/md/raid10.c          | 2 +-
>  8 files changed, 9 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/md/bcache/request.c b/drivers/md/bcache/request.c
> index 6d1de88..73ba5a6 100644
> --- a/drivers/md/bcache/request.c
> +++ b/drivers/md/bcache/request.c
> @@ -790,7 +790,7 @@ static void cached_dev_read_error(struct closure *cl)
>  		/* Retry from the backing device: */
>  		trace_bcache_read_retry(s->orig_bio);
>  
> -		s->iop.status = 0;
> +		s->iop.status = BLK_STS_OK;
>  		do_bio_hook(s, s->orig_bio, backing_request_endio);
>  
>  		/* XXX: invalidate cache */

Hi Xianting,

NACK for bcache part.

The change is incomplete, if you want to replace 0 by BLK_STS_OK, you
should check all locations
where s->iop.status is checked and replace with BLK_STS_OK when necessary.

One but not the only one example is,
871         if (s->iop.status)
872                 continue_at_nobarrier(cl, cached_dev_read_error,
bcache_wq);
Maybe you should change to
        if (s->iop.status != BLK_STS_OK)
                    continue_at_nobarrier(cl, cached_dev_read_error,
bcache_wq);


Just FYI.

Coly Li