| Message ID | 20221010093415.2779165-1-zhenzhong.duan@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | migration: Fix the minus value for compressed_size | expand |
* 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 >
>-----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
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) {
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(+)