| Message ID | 20211228092221.21068-4-wangyanan55@huawei.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | ARM virt: Introduce CPU clusters topology support | expand |
On 12/28/21 10:22, Yanan Wang wrote: > Wrap the CPU target specific parameters together into a single > variable except generic sockets/cores/threads, to make related > code lines shorter and more concise. > > No functional change intended. > > Signed-off-by: Yanan Wang <wangyanan55@huawei.com> > --- > hw/core/machine-smp.c | 17 ++++++++++------- > 1 file changed, 10 insertions(+), 7 deletions(-) > > diff --git a/hw/core/machine-smp.c b/hw/core/machine-smp.c > index b39ed21e65..4547d7bbdc 100644 > --- a/hw/core/machine-smp.c > +++ b/hw/core/machine-smp.c > @@ -79,6 +79,7 @@ void machine_parse_smp_config(MachineState *ms, > unsigned cores = config->has_cores ? config->cores : 0; > unsigned threads = config->has_threads ? config->threads : 0; > unsigned maxcpus = config->has_maxcpus ? config->maxcpus : 0; > + unsigned others; > > /* > * Specified CPU topology parameters must be greater than zero, > @@ -111,6 +112,8 @@ void machine_parse_smp_config(MachineState *ms, > dies = dies > 0 ? dies : 1; > clusters = clusters > 0 ? clusters : 1; > > + others = dies * clusters; This doesn't seem clearer to me... > /* compute missing values based on the provided ones */ > if (cpus == 0 && maxcpus == 0) { > sockets = sockets > 0 ? sockets : 1; > @@ -124,30 +127,30 @@ void machine_parse_smp_config(MachineState *ms, > if (sockets == 0) { > cores = cores > 0 ? cores : 1; > threads = threads > 0 ? threads : 1; > - sockets = maxcpus / (dies * clusters * cores * threads); > + sockets = maxcpus / (cores * threads * others); > } else if (cores == 0) { > threads = threads > 0 ? threads : 1; > - cores = maxcpus / (sockets * dies * clusters * threads); > + cores = maxcpus / (sockets * threads * others); > } > } 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 * clusters * threads); > + cores = maxcpus / (sockets * threads * others); > } else if (sockets == 0) { > threads = threads > 0 ? threads : 1; > - sockets = maxcpus / (dies * clusters * cores * threads); > + sockets = maxcpus / (cores * threads * others); > } > } > > /* try to calculate omitted threads at last */ > if (threads == 0) { > - threads = maxcpus / (sockets * dies * clusters * cores); > + threads = maxcpus / (sockets * cores * others); > } > } > > - maxcpus = maxcpus > 0 ? maxcpus : sockets * dies * clusters * cores * threads; > + maxcpus = maxcpus > 0 ? maxcpus : sockets * cores * threads * others; > cpus = cpus > 0 ? cpus : maxcpus; > > ms->smp.cpus = cpus; > @@ -159,7 +162,7 @@ void machine_parse_smp_config(MachineState *ms, > ms->smp.max_cpus = maxcpus; > > /* sanity-check of the computed topology */ > - if (sockets * dies * clusters * cores * threads != maxcpus) { > + if (sockets * cores * threads * others != maxcpus) { > g_autofree char *topo_msg = cpu_hierarchy_to_string(ms); > error_setg(errp, "Invalid CPU topology: " > "product of the hierarchy must match maxcpus: "
On 2021/12/29 3:23, Philippe Mathieu-Daudé wrote: > On 12/28/21 10:22, Yanan Wang wrote: >> Wrap the CPU target specific parameters together into a single >> variable except generic sockets/cores/threads, to make related >> code lines shorter and more concise. >> >> No functional change intended. >> >> Signed-off-by: Yanan Wang <wangyanan55@huawei.com> >> --- >> hw/core/machine-smp.c | 17 ++++++++++------- >> 1 file changed, 10 insertions(+), 7 deletions(-) >> >> diff --git a/hw/core/machine-smp.c b/hw/core/machine-smp.c >> index b39ed21e65..4547d7bbdc 100644 >> --- a/hw/core/machine-smp.c >> +++ b/hw/core/machine-smp.c >> @@ -79,6 +79,7 @@ void machine_parse_smp_config(MachineState *ms, >> unsigned cores = config->has_cores ? config->cores : 0; >> unsigned threads = config->has_threads ? config->threads : 0; >> unsigned maxcpus = config->has_maxcpus ? config->maxcpus : 0; >> + unsigned others; >> >> /* >> * Specified CPU topology parameters must be greater than zero, >> @@ -111,6 +112,8 @@ void machine_parse_smp_config(MachineState *ms, >> dies = dies > 0 ? dies : 1; >> clusters = clusters > 0 ? clusters : 1; >> >> + others = dies * clusters; > This doesn't seem clearer to me... Ok. We can just drop this patch. Thanks, Yanan >> /* compute missing values based on the provided ones */ >> if (cpus == 0 && maxcpus == 0) { >> sockets = sockets > 0 ? sockets : 1; >> @@ -124,30 +127,30 @@ void machine_parse_smp_config(MachineState *ms, >> if (sockets == 0) { >> cores = cores > 0 ? cores : 1; >> threads = threads > 0 ? threads : 1; >> - sockets = maxcpus / (dies * clusters * cores * threads); >> + sockets = maxcpus / (cores * threads * others); >> } else if (cores == 0) { >> threads = threads > 0 ? threads : 1; >> - cores = maxcpus / (sockets * dies * clusters * threads); >> + cores = maxcpus / (sockets * threads * others); >> } >> } 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 * clusters * threads); >> + cores = maxcpus / (sockets * threads * others); >> } else if (sockets == 0) { >> threads = threads > 0 ? threads : 1; >> - sockets = maxcpus / (dies * clusters * cores * threads); >> + sockets = maxcpus / (cores * threads * others); >> } >> } >> >> /* try to calculate omitted threads at last */ >> if (threads == 0) { >> - threads = maxcpus / (sockets * dies * clusters * cores); >> + threads = maxcpus / (sockets * cores * others); >> } >> } >> >> - maxcpus = maxcpus > 0 ? maxcpus : sockets * dies * clusters * cores * threads; >> + maxcpus = maxcpus > 0 ? maxcpus : sockets * cores * threads * others; >> cpus = cpus > 0 ? cpus : maxcpus; >> >> ms->smp.cpus = cpus; >> @@ -159,7 +162,7 @@ void machine_parse_smp_config(MachineState *ms, >> ms->smp.max_cpus = maxcpus; >> >> /* sanity-check of the computed topology */ >> - if (sockets * dies * clusters * cores * threads != maxcpus) { >> + if (sockets * cores * threads * others != maxcpus) { >> g_autofree char *topo_msg = cpu_hierarchy_to_string(ms); >> error_setg(errp, "Invalid CPU topology: " >> "product of the hierarchy must match maxcpus: " > .
diff --git a/hw/core/machine-smp.c b/hw/core/machine-smp.c index b39ed21e65..4547d7bbdc 100644 --- a/hw/core/machine-smp.c +++ b/hw/core/machine-smp.c @@ -79,6 +79,7 @@ void machine_parse_smp_config(MachineState *ms, unsigned cores = config->has_cores ? config->cores : 0; unsigned threads = config->has_threads ? config->threads : 0; unsigned maxcpus = config->has_maxcpus ? config->maxcpus : 0; + unsigned others; /* * Specified CPU topology parameters must be greater than zero, @@ -111,6 +112,8 @@ void machine_parse_smp_config(MachineState *ms, dies = dies > 0 ? dies : 1; clusters = clusters > 0 ? clusters : 1; + others = dies * clusters; + /* compute missing values based on the provided ones */ if (cpus == 0 && maxcpus == 0) { sockets = sockets > 0 ? sockets : 1; @@ -124,30 +127,30 @@ void machine_parse_smp_config(MachineState *ms, if (sockets == 0) { cores = cores > 0 ? cores : 1; threads = threads > 0 ? threads : 1; - sockets = maxcpus / (dies * clusters * cores * threads); + sockets = maxcpus / (cores * threads * others); } else if (cores == 0) { threads = threads > 0 ? threads : 1; - cores = maxcpus / (sockets * dies * clusters * threads); + cores = maxcpus / (sockets * threads * others); } } 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 * clusters * threads); + cores = maxcpus / (sockets * threads * others); } else if (sockets == 0) { threads = threads > 0 ? threads : 1; - sockets = maxcpus / (dies * clusters * cores * threads); + sockets = maxcpus / (cores * threads * others); } } /* try to calculate omitted threads at last */ if (threads == 0) { - threads = maxcpus / (sockets * dies * clusters * cores); + threads = maxcpus / (sockets * cores * others); } } - maxcpus = maxcpus > 0 ? maxcpus : sockets * dies * clusters * cores * threads; + maxcpus = maxcpus > 0 ? maxcpus : sockets * cores * threads * others; cpus = cpus > 0 ? cpus : maxcpus; ms->smp.cpus = cpus; @@ -159,7 +162,7 @@ void machine_parse_smp_config(MachineState *ms, ms->smp.max_cpus = maxcpus; /* sanity-check of the computed topology */ - if (sockets * dies * clusters * cores * threads != maxcpus) { + if (sockets * cores * threads * others != maxcpus) { g_autofree char *topo_msg = cpu_hierarchy_to_string(ms); error_setg(errp, "Invalid CPU topology: " "product of the hierarchy must match maxcpus: "
Wrap the CPU target specific parameters together into a single variable except generic sockets/cores/threads, to make related code lines shorter and more concise. No functional change intended. Signed-off-by: Yanan Wang <wangyanan55@huawei.com> --- hw/core/machine-smp.c | 17 ++++++++++------- 1 file changed, 10 insertions(+), 7 deletions(-)