Re: [PATCH v6 0/2] loop: Better discard for block devices

From: Darrick J. Wong
Date: Mon Nov 11 2019 - 20:33:01 EST


On Mon, Nov 11, 2019 at 10:50:28AM -0800, Evan Green wrote:
> This series addresses some errors seen when using the loop
> device directly backed by a block device. The first change plumbs
> out the correct error message, and the second change prevents the
> error from occurring in many cases.
>
> The errors look like this:
> [ 90.880875] print_req_error: I/O error, dev loop5, sector 0
>
> The errors occur when trying to do a discard or write zeroes operation
> on a loop device backed by a block device that does not support write zeroes.
> Firstly, the error itself is incorrectly reported as I/O error, but is
> actually EOPNOTSUPP. The first patch plumbs out EOPNOTSUPP to properly
> report the error.
>
> The second patch prevents these errors from occurring by mirroring the
> zeroing capabilities of the underlying block device into the loop device.
> Before this change, discard was always reported as being supported, and
> the loop device simply turns around and does an fallocate operation on the
> backing device. After this change, backing block devices that do support
> zeroing will continue to work as before, and continue to get all the
> benefits of doing that. Backing devices that do not support zeroing will
> fail earlier, avoiding hitting the loop device at all and ultimately
> avoiding this error in the logs.
>
> I can also confirm that this fixes test block/003 in the blktests, when
> running blktests on a loop device backed by a block device.
>
> Darrick, I see you've got a related change in linux-next. I'm not sure what
> the status of that is, so I didn't base my latest spin on top of yours.

AFAIK the patch you reference changes NOUNMAP requests to use
FALLOC_FL_ZERO_RANGE and is queued for 5.5, which means patch #2 will
clash with it. It sort of looks like patch #2 reimplements the patch
that Jens already pulled for 5.5, so you probably want to rebase this
series atop his for-next tree.... but you should really ask Jens.

--D

> Changes in v6:
> - Updated tags
>
> Changes in v5:
> - Don't mirror discard if lo_encrypt_key_size is non-zero (Gwendal)
>
> Changes in v4:
> - Mirror blkdev's write_zeroes into loopdev's discard_sectors.
>
> Changes in v3:
> - Updated tags
> - Updated commit description
>
> Changes in v2:
> - Unnested error if statement (Bart)
>
> Evan Green (2):
> loop: Report EOPNOTSUPP properly
> loop: Better discard support for block devices
>
> drivers/block/loop.c | 66 +++++++++++++++++++++++++++++---------------
> 1 file changed, 44 insertions(+), 22 deletions(-)
>
> --
> 2.21.0
>