Util-Linux Archive on lore.kernel.org
 help / color / Atom feed
* [PATCH] lib/loopdev.c: Retry LOOP_SET_STATUS64 on EAGAIN
@ 2019-05-03  8:28 Romain Izard
  2019-05-03 10:42 ` Karel Zak
  0 siblings, 1 reply; 3+ messages in thread
From: Romain Izard @ 2019-05-03  8:28 UTC (permalink / raw)
  To: Karel Zak; +Cc: util-linux, linux-block, Romain Izard

A recent bugfix in the Linux kernel made it possible for the
LOOP_SET_STATUS64 ioctl to fail when called with a non-null offset,
with an EAGAIN errno:

5db470e229e2 loop: drop caches if offset or block_size are changed

This fix changes a silent failure (where mount could sometimes access
the backing loop image through the cache without the specified offset)
to an explicit failure, and it has also been backported on stable
branches.

On a 5.0 kernel, other changes to the loop driver make it hard to get
generate the EAGAIN error, but this bugfix has also been backported to
stables branches, without these changes. At least with the 4.14 stable
branch, the EAGAIN error can be quickly generated with the following loop:

while mount -o loop,offset=239 disk point && umount point; do :; done

Retry the ioctl when it fails with EAGAIN, which means that mount or
losetup will eventually succeed when encountering this case.

Signed-off-by: Romain Izard <romain.izard.pro@gmail.com>
---
 lib/loopdev.c | 14 ++++++++++----
 1 file changed, 10 insertions(+), 4 deletions(-)

diff --git a/lib/loopdev.c b/lib/loopdev.c
index 5d2e95b7e..88436713e 100644
--- a/lib/loopdev.c
+++ b/lib/loopdev.c
@@ -1275,7 +1275,7 @@ static int loopcxt_check_size(struct loopdev_cxt *lc, int file_fd)
  */
 int loopcxt_setup_device(struct loopdev_cxt *lc)
 {
-	int file_fd, dev_fd, mode = O_RDWR, rc = -1, cnt = 0;
+	int file_fd, dev_fd, mode = O_RDWR, rc = -1, cnt = 0, err;
 	int errsv = 0;
 
 	if (!lc || !*lc->device || !lc->filename)
@@ -1354,7 +1354,10 @@ int loopcxt_setup_device(struct loopdev_cxt *lc)
 		goto err;
 	}
 
-	if (ioctl(dev_fd, LOOP_SET_STATUS64, &lc->info)) {
+	do {
+		err = ioctl(dev_fd, LOOP_SET_STATUS64, &lc->info);
+	} while (err && errno == EAGAIN);
+	if (err) {
 		rc = -errno;
 		errsv = errno;
 		DBG(SETUP, ul_debugobj(lc, "LOOP_SET_STATUS64 failed: %m"));
@@ -1399,7 +1402,7 @@ err:
  */
 int loopcxt_ioctl_status(struct loopdev_cxt *lc)
 {
-	int dev_fd, rc = -1;
+	int dev_fd, rc = -1, err;
 
 	errno = 0;
 	dev_fd = loopcxt_get_fd(lc);
@@ -1410,7 +1413,10 @@ int loopcxt_ioctl_status(struct loopdev_cxt *lc)
 	}
 	DBG(SETUP, ul_debugobj(lc, "device open: OK"));
 
-	if (ioctl(dev_fd, LOOP_SET_STATUS64, &lc->info)) {
+	do {
+		err = ioctl(dev_fd, LOOP_SET_STATUS64, &lc->info);
+	} while (err && errno == EAGAIN);
+	if (err) {
 		rc = -errno;
 		DBG(SETUP, ul_debugobj(lc, "LOOP_SET_STATUS64 failed: %m"));
 		return rc;
-- 
2.17.1


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

* Re: [PATCH] lib/loopdev.c: Retry LOOP_SET_STATUS64 on EAGAIN
  2019-05-03  8:28 [PATCH] lib/loopdev.c: Retry LOOP_SET_STATUS64 on EAGAIN Romain Izard
@ 2019-05-03 10:42 ` Karel Zak
  2019-05-03 12:10   ` Romain Izard
  0 siblings, 1 reply; 3+ messages in thread
From: Karel Zak @ 2019-05-03 10:42 UTC (permalink / raw)
  To: Romain Izard; +Cc: util-linux, linux-block

On Fri, May 03, 2019 at 10:28:19AM +0200, Romain Izard wrote:
> +	do {
> +		err = ioctl(dev_fd, LOOP_SET_STATUS64, &lc->info);
> +	} while (err && errno == EAGAIN);

 Would be better to use any delay in the loop? For example after
 EAGAIN read/write we use usleep(250000).

    Karel

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

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

* Re: [PATCH] lib/loopdev.c: Retry LOOP_SET_STATUS64 on EAGAIN
  2019-05-03 10:42 ` Karel Zak
@ 2019-05-03 12:10   ` Romain Izard
  0 siblings, 0 replies; 3+ messages in thread
From: Romain Izard @ 2019-05-03 12:10 UTC (permalink / raw)
  To: Karel Zak; +Cc: util-linux, linux-block

Le ven. 3 mai 2019 à 12:42, Karel Zak <kzak@redhat.com> a écrit :
>
> On Fri, May 03, 2019 at 10:28:19AM +0200, Romain Izard wrote:
> > +     do {
> > +             err = ioctl(dev_fd, LOOP_SET_STATUS64, &lc->info);
> > +     } while (err && errno == EAGAIN);
>
>  Would be better to use any delay in the loop? For example after
>  EAGAIN read/write we use usleep(250000).

As we are waiting for the kernel to do some cleanup, this is a good idea.
It seems to reduce the need for retries in my use case.

I will add this in a v2.

Best regards,
-- 
Romain Izard

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

end of thread, back to index

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-05-03  8:28 [PATCH] lib/loopdev.c: Retry LOOP_SET_STATUS64 on EAGAIN Romain Izard
2019-05-03 10:42 ` Karel Zak
2019-05-03 12:10   ` Romain Izard

Util-Linux Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/util-linux/0 util-linux/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 util-linux util-linux/ https://lore.kernel.org/util-linux \
		util-linux@vger.kernel.org util-linux@archiver.kernel.org
	public-inbox-index util-linux


Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.util-linux


AGPL code for this site: git clone https://public-inbox.org/ public-inbox