[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH v4 2/3] QIOChannelSocket: Implement io_writev_zerocopy & io_f
From: |
Leonardo Bras Soares Passos |
Subject: |
Re: [PATCH v4 2/3] QIOChannelSocket: Implement io_writev_zerocopy & io_flush_zerocopy for CONFIG_LINUX |
Date: |
Mon, 11 Oct 2021 16:44:52 -0300 |
Hello Eric,
On Mon, Oct 11, 2021 at 4:28 PM Eric Blake <eblake@redhat.com> wrote:
>
> On Sat, Oct 09, 2021 at 04:56:12AM -0300, Leonardo Bras wrote:
> > For CONFIG_LINUX, implement the new optional callbacks io_write_zerocopy and
> > io_flush_zerocopy on QIOChannelSocket, but enables it only when MSG_ZEROCOPY
> > feature is available in the host kernel, which is checked on
> > qio_channel_socket_connect_sync()
> >
> > qio_channel_socket_writev() contents were moved to a helper function
> > qio_channel_socket_writev_flags() which accepts an extra argument for flags.
> > (This argument is passed directly to sendmsg().
> >
> > The above helper function is used to implement qio_channel_socket_writev(),
> > with flags = 0, keeping it's behavior unchanged, and
>
> its (remember, "it's" is shorthand for "it is", which does not fit here)
>
> > qio_channel_socket_writev_zerocopy() with flags = MSG_ZEROCOPY.
> >
> > qio_channel_socket_flush_zerocopy() was implemented by counting how many
> > times
> > sendmsg(...,MSG_ZEROCOPY) was sucessfully called, and then reading the
> > socket's error queue, in order to find how many of them finished sending.
> > Flush will loop until those counters are the same, or until some error
> > ocurs.
>
> occurs
Thanks!
(I will check if my codespell is enabled in this setup)
>
> >
> > A new function qio_channel_socket_poll() was also created in order to avoid
> > busy-looping recvmsg() in qio_channel_socket_flush_zerocopy() while waiting
> > for
> > updates in socket's error queue.
> >
> > Notes on using writev_zerocopy():
> > 1: Buffer
> > - As MSG_ZEROCOPY tells the kernel to use the same user buffer to avoid
> > copying,
> > some caution is necessary to avoid overwriting any buffer before it's sent.
> > If something like this happen, a newer version of the buffer may be sent
> > instead.
> > - If this is a problem, it's recommended to call flush_zerocopy() before
> > freeing
> > or re-using the buffer.
> >
> > 2: Locked memory
> > - When using MSG_ZERCOCOPY, the buffer memory will be locked after queued,
> > and
> > unlocked after it's sent.
> > - Depending on the size of each buffer, and how often it's sent, it may
> > require
> > a larger amount of locked memory than usually available to non-root user.
> > - If the required amount of locked memory is not available, writev_zerocopy
> > will return an error, which can abort an operation like migration,
> > - Because of this, when an user code wants to add zerocopy as a feature, it
> > requires a mechanism to disable it, so it can still be acessible to less
> > privileged users.
> >
> > Signed-off-by: Leonardo Bras <leobras@redhat.com>
> > ---
> > include/io/channel-socket.h | 2 +
> > include/io/channel.h | 1 +
> > io/channel-socket.c | 180 ++++++++++++++++++++++++++++++++++--
> > 3 files changed, 173 insertions(+), 10 deletions(-)
> >
> > +static int qio_channel_socket_flush_zerocopy(QIOChannel *ioc,
> > + Error **errp)
> > +{
>
> > +
> > + /* No errors, count sucessfully finished sendmsg()*/
>
> Space before */
Thanks!
Also, typo on 'successfully'.
>
> > + sioc->zerocopy_sent += serr->ee_data - serr->ee_info + 1;
> > + }
> > + return 0;
> > +}
> > +
> > +#endif /* CONFIG_LINUX */
> > +
> > static int
> > qio_channel_socket_set_blocking(QIOChannel *ioc,
> > bool enabled,
> > @@ -787,6 +943,10 @@ static void qio_channel_socket_class_init(ObjectClass
> > *klass,
> > ioc_klass->io_set_delay = qio_channel_socket_set_delay;
> > ioc_klass->io_create_watch = qio_channel_socket_create_watch;
> > ioc_klass->io_set_aio_fd_handler =
> > qio_channel_socket_set_aio_fd_handler;
> > +#ifdef CONFIG_LINUX
> > + ioc_klass->io_writev_zerocopy = qio_channel_socket_writev_zerocopy;
> > + ioc_klass->io_flush_zerocopy = qio_channel_socket_flush_zerocopy;
> > +#endif
> > }
>
> I did a high-level look at the code, rather than an in-depth review of
> whether zero-copy was being used correctly.
It's so far been very helpful. Thanks!
Best regards,
Leo
- Re: [PATCH v4 1/3] QIOChannel: Add io_writev_zerocopy & io_flush_zerocopy callbacks, (continued)
Re: [PATCH v4 1/3] QIOChannel: Add io_writev_zerocopy & io_flush_zerocopy callbacks, Peter Xu, 2021/10/13
[PATCH v4 2/3] QIOChannelSocket: Implement io_writev_zerocopy & io_flush_zerocopy for CONFIG_LINUX, Leonardo Bras, 2021/10/09
[PATCH v4 3/3] multifd: Implement zerocopy write in multifd migration (multifd-zerocopy), Leonardo Bras, 2021/10/09
- Re: [PATCH v4 3/3] multifd: Implement zerocopy write in multifd migration (multifd-zerocopy), Eric Blake, 2021/10/11
- Re: [PATCH v4 3/3] multifd: Implement zerocopy write in multifd migration (multifd-zerocopy), Leonardo Bras Soares Passos, 2021/10/11
- Re: [PATCH v4 3/3] multifd: Implement zerocopy write in multifd migration (multifd-zerocopy), Markus Armbruster, 2021/10/12
- Re: [PATCH v4 3/3] multifd: Implement zerocopy write in multifd migration (multifd-zerocopy), Leonardo Bras Soares Passos, 2021/10/27
- Re: [PATCH v4 3/3] multifd: Implement zerocopy write in multifd migration (multifd-zerocopy), Markus Armbruster, 2021/10/28
- Re: [PATCH v4 3/3] multifd: Implement zerocopy write in multifd migration (multifd-zerocopy), Leonardo Bras Soares Passos, 2021/10/28
Re: [PATCH v4 3/3] multifd: Implement zerocopy write in multifd migration (multifd-zerocopy), Peter Xu, 2021/10/13