Re: [PATCH V3] file-posix: allow -EBUSY -EINVAL errors during write zeros on block

[John Snow <jsnow@redhat.com>]

On 3/9/21 7:16 PM, ChangLimin wrote:
> Since Linux 5.10, write zeros to a multipath device using
> ioctl(fd, BLKZEROOUT, range) with cache none or directsync return -EBUSY
> permanently.
> 

When do we get -EINVAL? Both of the commits referenced below don't 
specifically mention it, so I am not sure in which circumstances that 
might arise.

> Similar to handle_aiocb_write_zeroes_unmap, handle_aiocb_write_zeroes_block
> allow -EBUSY and -EINVAL errors during ioctl(fd, BLKZEROOUT, range).
> 
> Reference commit in Linux 5.10:
> https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git/commit/?id=384d87ef2c954fc58e6c5fd8253e4a1984f5fe02 
> <https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git/commit/?id=384d87ef2c954fc58e6c5fd8253e4a1984f5fe02>
> 
> Although it will be fixed in 5.12, I think it's good to avoid similar 
> problem in the future.
> https://lore.kernel.org/linux-block/53689a67-7591-0ad8-3e7d-dca9a626cd99@kernel.dk/ 
> <https://lore.kernel.org/linux-block/53689a67-7591-0ad8-3e7d-dca9a626cd99@kernel.dk/>
> 

Wait, if they're fixing the function to actually apply a different 
fallback path, shouldn't we *not* allow EBUSY?

> Signed-off-by: ChangLimin <changlm@chinatelecom.cn>
> ---
>   block/file-posix.c | 9 +++++++--
>   1 file changed, 7 insertions(+), 2 deletions(-)
> 
> diff --git a/block/file-posix.c b/block/file-posix.c
> index 05079b40ca..4e132db929 100644
> --- a/block/file-posix.c
> +++ b/block/file-posix.c
> @@ -1629,8 +1629,13 @@ static ssize_t 
> handle_aiocb_write_zeroes_block(RawPosixAIOData *aiocb)
>           } while (errno == EINTR);
> 
>           ret = translate_err(-errno);
> -        if (ret == -ENOTSUP) {
> -            s->has_write_zeroes = false;
> +        switch (ret) {
> +        case -ENOTSUP:
> +            s->has_write_zeroes = false; /* fall through */
> +        case -EINVAL:
> +        case -EBUSY:
> +            return -ENOTSUP;
> +            break;

oh, we're not "allowing" them, we're treating the failure *more 
seriously* so that we avoid attempting to call this function ever again 
for this FD.

Can you please add a brief comment here, something like:

/* Linux 5.10/5.11 may return these for multipath devices */

>           }
>       }
>   #endif
> --
> 2.27.0
>