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 <[email protected]>
---
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
* Brian Gerst <[email protected]> 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 <[email protected]>
Applied to tip/core/percpu, thanks Brian!
Ingo
Ingo Molnar wrote:
> * Brian Gerst <[email protected]> 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 <[email protected]>
>
> 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
(cc'ing James Bottomley.)
Tejun Heo wrote:
> Ingo Molnar wrote:
>> * Brian Gerst <[email protected]> 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 <[email protected]>
>> 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
On Tue, 2009-01-27 at 13:03 +0900, Tejun Heo wrote:
> (cc'ing James Bottomley.)
>
> Tejun Heo wrote:
> > Ingo Molnar wrote:
> >> * Brian Gerst <[email protected]> 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 <[email protected]>
> >> 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
From: James Bottomley <[email protected]>
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 <[email protected]>
Signed-off-by: Tejun Heo <[email protected]>
---
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
Impact: cosmetic cleanup
Signed-off-by: Tejun Heo <[email protected]>
---
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
* Tejun Heo <[email protected]> wrote:
> (cc'ing James Bottomley.)
>
> Tejun Heo wrote:
> > Ingo Molnar wrote:
> >> * Brian Gerst <[email protected]> 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 <[email protected]>
> >> 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
* Tejun Heo <[email protected]> wrote:
> From: James Bottomley <[email protected]>
>
> 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
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
On Tue, Jan 27, 2009 at 6:47 AM, Tejun Heo <[email protected]> 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
* Tejun Heo <[email protected]> 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
* Ingo Molnar <[email protected]> wrote:
>
> * Tejun Heo <[email protected]> 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
On Tue, 2009-01-27 at 12:37 +0100, Ingo Molnar wrote:
> * Tejun Heo <[email protected]> wrote:
>
> > From: James Bottomley <[email protected]>
> >
> > 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
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
* James Bottomley <[email protected]> wrote:
> On Tue, 2009-01-27 at 12:37 +0100, Ingo Molnar wrote:
> > * Tejun Heo <[email protected]> wrote:
> >
> > > From: James Bottomley <[email protected]>
> > >
> > > 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
On Tue, 2009-01-27 at 16:50 +0100, Ingo Molnar wrote:
> * James Bottomley <[email protected]> wrote:
>
> > On Tue, 2009-01-27 at 12:37 +0100, Ingo Molnar wrote:
> > > * Tejun Heo <[email protected]> wrote:
> > >
> > > > From: James Bottomley <[email protected]>
> > > >
> > > > 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
On Tue, Jan 27, 2009 at 11:04 AM, James Bottomley
<[email protected]> wrote:
> On Tue, 2009-01-27 at 16:50 +0100, Ingo Molnar wrote:
>> * James Bottomley <[email protected]> wrote:
>>
>> > On Tue, 2009-01-27 at 12:37 +0100, Ingo Molnar wrote:
>> > > * Tejun Heo <[email protected]> wrote:
>> > >
>> > > > From: James Bottomley <[email protected]>
>> > > >
>> > > > 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
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