* [PATCH 5.13] io_uring: don't set IORING_SQ_NEED_WAKEUP when sqthread is dying @ 2021-04-27 12:45 Hao Xu 2021-04-27 13:13 ` Pavel Begunkov 0 siblings, 1 reply; 5+ messages in thread From: Hao Xu @ 2021-04-27 12:45 UTC (permalink / raw) To: Jens Axboe; +Cc: io-uring, Pavel Begunkov, Joseph Qi we don't need to re-fork the sqthread over exec, so no need to set IORING_SQ_NEED_WAKEUP when sqthread is dying. Signed-off-by: Hao Xu <[email protected]> --- fs/io_uring.c | 2 -- 1 file changed, 2 deletions(-) diff --git a/fs/io_uring.c b/fs/io_uring.c index 6b578c380e73..92dcd1c21516 100644 --- a/fs/io_uring.c +++ b/fs/io_uring.c @@ -6897,8 +6897,6 @@ static int io_sq_thread(void *data) io_uring_cancel_sqpoll(sqd); sqd->thread = NULL; - list_for_each_entry(ctx, &sqd->ctx_list, sqd_list) - io_ring_set_wakeup_flag(ctx); io_run_task_work(); io_run_task_work_head(&sqd->park_task_work); mutex_unlock(&sqd->lock); -- 1.8.3.1 ^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH 5.13] io_uring: don't set IORING_SQ_NEED_WAKEUP when sqthread is dying 2021-04-27 12:45 [PATCH 5.13] io_uring: don't set IORING_SQ_NEED_WAKEUP when sqthread is dying Hao Xu @ 2021-04-27 13:13 ` Pavel Begunkov 2021-04-27 14:35 ` Hao Xu 0 siblings, 1 reply; 5+ messages in thread From: Pavel Begunkov @ 2021-04-27 13:13 UTC (permalink / raw) To: Hao Xu, Jens Axboe; +Cc: io-uring, Joseph Qi On 4/27/21 1:45 PM, Hao Xu wrote: > we don't need to re-fork the sqthread over exec, so no need to set > IORING_SQ_NEED_WAKEUP when sqthread is dying. It forces users to call io_uring_enter() for it to return -EOWNERDEAD. Consider that scenario with the ring given away to some other task not in current group, e.g. via socket. if (ctx->flags & IORING_SETUP_SQPOLL) { io_cqring_overflow_flush(ctx, false); ret = -EOWNERDEAD; if (unlikely(ctx->sq_data->thread == NULL)) { goto out; } ... } btw, can use a comment > > Signed-off-by: Hao Xu <[email protected]> > --- > fs/io_uring.c | 2 -- > 1 file changed, 2 deletions(-) > > diff --git a/fs/io_uring.c b/fs/io_uring.c > index 6b578c380e73..92dcd1c21516 100644 > --- a/fs/io_uring.c > +++ b/fs/io_uring.c > @@ -6897,8 +6897,6 @@ static int io_sq_thread(void *data) > > io_uring_cancel_sqpoll(sqd); > sqd->thread = NULL; > - list_for_each_entry(ctx, &sqd->ctx_list, sqd_list) > - io_ring_set_wakeup_flag(ctx); > io_run_task_work(); > io_run_task_work_head(&sqd->park_task_work); > mutex_unlock(&sqd->lock); > -- Pavel Begunkov ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH 5.13] io_uring: don't set IORING_SQ_NEED_WAKEUP when sqthread is dying 2021-04-27 13:13 ` Pavel Begunkov @ 2021-04-27 14:35 ` Hao Xu 2021-04-28 6:38 ` Hao Xu 0 siblings, 1 reply; 5+ messages in thread From: Hao Xu @ 2021-04-27 14:35 UTC (permalink / raw) To: Pavel Begunkov, Jens Axboe; +Cc: io-uring, Joseph Qi 在 2021/4/27 下午9:13, Pavel Begunkov 写道: > On 4/27/21 1:45 PM, Hao Xu wrote: >> we don't need to re-fork the sqthread over exec, so no need to set >> IORING_SQ_NEED_WAKEUP when sqthread is dying. > > It forces users to call io_uring_enter() for it to return > -EOWNERDEAD. Consider that scenario with the ring given > away to some other task not in current group, e.g. via socket. > Ah, I see. Thank you Pavel. > if (ctx->flags & IORING_SETUP_SQPOLL) { > io_cqring_overflow_flush(ctx, false); > > ret = -EOWNERDEAD; > if (unlikely(ctx->sq_data->thread == NULL)) { > goto out; > } > ... > } > > btw, can use a comment > >> >> Signed-off-by: Hao Xu <[email protected]> >> --- >> fs/io_uring.c | 2 -- >> 1 file changed, 2 deletions(-) >> >> diff --git a/fs/io_uring.c b/fs/io_uring.c >> index 6b578c380e73..92dcd1c21516 100644 >> --- a/fs/io_uring.c >> +++ b/fs/io_uring.c >> @@ -6897,8 +6897,6 @@ static int io_sq_thread(void *data) >> >> io_uring_cancel_sqpoll(sqd); >> sqd->thread = NULL; >> - list_for_each_entry(ctx, &sqd->ctx_list, sqd_list) >> - io_ring_set_wakeup_flag(ctx); >> io_run_task_work(); >> io_run_task_work_head(&sqd->park_task_work); >> mutex_unlock(&sqd->lock); >> > ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH 5.13] io_uring: don't set IORING_SQ_NEED_WAKEUP when sqthread is dying 2021-04-27 14:35 ` Hao Xu @ 2021-04-28 6:38 ` Hao Xu 2021-04-28 9:28 ` Pavel Begunkov 0 siblings, 1 reply; 5+ messages in thread From: Hao Xu @ 2021-04-28 6:38 UTC (permalink / raw) To: Pavel Begunkov, Jens Axboe; +Cc: io-uring, Joseph Qi 在 2021/4/27 下午10:35, Hao Xu 写道: > 在 2021/4/27 下午9:13, Pavel Begunkov 写道: >> On 4/27/21 1:45 PM, Hao Xu wrote: >>> we don't need to re-fork the sqthread over exec, so no need to set >>> IORING_SQ_NEED_WAKEUP when sqthread is dying. >> >> It forces users to call io_uring_enter() for it to return >> -EOWNERDEAD. Consider that scenario with the ring given >> away to some other task not in current group, e.g. via socket. >> > Ah, I see. Thank you Pavel. Here I've a question: for processes that aren't in same group, io_uring is now designed that sqthread cannot be shared between these processes? But It seems if users do fork(), they can still call io_uring_enter() in the forked task? >> if (ctx->flags & IORING_SETUP_SQPOLL) { >> io_cqring_overflow_flush(ctx, false); >> >> ret = -EOWNERDEAD; >> if (unlikely(ctx->sq_data->thread == NULL)) { >> goto out; >> } >> ... >> } >> >> btw, can use a comment >> >>> >>> Signed-off-by: Hao Xu <[email protected]> >>> --- >>> fs/io_uring.c | 2 -- >>> 1 file changed, 2 deletions(-) >>> >>> diff --git a/fs/io_uring.c b/fs/io_uring.c >>> index 6b578c380e73..92dcd1c21516 100644 >>> --- a/fs/io_uring.c >>> +++ b/fs/io_uring.c >>> @@ -6897,8 +6897,6 @@ static int io_sq_thread(void *data) >>> io_uring_cancel_sqpoll(sqd); >>> sqd->thread = NULL; >>> - list_for_each_entry(ctx, &sqd->ctx_list, sqd_list) >>> - io_ring_set_wakeup_flag(ctx); >>> io_run_task_work(); >>> io_run_task_work_head(&sqd->park_task_work); >>> mutex_unlock(&sqd->lock); >>> >> ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH 5.13] io_uring: don't set IORING_SQ_NEED_WAKEUP when sqthread is dying 2021-04-28 6:38 ` Hao Xu @ 2021-04-28 9:28 ` Pavel Begunkov 0 siblings, 0 replies; 5+ messages in thread From: Pavel Begunkov @ 2021-04-28 9:28 UTC (permalink / raw) To: Hao Xu, Jens Axboe; +Cc: io-uring, Joseph Qi On 4/28/21 7:38 AM, Hao Xu wrote: > 在 2021/4/27 下午10:35, Hao Xu 写道: >> 在 2021/4/27 下午9:13, Pavel Begunkov 写道: >>> On 4/27/21 1:45 PM, Hao Xu wrote: >>>> we don't need to re-fork the sqthread over exec, so no need to set >>>> IORING_SQ_NEED_WAKEUP when sqthread is dying. >>> >>> It forces users to call io_uring_enter() for it to return >>> -EOWNERDEAD. Consider that scenario with the ring given >>> away to some other task not in current group, e.g. via socket. >>> >> Ah, I see. Thank you Pavel. > Here I've a question: for processes that aren't in same group, io_uring > is now designed that sqthread cannot be shared between these processes? Right, sqthread can't be shared by rings created in different thread groups, but it doesn't mean the ring itself can't be shared. > But It seems if users do fork(), they can still call io_uring_enter() > in the forked task? IIRC, forking CLONE_THREAD task is discouraged but allowed, but in any case it's ok from the io_uring perspective. Requests will be fully executed in the context of the sqpoll task >>> if (ctx->flags & IORING_SETUP_SQPOLL) { >>> io_cqring_overflow_flush(ctx, false); >>> >>> ret = -EOWNERDEAD; >>> if (unlikely(ctx->sq_data->thread == NULL)) { >>> goto out; >>> } >>> ... >>> } >>> >>> btw, can use a comment >>> >>>> >>>> Signed-off-by: Hao Xu <[email protected]> >>>> --- >>>> fs/io_uring.c | 2 -- >>>> 1 file changed, 2 deletions(-) >>>> >>>> diff --git a/fs/io_uring.c b/fs/io_uring.c >>>> index 6b578c380e73..92dcd1c21516 100644 >>>> --- a/fs/io_uring.c >>>> +++ b/fs/io_uring.c >>>> @@ -6897,8 +6897,6 @@ static int io_sq_thread(void *data) >>>> io_uring_cancel_sqpoll(sqd); >>>> sqd->thread = NULL; >>>> - list_for_each_entry(ctx, &sqd->ctx_list, sqd_list) >>>> - io_ring_set_wakeup_flag(ctx); >>>> io_run_task_work(); >>>> io_run_task_work_head(&sqd->park_task_work); >>>> mutex_unlock(&sqd->lock); >>>> >>> > -- Pavel Begunkov ^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2021-04-28 9:28 UTC | newest] Thread overview: 5+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2021-04-27 12:45 [PATCH 5.13] io_uring: don't set IORING_SQ_NEED_WAKEUP when sqthread is dying Hao Xu 2021-04-27 13:13 ` Pavel Begunkov 2021-04-27 14:35 ` Hao Xu 2021-04-28 6:38 ` Hao Xu 2021-04-28 9:28 ` Pavel Begunkov
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox