FAILED: patch "[PATCH] io_uring: allow retry for O_NONBLOCK if async is supported" failed to apply to 5.14-stable tree

PATCH [email protected] 2 participants 5 messages Sep 18, 2021

Subsystems (2)

Stable branch VFS

lore

Share & tools

lore

[email protected]

author maintainer

2021-09-18 12:34 UTC #0 lore raw

The patch below does not apply to the 5.14-stable tree.
If someone wants it applied there, or to any other stable or longterm tree, then please email the backport, including the original git commit id to <[email protected]>.

thanks,

greg k-h

------------------ original commit in Linus's tree ------------------

From 5d329e1286b0a040264e239b80257c937f6e685f Mon Sep 17 00:00:00 2001
From: Jens Axboe <[email protected]>
Date: Tue, 14 Sep 2021 11:08:37 -0600
Subject: [PATCH] io_uring: allow retry for O_NONBLOCK if async is supported

A common complaint is that using O_NONBLOCK files with io_uring can be a bit of a pain. Be a bit nicer and allow normal retry IFF the file does support async behavior. This makes it possible to use io_uring more reliably with O_NONBLOCK files, for use cases where it either isn't possible or feasible to modify the file flags.

Cc [email protected]

Reported-and-tested-by Dan Melnic <[email protected]>

Signed-off-by Jens Axboe <[email protected]>

fs/io_uring.c +11 -5

index ae489ab275dd..3077f85a2638 100644
@@ -2843,7 +2843,8 @@ static bool io_file_supports_nowait(struct io_kiocb *req, int rw)
 	return __io_file_supports_nowait(req->file, rw);
 }
 
-static int io_prep_rw(struct io_kiocb *req, const struct io_uring_sqe *sqe)
+static int io_prep_rw(struct io_kiocb *req, const struct io_uring_sqe *sqe,
+		      int rw)
 {
 	struct io_ring_ctx *ctx = req->ctx;
 	struct kiocb *kiocb = &req->rw.kiocb;

@@ -2865,8 +2866,13 @@ static int io_prep_rw(struct io_kiocb *req, const struct io_uring_sqe *sqe)
 	if (unlikely(ret))
 		return ret;
 
-	/* don't allow async punt for O_NONBLOCK or RWF_NOWAIT */
-	if ((kiocb->ki_flags & IOCB_NOWAIT) || (file->f_flags & O_NONBLOCK))
+	/*
+	 * If the file is marked O_NONBLOCK, still allow retry for it if it
+	 * supports async. Otherwise it's impossible to use O_NONBLOCK files
+	 * reliably. If not, or it IOCB_NOWAIT is set, don't retry.
+	 */
+	if ((kiocb->ki_flags & IOCB_NOWAIT) ||
+	    ((file->f_flags & O_NONBLOCK) && !io_file_supports_nowait(req, rw)))
 		req->flags |= REQ_F_NOWAIT;
 
 	ioprio = READ_ONCE(sqe->ioprio);

@@ -3349,7 +3355,7 @@ static int io_read_prep(struct io_kiocb *req, const struct io_uring_sqe *sqe)
 {
 	if (unlikely(!(req->file->f_mode & FMODE_READ)))
 		return -EBADF;
-	return io_prep_rw(req, sqe);
+	return io_prep_rw(req, sqe, READ);
 }
 
 /*

@@ -3542,7 +3548,7 @@ static int io_write_prep(struct io_kiocb *req, const struct io_uring_sqe *sqe)
 {
 	if (unlikely(!(req->file->f_mode & FMODE_WRITE)))
 		return -EBADF;
-	return io_prep_rw(req, sqe);
+	return io_prep_rw(req, sqe, WRITE);
 }
 
 static int io_write(struct io_kiocb *req, unsigned int issue_flags)

Jens Axboe

2021-09-18 12:44 UTC #1 lore raw

Quoted 5 lines · On 9/18/21 6:34 AM, [email protected] wrote:

The patch below does not apply to the 5.14-stable tree.
If someone wants it applied there, or to any other stable or longterm tree, then please email the backport, including the original git commit id to <[email protected]>.

Just a trivial fuzz, here's a fixed version.

Signature · 1 line

Jens Axboe

commit 5d329e1286b0a040264e239b80257c937f6e685f upstream.

Cc [email protected]

Reported-and-tested-by Dan Melnic <[email protected]>

Signed-off-by Jens Axboe <[email protected]>

---
 fs/io_uring.c | 16 +++++++++++-----
 1 file changed, 11 insertions(+), 5 deletions(-)

fs/io_uring.c +11 -5

index ae489ab275dd..3077f85a2638 100644
@@ -2843,7 +2843,8 @@ static bool io_file_supports_nowait(struct io_kiocb *req, int rw)
 	return __io_file_supports_async(req->file, rw);
 }
 
-static int io_prep_rw(struct io_kiocb *req, const struct io_uring_sqe *sqe)
+static int io_prep_rw(struct io_kiocb *req, const struct io_uring_sqe *sqe,
+		      int rw)
 {
 	struct io_ring_ctx *ctx = req->ctx;
 	struct kiocb *kiocb = &req->rw.kiocb;

@@ -2865,8 +2866,13 @@ static int io_prep_rw(struct io_kiocb *req, const struct io_uring_sqe *sqe)
 	if (unlikely(ret))
 		return ret;
 
-	/* don't allow async punt for O_NONBLOCK or RWF_NOWAIT */
-	if ((kiocb->ki_flags & IOCB_NOWAIT) || (file->f_flags & O_NONBLOCK))
+	/*
+	 * If the file is marked O_NONBLOCK, still allow retry for it if it
+	 * supports async. Otherwise it's impossible to use O_NONBLOCK files
+	 * reliably. If not, or it IOCB_NOWAIT is set, don't retry.
+	 */
+	if ((kiocb->ki_flags & IOCB_NOWAIT) ||
+	    ((file->f_flags & O_NONBLOCK) && !io_file_supports_nowait(req, rw)))
 		req->flags |= REQ_F_NOWAIT;
 
 	ioprio = READ_ONCE(sqe->ioprio);

@@ -3349,7 +3355,7 @@ static int io_read_prep(struct io_kiocb *req, const struct io_uring_sqe *sqe)
 {
 	if (unlikely(!(req->file->f_mode & FMODE_READ)))
 		return -EBADF;
-	return io_prep_rw(req, sqe);
+	return io_prep_rw(req, sqe, READ);
 }
 
 /*

@@ -3542,7 +3548,7 @@ static int io_write_prep(struct io_kiocb *req, const struct io_uring_sqe *sqe)
 {
 	if (unlikely(!(req->file->f_mode & FMODE_WRITE)))
 		return -EBADF;
-	return io_prep_rw(req, sqe);
+	return io_prep_rw(req, sqe, WRITE);
 }
 
 static int io_write(struct io_kiocb *req, unsigned int issue_flags)

Signature · 1 line

2.33.0

Greg KH

author maintainer

2021-09-18 12:47 UTC #2 lore raw

Quoted 8 lines · Jens Axboe

Quoted 5 lines · On 9/18/21 6:34 AM, [email protected] wrote:
The patch below does not apply to the 5.14-stable tree.
If someone wants it applied there, or to any other stable or longterm tree, then please email the backport, including the original git commit id to <[email protected]>.
Just a trivial fuzz, here's a fixed version.

Sorry, no, the fuzz was easy, this breaks the build:

fs/io_uring.c: In function ‘io_prep_rw’:
fs/io_uring.c:2715:47: error: implicit declaration of function ‘io_file_supports_nowait’; did you mean ‘io_file_supports_async’? [-Werror=implicit-function-declaration]

 2715 |             ((file->f_flags & O_NONBLOCK) && !io_file_supports_nowait(req, rw)))
      |                                               ^~~~~~~~~~~~~~~~~~~~~~~
      |                                               io_file_supports_async

CHK kernel/kheaders_data.tar.xz
cc1: some warnings being treated as errors

Any hint as to what that function would be in 5.14.y?

thanks,

greg k-h

Jens Axboe

2021-09-18 12:49 UTC #3 lore raw

Quoted 14 lines · Greg KH

Quoted 8 lines · Jens Axboe
Quoted 5 lines · On 9/18/21 6:34 AM, [email protected] wrote:
The patch below does not apply to the 5.14-stable tree.
If someone wants it applied there, or to any other stable or longterm tree, then please email the backport, including the original git commit id to <[email protected]>.
Just a trivial fuzz, here's a fixed version.
Sorry, no, the fuzz was easy, this breaks the build:
fs/io_uring.c: In function ‘io_prep_rw’:
fs/io_uring.c:2715:47: error: implicit declaration of function ‘io_file_supports_nowait’; did you mean ‘io_file_supports_async’? [-Werror=implicit-function-declaration]

Quoted 8 lines

 2715 |             ((file->f_flags & O_NONBLOCK) && !io_file_supports_nowait(req, rw)))
      |                                               ^~~~~~~~~~~~~~~~~~~~~~~
      |                                               io_file_supports_async
CHK kernel/kheaders_data.tar.xz
cc1: some warnings being treated as errors
Any hint as to what that function would be in 5.14.y?

Ah sorry I'm an idiot, it should be that io_file_supports_nowait() that also caused the fuzz in applying.

Signature · 1 line

Jens Axboe

From 71e536f57c2f4309de4c2d5ae352eb830cdf776c Mon Sep 17 00:00:00 2001
From: Jens Axboe <[email protected]>
Date: Tue, 14 Sep 2021 11:08:37 -0600
Subject: [PATCH] io_uring: allow retry for O_NONBLOCK if async is supported

commit 5d329e1286b0a040264e239b80257c937f6e685f upstream.

Cc [email protected]

Reported-and-tested-by Dan Melnic <[email protected]>

Signed-off-by Jens Axboe <[email protected]>

---
 fs/io_uring.c | 16 +++++++++++-----
 1 file changed, 11 insertions(+), 5 deletions(-)

fs/io_uring.c +11 -5

index c5d4638f6d7f..483cc024a40f 100644
@@ -2683,7 +2683,8 @@ static bool io_file_supports_async(struct io_kiocb *req, int rw)
 	return __io_file_supports_async(req->file, rw);
 }
 
-static int io_prep_rw(struct io_kiocb *req, const struct io_uring_sqe *sqe)
+static int io_prep_rw(struct io_kiocb *req, const struct io_uring_sqe *sqe,
+		      int rw)
 {
 	struct io_ring_ctx *ctx = req->ctx;
 	struct kiocb *kiocb = &req->rw.kiocb;

@@ -2705,8 +2706,13 @@ static int io_prep_rw(struct io_kiocb *req, const struct io_uring_sqe *sqe)
 	if (unlikely(ret))
 		return ret;
 
-	/* don't allow async punt for O_NONBLOCK or RWF_NOWAIT */
-	if ((kiocb->ki_flags & IOCB_NOWAIT) || (file->f_flags & O_NONBLOCK))
+	/*
+	 * If the file is marked O_NONBLOCK, still allow retry for it if it
+	 * supports async. Otherwise it's impossible to use O_NONBLOCK files
+	 * reliably. If not, or it IOCB_NOWAIT is set, don't retry.
+	 */
+	if ((kiocb->ki_flags & IOCB_NOWAIT) ||
+	    ((file->f_flags & O_NONBLOCK) && !io_file_supports_async(req, rw)))
 		req->flags |= REQ_F_NOWAIT;
 
 	ioprio = READ_ONCE(sqe->ioprio);

@@ -3190,7 +3196,7 @@ static int io_read_prep(struct io_kiocb *req, const struct io_uring_sqe *sqe)
 {
 	if (unlikely(!(req->file->f_mode & FMODE_READ)))
 		return -EBADF;
-	return io_prep_rw(req, sqe);
+	return io_prep_rw(req, sqe, READ);
 }
 
 /*

@@ -3379,7 +3385,7 @@ static int io_write_prep(struct io_kiocb *req, const struct io_uring_sqe *sqe)
 {
 	if (unlikely(!(req->file->f_mode & FMODE_WRITE)))
 		return -EBADF;
-	return io_prep_rw(req, sqe);
+	return io_prep_rw(req, sqe, WRITE);
 }
 
 static int io_write(struct io_kiocb *req, unsigned int issue_flags)

Signature · 1 line

2.33.0

Greg KH

author maintainer

2021-09-18 12:59 UTC #4 lore raw

Quoted 22 lines · Jens Axboe

Quoted 21 lines · Greg KH
Quoted 8 lines · Jens Axboe
Quoted 5 lines · On 9/18/21 6:34 AM, [email protected] wrote:
The patch below does not apply to the 5.14-stable tree.
If someone wants it applied there, or to any other stable or longterm tree, then please email the backport, including the original git commit id to <[email protected]>.
Just a trivial fuzz, here's a fixed version.
Sorry, no, the fuzz was easy, this breaks the build:
fs/io_uring.c: In function ‘io_prep_rw’:
fs/io_uring.c:2715:47: error: implicit declaration of function ‘io_file_supports_nowait’; did you mean ‘io_file_supports_async’? [-Werror=implicit-function-declaration]
 2715 |             ((file->f_flags & O_NONBLOCK) && !io_file_supports_nowait(req, rw)))
      |                                               ^~~~~~~~~~~~~~~~~~~~~~~
      |                                               io_file_supports_async
CHK kernel/kheaders_data.tar.xz
cc1: some warnings being treated as errors

Quoted 4 lines

Quoted 1 line
Any hint as to what that function would be in 5.14.y?
Ah sorry I'm an idiot, it should be that io_file_supports_nowait() that also caused the fuzz in applying.

This version worked, thanks!

greg k-h