Re: [PATCH v2 02/10] migration: stop compression to allocate and free memory frequently

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 





On 03/28/2018 05:25 PM, Peter Xu wrote:
On Tue, Mar 27, 2018 at 05:10:35PM +0800, guangrong.xiao@xxxxxxxxx wrote:

[...]

@@ -357,10 +358,20 @@ static void compress_threads_save_cleanup(void)
      terminate_compression_threads();
      thread_count = migrate_compress_threads();
      for (i = 0; i < thread_count; i++) {
+        /*
+         * stream.opaque can be used to store private data, we use it
+         * as a indicator which shows if the thread is properly init'd
+         * or not
+         */
+        if (!comp_param[i].stream.opaque) {
+            break;
+        }

How about using comp_param[i].file?  The opaque seems to be hiding
deeper, and...


Yes, indeed, good suggestion.

          qemu_thread_join(compress_threads + i);
          qemu_fclose(comp_param[i].file);
          qemu_mutex_destroy(&comp_param[i].mutex);
          qemu_cond_destroy(&comp_param[i].cond);
+        deflateEnd(&comp_param[i].stream);
+        comp_param[i].stream.opaque = NULL;
      }
      qemu_mutex_destroy(&comp_done_lock);
      qemu_cond_destroy(&comp_done_cond);
@@ -370,12 +381,12 @@ static void compress_threads_save_cleanup(void)
      comp_param = NULL;
  }
-static void compress_threads_save_setup(void)
+static int compress_threads_save_setup(void)
  {
      int i, thread_count;
if (!migrate_use_compression()) {
-        return;
+        return 0;
      }
      thread_count = migrate_compress_threads();
      compress_threads = g_new0(QemuThread, thread_count);
@@ -383,6 +394,12 @@ static void compress_threads_save_setup(void)
      qemu_cond_init(&comp_done_cond);
      qemu_mutex_init(&comp_done_lock);
      for (i = 0; i < thread_count; i++) {
+        if (deflateInit(&comp_param[i].stream,
+                           migrate_compress_level()) != Z_OK) {

(indent issue)


Will fix.

+            goto exit;
+        }
+        comp_param[i].stream.opaque = &comp_param[i];

...here from document:

         ZEXTERN int ZEXPORT deflateInit OF((z_streamp strm, int level));

         Initializes the internal stream state for compression. The
         fields zalloc, zfree and opaque must be initialized before by
         the caller. If zalloc and zfree are set to Z_NULL, deflateInit
         updates them to use default allocation functions.

So shall we init opaque first?  Otherwise looks good to me.

No, opaque need to be init-ed only if zalloc and zfree are specified, it
is not the case in this patch.





[Index of Archives]     [KVM ARM]     [KVM ia64]     [KVM ppc]     [Virtualization Tools]     [Spice Development]     [Libvirt]     [Libvirt Users]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite Questions]     [Linux Kernel]     [Linux SCSI]     [XFree86]

  Powered by Linux