From: Pavel Begunkov <[email protected]>
To: Hao Xu <[email protected]>,
Xiaoguang Wang <[email protected]>,
[email protected]
Cc: [email protected], [email protected]
Subject: Re: [PATCH] io_uring: don't issue reqs in iopoll mode when ctx is dying
Date: Thu, 25 Feb 2021 10:55:15 +0000 [thread overview]
Message-ID: <[email protected]> (raw)
In-Reply-To: <[email protected]>
On 24/02/2021 12:42, Hao Xu wrote:
> 在 2021/2/8 上午1:30, Pavel Begunkov 写道:
>> On 06/02/2021 15:00, Xiaoguang Wang wrote:
>> Looks as you say, but the patch doesn't solve the issue completely.
>> 1. We must not do io_queue_async_work() under a different task context,
>> because of it potentially uses a different set of resources. So, I just
> Hi Pavel,
> I've some questions.
> - Why would the resources be potentially different? My understanding is the tasks which can share a uring instance must be in the same process, so they have same context(files, mm etc.)
"task" and "process" are pretty synonyms in this context, as well as
struct task. And they can have different files/mm, e.g. io_uring works
across fork.
> - Assume 1 is true, why don't we just do something like this:
> req->work.identity = req->task->io_uring->identity;
> since req->task points to the original context
Because there is more to be done to that, see grab_identity or something,
and it may be racy to do. As mentioned, identity will cease to exist with
Jens' patches, and in any case we can punt to task_work.
>
>> thought that it would be better to punt it to the right task context
>> via task_work. But...
>>
>> 2. ...iovec import from io_resubmit_prep() might happen after submit ends,
>> i.e. when iovec was freed in userspace. And that's not great at all.
> I didn't get it why here we need to import iovec. My understanding is OPs(such as readv writev) which need imoport iovec already did that in the first inline IO, what do I omit?
Nope, it may skip copying them. e.g. see -EIOCBQUEUED path of io_read().
> And if it is neccessary, how do we ensure the iovec wasn't freed in userspace at the reissuing time?
We should not import_iovec() for reissue after we left submit, and
that's exactly the problem happening with IOPOLL.
For __io_complete_rw(), Jens tells that it returns -EAGAIN IFF it
called the callback from the submission, correct me if I'm wrong.
I have never checked it myself, though
--
Pavel Begunkov
next prev parent reply other threads:[~2021-02-25 10:59 UTC|newest]
Thread overview: 16+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-02-06 15:00 [PATCH] io_uring: don't issue reqs in iopoll mode when ctx is dying Xiaoguang Wang
2021-02-07 17:30 ` Pavel Begunkov
2021-02-08 2:50 ` Xiaoguang Wang
2021-02-08 13:35 ` Pavel Begunkov
2021-02-22 13:23 ` Pavel Begunkov
2021-02-24 2:30 ` Xiaoguang Wang
2021-02-24 2:35 ` Jens Axboe
2021-02-24 2:45 ` Xiaoguang Wang
2021-02-24 2:51 ` Jens Axboe
2021-02-24 9:46 ` Pavel Begunkov
2021-02-24 9:59 ` Pavel Begunkov
2021-02-24 10:33 ` Pavel Begunkov
2021-02-24 9:38 ` Pavel Begunkov
2021-02-24 12:42 ` Hao Xu
2021-02-25 10:55 ` Pavel Begunkov [this message]
2021-02-24 3:23 ` Xiaoguang Wang
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