From: Nadav Amit <[email protected]>
To: Hao Xu <[email protected]>
Cc: Pavel Begunkov <[email protected]>,
Jens Axboe <[email protected]>,
"[email protected]" <[email protected]>,
"[email protected]" <[email protected]>
Subject: Re: [PATCH 1/2] io_uring: clear TIF_NOTIFY_SIGNAL when running task work
Date: Mon, 9 Aug 2021 04:50:00 +0000 [thread overview]
Message-ID: <[email protected]> (raw)
In-Reply-To: <[email protected]>
> On Aug 8, 2021, at 9:07 PM, Hao Xu <[email protected]> wrote:
>
> 在 2021/8/9 上午1:31, Nadav Amit 写道:
>>> On Aug 8, 2021, at 5:55 AM, Pavel Begunkov <[email protected]> wrote:
>>>
>>> On 8/8/21 1:13 AM, Nadav Amit wrote:
>>>> From: Nadav Amit <[email protected]>
>>>>
>>>> When using SQPOLL, the submission queue polling thread calls
>>>> task_work_run() to run queued work. However, when work is added with
>>>> TWA_SIGNAL - as done by io_uring itself - the TIF_NOTIFY_SIGNAL remains
>>>
>>> static int io_req_task_work_add(struct io_kiocb *req)
>>> {
>>> ...
>>> notify = (req->ctx->flags & IORING_SETUP_SQPOLL) ? TWA_NONE : TWA_SIGNAL;
>>> if (!task_work_add(tsk, &tctx->task_work, notify))
>>> ...
>>> }
>>>
>>> io_uring doesn't set TIF_NOTIFY_SIGNAL for SQPOLL. But if you see it, I'm
>>> rather curious who does.
>> I was saying io-uring, but I meant io-uring in the wider sense:
>> io_queue_worker_create().
>> Here is a call trace for when TWA_SIGNAL is used. io_queue_worker_create()
>> uses TWA_SIGNAL. It is called by io_wqe_dec_running(), and not shown due
>> to inlining:
> Hi Nadav, Pavel,
> How about trying to make this kind of call to use TWA_NONE for sqthread,
> though I know for this case currently there is no info to get to know if
> task is sqthread. I think we shouldn't kick sqthread.
It is possible, but it would break the abstractions and propagating
it would be disgusting. Let me give it some thought.
Regardless, I think that this patch should go to 5.14 and stable,
and any solution to avoid kicking the SQ should come on top (to be
safe).
next prev parent reply other threads:[~2021-08-09 4:50 UTC|newest]
Thread overview: 17+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-08-08 0:13 [PATCH 0/2] io_uring: bug fixes Nadav Amit
2021-08-08 0:13 ` [PATCH 1/2] io_uring: clear TIF_NOTIFY_SIGNAL when running task work Nadav Amit
2021-08-08 12:55 ` Pavel Begunkov
2021-08-08 17:31 ` Nadav Amit
2021-08-09 4:07 ` Hao Xu
2021-08-09 4:50 ` Nadav Amit [this message]
2021-08-09 10:35 ` Pavel Begunkov
2021-08-09 10:18 ` Pavel Begunkov
2021-08-09 21:48 ` Olivier Langlois
2021-08-10 8:28 ` Nadav Amit
2021-08-10 13:33 ` Olivier Langlois
2021-08-10 21:32 ` Pavel Begunkov
2021-08-11 2:33 ` Nadav Amit
2021-08-11 2:51 ` Jens Axboe
2021-08-11 5:40 ` I/O cancellation in io-uring (was: io_uring: clear TIF_NOTIFY_SIGNAL ...) Nadav Amit
2021-08-08 0:13 ` [PATCH 2/2] io_uring: Use WRITE_ONCE() when writing to sq_flags Nadav Amit
2021-08-09 13:53 ` [PATCH 0/2] io_uring: bug fixes Jens Axboe
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
[email protected] \
[email protected] \
[email protected] \
[email protected] \
[email protected] \
[email protected] \
[email protected] \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox