qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: "Daniel P. Berrangé" <berrange@redhat.com>
To: "wangyanan (Y)" <wangyanan55@huawei.com>
Cc: Barry Song <song.bao.hua@hisilicon.com>,
	Peter Maydell <peter.maydell@linaro.org>,
	Andrew Jones <drjones@redhat.com>,
	"Michael S . Tsirkin" <mst@redhat.com>,
	wanghaibin.wang@huawei.com, qemu-devel@nongnu.org,
	yangyicong@huawei.com, Shannon Zhao <shannon.zhaosl@gmail.com>,
	qemu-arm@nongnu.org, Alistair Francis <alistair.francis@wdc.com>,
	prime.zeng@hisilicon.com, Paolo Bonzini <pbonzini@redhat.com>,
	yuzenghui@huawei.com, Igor Mammedov <imammedo@redhat.com>,
	zhukeqian1@huawei.com, David Gibson <david@gibson.dropbear.id.au>
Subject: Re: [RFC PATCH v4 0/7] hw/arm/virt: Introduce cpu topology support
Date: Tue, 22 Jun 2021 15:10:57 +0100	[thread overview]
Message-ID: <YNHvcQAMLSpVcxaE@redhat.com> (raw)
In-Reply-To: <7fcc5f2d-cc84-3464-15cc-3bebb07f8190@huawei.com>

On Tue, Jun 22, 2021 at 10:04:52PM +0800, wangyanan (Y) wrote:
> Hi Daniel,
> 
> On 2021/6/22 20:41, Daniel P. Berrangé wrote:
> > On Tue, Jun 22, 2021 at 08:31:22PM +0800, wangyanan (Y) wrote:
> > > 
> > > On 2021/6/22 19:46, Andrew Jones wrote:
> > > > On Tue, Jun 22, 2021 at 11:18:09AM +0100, Daniel P. Berrangé wrote:
> > > > > On Tue, Jun 22, 2021 at 05:34:06PM +0800, Yanan Wang wrote:
> > > > > > Hi,
> > > > > > 
> > > > > > This is v4 of the series [1] that I posted to introduce support for
> > > > > > generating cpu topology descriptions to guest. Comments are welcome!
> > > > > > 
> > > > > > Description:
> > > > > > Once the view of an accurate virtual cpu topology is provided to guest,
> > > > > > with a well-designed vCPU pinning to the pCPU we may get a huge benefit,
> > > > > > e.g., the scheduling performance improvement. See Dario Faggioli's
> > > > > > research and the related performance tests in [2] for reference. So here
> > > > > > we go, this patch series introduces cpu topology support for ARM platform.
> > > > > > 
> > > > > > In this series, instead of quietly enforcing the support for the latest
> > > > > > machine type, a new parameter "expose=on|off" in -smp command line is
> > > > > > introduced to leave QEMU users a choice to decide whether to enable the
> > > > > > feature or not. This will allow the feature to work on different machine
> > > > > > types and also ideally compat with already in-use -smp command lines.
> > > > > > Also we make much stricter requirement for the topology configuration
> > > > > > with "expose=on".
> > > > > Seeing this 'expose=on' parameter feels to me like we're adding a
> > > > > "make-it-work=yes" parameter. IMHO this is just something that should
> > > > > be done by default for the current machine type version and beyond.
> > > > > I don't see the need for a parameter to turnthis on, especially since
> > > > > it is being made architecture specific.
> > > > > 
> > > > I agree.
> > > > 
> > > > Yanan, we never discussed an "expose" parameter in the previous versions
> > > > of this series. We discussed a "strict" parameter though, which would
> > > > allow existing command lines to "work" using assumptions of what the user
> > > > meant and strict=on users to get what they mean or an error saying that
> > > > they asked for something that won't work or would require unreasonable
> > > > assumptions. Why was this changed to an "expose" parameter?
> > > Yes, we indeed discuss a new "strict" parameter but not a "expose" in v2 [1]
> > > of this series.
> > > [1] https://patchwork.kernel.org/project/qemu-devel/patch/20210413080745.33004-6-wangyanan55@huawei.com/
> > > 
> > > And in the discussion, we hoped things would work like below with "strict"
> > > parameter:
> > > Users who want to describe cpu topology should provide cmdline like
> > > 
> > > -smp strict=on,cpus=4,sockets=2,cores=2,threads=1
> > > 
> > > and in this case we require an more accurate -smp configuration and
> > > then generate the cpu topology description through ACPI/DT.
> > > 
> > > While without a strict description, no cpu topology description would
> > > be generated, so they get nothing through ACPI/DT.
> > > 
> > > It seems to me that the "strict" parameter actually serves as a knob to
> > > turn on/off the exposure of topology, and this is the reason I changed
> > > the name.
> > Yes, the use of 'strict=on' is no better than expose=on IMHO.
> > 
> > If I give QEMU a cli
> > 
> >    -smp cpus=4,sockets=2,cores=2,threads=1
> > 
> > then I expect that topology to be exposed to the guest. I shouldn't
> > have to add extra flags to make that happen.
> > 
> > Looking at the thread, it seems the concern was around the fact that
> > the settings were not honoured historically and thus the CLI values
> > could be garbage. ie  -smp cpus=4,sockets=8,cores=3,thread=9
> This "-smp cpus=4,sockets=8,cores=3,threads=9" behaviors as a wrong
> configuration, and the parsing function already report error for this case.
> 
> We hope more complete config like "-smp 4,sockets=2,cores=2,threads=1"
> for exposure of topology, and the incomplete ones like "-smp 4,sockets=1"
> or "-smp 4, cores=1" are not acceptable any more because we are starting
> to expose the topology.

Incomplete specified topologies *are* acceptable.

The smp_parse method will automatically fill in any missing values.

ie,

  -smp 4,cores=1
  -smp cores=1
  -smp threads=1
  -smp sockets=4

are all functionally identical to

  -smp 4,sockets=4,cores=1,dies=1,threads=1


The QEMU man page says this explicitly

                 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.

note this qemu-options.hx doc will require updating since it will apply
to more than just the PC target.

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



  reply	other threads:[~2021-06-22 14:14 UTC|newest]

Thread overview: 34+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-06-22  9:34 Yanan Wang
2021-06-22  9:34 ` [RFC PATCH v4 1/7] vl: Add expose=on|off option support in -smp command line Yanan Wang
2021-06-22  9:34 ` [RFC PATCH v4 2/7] hw/arm/virt: Add separate -smp parsing function for ARM machines Yanan Wang
2021-06-22  9:34 ` [RFC PATCH v4 3/7] machine: disallow -smp expose=on for non-ARM machines Yanan Wang
2021-06-22  9:34 ` [RFC PATCH v4 4/7] device_tree: Add qemu_fdt_add_path Yanan Wang
2021-06-22  9:34 ` [RFC PATCH v4 5/7] hw/arm/virt: Add cpu-map to device tree Yanan Wang
2021-06-22  9:34 ` [RFC PATCH v4 6/7] hw/acpi/aml-build: Add Processor hierarchy node structure Yanan Wang
2021-06-22  9:34 ` [RFC PATCH v4 7/7] hw/acpi/aml-build: Generate PPTT table Yanan Wang
2021-06-22 10:18 ` [RFC PATCH v4 0/7] hw/arm/virt: Introduce cpu topology support Daniel P. Berrangé
2021-06-22 11:46   ` Andrew Jones
2021-06-22 12:31     ` wangyanan (Y)
2021-06-22 12:41       ` Daniel P. Berrangé
2021-06-22 14:04         ` wangyanan (Y)
2021-06-22 14:10           ` Daniel P. Berrangé [this message]
2021-06-22 14:15             ` Peter Maydell
2021-06-22 14:28               ` Daniel P. Berrangé
2021-06-28 11:14                 ` wangyanan (Y)
2021-06-28 11:31                   ` Daniel P. Berrangé
2021-06-28 11:53                     ` wangyanan (Y)
2021-06-22 14:29             ` Andrew Jones
2021-06-22 15:15               ` Daniel P. Berrangé
2021-06-22 15:40               ` Igor Mammedov
2021-06-22 17:08                 ` Andrew Jones
2021-06-22 17:14                 ` Daniel P. Berrangé
2021-06-22 17:29                   ` Andrew Jones
2021-06-22 17:39                     ` Daniel P. Berrangé
2021-06-28  8:43                       ` wangyanan (Y)
2021-06-28  8:58                         ` Andrew Jones
2021-06-28 10:48                           ` wangyanan (Y)
2021-06-30  6:36                           ` wangyanan (Y)
2021-06-30  8:30                             ` Andrew Jones
2021-06-30  9:37                               ` wangyanan (Y)
2021-06-30 11:56                                 ` Andrew Jones
2021-07-01  6:15                                   ` wangyanan (Y)

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=YNHvcQAMLSpVcxaE@redhat.com \
    --to=berrange@redhat.com \
    --cc=alistair.francis@wdc.com \
    --cc=david@gibson.dropbear.id.au \
    --cc=drjones@redhat.com \
    --cc=imammedo@redhat.com \
    --cc=mst@redhat.com \
    --cc=pbonzini@redhat.com \
    --cc=peter.maydell@linaro.org \
    --cc=prime.zeng@hisilicon.com \
    --cc=qemu-arm@nongnu.org \
    --cc=qemu-devel@nongnu.org \
    --cc=shannon.zhaosl@gmail.com \
    --cc=song.bao.hua@hisilicon.com \
    --cc=wanghaibin.wang@huawei.com \
    --cc=wangyanan55@huawei.com \
    --cc=yangyicong@huawei.com \
    --cc=yuzenghui@huawei.com \
    --cc=zhukeqian1@huawei.com \
    --subject='Re: [RFC PATCH v4 0/7] hw/arm/virt: Introduce cpu topology support' \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).