On Mon, Jul 19, 2021 at 11:20:36AM +0800, Yanan Wang wrote:
> Currently we directly calculate the omitted cpus based on the already
> provided parameters. This makes some cmdlines like:
> -smp maxcpus=16
> -smp sockets=2,maxcpus=16
> -smp sockets=2,dies=2,maxcpus=16
> -smp sockets=2,cores=4,maxcpus=16
> not work. We should probably use the computed paramters to calculate
> cpus when maxcpus is provided while cpus is omitted, which will make
> above configs start to work.
>
> Note: change in this patch won't affect any existing working cmdlines
> but allows more incomplete configs to be valid.
>
> Signed-off-by: Yanan Wang <wangyana...@huawei.com>
> ---
> hw/core/machine.c | 17 +++++++++--------
> 1 file changed, 9 insertions(+), 8 deletions(-)
>
> diff --git a/hw/core/machine.c b/hw/core/machine.c
> index c9f15b15a5..668f0a1553 100644
> --- a/hw/core/machine.c
> +++ b/hw/core/machine.c
> @@ -767,26 +767,27 @@ static void smp_parse(MachineState *ms,
> SMPConfiguration *config, Error **errp)
> return;
> }
>
> - /* compute missing values, prefer sockets over cores over threads */
> maxcpus = maxcpus > 0 ? maxcpus : cpus;
>
> - if (cpus == 0) {
> - sockets = sockets > 0 ? sockets : 1;
> - cores = cores > 0 ? cores : 1;
> - threads = threads > 0 ? threads : 1;
> - cpus = sockets * dies * cores * threads;
> - maxcpus = maxcpus > 0 ? maxcpus : cpus;
> - } else if (sockets == 0) {
> + /* compute missing values, prefer sockets over cores over threads */
> + if (sockets == 0) {
> cores = cores > 0 ? cores : 1;
> threads = threads > 0 ? threads : 1;
> sockets = maxcpus / (dies * cores * threads);
> + sockets = sockets > 0 ? sockets : 1;
> } else if (cores == 0) {
> threads = threads > 0 ? threads : 1;
> cores = maxcpus / (sockets * dies * threads);
> + cores = cores > 0 ? cores : 1;
> } else if (threads == 0) {
> threads = maxcpus / (sockets * dies * cores);
> + threads = threads > 0 ? threads : 1;
> }
I didn't think we wanted this rounding which this patch adds back into
cores and threads and now also sockets.
>
> + /* use the computed parameters to calculate the omitted cpus */
> + cpus = cpus > 0 ? cpus : sockets * dies * cores * threads;
> + maxcpus = maxcpus > 0 ? maxcpus : cpus;
It doesn't really matter, but I think I'd rather write this like
maxcpus = maxcpus > 0 ? maxcpus : sockets * dies * cores * threads;
cpus = cpus > 0 ? cpus : maxcpus;
> +
> if (sockets * dies * cores * threads < cpus) {
> g_autofree char *dies_msg = g_strdup_printf(
> mc->smp_dies_supported ? " * dies (%u)" : "", dies);
> --
> 2.19.1
>
Thanks,
drew
