Hello Zhao, On Mon, 4 Mar 2024 at 12:19, Zhao Liu <zhao1....@linux.intel.com> wrote: > > unsigned maxcpus = config->has_maxcpus ? config->maxcpus : 0; > > This indicates the default maxcpus is initialized as 0 if user doesn't > specifies it.
* 'has_maxcpus' should be set only if maxcpus > 0. If maxcpus == 0, then setting 'has_maxcpus=1' seems convoluted. > However, we could initialize maxcpus as other default value, e.g., > > maxcpus = config->has_maxcpus ? config->maxcpus : 1. === hw/core/machine.c machine_initfn /* default to mc->default_cpus */ ms->smp.cpus = mc->default_cpus; ms->smp.max_cpus = mc->default_cpus; static void machine_class_base_init(ObjectClass *oc, void *data) { MachineClass *mc = MACHINE_CLASS(oc); mc->max_cpus = mc->max_cpus ?: 1; mc->min_cpus = mc->min_cpus ?: 1; mc->default_cpus = mc->default_cpus ?: 1; } === * Looking at the above bits, it seems smp.cpus & smp.max_cpus are initialised to 1 via default_cpus in MachineClass object. >> if (config->has_maxcpus && config->maxcpus == 0) > This check only wants to identify the case that user sets the 0. > If the default maxcpus is initialized as 0, then (maxcpus == 0) will > fail if user doesn't set maxcpus. > > But it is still necessary to distinguish whether maxcpus is user-set or > auto-initialized. * If it is set to zero(0) either by user or by auto-initialise, it is still invalid, right? > If it is user-set, -smp should fail is there's invalid maxcpus/invalid > topology. > > Otherwise, if it is auto-initialized, its value should be adjusted based > on other topology components as the above calculation in (*). * Why have such diverging ways? * Could we simplify it as - If cpus/maxcpus==0, it is invalid, show an error and exit. - If cpus/maxcpus > 0, but incorrect for topology, then re-calculate the correct value based on topology parameters. If the re-calculated value is still incorrect or unsatisfactory, then show an error and exit. * Saying that user setting cpu/maxcpus=0 is invalid and auto-initialising it to zero(0) is valid, is not consistent. ...wdyt? --- - Prasad