[PATCH v4 03/10] hw/core/machine: Wrap target specific parameters together

Yanan Wang via posted 10 patches 4 years, 2 months ago
Maintainers: Eduardo Habkost <ehabkost@redhat.com>, Marcel Apfelbaum <marcel.apfelbaum@gmail.com>, Ani Sinha <ani@anisinha.ca>, "Michael S. Tsirkin" <mst@redhat.com>, Shannon Zhao <shannon.zhaosl@gmail.com>, "Philippe Mathieu-Daudé" <philmd@redhat.com>, Peter Maydell <peter.maydell@linaro.org>, Igor Mammedov <imammedo@redhat.com>, Eric Blake <eblake@redhat.com>, Paolo Bonzini <pbonzini@redhat.com>, Markus Armbruster <armbru@redhat.com>
There is a newer version of this series
[PATCH v4 03/10] hw/core/machine: Wrap target specific parameters together
Posted by Yanan Wang via 4 years, 2 months ago
Wrap the CPU target specific parameters together into a single
variable, so that we don't need to update the other lines but
a single line when new topology parameters are introduced.

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 87ceb45470..2a3f16e52b 100644
--- a/hw/core/machine-smp.c
+++ b/hw/core/machine-smp.c
@@ -77,6 +77,7 @@ void smp_parse(MachineState *ms, SMPConfiguration *config, Error **errp)
     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,
@@ -109,6 +110,8 @@ void smp_parse(MachineState *ms, SMPConfiguration *config, Error **errp)
     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;
@@ -122,30 +125,30 @@ void smp_parse(MachineState *ms, SMPConfiguration *config, Error **errp)
             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;
@@ -157,7 +160,7 @@ void smp_parse(MachineState *ms, SMPConfiguration *config, Error **errp)
     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: "
-- 
2.19.1


Re: [PATCH v4 03/10] hw/core/machine: Wrap target specific parameters together
Posted by Philippe Mathieu-Daudé 4 years, 1 month ago
On 11/21/21 13:24, Yanan Wang wrote:
> Wrap the CPU target specific parameters together into a single
> variable, so that we don't need to update the other lines but
> a single line when new topology parameters are introduced.

Where new params are introduced? Not in this series apparently.

> 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(-)


Re: [PATCH v4 03/10] hw/core/machine: Wrap target specific parameters together
Posted by wangyanan (Y) via 4 years, 1 month ago
Hi,

On 2021/12/16 21:23, Philippe Mathieu-Daudé wrote:
> On 11/21/21 13:24, Yanan Wang wrote:
>> Wrap the CPU target specific parameters together into a single
>> variable, so that we don't need to update the other lines but
>> a single line when new topology parameters are introduced.
> Where new params are introduced? Not in this series apparently.
The commit message may not clearly express what I mean.
A new parameter "clusters" is added in patch #2, and now we have
specific dies and clusters, I tried to wrap these two parameters together
so that the code lines can be shorter and look more concise.

If it's thought not that necessary to do this change, I will get rid of this
patch of course.

Thanks,
Yanan
>
>> 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(-)
> .