[PATCH 3/5] ram compress: Refactor ram compression functions

Yuan Liu posted 5 patches 1 year, 1 month ago
Maintainers: Paolo Bonzini <pbonzini@redhat.com>, "Marc-André Lureau" <marcandre.lureau@redhat.com>, "Daniel P. Berrangé" <berrange@redhat.com>, Thomas Huth <thuth@redhat.com>, "Philippe Mathieu-Daudé" <philmd@linaro.org>, Juan Quintela <quintela@redhat.com>, Peter Xu <peterx@redhat.com>, Fabiano Rosas <farosas@suse.de>, Leonardo Bras <leobras@redhat.com>, Eric Blake <eblake@redhat.com>, Markus Armbruster <armbru@redhat.com>
There is a newer version of this series
[PATCH 3/5] ram compress: Refactor ram compression functions
Posted by Yuan Liu 1 year, 1 month ago
Refactor legacy RAM compression functions to support both IAA
compression and CPU compression.

Signed-off-by: Yuan Liu <yuan1.liu@intel.com>
Reviewed-by: Nanhai Zou <nanhai.zou@intel.com>
---
 migration/migration.c    |  6 +--
 migration/ram-compress.c | 81 ++++++++++++++++++++++++++++++++--------
 migration/ram-compress.h | 10 ++---
 migration/ram.c          | 18 ++++++---
 4 files changed, 86 insertions(+), 29 deletions(-)

diff --git a/migration/migration.c b/migration/migration.c
index 585d3c8f55..08a9c313d0 100644
--- a/migration/migration.c
+++ b/migration/migration.c
@@ -237,7 +237,7 @@ void migration_incoming_state_destroy(void)
     struct MigrationIncomingState *mis = migration_incoming_get_current();
 
     multifd_load_cleanup();
-    compress_threads_load_cleanup();
+    ram_compress_load_cleanup();
 
     if (mis->to_src_file) {
         /* Tell source that we are done */
@@ -524,7 +524,7 @@ process_incoming_migration_co(void *opaque)
 
     assert(mis->from_src_file);
 
-    if (compress_threads_load_setup(mis->from_src_file)) {
+    if (ram_compress_load_setup(mis->from_src_file)) {
         error_report("Failed to setup decompress threads");
         goto fail;
     }
@@ -577,7 +577,7 @@ fail:
     qemu_fclose(mis->from_src_file);
 
     multifd_load_cleanup();
-    compress_threads_load_cleanup();
+    ram_compress_load_cleanup();
 
     exit(EXIT_FAILURE);
 }
diff --git a/migration/ram-compress.c b/migration/ram-compress.c
index 06254d8c69..47357352f7 100644
--- a/migration/ram-compress.c
+++ b/migration/ram-compress.c
@@ -105,11 +105,11 @@ static void *do_data_compress(void *opaque)
     return NULL;
 }
 
-void compress_threads_save_cleanup(void)
+static void compress_threads_save_cleanup(void)
 {
     int i, thread_count;
 
-    if (!migrate_compress() || !comp_param) {
+    if (!comp_param) {
         return;
     }
 
@@ -144,13 +144,10 @@ void compress_threads_save_cleanup(void)
     comp_param = NULL;
 }
 
-int compress_threads_save_setup(void)
+static int compress_threads_save_setup(void)
 {
     int i, thread_count;
 
-    if (!migrate_compress()) {
-        return 0;
-    }
     thread_count = migrate_compress_threads();
     compress_threads = g_new0(QemuThread, thread_count);
     comp_param = g_new0(CompressParam, thread_count);
@@ -370,6 +367,11 @@ int wait_for_decompress_done(void)
         return 0;
     }
 
+    if (migrate_compress_with_iaa()) {
+        /* Implement in next patch */
+        return 0;
+    }
+
     thread_count = migrate_decompress_threads();
     qemu_mutex_lock(&decomp_done_lock);
     for (idx = 0; idx < thread_count; idx++) {
@@ -381,13 +383,10 @@ int wait_for_decompress_done(void)
     return qemu_file_get_error(decomp_file);
 }
 
-void compress_threads_load_cleanup(void)
+static void compress_threads_load_cleanup(void)
 {
     int i, thread_count;
 
-    if (!migrate_compress()) {
-        return;
-    }
     thread_count = migrate_decompress_threads();
     for (i = 0; i < thread_count; i++) {
         /*
@@ -422,14 +421,10 @@ void compress_threads_load_cleanup(void)
     decomp_file = NULL;
 }
 
-int compress_threads_load_setup(QEMUFile *f)
+static int compress_threads_load_setup(QEMUFile *f)
 {
     int i, thread_count;
 
-    if (!migrate_compress()) {
-        return 0;
-    }
-
     thread_count = migrate_decompress_threads();
     decompress_threads = g_new0(QemuThread, thread_count);
     decomp_param = g_new0(DecompressParam, thread_count);
@@ -457,7 +452,7 @@ exit:
     return -1;
 }
 
-void decompress_data_with_multi_threads(QEMUFile *f, void *host, int len)
+static void decompress_data_with_multi_threads(QEMUFile *f, void *host, int len)
 {
     int idx, thread_count;
 
@@ -483,3 +478,57 @@ void decompress_data_with_multi_threads(QEMUFile *f, void *host, int len)
         }
     }
 }
+
+int ram_compress_save_setup(void)
+{
+    if (!migrate_compress()) {
+        return 0;
+    }
+    if (migrate_compress_with_iaa()) {
+        /* Implement in next patch */
+        return 0;
+    }
+    return compress_threads_save_setup();
+}
+
+void ram_compress_save_cleanup(void)
+{
+    if (!migrate_compress()) {
+        return;
+    }
+    if (migrate_compress_with_iaa()) {
+        /* Implement in next patch */
+        return;
+    }
+    compress_threads_save_cleanup();
+}
+
+void ram_decompress_data(QEMUFile *f, void *host, int len)
+{
+    if (migrate_compress_with_iaa()) {
+        /* Implement in next patch */
+    }
+    decompress_data_with_multi_threads(f, host, len);
+}
+
+int ram_compress_load_setup(QEMUFile *f)
+{
+    if (!migrate_compress()) {
+        return 0;
+    }
+    if (migrate_compress_with_iaa()) {
+        /* Implement in next patch */
+    }
+    return compress_threads_load_setup(f);
+}
+
+void ram_compress_load_cleanup(void)
+{
+    if (!migrate_compress()) {
+        return;
+    }
+    if (migrate_compress_with_iaa()) {
+        /* Implement in next patch */
+    }
+    compress_threads_load_cleanup();
+}
diff --git a/migration/ram-compress.h b/migration/ram-compress.h
index 6f7fe2f472..382083acf6 100644
--- a/migration/ram-compress.h
+++ b/migration/ram-compress.h
@@ -55,16 +55,16 @@ struct CompressParam {
 };
 typedef struct CompressParam CompressParam;
 
-void compress_threads_save_cleanup(void);
-int compress_threads_save_setup(void);
+void ram_compress_save_cleanup(void);
+int ram_compress_save_setup(void);
 
 void flush_compressed_data(int (send_queued_data(CompressParam *)));
 int compress_page_with_multi_thread(RAMBlock *block, ram_addr_t offset,
                                 int (send_queued_data(CompressParam *)));
 
 int wait_for_decompress_done(void);
-void compress_threads_load_cleanup(void);
-int compress_threads_load_setup(QEMUFile *f);
-void decompress_data_with_multi_threads(QEMUFile *f, void *host, int len);
+void ram_compress_load_cleanup(void);
+int ram_compress_load_setup(QEMUFile *f);
+void ram_decompress_data(QEMUFile *f, void *host, int len);
 
 #endif
diff --git a/migration/ram.c b/migration/ram.c
index e4bfd39f08..34ee1de332 100644
--- a/migration/ram.c
+++ b/migration/ram.c
@@ -1347,6 +1347,10 @@ static void ram_flush_compressed_data(RAMState *rs)
     if (!save_page_use_compression(rs)) {
         return;
     }
+    if (migrate_compress_with_iaa()) {
+        /* Implement in next patch */
+        return;
+    }
 
     flush_compressed_data(send_queued_data);
 }
@@ -2099,6 +2103,10 @@ static bool save_compress_page(RAMState *rs, PageSearchStatus *pss,
         return false;
     }
 
+    if (migrate_compress_with_iaa()) {
+        /* Implement in next patch */
+        return true;
+    }
     if (compress_page_with_multi_thread(block, offset, send_queued_data) > 0) {
         return true;
     }
@@ -2498,7 +2506,7 @@ static void ram_save_cleanup(void *opaque)
     }
 
     xbzrle_cleanup();
-    compress_threads_save_cleanup();
+    ram_compress_save_cleanup();
     ram_state_cleanup(rsp);
     g_free(migration_ops);
     migration_ops = NULL;
@@ -3023,14 +3031,14 @@ static int ram_save_setup(QEMUFile *f, void *opaque)
     RAMBlock *block;
     int ret;
 
-    if (compress_threads_save_setup()) {
+    if (ram_compress_save_setup()) {
         return -1;
     }
 
     /* migration has already setup the bitmap, reuse it. */
     if (!migration_in_colo_state()) {
         if (ram_init_all(rsp) != 0) {
-            compress_threads_save_cleanup();
+            ram_compress_save_cleanup();
             return -1;
         }
     }
@@ -3753,7 +3761,7 @@ int ram_load_postcopy(QEMUFile *f, int channel)
                 ret = -EINVAL;
                 break;
             }
-            decompress_data_with_multi_threads(f, page_buffer, len);
+            ram_decompress_data(f, page_buffer, len);
             break;
         case RAM_SAVE_FLAG_MULTIFD_FLUSH:
             multifd_recv_sync_main();
@@ -4022,7 +4030,7 @@ static int ram_load_precopy(QEMUFile *f)
                 ret = -EINVAL;
                 break;
             }
-            decompress_data_with_multi_threads(f, host, len);
+            ram_decompress_data(f, host, len);
             break;
 
         case RAM_SAVE_FLAG_XBZRLE:
-- 
2.39.3
Re: [PATCH 3/5] ram compress: Refactor ram compression functions
Posted by Juan Quintela 1 year, 1 month ago
Yuan Liu <yuan1.liu@intel.com> wrote:
> Refactor legacy RAM compression functions to support both IAA
> compression and CPU compression.
>
> Signed-off-by: Yuan Liu <yuan1.liu@intel.com>
> Reviewed-by: Nanhai Zou <nanhai.zou@intel.com>

Cmopression code is declared obsolete (see patches on list).
I don't think it is a good idea that you put things there.

And here you are doing two things:

- change several functions prefix from compress_threads to ram_compress
- Adding several hooks where you can add the iaa acceleration

Please, split in two different patches:
- rename/create new functions
- put the migrate_compress_with_iaa() hooks

Later, Juan.

> ---
>  migration/migration.c    |  6 +--
>  migration/ram-compress.c | 81 ++++++++++++++++++++++++++++++++--------
>  migration/ram-compress.h | 10 ++---
>  migration/ram.c          | 18 ++++++---
>  4 files changed, 86 insertions(+), 29 deletions(-)
>
> diff --git a/migration/migration.c b/migration/migration.c
> index 585d3c8f55..08a9c313d0 100644
> --- a/migration/migration.c
> +++ b/migration/migration.c
> @@ -237,7 +237,7 @@ void migration_incoming_state_destroy(void)
>      struct MigrationIncomingState *mis = migration_incoming_get_current();
>  
>      multifd_load_cleanup();
> -    compress_threads_load_cleanup();
> +    ram_compress_load_cleanup();
>  
>      if (mis->to_src_file) {
>          /* Tell source that we are done */
> @@ -524,7 +524,7 @@ process_incoming_migration_co(void *opaque)
>  
>      assert(mis->from_src_file);
>  
> -    if (compress_threads_load_setup(mis->from_src_file)) {
> +    if (ram_compress_load_setup(mis->from_src_file)) {
>          error_report("Failed to setup decompress threads");
>          goto fail;
>      }
> @@ -577,7 +577,7 @@ fail:
>      qemu_fclose(mis->from_src_file);
>  
>      multifd_load_cleanup();
> -    compress_threads_load_cleanup();
> +    ram_compress_load_cleanup();
>  
>      exit(EXIT_FAILURE);
>  }
> diff --git a/migration/ram-compress.c b/migration/ram-compress.c
> index 06254d8c69..47357352f7 100644
> --- a/migration/ram-compress.c
> +++ b/migration/ram-compress.c
> @@ -105,11 +105,11 @@ static void *do_data_compress(void *opaque)
>      return NULL;
>  }
>  
> -void compress_threads_save_cleanup(void)
> +static void compress_threads_save_cleanup(void)
>  {
>      int i, thread_count;
>  
> -    if (!migrate_compress() || !comp_param) {
> +    if (!comp_param) {
>          return;
>      }
>  
> @@ -144,13 +144,10 @@ void compress_threads_save_cleanup(void)
>      comp_param = NULL;
>  }
>  
> -int compress_threads_save_setup(void)
> +static int compress_threads_save_setup(void)
>  {
>      int i, thread_count;
>  
> -    if (!migrate_compress()) {
> -        return 0;
> -    }
>      thread_count = migrate_compress_threads();
>      compress_threads = g_new0(QemuThread, thread_count);
>      comp_param = g_new0(CompressParam, thread_count);
> @@ -370,6 +367,11 @@ int wait_for_decompress_done(void)
>          return 0;
>      }
>  
> +    if (migrate_compress_with_iaa()) {
> +        /* Implement in next patch */
> +        return 0;
> +    }
> +
>      thread_count = migrate_decompress_threads();
>      qemu_mutex_lock(&decomp_done_lock);
>      for (idx = 0; idx < thread_count; idx++) {
> @@ -381,13 +383,10 @@ int wait_for_decompress_done(void)
>      return qemu_file_get_error(decomp_file);
>  }
>  
> -void compress_threads_load_cleanup(void)
> +static void compress_threads_load_cleanup(void)
>  {
>      int i, thread_count;
>  
> -    if (!migrate_compress()) {
> -        return;
> -    }
>      thread_count = migrate_decompress_threads();
>      for (i = 0; i < thread_count; i++) {
>          /*
> @@ -422,14 +421,10 @@ void compress_threads_load_cleanup(void)
>      decomp_file = NULL;
>  }
>  
> -int compress_threads_load_setup(QEMUFile *f)
> +static int compress_threads_load_setup(QEMUFile *f)
>  {
>      int i, thread_count;
>  
> -    if (!migrate_compress()) {
> -        return 0;
> -    }
> -
>      thread_count = migrate_decompress_threads();
>      decompress_threads = g_new0(QemuThread, thread_count);
>      decomp_param = g_new0(DecompressParam, thread_count);
> @@ -457,7 +452,7 @@ exit:
>      return -1;
>  }
>  
> -void decompress_data_with_multi_threads(QEMUFile *f, void *host, int len)
> +static void decompress_data_with_multi_threads(QEMUFile *f, void *host, int len)
>  {
>      int idx, thread_count;
>  
> @@ -483,3 +478,57 @@ void decompress_data_with_multi_threads(QEMUFile *f, void *host, int len)
>          }
>      }
>  }
> +
> +int ram_compress_save_setup(void)
> +{
> +    if (!migrate_compress()) {
> +        return 0;
> +    }
> +    if (migrate_compress_with_iaa()) {
> +        /* Implement in next patch */
> +        return 0;
> +    }
> +    return compress_threads_save_setup();
> +}
> +
> +void ram_compress_save_cleanup(void)
> +{
> +    if (!migrate_compress()) {
> +        return;
> +    }
> +    if (migrate_compress_with_iaa()) {
> +        /* Implement in next patch */
> +        return;
> +    }
> +    compress_threads_save_cleanup();
> +}
> +
> +void ram_decompress_data(QEMUFile *f, void *host, int len)
> +{
> +    if (migrate_compress_with_iaa()) {
> +        /* Implement in next patch */
> +    }
> +    decompress_data_with_multi_threads(f, host, len);
> +}
> +
> +int ram_compress_load_setup(QEMUFile *f)
> +{
> +    if (!migrate_compress()) {
> +        return 0;
> +    }
> +    if (migrate_compress_with_iaa()) {
> +        /* Implement in next patch */
> +    }
> +    return compress_threads_load_setup(f);
> +}
> +
> +void ram_compress_load_cleanup(void)
> +{
> +    if (!migrate_compress()) {
> +        return;
> +    }
> +    if (migrate_compress_with_iaa()) {
> +        /* Implement in next patch */
> +    }
> +    compress_threads_load_cleanup();
> +}
> diff --git a/migration/ram-compress.h b/migration/ram-compress.h
> index 6f7fe2f472..382083acf6 100644
> --- a/migration/ram-compress.h
> +++ b/migration/ram-compress.h
> @@ -55,16 +55,16 @@ struct CompressParam {
>  };
>  typedef struct CompressParam CompressParam;
>  
> -void compress_threads_save_cleanup(void);
> -int compress_threads_save_setup(void);
> +void ram_compress_save_cleanup(void);
> +int ram_compress_save_setup(void);
>  
>  void flush_compressed_data(int (send_queued_data(CompressParam *)));
>  int compress_page_with_multi_thread(RAMBlock *block, ram_addr_t offset,
>                                  int (send_queued_data(CompressParam *)));
>  
>  int wait_for_decompress_done(void);
> -void compress_threads_load_cleanup(void);
> -int compress_threads_load_setup(QEMUFile *f);
> -void decompress_data_with_multi_threads(QEMUFile *f, void *host, int len);
> +void ram_compress_load_cleanup(void);
> +int ram_compress_load_setup(QEMUFile *f);
> +void ram_decompress_data(QEMUFile *f, void *host, int len);
>  
>  #endif
> diff --git a/migration/ram.c b/migration/ram.c
> index e4bfd39f08..34ee1de332 100644
> --- a/migration/ram.c
> +++ b/migration/ram.c
> @@ -1347,6 +1347,10 @@ static void ram_flush_compressed_data(RAMState *rs)
>      if (!save_page_use_compression(rs)) {
>          return;
>      }
> +    if (migrate_compress_with_iaa()) {
> +        /* Implement in next patch */
> +        return;
> +    }
>  
>      flush_compressed_data(send_queued_data);
>  }
> @@ -2099,6 +2103,10 @@ static bool save_compress_page(RAMState *rs, PageSearchStatus *pss,
>          return false;
>      }
>  
> +    if (migrate_compress_with_iaa()) {
> +        /* Implement in next patch */
> +        return true;
> +    }
>      if (compress_page_with_multi_thread(block, offset, send_queued_data) > 0) {
>          return true;
>      }
> @@ -2498,7 +2506,7 @@ static void ram_save_cleanup(void *opaque)
>      }
>  
>      xbzrle_cleanup();
> -    compress_threads_save_cleanup();
> +    ram_compress_save_cleanup();
>      ram_state_cleanup(rsp);
>      g_free(migration_ops);
>      migration_ops = NULL;
> @@ -3023,14 +3031,14 @@ static int ram_save_setup(QEMUFile *f, void *opaque)
>      RAMBlock *block;
>      int ret;
>  
> -    if (compress_threads_save_setup()) {
> +    if (ram_compress_save_setup()) {
>          return -1;
>      }
>  
>      /* migration has already setup the bitmap, reuse it. */
>      if (!migration_in_colo_state()) {
>          if (ram_init_all(rsp) != 0) {
> -            compress_threads_save_cleanup();
> +            ram_compress_save_cleanup();
>              return -1;
>          }
>      }
> @@ -3753,7 +3761,7 @@ int ram_load_postcopy(QEMUFile *f, int channel)
>                  ret = -EINVAL;
>                  break;
>              }
> -            decompress_data_with_multi_threads(f, page_buffer, len);
> +            ram_decompress_data(f, page_buffer, len);
>              break;
>          case RAM_SAVE_FLAG_MULTIFD_FLUSH:
>              multifd_recv_sync_main();
> @@ -4022,7 +4030,7 @@ static int ram_load_precopy(QEMUFile *f)
>                  ret = -EINVAL;
>                  break;
>              }
> -            decompress_data_with_multi_threads(f, host, len);
> +            ram_decompress_data(f, host, len);
>              break;
>  
>          case RAM_SAVE_FLAG_XBZRLE: