Add a flag IORING_URING_CMD_REISSUE that ->uring_cmd() implementations
can use to tell whether this is the first or subsequent issue of the
uring_cmd. This will allow ->uring_cmd() implementations to store
information in the io_uring_cmd's pdu across issues.
Signed-off-by: Caleb Sander Mateos <csander@purestorage.com>
---
include/linux/io_uring/cmd.h | 2 ++
io_uring/uring_cmd.c | 2 ++
2 files changed, 4 insertions(+)
diff --git a/include/linux/io_uring/cmd.h b/include/linux/io_uring/cmd.h
index 53408124c1e5..29892f54e0ac 100644
--- a/include/linux/io_uring/cmd.h
+++ b/include/linux/io_uring/cmd.h
@@ -6,10 +6,12 @@
#include <linux/io_uring_types.h>
#include <linux/blk-mq.h>
/* only top 8 bits of sqe->uring_cmd_flags for kernel internal use */
#define IORING_URING_CMD_CANCELABLE (1U << 30)
+/* io_uring_cmd is being issued again */
+#define IORING_URING_CMD_REISSUE (1U << 31)
struct io_uring_cmd {
struct file *file;
const struct io_uring_sqe *sqe;
/* callback to defer completions to task context */
diff --git a/io_uring/uring_cmd.c b/io_uring/uring_cmd.c
index 929cad6ee326..7cddc4c1c554 100644
--- a/io_uring/uring_cmd.c
+++ b/io_uring/uring_cmd.c
@@ -257,10 +257,12 @@ int io_uring_cmd(struct io_kiocb *req, unsigned int issue_flags)
req->iopoll_start = ktime_get_ns();
}
}
ret = file->f_op->uring_cmd(ioucmd, issue_flags);
+ if (ret == -EAGAIN)
+ ioucmd->flags |= IORING_URING_CMD_REISSUE;
if (ret == -EAGAIN || ret == -EIOCBQUEUED)
return ret;
if (ret < 0)
req_set_fail(req);
io_req_uring_cleanup(req, issue_flags);
--
2.45.2On 6/19/25 1:27 PM, Caleb Sander Mateos wrote:
> diff --git a/io_uring/uring_cmd.c b/io_uring/uring_cmd.c
> index 929cad6ee326..7cddc4c1c554 100644
> --- a/io_uring/uring_cmd.c
> +++ b/io_uring/uring_cmd.c
> @@ -257,10 +257,12 @@ int io_uring_cmd(struct io_kiocb *req, unsigned int issue_flags)
> req->iopoll_start = ktime_get_ns();
> }
> }
>
> ret = file->f_op->uring_cmd(ioucmd, issue_flags);
> + if (ret == -EAGAIN)
> + ioucmd->flags |= IORING_URING_CMD_REISSUE;
> if (ret == -EAGAIN || ret == -EIOCBQUEUED)
> return ret;
Probably fold that under the next statement?
if (ret == -EAGAIN || ret == -EIOCBQUEUED) {
if (ret == -EAGAIN) {
ioucmd->flags |= IORING_URING_CMD_REISSUE;
return ret;
}
?
--
Jens Axboe
On Tue, 1 Jul 2025 at 21:04, Jens Axboe <axboe@kernel.dk> wrote:
>
> On 6/19/25 1:27 PM, Caleb Sander Mateos wrote:
> > diff --git a/io_uring/uring_cmd.c b/io_uring/uring_cmd.c
> > index 929cad6ee326..7cddc4c1c554 100644
> > --- a/io_uring/uring_cmd.c
> > +++ b/io_uring/uring_cmd.c
> > @@ -257,10 +257,12 @@ int io_uring_cmd(struct io_kiocb *req, unsigned int issue_flags)
> > req->iopoll_start = ktime_get_ns();
> > }
> > }
> >
> > ret = file->f_op->uring_cmd(ioucmd, issue_flags);
> > + if (ret == -EAGAIN)
> > + ioucmd->flags |= IORING_URING_CMD_REISSUE;
> > if (ret == -EAGAIN || ret == -EIOCBQUEUED)
> > return ret;
>
> Probably fold that under the next statement?
>
> if (ret == -EAGAIN || ret == -EIOCBQUEUED) {
> if (ret == -EAGAIN) {
> ioucmd->flags |= IORING_URING_CMD_REISSUE;
> return ret;
> }
>
> ?
I'd argue the original looks simpler, cleaner.
> --
> Jens Axboe
>
On 7/2/25 1:27 PM, Daniel Vacek wrote:
> On Tue, 1 Jul 2025 at 21:04, Jens Axboe <axboe@kernel.dk> wrote:
>> Probably fold that under the next statement?
>>
>> if (ret == -EAGAIN || ret == -EIOCBQUEUED) {
>> if (ret == -EAGAIN) {
>> ioucmd->flags |= IORING_URING_CMD_REISSUE;
>> return ret;
>> }
>>
>> ?
>
> I'd argue the original looks simpler, cleaner.
I propose doing it this way:
if (ret == -EAGAIN) {
ioucmd->flags |= IORING_URING_CMD_REISSUE;
return ret;
}
if (ret == -EIOCBQUEUED)
return ret;
It's simpler because the -EAGAIN is only checked once :)
--
Ammar Faizi
On 7/2/25 12:44 AM, Ammar Faizi wrote:
> On 7/2/25 1:27 PM, Daniel Vacek wrote:
>> On Tue, 1 Jul 2025 at 21:04, Jens Axboe <axboe@kernel.dk> wrote:
>>> Probably fold that under the next statement?
>>>
>>> if (ret == -EAGAIN || ret == -EIOCBQUEUED) {
>>> if (ret == -EAGAIN) {
>>> ioucmd->flags |= IORING_URING_CMD_REISSUE;
>>> return ret;
>>> }
>>>
>>> ?
>>
>> I'd argue the original looks simpler, cleaner.
>
> I propose doing it this way:
>
> if (ret == -EAGAIN) {
> ioucmd->flags |= IORING_URING_CMD_REISSUE;
> return ret;
> }
>
> if (ret == -EIOCBQUEUED)
> return ret;
>
> It's simpler because the -EAGAIN is only checked once :)
Mine was mostly done for code generation reasons, though probably
the compiler is smart enough. I did consider yours as well, it's
more readable. However I'd then write it as:
if (ret == -EAGAIN) {
ioucmd->flags |= IORING_URING_CMD_REISSUE;
return -EAGAIN;
} else if (ret == -EIOCBQUEUED) {
return -EIOCBQUEUED;
}
But we're obviously nitpicking now. The bigger question as posed in
another patch in this series is whether we need IORING_URING_CMD_REISSUE
at all in the first place.
--
Jens Axboe
On Jul 2, 2025, at 14:44, Ammar Faizi <ammarfaizi2@gnuweeb.org> wrote:
>
> On 7/2/25 1:27 PM, Daniel Vacek wrote:
>> On Tue, 1 Jul 2025 at 21:04, Jens Axboe <axboe@kernel.dk> wrote:
>>> Probably fold that under the next statement?
>>>
>>> if (ret == -EAGAIN || ret == -EIOCBQUEUED) {
>>> if (ret == -EAGAIN) {
>>> ioucmd->flags |= IORING_URING_CMD_REISSUE;
>>> return ret;
>>> }
>>>
>>> ?
>> I'd argue the original looks simpler, cleaner.
>
> I propose doing it this way:
>
> if (ret == -EAGAIN) {
> ioucmd->flags |= IORING_URING_CMD_REISSUE;
> return ret;
> }
>
> if (ret == -EIOCBQUEUED)
> return ret;
>
> It's simpler because the -EAGAIN is only checked once :)
Agreed
>
> --
> Ammar Faizi
>
© 2016 - 2025 Red Hat, Inc.