[PATCH] migration: Fix the minus value for compressed_size

[PATCH] migration: Fix the minus value for compressed_size

[Zhenzhong Duan]

When update_compress_thread_counts() is called first time, there is
no data stream yet. We see compression_counters.compressed_size
becomes minus value shortly.

Signed-off-by: Zhenzhong Duan <zhenzhong.duan@intel.com>
---
 migration/ram.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/migration/ram.c b/migration/ram.c
index dc1de9ddbc68..510db95cdc36 100644
--- a/migration/ram.c
+++ b/migration/ram.c
@@ -1443,6 +1443,10 @@ static bool do_compress_ram_page(QEMUFile *f, z_stream *stream, RAMBlock *block,
 static void
 update_compress_thread_counts(const CompressParam *param, int bytes_xmit)
 {
+    if (bytes_xmit <= 0) {
+        return;
+    }
+
     ram_transferred_add(bytes_xmit);
 
     if (param->zero_page) {
-- 
2.25.1

Re: [PATCH] migration: Fix the minus value for compressed_size

[Dr. David Alan Gilbert]

* Zhenzhong Duan (zhenzhong.duan@intel.com) wrote:
> When update_compress_thread_counts() is called first time, there is
> no data stream yet. We see compression_counters.compressed_size
> becomes minus value shortly.
> 
> Signed-off-by: Zhenzhong Duan <zhenzhong.duan@intel.com>
> ---
>  migration/ram.c | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/migration/ram.c b/migration/ram.c
> index dc1de9ddbc68..510db95cdc36 100644
> --- a/migration/ram.c
> +++ b/migration/ram.c
> @@ -1443,6 +1443,10 @@ static bool do_compress_ram_page(QEMUFile *f, z_stream *stream, RAMBlock *block,
>  static void
>  update_compress_thread_counts(const CompressParam *param, int bytes_xmit)
>  {
> +    if (bytes_xmit <= 0) {
> +        return;
> +    }

What's the call path where that happens? The only place I see
bytes_xmit being less than 0 is in compress_page_with_multi_thread where
it's initialised to -1 - but it's always updated before the call
to update_compress_thread_counts.

I wonder if the real problem is:

    compression_counters.compressed_size += bytes_xmit - 8;

Is bytes_xmit being less than 8 for some reason?

Dave

>      ram_transferred_add(bytes_xmit);
>  
>      if (param->zero_page) {
> -- 
> 2.25.1
> 
-- 
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK

RE: [PATCH] migration: Fix the minus value for compressed_size

[Duan, Zhenzhong]

>-----Original Message-----
>From: Dr. David Alan Gilbert <dgilbert@redhat.com>
>Sent: Monday, October 10, 2022 6:35 PM
>To: Duan, Zhenzhong <zhenzhong.duan@intel.com>
>Cc: qemu-devel@nongnu.org; quintela@redhat.com
>Subject: Re: [PATCH] migration: Fix the minus value for compressed_size
>
>* Zhenzhong Duan (zhenzhong.duan@intel.com) wrote:
>> When update_compress_thread_counts() is called first time, there is no
>> data stream yet. We see compression_counters.compressed_size
>> becomes minus value shortly.
>>
>> Signed-off-by: Zhenzhong Duan <zhenzhong.duan@intel.com>
>> ---
>>  migration/ram.c | 4 ++++
>>  1 file changed, 4 insertions(+)
>>
>> diff --git a/migration/ram.c b/migration/ram.c index
>> dc1de9ddbc68..510db95cdc36 100644
>> --- a/migration/ram.c
>> +++ b/migration/ram.c
>> @@ -1443,6 +1443,10 @@ static bool do_compress_ram_page(QEMUFile *f,
>> z_stream *stream, RAMBlock *block,  static void
>> update_compress_thread_counts(const CompressParam *param, int
>> bytes_xmit)  {
>> +    if (bytes_xmit <= 0) {
>> +        return;
>> +    }
>
>What's the call path where that happens? The only place I see bytes_xmit
>being less than 0 is in compress_page_with_multi_thread where it's initialised
>to -1 - but it's always updated before the call to
>update_compress_thread_counts.
>
>I wonder if the real problem is:
>
>    compression_counters.compressed_size += bytes_xmit - 8;
>
>Is bytes_xmit being less than 8 for some reason?

bytes_xmit is 0 when update_compress_thread_counts() is called first time as comp_param[idx].file
is empty, which makes compression_counters.compressed_size=-8

(gdb) bt
#0  update_compress_thread_counts (param=0x7ffe340011d0, bytes_xmit=0) at ../migration/ram.c:1446
#1  0x0000555555cbf480 in flush_compressed_data (rs=0x7ffe34259d40) at ../migration/ram.c:1486
#2  0x0000555555cc0a53 in save_compress_page (rs=0x7ffe34259d40, block=0x555556aab280, offset=0) at ../migration/ram.c:2260
#3  0x0000555555cc0b0e in ram_save_target_page (rs=0x7ffe34259d40, pss=0x7ffece7fb800) at ../migration/ram.c:2290
#4  0x0000555555cc0fd8 in ram_save_host_page (rs=0x7ffe34259d40, pss=0x7ffece7fb800) at ../migration/ram.c:2487
#5  0x0000555555cc11ce in ram_find_and_save_block (rs=0x7ffe34259d40) at ../migration/ram.c:2576
#6  0x0000555555cc2863 in ram_save_iterate (f=0x555556a98b30, opaque=0x5555567c92e0 <ram_state>) at ../migration/ram.c:3304
#7  0x0000555555ae86bd in qemu_savevm_state_iterate (f=0x555556a98b30, postcopy=false) at ../migration/savevm.c:1261
#8  0x0000555555ad7500 in migration_iteration_run (s=0x555556ad5390) at ../migration/migration.c:3757
#9  0x0000555555ad7abb in migration_thread (opaque=0x555556ad5390) at ../migration/migration.c:3988
#10 0x0000555555f20012 in qemu_thread_start (args=0x555556862180) at ../util/qemu-thread-posix.c:504
#11 0x00007ffff78f5609 in start_thread (arg=<optimized out>) at pthread_create.c:477
#12 0x00007ffff781a163 in clone () at ../sysdeps/unix/sysv/linux/x86_64/clone.S:95
(gdb) n
(gdb) n
(gdb) p compression_counters
$6 = {pages = 0, busy = 0, busy_rate = 0, compressed_size = -8, compression_rate = 0}

Thanks
Zhenzhong