On 3/21/25 21:38, Caleb Sander Mateos wrote:
On Fri, Mar 21, 2025 at 1:34 PM Pavel Begunkov <asml.silence@xxxxxxxxx> wrote:
On 3/21/25 18:48, Caleb Sander Mateos wrote:
For uring_cmd operations with fixed buffers, the fixed buffer lookup
happens in io_uring_cmd_import_fixed(), called from the ->uring_cmd()
implementation. A ->uring_cmd() implementation could return -EAGAIN on
the initial issue for any reason before io_uring_cmd_import_fixed().
For example, nvme_uring_cmd_io() calls nvme_alloc_user_request() first,
which can return -EAGAIN if all tags in the tag set are in use.
That's up to command when it resolves the buffer, you can just
move the call to io_import_reg_buf() earlier in nvme cmd code
and not working it around at the io_uring side.
In general, it's a step back, it just got cleaned up from the
mess where node resolution and buffer imports were separate
steps and duplicate by every single request type that used it.
Yes, I considered just reordering the steps in nvme_uring_cmd_io().
But it seems easy for a future change to accidentally introduce
another point where the issue can go async before it has looked up the
fixed buffer. And I am imagining there will be more uring_cmd fixed
buffer users added (e.g. btrfs). This seems like a generic problem
rather than something specific to NVMe passthru.
That's working around the api for ordering requests, that's the
reason you have an ordering problem.
My other feeling is that the fixed buffer lookup is an io_uring-layer
detail, whereas the use of the buffer is more a concern of the
->uring_cmd() implementation. If only the opcodes were consistent
about how a fixed buffer is requested, we could do the lookup in the
generic io_uring code like fixed files already do.
That's one of things I'd hope was done better, and not only
specifically for registered buffers, but it's late for that.
But I'm open to implementing a different fix here if Jens would prefer.
It's not a fix, the behaviour is well within the constrained
of io_uring.
--
Pavel Begunkov
