On Wed, 20 Jun 2018, Chang S. Bae wrote: > CPU number initialization in vDSO is now a bit cleaned up by > the new helper functions. The helper functions will take > care of combing CPU and node number and reading each from
s/combing/combining/ please > the combined value. > > Suggested-by: Andy Lutomirski <l...@kernel.org> > Signed-off-by: Chang S. Bae <chang.seok....@intel.com> > Cc: H. Peter Anvin <h...@zytor.com> > Cc: Andi Kleen <a...@linux.intel.com> > Cc: Dave Hansen <dave.han...@linux.intel.com> > Cc: Thomas Gleixner <t...@linutronix.de> > Cc: Ingo Molnar <mi...@kernel.org> > Acked-by: Andy Lutomirski <l...@kernel.org> > --- > arch/x86/entry/vdso/vgetcpu.c | 4 ++-- > arch/x86/entry/vdso/vma.c | 16 ++++++++-------- > arch/x86/include/asm/segment.h | 28 ++++++++++++++++++++++++++++ > arch/x86/include/asm/vgtod.h | 2 -- > 4 files changed, 38 insertions(+), 12 deletions(-) > > diff --git a/arch/x86/entry/vdso/vgetcpu.c b/arch/x86/entry/vdso/vgetcpu.c > index 8ec3d1f..3284069 100644 > --- a/arch/x86/entry/vdso/vgetcpu.c > +++ b/arch/x86/entry/vdso/vgetcpu.c > @@ -18,9 +18,9 @@ __vdso_getcpu(unsigned *cpu, unsigned *node, struct > getcpu_cache *unused) > p = __getcpu(); While we are touching this, can we please as a first step change __getcpu() to something else? I've tripped over this several times in the past and confused it with a (nonexisting) variant of get_cpu(). > > if (cpu) > - *cpu = p & VGETCPU_CPU_MASK; > + *cpu = lsl_tscp_to_cpu(p); > > if (node) > - *node = p >> 12; > + *node = lsl_tscp_to_node(p); Are these new helpers going to be used at some other place than this? If not, then there is really no point at all. Then just go and make this: __vdso_getcpu(unsigned *cpu, unsigned *node, struct getcpu_cache *unused) { vdso_read_cpu_and_node(cpu, node); return 0; } > diff --git a/arch/x86/entry/vdso/vma.c b/arch/x86/entry/vdso/vma.c > index 833e229..1fc93da 100644 > --- a/arch/x86/entry/vdso/vma.c > +++ b/arch/x86/entry/vdso/vma.c > @@ -340,19 +340,19 @@ static void vgetcpu_cpu_init(void *arg) > int cpu = smp_processor_id(); > struct desc_struct d = { }; > unsigned long node = 0; > + unsigned long cpu_number = 0; That's hardly a CPU number. It's encoded CPU and node information. > #ifdef CONFIG_NUMA > node = cpu_to_node(cpu); > #endif While at it please get rid of the ifdeffery. If CONFIG_NUMA=n then cpu_to_node(cpu) resolves to (0). > + cpu_number = make_lsl_tscp(cpu, node); So the whole thing can be reduced to: u64 cpudata = vdso_encode_cpu_and_node(cpu, cpu_to_node(cpu)); Or some other sensible function name. Thanks, tglx