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

[Jorge Merlino]

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

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