[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH v11 11/14] machine: Make smp_parse generic enough for all arc
From: |
Markus Armbruster |
Subject: |
Re: [PATCH v11 11/14] machine: Make smp_parse generic enough for all arches |
Date: |
Tue, 28 Sep 2021 14:25:15 +0200 |
User-agent: |
Gnus/5.13 (Gnus v5.13) Emacs/27.2 (gnu/linux) |
Philippe Mathieu-Daudé <philmd@redhat.com> writes:
> On 9/28/21 05:57, Yanan Wang wrote:
>> Currently the only difference between smp_parse and pc_smp_parse
>> is the support of dies parameter and the related error reporting.
>> With some arch compat variables like "bool dies_supported", we can
>> make smp_parse generic enough for all arches and the PC specific
>> one can be removed.
>>
>> Making smp_parse() generic enough can reduce code duplication and
>> ease the code maintenance, and also allows extending the topology
>> with more arch specific members (e.g., clusters) in the future.
>>
>> Suggested-by: Andrew Jones <drjones@redhat.com>
>> Suggested-by: Daniel P. Berrange <berrange@redhat.com>
>> Signed-off-by: Yanan Wang <wangyanan55@huawei.com>
>> ---
>> hw/core/machine.c | 91 +++++++++++++++++++++++++++++++++++----------
>> hw/i386/pc.c | 84 +----------------------------------------
>> include/hw/boards.h | 9 +++++
>> 3 files changed, 81 insertions(+), 103 deletions(-)
>
>> +/*
>> + * smp_parse - Generic function used to parse the given SMP configuration
>> + *
>> + * Any missing parameter in "cpus/maxcpus/sockets/cores/threads" will be
>> + * automatically computed based on the provided ones.
>> + *
>> + * In the calculation of omitted sockets/cores/threads: we prefer sockets
>> + * over cores over threads before 6.2, while preferring cores over sockets
>> + * over threads since 6.2.
>> + *
>> + * In the calculation of cpus/maxcpus: When both maxcpus and cpus are
>> omitted,
>> + * maxcpus will be computed from the given parameters and cpus will be set
>> + * equal to maxcpus. When only one of maxcpus and cpus is given then the
>> + * omitted one will be set to its given counterpart's value. Both maxcpus
>> and
>> + * cpus may be specified, but maxcpus must be equal to or greater than cpus.
>> + *
>> + * For compatibility, apart from the parameters that will be computed, newly
>> + * introduced topology members which are likely to be target specific should
>> + * be directly set as 1 if they are omitted (e.g. dies for PC since 4.1).
>> + */
>> static void smp_parse(MachineState *ms, SMPConfiguration *config, Error
>> **errp)
>
> Can we have it return a boolean instead?
Yes, please. From error.h's big comment:
* = Rules =
[...]
* - Whenever practical, also return a value that indicates success /
* failure. This can make the error checking more concise, and can
* avoid useless error object creation and destruction. Note that
* we still have many functions returning void. We recommend
* • bool-valued functions return true on success / false on failure,
* • pointer-valued functions return non-null / null pointer, and
* • integer-valued functions return non-negative / negative.
- Re: [PATCH v11 03/14] machine: Uniformly use maxcpus to calculate the omitted parameters, (continued)
[PATCH v11 10/14] machine: Tweak the order of topology members in struct CpuTopology, Yanan Wang, 2021/09/27
[PATCH v11 13/14] machine: Move smp_prefer_sockets to struct SMPCompatProps, Yanan Wang, 2021/09/27
[PATCH v11 12/14] machine: Remove smp_parse callback from MachineClass, Yanan Wang, 2021/09/27
[PATCH v11 14/14] machine: Put all sanity-check in the generic SMP parser, Yanan Wang, 2021/09/27