From: Xiao Guangrong <guangrong.xiao@gmail.com> To: Peter Xu <peterx@redhat.com> Cc: kvm@vger.kernel.org, mst@redhat.com, mtosatti@redhat.com, Xiao Guangrong <xiaoguangrong@tencent.com>, qemu-devel@nongnu.org, pbonzini@redhat.com Subject: Re: [PATCH 2/8] migration: stop allocating and freeing memory frequently Date: Thu, 22 Mar 2018 19:57:54 +0800 [thread overview] Message-ID: <044788cf-fd00-83b3-6cd2-7df605397418@gmail.com> (raw) In-Reply-To: <20180321090644.GC20571@xz-mi> On 03/21/2018 05:06 PM, Peter Xu wrote: > On Tue, Mar 13, 2018 at 03:57:33PM +0800, guangrong.xiao@gmail.com wrote: >> From: Xiao Guangrong <xiaoguangrong@tencent.com> >> >> Current code uses compress2()/uncompress() to compress/decompress >> memory, these two function manager memory allocation and release >> internally, that causes huge memory is allocated and freed very >> frequently >> >> More worse, frequently returning memory to kernel will flush TLBs >> and trigger invalidation callbacks on mmu-notification which >> interacts with KVM MMU, that dramatically reduce the performance >> of VM >> >> So, we maintain the memory by ourselves and reuse it for each >> compression and decompression >> >> Signed-off-by: Xiao Guangrong <xiaoguangrong@tencent.com> >> --- >> migration/qemu-file.c | 34 ++++++++++-- >> migration/qemu-file.h | 6 ++- >> migration/ram.c | 142 +++++++++++++++++++++++++++++++++++++------------- >> 3 files changed, 140 insertions(+), 42 deletions(-) >> >> diff --git a/migration/qemu-file.c b/migration/qemu-file.c >> index 2ab2bf362d..1ff33a1ffb 100644 >> --- a/migration/qemu-file.c >> +++ b/migration/qemu-file.c >> @@ -658,6 +658,30 @@ uint64_t qemu_get_be64(QEMUFile *f) >> return v; >> } >> >> +/* return the size after compression, or negative value on error */ >> +static int qemu_compress_data(z_stream *stream, uint8_t *dest, size_t dest_len, >> + const uint8_t *source, size_t source_len) >> +{ >> + int err; >> + >> + err = deflateReset(stream); > > I'm not familiar with zlib, but I saw this in manual: > > https://www.zlib.net/manual.html > > This function is equivalent to deflateEnd followed by deflateInit, > but does not free and reallocate the internal compression state. The > stream will leave the compression level and any other attributes that > may have been set unchanged. > > I thought it was deflateInit() who is slow? Can we avoid the reset as deflateEnd() is worse as it frees memory to kernel which triggers TLB flush and mmu-notifier. > long as we make sure to deflateInit() before doing anything else? Actually, deflateReset() is cheap... :) > > Meanwhile, is there any performance number for this single patch? > Since I thought the old code is calling compress2() which contains > deflateInit() and deflateEnd() too, just like what current patch do? No, after the patch, we just call deflateInit() / deflateEnd() one time (in _setup() handler and _cleanup handler). Yes. This is the perf data from our production, after revert this patch: + 57.88% kqemu [kernel.kallsyms] [k] queued_spin_lock_slowpath + 10.55% kqemu [kernel.kallsyms] [k] __lock_acquire + 4.83% kqemu [kernel.kallsyms] [k] flush_tlb_func_common - 1.16% kqemu [kernel.kallsyms] [k] lock_acquire ▒ - lock_acquire ▒ - 15.68% _raw_spin_lock ▒ + 29.42% __schedule ▒ + 29.14% perf_event_context_sched_out ▒ + 23.60% tdp_page_fault ▒ + 10.54% do_anonymous_page ▒ + 2.07% kvm_mmu_notifier_invalidate_range_start ▒ + 1.83% zap_pte_range ▒ + 1.44% kvm_mmu_notifier_invalidate_range_end apply our work: + 51.92% kqemu [kernel.kallsyms] [k] queued_spin_lock_slowpath + 14.82% kqemu [kernel.kallsyms] [k] __lock_acquire + 1.47% kqemu [kernel.kallsyms] [k] mark_lock.clone.0 + 1.46% kqemu [kernel.kallsyms] [k] native_sched_clock + 1.31% kqemu [kernel.kallsyms] [k] lock_acquire + 1.24% kqemu libc-2.12.so [.] __memset_sse2 - 14.82% kqemu [kernel.kallsyms] [k] __lock_acquire ▒ - __lock_acquire ▒ - 99.75% lock_acquire ▒ - 18.38% _raw_spin_lock ▒ + 39.62% tdp_page_fault ▒ + 31.32% __schedule ▒ + 27.53% perf_event_context_sched_out ▒ + 0.58% hrtimer_interrupt You can see the TLB flush and mmu-lock contention have gone after this patch. > > It would be nice too if we can split the patch into two (decode, > encode) if you want, but that's optional. That's good to me, thank you, Peter.
WARNING: multiple messages have this Message-ID (diff)
From: Xiao Guangrong <guangrong.xiao@gmail.com> To: Peter Xu <peterx@redhat.com> Cc: pbonzini@redhat.com, mst@redhat.com, mtosatti@redhat.com, Xiao Guangrong <xiaoguangrong@tencent.com>, qemu-devel@nongnu.org, kvm@vger.kernel.org Subject: Re: [Qemu-devel] [PATCH 2/8] migration: stop allocating and freeing memory frequently Date: Thu, 22 Mar 2018 19:57:54 +0800 [thread overview] Message-ID: <044788cf-fd00-83b3-6cd2-7df605397418@gmail.com> (raw) In-Reply-To: <20180321090644.GC20571@xz-mi> On 03/21/2018 05:06 PM, Peter Xu wrote: > On Tue, Mar 13, 2018 at 03:57:33PM +0800, guangrong.xiao@gmail.com wrote: >> From: Xiao Guangrong <xiaoguangrong@tencent.com> >> >> Current code uses compress2()/uncompress() to compress/decompress >> memory, these two function manager memory allocation and release >> internally, that causes huge memory is allocated and freed very >> frequently >> >> More worse, frequently returning memory to kernel will flush TLBs >> and trigger invalidation callbacks on mmu-notification which >> interacts with KVM MMU, that dramatically reduce the performance >> of VM >> >> So, we maintain the memory by ourselves and reuse it for each >> compression and decompression >> >> Signed-off-by: Xiao Guangrong <xiaoguangrong@tencent.com> >> --- >> migration/qemu-file.c | 34 ++++++++++-- >> migration/qemu-file.h | 6 ++- >> migration/ram.c | 142 +++++++++++++++++++++++++++++++++++++------------- >> 3 files changed, 140 insertions(+), 42 deletions(-) >> >> diff --git a/migration/qemu-file.c b/migration/qemu-file.c >> index 2ab2bf362d..1ff33a1ffb 100644 >> --- a/migration/qemu-file.c >> +++ b/migration/qemu-file.c >> @@ -658,6 +658,30 @@ uint64_t qemu_get_be64(QEMUFile *f) >> return v; >> } >> >> +/* return the size after compression, or negative value on error */ >> +static int qemu_compress_data(z_stream *stream, uint8_t *dest, size_t dest_len, >> + const uint8_t *source, size_t source_len) >> +{ >> + int err; >> + >> + err = deflateReset(stream); > > I'm not familiar with zlib, but I saw this in manual: > > https://www.zlib.net/manual.html > > This function is equivalent to deflateEnd followed by deflateInit, > but does not free and reallocate the internal compression state. The > stream will leave the compression level and any other attributes that > may have been set unchanged. > > I thought it was deflateInit() who is slow? Can we avoid the reset as deflateEnd() is worse as it frees memory to kernel which triggers TLB flush and mmu-notifier. > long as we make sure to deflateInit() before doing anything else? Actually, deflateReset() is cheap... :) > > Meanwhile, is there any performance number for this single patch? > Since I thought the old code is calling compress2() which contains > deflateInit() and deflateEnd() too, just like what current patch do? No, after the patch, we just call deflateInit() / deflateEnd() one time (in _setup() handler and _cleanup handler). Yes. This is the perf data from our production, after revert this patch: + 57.88% kqemu [kernel.kallsyms] [k] queued_spin_lock_slowpath + 10.55% kqemu [kernel.kallsyms] [k] __lock_acquire + 4.83% kqemu [kernel.kallsyms] [k] flush_tlb_func_common - 1.16% kqemu [kernel.kallsyms] [k] lock_acquire ▒ - lock_acquire ▒ - 15.68% _raw_spin_lock ▒ + 29.42% __schedule ▒ + 29.14% perf_event_context_sched_out ▒ + 23.60% tdp_page_fault ▒ + 10.54% do_anonymous_page ▒ + 2.07% kvm_mmu_notifier_invalidate_range_start ▒ + 1.83% zap_pte_range ▒ + 1.44% kvm_mmu_notifier_invalidate_range_end apply our work: + 51.92% kqemu [kernel.kallsyms] [k] queued_spin_lock_slowpath + 14.82% kqemu [kernel.kallsyms] [k] __lock_acquire + 1.47% kqemu [kernel.kallsyms] [k] mark_lock.clone.0 + 1.46% kqemu [kernel.kallsyms] [k] native_sched_clock + 1.31% kqemu [kernel.kallsyms] [k] lock_acquire + 1.24% kqemu libc-2.12.so [.] __memset_sse2 - 14.82% kqemu [kernel.kallsyms] [k] __lock_acquire ▒ - __lock_acquire ▒ - 99.75% lock_acquire ▒ - 18.38% _raw_spin_lock ▒ + 39.62% tdp_page_fault ▒ + 31.32% __schedule ▒ + 27.53% perf_event_context_sched_out ▒ + 0.58% hrtimer_interrupt You can see the TLB flush and mmu-lock contention have gone after this patch. > > It would be nice too if we can split the patch into two (decode, > encode) if you want, but that's optional. That's good to me, thank you, Peter.
next prev parent reply other threads:[~2018-03-22 11:57 UTC|newest] Thread overview: 126+ messages / expand[flat|nested] mbox.gz Atom feed top 2018-03-13 7:57 [PATCH 0/8] migration: improve and cleanup compression guangrong.xiao 2018-03-13 7:57 ` [Qemu-devel] " guangrong.xiao 2018-03-13 7:57 ` [PATCH 1/8] migration: stop compressing page in migration thread guangrong.xiao 2018-03-13 7:57 ` [Qemu-devel] " guangrong.xiao 2018-03-15 10:25 ` Dr. David Alan Gilbert 2018-03-15 10:25 ` [Qemu-devel] " Dr. David Alan Gilbert 2018-03-16 8:05 ` Xiao Guangrong 2018-03-16 8:05 ` [Qemu-devel] " Xiao Guangrong 2018-03-19 12:11 ` Dr. David Alan Gilbert 2018-03-19 12:11 ` [Qemu-devel] " Dr. David Alan Gilbert 2018-03-21 8:19 ` Peter Xu 2018-03-21 8:19 ` [Qemu-devel] " Peter Xu 2018-03-22 11:38 ` Xiao Guangrong 2018-03-22 11:38 ` [Qemu-devel] " Xiao Guangrong 2018-03-26 9:02 ` Peter Xu 2018-03-26 9:02 ` [Qemu-devel] " Peter Xu 2018-03-26 15:43 ` Xiao Guangrong 2018-03-26 15:43 ` [Qemu-devel] " Xiao Guangrong 2018-03-27 7:33 ` Peter Xu 2018-03-27 7:33 ` [Qemu-devel] " Peter Xu 2018-03-27 19:12 ` Dr. David Alan Gilbert 2018-03-27 19:12 ` [Qemu-devel] " Dr. David Alan Gilbert 2018-03-28 3:01 ` Wang, Wei W 2018-03-28 3:01 ` [Qemu-devel] " Wang, Wei W 2018-03-27 15:24 ` Xiao Guangrong 2018-03-27 15:24 ` [Qemu-devel] " Xiao Guangrong 2018-03-28 7:30 ` Wei Wang 2018-03-28 7:30 ` [Qemu-devel] " Wei Wang 2018-03-28 7:37 ` Peter Xu 2018-03-28 7:37 ` [Qemu-devel] " Peter Xu 2018-03-28 8:30 ` Wei Wang 2018-03-28 8:30 ` [Qemu-devel] " Wei Wang 2018-03-13 7:57 ` [PATCH 2/8] migration: stop allocating and freeing memory frequently guangrong.xiao 2018-03-13 7:57 ` [Qemu-devel] " guangrong.xiao 2018-03-15 11:03 ` Dr. David Alan Gilbert 2018-03-15 11:03 ` [Qemu-devel] " Dr. David Alan Gilbert 2018-03-16 8:19 ` Xiao Guangrong 2018-03-16 8:19 ` [Qemu-devel] " Xiao Guangrong 2018-03-19 10:54 ` Dr. David Alan Gilbert 2018-03-19 10:54 ` [Qemu-devel] " Dr. David Alan Gilbert 2018-03-19 12:11 ` Xiao Guangrong 2018-03-19 12:11 ` [Qemu-devel] " Xiao Guangrong 2018-03-19 1:49 ` [PATCH 2/8] migration: stop allocating and freeingmemory frequently jiang.biao2 2018-03-19 1:49 ` [Qemu-devel] " jiang.biao2 2018-03-19 4:03 ` Xiao Guangrong 2018-03-19 4:03 ` [Qemu-devel] " Xiao Guangrong 2018-03-19 4:48 ` [PATCH 2/8] migration: stop allocating andfreeingmemory frequently jiang.biao2 2018-03-19 4:48 ` [Qemu-devel] " jiang.biao2 2018-03-21 9:06 ` [PATCH 2/8] migration: stop allocating and freeing memory frequently Peter Xu 2018-03-21 9:06 ` [Qemu-devel] " Peter Xu 2018-03-22 11:57 ` Xiao Guangrong [this message] 2018-03-22 11:57 ` Xiao Guangrong 2018-03-27 7:07 ` Peter Xu 2018-03-27 7:07 ` [Qemu-devel] " Peter Xu 2018-03-13 7:57 ` [PATCH 3/8] migration: support to detect compression and decompression errors guangrong.xiao 2018-03-13 7:57 ` [Qemu-devel] " guangrong.xiao 2018-03-15 11:29 ` Dr. David Alan Gilbert 2018-03-15 11:29 ` [Qemu-devel] " Dr. David Alan Gilbert 2018-03-16 8:25 ` Xiao Guangrong 2018-03-16 8:25 ` [Qemu-devel] " Xiao Guangrong 2018-03-19 7:56 ` [PATCH 3/8] migration: support to detect compressionand " jiang.biao2 2018-03-19 7:56 ` [Qemu-devel] " jiang.biao2 2018-03-19 8:01 ` Xiao Guangrong 2018-03-19 8:01 ` [Qemu-devel] " Xiao Guangrong 2018-03-21 10:00 ` [PATCH 3/8] migration: support to detect compression and " Peter Xu 2018-03-21 10:00 ` [Qemu-devel] " Peter Xu 2018-03-22 12:03 ` Xiao Guangrong 2018-03-22 12:03 ` [Qemu-devel] " Xiao Guangrong 2018-03-27 7:22 ` Peter Xu 2018-03-27 7:22 ` [Qemu-devel] " Peter Xu 2018-03-26 19:42 ` Xiao Guangrong 2018-03-26 19:42 ` [Qemu-devel] " Xiao Guangrong 2018-03-27 11:17 ` Peter Xu 2018-03-27 11:17 ` [Qemu-devel] " Peter Xu 2018-03-27 1:20 ` Xiao Guangrong 2018-03-27 1:20 ` [Qemu-devel] " Xiao Guangrong 2018-03-28 0:43 ` [PATCH 3/8] migration: support to detectcompression " jiang.biao2 2018-03-28 0:43 ` [Qemu-devel] " jiang.biao2 2018-03-27 14:35 ` Xiao Guangrong 2018-03-27 14:35 ` [Qemu-devel] " Xiao Guangrong 2018-03-28 3:03 ` Peter Xu 2018-03-28 3:03 ` [Qemu-devel] " Peter Xu 2018-03-28 4:08 ` [PATCH 3/8] migration: support todetectcompression " jiang.biao2 2018-03-28 4:08 ` [Qemu-devel] " jiang.biao2 2018-03-28 4:20 ` Peter Xu 2018-03-28 4:20 ` [Qemu-devel] " Peter Xu 2018-03-27 18:44 ` Xiao Guangrong 2018-03-27 18:44 ` [Qemu-devel] " Xiao Guangrong 2018-03-28 8:07 ` [PATCH 3/8] migration: support todetectcompressionand " jiang.biao2 2018-03-28 8:07 ` [Qemu-devel] " jiang.biao2 2018-03-13 7:57 ` [PATCH 4/8] migration: introduce control_save_page() guangrong.xiao 2018-03-13 7:57 ` [Qemu-devel] " guangrong.xiao 2018-03-15 11:37 ` Dr. David Alan Gilbert 2018-03-15 11:37 ` [Qemu-devel] " Dr. David Alan Gilbert 2018-03-16 8:52 ` Xiao Guangrong 2018-03-16 8:52 ` [Qemu-devel] " Xiao Guangrong 2018-03-27 7:47 ` Peter Xu 2018-03-27 7:47 ` [Qemu-devel] " Peter Xu 2018-03-13 7:57 ` [PATCH 5/8] migration: move calling control_save_page to the common place guangrong.xiao 2018-03-13 7:57 ` [Qemu-devel] " guangrong.xiao 2018-03-15 11:47 ` Dr. David Alan Gilbert 2018-03-15 11:47 ` [Qemu-devel] " Dr. David Alan Gilbert 2018-03-16 8:59 ` Xiao Guangrong 2018-03-16 8:59 ` [Qemu-devel] " Xiao Guangrong 2018-03-19 13:15 ` Dr. David Alan Gilbert 2018-03-19 13:15 ` [Qemu-devel] " Dr. David Alan Gilbert 2018-03-27 12:35 ` Peter Xu 2018-03-27 12:35 ` [Qemu-devel] " Peter Xu 2018-03-13 7:57 ` [PATCH 6/8] migration: move calling save_zero_page " guangrong.xiao 2018-03-13 7:57 ` [Qemu-devel] " guangrong.xiao 2018-03-15 12:27 ` Dr. David Alan Gilbert 2018-03-15 12:27 ` [Qemu-devel] " Dr. David Alan Gilbert 2018-03-27 12:49 ` Peter Xu 2018-03-27 12:49 ` [Qemu-devel] " Peter Xu 2018-03-13 7:57 ` [PATCH 7/8] migration: introduce save_normal_page() guangrong.xiao 2018-03-13 7:57 ` [Qemu-devel] " guangrong.xiao 2018-03-15 12:30 ` Dr. David Alan Gilbert 2018-03-15 12:30 ` [Qemu-devel] " Dr. David Alan Gilbert 2018-03-27 12:54 ` Peter Xu 2018-03-27 12:54 ` [Qemu-devel] " Peter Xu 2018-03-13 7:57 ` [PATCH 8/8] migration: remove ram_save_compressed_page() guangrong.xiao 2018-03-13 7:57 ` [Qemu-devel] " guangrong.xiao 2018-03-15 12:32 ` Dr. David Alan Gilbert 2018-03-15 12:32 ` [Qemu-devel] " Dr. David Alan Gilbert 2018-03-27 12:56 ` Peter Xu 2018-03-27 12:56 ` [Qemu-devel] " Peter Xu
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=044788cf-fd00-83b3-6cd2-7df605397418@gmail.com \ --to=guangrong.xiao@gmail.com \ --cc=kvm@vger.kernel.org \ --cc=mst@redhat.com \ --cc=mtosatti@redhat.com \ --cc=pbonzini@redhat.com \ --cc=peterx@redhat.com \ --cc=qemu-devel@nongnu.org \ --cc=xiaoguangrong@tencent.com \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.