All of lore.kernel.org
 help / color / mirror / Atom feed
From: Pavel Begunkov <asml.silence@gmail.com>
To: Jens Axboe <axboe@kernel.dk>, io-uring@vger.kernel.org
Subject: Re: [PATCH 0/4] implement vectored registered buffers for sendzc
Date: Thu, 24 Oct 2024 18:56:15 +0100	[thread overview]
Message-ID: <3376be3e-e5c4-4fbb-95bb-b3bcd0e9bd8b@gmail.com> (raw)
In-Reply-To: <3e6c3ff5-9116-4d50-9fa8-aae85ad24abc@kernel.dk>

On 10/24/24 18:19, Jens Axboe wrote:
> On 10/24/24 10:06 AM, Pavel Begunkov wrote:
>> On 10/24/24 16:45, Jens Axboe wrote:
...
>>> bv = kmsg->bvec;
>>> for_each_iov {
>>>      struct iovec iov;
>>>
>>>      unsafe_get_user(iov.iov_base, &user_iovec->iov_base, foo);
>>>      unsafe_get_user(iov.iov_len, &user_iovec->iov_len, foo);
>>>
>>>      import_to_bvec(bv, &iov);
>>>
>>>      user_iovec++;
>>>      bv++;
>>> }
>>>
>>> if it can be done at prep time, because then there's no need to store
>>> the iovec at all, it's already stable, just in bvecs. And this avoids
>>> overlapping iovec/bvec memory, and it avoids doing two iterations for
>>> import. Purely a poc, just tossing out ideas.
>>>
>>> But I haven't looked too closely at your series yet. In any case,
>>> whatever ends up working for you, will most likely be find for the
>>> bundled zerocopy send (non-vectored) as well, and I can just put it on
>>> top of that.
>>>
>>>> And you just made one towards delaying the imu resolution, which
>>>> is conceptually the right thing to do because of the mess with
>>>> links, just like it is with fixed files. That's why it need to
>>>> copy the iovec at the prep stage and resolve at the issue time.
>>>
>>> Indeed, prep time is certainly the place to do it. And the above
>>> incremental import should work fine then, as we won't care abot
>>> user_iovec being stable being prep.
>>
>> Seems like you're agreeing but then stating the opposite, there
>> is some confusion. I'm saying that IMHO the right API wise way
>> is resolving an imu at issue time, just like it's done for fixed
>> files, and what your recent series did for send zc.
> 
> Yeah early morning confusion I guess. And I do agree in principle,
> though for registered buffers, those have to be registered upfront
> anyway, so no confusion possible with prep vs issue there. For provided
> buffers, it only matters for the legacy ones, which generally should not
> be used. Doesn't change the fact that you're technically correct, the
> right time to resolve them would be at issue time.

I'm talking about sendmsg with iovec. Registered buffers should
be registered upfront, that's right, but iovec should be copied
at prep, and finally resolved into bvecs incl the imu/buffer lookup
at the issue time. And those are two different points in time,
maybe because of links, draining or anything else. And if they
should be at different moments, there is no way to do it while
copying iovec.

-- 
Pavel Begunkov

  reply	other threads:[~2024-10-24 17:55 UTC|newest]

Thread overview: 17+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-10-23  2:38 [PATCH 0/4] implement vectored registered buffers for sendzc Pavel Begunkov
2024-10-23  2:38 ` [PATCH 1/4] io_uring/net: introduce io_kmsg_set_iovec Pavel Begunkov
2024-10-23  2:38 ` [PATCH 2/4] io_uring/net: allow mixed bvec/iovec caching Pavel Begunkov
2024-10-23  2:38 ` [PATCH 3/4] io_uring: vectored registered buffer import Pavel Begunkov
2024-10-23  2:38 ` [PATCH 4/4] io_uring/net: sendzc with vectored fixed buffers Pavel Begunkov
2024-10-23 13:52 ` [PATCH 0/4] implement vectored registered buffers for sendzc Jens Axboe
2024-10-24 15:29   ` Pavel Begunkov
2024-10-24 15:45     ` Jens Axboe
2024-10-24 16:06       ` Pavel Begunkov
2024-10-24 17:19         ` Jens Axboe
2024-10-24 17:56           ` Pavel Begunkov [this message]
2024-10-24 18:00             ` Jens Axboe
2024-10-24 18:13               ` Pavel Begunkov
2024-10-24 19:56                 ` Jens Axboe
2024-10-24 22:14                   ` Pavel Begunkov
2024-10-24 22:22                     ` Jens Axboe
2024-10-24 22:51                       ` Pavel Begunkov

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 \
    --in-reply-to=3376be3e-e5c4-4fbb-95bb-b3bcd0e9bd8b@gmail.com \
    --to=asml.silence@gmail.com \
    --cc=axboe@kernel.dk \
    --cc=io-uring@vger.kernel.org \
    /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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.