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

[Igor Mammedov]

On Tue, 17 May 2022 20:46:50 +0200
Paolo Bonzini <pbonzini@redhat.com> wrote:

> On 5/17/22 14:38, 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).
> > 
> > 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;
> >   }
> >   
> >   static void host_memory_backend_post_init(Object *obj)  
> 
> Queued, thanks.

could you drop this patch pls (there is an more acceptable alternative,
see my other replies in this thread if we decide to put management policy
decisions in QEMU code).

(well unless layer violation is acceptable practice now
and it's really discouraging to do cleanup work if gets discarded)

PS:
There is no good default in this case (whatever number is picked
it could be good or bad depending on usecase).

> Paolo
>