Message ID | 20210728034848.75228-2-wangyanan55@huawei.com |
---|---|
State | New |
Headers | show |
Series | machine: smp parsing fixes and improvement | expand |
On Wed, Jul 28, 2021 at 11:48:38AM +0800, Yanan Wang wrote: > To pave the way for the functional improvement in later patches, > make some refactor/cleanup for the smp parsers, including using > local maxcpus instead of ms->smp.max_cpus in the calculation, > defaulting dies to 0 initially like other members, cleanup the > sanity check for dies. > > No functional change intended. > > Signed-off-by: Yanan Wang <wangyanan55@huawei.com> > --- > hw/core/machine.c | 19 +++++++++++-------- > hw/i386/pc.c | 23 ++++++++++++++--------- > 2 files changed, 25 insertions(+), 17 deletions(-) > > diff --git a/hw/core/machine.c b/hw/core/machine.c > index e1533dfc47..ffc0629854 100644 > --- a/hw/core/machine.c > +++ b/hw/core/machine.c > @@ -747,9 +747,11 @@ static void smp_parse(MachineState *ms, SMPConfiguration *config, Error **errp) > unsigned sockets = config->has_sockets ? config->sockets : 0; > unsigned cores = config->has_cores ? config->cores : 0; > unsigned threads = config->has_threads ? config->threads : 0; > + unsigned maxcpus = config->has_maxcpus ? config->maxcpus : 0; > > - if (config->has_dies && config->dies != 0 && config->dies != 1) { > + if (config->has_dies && config->dies > 1) { > error_setg(errp, "dies not supported by this machine's CPU topology"); > + return; > } > > /* compute missing values, prefer sockets over cores over threads */ > @@ -760,8 +762,8 @@ static void smp_parse(MachineState *ms, SMPConfiguration *config, Error **errp) > sockets = sockets > 0 ? sockets : 1; > cpus = cores * threads * sockets; > } else { > - ms->smp.max_cpus = config->has_maxcpus ? config->maxcpus : cpus; > - sockets = ms->smp.max_cpus / (cores * threads); > + maxcpus = maxcpus > 0 ? maxcpus : cpus; > + sockets = maxcpus / (sockets * cores); Should be divided by (cores * threads) like before. Thanks, drew
On 2021/7/29 4:16, Andrew Jones wrote: > On Wed, Jul 28, 2021 at 11:48:38AM +0800, Yanan Wang wrote: >> To pave the way for the functional improvement in later patches, >> make some refactor/cleanup for the smp parsers, including using >> local maxcpus instead of ms->smp.max_cpus in the calculation, >> defaulting dies to 0 initially like other members, cleanup the >> sanity check for dies. >> >> No functional change intended. >> >> Signed-off-by: Yanan Wang <wangyanan55@huawei.com> >> --- >> hw/core/machine.c | 19 +++++++++++-------- >> hw/i386/pc.c | 23 ++++++++++++++--------- >> 2 files changed, 25 insertions(+), 17 deletions(-) >> >> diff --git a/hw/core/machine.c b/hw/core/machine.c >> index e1533dfc47..ffc0629854 100644 >> --- a/hw/core/machine.c >> +++ b/hw/core/machine.c >> @@ -747,9 +747,11 @@ static void smp_parse(MachineState *ms, SMPConfiguration *config, Error **errp) >> unsigned sockets = config->has_sockets ? config->sockets : 0; >> unsigned cores = config->has_cores ? config->cores : 0; >> unsigned threads = config->has_threads ? config->threads : 0; >> + unsigned maxcpus = config->has_maxcpus ? config->maxcpus : 0; >> >> - if (config->has_dies && config->dies != 0 && config->dies != 1) { >> + if (config->has_dies && config->dies > 1) { >> error_setg(errp, "dies not supported by this machine's CPU topology"); >> + return; >> } >> >> /* compute missing values, prefer sockets over cores over threads */ >> @@ -760,8 +762,8 @@ static void smp_parse(MachineState *ms, SMPConfiguration *config, Error **errp) >> sockets = sockets > 0 ? sockets : 1; >> cpus = cores * threads * sockets; >> } else { >> - ms->smp.max_cpus = config->has_maxcpus ? config->maxcpus : cpus; >> - sockets = ms->smp.max_cpus / (cores * threads); >> + maxcpus = maxcpus > 0 ? maxcpus : cpus; >> + sockets = maxcpus / (sockets * cores); > Should be divided by (cores * threads) like before. Absolutely... Will fix it. Thanks, Yanan > Thanks, > drew > > .
diff --git a/hw/core/machine.c b/hw/core/machine.c index e1533dfc47..ffc0629854 100644 --- a/hw/core/machine.c +++ b/hw/core/machine.c @@ -747,9 +747,11 @@ static void smp_parse(MachineState *ms, SMPConfiguration *config, Error **errp) unsigned sockets = config->has_sockets ? config->sockets : 0; unsigned cores = config->has_cores ? config->cores : 0; unsigned threads = config->has_threads ? config->threads : 0; + unsigned maxcpus = config->has_maxcpus ? config->maxcpus : 0; - if (config->has_dies && config->dies != 0 && config->dies != 1) { + if (config->has_dies && config->dies > 1) { error_setg(errp, "dies not supported by this machine's CPU topology"); + return; } /* compute missing values, prefer sockets over cores over threads */ @@ -760,8 +762,8 @@ static void smp_parse(MachineState *ms, SMPConfiguration *config, Error **errp) sockets = sockets > 0 ? sockets : 1; cpus = cores * threads * sockets; } else { - ms->smp.max_cpus = config->has_maxcpus ? config->maxcpus : cpus; - sockets = ms->smp.max_cpus / (cores * threads); + maxcpus = maxcpus > 0 ? maxcpus : cpus; + sockets = maxcpus / (sockets * cores); } } else if (cores == 0) { threads = threads > 0 ? threads : 1; @@ -778,26 +780,27 @@ static void smp_parse(MachineState *ms, SMPConfiguration *config, Error **errp) return; } - ms->smp.max_cpus = config->has_maxcpus ? config->maxcpus : cpus; + maxcpus = maxcpus > 0 ? maxcpus : cpus; - if (ms->smp.max_cpus < cpus) { + if (maxcpus < cpus) { error_setg(errp, "maxcpus must be equal to or greater than smp"); return; } - if (sockets * cores * threads != ms->smp.max_cpus) { + if (sockets * cores * threads != maxcpus) { error_setg(errp, "Invalid CPU topology: " "sockets (%u) * cores (%u) * threads (%u) " "!= maxcpus (%u)", sockets, cores, threads, - ms->smp.max_cpus); + maxcpus); return; } ms->smp.cpus = cpus; + ms->smp.sockets = sockets; ms->smp.cores = cores; ms->smp.threads = threads; - ms->smp.sockets = sockets; + ms->smp.max_cpus = maxcpus; } static void machine_get_smp(Object *obj, Visitor *v, const char *name, diff --git a/hw/i386/pc.c b/hw/i386/pc.c index c2b9d62a35..acd31af452 100644 --- a/hw/i386/pc.c +++ b/hw/i386/pc.c @@ -716,9 +716,13 @@ static void pc_smp_parse(MachineState *ms, SMPConfiguration *config, Error **err { unsigned cpus = config->has_cpus ? config->cpus : 0; unsigned sockets = config->has_sockets ? config->sockets : 0; - unsigned dies = config->has_dies ? config->dies : 1; + unsigned dies = config->has_dies ? config->dies : 0; unsigned cores = config->has_cores ? config->cores : 0; unsigned threads = config->has_threads ? config->threads : 0; + unsigned maxcpus = config->has_maxcpus ? config->maxcpus : 0; + + /* directly default dies to 1 if it's omitted */ + dies = dies > 0 ? dies : 1; /* compute missing values, prefer sockets over cores over threads */ if (cpus == 0 || sockets == 0) { @@ -728,8 +732,8 @@ static void pc_smp_parse(MachineState *ms, SMPConfiguration *config, Error **err sockets = sockets > 0 ? sockets : 1; cpus = cores * threads * dies * sockets; } else { - ms->smp.max_cpus = config->has_maxcpus ? config->maxcpus : cpus; - sockets = ms->smp.max_cpus / (cores * threads * dies); + maxcpus = maxcpus > 0 ? maxcpus : cpus; + sockets = maxcpus / (dies * cores * threads); } } else if (cores == 0) { threads = threads > 0 ? threads : 1; @@ -746,27 +750,28 @@ static void pc_smp_parse(MachineState *ms, SMPConfiguration *config, Error **err return; } - ms->smp.max_cpus = config->has_maxcpus ? config->maxcpus : cpus; + maxcpus = maxcpus > 0 ? maxcpus : cpus; - if (ms->smp.max_cpus < cpus) { + if (maxcpus < cpus) { error_setg(errp, "maxcpus must be equal to or greater than smp"); return; } - if (sockets * dies * cores * threads != ms->smp.max_cpus) { + if (sockets * dies * cores * threads != maxcpus) { error_setg(errp, "Invalid CPU topology deprecated: " "sockets (%u) * dies (%u) * cores (%u) * threads (%u) " "!= maxcpus (%u)", sockets, dies, cores, threads, - ms->smp.max_cpus); + maxcpus); return; } ms->smp.cpus = cpus; - ms->smp.cores = cores; - ms->smp.threads = threads; ms->smp.sockets = sockets; ms->smp.dies = dies; + ms->smp.cores = cores; + ms->smp.threads = threads; + ms->smp.maxcpus = maxcpus; } static
To pave the way for the functional improvement in later patches, make some refactor/cleanup for the smp parsers, including using local maxcpus instead of ms->smp.max_cpus in the calculation, defaulting dies to 0 initially like other members, cleanup the sanity check for dies. No functional change intended. Signed-off-by: Yanan Wang <wangyanan55@huawei.com> --- hw/core/machine.c | 19 +++++++++++-------- hw/i386/pc.c | 23 ++++++++++++++--------- 2 files changed, 25 insertions(+), 17 deletions(-)