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