Re: [PATCH] hostmem: default the amount of prealloc-threads to smp-cpus

[Jaroslav Jindrák]

On Tue, 17 May 2022 at 17:12, Igor Mammedov <imammedo@redhat.com> wrote:
>
> On Tue, 17 May 2022 14:38:58 +0200
> dzejrou@gmail.com wrote:
>
> > From: Jaroslav Jindrak <dzejrou@gmail.com>
> >
> > Prior to the introduction of the prealloc-threads property, the amount
> > of threads used to preallocate memory was derived from the value of
> > smp-cpus passed to qemu, the amount of physical cpus of the host
> > and a hardcoded maximum value. When the prealloc-threads property
> > was introduced, it included a default of 1 in backends/hostmem.c and
> > a default of smp-cpus using the sugar API for the property itself. The
> > latter default is not used when the property is not specified on qemu's
> > command line, so guests that were not adjusted for this change suddenly
> > started to use the default of 1 thread to preallocate memory, which
> > resulted in observable slowdowns in guest boots for guests with large
> > memory (e.g. when using libvirt <8.2.0 or managing guests manually).
>
> current behavior in QEMU is intentionally conservative. threads
> number is subject to host configuration and limitations management
> layer puts on it and it's not QEMU job to conjure magic numbers that
> are host/workload depended.
> If user needs more prealloc threads they need to specify it explicitly
> for each memory backend (i.e. convince management to do it or fix your
> scripts to so).

I understand that, but I'd say that a sudden change of the default to
1, which can
lead to guest boot slowdowns of about 2-8 times (and boots taking several
minutes) because of a change that was not very well documented (to the point
that it took libvirt about 2 years [0] to add support for this
property) does qualify
as a regression. After all, this patch does not conjure any new
numbers, but rather
restores the original behavior of qemu if the user does _not_ use the new
prealloc-threads property.

[0] 
https://github.com/libvirt/libvirt/commit/ba7f98126fa84d354ce72929b77cc111a9a557a9

> CCing Michal, as he recently looked into similar topic.
>
> To behave it the old way you need to use legacy -mem-prealloc option.
>
>
> > This commit restores the original behavior for these cases while not
> > impacting guests started with the prealloc-threads property in any way.
> >
> > Fixes: 220c1fd864e9d ("hostmem: introduce "prealloc-threads" property")
> > Signed-off-by: Jaroslav Jindrak <dzejrou@gmail.com>
> > ---
> >  backends/hostmem.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/backends/hostmem.c b/backends/hostmem.c
> > index a7bae3d713..624bb7ecd3 100644
> > --- a/backends/hostmem.c
> > +++ b/backends/hostmem.c
> > @@ -274,7 +274,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;
> pls, do not add more dependencies to random external objects to memory 
> backends.
>
> If you have to do that, use machine compat properties instead, but then
> the essence of the issue stays the same (user shall define optimal threads
> number and provide it to qemu explicitly)
>
> >  }
> >
> >  static void host_memory_backend_post_init(Object *obj)
>