Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1397288 > unrolled thread

Re: Regression introduced by cf6d445f68974d0b15a14cf6021be38a91f2b5d8

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  Re: Regression introduced by  cf6d445f68974d0b15a14cf6021be38a91f2b5d8 Thomas Gleixner <tglx@linutronix.de> - 2016-05-09 22:00 +0200
    RE: Regression introduced by  cf6d445f68974d0b15a14cf6021be38a91f2b5d8 "Liang, Kan" <kan.liang@intel.com> - 2016-05-09 23:00 +0200
    Re: Regression introduced by cf6d445f68974d0b15a14cf6021be38a91f2b5d8 Peter Zijlstra <peterz@infradead.org> - 2016-05-10 09:00 +0200

#1397288 — Re: Regression introduced by cf6d445f68974d0b15a14cf6021be38a91f2b5d8

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?

Thanks,

	tglx

[toc] | [next] | [standalone]

#1397386

> 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

[toc] | [prev] | [next] | [standalone]

#1397737 — Re: Regression introduced by cf6d445f68974d0b15a14cf6021be38a91f2b5d8

On Mon, May 09, 2016 at 09:48:18PM +0200, Thomas Gleixner wrote:

> 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

This I think; because:

> 
> 2) Make detect_extended_topology() functional for SMP=n, so the real number of
>    cores is detected

UP has no business 'knowing' anything about cores/threads and general
topology stuff.

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web