Message ID | 20210728034848.75228-7-wangyanan55@huawei.com (mailing list archive) |
---|---|
State | New, archived |
Headers | show |
Series | machine: smp parsing fixes and improvement | expand |
On Wed, Jul 28, 2021 at 11:48:43AM +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. > > Acked-by: David Gibson <david@gibson.dropbear.id.au> > Suggested-by: Daniel P. Berrange <berrange@redhat.com> > Signed-off-by: Yanan Wang <wangyanan55@huawei.com> > --- > hw/arm/virt.c | 1 + > hw/core/machine.c | 36 ++++++++++++++++++++++++++---------- > hw/i386/pc.c | 36 ++++++++++++++++++++++++++---------- > 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 | 3 ++- > 9 files changed, 60 insertions(+), 21 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 458d9736e3..a8173a0f45 100644 > --- a/hw/core/machine.c > +++ b/hw/core/machine.c > @@ -746,6 +746,7 @@ void machine_set_cpu_numa_node(MachineState *machine, > > static void smp_parse(MachineState *ms, SMPConfiguration *config, Error **errp) > { > + MachineClass *mc = MACHINE_GET_CLASS(ms); > unsigned cpus = config->has_cpus ? config->cpus : 0; > unsigned sockets = config->has_sockets ? config->sockets : 0; > unsigned cores = config->has_cores ? config->cores : 0; > @@ -757,7 +758,7 @@ static void smp_parse(MachineState *ms, SMPConfiguration *config, Error **errp) > return; > } > > - /* compute missing values, prefer sockets over cores over threads */ > + /* compute missing values based on the provided ones */ > if (cpus == 0 && maxcpus == 0) { > sockets = sockets > 0 ? sockets : 1; > cores = cores > 0 ? cores : 1; > @@ -765,15 +766,30 @@ static void smp_parse(MachineState *ms, SMPConfiguration *config, Error **errp) > } else { > maxcpus = maxcpus > 0 ? maxcpus : cpus; > > - if (sockets == 0) { > - cores = cores > 0 ? cores : 1; > - threads = threads > 0 ? threads : 1; > - sockets = maxcpus / (cores * threads); > - } else if (cores == 0) { > - threads = threads > 0 ? threads : 1; > - cores = maxcpus / (sockets * threads); > - } else if (threads == 0) { > - threads = maxcpus / (sockets * cores); > + if (mc->smp_prefer_sockets) { > + /* prefer sockets over cores over threads before 6.2 */ > + if (sockets == 0) { > + cores = cores > 0 ? cores : 1; > + threads = threads > 0 ? threads : 1; > + sockets = maxcpus / (cores * threads); > + } else if (cores == 0) { > + threads = threads > 0 ? threads : 1; > + cores = maxcpus / (sockets * threads); > + } else if (threads == 0) { > + threads = maxcpus / (sockets * cores); > + } > + } else { > + /* prefer cores over sockets over threads since 6.2 */ > + if (cores == 0) { > + sockets = sockets > 0 ? sockets : 1; > + threads = threads > 0 ? threads : 1; > + cores = maxcpus / (sockets * threads); > + } else if (sockets == 0) { > + threads = threads > 0 ? threads : 1; > + sockets = maxcpus / (cores * threads); > + } else if (threads == 0) { > + threads = maxcpus / (sockets * cores); > + } > } > } > > diff --git a/hw/i386/pc.c b/hw/i386/pc.c > index 8c2235ac46..77ab764c5d 100644 > --- a/hw/i386/pc.c > +++ b/hw/i386/pc.c > @@ -717,6 +717,7 @@ void pc_acpi_smi_interrupt(void *opaque, int irq, int level) > */ > static void pc_smp_parse(MachineState *ms, SMPConfiguration *config, Error **errp) > { > + MachineClass *mc = MACHINE_GET_CLASS(ms); > unsigned cpus = config->has_cpus ? config->cpus : 0; > unsigned sockets = config->has_sockets ? config->sockets : 0; > unsigned dies = config->has_dies ? config->dies : 0; > @@ -727,7 +728,7 @@ static void pc_smp_parse(MachineState *ms, SMPConfiguration *config, Error **err > /* directly default dies to 1 if it's omitted */ > dies = dies > 0 ? dies : 1; > > - /* compute missing values, prefer sockets over cores over threads */ > + /* compute missing values based on the provided ones */ > if (cpus == 0 && maxcpus == 0) { > sockets = sockets > 0 ? sockets : 1; > cores = cores > 0 ? cores : 1; > @@ -735,15 +736,30 @@ static void pc_smp_parse(MachineState *ms, SMPConfiguration *config, Error **err > } else { > maxcpus = maxcpus > 0 ? maxcpus : cpus; > > - if (sockets == 0) { > - cores = cores > 0 ? cores : 1; > - threads = threads > 0 ? threads : 1; > - sockets = maxcpus / (dies * cores * threads); > - } else if (cores == 0) { > - threads = threads > 0 ? threads : 1; > - cores = maxcpus / (sockets * dies * threads); > - } else if (threads == 0) { > - threads = maxcpus / (sockets * dies * cores); > + if (mc->smp_prefer_sockets) { > + /* prefer sockets over cores over threads before 6.2 */ > + if (sockets == 0) { > + cores = cores > 0 ? cores : 1; > + threads = threads > 0 ? threads : 1; > + sockets = maxcpus / (dies * cores * threads); > + } else if (cores == 0) { > + threads = threads > 0 ? threads : 1; > + cores = maxcpus / (sockets * dies * threads); > + } else if (threads == 0) { > + threads = maxcpus / (sockets * dies * cores); > + } > + } else { > + /* prefer cores over sockets over threads since 6.2 */ > + if (cores == 0) { > + sockets = sockets > 0 ? sockets : 1; > + threads = threads > 0 ? threads : 1; > + cores = maxcpus / (sockets * dies * threads); > + } else if (sockets == 0) { > + threads = threads > 0 ? threads : 1; > + sockets = maxcpus / (dies * cores * threads); > + } else if (threads == 0) { > + threads = maxcpus / (sockets * dies * cores); > + } > } > } > > 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 463a5514f9..2ae039b74f 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 auto_enable_numa; > + bool smp_prefer_sockets; > const char *default_ram_id; > > HotplugHandler *(*get_hotplug_handler)(MachineState *machine, > diff --git a/qemu-options.hx b/qemu-options.hx > index 0912236b4b..450f511ca7 100644 > --- a/qemu-options.hx > +++ b/qemu-options.hx > @@ -234,7 +234,8 @@ SRST > 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. > + liable to change. Prior to 6.2 the preference was sockets over cores > + over threads. Since 6.2 the preference is cores over sockets over threads. > ERST > > DEF("numa", HAS_ARG, QEMU_OPTION_numa, > -- > 2.19.1 > Reviewed-by: Andrew Jones <drjones@redhat.com>
On Wed, Jul 28 2021, Yanan Wang <wangyanan55@huawei.com> 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. > > Acked-by: David Gibson <david@gibson.dropbear.id.au> > Suggested-by: Daniel P. Berrange <berrange@redhat.com> > Signed-off-by: Yanan Wang <wangyanan55@huawei.com> > --- > hw/arm/virt.c | 1 + > hw/core/machine.c | 36 ++++++++++++++++++++++++++---------- > hw/i386/pc.c | 36 ++++++++++++++++++++++++++---------- > 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 | 3 ++- > 9 files changed, 60 insertions(+), 21 deletions(-) Acked-by: Cornelia Huck <cohuck@redhat.com>
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 458d9736e3..a8173a0f45 100644 --- a/hw/core/machine.c +++ b/hw/core/machine.c @@ -746,6 +746,7 @@ void machine_set_cpu_numa_node(MachineState *machine, static void smp_parse(MachineState *ms, SMPConfiguration *config, Error **errp) { + MachineClass *mc = MACHINE_GET_CLASS(ms); unsigned cpus = config->has_cpus ? config->cpus : 0; unsigned sockets = config->has_sockets ? config->sockets : 0; unsigned cores = config->has_cores ? config->cores : 0; @@ -757,7 +758,7 @@ static void smp_parse(MachineState *ms, SMPConfiguration *config, Error **errp) return; } - /* compute missing values, prefer sockets over cores over threads */ + /* compute missing values based on the provided ones */ if (cpus == 0 && maxcpus == 0) { sockets = sockets > 0 ? sockets : 1; cores = cores > 0 ? cores : 1; @@ -765,15 +766,30 @@ static void smp_parse(MachineState *ms, SMPConfiguration *config, Error **errp) } else { maxcpus = maxcpus > 0 ? maxcpus : cpus; - if (sockets == 0) { - cores = cores > 0 ? cores : 1; - threads = threads > 0 ? threads : 1; - sockets = maxcpus / (cores * threads); - } else if (cores == 0) { - threads = threads > 0 ? threads : 1; - cores = maxcpus / (sockets * threads); - } else if (threads == 0) { - threads = maxcpus / (sockets * cores); + if (mc->smp_prefer_sockets) { + /* prefer sockets over cores over threads before 6.2 */ + if (sockets == 0) { + cores = cores > 0 ? cores : 1; + threads = threads > 0 ? threads : 1; + sockets = maxcpus / (cores * threads); + } else if (cores == 0) { + threads = threads > 0 ? threads : 1; + cores = maxcpus / (sockets * threads); + } else if (threads == 0) { + threads = maxcpus / (sockets * cores); + } + } else { + /* prefer cores over sockets over threads since 6.2 */ + if (cores == 0) { + sockets = sockets > 0 ? sockets : 1; + threads = threads > 0 ? threads : 1; + cores = maxcpus / (sockets * threads); + } else if (sockets == 0) { + threads = threads > 0 ? threads : 1; + sockets = maxcpus / (cores * threads); + } else if (threads == 0) { + threads = maxcpus / (sockets * cores); + } } } diff --git a/hw/i386/pc.c b/hw/i386/pc.c index 8c2235ac46..77ab764c5d 100644 --- a/hw/i386/pc.c +++ b/hw/i386/pc.c @@ -717,6 +717,7 @@ void pc_acpi_smi_interrupt(void *opaque, int irq, int level) */ static void pc_smp_parse(MachineState *ms, SMPConfiguration *config, Error **errp) { + MachineClass *mc = MACHINE_GET_CLASS(ms); unsigned cpus = config->has_cpus ? config->cpus : 0; unsigned sockets = config->has_sockets ? config->sockets : 0; unsigned dies = config->has_dies ? config->dies : 0; @@ -727,7 +728,7 @@ static void pc_smp_parse(MachineState *ms, SMPConfiguration *config, Error **err /* directly default dies to 1 if it's omitted */ dies = dies > 0 ? dies : 1; - /* compute missing values, prefer sockets over cores over threads */ + /* compute missing values based on the provided ones */ if (cpus == 0 && maxcpus == 0) { sockets = sockets > 0 ? sockets : 1; cores = cores > 0 ? cores : 1; @@ -735,15 +736,30 @@ static void pc_smp_parse(MachineState *ms, SMPConfiguration *config, Error **err } else { maxcpus = maxcpus > 0 ? maxcpus : cpus; - if (sockets == 0) { - cores = cores > 0 ? cores : 1; - threads = threads > 0 ? threads : 1; - sockets = maxcpus / (dies * cores * threads); - } else if (cores == 0) { - threads = threads > 0 ? threads : 1; - cores = maxcpus / (sockets * dies * threads); - } else if (threads == 0) { - threads = maxcpus / (sockets * dies * cores); + if (mc->smp_prefer_sockets) { + /* prefer sockets over cores over threads before 6.2 */ + if (sockets == 0) { + cores = cores > 0 ? cores : 1; + threads = threads > 0 ? threads : 1; + sockets = maxcpus / (dies * cores * threads); + } else if (cores == 0) { + threads = threads > 0 ? threads : 1; + cores = maxcpus / (sockets * dies * threads); + } else if (threads == 0) { + threads = maxcpus / (sockets * dies * cores); + } + } else { + /* prefer cores over sockets over threads since 6.2 */ + if (cores == 0) { + sockets = sockets > 0 ? sockets : 1; + threads = threads > 0 ? threads : 1; + cores = maxcpus / (sockets * dies * threads); + } else if (sockets == 0) { + threads = threads > 0 ? threads : 1; + sockets = maxcpus / (dies * cores * threads); + } else if (threads == 0) { + threads = maxcpus / (sockets * dies * cores); + } } } 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 463a5514f9..2ae039b74f 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 auto_enable_numa; + bool smp_prefer_sockets; const char *default_ram_id; HotplugHandler *(*get_hotplug_handler)(MachineState *machine, diff --git a/qemu-options.hx b/qemu-options.hx index 0912236b4b..450f511ca7 100644 --- a/qemu-options.hx +++ b/qemu-options.hx @@ -234,7 +234,8 @@ SRST 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. + liable to change. Prior to 6.2 the preference was sockets over cores + over threads. Since 6.2 the preference is cores over sockets over threads. ERST DEF("numa", HAS_ARG, QEMU_OPTION_numa,