Re: [PATCH v2] loop: fix I/O error on fsync() in detached loop devices

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Wed, Jan 6, 2021 at 6:08 AM Ming Lei <ming.lei@xxxxxxxxxx> wrote:
>
> On Tue, Jan 05, 2021 at 10:54:19AM -0300, Mauricio Faria de Oliveira wrote:
> > 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.
> >
> > 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
>
> But IO on detached loop should have been failed, right? The magic is
> that submit_bio_checks() filters FLUSH request for queues which doesn't
> support writeback cache, and always fake a normal completion.
>

Hey Ming, thanks for taking a look at this.

Well, it depends -- currently read() works (without I/O errors) and
write() fails (ENOSPC).
Example tests are provided below.

And that's consistent before and after attach/detach; so, I thought
fsync() should follow.

> I understand that the issue is that user becomes confused with this observation
> because no such failure if they run 'parted -s /dev/loop0 print' on one detached
> loop disk if it is never attached.
>

That is indeed one of the issues. There's also a monitoring/alerting
perspective that
would benefit; e.g., sosreport runs parted, it's run on data
collection for support cases.
Now, that I/O error message is thrown in the logs, and some mon/alert
tools might not
yet have filters to ignore (detached) loop devices, and alert. It'd be
nice to deflect that.

It's not a common issue, to be honest; but the consistency point
seemed fair to me,
as essentially the current code doesn't deinitialize something it
previously initialized,
and the block device is left running with that enabled regardless.

cheers,

Setup:

    # DEV=/dev/loop7
    # IMG=/tmp/image
    # truncate --size 1M $IMG

Before attach/detach:

    # dd if=$DEV of=/dev/null
    0+0 records in
    0+0 records out
    0 bytes copied, 0.00011206 s, 0.0 kB/s

    # echo $?
    0

    # dd if=/dev/zero of=$DEV
    dd: writing to '/dev/loop7': No space left on device
    1+0 records in
    0+0 records out
    0 bytes copied, 0.000229225 s, 0.0 kB/s

    # echo $?
    1

After attach/detach: (same)

    # losetup $DEV $IMG
    # losetup -d $DEV

    # dd if=$DEV of=/dev/null
    0+0 records in
    0+0 records out
    0 bytes copied, 0.000102131 s, 0.0 kB/s

    # echo $?
    0

    # dd if=/dev/zero of=$DEV
    dd: writing to '/dev/loop7': No space left on device
    1+0 records in
    0+0 records out
    0 bytes copied, 0.000259658 s, 0.0 kB/s

    # echo $?
    1


>
> Thanks,
> Ming
>


-- 
Mauricio Faria de Oliveira



[Index of Archives]     [Linux RAID]     [Linux SCSI]     [Linux ATA RAID]     [IDE]     [Linux Wireless]     [Linux Kernel]     [ATH6KL]     [Linux Bluetooth]     [Linux Netdev]     [Kernel Newbies]     [Security]     [Git]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Device Mapper]

  Powered by Linux