public inbox for [email protected]
 help / color / mirror / Atom feed
* [PATCH] io_uring: trigger worker exit when ring is exiting
@ 2021-09-02 17:56 Jens Axboe
  2021-09-02 18:04 ` Pavel Begunkov
  0 siblings, 1 reply; 4+ messages in thread
From: Jens Axboe @ 2021-09-02 17:56 UTC (permalink / raw)
  To: io-uring

If a task exits normally, then the teardown of async workers happens
quickly. But if it simply closes the ring fd, then the async teardown
ends up waiting for workers to timeout if they are sleeping, which can
then take up to 5 seconds (by default). This isn't a big issue as this
happens off the workqueue path, but let's be nicer and ensure that we
exit as quick as possible.

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

---

diff --git a/fs/io-wq.c b/fs/io-wq.c
index d80e4a735677..60cd841c6c57 100644
--- a/fs/io-wq.c
+++ b/fs/io-wq.c
@@ -1130,7 +1130,17 @@ static bool io_task_work_match(struct callback_head *cb, void *data)
 
 void io_wq_exit_start(struct io_wq *wq)
 {
+	int node;
+
 	set_bit(IO_WQ_BIT_EXIT, &wq->state);
+
+	rcu_read_lock();
+	for_each_node(node) {
+		struct io_wqe *wqe = wq->wqes[node];
+
+		io_wq_for_each_worker(wqe, io_wq_worker_wake, NULL);
+	}
+	rcu_read_unlock();
 }
 
 static void io_wq_exit_workers(struct io_wq *wq)
diff --git a/fs/io_uring.c b/fs/io_uring.c
index 2bde732a1183..9936ebaa8180 100644
--- a/fs/io_uring.c
+++ b/fs/io_uring.c
@@ -9158,6 +9158,7 @@ static bool io_kill_timeouts(struct io_ring_ctx *ctx, struct task_struct *tsk,
 
 static void io_ring_ctx_wait_and_kill(struct io_ring_ctx *ctx)
 {
+	struct io_uring_task *tctx = current->io_uring;
 	unsigned long index;
 	struct creds *creds;
 
@@ -9175,6 +9176,10 @@ static void io_ring_ctx_wait_and_kill(struct io_ring_ctx *ctx)
 	/* if we failed setting up the ctx, we might not have any rings */
 	io_iopoll_try_reap_events(ctx);
 
+	/* trigger exit, if it hasn't been done already */
+	if (tctx->io_wq)
+		io_wq_exit_start(tctx->io_wq);
+
 	INIT_WORK(&ctx->exit_work, io_ring_exit_work);
 	/*
 	 * Use system_unbound_wq to avoid spawning tons of event kworkers

-- 
Jens Axboe


^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [PATCH] io_uring: trigger worker exit when ring is exiting
  2021-09-02 17:56 [PATCH] io_uring: trigger worker exit when ring is exiting Jens Axboe
@ 2021-09-02 18:04 ` Pavel Begunkov
  2021-09-02 18:08   ` Jens Axboe
  0 siblings, 1 reply; 4+ messages in thread
From: Pavel Begunkov @ 2021-09-02 18:04 UTC (permalink / raw)
  To: Jens Axboe, io-uring

On 9/2/21 6:56 PM, Jens Axboe wrote:
> If a task exits normally, then the teardown of async workers happens
> quickly. But if it simply closes the ring fd, then the async teardown
> ends up waiting for workers to timeout if they are sleeping, which can
> then take up to 5 seconds (by default). This isn't a big issue as this
> happens off the workqueue path, but let's be nicer and ensure that we
> exit as quick as possible.

ring = io_uring_init();
...
io_uring_close(&ring); // triggers io_ring_ctx_wait_and_kill()
rint2 = io_uring_init();
...

It looks IO_WQ_BIT_EXIT there will be troublesome.

> Signed-off-by: Jens Axboe <[email protected]>
> 
> ---
> 
> diff --git a/fs/io-wq.c b/fs/io-wq.c
> index d80e4a735677..60cd841c6c57 100644
> --- a/fs/io-wq.c
> +++ b/fs/io-wq.c
> @@ -1130,7 +1130,17 @@ static bool io_task_work_match(struct callback_head *cb, void *data)
>  
>  void io_wq_exit_start(struct io_wq *wq)
>  {
> +	int node;
> +
>  	set_bit(IO_WQ_BIT_EXIT, &wq->state);
> +
> +	rcu_read_lock();
> +	for_each_node(node) {
> +		struct io_wqe *wqe = wq->wqes[node];
> +
> +		io_wq_for_each_worker(wqe, io_wq_worker_wake, NULL);
> +	}
> +	rcu_read_unlock();
>  }
>  
>  static void io_wq_exit_workers(struct io_wq *wq)
> diff --git a/fs/io_uring.c b/fs/io_uring.c
> index 2bde732a1183..9936ebaa8180 100644
> --- a/fs/io_uring.c
> +++ b/fs/io_uring.c
> @@ -9158,6 +9158,7 @@ static bool io_kill_timeouts(struct io_ring_ctx *ctx, struct task_struct *tsk,
>  
>  static void io_ring_ctx_wait_and_kill(struct io_ring_ctx *ctx)
>  {
> +	struct io_uring_task *tctx = current->io_uring;
>  	unsigned long index;
>  	struct creds *creds;
>  
> @@ -9175,6 +9176,10 @@ static void io_ring_ctx_wait_and_kill(struct io_ring_ctx *ctx)
>  	/* if we failed setting up the ctx, we might not have any rings */
>  	io_iopoll_try_reap_events(ctx);
>  
> +	/* trigger exit, if it hasn't been done already */
> +	if (tctx->io_wq)
> +		io_wq_exit_start(tctx->io_wq);
> +
>  	INIT_WORK(&ctx->exit_work, io_ring_exit_work);
>  	/*
>  	 * Use system_unbound_wq to avoid spawning tons of event kworkers
> 

-- 
Pavel Begunkov

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] io_uring: trigger worker exit when ring is exiting
  2021-09-02 18:04 ` Pavel Begunkov
@ 2021-09-02 18:08   ` Jens Axboe
  2021-09-02 18:19     ` Jens Axboe
  0 siblings, 1 reply; 4+ messages in thread
From: Jens Axboe @ 2021-09-02 18:08 UTC (permalink / raw)
  To: Pavel Begunkov, io-uring

On 9/2/21 12:04 PM, Pavel Begunkov wrote:
> On 9/2/21 6:56 PM, Jens Axboe wrote:
>> If a task exits normally, then the teardown of async workers happens
>> quickly. But if it simply closes the ring fd, then the async teardown
>> ends up waiting for workers to timeout if they are sleeping, which can
>> then take up to 5 seconds (by default). This isn't a big issue as this
>> happens off the workqueue path, but let's be nicer and ensure that we
>> exit as quick as possible.
> 
> ring = io_uring_init();
> ...
> io_uring_close(&ring); // triggers io_ring_ctx_wait_and_kill()
> rint2 = io_uring_init();
> ...
> 
> It looks IO_WQ_BIT_EXIT there will be troublesome.

I wonder if we can get by with just a wakeup. Let me test...

-- 
Jens Axboe


^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] io_uring: trigger worker exit when ring is exiting
  2021-09-02 18:08   ` Jens Axboe
@ 2021-09-02 18:19     ` Jens Axboe
  0 siblings, 0 replies; 4+ messages in thread
From: Jens Axboe @ 2021-09-02 18:19 UTC (permalink / raw)
  To: Pavel Begunkov, io-uring

On 9/2/21 12:08 PM, Jens Axboe wrote:
> On 9/2/21 12:04 PM, Pavel Begunkov wrote:
>> On 9/2/21 6:56 PM, Jens Axboe wrote:
>>> If a task exits normally, then the teardown of async workers happens
>>> quickly. But if it simply closes the ring fd, then the async teardown
>>> ends up waiting for workers to timeout if they are sleeping, which can
>>> then take up to 5 seconds (by default). This isn't a big issue as this
>>> happens off the workqueue path, but let's be nicer and ensure that we
>>> exit as quick as possible.
>>
>> ring = io_uring_init();
>> ...
>> io_uring_close(&ring); // triggers io_ring_ctx_wait_and_kill()
>> rint2 = io_uring_init();
>> ...
>>
>> It looks IO_WQ_BIT_EXIT there will be troublesome.
> 
> I wonder if we can get by with just a wakeup. Let me test...

Nah not enough. Would be nice to fix though, pretty annoying that the
threads sit there for 5 seconds doing nothing.

-- 
Jens Axboe


^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2021-09-02 18:19 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2021-09-02 17:56 [PATCH] io_uring: trigger worker exit when ring is exiting Jens Axboe
2021-09-02 18:04 ` Pavel Begunkov
2021-09-02 18:08   ` Jens Axboe
2021-09-02 18:19     ` Jens Axboe

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox