[PATCH] x86-32: Fix __per_cpu_load relocation

[PATCH] x86-32: Fix __per_cpu_load relocation

[Brian Gerst]

This patch fixes this error:
WARNING: Absolute relocations present
Offset     Info     Type     Sym.Value Sym.Name
c0a4e07d 00e78001   R_386_32 c0ab0000  __per_cpu_load

Now, __per_cpu_load is a section-relative symbol:
c0aa4000 D __per_cpu_load
c0aa4000 A __per_cpu_load_abs

Signed-off-by: Brian Gerst <brgerst@gmail.com>
---
 include/asm-generic/vmlinux.lds.h |    7 ++++---
 1 files changed, 4 insertions(+), 3 deletions(-)

diff --git a/include/asm-generic/vmlinux.lds.h b/include/asm-generic/vmlinux.lds.h
index 53e21f3..f3180a8 100644
--- a/include/asm-generic/vmlinux.lds.h
+++ b/include/asm-generic/vmlinux.lds.h
@@ -451,17 +451,18 @@
  * end offset.
  */
 #define PERCPU_VADDR(vaddr, phdr)					\
-	VMLINUX_SYMBOL(__per_cpu_load) = .;				\
-	.data.percpu vaddr : AT(VMLINUX_SYMBOL(__per_cpu_load)		\
+	VMLINUX_SYMBOL(__per_cpu_load_abs) = .;				\
+	.data.percpu vaddr : AT(VMLINUX_SYMBOL(__per_cpu_load_abs)	\
 				- LOAD_OFFSET) {			\
 		VMLINUX_SYMBOL(__per_cpu_start) = .;			\
+		VMLINUX_SYMBOL(__per_cpu_load) = LOADADDR(.data.percpu) + LOAD_OFFSET;\
 		*(.data.percpu.first)					\
 		*(.data.percpu.page_aligned)				\
 		*(.data.percpu)						\
 		*(.data.percpu.shared_aligned)				\
 		VMLINUX_SYMBOL(__per_cpu_end) = .;			\
 	} phdr								\
-	. = VMLINUX_SYMBOL(__per_cpu_load) + SIZEOF(.data.percpu);
+	. = VMLINUX_SYMBOL(__per_cpu_load_abs) + SIZEOF(.data.percpu);
 
 /**
  * PERCPU - define output section for percpu area, simple version
-- 
1.6.1

Re: [PATCH] x86-32: Fix __per_cpu_load relocation

[Ingo Molnar]

* Brian Gerst <brgerst@gmail.com> wrote:

> This patch fixes this error:
> WARNING: Absolute relocations present
> Offset     Info     Type     Sym.Value Sym.Name
> c0a4e07d 00e78001   R_386_32 c0ab0000  __per_cpu_load
> 
> Now, __per_cpu_load is a section-relative symbol:
> c0aa4000 D __per_cpu_load
> c0aa4000 A __per_cpu_load_abs
> 
> Signed-off-by: Brian Gerst <brgerst@gmail.com>

Applied to tip/core/percpu, thanks Brian!

	Ingo

Re: [PATCH] x86-32: Fix __per_cpu_load relocation

[Tejun Heo]

Ingo Molnar wrote:
> * Brian Gerst <brgerst@gmail.com> wrote:
> 
>> This patch fixes this error:
>> WARNING: Absolute relocations present
>> Offset     Info     Type     Sym.Value Sym.Name
>> c0a4e07d 00e78001   R_386_32 c0ab0000  __per_cpu_load
>>
>> Now, __per_cpu_load is a section-relative symbol:
>> c0aa4000 D __per_cpu_load
>> c0aa4000 A __per_cpu_load_abs
>>
>> Signed-off-by: Brian Gerst <brgerst@gmail.com>
> 
> Applied to tip/core/percpu, thanks Brian!

Heh.. Thanks.  Sorry about lack of response.  It's lunar new year's
day here and I'm off till tomorrow.  I'll start reviewing and
integrating posted patches from tomorrow.

-- 
tejun

Re: [PATCH] x86-32: Fix __per_cpu_load relocation

[Tejun Heo]

(cc'ing James Bottomley.)

Tejun Heo wrote:
> Ingo Molnar wrote:
>> * Brian Gerst <brgerst@gmail.com> wrote:
>>
>>> This patch fixes this error:
>>> WARNING: Absolute relocations present
>>> Offset     Info     Type     Sym.Value Sym.Name
>>> c0a4e07d 00e78001   R_386_32 c0ab0000  __per_cpu_load
>>>
>>> Now, __per_cpu_load is a section-relative symbol:
>>> c0aa4000 D __per_cpu_load
>>> c0aa4000 A __per_cpu_load_abs
>>>
>>> Signed-off-by: Brian Gerst <brgerst@gmail.com>
>> Applied to tip/core/percpu, thanks Brian!
> 
> Heh.. Thanks.  Sorry about lack of response.  It's lunar new year's
> day here and I'm off till tomorrow.  I'll start reviewing and
> integrating posted patches from tomorrow.

Well, I just had time to do it.  All the patches look fine to me.
Very nice cleanup.  The git tree is at the following URL.

  http://git.kernel.org/?p=linux/kernel/git/tj/misc.git;a=shortlog;h=tj-percpu
  git://git.kernel.org/pub/scm/linux/kernel/git/tj/misc.git tj-percpu

The head commit is 2697fbd5faf19c84c17441b1752bdcbdcfd1248c.  James,
this patchset converts voyager to share generic x86 percpu code.  Can
you please review whether the change looks good for voyager?

Thanks.

-- 
tejun

Re: [PATCH] x86-32: Fix __per_cpu_load relocation

[James Bottomley]

On Tue, 2009-01-27 at 13:03 +0900, Tejun Heo wrote:
> (cc'ing James Bottomley.)
> 
> Tejun Heo wrote:
> > Ingo Molnar wrote:
> >> * Brian Gerst <brgerst@gmail.com> wrote:
> >>
> >>> This patch fixes this error:
> >>> WARNING: Absolute relocations present
> >>> Offset     Info     Type     Sym.Value Sym.Name
> >>> c0a4e07d 00e78001   R_386_32 c0ab0000  __per_cpu_load
> >>>
> >>> Now, __per_cpu_load is a section-relative symbol:
> >>> c0aa4000 D __per_cpu_load
> >>> c0aa4000 A __per_cpu_load_abs
> >>>
> >>> Signed-off-by: Brian Gerst <brgerst@gmail.com>
> >> Applied to tip/core/percpu, thanks Brian!
> > 
> > Heh.. Thanks.  Sorry about lack of response.  It's lunar new year's
> > day here and I'm off till tomorrow.  I'll start reviewing and
> > integrating posted patches from tomorrow.
> 
> Well, I just had time to do it.  All the patches look fine to me.
> Very nice cleanup.  The git tree is at the following URL.
> 
>   http://git.kernel.org/?p=linux/kernel/git/tj/misc.git;a=shortlog;h=tj-percpu
>   git://git.kernel.org/pub/scm/linux/kernel/git/tj/misc.git tj-percpu
> 
> The head commit is 2697fbd5faf19c84c17441b1752bdcbdcfd1248c.  James,
> this patchset converts voyager to share generic x86 percpu code.  Can
> you please review whether the change looks good for voyager?

Erm ... it's a bit difficult to tell from the tree what is specific to
voyager and what isn't.

I ran across a simple build failure:

  LD      .tmp_vmlinux1
arch/x86/kernel/built-in.o: In function `setup_per_cpu_areas':
/home/jejb/git/BUILD-voyager/arch/x86/kernel/setup_percpu.c:128: undefined reference to `x86_cpu_to_apicid_early_ptr'
/home/jejb/git/BUILD-voyager/arch/x86/kernel/setup_percpu.c:129: undefined reference to `x86_bios_cpu_apicid_early_ptr'
make: *** [.tmp_vmlinux1] Error 1

which is easily fixed below.

Not sure about the GDT changes, but will boot test them tomorrow.

James

---

diff --git a/arch/x86/kernel/setup_percpu.c b/arch/x86/kernel/setup_percpu.c
index 4caa78d..42b3b35 100644
--- a/arch/x86/kernel/setup_percpu.c
+++ b/arch/x86/kernel/setup_percpu.c
@@ -125,8 +125,10 @@ void __init setup_per_cpu_areas(void)
 	}
 
 	/* indicate the early static arrays will soon be gone */
+#ifdef X86_LOCAL_APIC
 	early_per_cpu_ptr(x86_cpu_to_apicid) = NULL;
 	early_per_cpu_ptr(x86_bios_cpu_apicid) = NULL;
+#endif
 #if defined(CONFIG_X86_64) && defined(CONFIG_NUMA)
 	early_per_cpu_ptr(x86_cpu_to_node_map) = NULL;
 #endif

[PATCH 1/2 #tj-percpu] x86: fix build breakage on voyage

[Tejun Heo]

From: James Bottomley <James.Bottomley@HansenPartnership.com>

Impact: build fix

x86_cpu_to_apicid and x86_bios_cpu_apicid aren't defined for voyage.
Earlier patch forgot to conditionalize early percpu clearing.  Fix it.

Signed-off-by: James Bottomley <James.Bottomley@HansenPartnership.com>
Signed-off-by: Tejun Heo <tj@kernel.org>
---
These two patches have been merged into #tj-percpu.  Thanks.

 arch/x86/kernel/setup_percpu.c |    2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)

diff --git a/arch/x86/kernel/setup_percpu.c b/arch/x86/kernel/setup_percpu.c
index 4caa78d..c7458ea 100644
--- a/arch/x86/kernel/setup_percpu.c
+++ b/arch/x86/kernel/setup_percpu.c
@@ -125,8 +125,10 @@ void __init setup_per_cpu_areas(void)
 	}
 
 	/* indicate the early static arrays will soon be gone */
+#ifdef CONFIG_X86_LOCAL_APIC
 	early_per_cpu_ptr(x86_cpu_to_apicid) = NULL;
 	early_per_cpu_ptr(x86_bios_cpu_apicid) = NULL;
+#endif
 #if defined(CONFIG_X86_64) && defined(CONFIG_NUMA)
 	early_per_cpu_ptr(x86_cpu_to_node_map) = NULL;
 #endif
-- 
1.6.0.2

[PATCH 2/2 #tj-percpu] x86: clean up indentation in setup_per_cpu_areas()

[Tejun Heo]

Impact: cosmetic cleanup

Signed-off-by: Tejun Heo <tj@kernel.org>
---
 arch/x86/kernel/setup_percpu.c |   17 ++++++++++-------
 1 files changed, 10 insertions(+), 7 deletions(-)

diff --git a/arch/x86/kernel/setup_percpu.c b/arch/x86/kernel/setup_percpu.c
index c7458ea..0d1e7ac 100644
--- a/arch/x86/kernel/setup_percpu.c
+++ b/arch/x86/kernel/setup_percpu.c
@@ -96,22 +96,25 @@ void __init setup_per_cpu_areas(void)
 		per_cpu(cpu_number, cpu) = cpu;
 		setup_percpu_segment(cpu);
 		/*
-		 * Copy data used in early init routines from the initial arrays to the
-		 * per cpu data areas.  These arrays then become expendable and the
-		 * *_early_ptr's are zeroed indicating that the static arrays are gone.
+		 * Copy data used in early init routines from the
+		 * initial arrays to the per cpu data areas.  These
+		 * arrays then become expendable and the *_early_ptr's
+		 * are zeroed indicating that the static arrays are
+		 * gone.
 		 */
 #ifdef CONFIG_X86_LOCAL_APIC
 		per_cpu(x86_cpu_to_apicid, cpu) =
-				early_per_cpu_map(x86_cpu_to_apicid, cpu);
+			early_per_cpu_map(x86_cpu_to_apicid, cpu);
 		per_cpu(x86_bios_cpu_apicid, cpu) =
-				early_per_cpu_map(x86_bios_cpu_apicid, cpu);
+			early_per_cpu_map(x86_bios_cpu_apicid, cpu);
 #endif
 #ifdef CONFIG_X86_64
 		per_cpu(irq_stack_ptr, cpu) =
-			per_cpu(irq_stack_union.irq_stack, cpu) + IRQ_STACK_SIZE - 64;
+			per_cpu(irq_stack_union.irq_stack, cpu) +
+			IRQ_STACK_SIZE - 64;
 #ifdef CONFIG_NUMA
 		per_cpu(x86_cpu_to_node_map, cpu) =
-				early_per_cpu_map(x86_cpu_to_node_map, cpu);
+			early_per_cpu_map(x86_cpu_to_node_map, cpu);
 #endif
 #endif
 		/*
-- 
1.6.0.2

Re: [PATCH 1/2 #tj-percpu] x86: fix build breakage on voyage

[Ingo Molnar]

* Tejun Heo <htejun@gmail.com> wrote:

> From: James Bottomley <James.Bottomley@HansenPartnership.com>
> 
> Impact: build fix
> 
> x86_cpu_to_apicid and x86_bios_cpu_apicid aren't defined for voyage. 
> Earlier patch forgot to conditionalize early percpu clearing.  Fix it.

> +#ifdef CONFIG_X86_LOCAL_APIC
>  	early_per_cpu_ptr(x86_cpu_to_apicid) = NULL;
>  	early_per_cpu_ptr(x86_bios_cpu_apicid) = NULL;
> +#endif

That patch is not acceptable - it is ugly and it adds another set of
#ifdefs to an already complex piece of code.

As i explained it to James in recent threads, the clean and acceptable 
solution to this class of problems is to switch Voyager away from that 
fragile subarch code to proper generic x86 code. (just like we did it for 
other subarchitectures)

There is nothing in Voyager that justifies special treatment in the area 
of x86 percpu code.

This is one of the mails that explains the principles:

  http://lkml.indiana.edu/hypermail/linux/kernel/0901.2/00954.html

Or - if there's no time/interest in doing that, we can mark Voyager as 
CONFIG_BROKEN.

	Ingo

Re: [PATCH 1/2 #tj-percpu] x86: fix build breakage on voyage

[Tejun Heo]

Hello, Ingo.

Ingo Molnar wrote:
>> +#ifdef CONFIG_X86_LOCAL_APIC
>>  	early_per_cpu_ptr(x86_cpu_to_apicid) = NULL;
>>  	early_per_cpu_ptr(x86_bios_cpu_apicid) = NULL;
>> +#endif
> 
> That patch is not acceptable - it is ugly and it adds another set of
> #ifdefs to an already complex piece of code.

Well, although the patch itself does add #ifdef, if you look over the
whole series, voyager is now a much more conforming citizen in the x86
world.  There are several solutions to this particular one.

1. Just let apic stuff defined and not use it in voyager if the ifdef
   is disturbing.  IIUC, apic isn't used in voyager at all, right?

2. Clean up early percpu stuff so that it each early percpu variable
   doesn't need to be explicitly copied and cleared, which is the
   actual problem here.

3. But, then again, the current interim and ugly way of doing it isn't
   too bad considering the small number of early per cpu users.

To me the current form doesn't look too bad but if it's too ugly,
maybe doing #2 is not such a bad idea such that early percpu can be
transferred to percpu in more systematic way.  It still feels a bit
like overdoing it tho.

What do you think?

Thanks.

-- 
tejun

Re: [PATCH 1/2 #tj-percpu] x86: fix build breakage on voyage

[Brian Gerst]

On Tue, Jan 27, 2009 at 6:47 AM, Tejun Heo <htejun@gmail.com> wrote:
> Hello, Ingo.
>
> Ingo Molnar wrote:
>>> +#ifdef CONFIG_X86_LOCAL_APIC
>>>      early_per_cpu_ptr(x86_cpu_to_apicid) = NULL;
>>>      early_per_cpu_ptr(x86_bios_cpu_apicid) = NULL;
>>> +#endif
>>
>> That patch is not acceptable - it is ugly and it adds another set of
>> #ifdefs to an already complex piece of code.
>
> Well, although the patch itself does add #ifdef, if you look over the
> whole series, voyager is now a much more conforming citizen in the x86
> world.  There are several solutions to this particular one.
>
> 1. Just let apic stuff defined and not use it in voyager if the ifdef
>   is disturbing.  IIUC, apic isn't used in voyager at all, right?
>
> 2. Clean up early percpu stuff so that it each early percpu variable
>   doesn't need to be explicitly copied and cleared, which is the
>   actual problem here.
>
> 3. But, then again, the current interim and ugly way of doing it isn't
>   too bad considering the small number of early per cpu users.
>
> To me the current form doesn't look too bad but if it's too ugly,
> maybe doing #2 is not such a bad idea such that early percpu can be
> transferred to percpu in more systematic way.  It still feels a bit
> like overdoing it tho.
>
> What do you think?

I thought about ways to make the early percpu code more general, but
with only three current users, any solution seemed to be overkill.
What I can do is eliminate the pointers and use a single flag to mark
the early maps as dead.

--
Brian Gerst

Re: [PATCH 1/2 #tj-percpu] x86: fix build breakage on voyage

[Ingo Molnar]

* Tejun Heo <htejun@gmail.com> wrote:

> Hello, Ingo.
> 
> Ingo Molnar wrote:
> >> +#ifdef CONFIG_X86_LOCAL_APIC
> >>  	early_per_cpu_ptr(x86_cpu_to_apicid) = NULL;
> >>  	early_per_cpu_ptr(x86_bios_cpu_apicid) = NULL;
> >> +#endif
> > 
> > That patch is not acceptable - it is ugly and it adds another set of
> > #ifdefs to an already complex piece of code.
> 
> Well, although the patch itself does add #ifdef, if you look over the 
> whole series, voyager is now a much more conforming citizen in the x86 
> world.  There are several solutions to this particular one.
> 
> 1. Just let apic stuff defined and not use it in voyager if the ifdef
>    is disturbing.  IIUC, apic isn't used in voyager at all, right?
> 
> 2. Clean up early percpu stuff so that it each early percpu variable
>    doesn't need to be explicitly copied and cleared, which is the
>    actual problem here.
> 
> 3. But, then again, the current interim and ugly way of doing it isn't
>    too bad considering the small number of early per cpu users.
> 
> To me the current form doesn't look too bad but if it's too ugly, maybe 
> doing #2 is not such a bad idea such that early percpu can be 
> transferred to percpu in more systematic way.  It still feels a bit like 
> overdoing it tho.
> 
> What do you think?

This issue might be minor, but it's the death of a thousand cuts. It 
should switch to the generic x86 code, use smp_ops to wrap/express its own 
SMP weirdnesses [and extend it where needed - because _that_ is a step 
forward for the whole code - fixing build bugs isnt] and then such 
problems simply wont occur.

	Ingo

Re: [PATCH 1/2 #tj-percpu] x86: fix build breakage on voyage

[Ingo Molnar]

* Ingo Molnar <mingo@elte.hu> wrote:

> 
> * Tejun Heo <htejun@gmail.com> wrote:
> 
> > Hello, Ingo.
> > 
> > Ingo Molnar wrote:
> > >> +#ifdef CONFIG_X86_LOCAL_APIC
> > >>  	early_per_cpu_ptr(x86_cpu_to_apicid) = NULL;
> > >>  	early_per_cpu_ptr(x86_bios_cpu_apicid) = NULL;
> > >> +#endif
> > > 
> > > That patch is not acceptable - it is ugly and it adds another set of
> > > #ifdefs to an already complex piece of code.
> > 
> > Well, although the patch itself does add #ifdef, if you look over the 
> > whole series, voyager is now a much more conforming citizen in the x86 
> > world.  There are several solutions to this particular one.
> > 
> > 1. Just let apic stuff defined and not use it in voyager if the ifdef
> >    is disturbing.  IIUC, apic isn't used in voyager at all, right?
> > 
> > 2. Clean up early percpu stuff so that it each early percpu variable
> >    doesn't need to be explicitly copied and cleared, which is the
> >    actual problem here.
> > 
> > 3. But, then again, the current interim and ugly way of doing it isn't
> >    too bad considering the small number of early per cpu users.
> > 
> > To me the current form doesn't look too bad but if it's too ugly, maybe 
> > doing #2 is not such a bad idea such that early percpu can be 
> > transferred to percpu in more systematic way.  It still feels a bit like 
> > overdoing it tho.
> > 
> > What do you think?
> 
> This issue might be minor, but it's the death of a thousand cuts. It 
> should switch to the generic x86 code, use smp_ops to wrap/express its 
> own SMP weirdnesses [and extend it where needed - because _that_ is a 
> step forward for the whole code - fixing build bugs isnt] and then such 
> problems simply wont occur.

btw., i have pulled it to not hold up the flow, but this is the absolutely 
last warning for Voyager to get fixed. There's perhaps two Voyager systems 
left in existence running mainline kernels (both owned by James) while the 
code you are hacking on code that affects tens of millions of Linux boxes.

Every minute you waste on thinking about Voyager build issues has a 
multiplier effect of 1:1000,000. Voyager needs to be isolated into 
arch/x86/kernel/voyager_quirks.c and that's it.

	Ingo

Re: [PATCH 1/2 #tj-percpu] x86: fix build breakage on voyage

[James Bottomley]

On Tue, 2009-01-27 at 20:47 +0900, Tejun Heo wrote:
> Hello, Ingo.
> 
> Ingo Molnar wrote:
> >> +#ifdef CONFIG_X86_LOCAL_APIC
> >>  	early_per_cpu_ptr(x86_cpu_to_apicid) = NULL;
> >>  	early_per_cpu_ptr(x86_bios_cpu_apicid) = NULL;
> >> +#endif
> > 
> > That patch is not acceptable - it is ugly and it adds another set of
> > #ifdefs to an already complex piece of code.
> 
> Well, although the patch itself does add #ifdef, if you look over the
> whole series, voyager is now a much more conforming citizen in the x86
> world.  There are several solutions to this particular one.
> 
> 1. Just let apic stuff defined and not use it in voyager if the ifdef
>    is disturbing.  IIUC, apic isn't used in voyager at all, right?
> 
> 2. Clean up early percpu stuff so that it each early percpu variable
>    doesn't need to be explicitly copied and cleared, which is the
>    actual problem here.

Right ... there's a variant of this where the copying and clearing is
done within the actual file that defines the variable (i.e. a hook
approach).

> 3. But, then again, the current interim and ugly way of doing it isn't
>    too bad considering the small number of early per cpu users.
> 
> To me the current form doesn't look too bad but if it's too ugly,
> maybe doing #2 is not such a bad idea such that early percpu can be
> transferred to percpu in more systematic way.  It still feels a bit
> like overdoing it tho.
> 
> What do you think?

What's the actual goal of setup_percpu.c?  To have a single location for
all the per_cpu initialisation?  If so then it's hard to get away from
all the ifdefs.  If there are alternative ways of arranging the file
then I think splitting some of the conditional pieces into other files
would work.

James

Re: [PATCH 1/2 #tj-percpu] x86: fix build breakage on voyage

[James Bottomley]

On Tue, 2009-01-27 at 12:37 +0100, Ingo Molnar wrote:
> * Tejun Heo <htejun@gmail.com> wrote:
> 
> > From: James Bottomley <James.Bottomley@HansenPartnership.com>
> > 
> > Impact: build fix
> > 
> > x86_cpu_to_apicid and x86_bios_cpu_apicid aren't defined for voyage. 
> > Earlier patch forgot to conditionalize early percpu clearing.  Fix it.
> 
> > +#ifdef CONFIG_X86_LOCAL_APIC
> >  	early_per_cpu_ptr(x86_cpu_to_apicid) = NULL;
> >  	early_per_cpu_ptr(x86_bios_cpu_apicid) = NULL;
> > +#endif
> 
> That patch is not acceptable - it is ugly and it adds another set of
> #ifdefs to an already complex piece of code.
> 
> As i explained it to James in recent threads, the clean and acceptable 
> solution to this class of problems is to switch Voyager away from that 
> fragile subarch code to proper generic x86 code. (just like we did it for 
> other subarchitectures)
> 
> There is nothing in Voyager that justifies special treatment in the area 
> of x86 percpu code.
> 
> This is one of the mails that explains the principles:
> 
>   http://lkml.indiana.edu/hypermail/linux/kernel/0901.2/00954.html
> 
> Or - if there's no time/interest in doing that, we can mark Voyager as 
> CONFIG_BROKEN.

Have you quite finished?

The problem isn't actually voyager specific ... the compile breaks if
you build UP with no local APIC ... for the same reason (you have apic
dependent variables in generic code).   Whether voyager is converted to
your quirk infrastructure is irrelevant ... doubly so since the failure
can be produced with UP PC x86 which is already converted.

That file is already a forest of #ifdefs:

jejb@hobholes> grep \\#if setup_percpu.c
#ifdef CONFIG_DEBUG_PER_CPU_MAPS
#ifdef CONFIG_X86_64
#ifdef CONFIG_X86_32
#ifndef CONFIG_NEED_MULTIPLE_NODES
#ifdef CONFIG_X86_LOCAL_APIC
#ifdef CONFIG_X86_64
#ifdef CONFIG_NUMA
#if defined(CONFIG_X86_64) && defined(CONFIG_NUMA)

it can't but be otherwise because it's a monolithic initialisation of a
set of variables defined in conditionally compiled files.

James

Re: [PATCH 1/2 #tj-percpu] x86: fix build breakage on voyage

[Ingo Molnar]

* James Bottomley <James.Bottomley@HansenPartnership.com> wrote:

> On Tue, 2009-01-27 at 12:37 +0100, Ingo Molnar wrote:
> > * Tejun Heo <htejun@gmail.com> wrote:
> > 
> > > From: James Bottomley <James.Bottomley@HansenPartnership.com>
> > > 
> > > Impact: build fix
> > > 
> > > x86_cpu_to_apicid and x86_bios_cpu_apicid aren't defined for voyage. 
> > > Earlier patch forgot to conditionalize early percpu clearing.  Fix it.
> > 
> > > +#ifdef CONFIG_X86_LOCAL_APIC
> > >  	early_per_cpu_ptr(x86_cpu_to_apicid) = NULL;
> > >  	early_per_cpu_ptr(x86_bios_cpu_apicid) = NULL;
> > > +#endif
> > 
> > That patch is not acceptable - it is ugly and it adds another set of
> > #ifdefs to an already complex piece of code.
> > 
> > As i explained it to James in recent threads, the clean and acceptable 
> > solution to this class of problems is to switch Voyager away from that 
> > fragile subarch code to proper generic x86 code. (just like we did it for 
> > other subarchitectures)
> > 
> > There is nothing in Voyager that justifies special treatment in the area 
> > of x86 percpu code.
> > 
> > This is one of the mails that explains the principles:
> > 
> >   http://lkml.indiana.edu/hypermail/linux/kernel/0901.2/00954.html
> > 
> > Or - if there's no time/interest in doing that, we can mark Voyager as 
> > CONFIG_BROKEN.
> 
> Have you quite finished?

What is that supposed to mean?

	Ingo

Re: [PATCH 1/2 #tj-percpu] x86: fix build breakage on voyage

[James Bottomley]

On Tue, 2009-01-27 at 16:50 +0100, Ingo Molnar wrote:
> * James Bottomley <James.Bottomley@HansenPartnership.com> wrote:
> 
> > On Tue, 2009-01-27 at 12:37 +0100, Ingo Molnar wrote:
> > > * Tejun Heo <htejun@gmail.com> wrote:
> > > 
> > > > From: James Bottomley <James.Bottomley@HansenPartnership.com>
> > > > 
> > > > Impact: build fix
> > > > 
> > > > x86_cpu_to_apicid and x86_bios_cpu_apicid aren't defined for voyage. 
> > > > Earlier patch forgot to conditionalize early percpu clearing.  Fix it.
> > > 
> > > > +#ifdef CONFIG_X86_LOCAL_APIC
> > > >  	early_per_cpu_ptr(x86_cpu_to_apicid) = NULL;
> > > >  	early_per_cpu_ptr(x86_bios_cpu_apicid) = NULL;
> > > > +#endif
> > > 
> > > That patch is not acceptable - it is ugly and it adds another set of
> > > #ifdefs to an already complex piece of code.
> > > 
> > > As i explained it to James in recent threads, the clean and acceptable 
> > > solution to this class of problems is to switch Voyager away from that 
> > > fragile subarch code to proper generic x86 code. (just like we did it for 
> > > other subarchitectures)
> > > 
> > > There is nothing in Voyager that justifies special treatment in the area 
> > > of x86 percpu code.
> > > 
> > > This is one of the mails that explains the principles:
> > > 
> > >   http://lkml.indiana.edu/hypermail/linux/kernel/0901.2/00954.html
> > > 
> > > Or - if there's no time/interest in doing that, we can mark Voyager as 
> > > CONFIG_BROKEN.
> > 
> > Have you quite finished?
> 
> What is that supposed to mean?

It's a conventional response implying your rant wasn't factually
connected to the actual problem at hand.  The justification was actually
in the text you cut ... but boils down to you can reproduce it in a non
voyager configuration, so it's not a voyager specific problem.

The actual problem, as I see it, is how (or whether) to get rid of the
nine #if/#ifdefs that clutter setup_percpu.c ... none of which is
voyager specific.

James

Re: [PATCH 1/2 #tj-percpu] x86: fix build breakage on voyage

[Brian Gerst]

On Tue, Jan 27, 2009 at 11:04 AM, James Bottomley
<James.Bottomley@hansenpartnership.com> wrote:
> On Tue, 2009-01-27 at 16:50 +0100, Ingo Molnar wrote:
>> * James Bottomley <James.Bottomley@HansenPartnership.com> wrote:
>>
>> > On Tue, 2009-01-27 at 12:37 +0100, Ingo Molnar wrote:
>> > > * Tejun Heo <htejun@gmail.com> wrote:
>> > >
>> > > > From: James Bottomley <James.Bottomley@HansenPartnership.com>
>> > > >
>> > > > Impact: build fix
>> > > >
>> > > > x86_cpu_to_apicid and x86_bios_cpu_apicid aren't defined for voyage.
>> > > > Earlier patch forgot to conditionalize early percpu clearing.  Fix it.
>> > >
>> > > > +#ifdef CONFIG_X86_LOCAL_APIC
>> > > >         early_per_cpu_ptr(x86_cpu_to_apicid) = NULL;
>> > > >         early_per_cpu_ptr(x86_bios_cpu_apicid) = NULL;
>> > > > +#endif
>> > >
>> > > That patch is not acceptable - it is ugly and it adds another set of
>> > > #ifdefs to an already complex piece of code.
>> > >
>> > > As i explained it to James in recent threads, the clean and acceptable
>> > > solution to this class of problems is to switch Voyager away from that
>> > > fragile subarch code to proper generic x86 code. (just like we did it for
>> > > other subarchitectures)
>> > >
>> > > There is nothing in Voyager that justifies special treatment in the area
>> > > of x86 percpu code.
>> > >
>> > > This is one of the mails that explains the principles:
>> > >
>> > >   http://lkml.indiana.edu/hypermail/linux/kernel/0901.2/00954.html
>> > >
>> > > Or - if there's no time/interest in doing that, we can mark Voyager as
>> > > CONFIG_BROKEN.
>> >
>> > Have you quite finished?
>>
>> What is that supposed to mean?
>
> It's a conventional response implying your rant wasn't factually
> connected to the actual problem at hand.  The justification was actually
> in the text you cut ... but boils down to you can reproduce it in a non
> voyager configuration, so it's not a voyager specific problem.
>
> The actual problem, as I see it, is how (or whether) to get rid of the
> nine #if/#ifdefs that clutter setup_percpu.c ... none of which is
> voyager specific.

To be fair, setup_percpu.c isn't built on UP.  But we're splitting
hairs over just 3 conditional variables.  I'm open to ideas, but I'm
quite certain that any general solution will have more overhead than
the current code.  I am looking at a patch to remove the early percpu
pointers, so the second set of ifdefs would go away.

--
Brian Gerst

Re: [PATCH 1/2 #tj-percpu] x86: fix build breakage on voyage

[James Bottomley]

On Tue, 2009-01-27 at 11:25 -0500, Brian Gerst wrote:
> To be fair, setup_percpu.c isn't built on UP.  But we're splitting
> hairs over just 3 conditional variables.  I'm open to ideas, but I'm
> quite certain that any general solution will have more overhead than
> the current code.  I am looking at a patch to remove the early percpu
> pointers, so the second set of ifdefs would go away.

Well, how much do you need rid of?  All the local apic stuff can move
into apic.c as something like x86_percpu_apic_setup(cpu) for the in loop
stuff and x86_percpu_apic_global() for the rest ... these then become
weak empty functions in setup_percpu.c.  That elimiates the alleged
voyager problem.

The other cases are nastier:  there's a numa case, an x86_84 case and a
both numa and x86_64 case.  We only have files for the x86_64 case.  At
a stretch the numa cases could move into mm/numa_${BITS}.c

This isn't really hugely critical path so this type of approach would
work ... I can cook up a patch if it's acceptable? 

James

Re: [PATCH] x86-32: Fix __per_cpu_load relocation

[Ingo Molnar]

* Tejun Heo <htejun@gmail.com> wrote:

> (cc'ing James Bottomley.)
> 
> Tejun Heo wrote:
> > Ingo Molnar wrote:
> >> * Brian Gerst <brgerst@gmail.com> wrote:
> >>
> >>> This patch fixes this error:
> >>> WARNING: Absolute relocations present
> >>> Offset     Info     Type     Sym.Value Sym.Name
> >>> c0a4e07d 00e78001   R_386_32 c0ab0000  __per_cpu_load
> >>>
> >>> Now, __per_cpu_load is a section-relative symbol:
> >>> c0aa4000 D __per_cpu_load
> >>> c0aa4000 A __per_cpu_load_abs
> >>>
> >>> Signed-off-by: Brian Gerst <brgerst@gmail.com>
> >> Applied to tip/core/percpu, thanks Brian!
> > 
> > Heh.. Thanks.  Sorry about lack of response.  It's lunar new year's
> > day here and I'm off till tomorrow.  I'll start reviewing and
> > integrating posted patches from tomorrow.
> 
> Well, I just had time to do it.  All the patches look fine to me.
> Very nice cleanup.  The git tree is at the following URL.
> 
>   http://git.kernel.org/?p=linux/kernel/git/tj/misc.git;a=shortlog;h=tj-percpu
>   git://git.kernel.org/pub/scm/linux/kernel/git/tj/misc.git tj-percpu

Pulled into tip/core/percpu, thanks Tejun!

	Ingo