From: Pavel Begunkov <[email protected]>
To: Jens Axboe <[email protected]>,
Caleb Sander Mateos <[email protected]>
Cc: Riley Thomasson <[email protected]>,
[email protected], [email protected]
Subject: Re: [PATCH 0/2] uring_cmd SQE corruptions
Date: Thu, 13 Feb 2025 14:48:19 +0000 [thread overview]
Message-ID: <[email protected]> (raw)
In-Reply-To: <[email protected]>
On 2/12/25 20:55, Jens Axboe wrote:
> On 2/12/25 1:45 PM, Caleb Sander Mateos wrote:
...
>> However, uring_cmd's can be issued async in other cases not enumerated
>> by 5eff57fa9f3a, also leading to SQE corruption. These include requests
>> besides the first in a linked chain, which are only issued once prior
>> requests complete. Requests waiting for a drain to complete would also
>> be initially issued async.
>>
>> While it's probably possible for io_uring_cmd_prep_setup() to check for
>> each of these cases and avoid deferring the SQE memcpy(), we feel it
>> might be safer to revert 5eff57fa9f3a to avoid the corruption risk.
>> As discussed recently in regard to the ublk zero-copy patches[1], new
>> async paths added in the future could break these delicate assumptions.
>
> I don't think it's particularly delicate - did you manage to catch the
> case queueing a request for async execution where the sqe wasn't already
> copied? I did take a quick look after our out-of-band conversation, and
> the only missing bit I immediately spotted is using SQPOLL. But I don't
> think you're using that, right? And in any case, lifetime of SQEs with
> SQPOLL is the duration of the request anyway, so should not pose any
Not really, it's not bound to the syscall, but the user could always
check or wait (i.e. IORING_ENTER_SQ_WAIT) for empty sqes and reuse
them before completion.
--
Pavel Begunkov
next prev parent reply other threads:[~2025-02-13 14:47 UTC|newest]
Thread overview: 19+ messages / expand[flat|nested] mbox.gz Atom feed top
2025-02-12 20:45 [PATCH 0/2] uring_cmd SQE corruptions Caleb Sander Mateos
2025-02-12 20:45 ` [PATCH 1/2] io_uring/uring_cmd: don't assume io_uring_cmd_data layout Caleb Sander Mateos
2025-02-12 20:45 ` [PATCH 2/2] io_uring/uring_cmd: switch sqe to async_data on EAGAIN Caleb Sander Mateos
2025-02-12 20:55 ` [PATCH 0/2] uring_cmd SQE corruptions Jens Axboe
2025-02-12 21:02 ` Jens Axboe
2025-02-12 21:58 ` Caleb Sander
2025-02-12 22:34 ` Jens Axboe
2025-02-12 22:52 ` Caleb Sander
2025-02-12 22:56 ` Jens Axboe
2025-02-12 21:54 ` Caleb Sander
2025-02-12 22:39 ` Jens Axboe
2025-02-12 23:07 ` Caleb Sander Mateos
2025-02-12 23:21 ` Keith Busch
2025-02-12 23:46 ` Caleb Sander Mateos
2025-02-12 23:55 ` Jens Axboe
2025-02-13 16:28 ` Pavel Begunkov
2025-02-13 16:11 ` Pavel Begunkov
2025-02-13 14:48 ` Pavel Begunkov [this message]
2025-02-13 18:13 ` 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