linux-block.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* loop_set_block_size: loop0 () has still dirty pages (nrpages=2)
@ 2021-06-08 12:01 Ingo Franzki
  2021-06-10 14:45 ` Ming Lei
  0 siblings, 1 reply; 8+ messages in thread
From: Ingo Franzki @ 2021-06-08 12:01 UTC (permalink / raw)
  To: Jens Axboe, linux-block, linux-kernel; +Cc: Juergen Christ

Hi all,

we occasionally encounter a problem when setting up a loop device in one of our automated testcases.

We set up a loop device as follows:

    # dd if=/dev/zero of=/var/tmp/loopbackfile1.img bs=1M count=2500 status=none
    # losetup --sector-size 4096 -fP --show /var/tmp/loopbackfile1.img

This works fine most of the times, but in the seldom case of the error, we get 'losetup: /var/tmp/loopbackfile1.img: failed to set up loop device: Resource temporarily unavailable'.

I am sure that no other loop device is currently defined, so we don't run out of loop devices.

We also see the following message in the syslog when the error occurs:

     loop_set_block_size: loop0 () has still dirty pages (nrpages=2)

The nrpages number varies from time to time. 

"Resource temporarily unavailable" is EAGAIN, and function loop_set_block_size() in drivers/block/loop.c returns this after printing the syslog message via pr_warn:

static int loop_set_block_size(struct loop_device *lo, unsigned long arg)
{
	int err = 0;

	if (lo->lo_state != Lo_bound)
		return -ENXIO;

	err = loop_validate_block_size(arg);
	if (err)
		return err;

	if (lo->lo_queue->limits.logical_block_size == arg)
		return 0;

	sync_blockdev(lo->lo_device);
	invalidate_bdev(lo->lo_device);

	blk_mq_freeze_queue(lo->lo_queue);

	/* invalidate_bdev should have truncated all the pages */
	if (lo->lo_device->bd_inode->i_mapping->nrpages) {
		err = -EAGAIN;
		pr_warn("%s: loop%d (%s) has still dirty pages (nrpages=%lu)\n",
			__func__, lo->lo_number, lo->lo_file_name,
			lo->lo_device->bd_inode->i_mapping->nrpages);
		goto out_unfreeze;
	}

	blk_queue_logical_block_size(lo->lo_queue, arg);
	blk_queue_physical_block_size(lo->lo_queue, arg);
	blk_queue_io_min(lo->lo_queue, arg);
	loop_update_dio(lo);
out_unfreeze:
	blk_mq_unfreeze_queue(lo->lo_queue);

	return err;
}

So looks like invalidate_bdev() did actually not truncate all the pages under some circumstances....

The problem only happens when '--sector-size 4096' is specified, with the default sector size is always works. It does not call loop_set_block_size() in the default case I guess.

The loop0 device has certainly be used by other testcases before, most likely with the default block size. But at the time of this run, no loop device is currently active (losetup shows nothing). 

Anyone have an idea what goes wrong here? 

This happens on upstream kernels on the s390x platform, but I can't tell if is related to the platform or a specific kernel version. 
The failing use case is not that old, so I can't tell if it would have happened on earlier kernels or not, or since when it happens. 

Any help is appreciated!

-- 
Ingo Franzki
ifranzki@linux.ibm.com  
Linux on IBM Z Development, Schoenaicher Str. 220, 71032 Boeblingen, Germany

IBM Deutschland Research & Development GmbH / Vorsitzender des Aufsichtsrats: Matthias Hartmann
Geschäftsführung: Dirk Wittkopp
Sitz der Gesellschaft: Böblingen / Registergericht: Amtsgericht Stuttgart, HRB 243294
IBM DATA Privacy Statement: https://www.ibm.com/privacy/us/en/

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: loop_set_block_size: loop0 () has still dirty pages (nrpages=2)
  2021-06-08 12:01 loop_set_block_size: loop0 () has still dirty pages (nrpages=2) Ingo Franzki
@ 2021-06-10 14:45 ` Ming Lei
  2021-06-14  7:35   ` Ingo Franzki
  0 siblings, 1 reply; 8+ messages in thread
From: Ming Lei @ 2021-06-10 14:45 UTC (permalink / raw)
  To: Ingo Franzki; +Cc: Jens Axboe, linux-block, linux-kernel, Juergen Christ

On Tue, Jun 08, 2021 at 02:01:29PM +0200, Ingo Franzki wrote:
> Hi all,
> 
> we occasionally encounter a problem when setting up a loop device in one of our automated testcases.
> 
> We set up a loop device as follows:
> 
>     # dd if=/dev/zero of=/var/tmp/loopbackfile1.img bs=1M count=2500 status=none
>     # losetup --sector-size 4096 -fP --show /var/tmp/loopbackfile1.img
> 
> This works fine most of the times, but in the seldom case of the error, we get 'losetup: /var/tmp/loopbackfile1.img: failed to set up loop device: Resource temporarily unavailable'.
> 
> I am sure that no other loop device is currently defined, so we don't run out of loop devices.
> 
> We also see the following message in the syslog when the error occurs:
> 
>      loop_set_block_size: loop0 () has still dirty pages (nrpages=2)
> 
> The nrpages number varies from time to time. 
> 
> "Resource temporarily unavailable" is EAGAIN, and function loop_set_block_size() in drivers/block/loop.c returns this after printing the syslog message via pr_warn:
> 
> static int loop_set_block_size(struct loop_device *lo, unsigned long arg)
> {
> 	int err = 0;
> 
> 	if (lo->lo_state != Lo_bound)
> 		return -ENXIO;
> 
> 	err = loop_validate_block_size(arg);
> 	if (err)
> 		return err;
> 
> 	if (lo->lo_queue->limits.logical_block_size == arg)
> 		return 0;
> 
> 	sync_blockdev(lo->lo_device);
> 	invalidate_bdev(lo->lo_device);
> 
> 	blk_mq_freeze_queue(lo->lo_queue);
> 
> 	/* invalidate_bdev should have truncated all the pages */
> 	if (lo->lo_device->bd_inode->i_mapping->nrpages) {
> 		err = -EAGAIN;
> 		pr_warn("%s: loop%d (%s) has still dirty pages (nrpages=%lu)\n",
> 			__func__, lo->lo_number, lo->lo_file_name,
> 			lo->lo_device->bd_inode->i_mapping->nrpages);
> 		goto out_unfreeze;
> 	}
> 
> 	blk_queue_logical_block_size(lo->lo_queue, arg);
> 	blk_queue_physical_block_size(lo->lo_queue, arg);
> 	blk_queue_io_min(lo->lo_queue, arg);
> 	loop_update_dio(lo);
> out_unfreeze:
> 	blk_mq_unfreeze_queue(lo->lo_queue);
> 
> 	return err;
> }
> 
> So looks like invalidate_bdev() did actually not truncate all the pages under some circumstances....
> 
> The problem only happens when '--sector-size 4096' is specified, with the default sector size is always works. It does not call loop_set_block_size() in the default case I guess.
> 
> The loop0 device has certainly be used by other testcases before, most likely with the default block size. But at the time of this run, no loop device is currently active (losetup shows nothing). 
> 
> Anyone have an idea what goes wrong here? 

It returns '-EAGAIN' to ask userspace to try again.

I understand loop_set_block_size() doesn't prevent page cache of this
loop disk from being dirtied, so it isn't strange to
see lo_device->bd_inode->i_mapping->nrpages isn't zero after sync_blockdev()
& invalidate_bdev() on loop.

> 
> This happens on upstream kernels on the s390x platform, but I can't tell if is related to the platform or a specific kernel version. 

It can be reproduced easily when you run buffered write on loop disk,
meantime keeping to change block size from one to another on this loop.


Thanks,
Ming


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: loop_set_block_size: loop0 () has still dirty pages (nrpages=2)
  2021-06-10 14:45 ` Ming Lei
@ 2021-06-14  7:35   ` Ingo Franzki
  2021-06-14 22:37     ` Ming Lei
  2021-06-15  8:42     ` Karel Zak
  0 siblings, 2 replies; 8+ messages in thread
From: Ingo Franzki @ 2021-06-14  7:35 UTC (permalink / raw)
  To: Ming Lei, Karel Zak; +Cc: Jens Axboe, linux-block, linux-kernel, Juergen Christ

On 10.06.2021 16:45, Ming Lei wrote:
> On Tue, Jun 08, 2021 at 02:01:29PM +0200, Ingo Franzki wrote:
>> Hi all,
>>
>> we occasionally encounter a problem when setting up a loop device in one of our automated testcases.
>>
>> We set up a loop device as follows:
>>
>>     # dd if=/dev/zero of=/var/tmp/loopbackfile1.img bs=1M count=2500 status=none
>>     # losetup --sector-size 4096 -fP --show /var/tmp/loopbackfile1.img
>>
>> This works fine most of the times, but in the seldom case of the error, we get 'losetup: /var/tmp/loopbackfile1.img: failed to set up loop device: Resource temporarily unavailable'.
>>
>> I am sure that no other loop device is currently defined, so we don't run out of loop devices.
>>
>> We also see the following message in the syslog when the error occurs:
>>
>>      loop_set_block_size: loop0 () has still dirty pages (nrpages=2)
>>
>> The nrpages number varies from time to time. 
>>
>> "Resource temporarily unavailable" is EAGAIN, and function loop_set_block_size() in drivers/block/loop.c returns this after printing the syslog message via pr_warn:
>>
>> static int loop_set_block_size(struct loop_device *lo, unsigned long arg)
>> {
>> 	int err = 0;
>>
>> 	if (lo->lo_state != Lo_bound)
>> 		return -ENXIO;
>>
>> 	err = loop_validate_block_size(arg);
>> 	if (err)
>> 		return err;
>>
>> 	if (lo->lo_queue->limits.logical_block_size == arg)
>> 		return 0;
>>
>> 	sync_blockdev(lo->lo_device);
>> 	invalidate_bdev(lo->lo_device);
>>
>> 	blk_mq_freeze_queue(lo->lo_queue);
>>
>> 	/* invalidate_bdev should have truncated all the pages */
>> 	if (lo->lo_device->bd_inode->i_mapping->nrpages) {
>> 		err = -EAGAIN;
>> 		pr_warn("%s: loop%d (%s) has still dirty pages (nrpages=%lu)\n",
>> 			__func__, lo->lo_number, lo->lo_file_name,
>> 			lo->lo_device->bd_inode->i_mapping->nrpages);
>> 		goto out_unfreeze;
>> 	}
>>
>> 	blk_queue_logical_block_size(lo->lo_queue, arg);
>> 	blk_queue_physical_block_size(lo->lo_queue, arg);
>> 	blk_queue_io_min(lo->lo_queue, arg);
>> 	loop_update_dio(lo);
>> out_unfreeze:
>> 	blk_mq_unfreeze_queue(lo->lo_queue);
>>
>> 	return err;
>> }
>>
>> So looks like invalidate_bdev() did actually not truncate all the pages under some circumstances....
>>
>> The problem only happens when '--sector-size 4096' is specified, with the default sector size is always works. It does not call loop_set_block_size() in the default case I guess.
>>
>> The loop0 device has certainly be used by other testcases before, most likely with the default block size. But at the time of this run, no loop device is currently active (losetup shows nothing). 
>>
>> Anyone have an idea what goes wrong here? 
> 
> It returns '-EAGAIN' to ask userspace to try again.
> 
> I understand loop_set_block_size() doesn't prevent page cache of this
> loop disk from being dirtied, so it isn't strange to
> see lo_device->bd_inode->i_mapping->nrpages isn't zero after sync_blockdev()
> & invalidate_bdev() on loop.
> 

OK, that makes sense from the kernel perspective. 

However, shouldn't then the losetup userspace utility implement some kind of retry logic in case of -EAGAIN ?
I don't see that in the source of losetup.c nor in loopdev.c in the util-linux package. There is a retry loop in create_loop() in losetup.c retrying loopcxt_setup_device() in case of EBUSY, but not in case of EAGAIN.

And losetup also hides the original error code and just returns EXIT_FAILURE in case of a failure. So no real good chance for the script that uses losetup to catch that error situation and perform a retry itself.

Adding Karel Zak (the maintainer of util-linux).

@Karel Zak: How about adding EAGAIN to the condition for performing a retry? 

Something like this:

-		if (errno == EBUSY && !hasdev && ntries < 64) {
+		if ((errno == EBUSY || errno == EAGAIN) && !hasdev && ntries < 64) {
			xusleep(200000);
			ntries++;
			continue;
		}

>>
>> This happens on upstream kernels on the s390x platform, but I can't tell if is related to the platform or a specific kernel version. 
> 
> It can be reproduced easily when you run buffered write on loop disk,
> meantime keeping to change block size from one to another on this loop.

Yes it is very likely that this is happening in our use case. 
> 
> 
> Thanks,
> Ming
> 


-- 
Ingo Franzki
eMail: ifranzki@linux.ibm.com  
Linux on IBM Z Development, Schoenaicher Str. 220, 71032 Boeblingen, Germany

IBM Deutschland Research & Development GmbH / Vorsitzender des Aufsichtsrats: Matthias Hartmann
Geschäftsführung: Dirk Wittkopp
Sitz der Gesellschaft: Böblingen / Registergericht: Amtsgericht Stuttgart, HRB 243294
IBM DATA Privacy Statement: https://www.ibm.com/privacy/us/en/

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: loop_set_block_size: loop0 () has still dirty pages (nrpages=2)
  2021-06-14  7:35   ` Ingo Franzki
@ 2021-06-14 22:37     ` Ming Lei
  2021-06-15  8:06       ` Jan Kara
  2021-06-15  8:42     ` Karel Zak
  1 sibling, 1 reply; 8+ messages in thread
From: Ming Lei @ 2021-06-14 22:37 UTC (permalink / raw)
  To: Ingo Franzki
  Cc: Karel Zak, Jens Axboe, linux-block, linux-kernel, Juergen Christ,
	Jan Kara

On Mon, Jun 14, 2021 at 09:35:30AM +0200, Ingo Franzki wrote:
> On 10.06.2021 16:45, Ming Lei wrote:
> > On Tue, Jun 08, 2021 at 02:01:29PM +0200, Ingo Franzki wrote:
> >> Hi all,
> >>
> >> we occasionally encounter a problem when setting up a loop device in one of our automated testcases.
> >>
> >> We set up a loop device as follows:
> >>
> >>     # dd if=/dev/zero of=/var/tmp/loopbackfile1.img bs=1M count=2500 status=none
> >>     # losetup --sector-size 4096 -fP --show /var/tmp/loopbackfile1.img
> >>
> >> This works fine most of the times, but in the seldom case of the error, we get 'losetup: /var/tmp/loopbackfile1.img: failed to set up loop device: Resource temporarily unavailable'.
> >>
> >> I am sure that no other loop device is currently defined, so we don't run out of loop devices.
> >>
> >> We also see the following message in the syslog when the error occurs:
> >>
> >>      loop_set_block_size: loop0 () has still dirty pages (nrpages=2)
> >>
> >> The nrpages number varies from time to time. 
> >>
> >> "Resource temporarily unavailable" is EAGAIN, and function loop_set_block_size() in drivers/block/loop.c returns this after printing the syslog message via pr_warn:
> >>
> >> static int loop_set_block_size(struct loop_device *lo, unsigned long arg)
> >> {
> >> 	int err = 0;
> >>
> >> 	if (lo->lo_state != Lo_bound)
> >> 		return -ENXIO;
> >>
> >> 	err = loop_validate_block_size(arg);
> >> 	if (err)
> >> 		return err;
> >>
> >> 	if (lo->lo_queue->limits.logical_block_size == arg)
> >> 		return 0;
> >>
> >> 	sync_blockdev(lo->lo_device);
> >> 	invalidate_bdev(lo->lo_device);
> >>
> >> 	blk_mq_freeze_queue(lo->lo_queue);
> >>
> >> 	/* invalidate_bdev should have truncated all the pages */
> >> 	if (lo->lo_device->bd_inode->i_mapping->nrpages) {
> >> 		err = -EAGAIN;
> >> 		pr_warn("%s: loop%d (%s) has still dirty pages (nrpages=%lu)\n",
> >> 			__func__, lo->lo_number, lo->lo_file_name,
> >> 			lo->lo_device->bd_inode->i_mapping->nrpages);
> >> 		goto out_unfreeze;
> >> 	}
> >>
> >> 	blk_queue_logical_block_size(lo->lo_queue, arg);
> >> 	blk_queue_physical_block_size(lo->lo_queue, arg);
> >> 	blk_queue_io_min(lo->lo_queue, arg);
> >> 	loop_update_dio(lo);
> >> out_unfreeze:
> >> 	blk_mq_unfreeze_queue(lo->lo_queue);
> >>
> >> 	return err;
> >> }
> >>
> >> So looks like invalidate_bdev() did actually not truncate all the pages under some circumstances....
> >>
> >> The problem only happens when '--sector-size 4096' is specified, with the default sector size is always works. It does not call loop_set_block_size() in the default case I guess.
> >>
> >> The loop0 device has certainly be used by other testcases before, most likely with the default block size. But at the time of this run, no loop device is currently active (losetup shows nothing). 
> >>
> >> Anyone have an idea what goes wrong here? 
> > 
> > It returns '-EAGAIN' to ask userspace to try again.
> > 
> > I understand loop_set_block_size() doesn't prevent page cache of this
> > loop disk from being dirtied, so it isn't strange to
> > see lo_device->bd_inode->i_mapping->nrpages isn't zero after sync_blockdev()
> > & invalidate_bdev() on loop.
> > 
> 
> OK, that makes sense from the kernel perspective. 

We might improve this code by holding ->i_rwsem / mapping->invalidate_lock in
loop_set_block_size() to prevent new dirtying pages, but this still
can't guarantee that i_mapping->nrpages can become 0 after sync &
revalidate bdev. Or maybe replace invalidate_bdev() with truncate_bdev_range().



Thanks,
Ming


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: loop_set_block_size: loop0 () has still dirty pages (nrpages=2)
  2021-06-14 22:37     ` Ming Lei
@ 2021-06-15  8:06       ` Jan Kara
  0 siblings, 0 replies; 8+ messages in thread
From: Jan Kara @ 2021-06-15  8:06 UTC (permalink / raw)
  To: Ming Lei
  Cc: Ingo Franzki, Karel Zak, Jens Axboe, linux-block, linux-kernel,
	Juergen Christ, Jan Kara

On Tue 15-06-21 06:37:25, Ming Lei wrote:
> On Mon, Jun 14, 2021 at 09:35:30AM +0200, Ingo Franzki wrote:
> > On 10.06.2021 16:45, Ming Lei wrote:
> > > On Tue, Jun 08, 2021 at 02:01:29PM +0200, Ingo Franzki wrote:
> > >> Hi all,
> > >>
> > >> we occasionally encounter a problem when setting up a loop device in one of our automated testcases.
> > >>
> > >> We set up a loop device as follows:
> > >>
> > >>     # dd if=/dev/zero of=/var/tmp/loopbackfile1.img bs=1M count=2500 status=none
> > >>     # losetup --sector-size 4096 -fP --show /var/tmp/loopbackfile1.img
> > >>
> > >> This works fine most of the times, but in the seldom case of the error, we get 'losetup: /var/tmp/loopbackfile1.img: failed to set up loop device: Resource temporarily unavailable'.
> > >>
> > >> I am sure that no other loop device is currently defined, so we don't run out of loop devices.
> > >>
> > >> We also see the following message in the syslog when the error occurs:
> > >>
> > >>      loop_set_block_size: loop0 () has still dirty pages (nrpages=2)
> > >>
> > >> The nrpages number varies from time to time. 
> > >>
> > >> "Resource temporarily unavailable" is EAGAIN, and function loop_set_block_size() in drivers/block/loop.c returns this after printing the syslog message via pr_warn:
> > >>
> > >> static int loop_set_block_size(struct loop_device *lo, unsigned long arg)
> > >> {
> > >> 	int err = 0;
> > >>
> > >> 	if (lo->lo_state != Lo_bound)
> > >> 		return -ENXIO;
> > >>
> > >> 	err = loop_validate_block_size(arg);
> > >> 	if (err)
> > >> 		return err;
> > >>
> > >> 	if (lo->lo_queue->limits.logical_block_size == arg)
> > >> 		return 0;
> > >>
> > >> 	sync_blockdev(lo->lo_device);
> > >> 	invalidate_bdev(lo->lo_device);
> > >>
> > >> 	blk_mq_freeze_queue(lo->lo_queue);
> > >>
> > >> 	/* invalidate_bdev should have truncated all the pages */
> > >> 	if (lo->lo_device->bd_inode->i_mapping->nrpages) {
> > >> 		err = -EAGAIN;
> > >> 		pr_warn("%s: loop%d (%s) has still dirty pages (nrpages=%lu)\n",
> > >> 			__func__, lo->lo_number, lo->lo_file_name,
> > >> 			lo->lo_device->bd_inode->i_mapping->nrpages);
> > >> 		goto out_unfreeze;
> > >> 	}
> > >>
> > >> 	blk_queue_logical_block_size(lo->lo_queue, arg);
> > >> 	blk_queue_physical_block_size(lo->lo_queue, arg);
> > >> 	blk_queue_io_min(lo->lo_queue, arg);
> > >> 	loop_update_dio(lo);
> > >> out_unfreeze:
> > >> 	blk_mq_unfreeze_queue(lo->lo_queue);
> > >>
> > >> 	return err;
> > >> }
> > >>
> > >> So looks like invalidate_bdev() did actually not truncate all the pages under some circumstances....
> > >>
> > >> The problem only happens when '--sector-size 4096' is specified, with the default sector size is always works. It does not call loop_set_block_size() in the default case I guess.
> > >>
> > >> The loop0 device has certainly be used by other testcases before, most likely with the default block size. But at the time of this run, no loop device is currently active (losetup shows nothing). 
> > >>
> > >> Anyone have an idea what goes wrong here? 
> > > 
> > > It returns '-EAGAIN' to ask userspace to try again.
> > > 
> > > I understand loop_set_block_size() doesn't prevent page cache of this
> > > loop disk from being dirtied, so it isn't strange to
> > > see lo_device->bd_inode->i_mapping->nrpages isn't zero after sync_blockdev()
> > > & invalidate_bdev() on loop.
> > > 
> > 
> > OK, that makes sense from the kernel perspective. 
> 
> We might improve this code by holding ->i_rwsem / mapping->invalidate_lock in
> loop_set_block_size() to prevent new dirtying pages, but this still
> can't guarantee that i_mapping->nrpages can become 0 after sync &
> revalidate bdev. Or maybe replace invalidate_bdev() with truncate_bdev_range().

i_rwsem won't be enough because even racing with reads into bdev page cache
(which is what I suspect is happening here) will cause EAGAIN error and
reads are not protected by i_rwsem. But after the invalidate_lock work
lands, we should have enough to implement atomic (wrt any page cache
operation) flush & invalidate sequence for a bdevs.

								Honza
-- 
Jan Kara <jack@suse.com>
SUSE Labs, CR

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: loop_set_block_size: loop0 () has still dirty pages (nrpages=2)
  2021-06-14  7:35   ` Ingo Franzki
  2021-06-14 22:37     ` Ming Lei
@ 2021-06-15  8:42     ` Karel Zak
  2021-06-16  7:22       ` Ingo Franzki
  1 sibling, 1 reply; 8+ messages in thread
From: Karel Zak @ 2021-06-15  8:42 UTC (permalink / raw)
  To: Ingo Franzki
  Cc: Ming Lei, Jens Axboe, linux-block, linux-kernel, Juergen Christ

On Mon, Jun 14, 2021 at 09:35:30AM +0200, Ingo Franzki wrote:
> However, shouldn't then the losetup userspace utility implement some kind of retry logic in case of -EAGAIN ?
> I don't see that in the source of losetup.c nor in loopdev.c in the util-linux package. There is a retry loop in create_loop() in losetup.c retrying loopcxt_setup_device() in case of EBUSY, but not in case of EAGAIN.
> 
> And losetup also hides the original error code and just returns EXIT_FAILURE in case of a failure. So no real good chance for the script that uses losetup to catch that error situation and perform a retry itself.
> 
> Adding Karel Zak (the maintainer of util-linux).
> 
> @Karel Zak: How about adding EAGAIN to the condition for performing a retry? 
> 
> Something like this:
> 
> -		if (errno == EBUSY && !hasdev && ntries < 64) {
> +		if ((errno == EBUSY || errno == EAGAIN) && !hasdev && ntries < 64) {
> 			xusleep(200000);
> 			ntries++;
> 			continue;
> 		}
 
EAGAIN sounds like the best reason to try it again :-) 

Committed, it will be also available in v2.37.1.

  Karel


-- 
 Karel Zak  <kzak@redhat.com>
 http://karelzak.blogspot.com


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: loop_set_block_size: loop0 () has still dirty pages (nrpages=2)
  2021-06-15  8:42     ` Karel Zak
@ 2021-06-16  7:22       ` Ingo Franzki
  2021-06-16  7:44         ` Karel Zak
  0 siblings, 1 reply; 8+ messages in thread
From: Ingo Franzki @ 2021-06-16  7:22 UTC (permalink / raw)
  To: Karel Zak; +Cc: Ming Lei, Jens Axboe, linux-block, linux-kernel, Juergen Christ

On 15.06.2021 10:42, Karel Zak wrote:
> On Mon, Jun 14, 2021 at 09:35:30AM +0200, Ingo Franzki wrote:
>> However, shouldn't then the losetup userspace utility implement some kind of retry logic in case of -EAGAIN ?
>> I don't see that in the source of losetup.c nor in loopdev.c in the util-linux package. There is a retry loop in create_loop() in losetup.c retrying loopcxt_setup_device() in case of EBUSY, but not in case of EAGAIN.
>>
>> And losetup also hides the original error code and just returns EXIT_FAILURE in case of a failure. So no real good chance for the script that uses losetup to catch that error situation and perform a retry itself.
>>
>> Adding Karel Zak (the maintainer of util-linux).
>>
>> @Karel Zak: How about adding EAGAIN to the condition for performing a retry? 
>>
>> Something like this:
>>
>> -		if (errno == EBUSY && !hasdev && ntries < 64) {
>> +		if ((errno == EBUSY || errno == EAGAIN) && !hasdev && ntries < 64) {
>> 			xusleep(200000);
>> 			ntries++;
>> 			continue;
>> 		}
>  
> EAGAIN sounds like the best reason to try it again :-) 
> 
> Committed, it will be also available in v2.37.1.

Thanks a lot for the quick resolution!

Do you by any chance know if Fedora 34 will be updated with v2.37.1? 
I guess Fedora 35 will get it in any case.
> 
>   Karel
> 
> 


-- 
Ingo Franzki
eMail: ifranzki@linux.ibm.com  
Linux on IBM Z Development, Schoenaicher Str. 220, 71032 Boeblingen, Germany

IBM Deutschland Research & Development GmbH / Vorsitzender des Aufsichtsrats: Matthias Hartmann
Geschäftsführung: Dirk Wittkopp
Sitz der Gesellschaft: Böblingen / Registergericht: Amtsgericht Stuttgart, HRB 243294
IBM DATA Privacy Statement: https://www.ibm.com/privacy/us/en/

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: loop_set_block_size: loop0 () has still dirty pages (nrpages=2)
  2021-06-16  7:22       ` Ingo Franzki
@ 2021-06-16  7:44         ` Karel Zak
  0 siblings, 0 replies; 8+ messages in thread
From: Karel Zak @ 2021-06-16  7:44 UTC (permalink / raw)
  To: Ingo Franzki
  Cc: Ming Lei, Jens Axboe, linux-block, linux-kernel, Juergen Christ

On Wed, Jun 16, 2021 at 09:22:17AM +0200, Ingo Franzki wrote:
> On 15.06.2021 10:42, Karel Zak wrote:
> > On Mon, Jun 14, 2021 at 09:35:30AM +0200, Ingo Franzki wrote:
> >> However, shouldn't then the losetup userspace utility implement some kind of retry logic in case of -EAGAIN ?
> >> I don't see that in the source of losetup.c nor in loopdev.c in the util-linux package. There is a retry loop in create_loop() in losetup.c retrying loopcxt_setup_device() in case of EBUSY, but not in case of EAGAIN.
> >>
> >> And losetup also hides the original error code and just returns EXIT_FAILURE in case of a failure. So no real good chance for the script that uses losetup to catch that error situation and perform a retry itself.
> >>
> >> Adding Karel Zak (the maintainer of util-linux).
> >>
> >> @Karel Zak: How about adding EAGAIN to the condition for performing a retry? 
> >>
> >> Something like this:
> >>
> >> -		if (errno == EBUSY && !hasdev && ntries < 64) {
> >> +		if ((errno == EBUSY || errno == EAGAIN) && !hasdev && ntries < 64) {
> >> 			xusleep(200000);
> >> 			ntries++;
> >> 			continue;
> >> 		}
> >  
> > EAGAIN sounds like the best reason to try it again :-) 
> > 
> > Committed, it will be also available in v2.37.1.
> 
> Thanks a lot for the quick resolution!
> 
> Do you by any chance know if Fedora 34 will be updated with v2.37.1? 

I'd like to keep f34 based on v2.36.2, but I can backport the patch to f34.

 Karel


-- 
 Karel Zak  <kzak@redhat.com>
 http://karelzak.blogspot.com


^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2021-06-16  7:45 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-06-08 12:01 loop_set_block_size: loop0 () has still dirty pages (nrpages=2) Ingo Franzki
2021-06-10 14:45 ` Ming Lei
2021-06-14  7:35   ` Ingo Franzki
2021-06-14 22:37     ` Ming Lei
2021-06-15  8:06       ` Jan Kara
2021-06-15  8:42     ` Karel Zak
2021-06-16  7:22       ` Ingo Franzki
2021-06-16  7:44         ` Karel Zak

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).