Hello Eric, On Mon, Oct 11, 2021 at 4:28 PM Eric Blake <ebl...@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 <leob...@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