diff mbox

[v7,09/14] migration: Make compression co-work with xbzrle

Message ID 1428474011-30797-10-git-send-email-liang.z.li@intel.com
State New
Headers show

Commit Message

Li, Liang Z April 8, 2015, 6:20 a.m. UTC
Now, multiple thread compression can co-work with xbzrle. when
xbzrle is on, multiple thread compression will only work at the
first round of RAM data sync.

Signed-off-by: Liang Li <liang.z.li@intel.com>
Signed-off-by: Yang Zhang <yang.z.zhang@intel.com>
Reviewed-by: Dr.David Alan Gilbert <dgilbert@redhat.com>
---
 arch_init.c | 12 +++++++++++-
 1 file changed, 11 insertions(+), 1 deletion(-)

Comments

Juan Quintela April 8, 2015, 11:26 a.m. UTC | #1
Liang Li <liang.z.li@intel.com> wrote:
> Now, multiple thread compression can co-work with xbzrle. when
> xbzrle is on, multiple thread compression will only work at the
> first round of RAM data sync.
>
> Signed-off-by: Liang Li <liang.z.li@intel.com>
> Signed-off-by: Yang Zhang <yang.z.zhang@intel.com>
> Reviewed-by: Dr.David Alan Gilbert <dgilbert@redhat.com>

Reviewed-by: Juan Quintela <quintela@redhat.com>


>  /* The empty QEMUFileOps will be used by file in CompressParam */
>  static const QEMUFileOps empty_ops = { };
> +static bool compression_switch;
> +
>  static DecompressParam *decomp_param;
>  static QemuThread *decompress_threads;
>  static uint8_t *compressed_data_buf;
> @@ -436,6 +438,7 @@ void migrate_compress_threads_create(void)
>      if (!migrate_use_compression()) {
>          return;
>      }
> +    compression_switch = true;
>      thread_count = migrate_compress_threads();
>      compress_threads = g_new0(QemuThread, thread_count);
>      comp_param = g_new0(CompressParam, thread_count);
> @@ -1059,9 +1062,16 @@ static int ram_find_and_save_block(QEMUFile *f, bool last_stage,
>                  block = QLIST_FIRST_RCU(&ram_list.blocks);
>                  complete_round = true;
>                  ram_bulk_stage = false;
> +                if (migrate_use_xbzrle()) {
> +                    /* If xbzrle is on, stop using the data compression at this
> +                     * point. In theory, xbzrle can do better than compression.
> +                     */
> +                    flush_compressed_data(f);
> +                    compression_switch = false;
> +                }

I still think that it should be better:
a- don't mix them (or)
b- if we mix them, just use compression always that we sent whole pages.
   if xbzrle is not able to compress a page, use compression after
   putting the page on the cache.  i.e. try first to set through xbzrle,
   and if that don't work, use compression if possible (after zecond
   iteration, of course).)


But as you are the one doing the code....

Later, Juan.

PD. Yes, it can be changed later.
Li, Liang Z April 8, 2015, 1:52 p.m. UTC | #2
> > Now, multiple thread compression can co-work with xbzrle. when xbzrle
> > is on, multiple thread compression will only work at the first round
> > of RAM data sync.
> >
> > Signed-off-by: Liang Li <liang.z.li@intel.com>
> > Signed-off-by: Yang Zhang <yang.z.zhang@intel.com>
> > Reviewed-by: Dr.David Alan Gilbert <dgilbert@redhat.com>
> 
> Reviewed-by: Juan Quintela <quintela@redhat.com>
> 
> 
> >  /* The empty QEMUFileOps will be used by file in CompressParam */
> > static const QEMUFileOps empty_ops = { };
> > +static bool compression_switch;
> > +
> >  static DecompressParam *decomp_param;  static QemuThread
> > *decompress_threads;  static uint8_t *compressed_data_buf; @@ -436,6
> > +438,7 @@ void migrate_compress_threads_create(void)
> >      if (!migrate_use_compression()) {
> >          return;
> >      }
> > +    compression_switch = true;
> >      thread_count = migrate_compress_threads();
> >      compress_threads = g_new0(QemuThread, thread_count);
> >      comp_param = g_new0(CompressParam, thread_count); @@ -1059,9
> > +1062,16 @@ static int ram_find_and_save_block(QEMUFile *f, bool
> last_stage,
> >                  block = QLIST_FIRST_RCU(&ram_list.blocks);
> >                  complete_round = true;
> >                  ram_bulk_stage = false;
> > +                if (migrate_use_xbzrle()) {
> > +                    /* If xbzrle is on, stop using the data compression at this
> > +                     * point. In theory, xbzrle can do better than compression.
> > +                     */
> > +                    flush_compressed_data(f);
> > +                    compression_switch = false;
> > +                }
> 
> I still think that it should be better:
> a- don't mix them (or)
> b- if we mix them, just use compression always that we sent whole pages.
>    if xbzrle is not able to compress a page, use compression after
>    putting the page on the cache.  i.e. try first to set through xbzrle,
>    and if that don't work, use compression if possible (after zecond
>    iteration, of course).)
> 
> 
> But as you are the one doing the code....
> 

I prefer the b solution that you suggest, maybe I can send a patch later. But now, I don't want to
make a big changes before the patch being merged.

Liang

> Later, Juan.
> 
> PD. Yes, it can be changed later.
diff mbox

Patch

diff --git a/arch_init.c b/arch_init.c
index d657a6c..8fb2ea4 100644
--- a/arch_init.c
+++ b/arch_init.c
@@ -351,6 +351,8 @@  static QemuMutex *comp_done_lock;
 static QemuCond *comp_done_cond;
 /* The empty QEMUFileOps will be used by file in CompressParam */
 static const QEMUFileOps empty_ops = { };
+static bool compression_switch;
+
 static DecompressParam *decomp_param;
 static QemuThread *decompress_threads;
 static uint8_t *compressed_data_buf;
@@ -436,6 +438,7 @@  void migrate_compress_threads_create(void)
     if (!migrate_use_compression()) {
         return;
     }
+    compression_switch = true;
     thread_count = migrate_compress_threads();
     compress_threads = g_new0(QemuThread, thread_count);
     comp_param = g_new0(CompressParam, thread_count);
@@ -1059,9 +1062,16 @@  static int ram_find_and_save_block(QEMUFile *f, bool last_stage,
                 block = QLIST_FIRST_RCU(&ram_list.blocks);
                 complete_round = true;
                 ram_bulk_stage = false;
+                if (migrate_use_xbzrle()) {
+                    /* If xbzrle is on, stop using the data compression at this
+                     * point. In theory, xbzrle can do better than compression.
+                     */
+                    flush_compressed_data(f);
+                    compression_switch = false;
+                }
             }
         } else {
-            if (migrate_use_compression()) {
+            if (compression_switch && migrate_use_compression()) {
                 pages = ram_save_compressed_page(f, block, offset, last_stage,
                                                  bytes_transferred);
             } else {