2022-04-22 20:44:11

by Dylan Yudaken

[permalink] [raw]
Subject: [PATCH 0/6] return an error when cqe is dropped

This series addresses a rare but real error condition when a CQE is
dropped. Many applications rely on 1 SQE resulting in 1 CQE and may even
block waiting for the CQE. In overflow conditions if the GFP_ATOMIC
allocation fails, the CQE is dropped and a counter is incremented. However
the application is not actively signalled that something bad has
happened. We would like to indicate this error condition to the
application but in a way that does not rely on the application doing
invasive changes such as checking a flag before each wait.

This series returns an error code to the application when the error hits,
and then resets the error condition. If the application is ok with this
error it can continue as is, or more likely it can clean up sanely.

Patches 1&2 add tracing for overflows
Patches 3&4 prep for adding this error
Patch 5 is the main one returning an error
Patch 6 allows liburing to test these conditions more easily with IOPOLL

Dylan Yudaken (6):
io_uring: add trace support for CQE overflow
io_uring: trace cqe overflows
io_uring: rework io_uring_enter to simplify return value
io_uring: use constants for cq_overflow bitfield
io_uring: return an error when cqe is dropped
io_uring: allow NOP opcode in IOPOLL mode

fs/io_uring.c | 89 ++++++++++++++++++++++-----------
include/trace/events/io_uring.h | 42 +++++++++++++++-
2 files changed, 102 insertions(+), 29 deletions(-)


base-commit: 7c648b7d6186c59ed3a0e0ae4b774aaf4b415ef2
--
2.30.2


2022-04-22 20:54:04

by Dylan Yudaken

[permalink] [raw]
Subject: [PATCH 6/6] io_uring: allow NOP opcode in IOPOLL mode

This is useful for tests so that IOPOLL can be tested without requiring
files. NOP is acceptable in IOPOLL as it always completes immediately.

Signed-off-by: Dylan Yudaken <[email protected]>
---
fs/io_uring.c | 5 -----
1 file changed, 5 deletions(-)

diff --git a/fs/io_uring.c b/fs/io_uring.c
index e46dc67c917c..a4e42ba708b4 100644
--- a/fs/io_uring.c
+++ b/fs/io_uring.c
@@ -4526,11 +4526,6 @@ static int io_splice(struct io_kiocb *req, unsigned int issue_flags)
*/
static int io_nop(struct io_kiocb *req, unsigned int issue_flags)
{
- struct io_ring_ctx *ctx = req->ctx;
-
- if (unlikely(ctx->flags & IORING_SETUP_IOPOLL))
- return -EINVAL;
-
__io_req_complete(req, issue_flags, 0, 0);
return 0;
}
--
2.30.2

2022-04-22 21:51:45

by Jens Axboe

[permalink] [raw]
Subject: Re: [PATCH 6/6] io_uring: allow NOP opcode in IOPOLL mode

On Thu, Apr 21, 2022 at 3:17 AM Dylan Yudaken <[email protected]> wrote:
>
> This is useful for tests so that IOPOLL can be tested without requiring
> files. NOP is acceptable in IOPOLL as it always completes immediately.

This one actually breaks two liburing test cases (link and defer) that
assume NOP on IOPOLL will return -EINVAL. Not a huge deal, but we do
need to figure out how to make them reliably -EINVAL in a different
way then.

Maybe add a nop_flags to the usual flags spot in the sqe, and define
a flag that says NOP_IOPOLL or something. Require this flag set for
allowing NOP on iopoll. That'd allow testing, but still retain the
-EINVAL behavior if not set.

Alternatively, modify test cases...

I'll drop this one for now, just because it fails the regression
tests.

--
Jens Axboe

2022-04-22 22:06:39

by Dylan Yudaken

[permalink] [raw]
Subject: Re: [PATCH 6/6] io_uring: allow NOP opcode in IOPOLL mode

On Thu, 2022-04-21 at 17:33 -0600, Jens Axboe wrote:
> On Thu, Apr 21, 2022 at 3:17 AM Dylan Yudaken <[email protected]> wrote:
> >
> > This is useful for tests so that IOPOLL can be tested without
> > requiring
> > files. NOP is acceptable in IOPOLL as it always completes
> > immediately.
>
> This one actually breaks two liburing test cases (link and defer)
> that
> assume NOP on IOPOLL will return -EINVAL. Not a huge deal, but we do
> need to figure out how to make them reliably -EINVAL in a different
> way then.
>
> Maybe add a nop_flags to the usual flags spot in the sqe, and define
> a flag that says NOP_IOPOLL or something. Require this flag set for
> allowing NOP on iopoll. That'd allow testing, but still retain the
> -EINVAL behavior if not set.
>
> Alternatively, modify test cases...
>
> I'll drop this one for now, just because it fails the regression
> tests.
>

That's fine - sorry I didn't notice that. I think fixing the tests is
the better approach here. It should be easy to get an -EINVAL from it.