* [PATCH 1/3] io_uring: have cancelation API accept io_uring_task directly
2022-06-19 2:07 [PATCHSET RFC for-next 0/3] Add io_uring_register() based cancel Jens Axboe
@ 2022-06-19 2:07 ` Jens Axboe
2022-06-19 2:07 ` [PATCH 2/3] io_uring: add IORING_ASYNC_CANCEL_FD_FIXED cancel flag Jens Axboe
2022-06-19 2:07 ` [PATCH 3/3] io_uring: add sync cancelation API through io_uring_register() Jens Axboe
2 siblings, 0 replies; 6+ messages in thread
From: Jens Axboe @ 2022-06-19 2:07 UTC (permalink / raw)
To: io-uring; +Cc: asml.silence, carter.li, Jens Axboe
We just use the io_kiocb passed in to find the io_uring_task, and we
already pass in the ctx via cd->ctx anyway.
Signed-off-by: Jens Axboe <[email protected]>
---
io_uring/cancel.c | 17 +++++++++--------
io_uring/cancel.h | 2 +-
io_uring/timeout.c | 2 +-
3 files changed, 11 insertions(+), 10 deletions(-)
diff --git a/io_uring/cancel.c b/io_uring/cancel.c
index d1e7f5a955ab..500ee5f5fd23 100644
--- a/io_uring/cancel.c
+++ b/io_uring/cancel.c
@@ -77,15 +77,15 @@ static int io_async_cancel_one(struct io_uring_task *tctx,
return ret;
}
-int io_try_cancel(struct io_kiocb *req, struct io_cancel_data *cd,
+int io_try_cancel(struct io_uring_task *tctx, struct io_cancel_data *cd,
unsigned issue_flags)
{
- struct io_ring_ctx *ctx = req->ctx;
+ struct io_ring_ctx *ctx = cd->ctx;
int ret;
- WARN_ON_ONCE(!io_wq_current_is_worker() && req->task != current);
+ WARN_ON_ONCE(!io_wq_current_is_worker() && tctx != current->io_uring);
- ret = io_async_cancel_one(req->task->io_uring, cd);
+ ret = io_async_cancel_one(tctx, cd);
/*
* Fall-through even for -EALREADY, as we may have poll armed
* that need unarming.
@@ -104,7 +104,6 @@ int io_try_cancel(struct io_kiocb *req, struct io_cancel_data *cd,
return ret;
}
-
int io_async_cancel_prep(struct io_kiocb *req, const struct io_uring_sqe *sqe)
{
struct io_cancel *cancel = io_kiocb_to_cmd(req);
@@ -127,7 +126,8 @@ int io_async_cancel_prep(struct io_kiocb *req, const struct io_uring_sqe *sqe)
return 0;
}
-static int __io_async_cancel(struct io_cancel_data *cd, struct io_kiocb *req,
+static int __io_async_cancel(struct io_cancel_data *cd,
+ struct io_uring_task *tctx,
unsigned int issue_flags)
{
bool all = cd->flags & (IORING_ASYNC_CANCEL_ALL|IORING_ASYNC_CANCEL_ANY);
@@ -136,7 +136,7 @@ static int __io_async_cancel(struct io_cancel_data *cd, struct io_kiocb *req,
int ret, nr = 0;
do {
- ret = io_try_cancel(req, cd, issue_flags);
+ ret = io_try_cancel(tctx, cd, issue_flags);
if (ret == -ENOENT)
break;
if (!all)
@@ -170,6 +170,7 @@ int io_async_cancel(struct io_kiocb *req, unsigned int issue_flags)
.flags = cancel->flags,
.seq = atomic_inc_return(&req->ctx->cancel_seq),
};
+ struct io_uring_task *tctx = req->task->io_uring;
int ret;
if (cd.flags & IORING_ASYNC_CANCEL_FD) {
@@ -185,7 +186,7 @@ int io_async_cancel(struct io_kiocb *req, unsigned int issue_flags)
cd.file = req->file;
}
- ret = __io_async_cancel(&cd, req, issue_flags);
+ ret = __io_async_cancel(&cd, tctx, issue_flags);
done:
if (ret < 0)
req_set_fail(req);
diff --git a/io_uring/cancel.h b/io_uring/cancel.h
index 2338012a5b06..1bc7e917ce94 100644
--- a/io_uring/cancel.h
+++ b/io_uring/cancel.h
@@ -16,6 +16,6 @@ struct io_cancel_data {
int io_async_cancel_prep(struct io_kiocb *req, const struct io_uring_sqe *sqe);
int io_async_cancel(struct io_kiocb *req, unsigned int issue_flags);
-int io_try_cancel(struct io_kiocb *req, struct io_cancel_data *cd,
+int io_try_cancel(struct io_uring_task *tctx, struct io_cancel_data *cd,
unsigned int issue_flags);
void init_hash_table(struct io_hash_table *table, unsigned size);
diff --git a/io_uring/timeout.c b/io_uring/timeout.c
index 557c637af158..a01480a9d29f 100644
--- a/io_uring/timeout.c
+++ b/io_uring/timeout.c
@@ -272,7 +272,7 @@ static void io_req_task_link_timeout(struct io_kiocb *req, bool *locked)
.data = prev->cqe.user_data,
};
- ret = io_try_cancel(req, &cd, issue_flags);
+ ret = io_try_cancel(req->task->io_uring, &cd, issue_flags);
}
io_req_set_res(req, ret ?: -ETIME, 0);
io_req_complete_post(req);
--
2.35.1
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH 2/3] io_uring: add IORING_ASYNC_CANCEL_FD_FIXED cancel flag
2022-06-19 2:07 [PATCHSET RFC for-next 0/3] Add io_uring_register() based cancel Jens Axboe
2022-06-19 2:07 ` [PATCH 1/3] io_uring: have cancelation API accept io_uring_task directly Jens Axboe
@ 2022-06-19 2:07 ` Jens Axboe
2022-06-19 2:07 ` [PATCH 3/3] io_uring: add sync cancelation API through io_uring_register() Jens Axboe
2 siblings, 0 replies; 6+ messages in thread
From: Jens Axboe @ 2022-06-19 2:07 UTC (permalink / raw)
To: io-uring; +Cc: asml.silence, carter.li, Jens Axboe
In preparation for not having a request to pass in that carries this
state, add a separate cancelation flag that allows the caller to ask
for a fixed file for cancelation.
Signed-off-by: Jens Axboe <[email protected]>
---
include/uapi/linux/io_uring.h | 2 ++
io_uring/cancel.c | 9 ++++++---
2 files changed, 8 insertions(+), 3 deletions(-)
diff --git a/include/uapi/linux/io_uring.h b/include/uapi/linux/io_uring.h
index 8715f0942ec2..d69dac9bb02c 100644
--- a/include/uapi/linux/io_uring.h
+++ b/include/uapi/linux/io_uring.h
@@ -244,10 +244,12 @@ enum io_uring_op {
* IORING_ASYNC_CANCEL_FD Key off 'fd' for cancelation rather than the
* request 'user_data'
* IORING_ASYNC_CANCEL_ANY Match any request
+ * IORING_ASYNC_CANCEL_FD_FIXED 'fd' passed in is a fixed descriptor
*/
#define IORING_ASYNC_CANCEL_ALL (1U << 0)
#define IORING_ASYNC_CANCEL_FD (1U << 1)
#define IORING_ASYNC_CANCEL_ANY (1U << 2)
+#define IORING_ASYNC_CANCEL_FD_FIXED (1U << 3)
/*
* send/sendmsg and recv/recvmsg flags (sqe->addr2)
diff --git a/io_uring/cancel.c b/io_uring/cancel.c
index 500ee5f5fd23..da486de07029 100644
--- a/io_uring/cancel.c
+++ b/io_uring/cancel.c
@@ -24,7 +24,7 @@ struct io_cancel {
};
#define CANCEL_FLAGS (IORING_ASYNC_CANCEL_ALL | IORING_ASYNC_CANCEL_FD | \
- IORING_ASYNC_CANCEL_ANY)
+ IORING_ASYNC_CANCEL_ANY | IORING_ASYNC_CANCEL_FD_FIXED)
static bool io_cancel_cb(struct io_wq_work *work, void *data)
{
@@ -174,11 +174,14 @@ int io_async_cancel(struct io_kiocb *req, unsigned int issue_flags)
int ret;
if (cd.flags & IORING_ASYNC_CANCEL_FD) {
- if (req->flags & REQ_F_FIXED_FILE)
+ if (req->flags & REQ_F_FIXED_FILE ||
+ cd.flags & IORING_ASYNC_CANCEL_FD_FIXED) {
+ req->flags |= REQ_F_FIXED_FILE;
req->file = io_file_get_fixed(req, cancel->fd,
issue_flags);
- else
+ } else {
req->file = io_file_get_normal(req, cancel->fd);
+ }
if (!req->file) {
ret = -EBADF;
goto done;
--
2.35.1
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH 3/3] io_uring: add sync cancelation API through io_uring_register()
2022-06-19 2:07 [PATCHSET RFC for-next 0/3] Add io_uring_register() based cancel Jens Axboe
2022-06-19 2:07 ` [PATCH 1/3] io_uring: have cancelation API accept io_uring_task directly Jens Axboe
2022-06-19 2:07 ` [PATCH 2/3] io_uring: add IORING_ASYNC_CANCEL_FD_FIXED cancel flag Jens Axboe
@ 2022-06-19 2:07 ` Jens Axboe
2022-06-19 11:16 ` Ammar Faizi
2 siblings, 1 reply; 6+ messages in thread
From: Jens Axboe @ 2022-06-19 2:07 UTC (permalink / raw)
To: io-uring; +Cc: asml.silence, carter.li, Jens Axboe
The io_uring cancelation API is async, like any other API that we expose
there. For the case of finding a request to cancel, or not finding one,
it is fully sync in that when submission returns, the CQE for both the
cancelation request and the targeted request have been posted to the
CQ ring.
However, if the targeted work is being executed by io-wq, the API can
only start the act of canceling it. This makes it difficult to use in
some circumstances, as the caller then has to wait for the CQEs to come
in and match on the same cancelation data there.
Provide a IORING_REGISTER_SYNC_CANCEL command for io_uring_register()
that does sync cancelations, always. For the io-wq case, it'll wait
for the cancelation to come in before returning. The only expected
returns from this API is:
0 Request found and canceled fine.
> 0 Requests found and canceled. Only happens if asked to
cancel multiple requests, and if the work wasn't in
progress.
-ENOENT Request not found.
-ETIME A timeout on the operation was requested, but the timeout
expired before we could cancel.
and we won't get -EALREADY via this API.
If the timeout value passed in is -1 (tv_sec and tv_nsec), then that
means that no timeout is requested. Otherwise, the timespec passed in
is the amount of time the sync cancel will wait for a successful
cancelation.
Link: https://github.com/axboe/liburing/discussions/608
Signed-off-by: Jens Axboe <[email protected]>
---
include/uapi/linux/io_uring.h | 15 +++++
io_uring/cancel.c | 107 ++++++++++++++++++++++++++++++++++
io_uring/cancel.h | 2 +
io_uring/io_uring.c | 6 ++
4 files changed, 130 insertions(+)
diff --git a/include/uapi/linux/io_uring.h b/include/uapi/linux/io_uring.h
index d69dac9bb02c..09e7c3b13d2d 100644
--- a/include/uapi/linux/io_uring.h
+++ b/include/uapi/linux/io_uring.h
@@ -10,6 +10,7 @@
#include <linux/fs.h>
#include <linux/types.h>
+#include <linux/time_types.h>
/*
* IO submission data structure (Submission Queue Entry)
@@ -425,6 +426,9 @@ enum {
IORING_REGISTER_PBUF_RING = 22,
IORING_UNREGISTER_PBUF_RING = 23,
+ /* sync cancelation API */
+ IORING_REGISTER_SYNC_CANCEL = 24,
+
/* this goes last */
IORING_REGISTER_LAST
};
@@ -560,4 +564,15 @@ struct io_uring_getevents_arg {
__u64 ts;
};
+/*
+ * Argument for IORING_REGISTER_SYNC_CANCEL
+ */
+struct io_uring_sync_cancel_reg {
+ __u64 addr;
+ __s32 fd;
+ __u32 flags;
+ struct __kernel_timespec timeout;
+ __u64 pad[4];
+};
+
#endif
diff --git a/io_uring/cancel.c b/io_uring/cancel.c
index da486de07029..78b5a3088ab3 100644
--- a/io_uring/cancel.c
+++ b/io_uring/cancel.c
@@ -6,6 +6,7 @@
#include <linux/mm.h>
#include <linux/slab.h>
#include <linux/namei.h>
+#include <linux/nospec.h>
#include <linux/io_uring.h>
#include <uapi/linux/io_uring.h>
@@ -206,3 +207,109 @@ void init_hash_table(struct io_hash_table *table, unsigned size)
INIT_HLIST_HEAD(&table->hbs[i].list);
}
}
+
+static int __io_sync_cancel(struct io_uring_task *tctx,
+ struct io_cancel_data *cd, int fd)
+{
+ struct io_ring_ctx *ctx = cd->ctx;
+
+ /* fixed must be grabbed every time since we drop the uring_lock */
+ if ((cd->flags & IORING_ASYNC_CANCEL_FD) &&
+ (cd->flags & IORING_ASYNC_CANCEL_FD_FIXED)) {
+ unsigned long file_ptr;
+
+ if (unlikely(fd > ctx->nr_user_files))
+ return -EBADF;
+ fd = array_index_nospec(fd, ctx->nr_user_files);
+ file_ptr = io_fixed_file_slot(&ctx->file_table, fd)->file_ptr;
+ cd->file = (struct file *) (file_ptr & FFS_MASK);
+ if (!cd->file)
+ return -EBADF;
+ }
+
+ return __io_async_cancel(cd, tctx, 0);
+}
+
+int io_sync_cancel(struct io_ring_ctx *ctx, void __user *arg)
+ __must_hold(&ctx->uring_lock)
+{
+ struct io_cancel_data cd = {
+ .ctx = ctx,
+ .seq = atomic_inc_return(&ctx->cancel_seq),
+ };
+ ktime_t timeout = KTIME_MAX;
+ struct io_uring_sync_cancel_reg sc;
+ struct fd f = { };
+ DEFINE_WAIT(wait);
+ int ret;
+
+ if (copy_from_user(&sc, arg, sizeof(sc)))
+ return -EFAULT;
+ if (sc.flags & ~CANCEL_FLAGS)
+ return -EINVAL;
+ if (sc.pad[0] || sc.pad[1] || sc.pad[2] || sc.pad[3])
+ return -EINVAL;
+
+ cd.data = sc.addr;
+ cd.flags = sc.flags;
+
+ /* we can grab a normal file descriptor upfront */
+ if ((cd.flags & IORING_ASYNC_CANCEL_FD) &&
+ !(cd.flags & IORING_ASYNC_CANCEL_FD_FIXED)) {
+ f = fdget(sc.fd);
+ if (!f.file)
+ return -EBADF;
+ cd.file = f.file;
+ }
+
+ ret = __io_sync_cancel(current->io_uring, &cd, sc.fd);
+
+ /* found something, done! */
+ if (ret != -EALREADY)
+ goto out;
+
+ if (sc.timeout.tv_sec != -1UL || sc.timeout.tv_nsec != -1UL) {
+ struct timespec64 ts = {
+ .tv_sec = sc.timeout.tv_sec,
+ .tv_nsec = sc.timeout.tv_nsec
+ };
+
+ timeout = ktime_add_ns(timespec64_to_ktime(ts), ktime_get_ns());
+ }
+
+ /*
+ * Keep looking until we get -ENOENT. we'll get woken everytime
+ * every time a request completes and will retry the cancelation.
+ */
+ do {
+ cd.seq = atomic_inc_return(&ctx->cancel_seq),
+
+ prepare_to_wait(&ctx->cq_wait, &wait, TASK_INTERRUPTIBLE);
+
+ ret = __io_sync_cancel(current->io_uring, &cd, sc.fd);
+
+ if (ret != -EALREADY)
+ break;
+
+ mutex_unlock(&ctx->uring_lock);
+ ret = io_run_task_work_sig();
+ if (ret < 0) {
+ mutex_lock(&ctx->uring_lock);
+ break;
+ }
+ ret = schedule_hrtimeout(&timeout, HRTIMER_MODE_ABS);
+ mutex_lock(&ctx->uring_lock);
+ if (!ret) {
+ ret = -ETIME;
+ break;
+ }
+ } while (1);
+
+ finish_wait(&ctx->cq_wait, &wait);
+
+ if (ret == -ENOENT || ret > 0)
+ ret = 0;
+out:
+ fdput(f);
+ return ret;
+}
diff --git a/io_uring/cancel.h b/io_uring/cancel.h
index 1bc7e917ce94..6a59ee484d0c 100644
--- a/io_uring/cancel.h
+++ b/io_uring/cancel.h
@@ -19,3 +19,5 @@ int io_async_cancel(struct io_kiocb *req, unsigned int issue_flags);
int io_try_cancel(struct io_uring_task *tctx, struct io_cancel_data *cd,
unsigned int issue_flags);
void init_hash_table(struct io_hash_table *table, unsigned size);
+
+int io_sync_cancel(struct io_ring_ctx *ctx, void __user *arg);
diff --git a/io_uring/io_uring.c b/io_uring/io_uring.c
index 430e65494989..8abb841e424d 100644
--- a/io_uring/io_uring.c
+++ b/io_uring/io_uring.c
@@ -3911,6 +3911,12 @@ static int __io_uring_register(struct io_ring_ctx *ctx, unsigned opcode,
break;
ret = io_unregister_pbuf_ring(ctx, arg);
break;
+ case IORING_REGISTER_SYNC_CANCEL:
+ ret = -EINVAL;
+ if (!arg || nr_args != 1)
+ break;
+ ret = io_sync_cancel(ctx, arg);
+ break;
default:
ret = -EINVAL;
break;
--
2.35.1
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH 3/3] io_uring: add sync cancelation API through io_uring_register()
2022-06-19 2:07 ` [PATCH 3/3] io_uring: add sync cancelation API through io_uring_register() Jens Axboe
@ 2022-06-19 11:16 ` Ammar Faizi
2022-06-19 12:15 ` Jens Axboe
0 siblings, 1 reply; 6+ messages in thread
From: Ammar Faizi @ 2022-06-19 11:16 UTC (permalink / raw)
To: Jens Axboe; +Cc: Pavel Begunkov, Carter Li, io-uring Mailing List
Hi Jens,
On 6/19/22 9:07 AM, Jens Axboe wrote:
> + cd.seq = atomic_inc_return(&ctx->cancel_seq),
Not sure if it's intentional, but the comma after atomic_inc return()
looks unique. While that's valid syntax, I think you want to use a
semicolon to end it consistently.
> +
> + prepare_to_wait(&ctx->cq_wait, &wait, TASK_INTERRUPTIBLE);
> +
> + ret = __io_sync_cancel(current->io_uring, &cd, sc.fd);
--
Ammar Faizi
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 3/3] io_uring: add sync cancelation API through io_uring_register()
2022-06-19 11:16 ` Ammar Faizi
@ 2022-06-19 12:15 ` Jens Axboe
0 siblings, 0 replies; 6+ messages in thread
From: Jens Axboe @ 2022-06-19 12:15 UTC (permalink / raw)
To: Ammar Faizi; +Cc: Pavel Begunkov, Carter Li, io-uring Mailing List
On 6/19/22 5:16 AM, Ammar Faizi wrote:
> Hi Jens,
>
> On 6/19/22 9:07 AM, Jens Axboe wrote:
>> + cd.seq = atomic_inc_return(&ctx->cancel_seq),
>
> Not sure if it's intentional, but the comma after atomic_inc return()
> looks unique. While that's valid syntax, I think you want to use a
> semicolon to end it consistently.
Oops. Not intentional, that line was copied from the struct init at the
top of that function. I'll fix it up, thanks.
--
Jens Axboe
^ permalink raw reply [flat|nested] 6+ messages in thread