[2.6.33-rcX][regression] commit e8c93f breaks sn2 build

[2.6.33-rcX][regression] commit e8c93f breaks sn2 build

[Hedi Berriche]

[[ For a yet to be determined reason I seem to be unable to post to this list using
my sgi.com address, sending this email from an alternative address, apologies if/when
the problem gets fixed we end with duplicate messages ]]

Tony,

Here's the chronologically sorted commits that led to the problem at hand:

    - commit 204fba4aa303ea4a7bb726a539bf4a5b9e3203d0
       Author: Tejun Heo <tj@kernel.org>
       Date:   Wed Jun 24 15:13:45 2009 +0900

       percpu: cleanup percpu array definitions

    broke the sn2 build.

    - commit b94b08081fcecf83fa690d6c5664f6316fe72208
        Author: Randy Dunlap <randy.dunlap@oracle.com>
        Date:   Wed Oct 14 15:10:03 2009 -0700

        [IA64] fix percpu warnings

        Fix percpu types warning in ia64/sn:

        arch/ia64/sn/kernel/setup.c:74: error: conflicting types for '__pcpu_scope___sn_cnodeid_to_nasid'
        arch/ia64/include/asm/sn/arch.h:74: error: previous declaration of '__pcpu_scope___sn_cnodeid_to_nasid' was here

   fixed the build problem but because of genksysms not being able to properly
    parse the per-cpu array declaration, we ended up with the following problem:

           WARNING: "per_cpu____sn_cnodeid_to_nasid" [drivers/misc/sgi-xp/xpc.ko] has no CRC!

    - commit e8c93fc7b7221b6ac7e7ddbd0e21e205bf9e801a
      Author: Tony Luck <tony.luck@intel.com>
      Date:   Mon Nov 2 09:23:08 2009 -0800

      Revert "[IA64] fix percpu warnings"

      This reverts commit b94b08081fcecf83fa690d6c5664f6316fe72208.

    reverted b94b08 but in doing do got us back to a broken build state.

Until genksyms is fixed/improved should we just revert the bit of commit 204fba4
that pertains to arch/ia64/sn/kernel/setup.c?

Cheers,
Hedi.

P.S. http://marc.info/?l=linux-ia64&m\x125555886516823&w=2 for the detailed history...

diff --git a/arch/ia64/sn/kernel/setup.c b/arch/ia64/sn/kernel/setup.c
index ece1bf9..e456f06 100644
--- a/arch/ia64/sn/kernel/setup.c
+++ b/arch/ia64/sn/kernel/setup.c
@@ -71,7 +71,7 @@ EXPORT_SYMBOL(sn_rtc_cycles_per_second);
 DEFINE_PER_CPU(struct sn_hub_info_s, __sn_hub_info);
 EXPORT_PER_CPU_SYMBOL(__sn_hub_info);
 
-DEFINE_PER_CPU(short [MAX_COMPACT_NODES], __sn_cnodeid_to_nasid);
+DEFINE_PER_CPU(short, __sn_cnodeid_to_nasid[MAX_COMPACT_NODES]);
 EXPORT_PER_CPU_SYMBOL(__sn_cnodeid_to_nasid);
 
 DEFINE_PER_CPU(struct nodepda_s *, __sn_nodepda);

RE: [2.6.33-rcX][regression] commit e8c93f breaks sn2 build

[Luck, Tony]

-DEFINE_PER_CPU(short [MAX_COMPACT_NODES], __sn_cnodeid_to_nasid);
+DEFINE_PER_CPU(short, __sn_cnodeid_to_nasid[MAX_COMPACT_NODES]);


This form was claimed to be syntactically less desirable in the
e-mail threads you quoted ... but it wasn't ever specified what
the problems with it were.

Can you confirm that an sn2 kernel built with this change
boots, and does the right things with this data structure?

If so, send me the patch with a "Signed-off-by" on it and
I'll apply it.

-Tony

Re: [2.6.33-rcX][regression] commit e8c93f breaks sn2 build

[Hedi Berriche]

On Tue, Feb 23, 2010 at 23:16 Luck, Tony wrote:
| -DEFINE_PER_CPU(short [MAX_COMPACT_NODES], __sn_cnodeid_to_nasid);
| +DEFINE_PER_CPU(short, __sn_cnodeid_to_nasid[MAX_COMPACT_NODES]);
| 
| This form was claimed to be syntactically less desirable in the
| e-mail threads you quoted ... but it wasn't ever specified what
| the problems with it were.

Yep, saw that, but then again that's how the code has been for quite sometime
until the percup cleanup came along

    commit 204fba4aa303ea4a7bb726a539bf4a5b9e3203d0
    Author: Tejun Heo <tj@kernel.org>
    Date:   Wed Jun 24 15:13:45 2009 +0900

    percpu: cleanup percpu array definitions

IOW, it may syntactically less desirable but it's not completely b0rked either.

That said if there's a better way than reverting the change, to get this fixed,
either by teaching genksyms how to grok the percpu arrays, or any other
acceptable approach, I am all for it.

| Can you confirm that an sn2 kernel built with this change boots, and does the
| right things with this data structure?

AFAICT that's indeed the case.

| If so, send me the patch with a "Signed-off-by" on it and I'll apply it.

Here goes:

Revert the change made to arch/ia64/sn/kernel/setup.c by commit
204fba4aa303ea4a7bb726a539bf4a5b9e3203d0 as it breaks the build.

Fixing the build the b94b08081fcecf83fa690d6c5664f6316fe72208 way
breaks xpc because genksyms then fails to generate an CRC for
per_cpu____sn_cnodeid_to_nasid.

Signed-off-by: Hedi Berriche <hedi@sgi.com>

diff --git a/arch/ia64/sn/kernel/setup.c b/arch/ia64/sn/kernel/setup.c
index ece1bf9..e456f06 100644
--- a/arch/ia64/sn/kernel/setup.c
+++ b/arch/ia64/sn/kernel/setup.c
@@ -71,7 +71,7 @@ EXPORT_SYMBOL(sn_rtc_cycles_per_second);
 DEFINE_PER_CPU(struct sn_hub_info_s, __sn_hub_info);
 EXPORT_PER_CPU_SYMBOL(__sn_hub_info);

-DEFINE_PER_CPU(short [MAX_COMPACT_NODES], __sn_cnodeid_to_nasid);
+DEFINE_PER_CPU(short, __sn_cnodeid_to_nasid[MAX_COMPACT_NODES]);
 EXPORT_PER_CPU_SYMBOL(__sn_cnodeid_to_nasid);

 DEFINE_PER_CPU(struct nodepda_s *, __sn_nodepda);

-- 
Hedi Berriche
Global Product Support
http://www.sgi.com/support