1. Patchew
  2. QEMU
  3. View series

[PATCH] io/channel-socket: move cmsg memset into fd-passing branches

Bin Guo posted 1 patch 1 month, 2 weeks ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20260529021324.52124-1-guobin@linux.alibaba.com
Maintainers: "Daniel P. Berrangé" <berrange@redhat.com>
io/channel-socket.c | 11 +++++------
1 file changed, 5 insertions(+), 6 deletions(-)
[PATCH] io/channel-socket: move cmsg memset into fd-passing branches
Posted by Bin Guo 1 month, 2 weeks ago 
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.

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 <guobin@linux.alibaba.com>
---
 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)];
     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)
Re: [PATCH] io/channel-socket: move cmsg memset into fd-passing branches
Posted by Daniel P. Berrangé 1 month, 2 weeks ago 
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 <guobin@linux.alibaba.com>
> ---
>  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 :|
Re: [PATCH] io/channel-socket: move cmsg memset into fd-passing branches
Posted by Bin Guo 1 month, 2 weeks ago 
On Wed, Jun 04, 2026 at 08:42:xx +0000, Daniel P. Berrangé wrote:
> 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.

You're right, I missed the -ftrivial-auto-var-init=zero flag in
meson.build.  The memset() is indeed redundant and the performance
rationale in the patch description doesn't hold.

I'll drop this patch.

Thanks for the review.

Bin Guo

Patchew 2.1-devel-ea86937

© 2016 - 2026 Red Hat, Inc.