Re: [PATCH v6 2/7] migration/multifd: put IOV initialization into compression method

2024-05-28 Thread Peter Xu 
On Tue, May 28, 2024 at 01:36:38PM +, Liu, Yuan1 wrote:
> > -Original Message-
> > From: Peter Xu 
> > Sent: Tuesday, May 28, 2024 4:51 AM
> > To: Liu, Yuan1 
> > Cc: faro...@suse.de; qemu-devel@nongnu.org; Zou, Nanhai
> > 
> > Subject: Re: [PATCH v6 2/7] migration/multifd: put IOV initialization into
> > compression method
> > 
> > On Mon, May 06, 2024 at 12:57:46AM +0800, Yuan Liu wrote:
> > > Different compression methods may require different numbers of IOVs.
> > > Based on streaming compression of zlib and zstd, all pages will be
> > > compressed to a data block, so two IOVs are needed for packet header
> > > and compressed data block.
> > >
> > > Signed-off-by: Yuan Liu 
> > > Reviewed-by: Nanhai Zou 
> > > ---
> > >  migration/multifd-zlib.c |  7 +++
> > >  migration/multifd-zstd.c |  8 +++-
> > >  migration/multifd.c  | 22 --
> > >  3 files changed, 26 insertions(+), 11 deletions(-)
> > >
> > > diff --git a/migration/multifd-zlib.c b/migration/multifd-zlib.c
> > > index 737a9645d2..2ced69487e 100644
> > > --- a/migration/multifd-zlib.c
> > > +++ b/migration/multifd-zlib.c
> > > @@ -70,6 +70,10 @@ static int zlib_send_setup(MultiFDSendParams *p,
> > Error **errp)
> > >  goto err_free_zbuff;
> > >  }
> > >  p->compress_data = z;
> > > +
> > > +/* Needs 2 IOVs, one for packet header and one for compressed data
> > */
> > > +p->iov = g_new0(struct iovec, 2);
> > > +
> > >  return 0;
> > >
> > >  err_free_zbuff:
> > > @@ -101,6 +105,9 @@ static void zlib_send_cleanup(MultiFDSendParams *p,
> > Error **errp)
> > >  z->buf = NULL;
> > >  g_free(p->compress_data);
> > >  p->compress_data = NULL;
> > > +
> > > +g_free(p->iov);
> > > +p->iov = NULL;
> > >  }
> > >
> > >  /**
> > > diff --git a/migration/multifd-zstd.c b/migration/multifd-zstd.c
> > > index 256858df0a..ca17b7e310 100644
> > > --- a/migration/multifd-zstd.c
> > > +++ b/migration/multifd-zstd.c
> > > @@ -52,7 +52,6 @@ static int zstd_send_setup(MultiFDSendParams *p, Error
> > **errp)
> > >  struct zstd_data *z = g_new0(struct zstd_data, 1);
> > >  int res;
> > >
> > > -p->compress_data = z;
> > >  z->zcs = ZSTD_createCStream();
> > >  if (!z->zcs) {
> > >  g_free(z);
> > > @@ -77,6 +76,10 @@ static int zstd_send_setup(MultiFDSendParams *p,
> > Error **errp)
> > >  error_setg(errp, "multifd %u: out of memory for zbuff", p->id);
> > >  return -1;
> > >  }
> > > +p->compress_data = z;
> > > +
> > > +/* Needs 2 IOVs, one for packet header and one for compressed data
> > */
> > > +p->iov = g_new0(struct iovec, 2);
> > >  return 0;
> > >  }
> > >
> > > @@ -98,6 +101,9 @@ static void zstd_send_cleanup(MultiFDSendParams *p,
> > Error **errp)
> > >  z->zbuff = NULL;
> > >  g_free(p->compress_data);
> > >  p->compress_data = NULL;
> > > +
> > > +g_free(p->iov);
> > > +p->iov = NULL;
> > >  }
> > >
> > >  /**
> > > diff --git a/migration/multifd.c b/migration/multifd.c
> > > index f317bff077..d82885fdbb 100644
> > > --- a/migration/multifd.c
> > > +++ b/migration/multifd.c
> > > @@ -137,6 +137,13 @@ static int nocomp_send_setup(MultiFDSendParams *p,
> > Error **errp)
> > >  p->write_flags |= QIO_CHANNEL_WRITE_FLAG_ZERO_COPY;
> > >  }
> > >
> > > +if (multifd_use_packets()) {
> > > +/* We need one extra place for the packet header */
> > > +p->iov = g_new0(struct iovec, p->page_count + 1);
> > > +} else {
> > > +p->iov = g_new0(struct iovec, p->page_count);
> > > +}
> > > +
> > >  return 0;
> > >  }
> > >
> > > @@ -150,6 +157,8 @@ static int nocomp_send_setup(MultiFDSendParams *p,
> > Error **errp)
> > >   */
> > >  static void nocomp_send_cleanup(MultiFDSendParams *p, Error **errp)
> > >  {
> > > +g_free(p->iov);
> > > +p->iov = NULL;
> > >  return;
> > >  }
> &g

RE: [PATCH v6 2/7] migration/multifd: put IOV initialization into compression method

2024-05-28 Thread Liu, Yuan1 
> -Original Message-
> From: Peter Xu 
> Sent: Tuesday, May 28, 2024 4:51 AM
> To: Liu, Yuan1 
> Cc: faro...@suse.de; qemu-devel@nongnu.org; Zou, Nanhai
> 
> Subject: Re: [PATCH v6 2/7] migration/multifd: put IOV initialization into
> compression method
> 
> On Mon, May 06, 2024 at 12:57:46AM +0800, Yuan Liu wrote:
> > Different compression methods may require different numbers of IOVs.
> > Based on streaming compression of zlib and zstd, all pages will be
> > compressed to a data block, so two IOVs are needed for packet header
> > and compressed data block.
> >
> > Signed-off-by: Yuan Liu 
> > Reviewed-by: Nanhai Zou 
> > ---
> >  migration/multifd-zlib.c |  7 +++
> >  migration/multifd-zstd.c |  8 +++-
> >  migration/multifd.c  | 22 --
> >  3 files changed, 26 insertions(+), 11 deletions(-)
> >
> > diff --git a/migration/multifd-zlib.c b/migration/multifd-zlib.c
> > index 737a9645d2..2ced69487e 100644
> > --- a/migration/multifd-zlib.c
> > +++ b/migration/multifd-zlib.c
> > @@ -70,6 +70,10 @@ static int zlib_send_setup(MultiFDSendParams *p,
> Error **errp)
> >  goto err_free_zbuff;
> >  }
> >  p->compress_data = z;
> > +
> > +/* Needs 2 IOVs, one for packet header and one for compressed data
> */
> > +p->iov = g_new0(struct iovec, 2);
> > +
> >  return 0;
> >
> >  err_free_zbuff:
> > @@ -101,6 +105,9 @@ static void zlib_send_cleanup(MultiFDSendParams *p,
> Error **errp)
> >  z->buf = NULL;
> >  g_free(p->compress_data);
> >  p->compress_data = NULL;
> > +
> > +g_free(p->iov);
> > +p->iov = NULL;
> >  }
> >
> >  /**
> > diff --git a/migration/multifd-zstd.c b/migration/multifd-zstd.c
> > index 256858df0a..ca17b7e310 100644
> > --- a/migration/multifd-zstd.c
> > +++ b/migration/multifd-zstd.c
> > @@ -52,7 +52,6 @@ static int zstd_send_setup(MultiFDSendParams *p, Error
> **errp)
> >  struct zstd_data *z = g_new0(struct zstd_data, 1);
> >  int res;
> >
> > -p->compress_data = z;
> >  z->zcs = ZSTD_createCStream();
> >  if (!z->zcs) {
> >  g_free(z);
> > @@ -77,6 +76,10 @@ static int zstd_send_setup(MultiFDSendParams *p,
> Error **errp)
> >  error_setg(errp, "multifd %u: out of memory for zbuff", p->id);
> >  return -1;
> >  }
> > +p->compress_data = z;
> > +
> > +/* Needs 2 IOVs, one for packet header and one for compressed data
> */
> > +p->iov = g_new0(struct iovec, 2);
> >  return 0;
> >  }
> >
> > @@ -98,6 +101,9 @@ static void zstd_send_cleanup(MultiFDSendParams *p,
> Error **errp)
> >  z->zbuff = NULL;
> >  g_free(p->compress_data);
> >  p->compress_data = NULL;
> > +
> > +g_free(p->iov);
> > +p->iov = NULL;
> >  }
> >
> >  /**
> > diff --git a/migration/multifd.c b/migration/multifd.c
> > index f317bff077..d82885fdbb 100644
> > --- a/migration/multifd.c
> > +++ b/migration/multifd.c
> > @@ -137,6 +137,13 @@ static int nocomp_send_setup(MultiFDSendParams *p,
> Error **errp)
> >  p->write_flags |= QIO_CHANNEL_WRITE_FLAG_ZERO_COPY;
> >  }
> >
> > +if (multifd_use_packets()) {
> > +/* We need one extra place for the packet header */
> > +p->iov = g_new0(struct iovec, p->page_count + 1);
> > +} else {
> > +p->iov = g_new0(struct iovec, p->page_count);
> > +}
> > +
> >  return 0;
> >  }
> >
> > @@ -150,6 +157,8 @@ static int nocomp_send_setup(MultiFDSendParams *p,
> Error **errp)
> >   */
> >  static void nocomp_send_cleanup(MultiFDSendParams *p, Error **errp)
> >  {
> > +g_free(p->iov);
> > +p->iov = NULL;
> >  return;
> >  }
> >
> > @@ -228,6 +237,7 @@ static int nocomp_send_prepare(MultiFDSendParams *p,
> Error **errp)
> >   */
> >  static int nocomp_recv_setup(MultiFDRecvParams *p, Error **errp)
> >  {
> > +p->iov = g_new0(struct iovec, p->page_count);
> >  return 0;
> >  }
> >
> > @@ -240,6 +250,8 @@ static int nocomp_recv_setup(MultiFDRecvParams *p,
> Error **errp)
> >   */
> >  static void nocomp_recv_cleanup(MultiFDRecvParams *p)
> >  {
> > +g_free(p->iov);
> > +p->iov = NULL;
> >  }
> 
> Are recv_

Re: [PATCH v6 2/7] migration/multifd: put IOV initialization into compression method

2024-05-27 Thread Peter Xu 
On Mon, May 06, 2024 at 12:57:46AM +0800, Yuan Liu wrote:
> Different compression methods may require different numbers of IOVs.
> Based on streaming compression of zlib and zstd, all pages will be
> compressed to a data block, so two IOVs are needed for packet header
> and compressed data block.
> 
> Signed-off-by: Yuan Liu 
> Reviewed-by: Nanhai Zou 
> ---
>  migration/multifd-zlib.c |  7 +++
>  migration/multifd-zstd.c |  8 +++-
>  migration/multifd.c  | 22 --
>  3 files changed, 26 insertions(+), 11 deletions(-)
> 
> diff --git a/migration/multifd-zlib.c b/migration/multifd-zlib.c
> index 737a9645d2..2ced69487e 100644
> --- a/migration/multifd-zlib.c
> +++ b/migration/multifd-zlib.c
> @@ -70,6 +70,10 @@ static int zlib_send_setup(MultiFDSendParams *p, Error 
> **errp)
>  goto err_free_zbuff;
>  }
>  p->compress_data = z;
> +
> +/* Needs 2 IOVs, one for packet header and one for compressed data */
> +p->iov = g_new0(struct iovec, 2);
> +
>  return 0;
>  
>  err_free_zbuff:
> @@ -101,6 +105,9 @@ static void zlib_send_cleanup(MultiFDSendParams *p, Error 
> **errp)
>  z->buf = NULL;
>  g_free(p->compress_data);
>  p->compress_data = NULL;
> +
> +g_free(p->iov);
> +p->iov = NULL;
>  }
>  
>  /**
> diff --git a/migration/multifd-zstd.c b/migration/multifd-zstd.c
> index 256858df0a..ca17b7e310 100644
> --- a/migration/multifd-zstd.c
> +++ b/migration/multifd-zstd.c
> @@ -52,7 +52,6 @@ static int zstd_send_setup(MultiFDSendParams *p, Error 
> **errp)
>  struct zstd_data *z = g_new0(struct zstd_data, 1);
>  int res;
>  
> -p->compress_data = z;
>  z->zcs = ZSTD_createCStream();
>  if (!z->zcs) {
>  g_free(z);
> @@ -77,6 +76,10 @@ static int zstd_send_setup(MultiFDSendParams *p, Error 
> **errp)
>  error_setg(errp, "multifd %u: out of memory for zbuff", p->id);
>  return -1;
>  }
> +p->compress_data = z;
> +
> +/* Needs 2 IOVs, one for packet header and one for compressed data */
> +p->iov = g_new0(struct iovec, 2);
>  return 0;
>  }
>  
> @@ -98,6 +101,9 @@ static void zstd_send_cleanup(MultiFDSendParams *p, Error 
> **errp)
>  z->zbuff = NULL;
>  g_free(p->compress_data);
>  p->compress_data = NULL;
> +
> +g_free(p->iov);
> +p->iov = NULL;
>  }
>  
>  /**
> diff --git a/migration/multifd.c b/migration/multifd.c
> index f317bff077..d82885fdbb 100644
> --- a/migration/multifd.c
> +++ b/migration/multifd.c
> @@ -137,6 +137,13 @@ static int nocomp_send_setup(MultiFDSendParams *p, Error 
> **errp)
>  p->write_flags |= QIO_CHANNEL_WRITE_FLAG_ZERO_COPY;
>  }
>  
> +if (multifd_use_packets()) {
> +/* We need one extra place for the packet header */
> +p->iov = g_new0(struct iovec, p->page_count + 1);
> +} else {
> +p->iov = g_new0(struct iovec, p->page_count);
> +}
> +
>  return 0;
>  }
>  
> @@ -150,6 +157,8 @@ static int nocomp_send_setup(MultiFDSendParams *p, Error 
> **errp)
>   */
>  static void nocomp_send_cleanup(MultiFDSendParams *p, Error **errp)
>  {
> +g_free(p->iov);
> +p->iov = NULL;
>  return;
>  }
>  
> @@ -228,6 +237,7 @@ static int nocomp_send_prepare(MultiFDSendParams *p, 
> Error **errp)
>   */
>  static int nocomp_recv_setup(MultiFDRecvParams *p, Error **errp)
>  {
> +p->iov = g_new0(struct iovec, p->page_count);
>  return 0;
>  }
>  
> @@ -240,6 +250,8 @@ static int nocomp_recv_setup(MultiFDRecvParams *p, Error 
> **errp)
>   */
>  static void nocomp_recv_cleanup(MultiFDRecvParams *p)
>  {
> +g_free(p->iov);
> +p->iov = NULL;
>  }

Are recv_setup()/recv_cleanup() for zstd/zlib missing?

If the iov will be managed by the compressors, I'm wondering whether we
should start assert(p->iov) after send|recv_setup(), and assert(!p->iov)
after send|recv_cleanup().  That'll guard all these.

>  
>  /**
> @@ -783,8 +795,6 @@ static bool 
> multifd_send_cleanup_channel(MultiFDSendParams *p, Error **errp)
>  p->packet_len = 0;
>  g_free(p->packet);
>  p->packet = NULL;
> -g_free(p->iov);
> -p->iov = NULL;
>  multifd_send_state->ops->send_cleanup(p, errp);
>  
>  return *errp == NULL;
> @@ -1179,11 +1189,6 @@ bool multifd_send_setup(void)
>  p->packet = g_malloc0(p->packet_len);
>  p->packet->magic = cpu_to_be32(MULTIFD_MAGIC);
>  p->packet->version = cpu_to_be32(MULTIFD_VERSION);
> -
> -/* We need one extra place for the packet header */
> -p->iov = g_new0(struct iovec, page_count + 1);
> -} else {
> -p->iov = g_new0(struct iovec, page_count);
>  }
>  p->name = g_strdup_printf("multifdsend_%d", i);
>  p->page_size = qemu_target_page_size();
> @@ -1353,8 +1358,6 @@ static void 
> multifd_recv_cleanup_channel(MultiFDRecvParams *p)
>  p->packet_len = 0;
>  g_free(p->packet);
>  p->packet = NULL;
> -g_free(p->iov);
> -p->iov

Re: [PATCH v6 2/7] migration/multifd: put IOV initialization into compression method

2024-05-10 Thread Fabiano Rosas 
Yuan Liu  writes:

> Different compression methods may require different numbers of IOVs.
> Based on streaming compression of zlib and zstd, all pages will be
> compressed to a data block, so two IOVs are needed for packet header
> and compressed data block.
>
> Signed-off-by: Yuan Liu 
> Reviewed-by: Nanhai Zou 

Reviewed-by: Fabiano Rosas

[PATCH v6 2/7] migration/multifd: put IOV initialization into compression method

2024-05-06 Thread Yuan Liu 
Different compression methods may require different numbers of IOVs.
Based on streaming compression of zlib and zstd, all pages will be
compressed to a data block, so two IOVs are needed for packet header
and compressed data block.

Signed-off-by: Yuan Liu 
Reviewed-by: Nanhai Zou 
---
 migration/multifd-zlib.c |  7 +++
 migration/multifd-zstd.c |  8 +++-
 migration/multifd.c  | 22 --
 3 files changed, 26 insertions(+), 11 deletions(-)

diff --git a/migration/multifd-zlib.c b/migration/multifd-zlib.c
index 737a9645d2..2ced69487e 100644
--- a/migration/multifd-zlib.c
+++ b/migration/multifd-zlib.c
@@ -70,6 +70,10 @@ static int zlib_send_setup(MultiFDSendParams *p, Error 
**errp)
 goto err_free_zbuff;
 }
 p->compress_data = z;
+
+/* Needs 2 IOVs, one for packet header and one for compressed data */
+p->iov = g_new0(struct iovec, 2);
+
 return 0;
 
 err_free_zbuff:
@@ -101,6 +105,9 @@ static void zlib_send_cleanup(MultiFDSendParams *p, Error 
**errp)
 z->buf = NULL;
 g_free(p->compress_data);
 p->compress_data = NULL;
+
+g_free(p->iov);
+p->iov = NULL;
 }
 
 /**
diff --git a/migration/multifd-zstd.c b/migration/multifd-zstd.c
index 256858df0a..ca17b7e310 100644
--- a/migration/multifd-zstd.c
+++ b/migration/multifd-zstd.c
@@ -52,7 +52,6 @@ static int zstd_send_setup(MultiFDSendParams *p, Error **errp)
 struct zstd_data *z = g_new0(struct zstd_data, 1);
 int res;
 
-p->compress_data = z;
 z->zcs = ZSTD_createCStream();
 if (!z->zcs) {
 g_free(z);
@@ -77,6 +76,10 @@ static int zstd_send_setup(MultiFDSendParams *p, Error 
**errp)
 error_setg(errp, "multifd %u: out of memory for zbuff", p->id);
 return -1;
 }
+p->compress_data = z;
+
+/* Needs 2 IOVs, one for packet header and one for compressed data */
+p->iov = g_new0(struct iovec, 2);
 return 0;
 }
 
@@ -98,6 +101,9 @@ static void zstd_send_cleanup(MultiFDSendParams *p, Error 
**errp)
 z->zbuff = NULL;
 g_free(p->compress_data);
 p->compress_data = NULL;
+
+g_free(p->iov);
+p->iov = NULL;
 }
 
 /**
diff --git a/migration/multifd.c b/migration/multifd.c
index f317bff077..d82885fdbb 100644
--- a/migration/multifd.c
+++ b/migration/multifd.c
@@ -137,6 +137,13 @@ static int nocomp_send_setup(MultiFDSendParams *p, Error 
**errp)
 p->write_flags |= QIO_CHANNEL_WRITE_FLAG_ZERO_COPY;
 }
 
+if (multifd_use_packets()) {
+/* We need one extra place for the packet header */
+p->iov = g_new0(struct iovec, p->page_count + 1);
+} else {
+p->iov = g_new0(struct iovec, p->page_count);
+}
+
 return 0;
 }
 
@@ -150,6 +157,8 @@ static int nocomp_send_setup(MultiFDSendParams *p, Error 
**errp)
  */
 static void nocomp_send_cleanup(MultiFDSendParams *p, Error **errp)
 {
+g_free(p->iov);
+p->iov = NULL;
 return;
 }
 
@@ -228,6 +237,7 @@ static int nocomp_send_prepare(MultiFDSendParams *p, Error 
**errp)
  */
 static int nocomp_recv_setup(MultiFDRecvParams *p, Error **errp)
 {
+p->iov = g_new0(struct iovec, p->page_count);
 return 0;
 }
 
@@ -240,6 +250,8 @@ static int nocomp_recv_setup(MultiFDRecvParams *p, Error 
**errp)
  */
 static void nocomp_recv_cleanup(MultiFDRecvParams *p)
 {
+g_free(p->iov);
+p->iov = NULL;
 }
 
 /**
@@ -783,8 +795,6 @@ static bool multifd_send_cleanup_channel(MultiFDSendParams 
*p, Error **errp)
 p->packet_len = 0;
 g_free(p->packet);
 p->packet = NULL;
-g_free(p->iov);
-p->iov = NULL;
 multifd_send_state->ops->send_cleanup(p, errp);
 
 return *errp == NULL;
@@ -1179,11 +1189,6 @@ bool multifd_send_setup(void)
 p->packet = g_malloc0(p->packet_len);
 p->packet->magic = cpu_to_be32(MULTIFD_MAGIC);
 p->packet->version = cpu_to_be32(MULTIFD_VERSION);
-
-/* We need one extra place for the packet header */
-p->iov = g_new0(struct iovec, page_count + 1);
-} else {
-p->iov = g_new0(struct iovec, page_count);
 }
 p->name = g_strdup_printf("multifdsend_%d", i);
 p->page_size = qemu_target_page_size();
@@ -1353,8 +1358,6 @@ static void 
multifd_recv_cleanup_channel(MultiFDRecvParams *p)
 p->packet_len = 0;
 g_free(p->packet);
 p->packet = NULL;
-g_free(p->iov);
-p->iov = NULL;
 g_free(p->normal);
 p->normal = NULL;
 g_free(p->zero);
@@ -1602,7 +1605,6 @@ int multifd_recv_setup(Error **errp)
 p->packet = g_malloc0(p->packet_len);
 }
 p->name = g_strdup_printf("multifdrecv_%d", i);
-p->iov = g_new0(struct iovec, page_count);
 p->normal = g_new0(ram_addr_t, page_count);
 p->zero = g_new0(ram_addr_t, page_count);
 p->page_count = page_count;
-- 
2.39.3