On Fri, May 29, 2026 at 10:13:24AM +0800, Bin Guo wrote: > In qio_channel_socket_readv() and qio_channel_socket_writev(), the > control message buffer is unconditionally zeroed on every call, even > when no file descriptors are being transferred. The memset touches > CMSG_SPACE(sizeof(int) * SOCKET_MAX_FDS) bytes -- typically 80 bytes > on x86-64 -- on every I/O operation.
Did you have a real world scenario where this creates an I/O performance problem, or is this just a optimization to address a theoretical concern ? > Move the memset into the conditional branches that actually set up > msg_control, so the common fast path (no fd passing) avoids the > unnecessary zeroing. Also narrow the scope of fdsize and cmsg in > writev() to the branch that uses them. > > No functional change for paths that transfer file descriptors. > > Signed-off-by: Bin Guo <[email protected]> > --- > io/channel-socket.c | 11 +++++------ > 1 file changed, 5 insertions(+), 6 deletions(-) > > diff --git a/io/channel-socket.c b/io/channel-socket.c > index 3053b35ad8..d19c73cacc 100644 > --- a/io/channel-socket.c > +++ b/io/channel-socket.c > @@ -562,11 +562,10 @@ static ssize_t qio_channel_socket_readv(QIOChannel *ioc, > char control[CMSG_SPACE(sizeof(int) * SOCKET_MAX_FDS)]; QEMU builds with -ftrivial-auto-var-init=zero always set so this control variable will always be zero-initialized by the compiler making the following memset() pointless/redundant. AFAICT, the compiler optimization will identify this and remove the memset() call entirely. IOW, I don't believe the change proposed in your patch will have any effect. If we want to be explicit about this then we should just do char control[CMSG_SPACE(sizeof(int) * SOCKET_MAX_FDS)] = {}; and remove the memset() from our code entirely. > int sflags = 0; > > - memset(control, 0, CMSG_SPACE(sizeof(int) * SOCKET_MAX_FDS)); > - > msg.msg_iov = (struct iovec *)iov; > msg.msg_iovlen = niov; > if (fds && nfds) { > + memset(control, 0, CMSG_SPACE(sizeof(int) * SOCKET_MAX_FDS)); > msg.msg_control = control; > msg.msg_controllen = sizeof(control); > #ifdef MSG_CMSG_CLOEXEC > @@ -622,20 +621,19 @@ static ssize_t qio_channel_socket_writev(QIOChannel > *ioc, > ssize_t ret; > struct msghdr msg = { NULL, }; > char control[CMSG_SPACE(sizeof(int) * SOCKET_MAX_FDS)]; > - size_t fdsize = sizeof(int) * nfds; > - struct cmsghdr *cmsg; > int sflags = 0; > #ifdef QEMU_MSG_ZEROCOPY > bool blocking = sioc->blocking; > bool zerocopy_flushed_once = false; > #endif > > - memset(control, 0, CMSG_SPACE(sizeof(int) * SOCKET_MAX_FDS)); > - > msg.msg_iov = (struct iovec *)iov; > msg.msg_iovlen = niov; > > if (nfds) { > + size_t fdsize = sizeof(int) * nfds; > + struct cmsghdr *cmsg; > + > if (nfds > SOCKET_MAX_FDS) { > error_setg_errno(errp, EINVAL, > "Only %d FDs can be sent, got %zu", > @@ -643,6 +641,7 @@ static ssize_t qio_channel_socket_writev(QIOChannel *ioc, > return -1; > } > > + memset(control, 0, CMSG_SPACE(sizeof(int) * SOCKET_MAX_FDS)); > msg.msg_control = control; > msg.msg_controllen = CMSG_SPACE(sizeof(int) * nfds); > > -- > 2.50.1 (Apple Git-155) > > With regards, Daniel -- |: https://berrange.com ~~ https://hachyderm.io/@berrange :| |: https://libvirt.org ~~ https://entangle-photo.org :| |: https://pixelfed.art/berrange ~~ https://fstop138.berrange.com :|
