On Mon, Jul 19, 2021 at 11:20:39AM +0800, Yanan Wang wrote: > In the real SMP hardware topology world, it's much more likely that > we have high cores-per-socket counts and few sockets totally. While > the current preference of sockets over cores in smp parsing results > in a virtual cpu topology with low cores-per-sockets counts and a > large number of sockets, which is just contrary to the real world. > > Given that it is better to make the virtual cpu topology be more > reflective of the real world and also for the sake of compatibility, > we start to prefer cores over sockets over threads in smp parsing > since machine type 6.2 for different arches. > > In this patch, a boolean "smp_prefer_sockets" is added, and we only > enable the old preference on older machines and enable the new one > since type 6.2 for all arches by using the machine compat mechanism. > > Suggested-by: Daniel P. Berrange <berra...@redhat.com> > Signed-off-by: Yanan Wang <wangyana...@huawei.com>
ppc parts
Acked-by: David Gibson <da...@gibson.dropbear.id.au>
Note that for the pseries machine types, being paravirtual, there is
essentially no guest visible difference between "cores" and "sockets.
> ---
> hw/arm/virt.c | 1 +
> hw/core/machine.c | 59 +++++++++++++++++++++++++++++---------
> hw/i386/pc_piix.c | 1 +
> hw/i386/pc_q35.c | 1 +
> hw/ppc/spapr.c | 1 +
> hw/s390x/s390-virtio-ccw.c | 1 +
> include/hw/boards.h | 1 +
> qemu-options.hx | 4 ++-
> 8 files changed, 55 insertions(+), 14 deletions(-)
>
> diff --git a/hw/arm/virt.c b/hw/arm/virt.c
> index 01165f7f53..7babea40dc 100644
> --- a/hw/arm/virt.c
> +++ b/hw/arm/virt.c
> @@ -2797,6 +2797,7 @@ static void virt_machine_6_1_options(MachineClass *mc)
> {
> virt_machine_6_2_options(mc);
> compat_props_add(mc->compat_props, hw_compat_6_1, hw_compat_6_1_len);
> + mc->smp_prefer_sockets = true;
> }
> DEFINE_VIRT_MACHINE(6, 1)
>
> diff --git a/hw/core/machine.c b/hw/core/machine.c
> index 63439c4a6d..c074425015 100644
> --- a/hw/core/machine.c
> +++ b/hw/core/machine.c
> @@ -744,6 +744,22 @@ void machine_set_cpu_numa_node(MachineState *machine,
> }
> }
>
> +/*
> + * smp_parse - Generic function used to parse the given SMP configuration
> + *
> + * The topology parameters must be specified equal to or great than one
> + * or just omitted, explicit configuration like "cpus=0" is not allowed.
> + * The omitted parameters will be calculated based on the provided ones.
> + *
> + * maxcpus will default to the value of cpus if omitted and will be used
> + * to compute the missing sockets/cores/threads. cpus will be calculated
> + * from the computed parametrs if omitted.
> + *
> + * In calculation of omitted arch-netural sockets/cores/threads, we prefer
> + * sockets over cores over threads before 6.2, while prefer cores over
> + * sockets over threads since 6.2 on. The arch-specific dies will directly
> + * default to 1 if omitted.
> + */
> static void smp_parse(MachineState *ms, SMPConfiguration *config, Error
> **errp)
> {
> MachineClass *mc = MACHINE_GET_CLASS(ms);
> @@ -772,19 +788,36 @@ static void smp_parse(MachineState *ms,
> SMPConfiguration *config, Error **errp)
>
> maxcpus = maxcpus > 0 ? maxcpus : cpus;
>
> - /* compute missing values, prefer sockets over cores over threads */
> - if (sockets == 0) {
> - cores = cores > 0 ? cores : 1;
> - threads = threads > 0 ? threads : 1;
> - sockets = maxcpus / (dies * cores * threads);
> - sockets = sockets > 0 ? sockets : 1;
> - } else if (cores == 0) {
> - threads = threads > 0 ? threads : 1;
> - cores = maxcpus / (sockets * dies * threads);
> - cores = cores > 0 ? cores : 1;
> - } else if (threads == 0) {
> - threads = maxcpus / (sockets * dies * cores);
> - threads = threads > 0 ? threads : 1;
> + /* prefer sockets over cores over threads before 6.2 */
> + if (mc->smp_prefer_sockets) {
> + if (sockets == 0) {
> + cores = cores > 0 ? cores : 1;
> + threads = threads > 0 ? threads : 1;
> + sockets = maxcpus / (dies * cores * threads);
> + sockets = sockets > 0 ? sockets : 1;
> + } else if (cores == 0) {
> + threads = threads > 0 ? threads : 1;
> + cores = maxcpus / (sockets * dies * threads);
> + cores = cores > 0 ? cores : 1;
> + } else if (threads == 0) {
> + threads = maxcpus / (sockets * dies * cores);
> + threads = threads > 0 ? threads : 1;
> + }
> + /* prefer cores over sockets over threads since 6.2 */
> + } else {
> + if (cores == 0) {
> + sockets = sockets > 0 ? sockets : 1;
> + threads = threads > 0 ? threads : 1;
> + cores = maxcpus / (sockets * dies * threads);
> + cores = cores > 0 ? cores : 1;
> + } else if (sockets == 0) {
> + threads = threads > 0 ? threads : 1;
> + sockets = maxcpus / (dies * cores * threads);
> + sockets = sockets > 0 ? sockets : 1;
> + } else if (threads == 0) {
> + threads = maxcpus / (sockets * dies * cores);
> + threads = threads > 0 ? threads : 1;
> + }
> }
>
> /* use the computed parameters to calculate the omitted cpus */
> diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
> index fd5c2277f2..9b811fc6ca 100644
> --- a/hw/i386/pc_piix.c
> +++ b/hw/i386/pc_piix.c
> @@ -432,6 +432,7 @@ static void pc_i440fx_6_1_machine_options(MachineClass *m)
> m->is_default = false;
> compat_props_add(m->compat_props, hw_compat_6_1, hw_compat_6_1_len);
> compat_props_add(m->compat_props, pc_compat_6_1, pc_compat_6_1_len);
> + m->smp_prefer_sockets = true;
> }
>
> DEFINE_I440FX_MACHINE(v6_1, "pc-i440fx-6.1", NULL,
> diff --git a/hw/i386/pc_q35.c b/hw/i386/pc_q35.c
> index b45903b15e..88efb7fde4 100644
> --- a/hw/i386/pc_q35.c
> +++ b/hw/i386/pc_q35.c
> @@ -372,6 +372,7 @@ static void pc_q35_6_1_machine_options(MachineClass *m)
> m->alias = NULL;
> compat_props_add(m->compat_props, hw_compat_6_1, hw_compat_6_1_len);
> compat_props_add(m->compat_props, pc_compat_6_1, pc_compat_6_1_len);
> + m->smp_prefer_sockets = true;
> }
>
> DEFINE_Q35_MACHINE(v6_1, "pc-q35-6.1", NULL,
> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> index d39fd4e644..a481fade51 100644
> --- a/hw/ppc/spapr.c
> +++ b/hw/ppc/spapr.c
> @@ -4702,6 +4702,7 @@ static void
> spapr_machine_6_1_class_options(MachineClass *mc)
> {
> spapr_machine_6_2_class_options(mc);
> compat_props_add(mc->compat_props, hw_compat_6_1, hw_compat_6_1_len);
> + mc->smp_prefer_sockets = true;
> }
>
> DEFINE_SPAPR_MACHINE(6_1, "6.1", false);
> diff --git a/hw/s390x/s390-virtio-ccw.c b/hw/s390x/s390-virtio-ccw.c
> index 4d25278cf2..b40e647883 100644
> --- a/hw/s390x/s390-virtio-ccw.c
> +++ b/hw/s390x/s390-virtio-ccw.c
> @@ -809,6 +809,7 @@ static void ccw_machine_6_1_class_options(MachineClass
> *mc)
> {
> ccw_machine_6_2_class_options(mc);
> compat_props_add(mc->compat_props, hw_compat_6_1, hw_compat_6_1_len);
> + mc->smp_prefer_sockets = true;
> }
> DEFINE_CCW_MACHINE(6_1, "6.1", false);
>
> diff --git a/include/hw/boards.h b/include/hw/boards.h
> index 2832f0f8aa..8df885c9d2 100644
> --- a/include/hw/boards.h
> +++ b/include/hw/boards.h
> @@ -247,6 +247,7 @@ struct MachineClass {
> bool nvdimm_supported;
> bool numa_mem_supported;
> bool smp_dies_supported;
> + bool smp_prefer_sockets;
> bool auto_enable_numa;
> const char *default_ram_id;
>
> diff --git a/qemu-options.hx b/qemu-options.hx
> index 0c9ddc0274..6ef57e838c 100644
> --- a/qemu-options.hx
> +++ b/qemu-options.hx
> @@ -227,7 +227,9 @@ SRST
> 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.
> + behaviour is considered liable to change. The historical preference of
> + sockets over cores over threads works before 6.2, and a new preference
> + of cores over sockets over threads starts to work since 6.2 on.
> ERST
>
> DEF("numa", HAS_ARG, QEMU_OPTION_numa,
--
David Gibson | I'll have my music baroque, and my code
david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson
signature.asc
Description: PGP signature
