On 10/10/24 9:07 PM, Ming Lei wrote:

On Thu, Oct 10, 2024 at 08:39:12PM -0600, Jens Axboe wrote:

quoted

On 10/10/24 8:30 PM, Ming Lei wrote:

quoted

Hi Jens,

On Thu, Oct 10, 2024 at 01:31:21PM -0600, Jens Axboe wrote:

quoted

Hi,

Discussed this with Pavel, and on his suggestion, I tried prototyping a
"buffer update" opcode. Basically it works like
IORING_REGISTER_BUFFERS_UPDATE in that it can update an existing buffer
registration. But it works as an sqe rather than being a sync opcode.

The idea here is that you could do that upfront, or as part of a chain,
and have it be generically available, just like any other buffer that
was registered upfront. You do need an empty table registered first,
which can just be sparse. And since you can pick the slot it goes into,
you can rely on that slot afterwards (either as a link, or just the
following sqe).

Quick'n dirty obviously, but I did write a quick test case too to verify
that:

1) It actually works (it seems to)

It doesn't work for ublk zc since ublk needs to provide one kernel buffer
for fs rw & net send/recv to consume, and the kernel buffer is invisible
to userspace. But  __io_register_rsrc_update() only can register userspace
buffer.

I'd be surprised if this simple one was enough! In terms of user vs
kernel buffer, you could certainly use the same mechanism, and just
ensure that buffers are tagged appropriately. I need to think about that
a little bit.

It is actually same with IORING_OP_PROVIDE_BUFFERS, so the following
consumer OPs have to wait until this OP_BUF_UPDATE is completed.

See below for the registered vs provided buffer confusion that seems to
be a confusion issue here.

Suppose we have N consumers OPs which depends on OP_BUF_UPDATE.

1) all N OPs are linked with OP_BUF_UPDATE

Or

2) submit OP_BUF_UPDATE first, and wait its completion, then submit N
OPs concurrently.

Correct

But 1) and 2) may slow the IO handing.  In 1) all N OPs are serialized,
and 1 extra syscall is introduced in 2).

Yes you don't want do do #1. But the OP_BUF_UPDATE is cheap enough that
you can just do it upfront. It's not ideal in terms of usage, and I get
where the grouping comes from. But is it possible to do the grouping in
a less intrusive fashion with OP_BUF_UPDATE? Because it won't change any
of the other ops in terms of buffer consumption, they'd just need fixed
buffer support and you'd flag the buffer index in sqe->buf_index. And
the nice thing about that is that while fixed/registered buffers aren't
really used on the networking side yet (as they don't bring any benefit
yet), adding support for them could potentially be useful down the line
anyway.

The same thing exists in the next OP_BUF_UPDATE which has to wait until
all the previous buffer consumers are done. So the same slow thing are
doubled. Not mention the application will become more complicated.

It does not, you can do an update on a buffer that's already inflight.

Here the provided buffer is only visible among the N OPs wide, and making
it global isn't necessary, and slow things down. And has kbuf lifetime
issue.

I was worried about it being too slow too, but the basic testing seems
like it's fine. Yes with updates inflight it'll make it a tad bit
slower, but really should not be a concern. I'd argue that even doing
the very basic of things, which would be:

1) Submit OP_BUF_UPDATE, get completion
2) Do the rest of the ops

would be totally fine in terms of performance. OP_BUF_UPDATE will
_always_ completely immediately and inline, which means that it'll
_always_ be immediately available post submission. The only think you'd
ever have to worry about in terms of failure is a badly formed request,
which is a programming issue, or running out of memory on the host.

Also it makes error handling more complicated, io_uring has to remove
the kernel buffer when the current task is exit, dependency or order with
buffer provider is introduced.

Why would that be? They belong to the ring, so should be torn down as
part of the ring anyway? Why would they be task-private, but not
ring-private?

quoted

There are certainly many different ways that can get propagated which
would not entail a complicated mechanism. I really like the aspect of
having the identifier being the same thing that we already use, and
hence not needing to be something new on the side.

quoted

Also multiple OPs may consume the buffer concurrently, which can't be
supported by buffer select.

Why not? You can certainly have multiple ops using the same registered
buffer concurrently right now.

Please see the above problem.

Also I remember that the selected buffer is removed from buffer list,
see io_provided_buffer_select(), but maybe I am wrong.

You're mixing up provided and registered buffers. Provided buffers are
ones that the applications gives to the kernel, and the kernel grabs and
consumes them. Then the application replenishes, repeat.

Registered buffers are entirely different, those are registered with the
kernel and we can do things like pre-gup the pages so we don't have to
do them for every IO. They are entirely persistent, any multiple ops can
keep using them, concurrently. They don't get consumed by an IO like
provided buffers, they remain in place until they get unregistered (or
updated, like my patch) at some point.

-- 
Jens Axboe