Re: [PATCH 4/4] qemu-options: rewrite help for -smp options

From: "Daniel P. Berrangé" <berrange@redhat.com>
To: "wangyanan (Y)" <wangyanan55@huawei.com>
Cc: Andrew Jones <drjones@redhat.com>,
	Eduardo Habkost <ehabkost@redhat.com>,
	Richard Henderson <richard.henderson@linaro.org>,
	qemu-devel@nongnu.org, Paolo Bonzini <pbonzini@redhat.com>,
	Igor Mammedov <imammedo@redhat.com>
Subject: Re: [PATCH 4/4] qemu-options: rewrite help for -smp options
Date: Fri, 9 Jul 2021 15:15:13 +0100	[thread overview]
Message-ID: <YOhZ8RncPfTq6LiR@redhat.com> (raw)
In-Reply-To: <0fe6ae73-62f2-569b-1e9d-04bd7da7458a@huawei.com>

On Mon, Jun 28, 2021 at 09:46:16PM +0800, wangyanan (Y) wrote:
> Hi Daniel,
> 
> On 2021/6/28 19:30, Daniel P. Berrangé wrote:
> > The -smp option help is peculiarly specific about mentioning the CPU
> > upper limits, but these are wrong. The "PC" target has varying max
> > CPU counts depending on the machine type picked. Notes about guest
> > OS limits are inappropriate for QEMU docs. There are way too many
> > machine types for it to be practical to mention actual limits, and
> > some limits are even modified by downstream distribtions. Thus it
> > is better to remove the specific limits entirely.
> > 
> > The CPU topology reporting is also not neccessarily specific to the
> > PC platform and descriptions around the rules of usage are somewhat
> > terse. Expand this information with some examples to show effects
> > of defaulting.
> > 
> > Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
> > ---
> >   qemu-options.hx | 29 +++++++++++++++++++++--------
> >   1 file changed, 21 insertions(+), 8 deletions(-)
> > 
> > diff --git a/qemu-options.hx b/qemu-options.hx
> > index 5871df7291..0021e9ec7b 100644
> > --- a/qemu-options.hx
> > +++ b/qemu-options.hx
> > @@ -207,14 +207,27 @@ DEF("smp", HAS_ARG, QEMU_OPTION_smp,
> >           QEMU_ARCH_ALL)
> >   SRST
> >   ``-smp [[cpus=]n][,maxcpus=maxcpus][,sockets=sockets][,dies=dies][,cores=cores][,threads=threads]``
> > -    Simulate an SMP system with n CPUs. On the PC target, up to 255 CPUs
> > -    are supported. On Sparc32 target, Linux limits the number of usable
> > -    CPUs to 4. For the PC target, the number of cores per die, the
> > -    number of threads per cores, the number of dies per packages and the
> > -    total number of sockets can be specified. Missing values will be
> > -    computed. If any on the three values is given, the total number of
> > -    CPUs n can be omitted. maxcpus specifies the maximum number of
> > -    hotpluggable CPUs.
> > +    Simulate an SMP system with '\ ``n``\ ' CPUs initially present on
> Should be "a SMP system".

Pre-existing bug, but I'll fix it anyway

> > +    the machine type board. On boards supporting CPU hotplug, the optional
> > +    '\ ``maxcpus``\ ' parameter can be set to enable further CPUs to be
> > +    added at runtime. If omitted the maximum number of CPUs will be
> > +    set to match the initial CPU count. Both parameters are subject to
> > +    an upper limit that is determined by the specific machine type chosen.
> > +
> > +    To control reporting of CPU topology information, the number of sockets,
> > +    dies per socket, cores per die, and threads per core can be specified.
> > +    The sum `` sockets * cores * dies * threads `` must be equal to the
> > +    maximum CPU count. CPU targets may only support a subset of the topology
> > +    parameters. Where a CPU target does not support use of a particular
> > +    topology parameter, its value should be assumed to be 1 for the purpose
> > +    of computing the CPU maximum count.
> > +
> Explicitly saying "sockets * dies * cores * threads" seems not arch-neutral
> at
> first glance, although we have the explanation behind. How about the
> following statement for this paragraph?
> 
> "
> To control reporting of CPU topology information, at most the number of
> sockets,
> dies per socket, cores per die, and threads per core can be specified. CPU
> targets
> may only support a subset of the topology parameters. If a CPU target does
> not
> support use of a particular topology parameter, it must not be specified.
> The sum
> of the supported subset of parameters must be equal to the maximum CPU
> count.
> "
> 
> I think this also make it easier to expand if we are going to add one more
> topology
> parameter, e.g, cluster, in the future.

I won't make this suggested change, since we discussed against another
patch that mgmt apps like libvirt will already be setting 'dies=1' for
any target. We merely need QEMU to reject values > 1 if not supported.

> > +    Either the initial CPU count, or at least one of the topology parameters
> > +    must be specified. Values for any omitted parameters will be computed
> > +    from those which are given. Historically preference was given to the
> > +    coarsest topology parameters when computing missing values (ie sockets
> > +    preferred over cores, which were preferred over threads), however, this
> > +    behaviour is considered liable to change.
> >   ERST
> >   DEF("numa", HAS_ARG, QEMU_OPTION_numa,
> Thanks,
> Yanan
> .
> 

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 :|