qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: "Daniel P. Berrangé" <berrange@redhat.com>
To: Igor Mammedov <imammedo@redhat.com>
Cc: mikughoull@gmail.com, David Hildenbrand <david@redhat.com>,
	qemu-devel@nongnu.org, ehabkost@redhat.com,
	Nan Wang <wangnan.light@bytedance.com>
Subject: Re: [External] Re: [PATCH] hostmem: change default prealloc threads number
Date: Wed, 29 Sep 2021 10:22:39 +0100	[thread overview]
Message-ID: <YVQwX3RHcafMwMgy@redhat.com> (raw)
In-Reply-To: <20210929110531.0724f911@redhat.com>

On Wed, Sep 29, 2021 at 11:05:31AM +0200, Igor Mammedov wrote:
> On Tue, 28 Sep 2021 00:47:01 +0800
> Nan Wang <wangnan.light@bytedance.com> wrote:
> 
> > On 2021/9/27 11:16, David Hildenbrand wrote:
> > > On 27.09.21 15:19, Nan Wang wrote:  
> > >> From: "wangnan.light" <wangnan.light@bytedance.com>
> > >>
> > >> the default number of prealloc threads is 1, for huge memory backend
> > >> file, single thread touch page is really slow.
> > >> We can adjust thread number by prealloc-threads property, but if the
> > >> default value updated to MachineState::smp::cpus may be better.
> > >> For example, old version of qemu(prealloc-threads have not been
> > >> introduced yet), the value of threads num is MachineState::smp::cpus,
> > >> so if someone use the same commandline to start current verion of qemu
> > >> and old version of qemu which will lead to different behaviors.  
> > > 
> > > The introducing patch mentions:
> > > 
> > > commit ffac16fab33bb42f17e47624985220c1fd864e9d
> > > Author: Igor Mammedov <imammedo@redhat.com>
> > > Date:   Wed Feb 19 11:09:50 2020 -0500
> > > 
> > >      hostmem: introduce "prealloc-threads" property
> > > 
> > >      the property will allow user to specify number of threads to use
> > >      in pre-allocation stage. It also will allow to reduce implicit
> > >      hostmem dependency on current_machine.
> > >      On object creation it will default to 1, but via machine
> > >      compat property it will be updated to MachineState::smp::cpus
> > >      to keep current behavior for hostmem and main RAM (which is
> > >      now also hostmem based).
> > > 
> > > So it looks like we want to do the latter via compat properties eventually.
> > > 
> > > However, I'd like to note that more prealloc threads might be good for 
> > > large backends, and might be bad for small backends. To me, it feels 
> > > like a workload that relies on this should really do this manually. So I 
> > > am still not sure if this is the right thing to do.  
> > Yes, I agree with you "more prealloc threas are good for large backends, 
> > and bad for small backends". But I think most situation large backends 
> > always with large vcpu numbers and small backens always with small vcpu 
> > numbers, because most users will not create a vm with large vcpu numbers 
> > with small memory.
> > 
> > 
> > > 
> > > Note that qapi/qom.json:
> > > 
> > > "@prealloc-threads: number of CPU threads to use for prealloc (default: 
> > > 1", so that doc would be wrong now.
> > > 
> > > Why exactly can't workload that cares not simply set this manually? 
> > > Performance tuning smells like something to be done manually for a 
> > > specific workload.
> > >  
> > It is a simply way that let workload set the prealloc threads manually. 
> > For example, for large backends it set many prealloc threads, and set 1 
> > prealloc threads manually for small backends. Yes, workload can 
> > `maunally` set prealloc thread to 1, rather than use `default` value 1.
> > So when workload want to(or maybe just forget specify the 
> > prealloc-threads property) use the default value, I think the 
> > MachineState::smp::cpus maybe better than 1.
> 
> as commit mentioned by David states, it creates implicit dependency
> on Machine and we were working getting rid of such dependencies
> where it's possible.
> 
> So if you have to change prealloc-threads to a larger number,
> you can try to use specific machine compat properties to do it,
> instead of pushing machine to generic backend code. But 'good'
> default for your workload doesn't guaranties it's a good one
> another.
> 
> My preference is that user (mgmt layer) should set property
> explicitly if it cares. It's leads to more stable VM config,
> as opposed to using defaults which could change over time and
> unexpectedly 'regress' such VMs, and can factor in host/workload
> specific nuances without need to change QEMU.

Setting prealloc_threads to match vCPUs count feels like it is making
an assumption that if we've allowed 4 vCPUs, it is OK for the prealloc
to consume 4 host CPUs. This assumption could be tricky when QEMU is
strictly pinned to host CPUs, as vCPU threads are pinned to some pCPUs
but emulator threads might be pinned differently.

Would there still be a performance advantage to prealloc_threads > 1,
if all non-vCPU threads are pinned to the same single host CPU ?


Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|



  parent reply	other threads:[~2021-09-29  9:24 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <20210927131951.1810823-1-user@n248-145-203>
2021-09-27 15:16 ` [PATCH] hostmem: change default prealloc threads number David Hildenbrand
2021-09-27 16:47   ` [External] " Nan Wang
2021-09-29  9:05     ` Igor Mammedov
2021-09-29  9:10       ` David Hildenbrand
2021-09-29  9:22       ` Daniel P. Berrangé [this message]
2021-09-29 14:46         ` Igor Mammedov

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=YVQwX3RHcafMwMgy@redhat.com \
    --to=berrange@redhat.com \
    --cc=david@redhat.com \
    --cc=ehabkost@redhat.com \
    --cc=imammedo@redhat.com \
    --cc=mikughoull@gmail.com \
    --cc=qemu-devel@nongnu.org \
    --cc=wangnan.light@bytedance.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: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).