Re: [PATCH v3 01/23] multifd: Delete useless operation

[Dr. David Alan Gilbert]

* Juan Quintela (quintela@redhat.com) wrote:
> We are divining by page_size to multiply again in the only use.
             ^--- typo
> Once there, impreve the comments.
                  ^--- typo
> 
> Signed-off-by: Juan Quintela <quintela@redhat.com>

OK, with the typo's fixed:

Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>

but, could you also explain the  x 2 (that's no worse than the current
code); is this defined somewhere in zlib?  I thought there was a routine
that told you the worst case?

Dave
> ---
>  migration/multifd-zlib.c | 13 ++++---------
>  migration/multifd-zstd.c | 13 ++++---------
>  2 files changed, 8 insertions(+), 18 deletions(-)
> 
> diff --git a/migration/multifd-zlib.c b/migration/multifd-zlib.c
> index ab4ba75d75..3fc7813b44 100644
> --- a/migration/multifd-zlib.c
> +++ b/migration/multifd-zlib.c
> @@ -42,7 +42,6 @@ struct zlib_data {
>   */
>  static int zlib_send_setup(MultiFDSendParams *p, Error **errp)
>  {
> -    uint32_t page_count = MULTIFD_PACKET_SIZE / qemu_target_page_size();
>      struct zlib_data *z = g_malloc0(sizeof(struct zlib_data));
>      z_stream *zs = &z->zs;
>  
> @@ -54,9 +53,8 @@ static int zlib_send_setup(MultiFDSendParams *p, Error 
> **errp)
>          error_setg(errp, "multifd %d: deflate init failed", p->id);
>          return -1;
>      }
> -    /* We will never have more than page_count pages */
> -    z->zbuff_len = page_count * qemu_target_page_size();
> -    z->zbuff_len *= 2;
> +    /* To be safe, we reserve twice the size of the packet */
> +    z->zbuff_len = MULTIFD_PACKET_SIZE * 2;
>      z->zbuff = g_try_malloc(z->zbuff_len);
>      if (!z->zbuff) {
>          deflateEnd(&z->zs);
> @@ -180,7 +178,6 @@ static int zlib_send_write(MultiFDSendParams *p, uint32_t 
> used, Error **errp)
>   */
>  static int zlib_recv_setup(MultiFDRecvParams *p, Error **errp)
>  {
> -    uint32_t page_count = MULTIFD_PACKET_SIZE / qemu_target_page_size();
>      struct zlib_data *z = g_malloc0(sizeof(struct zlib_data));
>      z_stream *zs = &z->zs;
>  
> @@ -194,10 +191,8 @@ static int zlib_recv_setup(MultiFDRecvParams *p, Error 
> **errp)
>          error_setg(errp, "multifd %d: inflate init failed", p->id);
>          return -1;
>      }
> -    /* We will never have more than page_count pages */
> -    z->zbuff_len = page_count * qemu_target_page_size();
> -    /* We know compression "could" use more space */
> -    z->zbuff_len *= 2;
> +    /* To be safe, we reserve twice the size of the packet */
> +    z->zbuff_len = MULTIFD_PACKET_SIZE * 2;
>      z->zbuff = g_try_malloc(z->zbuff_len);
>      if (!z->zbuff) {
>          inflateEnd(zs);
> diff --git a/migration/multifd-zstd.c b/migration/multifd-zstd.c
> index 693bddf8c9..cc3b8869c0 100644
> --- a/migration/multifd-zstd.c
> +++ b/migration/multifd-zstd.c
> @@ -47,7 +47,6 @@ struct zstd_data {
>   */
>  static int zstd_send_setup(MultiFDSendParams *p, Error **errp)
>  {
> -    uint32_t page_count = MULTIFD_PACKET_SIZE / qemu_target_page_size();
>      struct zstd_data *z = g_new0(struct zstd_data, 1);
>      int res;
>  
> @@ -67,9 +66,8 @@ static int zstd_send_setup(MultiFDSendParams *p, Error 
> **errp)
>                     p->id, ZSTD_getErrorName(res));
>          return -1;
>      }
> -    /* We will never have more than page_count pages */
> -    z->zbuff_len = page_count * qemu_target_page_size();
> -    z->zbuff_len *= 2;
> +    /* To be safe, we reserve twice the size of the packet */
> +    z->zbuff_len = MULTIFD_PACKET_SIZE * 2;
>      z->zbuff = g_try_malloc(z->zbuff_len);
>      if (!z->zbuff) {
>          ZSTD_freeCStream(z->zcs);
> @@ -191,7 +189,6 @@ static int zstd_send_write(MultiFDSendParams *p, uint32_t 
> used, Error **errp)
>   */
>  static int zstd_recv_setup(MultiFDRecvParams *p, Error **errp)
>  {
> -    uint32_t page_count = MULTIFD_PACKET_SIZE / qemu_target_page_size();
>      struct zstd_data *z = g_new0(struct zstd_data, 1);
>      int ret;
>  
> @@ -212,10 +209,8 @@ static int zstd_recv_setup(MultiFDRecvParams *p, Error 
> **errp)
>          return -1;
>      }
>  
> -    /* We will never have more than page_count pages */
> -    z->zbuff_len = page_count * qemu_target_page_size();
> -    /* We know compression "could" use more space */
> -    z->zbuff_len *= 2;
> +    /* To be safe, we reserve twice the size of the packet */
> +    z->zbuff_len = MULTIFD_PACKET_SIZE * 2;
>      z->zbuff = g_try_malloc(z->zbuff_len);
>      if (!z->zbuff) {
>          ZSTD_freeDStream(z->zds);
> -- 
> 2.33.1
> 
-- 
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK