On Thu, Apr 02, 2020 at 07:06:03PM +0200, Andrzej Pietrasiewicz wrote:
> From: Evan Green <evgreen@xxxxxxxxxxxx>
>
> If the backing device for a loop device is itself a block device,
> then mirror the "write zeroes" capabilities of the underlying
> block device into the loop device. Copy this capability into both
> max_write_zeroes_sectors and max_discard_sectors of the loop device.
>
> The reason for this is that REQ_OP_DISCARD on a loop device translates
> into blkdev_issue_zeroout(), rather than blkdev_issue_discard(). This
> presents a consistent interface for loop devices (that discarded data
> is zeroed), regardless of the backing device type of the loop device.
> There should be no behavior change for loop devices backed by regular
> files.
>
> This change fixes blktest block/003, and removes an extraneous
> error print in block/013 when testing on a loop device backed
> by a block device that does not support discard.
>
> Signed-off-by: Evan Green <evgreen@xxxxxxxxxxxx>
> Reviewed-by: Gwendal Grignou <gwendal@xxxxxxxxxxxx>
> Reviewed-by: Chaitanya Kulkarni <chaitanya.kulkarni@xxxxxxx>
> [used updated version of Evan's comment in loop_config_discard()]
> Signed-off-by: Andrzej Pietrasiewicz <andrzej.p@xxxxxxxxxxxxx>
> ---
> drivers/block/loop.c | 41 ++++++++++++++++++++++++++++++-----------
> 1 file changed, 30 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/block/loop.c b/drivers/block/loop.c
> index 6969be9a855a..d7f30338b8ec 100644
> --- a/drivers/block/loop.c
> +++ b/drivers/block/loop.c
> @@ -427,11 +427,12 @@ static int lo_fallocate(struct loop_device *lo, struct request *rq, loff_t pos,
> * information.
> */
> struct file *file = lo->lo_backing_file;
> + struct request_queue *q = lo->lo_queue;
> int ret;
>
> mode |= FALLOC_FL_KEEP_SIZE;
>
> - if ((!file->f_op->fallocate) || lo->lo_encrypt_key_size) {
> + if (!blk_queue_discard(q)) {
> ret = -EOPNOTSUPP;
> goto out;
> }
> @@ -864,6 +865,22 @@ static void loop_config_discard(struct loop_device *lo)
> struct file *file = lo->lo_backing_file;
> struct inode *inode = file->f_mapping->host;
> struct request_queue *q = lo->lo_queue;
> + struct request_queue *backingq;
> +
> + /*
> + * If the backing device is a block device, mirror its zeroing
> + * capability. Set the discard sectors to the block device's zeroing
> + * capabilities because loop discards result in blkdev_issue_zeroout(),
> + * not blkdev_issue_discard(). This maintains consistent behavior with
> + * file-backed loop devices: discarded regions read back as zero.
> + */
> + if (S_ISBLK(inode->i_mode) && !lo->lo_encrypt_key_size) {
> + backingq = bdev_get_queue(inode->i_bdev);
The backingq could move into this local scope.
> + } else if ((!file->f_op->fallocate) || lo->lo_encrypt_key_size) {
No need for the inner braces.
But the actual functionality looks good to me.
