Cmnt: [SRU][F][PATCH 1/1] loop: fix I/O error on fsync() in detached loop devices

[Thadeu Lima de Souza Cascardo]

On Thu, Mar 30, 2023 at 10:33:21AM -0300, Jorge Merlino wrote:
> Hi,
> 
> I used git format-patch and git send-email. I'm not sure what From header I
> am missing. The .patch files I sent had From headers.
> 
> Thanks
> Jorge
> 

So probably the authorship was changed during the backport process. That has
happened before to me. Not really sure about the details, but something to be
aware of.

Cascardo.

> On 30/3/23 08:52, Thadeu Lima de Souza Cascardo wrote:
> > Not a formal NACK, but rather a comment when applying and when sending other
> > fixes:
> > 
> > The author information is incorrect here. Upstream commit says it is Mauricio,
> > but the pseudo-header From: is missing from your message. How did you prepare
> > and submit this patch to the list?
> > 
> > Cascardo.
> > 
> > On Wed, Mar 29, 2023 at 05:20:28PM -0300, Jorge Merlino wrote:
> > > BugLink: https://bugs.launchpad.net/bugs/1856871
> > > 
> > > There's an I/O error on fsync() in a detached loop device
> > > if it has been previously attached.
> > > 
> > > The issue is write cache is enabled in the attach path in
> > > loop_configure() but it isn't disabled in the detach path;
> > > thus it remains enabled in the block device regardless of
> > > whether it is attached or not.
> > > 
> > > Now fsync() can get an I/O request that will just be failed
> > > later in loop_queue_rq() as device's state is not 'Lo_bound'.
> > > 
> > > So, disable write cache in the detach path.
> > > 
> > > Do so based on the queue flag, not the loop device flag for
> > > read-only (used to enable) as the queue flag can be changed
> > > via sysfs even on read-only loop devices (e.g., losetup -r.)
> > > 
> > > Test-case:
> > > 
> > >      # DEV=/dev/loop7
> > > 
> > >      # IMG=/tmp/image
> > >      # truncate --size 1M $IMG
> > > 
> > >      # losetup $DEV $IMG
> > >      # losetup -d $DEV
> > > 
> > > Before:
> > > 
> > >      # strace -e fsync parted -s $DEV print 2>&1 | grep fsync
> > >      fsync(3)                                = -1 EIO (Input/output error)
> > >      Warning: Error fsyncing/closing /dev/loop7: Input/output error
> > >      [  982.529929] blk_update_request: I/O error, dev loop7, sector 0 op 0x1:(WRITE) flags 0x800 phys_seg 0 prio class 0
> > > 
> > > After:
> > > 
> > >      # strace -e fsync parted -s $DEV print 2>&1 | grep fsync
> > >      fsync(3)                                = 0
> > > 
> > > Co-developed-by: Eric Desrochers <eric.desrochers at canonical.com>
> > > Signed-off-by: Eric Desrochers <eric.desrochers at canonical.com>
> > > Signed-off-by: Mauricio Faria de Oliveira <mfo at canonical.com>
> > > Tested-by: Gabriel Krisman Bertazi <krisman at collabora.com>
> > > Reviewed-by: Ming Lei <ming.lei at redhat.com>
> > > Signed-off-by: Jens Axboe <axboe at kernel.dk>
> > > (cherry picked from commit 4ceddce55eb35d15b0f87f5dcf6f0058fd15d3a4)
> > > Signed-off-by: Jorge Merlino <jorge.merlino at canonical.com>
> > > ---
> > >   drivers/block/loop.c | 3 +++
> > >   1 file changed, 3 insertions(+)
> > > 
> > > diff --git a/drivers/block/loop.c b/drivers/block/loop.c
> > > index 5ab7985fff8a..74ceba11ca0d 100644
> > > --- a/drivers/block/loop.c
> > > +++ b/drivers/block/loop.c
> > > @@ -1186,6 +1186,9 @@ static int __loop_clr_fd(struct loop_device *lo, bool release)
> > >   		goto out_unlock;
> > >   	}
> > > +	if (test_bit(QUEUE_FLAG_WC, &lo->lo_queue->queue_flags))
> > > +		blk_queue_write_cache(lo->lo_queue, false, false);
> > > +
> > >   	/* freeze request queue during the transition */
> > >   	blk_mq_freeze_queue(lo->lo_queue);
> > > -- 
> > > 2.25.1
> > > 
> > > 
> > > -- 
> > > kernel-team mailing list
> > > kernel-team at lists.ubuntu.com
> > > https://lists.ubuntu.com/mailman/listinfo/kernel-team