[v1] qom.json: default the prealloc-threads to smp-cpus

[PATCH] qom.json: default the prealloc-threads to smp-cpus

Posted by Zhenyu Zhang 1 year, 6 months ago

Since the amount of prealloc-threads to smp-cpus is
defaulted in hostmem, so sync this information.

Signed-off-by: Zhenyu Zhang <zhenyzha@redhat.com>
---
 qapi/qom.json | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/qapi/qom.json b/qapi/qom.json
index 87fcad2423..ac4cd213a7 100644
--- a/qapi/qom.json
+++ b/qapi/qom.json
@@ -576,7 +576,7 @@
 #
 # @prealloc: if true, preallocate memory (default: false)
 #
-# @prealloc-threads: number of CPU threads to use for prealloc (default: 1)
+# @prealloc-threads: number of CPU threads to use for prealloc (default: smp-cpus) (since 7.1)
 #
 # @prealloc-context: thread context to use for creation of preallocation threads
 #                    (default: none) (since 7.2)
-- 
2.31.1

Re: [PATCH] qom.json: default the prealloc-threads to smp-cpus

Posted by Philippe Mathieu-Daudé 1 year, 6 months ago

On 3/11/22 11:47, Zhenyu Zhang wrote:
> Since the amount of prealloc-threads to smp-cpus is
> defaulted in hostmem, so sync this information.
> 
> Signed-off-by: Zhenyu Zhang <zhenyzha@redhat.com>
> ---
>   qapi/qom.json | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/qapi/qom.json b/qapi/qom.json
> index 87fcad2423..ac4cd213a7 100644
> --- a/qapi/qom.json
> +++ b/qapi/qom.json
> @@ -576,7 +576,7 @@
>   #
>   # @prealloc: if true, preallocate memory (default: false)
>   #
> -# @prealloc-threads: number of CPU threads to use for prealloc (default: 1)
> +# @prealloc-threads: number of CPU threads to use for prealloc (default: smp-cpus) (since 7.1)

The property is present since 5.0. Shouldn't this be "(default: 
smp-cpus) (since 5.0)"?

>   #
>   # @prealloc-context: thread context to use for creation of preallocation threads
>   #                    (default: none) (since 7.2)

Re: [PATCH] qom.json: default the prealloc-threads to smp-cpus

Posted by Markus Armbruster 1 year, 6 months ago

Philippe Mathieu-Daudé <philmd@linaro.org> writes:

> On 3/11/22 11:47, Zhenyu Zhang wrote:
>> Since the amount of prealloc-threads to smp-cpus is
>> defaulted in hostmem, so sync this information.

Has this always defaulted to smp-cpus, or did this change along the way?

>> Signed-off-by: Zhenyu Zhang <zhenyzha@redhat.com>
>> ---
>>   qapi/qom.json | 2 +-
>>   1 file changed, 1 insertion(+), 1 deletion(-)
>> diff --git a/qapi/qom.json b/qapi/qom.json
>> index 87fcad2423..ac4cd213a7 100644
>> --- a/qapi/qom.json
>> +++ b/qapi/qom.json
>> @@ -576,7 +576,7 @@
>>   #
>>   # @prealloc: if true, preallocate memory (default: false)
>>   #
>> -# @prealloc-threads: number of CPU threads to use for prealloc (default: 1)
>> +# @prealloc-threads: number of CPU threads to use for prealloc (default: smp-cpus) (since 7.1)
>
> The property is present since 5.0. Shouldn't this be "(default: smp-cpus) (since 5.0)"?

Yes.

>>   #
>>   # @prealloc-context: thread context to use for creation of preallocation threads
>>   #                    (default: none) (since 7.2)

Re: [PATCH] qom.json: default the prealloc-threads to smp-cpus

Posted by Igor Mammedov 1 year, 6 months ago

On Fri, 04 Nov 2022 05:53:02 +0100
Markus Armbruster <armbru@redhat.com> wrote:

> Philippe Mathieu-Daudé <philmd@linaro.org> writes:
> 
> > On 3/11/22 11:47, Zhenyu Zhang wrote:  
> >> Since the amount of prealloc-threads to smp-cpus is
> >> defaulted in hostmem, so sync this information.  
> 
> Has this always defaulted to smp-cpus, or did this change along the way?
yes and no
(originally value was 1 with vl.c fixing it up to smp-cpus when
legacy -mem-prealloc was used)
  ffac16fab33bb hostmem: introduce "prealloc-threads" property
however later on
  2a4e02d104b1 hostmem: set default prealloc_threads to valid value
hack was merged (sigh),
brought back direct dependency on machine->smp.cpus to memory
allocation and that flipped default to number of smp-cpus.

> >> Signed-off-by: Zhenyu Zhang <zhenyzha@redhat.com>
> >> ---
> >>   qapi/qom.json | 2 +-
> >>   1 file changed, 1 insertion(+), 1 deletion(-)
> >> diff --git a/qapi/qom.json b/qapi/qom.json
> >> index 87fcad2423..ac4cd213a7 100644
> >> --- a/qapi/qom.json
> >> +++ b/qapi/qom.json
> >> @@ -576,7 +576,7 @@
> >>   #
> >>   # @prealloc: if true, preallocate memory (default: false)
> >>   #
> >> -# @prealloc-threads: number of CPU threads to use for prealloc (default: 1)
> >> +# @prealloc-threads: number of CPU threads to use for prealloc (default: smp-cpus) (since 7.1)  
> >
> > The property is present since 5.0. Shouldn't this be "(default: smp-cpus) (since 5.0)"?  
> 
> Yes.
> 
> >>   #
> >>   # @prealloc-context: thread context to use for creation of preallocation threads
> >>   #                    (default: none) (since 7.2)  
>

Re: [PATCH] qom.json: default the prealloc-threads to smp-cpus

Posted by Zhenyu Zhang 1 year, 6 months ago

Hello Philippe

I checked out this modified patch:
https://www.mail-archive.com/qemu-devel@nongnu.org/msg888815.html
It should have been merged in 7.1



On Thu, Nov 3, 2022 at 7:00 PM Philippe Mathieu-Daudé <philmd@linaro.org> wrote:
>
> On 3/11/22 11:47, Zhenyu Zhang wrote:
> > Since the amount of prealloc-threads to smp-cpus is
> > defaulted in hostmem, so sync this information.
> >
> > Signed-off-by: Zhenyu Zhang <zhenyzha@redhat.com>
> > ---
> >   qapi/qom.json | 2 +-
> >   1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/qapi/qom.json b/qapi/qom.json
> > index 87fcad2423..ac4cd213a7 100644
> > --- a/qapi/qom.json
> > +++ b/qapi/qom.json
> > @@ -576,7 +576,7 @@
> >   #
> >   # @prealloc: if true, preallocate memory (default: false)
> >   #
> > -# @prealloc-threads: number of CPU threads to use for prealloc (default: 1)
> > +# @prealloc-threads: number of CPU threads to use for prealloc (default: smp-cpus) (since 7.1)
>
> The property is present since 5.0. Shouldn't this be "(default:
> smp-cpus) (since 5.0)"?
>
> >   #
> >   # @prealloc-context: thread context to use for creation of preallocation threads
> >   #                    (default: none) (since 7.2)
>

Re: [PATCH] qom.json: default the prealloc-threads to smp-cpus

Posted by Philippe Mathieu-Daudé 1 year, 6 months ago

On 4/11/22 02:57, Zhenyu Zhang wrote:
> Hello Philippe
> 
> I checked out this modified patch:
> https://www.mail-archive.com/qemu-devel@nongnu.org/msg888815.html
> It should have been merged in 7.1

This changed the default value, not the property availability.

IIUC the '(since X)' format document when a property got added,
regardless its default value.

This property is available since 5.0.

The default value changed in 7.1. You are correct we need to update
the new value in documentation, but the 'since' tag should refer
to when the property got added: 5.0.

> On Thu, Nov 3, 2022 at 7:00 PM Philippe Mathieu-Daudé <philmd@linaro.org> wrote:
>>
>> On 3/11/22 11:47, Zhenyu Zhang wrote:
>>> Since the amount of prealloc-threads to smp-cpus is
>>> defaulted in hostmem, so sync this information.
>>>
>>> Signed-off-by: Zhenyu Zhang <zhenyzha@redhat.com>
>>> ---
>>>    qapi/qom.json | 2 +-
>>>    1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/qapi/qom.json b/qapi/qom.json
>>> index 87fcad2423..ac4cd213a7 100644
>>> --- a/qapi/qom.json
>>> +++ b/qapi/qom.json
>>> @@ -576,7 +576,7 @@
>>>    #
>>>    # @prealloc: if true, preallocate memory (default: false)
>>>    #
>>> -# @prealloc-threads: number of CPU threads to use for prealloc (default: 1)
>>> +# @prealloc-threads: number of CPU threads to use for prealloc (default: smp-cpus) (since 7.1)
>>
>> The property is present since 5.0. Shouldn't this be "(default:
>> smp-cpus) (since 5.0)"?
>>
>>>    #
>>>    # @prealloc-context: thread context to use for creation of preallocation threads
>>>    #                    (default: none) (since 7.2)
>>
>

Re: [PATCH] qom.json: default the prealloc-threads to smp-cpus

Posted by Zhenyu Zhang 1 year, 6 months ago

Ok, many thanks for the clarification.
I'll post a v2 patch to correct it.

On Fri, Nov 4, 2022 at 5:29 PM Philippe Mathieu-Daudé <philmd@linaro.org> wrote:
>
> On 4/11/22 02:57, Zhenyu Zhang wrote:
> > Hello Philippe
> >
> > I checked out this modified patch:
> > https://www.mail-archive.com/qemu-devel@nongnu.org/msg888815.html
> > It should have been merged in 7.1
>
> This changed the default value, not the property availability.
>
> IIUC the '(since X)' format document when a property got added,
> regardless its default value.
>
> This property is available since 5.0.
>
> The default value changed in 7.1. You are correct we need to update
> the new value in documentation, but the 'since' tag should refer
> to when the property got added: 5.0.
>
> > On Thu, Nov 3, 2022 at 7:00 PM Philippe Mathieu-Daudé <philmd@linaro.org> wrote:
> >>
> >> On 3/11/22 11:47, Zhenyu Zhang wrote:
> >>> Since the amount of prealloc-threads to smp-cpus is
> >>> defaulted in hostmem, so sync this information.
> >>>
> >>> Signed-off-by: Zhenyu Zhang <zhenyzha@redhat.com>
> >>> ---
> >>>    qapi/qom.json | 2 +-
> >>>    1 file changed, 1 insertion(+), 1 deletion(-)
> >>>
> >>> diff --git a/qapi/qom.json b/qapi/qom.json
> >>> index 87fcad2423..ac4cd213a7 100644
> >>> --- a/qapi/qom.json
> >>> +++ b/qapi/qom.json
> >>> @@ -576,7 +576,7 @@
> >>>    #
> >>>    # @prealloc: if true, preallocate memory (default: false)
> >>>    #
> >>> -# @prealloc-threads: number of CPU threads to use for prealloc (default: 1)
> >>> +# @prealloc-threads: number of CPU threads to use for prealloc (default: smp-cpus) (since 7.1)
> >>
> >> The property is present since 5.0. Shouldn't this be "(default:
> >> smp-cpus) (since 5.0)"?
> >>
> >>>    #
> >>>    # @prealloc-context: thread context to use for creation of preallocation threads
> >>>    #                    (default: none) (since 7.2)
> >>
> >
>