[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH v5 03/14] hw/core/machine: Wrap target specific parameters to
From: |
Philippe Mathieu-Daudé |
Subject: |
Re: [PATCH v5 03/14] hw/core/machine: Wrap target specific parameters together |
Date: |
Tue, 28 Dec 2021 20:23:02 +0100 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101 Thunderbird/91.4.0 |
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: "