Re: [PATCH v6 01/14] x86: add socket_to_cpumask

[Chao Peng <chao.p.peng@linux.intel.com>]

On Tue, May 05, 2015 at 11:43:57AM +0100, Jan Beulich wrote:
> >>> On 05.05.15 at 12:25, <chao.p.peng@linux.intel.com> wrote:
> > On Tue, May 05, 2015 at 10:11:15AM +0100, Jan Beulich wrote:
> >> >>> On 05.05.15 at 09:44, <chao.p.peng@linux.intel.com> wrote:
> >> > On Fri, Apr 24, 2015 at 03:46:11PM +0100, Jan Beulich wrote:
> >> >> >>> On 23.04.15 at 11:55, <chao.p.peng@linux.intel.com> wrote:
> >> >> > @@ -717,6 +722,14 @@ void __init smp_prepare_cpus(unsigned int max_cpus)
> >> >> >  
> >> >> >      stack_base[0] = stack_start;
> >> >> >  
> >> >> > +    nr_sockets = DIV_ROUND_UP(nr_cpu_ids, boot_cpu_data.x86_max_cores *
> >> >> > +                                          
> > boot_cpu_data.x86_num_siblings);
> >> >> 
> >> >> I think this is going to be problematic when there are more CPUs
> >> >> in the system than Xen can (or is told to) handle.
> >> > 
> >> > After dived into the code I think there is no way we can do it
> >> > correctly, especially when we want to get the nr_sockets which counts
> >> > in the unplugged socket(s) as well.
> >> 
> >> Without you explaining why it's really hard to judge whether that's
> >> indeed the case. After all we have all necessary information in hand
> >> when booting all CPUs, so I can't see why we shouldn't be able to
> >> use that information also when booting only a subset.
> > 
> > My concern is the original APIC IDs do not need to be consecutive. Then
> > in such case even we do
> > 
> > DIV_ROUND_UP(cpu_num_from_madt, boot_cpu_data.x86_max_cores *
> >                                 boot_cpu_data.x86_num_siblings);
> > 
> > would not be correct.
> > 
> > E.g. If we have a machine like this (Each package has two cores and each
> > core has two threads but only 3 processors enumerated in MADT):
> > 
> > APIC_ID  Package_ID Core_ID SMT_ID
> >  1(001)  0          0       1
> >  2(010)  0          1       0
> >  4(100)  1          0       0
> > 
> > Then nr_sockets = ROUND_UP( 3 / (2 * 2) ) = 1 but we do have two sockets.
> 
> But that's the case regardless of how many CPUs we actually boot.
> Or what am I overlooking here?
> 
So now we have two CPU numbers. One is the original processor count we
got from MADT, the other one is nr_cpu_ids we actually use.

To calculate nr_socket correctly, the former is what we need. While what
I concern here is even for the original processors we got from MADT,
can we trust they are always continuous?

I'm not sure for it. But if they are not, then the above calculation is
bogus.

Chao