On Wed, Jun 01, 2022 at 05:37:10PM +0800, 徐闯 wrote: > > On 2022/5/13 下午2:28, Leonardo Bras wrote: > > For CONFIG_LINUX, implement the new zero copy flag and the optional callback > > io_flush 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_flush() was implemented by counting how many times > > sendmsg(...,MSG_ZEROCOPY) was successfully 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 > > occurs. > > > > Notes on using writev() with QIO_CHANNEL_WRITE_FLAG_ZERO_COPY: > > 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 qio_channel_flush() 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_zero_copy > > will return an error, which can abort an operation like migration, > > - Because of this, when an user code wants to add zero copy as a feature, it > > requires a mechanism to disable it, so it can still be accessible to less > > privileged users. > > > > Signed-off-by: Leonardo Bras <leob...@redhat.com> > > Reviewed-by: Peter Xu <pet...@redhat.com> > > Reviewed-by: Daniel P. Berrangé <berra...@redhat.com> > > Reviewed-by: Juan Quintela <quint...@redhat.com> > > --- > > include/io/channel-socket.h | 2 + > > io/channel-socket.c | 116 ++++++++++++++++++++++++++++++++++-- > > 2 files changed, 114 insertions(+), 4 deletions(-) > > > > diff --git a/include/io/channel-socket.h b/include/io/channel-socket.h > > index e747e63514..513c428fe4 100644 > > --- a/include/io/channel-socket.h > > +++ b/include/io/channel-socket.h > > @@ -47,6 +47,8 @@ struct QIOChannelSocket { > > socklen_t localAddrLen; > > struct sockaddr_storage remoteAddr; > > socklen_t remoteAddrLen; > > + ssize_t zero_copy_queued; > > + ssize_t zero_copy_sent; > > }; > Hi, Leonardo. I'm also paying attention to the application of MSG_ZEROCOPY > in live migration recently. I noticed that you defined a member > `zero_copy_queued` in the struct QIOChannelSocket, but I can't find out > where the value of this member has been changed in your patch. Can you > answer it for me? >
Good point.. it should probably be increased when queuing the pages. We'd better fix it up or it seems the flush() will be literally an no-op.. Two things in qio_channel_socket_flush() we can do to make sure it'll work as expected, imo: 1) make ret=-1 as initial value, rather than 1 - we only check negative errors in the caller so we could have missed a positive "1" 2) add a tracepoint into the loop of updating zero_copy_sent Leo, what's your take? Thanks, -- Peter Xu