| Message ID | 20210813023912.105880-7-wangyanan55@huawei.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | machine: smp parsing fixes and improvement | expand |
> 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 <berrange@redhat.com> > Signed-off-by: Yanan Wang <wangyanan55@huawei.com> > Acked-by: David Gibson <david@gibson.dropbear.id.au> > Acked-by: Cornelia Huck <cohuck@redhat.com> > Reviewed-by: Andrew Jones <drjones@redhat.com> > --- > hw/arm/virt.c | 1 + > hw/core/machine.c | 35 ++++++++++++++++++++++++++--------- > hw/i386/pc.c | 35 ++++++++++++++++++++++++++--------- > 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(+), 19 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; Will this be set for virt_machine_6_2_? > } > DEFINE_VIRT_MACHINE(6, 1) > > diff --git a/hw/core/machine.c b/hw/core/machine.c > index bdce80df32..15b41c52e8 100644 > --- a/hw/core/machine.c > +++ b/hw/core/machine.c > @@ -748,6 +748,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; > @@ -759,7 +760,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; > @@ -767,14 +768,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) { > + if (mc->smp_prefer_sockets) { > + /* prefer sockets over cores 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 { > + /* prefer cores over sockets 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); > + } > + } > + > + /* try to calculate omitted threads at last */ > + if (threads == 0) { > threads = maxcpus / (sockets * cores); > } > } > diff --git a/hw/i386/pc.c b/hw/i386/pc.c > index afd8b9c283..4b05ff7160 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,14 +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) { > + if (mc->smp_prefer_sockets) { > + /* prefer sockets over cores 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 { > + /* prefer cores over sockets 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); > + } > + } > + > + /* try to calculate omitted threads at last */ > + 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 06f819177e..451d2cd817 100644 > --- a/qemu-options.hx > +++ b/qemu-options.hx > @@ -238,7 +238,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 >
On 2021/8/14 0:30, Pankaj Gupta 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 <berrange@redhat.com> >> Signed-off-by: Yanan Wang <wangyanan55@huawei.com> >> Acked-by: David Gibson <david@gibson.dropbear.id.au> >> Acked-by: Cornelia Huck <cohuck@redhat.com> >> Reviewed-by: Andrew Jones <drjones@redhat.com> >> --- >> hw/arm/virt.c | 1 + >> hw/core/machine.c | 35 ++++++++++++++++++++++++++--------- >> hw/i386/pc.c | 35 ++++++++++++++++++++++++++--------- >> 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(+), 19 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; > Will this be set for virt_machine_6_2_? > Hi, We hope to start to assume cores over sockets on virt_6_2 machines and the later, and keep the old style of assuming sockets over cores on virt_6_1 machines and the older. Is there any concern here? Thanks, Yanan >> } >> DEFINE_VIRT_MACHINE(6, 1) >> >> diff --git a/hw/core/machine.c b/hw/core/machine.c >> index bdce80df32..15b41c52e8 100644 >> --- a/hw/core/machine.c >> +++ b/hw/core/machine.c >> @@ -748,6 +748,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; >> @@ -759,7 +760,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; >> @@ -767,14 +768,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) { >> + if (mc->smp_prefer_sockets) { >> + /* prefer sockets over cores 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 { >> + /* prefer cores over sockets 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); >> + } >> + } >> + >> + /* try to calculate omitted threads at last */ >> + if (threads == 0) { >> threads = maxcpus / (sockets * cores); >> } >> } >> diff --git a/hw/i386/pc.c b/hw/i386/pc.c >> index afd8b9c283..4b05ff7160 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,14 +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) { >> + if (mc->smp_prefer_sockets) { >> + /* prefer sockets over cores 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 { >> + /* prefer cores over sockets 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); >> + } >> + } >> + >> + /* try to calculate omitted threads at last */ >> + 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 06f819177e..451d2cd817 100644 >> --- a/qemu-options.hx >> +++ b/qemu-options.hx >> @@ -238,7 +238,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 >> > .
> On 2021/8/14 0:30, Pankaj Gupta 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 <berrange@redhat.com> > >> Signed-off-by: Yanan Wang <wangyanan55@huawei.com> > >> Acked-by: David Gibson <david@gibson.dropbear.id.au> > >> Acked-by: Cornelia Huck <cohuck@redhat.com> > >> Reviewed-by: Andrew Jones <drjones@redhat.com> > >> --- > >> hw/arm/virt.c | 1 + > >> hw/core/machine.c | 35 ++++++++++++++++++++++++++--------- > >> hw/i386/pc.c | 35 ++++++++++++++++++++++++++--------- > >> 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(+), 19 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; > > Will this be set for virt_machine_6_2_? > > > Hi, > > We hope to start to assume cores over sockets on virt_6_2 machines > and the later, and keep the old style of assuming sockets over cores > on virt_6_1 machines and the older. Is there any concern here? O.k. Thank you for clarifying. Best regards, Pankaj > > Thanks, > Yanan > >> } > >> DEFINE_VIRT_MACHINE(6, 1) > >> > >> diff --git a/hw/core/machine.c b/hw/core/machine.c > >> index bdce80df32..15b41c52e8 100644 > >> --- a/hw/core/machine.c > >> +++ b/hw/core/machine.c > >> @@ -748,6 +748,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; > >> @@ -759,7 +760,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; > >> @@ -767,14 +768,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) { > >> + if (mc->smp_prefer_sockets) { > >> + /* prefer sockets over cores 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 { > >> + /* prefer cores over sockets 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); > >> + } > >> + } > >> + > >> + /* try to calculate omitted threads at last */ > >> + if (threads == 0) { > >> threads = maxcpus / (sockets * cores); > >> } > >> } > >> diff --git a/hw/i386/pc.c b/hw/i386/pc.c > >> index afd8b9c283..4b05ff7160 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,14 +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) { > >> + if (mc->smp_prefer_sockets) { > >> + /* prefer sockets over cores 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 { > >> + /* prefer cores over sockets 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); > >> + } > >> + } > >> + > >> + /* try to calculate omitted threads at last */ > >> + 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 06f819177e..451d2cd817 100644 > >> --- a/qemu-options.hx > >> +++ b/qemu-options.hx > >> @@ -238,7 +238,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 > >> > > . >
> 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 <berrange@redhat.com> > Signed-off-by: Yanan Wang <wangyanan55@huawei.com> > Acked-by: David Gibson <david@gibson.dropbear.id.au> > Acked-by: Cornelia Huck <cohuck@redhat.com> > Reviewed-by: Andrew Jones <drjones@redhat.com> > --- > hw/arm/virt.c | 1 + > hw/core/machine.c | 35 ++++++++++++++++++++++++++--------- > hw/i386/pc.c | 35 ++++++++++++++++++++++++++--------- > 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(+), 19 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 bdce80df32..15b41c52e8 100644 > --- a/hw/core/machine.c > +++ b/hw/core/machine.c > @@ -748,6 +748,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; > @@ -759,7 +760,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; > @@ -767,14 +768,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) { > + if (mc->smp_prefer_sockets) { > + /* prefer sockets over cores 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 { > + /* prefer cores over sockets 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); > + } > + } > + > + /* try to calculate omitted threads at last */ > + if (threads == 0) { > threads = maxcpus / (sockets * cores); > } > } > diff --git a/hw/i386/pc.c b/hw/i386/pc.c > index afd8b9c283..4b05ff7160 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,14 +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) { > + if (mc->smp_prefer_sockets) { > + /* prefer sockets over cores 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 { > + /* prefer cores over sockets 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); > + } > + } > + > + /* try to calculate omitted threads at last */ > + 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 06f819177e..451d2cd817 100644 > --- a/qemu-options.hx > +++ b/qemu-options.hx > @@ -238,7 +238,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, > -- Reviewed-by: Pankaj Gupta <pankaj.gupta@ionos.com>
On 2021/8/16 16:02, Pankaj Gupta 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 <berrange@redhat.com> >> Signed-off-by: Yanan Wang <wangyanan55@huawei.com> >> Acked-by: David Gibson <david@gibson.dropbear.id.au> >> Acked-by: Cornelia Huck <cohuck@redhat.com> >> Reviewed-by: Andrew Jones <drjones@redhat.com> >> --- >> hw/arm/virt.c | 1 + >> hw/core/machine.c | 35 ++++++++++++++++++++++++++--------- >> hw/i386/pc.c | 35 ++++++++++++++++++++++++++--------- >> 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(+), 19 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 bdce80df32..15b41c52e8 100644 >> --- a/hw/core/machine.c >> +++ b/hw/core/machine.c >> @@ -748,6 +748,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; >> @@ -759,7 +760,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; >> @@ -767,14 +768,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) { >> + if (mc->smp_prefer_sockets) { >> + /* prefer sockets over cores 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 { >> + /* prefer cores over sockets 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); >> + } >> + } >> + >> + /* try to calculate omitted threads at last */ >> + if (threads == 0) { >> threads = maxcpus / (sockets * cores); >> } >> } >> diff --git a/hw/i386/pc.c b/hw/i386/pc.c >> index afd8b9c283..4b05ff7160 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,14 +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) { >> + if (mc->smp_prefer_sockets) { >> + /* prefer sockets over cores 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 { >> + /* prefer cores over sockets 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); >> + } >> + } >> + >> + /* try to calculate omitted threads at last */ >> + 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 06f819177e..451d2cd817 100644 >> --- a/qemu-options.hx >> +++ b/qemu-options.hx >> @@ -238,7 +238,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, >> -- > Reviewed-by: Pankaj Gupta <pankaj.gupta@ionos.com> > . Thanks! Yanan .
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 bdce80df32..15b41c52e8 100644 --- a/hw/core/machine.c +++ b/hw/core/machine.c @@ -748,6 +748,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; @@ -759,7 +760,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; @@ -767,14 +768,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) { + if (mc->smp_prefer_sockets) { + /* prefer sockets over cores 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 { + /* prefer cores over sockets 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); + } + } + + /* try to calculate omitted threads at last */ + if (threads == 0) { threads = maxcpus / (sockets * cores); } } diff --git a/hw/i386/pc.c b/hw/i386/pc.c index afd8b9c283..4b05ff7160 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,14 +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) { + if (mc->smp_prefer_sockets) { + /* prefer sockets over cores 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 { + /* prefer cores over sockets 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); + } + } + + /* try to calculate omitted threads at last */ + 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 06f819177e..451d2cd817 100644 --- a/qemu-options.hx +++ b/qemu-options.hx @@ -238,7 +238,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,