Message ID | 20210928035755.11684-6-wangyanan55@huawei.com |
---|---|
State | New |
Headers | show |
Series | machine: smp parsing fixes and improvement | expand |
On Tue, Sep 28, 2021 at 11:57:46AM +0800, Yanan Wang wrote: > We have two requirements for a valid SMP configuration: > the product of "sockets * cores * threads" must represent all the > possible cpus, i.e., max_cpus, and then must include the initially > present cpus, i.e., smp_cpus. > > So we only need to ensure 1) "sockets * cores * threads == maxcpus" > at first and then ensure 2) "maxcpus >= cpus". With a reasonable > order of the sanity check, we can simplify the error reporting code. > When reporting an error message we also report the exact value of > each topology member to make users easily see what's going on. > > Signed-off-by: Yanan Wang <wangyanan55@huawei.com> > Reviewed-by: Andrew Jones <drjones@redhat.com> > Reviewed-by: Pankaj Gupta <pankaj.gupta@ionos.com> > --- > hw/core/machine.c | 22 +++++++++------------- > hw/i386/pc.c | 24 ++++++++++-------------- > 2 files changed, 19 insertions(+), 27 deletions(-) Reviewed-by: Daniel P. Berrangé <berrange@redhat.com> Regards, Daniel
On 9/28/21 05:57, Yanan Wang wrote: > We have two requirements for a valid SMP configuration: > the product of "sockets * cores * threads" must represent all the > possible cpus, i.e., max_cpus, and then must include the initially > present cpus, i.e., smp_cpus. > > So we only need to ensure 1) "sockets * cores * threads == maxcpus" > at first and then ensure 2) "maxcpus >= cpus". With a reasonable > order of the sanity check, we can simplify the error reporting code. > When reporting an error message we also report the exact value of > each topology member to make users easily see what's going on. > > Signed-off-by: Yanan Wang <wangyanan55@huawei.com> > Reviewed-by: Andrew Jones <drjones@redhat.com> > Reviewed-by: Pankaj Gupta <pankaj.gupta@ionos.com> > --- > hw/core/machine.c | 22 +++++++++------------- > hw/i386/pc.c | 24 ++++++++++-------------- > 2 files changed, 19 insertions(+), 27 deletions(-) Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
diff --git a/hw/core/machine.c b/hw/core/machine.c index fe935cb4a3..f1b30b3101 100644 --- a/hw/core/machine.c +++ b/hw/core/machine.c @@ -782,25 +782,21 @@ static void smp_parse(MachineState *ms, SMPConfiguration *config, Error **errp) maxcpus = maxcpus > 0 ? maxcpus : sockets * cores * threads; cpus = cpus > 0 ? cpus : maxcpus; - if (sockets * cores * threads < cpus) { - error_setg(errp, "cpu topology: " - "sockets (%u) * cores (%u) * threads (%u) < " - "smp_cpus (%u)", - sockets, cores, threads, cpus); + if (sockets * cores * threads != maxcpus) { + error_setg(errp, "Invalid CPU topology: " + "product of the hierarchy must match maxcpus: " + "sockets (%u) * cores (%u) * threads (%u) " + "!= maxcpus (%u)", + sockets, cores, threads, maxcpus); return; } if (maxcpus < cpus) { - error_setg(errp, "maxcpus must be equal to or greater than smp"); - return; - } - - if (sockets * cores * threads != maxcpus) { error_setg(errp, "Invalid CPU topology: " + "maxcpus must be equal to or greater than smp: " "sockets (%u) * cores (%u) * threads (%u) " - "!= maxcpus (%u)", - sockets, cores, threads, - maxcpus); + "== maxcpus (%u) < smp_cpus (%u)", + sockets, cores, threads, maxcpus, cpus); return; } diff --git a/hw/i386/pc.c b/hw/i386/pc.c index d9382b7d57..a37eef8057 100644 --- a/hw/i386/pc.c +++ b/hw/i386/pc.c @@ -749,25 +749,21 @@ static void pc_smp_parse(MachineState *ms, SMPConfiguration *config, Error **err maxcpus = maxcpus > 0 ? maxcpus : sockets * dies * cores * threads; cpus = cpus > 0 ? cpus : maxcpus; - if (sockets * dies * cores * threads < cpus) { - error_setg(errp, "cpu topology: " - "sockets (%u) * dies (%u) * cores (%u) * threads (%u) < " - "smp_cpus (%u)", - sockets, dies, cores, threads, cpus); + if (sockets * dies * cores * threads != maxcpus) { + error_setg(errp, "Invalid CPU topology: " + "product of the hierarchy must match maxcpus: " + "sockets (%u) * dies (%u) * cores (%u) * threads (%u) " + "!= maxcpus (%u)", + sockets, dies, cores, threads, maxcpus); return; } if (maxcpus < cpus) { - error_setg(errp, "maxcpus must be equal to or greater than smp"); - return; - } - - if (sockets * dies * cores * threads != maxcpus) { - error_setg(errp, "Invalid CPU topology deprecated: " + error_setg(errp, "Invalid CPU topology: " + "maxcpus must be equal to or greater than smp: " "sockets (%u) * dies (%u) * cores (%u) * threads (%u) " - "!= maxcpus (%u)", - sockets, dies, cores, threads, - maxcpus); + "== maxcpus (%u) < smp_cpus (%u)", + sockets, dies, cores, threads, maxcpus, cpus); return; }