RE: Regression introduced by cf6d445f68974d0b15a14cf6021be38a91f2b5d8

From: Liang, Kan
Date: Mon May 09 2016 - 16:55:42 EST



> Josef,
>
> On Mon, 9 May 2016, Josef Bacik wrote:
>
> > I've hit a regression that was introduced by the commit in $SUBJECT.
> > I'm building a minimal kernel config that doesn't have CONFIG_SMP set,
> > which results in the topology for the box to look different than with
> > CONFIG_SMP set. Specifically boot_cpu_data.x86_max_cores comes out to
> > 16 without CONFIG_SMP set, but 12 with CONFIG_SMP set.
>
> So the commit is just exposing the underlying wreckage.
>
> x86_max_cores is set via:
>
> detect_extended_topology(), which is a NOOP in case of SMP=n
>
> or
>
> if the above is not available, which is the case for SMP=n, then it
> uses:
>
> intel_num_cpu_cores()
>
> That's using the cache leaf Bits 26-31:
>
> Maximum number of addressable IDs for processor cores in the physical
> package
>
> And that's 16 for this CPU, but that has nothing to do with the actual
> number of cores in the package.
>
> So that explains the wreckage you are seing. We have two options to deal
> with
> this:
>
> 1) Make intel_num_cpu_cores() a NOOP for SMP=n, so x86_max_cores = 1
>
> 2) Make detect_extended_topology() functional for SMP=n, so the real
> number of
> cores is detected
>
> Both options work and make sense.
>
> Though I don't know whether the uncore stuff wants to see all boxes of a
> package even in the SMP=n case. Kan?

I think the boxes num should not be larger than the max cores.
So if SMP=n, there is only one core. The boxes num should be one as well.
Option one looks good to me.

Thanks,
Kan