[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH 07/12] migration: hold the lock only if it is re
From: |
Peter Xu |
Subject: |
Re: [Qemu-devel] [PATCH 07/12] migration: hold the lock only if it is really needed |
Date: |
Wed, 18 Jul 2018 18:18:00 +0800 |
User-agent: |
Mutt/1.10.0 (2018-05-17) |
On Wed, Jul 18, 2018 at 04:56:13PM +0800, Xiao Guangrong wrote:
>
>
> On 07/12/2018 04:26 PM, Peter Xu wrote:
> > On Thu, Jul 12, 2018 at 03:47:57PM +0800, Xiao Guangrong wrote:
> > >
> > >
> > > On 07/11/2018 04:21 PM, Peter Xu wrote:
> > > > On Thu, Jun 28, 2018 at 05:33:58PM +0800, Xiao Guangrong wrote:
> > > > >
> > > > >
> > > > > On 06/19/2018 03:36 PM, Peter Xu wrote:
> > > > > > On Mon, Jun 04, 2018 at 05:55:15PM +0800, address@hidden wrote:
> > > > > > > From: Xiao Guangrong <address@hidden>
> > > > > > >
> > > > > > > Try to hold src_page_req_mutex only if the queue is not
> > > > > > > empty
> > > > > >
> > > > > > Pure question: how much this patch would help? Basically if you are
> > > > > > running compression tests then I think it means you are with precopy
> > > > > > (since postcopy cannot work with compression yet), then here the
> > > > > > lock
> > > > > > has no contention at all.
> > > > >
> > > > > Yes, you are right, however we can observe it is in the top functions
> > > > > (after revert this patch):
> > > > >
> > > > > Samples: 29K of event 'cycles', Event count (approx.): 22263412260
> > > > > + 7.99% kqemu qemu-system-x86_64 [.] ram_bytes_total
> > > > > + 6.95% kqemu [kernel.kallsyms] [k]
> > > > > copy_user_enhanced_fast_string
> > > > > + 6.23% kqemu qemu-system-x86_64 [.] qemu_put_qemu_file
> > > > > + 6.20% kqemu qemu-system-x86_64 [.] qemu_event_set
> > > > > + 5.80% kqemu qemu-system-x86_64 [.] __ring_put
> > > > > + 4.82% kqemu qemu-system-x86_64 [.]
> > > > > compress_thread_data_done
> > > > > + 4.11% kqemu qemu-system-x86_64 [.] ring_is_full
> > > > > + 3.07% kqemu qemu-system-x86_64 [.]
> > > > > threads_submit_request_prepare
> > > > > + 2.83% kqemu qemu-system-x86_64 [.] ring_mp_get
> > > > > + 2.71% kqemu qemu-system-x86_64 [.] __ring_is_full
> > > > > + 2.46% kqemu qemu-system-x86_64 [.] buffer_zero_sse2
> > > > > + 2.40% kqemu qemu-system-x86_64 [.] add_to_iovec
> > > > > + 2.21% kqemu qemu-system-x86_64 [.] ring_get
> > > > > + 1.96% kqemu [kernel.kallsyms] [k] __lock_acquire
> > > > > + 1.90% kqemu libc-2.12.so [.] memcpy
> > > > > + 1.55% kqemu qemu-system-x86_64 [.] ring_len
> > > > > + 1.12% kqemu libpthread-2.12.so [.] pthread_mutex_unlock
> > > > > + 1.11% kqemu qemu-system-x86_64 [.] ram_find_and_save_block
> > > > > + 1.07% kqemu qemu-system-x86_64 [.] ram_save_host_page
> > > > > + 1.04% kqemu qemu-system-x86_64 [.] qemu_put_buffer
> > > > > + 0.97% kqemu qemu-system-x86_64 [.]
> > > > > compress_page_with_multi_thread
> > > > > + 0.96% kqemu qemu-system-x86_64 [.] ram_save_target_page
> > > > > + 0.93% kqemu libpthread-2.12.so [.] pthread_mutex_lock
> > > >
> > > > (sorry to respond late; I was busy with other stuff for the
> > > > release...)
> > > >
> > >
> > > You're welcome. :)
> > >
> > > > I am trying to find out anything related to unqueue_page() but I
> > > > failed. Did I miss anything obvious there?
> > > >
> > >
> > > unqueue_page() was not listed here indeed, i think the function
> > > itself is light enough (a check then directly return) so it
> > > did not leave a trace here.
> > >
> > > This perf data was got after reverting this patch, i.e, it's
> > > based on the lockless multithread model, then unqueue_page() is
> > > the only place using mutex in the main thread.
> > >
> > > And you can see the overload of mutext was gone after applying
> > > this patch in the mail i replied to Dave.
> >
> > I see. It's not a big portion of CPU resource, though of course I
> > don't have reason to object to this change as well.
> >
> > Actually what interested me more is why ram_bytes_total() is such a
> > hot spot. AFAIU it's only called in ram_find_and_save_block() per
> > call, and it should be mostly a constant if we don't plug/unplug
> > memories. Not sure that means that's a better spot to work on.
> >
>
> I noticed it too. That could be another work we will work on. :)
Yeah I'm looking forward to that. :)
Btw, please feel free to post the common codes separately in your next
post if the series grows even bigger (that may help even for
no-compression migrations), I would bet they'll have a better chance
to be reviewed and merged faster (though it'll possibly be after QEMU
3.0).
Regards,
--
Peter Xu