* [PATCH v2 1/4] io-wq: reorder cancellation pending -> running
2020-06-15 7:24 [PATCH v2 0/4][RESEND] cancel all reqs of an exiting task Pavel Begunkov
@ 2020-06-15 7:24 ` Pavel Begunkov
2020-06-15 7:24 ` [PATCH v2 2/4] io-wq: add an option to cancel all matched reqs Pavel Begunkov
` (3 subsequent siblings)
4 siblings, 0 replies; 6+ messages in thread
From: Pavel Begunkov @ 2020-06-15 7:24 UTC (permalink / raw)
To: Jens Axboe, io-uring, linux-kernel
Go all over all pending lists and cancel works there, and only then
try to match running requests. No functional changes here, just a
preparation for bulk cancellation.
Signed-off-by: Pavel Begunkov <[email protected]>
---
fs/io-wq.c | 54 ++++++++++++++++++++++++++++++++----------------------
1 file changed, 32 insertions(+), 22 deletions(-)
diff --git a/fs/io-wq.c b/fs/io-wq.c
index d7dc638f4b8e..9e7d277de248 100644
--- a/fs/io-wq.c
+++ b/fs/io-wq.c
@@ -932,19 +932,14 @@ static bool io_wq_worker_cancel(struct io_worker *worker, void *data)
return ret;
}
-static enum io_wq_cancel io_wqe_cancel_work(struct io_wqe *wqe,
- struct io_cb_cancel_data *match)
+static bool io_wqe_cancel_pending_work(struct io_wqe *wqe,
+ struct io_cb_cancel_data *match)
{
struct io_wq_work_node *node, *prev;
struct io_wq_work *work;
unsigned long flags;
bool found = false;
- /*
- * First check pending list, if we're lucky we can just remove it
- * from there. CANCEL_OK means that the work is returned as-new,
- * no completion will be posted for it.
- */
spin_lock_irqsave(&wqe->lock, flags);
wq_list_for_each(node, prev, &wqe->work_list) {
work = container_of(node, struct io_wq_work, list);
@@ -957,21 +952,20 @@ static enum io_wq_cancel io_wqe_cancel_work(struct io_wqe *wqe,
}
spin_unlock_irqrestore(&wqe->lock, flags);
- if (found) {
+ if (found)
io_run_cancel(work, wqe);
- return IO_WQ_CANCEL_OK;
- }
+ return found;
+}
+
+static bool io_wqe_cancel_running_work(struct io_wqe *wqe,
+ struct io_cb_cancel_data *match)
+{
+ bool found;
- /*
- * Now check if a free (going busy) or busy worker has the work
- * currently running. If we find it there, we'll return CANCEL_RUNNING
- * as an indication that we attempt to signal cancellation. The
- * completion will run normally in this case.
- */
rcu_read_lock();
found = io_wq_for_each_worker(wqe, io_wq_worker_cancel, match);
rcu_read_unlock();
- return found ? IO_WQ_CANCEL_RUNNING : IO_WQ_CANCEL_NOTFOUND;
+ return found;
}
enum io_wq_cancel io_wq_cancel_cb(struct io_wq *wq, work_cancel_fn *cancel,
@@ -981,18 +975,34 @@ enum io_wq_cancel io_wq_cancel_cb(struct io_wq *wq, work_cancel_fn *cancel,
.fn = cancel,
.data = data,
};
- enum io_wq_cancel ret = IO_WQ_CANCEL_NOTFOUND;
int node;
+ /*
+ * First check pending list, if we're lucky we can just remove it
+ * from there. CANCEL_OK means that the work is returned as-new,
+ * no completion will be posted for it.
+ */
for_each_node(node) {
struct io_wqe *wqe = wq->wqes[node];
- ret = io_wqe_cancel_work(wqe, &match);
- if (ret != IO_WQ_CANCEL_NOTFOUND)
- break;
+ if (io_wqe_cancel_pending_work(wqe, &match))
+ return IO_WQ_CANCEL_OK;
}
- return ret;
+ /*
+ * Now check if a free (going busy) or busy worker has the work
+ * currently running. If we find it there, we'll return CANCEL_RUNNING
+ * as an indication that we attempt to signal cancellation. The
+ * completion will run normally in this case.
+ */
+ for_each_node(node) {
+ struct io_wqe *wqe = wq->wqes[node];
+
+ if (io_wqe_cancel_running_work(wqe, &match))
+ return IO_WQ_CANCEL_RUNNING;
+ }
+
+ return IO_WQ_CANCEL_NOTFOUND;
}
static bool io_wq_io_cb_cancel_data(struct io_wq_work *work, void *data)
--
2.24.0
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH v2 2/4] io-wq: add an option to cancel all matched reqs
2020-06-15 7:24 [PATCH v2 0/4][RESEND] cancel all reqs of an exiting task Pavel Begunkov
2020-06-15 7:24 ` [PATCH v2 1/4] io-wq: reorder cancellation pending -> running Pavel Begunkov
@ 2020-06-15 7:24 ` Pavel Begunkov
2020-06-15 7:24 ` [PATCH v2 3/4] io_uring: cancel all task's requests on exit Pavel Begunkov
` (2 subsequent siblings)
4 siblings, 0 replies; 6+ messages in thread
From: Pavel Begunkov @ 2020-06-15 7:24 UTC (permalink / raw)
To: Jens Axboe, io-uring, linux-kernel
This adds support for cancelling all io-wq works matching a predicate.
It isn't used yet, so no change in observable behaviour.
Signed-off-by: Pavel Begunkov <[email protected]>
---
fs/io-wq.c | 60 +++++++++++++++++++++++++++++----------------------
fs/io-wq.h | 2 +-
fs/io_uring.c | 2 +-
3 files changed, 36 insertions(+), 28 deletions(-)
diff --git a/fs/io-wq.c b/fs/io-wq.c
index 9e7d277de248..3b0bd956e539 100644
--- a/fs/io-wq.c
+++ b/fs/io-wq.c
@@ -908,13 +908,15 @@ void io_wq_cancel_all(struct io_wq *wq)
struct io_cb_cancel_data {
work_cancel_fn *fn;
void *data;
+ int nr_running;
+ int nr_pending;
+ bool cancel_all;
};
static bool io_wq_worker_cancel(struct io_worker *worker, void *data)
{
struct io_cb_cancel_data *match = data;
unsigned long flags;
- bool ret = false;
/*
* Hold the lock to avoid ->cur_work going out of scope, caller
@@ -925,55 +927,55 @@ static bool io_wq_worker_cancel(struct io_worker *worker, void *data)
!(worker->cur_work->flags & IO_WQ_WORK_NO_CANCEL) &&
match->fn(worker->cur_work, match->data)) {
send_sig(SIGINT, worker->task, 1);
- ret = true;
+ match->nr_running++;
}
spin_unlock_irqrestore(&worker->lock, flags);
- return ret;
+ return match->nr_running && !match->cancel_all;
}
-static bool io_wqe_cancel_pending_work(struct io_wqe *wqe,
+static void io_wqe_cancel_pending_work(struct io_wqe *wqe,
struct io_cb_cancel_data *match)
{
struct io_wq_work_node *node, *prev;
struct io_wq_work *work;
unsigned long flags;
- bool found = false;
+retry:
spin_lock_irqsave(&wqe->lock, flags);
wq_list_for_each(node, prev, &wqe->work_list) {
work = container_of(node, struct io_wq_work, list);
+ if (!match->fn(work, match->data))
+ continue;
- if (match->fn(work, match->data)) {
- wq_list_del(&wqe->work_list, node, prev);
- found = true;
- break;
- }
+ wq_list_del(&wqe->work_list, node, prev);
+ spin_unlock_irqrestore(&wqe->lock, flags);
+ io_run_cancel(work, wqe);
+ match->nr_pending++;
+ if (!match->cancel_all)
+ return;
+
+ /* not safe to continue after unlock */
+ goto retry;
}
spin_unlock_irqrestore(&wqe->lock, flags);
-
- if (found)
- io_run_cancel(work, wqe);
- return found;
}
-static bool io_wqe_cancel_running_work(struct io_wqe *wqe,
+static void io_wqe_cancel_running_work(struct io_wqe *wqe,
struct io_cb_cancel_data *match)
{
- bool found;
-
rcu_read_lock();
- found = io_wq_for_each_worker(wqe, io_wq_worker_cancel, match);
+ io_wq_for_each_worker(wqe, io_wq_worker_cancel, match);
rcu_read_unlock();
- return found;
}
enum io_wq_cancel io_wq_cancel_cb(struct io_wq *wq, work_cancel_fn *cancel,
- void *data)
+ void *data, bool cancel_all)
{
struct io_cb_cancel_data match = {
- .fn = cancel,
- .data = data,
+ .fn = cancel,
+ .data = data,
+ .cancel_all = cancel_all,
};
int node;
@@ -985,7 +987,8 @@ enum io_wq_cancel io_wq_cancel_cb(struct io_wq *wq, work_cancel_fn *cancel,
for_each_node(node) {
struct io_wqe *wqe = wq->wqes[node];
- if (io_wqe_cancel_pending_work(wqe, &match))
+ io_wqe_cancel_pending_work(wqe, &match);
+ if (match.nr_pending && !match.cancel_all)
return IO_WQ_CANCEL_OK;
}
@@ -998,10 +1001,15 @@ enum io_wq_cancel io_wq_cancel_cb(struct io_wq *wq, work_cancel_fn *cancel,
for_each_node(node) {
struct io_wqe *wqe = wq->wqes[node];
- if (io_wqe_cancel_running_work(wqe, &match))
+ io_wqe_cancel_running_work(wqe, &match);
+ if (match.nr_running && !match.cancel_all)
return IO_WQ_CANCEL_RUNNING;
}
+ if (match.nr_running)
+ return IO_WQ_CANCEL_RUNNING;
+ if (match.nr_pending)
+ return IO_WQ_CANCEL_OK;
return IO_WQ_CANCEL_NOTFOUND;
}
@@ -1012,7 +1020,7 @@ static bool io_wq_io_cb_cancel_data(struct io_wq_work *work, void *data)
enum io_wq_cancel io_wq_cancel_work(struct io_wq *wq, struct io_wq_work *cwork)
{
- return io_wq_cancel_cb(wq, io_wq_io_cb_cancel_data, (void *)cwork);
+ return io_wq_cancel_cb(wq, io_wq_io_cb_cancel_data, (void *)cwork, false);
}
static bool io_wq_pid_match(struct io_wq_work *work, void *data)
@@ -1026,7 +1034,7 @@ enum io_wq_cancel io_wq_cancel_pid(struct io_wq *wq, pid_t pid)
{
void *data = (void *) (unsigned long) pid;
- return io_wq_cancel_cb(wq, io_wq_pid_match, data);
+ return io_wq_cancel_cb(wq, io_wq_pid_match, data, false);
}
struct io_wq *io_wq_create(unsigned bounded, struct io_wq_data *data)
diff --git a/fs/io-wq.h b/fs/io-wq.h
index 8e138fa88b9f..7d5bd431c5e3 100644
--- a/fs/io-wq.h
+++ b/fs/io-wq.h
@@ -130,7 +130,7 @@ enum io_wq_cancel io_wq_cancel_pid(struct io_wq *wq, pid_t pid);
typedef bool (work_cancel_fn)(struct io_wq_work *, void *);
enum io_wq_cancel io_wq_cancel_cb(struct io_wq *wq, work_cancel_fn *cancel,
- void *data);
+ void *data, bool cancel_all);
struct task_struct *io_wq_get_task(struct io_wq *wq);
diff --git a/fs/io_uring.c b/fs/io_uring.c
index 5b0249140ff5..7f18c29388d6 100644
--- a/fs/io_uring.c
+++ b/fs/io_uring.c
@@ -4773,7 +4773,7 @@ static int io_async_cancel_one(struct io_ring_ctx *ctx, void *sqe_addr)
enum io_wq_cancel cancel_ret;
int ret = 0;
- cancel_ret = io_wq_cancel_cb(ctx->io_wq, io_cancel_cb, sqe_addr);
+ cancel_ret = io_wq_cancel_cb(ctx->io_wq, io_cancel_cb, sqe_addr, false);
switch (cancel_ret) {
case IO_WQ_CANCEL_OK:
ret = 0;
--
2.24.0
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH v2 3/4] io_uring: cancel all task's requests on exit
2020-06-15 7:24 [PATCH v2 0/4][RESEND] cancel all reqs of an exiting task Pavel Begunkov
2020-06-15 7:24 ` [PATCH v2 1/4] io-wq: reorder cancellation pending -> running Pavel Begunkov
2020-06-15 7:24 ` [PATCH v2 2/4] io-wq: add an option to cancel all matched reqs Pavel Begunkov
@ 2020-06-15 7:24 ` Pavel Begunkov
2020-06-15 7:24 ` [PATCH v2 4/4] io_uring: batch cancel in io_uring_cancel_files() Pavel Begunkov
2020-06-15 15:04 ` [PATCH v2 0/4][RESEND] cancel all reqs of an exiting task Jens Axboe
4 siblings, 0 replies; 6+ messages in thread
From: Pavel Begunkov @ 2020-06-15 7:24 UTC (permalink / raw)
To: Jens Axboe, io-uring, linux-kernel
If a process is going away, io_uring_flush() will cancel only 1
request with a matching pid. Cancel all of them
Signed-off-by: Pavel Begunkov <[email protected]>
---
fs/io-wq.c | 14 --------------
fs/io-wq.h | 1 -
fs/io_uring.c | 14 ++++++++++++--
3 files changed, 12 insertions(+), 17 deletions(-)
diff --git a/fs/io-wq.c b/fs/io-wq.c
index 3b0bd956e539..a44ad3b98886 100644
--- a/fs/io-wq.c
+++ b/fs/io-wq.c
@@ -1023,20 +1023,6 @@ enum io_wq_cancel io_wq_cancel_work(struct io_wq *wq, struct io_wq_work *cwork)
return io_wq_cancel_cb(wq, io_wq_io_cb_cancel_data, (void *)cwork, false);
}
-static bool io_wq_pid_match(struct io_wq_work *work, void *data)
-{
- pid_t pid = (pid_t) (unsigned long) data;
-
- return work->task_pid == pid;
-}
-
-enum io_wq_cancel io_wq_cancel_pid(struct io_wq *wq, pid_t pid)
-{
- void *data = (void *) (unsigned long) pid;
-
- return io_wq_cancel_cb(wq, io_wq_pid_match, data, false);
-}
-
struct io_wq *io_wq_create(unsigned bounded, struct io_wq_data *data)
{
int ret = -ENOMEM, node;
diff --git a/fs/io-wq.h b/fs/io-wq.h
index 7d5bd431c5e3..b72538fe5afd 100644
--- a/fs/io-wq.h
+++ b/fs/io-wq.h
@@ -125,7 +125,6 @@ static inline bool io_wq_is_hashed(struct io_wq_work *work)
void io_wq_cancel_all(struct io_wq *wq);
enum io_wq_cancel io_wq_cancel_work(struct io_wq *wq, struct io_wq_work *cwork);
-enum io_wq_cancel io_wq_cancel_pid(struct io_wq *wq, pid_t pid);
typedef bool (work_cancel_fn)(struct io_wq_work *, void *);
diff --git a/fs/io_uring.c b/fs/io_uring.c
index 7f18c29388d6..8bde42775693 100644
--- a/fs/io_uring.c
+++ b/fs/io_uring.c
@@ -7428,6 +7428,13 @@ static void io_uring_cancel_files(struct io_ring_ctx *ctx,
}
}
+static bool io_cancel_pid_cb(struct io_wq_work *work, void *data)
+{
+ pid_t pid = (pid_t) (unsigned long) data;
+
+ return work->task_pid == pid;
+}
+
static int io_uring_flush(struct file *file, void *data)
{
struct io_ring_ctx *ctx = file->private_data;
@@ -7437,8 +7444,11 @@ static int io_uring_flush(struct file *file, void *data)
/*
* If the task is going away, cancel work it may have pending
*/
- if (fatal_signal_pending(current) || (current->flags & PF_EXITING))
- io_wq_cancel_pid(ctx->io_wq, task_pid_vnr(current));
+ if (fatal_signal_pending(current) || (current->flags & PF_EXITING)) {
+ void *data = (void *) (unsigned long)task_pid_vnr(current);
+
+ io_wq_cancel_cb(ctx->io_wq, io_cancel_pid_cb, data, true);
+ }
return 0;
}
--
2.24.0
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH v2 4/4] io_uring: batch cancel in io_uring_cancel_files()
2020-06-15 7:24 [PATCH v2 0/4][RESEND] cancel all reqs of an exiting task Pavel Begunkov
` (2 preceding siblings ...)
2020-06-15 7:24 ` [PATCH v2 3/4] io_uring: cancel all task's requests on exit Pavel Begunkov
@ 2020-06-15 7:24 ` Pavel Begunkov
2020-06-15 15:04 ` [PATCH v2 0/4][RESEND] cancel all reqs of an exiting task Jens Axboe
4 siblings, 0 replies; 6+ messages in thread
From: Pavel Begunkov @ 2020-06-15 7:24 UTC (permalink / raw)
To: Jens Axboe, io-uring, linux-kernel
Instead of waiting for each request one by one, first try to cancel all
of them in a batched manner, and then go over inflight_list/etc to reap
leftovers.
Signed-off-by: Pavel Begunkov <[email protected]>
---
fs/io_uring.c | 13 +++++++++++++
1 file changed, 13 insertions(+)
diff --git a/fs/io_uring.c b/fs/io_uring.c
index 8bde42775693..5b5cab6691d2 100644
--- a/fs/io_uring.c
+++ b/fs/io_uring.c
@@ -7370,9 +7370,22 @@ static int io_uring_release(struct inode *inode, struct file *file)
return 0;
}
+static bool io_wq_files_match(struct io_wq_work *work, void *data)
+{
+ struct files_struct *files = data;
+
+ return work->files == files;
+}
+
static void io_uring_cancel_files(struct io_ring_ctx *ctx,
struct files_struct *files)
{
+ if (list_empty_careful(&ctx->inflight_list))
+ return;
+
+ /* cancel all at once, should be faster than doing it one by one*/
+ io_wq_cancel_cb(ctx->io_wq, io_wq_files_match, files, true);
+
while (!list_empty_careful(&ctx->inflight_list)) {
struct io_kiocb *cancel_req = NULL, *req;
DEFINE_WAIT(wait);
--
2.24.0
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH v2 0/4][RESEND] cancel all reqs of an exiting task
2020-06-15 7:24 [PATCH v2 0/4][RESEND] cancel all reqs of an exiting task Pavel Begunkov
` (3 preceding siblings ...)
2020-06-15 7:24 ` [PATCH v2 4/4] io_uring: batch cancel in io_uring_cancel_files() Pavel Begunkov
@ 2020-06-15 15:04 ` Jens Axboe
4 siblings, 0 replies; 6+ messages in thread
From: Jens Axboe @ 2020-06-15 15:04 UTC (permalink / raw)
To: Pavel Begunkov, io-uring, linux-kernel
On 6/15/20 1:24 AM, Pavel Begunkov wrote:
> io_uring_flush() {
> ...
> if (fatal_signal_pending(current) || (current->flags & PF_EXITING))
> io_wq_cancel_pid(ctx->io_wq, task_pid_vnr(current));
> }
>
> This cancels only the first matched request. The pathset is mainly
> about fixing that. [1,2] are preps, [3/4] is the fix.
>
> The [4/4] tries to improve the worst case for io_uring_cancel_files(),
> that's when they are a lot of inflights with ->files. Instead of doing
> {kill(); wait();} one by one, it cancels all of them at once.
Applied, thanks.
--
Jens Axboe
^ permalink raw reply [flat|nested] 6+ messages in thread