Re: [External] Re: [PATCH] hostmem: change default prealloc threads number

[Nan Wang]

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.


> > Signed-off-by: wangnan.light <wangnan.light@bytedance.com>
> > ---
> >   backends/hostmem.c | 2 +-
> >   hw/core/machine.c  | 5 +++++
> >   2 files changed, 6 insertions(+), 1 deletion(-)
> >
> > diff --git a/backends/hostmem.c b/backends/hostmem.c
> > index 4c05862ed5..c4a249b7e6 100644
> > --- a/backends/hostmem.c
> > +++ b/backends/hostmem.c
> > @@ -273,7 +273,7 @@ static void host_memory_backend_init(Object *obj)
> >       backend->merge = machine_mem_merge(machine);
> >       backend->dump = machine_dump_guest_core(machine);
> >       backend->reserve = true;
> > -    backend->prealloc_threads = 1;
> > +    backend->prealloc_threads = machine_smp_cpus(machine);
> >   }
> >   static void host_memory_backend_post_init(Object *obj)
> > diff --git a/hw/core/machine.c b/hw/core/machine.c
> > index 067f42b528..95ba5b1477 100644
> > --- a/hw/core/machine.c
> > +++ b/hw/core/machine.c
> > @@ -1065,6 +1065,11 @@ bool machine_dump_guest_core(MachineState 
> > *machine)
> >       return machine->dump_guest_core;
> >   }
> > +bool machine_smp_cpus(MachineState *machine)
> > +{
> > +    return machine->smp.cpus;
> > +}
> > +
> >   bool machine_mem_merge(MachineState *machine)
> >   {
> >       return machine->mem_merge;