Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
Date:   Mon, 11 Nov 2019 17:32:49 -0800
From:   "Darrick J. Wong" <darrick.wong@oracle.com>
To:     Evan Green <evgreen@chromium.org>
Cc:     Jens Axboe <axboe@kernel.dk>,
        Martin K Petersen <martin.petersen@oracle.com>,
        Gwendal Grignou <gwendal@chromium.org>,
        Ming Lei <ming.lei@redhat.com>,
        Alexis Savery <asavery@chromium.org>,
        Douglas Anderson <dianders@chromium.org>,
        Bart Van Assche <bvanassche@acm.org>,
        linux-block@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH v6 0/2] loop: Better discard for block devices
Message-ID: <20191112013249.GD6235@magnolia>
References: <20191111185030.215451-1-evgreen@chromium.org>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20191111185030.215451-1-evgreen@chromium.org>
User-Agent: Mutt/1.9.4 (2018-02-28)
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

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
>