>= 2.6.30 broken alpha smp build

>= 2.6.30 broken alpha smp build

[maximilian attems]

CC [M]  arch/alpha/kernel/srm_env.o
In file included from /build/buildd-linux-2.6_2.6.30-1-alpha-vHGTgq/linux-2.6-2.6.30/debian/build/source_alpha_none/include/linux/percpu.h:10,
                 from /build/buildd-linux-2.6_2.6.30-1-alpha-vHGTgq/linux-2.6-2.6.30/debian/build/source_alpha_none/arch/alpha/include/asm/local.h:4,
                 from /build/buildd-linux-2.6_2.6.30-1-alpha-vHGTgq/linux-2.6-2.6.30/debian/build/source_alpha_none/include/linux/module.h:20,
                 from /build/buildd-linux-2.6_2.6.30-1-alpha-vHGTgq/linux-2.6-2.6.30/debian/build/source_alpha_none/arch/alpha/kernel/srm_env.c:33:
/build/buildd-linux-2.6_2.6.30-1-alpha-vHGTgq/linux-2.6-2.6.30/debian/build/source_alpha_none/arch/alpha/include/asm/percpu.h:100:1: error: "PER_CPU_ATTRIBUTES" redefined
/build/buildd-linux-2.6_2.6.30-1-alpha-vHGTgq/linux-2.6-2.6.30/debian/build/source_alpha_none/arch/alpha/include/asm/percpu.h:52:1: error: this is the location of the previous definition
make[6]: *** [arch/alpha/kernel/srm_env.o] Error 1
make[5]: *** [arch/alpha/kernel] Error 2
make[4]: *** [sub-make] Error 2
make[3]: *** [all] Error 2
make[2]: *** [debian/stamps/build_alpha_none_alpha-smp_plain] Error 2
make[1]: *** [build_alpha_none_alpha-smp_real] Error 2

belows patch fixes aboves error but gives a new one on SMP

diff --git a/arch/alpha/include/asm/percpu.h b/arch/alpha/include/asm/percpu.h
index 06c5c7a..622d585 100644
--- a/arch/alpha/include/asm/percpu.h
+++ b/arch/alpha/include/asm/percpu.h
@@ -95,8 +95,8 @@ extern unsigned long __per_cpu_offset[NR_CPUS];
 #define PER_CPU_SHARED_ALIGNED_SECTION ""
 #define PER_CPU_FIRST_SECTION ""
 
-#endif
-
 #define PER_CPU_ATTRIBUTES
 
+#endif
+
 #endif /* __ALPHA_PERCPU_H */

  CC [M]  arch/alpha/kernel/srm_env.o

cc1: warnings being treated as errors
In file included from /home/maks/src/linux-2.6-2.6.30/debian/build/source_alpha_none/include/linux/sched.h:86,
                 from /home/maks/src/linux-2.6-2.6.30/debian/build/source_alpha_none/arch/alpha/include/asm/uaccess.h:5,
                 from /home/maks/src/linux-2.6-2.6.30/debian/build/source_alpha_none/arch/alpha/kernel/srm_env.c:37:
/home/maks/src/linux-2.6-2.6.30/debian/build/source_alpha_none/include/linux/hrtimer.h:310: error: �~@~X__used__�~@~Y attribute ignored
In file included from /home/maks/src/linux-2.6-2.6.30/debian/build/source_alpha_none/arch/alpha/include/asm/uaccess.h:5,
                 from /home/maks/src/linux-2.6-2.6.30/debian/build/source_alpha_none/arch/alpha/kernel/srm_env.c:37:
/home/maks/src/linux-2.6-2.6.30/debian/build/source_alpha_none/include/linux/sched.h:134: error: �~@~X__used__�~@~Y attribute ignored
make[6]: *** [arch/alpha/kernel/srm_env.o] Error 1


-- 
maks

Re: >= 2.6.30 broken alpha smp build

[Andrew Morton]

On Thu, 25 Jun 2009 11:08:42 +0200
maximilian attems <max@stro.at> wrote:

> CC [M]  arch/alpha/kernel/srm_env.o
> In file included from /build/buildd-linux-2.6_2.6.30-1-alpha-vHGTgq/linux-2.6-2.6.30/debian/build/source_alpha_none/include/linux/percpu.h:10,
>                  from /build/buildd-linux-2.6_2.6.30-1-alpha-vHGTgq/linux-2.6-2.6.30/debian/build/source_alpha_none/arch/alpha/include/asm/local.h:4,
>                  from /build/buildd-linux-2.6_2.6.30-1-alpha-vHGTgq/linux-2.6-2.6.30/debian/build/source_alpha_none/include/linux/module.h:20,
>                  from /build/buildd-linux-2.6_2.6.30-1-alpha-vHGTgq/linux-2.6-2.6.30/debian/build/source_alpha_none/arch/alpha/kernel/srm_env.c:33:
> /build/buildd-linux-2.6_2.6.30-1-alpha-vHGTgq/linux-2.6-2.6.30/debian/build/source_alpha_none/arch/alpha/include/asm/percpu.h:100:1: error: "PER_CPU_ATTRIBUTES" redefined
> /build/buildd-linux-2.6_2.6.30-1-alpha-vHGTgq/linux-2.6-2.6.30/debian/build/source_alpha_none/arch/alpha/include/asm/percpu.h:52:1: error: this is the location of the previous definition
> make[6]: *** [arch/alpha/kernel/srm_env.o] Error 1
> make[5]: *** [arch/alpha/kernel] Error 2
> make[4]: *** [sub-make] Error 2
> make[3]: *** [all] Error 2
> make[2]: *** [debian/stamps/build_alpha_none_alpha-smp_plain] Error 2
> make[1]: *** [build_alpha_none_alpha-smp_real] Error 2

Me too.

> belows patch fixes aboves error but gives a new one on SMP
> 
> diff --git a/arch/alpha/include/asm/percpu.h b/arch/alpha/include/asm/percpu.h
> index 06c5c7a..622d585 100644
> --- a/arch/alpha/include/asm/percpu.h
> +++ b/arch/alpha/include/asm/percpu.h
> @@ -95,8 +95,8 @@ extern unsigned long __per_cpu_offset[NR_CPUS];
>  #define PER_CPU_SHARED_ALIGNED_SECTION ""
>  #define PER_CPU_FIRST_SECTION ""
>  
> -#endif
> -
>  #define PER_CPU_ATTRIBUTES
>  
> +#endif
> +
>  #endif /* __ALPHA_PERCPU_H */
> 
>   CC [M]  arch/alpha/kernel/srm_env.o

That fixed it.

> cc1: warnings being treated as errors
> In file included from /home/maks/src/linux-2.6-2.6.30/debian/build/source_alpha_none/include/linux/sched.h:86,
>                  from /home/maks/src/linux-2.6-2.6.30/debian/build/source_alpha_none/arch/alpha/include/asm/uaccess.h:5,
>                  from /home/maks/src/linux-2.6-2.6.30/debian/build/source_alpha_none/arch/alpha/kernel/srm_env.c:37:
> /home/maks/src/linux-2.6-2.6.30/debian/build/source_alpha_none/include/linux/hrtimer.h:310: error: ___~@~X__used_____~@~Y attribute ignored
> In file included from /home/maks/src/linux-2.6-2.6.30/debian/build/source_alpha_none/arch/alpha/include/asm/uaccess.h:5,
>                  from /home/maks/src/linux-2.6-2.6.30/debian/build/source_alpha_none/arch/alpha/kernel/srm_env.c:37:
> /home/maks/src/linux-2.6-2.6.30/debian/build/source_alpha_none/include/linux/sched.h:134: error: ___~@~X__used_____~@~Y attribute ignored
> make[6]: *** [arch/alpha/kernel/srm_env.o] Error 1
> 

This is the

#define PER_CPU_ATTRIBUTES	__used

in arch/alpha/include/asm/percpu.h, added by 

commit 9267b4b3880d00dc2dab90f1d817c856939114f7
Author:     Ivan Kokshaysky <ink@jurassic.park.msu.ru>
AuthorDate: Sat Jun 21 03:25:39 2008 +0400
Commit:     Linus Torvalds <torvalds@linux-foundation.org>
CommitDate: Fri Jun 20 16:46:10 2008 -0700

    alpha: fix module load failures on smp (bug #10926)


There's no indication in the changelog why this was added and afacit no
other architectures do it.  So I'm at a loss to help here. 

Re: >= 2.6.30 broken alpha smp build

[Tejun Heo]

Andrew Morton wrote:
> This is the
> 
> #define PER_CPU_ATTRIBUTES	__used
> 
> in arch/alpha/include/asm/percpu.h, added by 
> 
> commit 9267b4b3880d00dc2dab90f1d817c856939114f7
> Author:     Ivan Kokshaysky <ink@jurassic.park.msu.ru>
> AuthorDate: Sat Jun 21 03:25:39 2008 +0400
> Commit:     Linus Torvalds <torvalds@linux-foundation.org>
> CommitDate: Fri Jun 20 16:46:10 2008 -0700
> 
>     alpha: fix module load failures on smp (bug #10926)
> 
> There's no indication in the changelog why this was added and afacit no
> other architectures do it.  So I'm at a loss to help here. 

The __used attribute is dropped by a patch in percpu patchset as it
caused build warning on my test builds and wasn't even effective
as-was (overriden by later NULL definition).  Ivan, any ideas why it's
there?

Thanks.

-- 
tejun

Re: >= 2.6.30 broken alpha smp build

[Ivan Kokshaysky]

On Fri, Jun 26, 2009 at 09:43:22AM +0900, Tejun Heo wrote:
> The __used attribute is dropped by a patch in percpu patchset as it
> caused build warning on my test builds and wasn't even effective
> as-was (overriden by later NULL definition).  Ivan, any ideas why it's
> there?

It was intended for suppressing the compiler warnings about "unused"
static per-cpu variables, as they were only referenced inside the asm code.

Now it seems to be broken with DECLARE_PER_CPU thing, so I think your
patch that kills __used attribute should go in as a compile fix ASAP -
we can live with a few compile warnings until the rest of percpu patchset
gets merged.

Ivan.

Re: >= 2.6.30 broken alpha smp build

[Andrew Morton]

On Fri, 26 Jun 2009 16:47:05 +0400
Ivan Kokshaysky <ink@jurassic.park.msu.ru> wrote:

> On Fri, Jun 26, 2009 at 09:43:22AM +0900, Tejun Heo wrote:
> > The __used attribute is dropped by a patch in percpu patchset as it
> > caused build warning on my test builds and wasn't even effective
> > as-was (overriden by later NULL definition).  Ivan, any ideas why it's
> > there?
> 
> It was intended for suppressing the compiler warnings about "unused"
> static per-cpu variables, as they were only referenced inside the asm code.
> 
> Now it seems to be broken with DECLARE_PER_CPU thing, so I think your
> patch that kills __used attribute should go in as a compile fix ASAP -
> we can live with a few compile warnings until the rest of percpu patchset
> gets merged.
> 

OK, we have a wart.


When I do this:

--- a/arch/alpha/include/asm/percpu.h~a
+++ a/arch/alpha/include/asm/percpu.h
@@ -49,7 +49,6 @@ extern unsigned long __per_cpu_offset[NR
 		: "=&r"(__ptr), "=&r"(tmp_gp));		\
 	(typeof(&per_cpu_var(var)))(__ptr + (offset)); })
 
-#define PER_CPU_ATTRIBUTES	__used
 
 #endif /* MODULE */
 
_


I get

In file included from include/linux/sched.h:89,
                 from /usr/src/devel/arch/alpha/include/asm/uaccess.h:5,
                 from arch/alpha/kernel/srm_env.c:37:
include/linux/hrtimer.h:314: error: expected '=', ',', ';', 'asm' or '__attribute__' before 'typeof'
In file included from /usr/src/devel/arch/alpha/include/asm/uaccess.h:5,
                 from arch/alpha/kernel/srm_env.c:37:
include/linux/sched.h:138: error: expected '=', ',', ';', 'asm' or '__attribute__' before 'typeof'


But hrtimer.h correctly includes percpu.h.  I would have expected the
above patch to fix the build.

The reason for it not fixing the build is that this:

#ifndef PER_CPU_ATTRIBUTES
#define PER_CPU_ATTRIBUTES
#endif

is in asm-generic/percpu.h, which alpha does not use.

Would it not have been better to put the above three lines into
linux/percpu.h so that alpha's asm/percpu.h doesn't need to define
PER_CPU_ATTRIBUTES at all?


Ho hum.  Anyway, we need a backportable fix, so I'll put this through
some build testing:

--- a/arch/alpha/include/asm/percpu.h~a
+++ a/arch/alpha/include/asm/percpu.h
@@ -49,7 +49,7 @@ extern unsigned long __per_cpu_offset[NR
 		: "=&r"(__ptr), "=&r"(tmp_gp));		\
 	(typeof(&per_cpu_var(var)))(__ptr + (offset)); })
 
-#define PER_CPU_ATTRIBUTES	__used
+#define PER_CPU_ATTRIBUTES
 
 #endif /* MODULE */
 
_

Re: >= 2.6.30 broken alpha smp build

[Andrew Morton]

On Fri, 26 Jun 2009 11:57:46 -0700
Andrew Morton <akpm@linux-foundation.org> wrote:

> Anyway, we need a backportable fix, so I'll put this through
> some build testing:

The allmodconfig build has more problems:

ERROR: "per_cpu__cookie_scratch" [net/ipv6/ipv6.ko] undefined!
ERROR: "per_cpu__rcu_torture_batch" [kernel/rcutorture.ko] undefined!
ERROR: "per_cpu__rcu_torture_count" [kernel/rcutorture.ko] undefined!
ERROR: "clk_enable" [drivers/i2c/busses/i2c-designware.ko] undefined!
ERROR: "clk_get" [drivers/i2c/busses/i2c-designware.ko] undefined!
ERROR: "clk_put" [drivers/i2c/busses/i2c-designware.ko] undefined!
ERROR: "clk_disable" [drivers/i2c/busses/i2c-designware.ko] undefined!
ERROR: "clk_get_rate" [drivers/i2c/busses/i2c-designware.ko] undefined!
ERROR: "per_cpu__msg_schedule" [crypto/sha512_generic.ko] undefined!
ERROR: "per_cpu__ioc_count" [block/cfq-iosched.ko] undefined!
ERROR: "per_cpu__ioc_count" [block/as-iosched.ko] undefined!

It looks like making these DEFINE_PER_CPUs non-static will fix it up. 
What's that all about?

[PATCH] alpha: fix percpu build breakage

[Tejun Heo]

alpha percpu access requires custom SHIFT_PERCPU_PTR() definition for
modules to work around addressing range limitation.  This is done via
generating inline assembly using C preprocessing which forces the
assembler to generate external reference.  This happens behind the
compiler's back and makes the compiler think that static percpu
variables in modules are unused.

This used to be worked around by using __unused attribute for percpu
variables which prevent the compiler from omitting the variable;
however, recent declare/definition attribute unification change broke
this as __used can't be used for declaration.  Also, in the process,
PER_CPU_ATTRIBUTES definition in alpha percpu.h got broken.

This patch adds PER_CPU_DEF_ATTRIBUTES which is only used for
definitions and make alpha use it to add __used for percpu variables
in modules.  This also fixes the PER_CPU_ATTRIBUTES double definition
bug.

Signed-off-by: Tejun Heo <tj@kernel.org>
---
 arch/alpha/include/asm/percpu.h |    6 +++---
 include/asm-generic/percpu.h    |    4 ++++
 include/linux/percpu-defs.h     |    3 ++-
 3 files changed, 9 insertions(+), 4 deletions(-)

diff --git a/arch/alpha/include/asm/percpu.h b/arch/alpha/include/asm/percpu.h
index 06c5c7a..b663f1f 100644
--- a/arch/alpha/include/asm/percpu.h
+++ b/arch/alpha/include/asm/percpu.h
@@ -30,7 +30,7 @@ extern unsigned long __per_cpu_offset[NR_CPUS];
 
 #ifndef MODULE
 #define SHIFT_PERCPU_PTR(var, offset) RELOC_HIDE(&per_cpu_var(var), (offset))
-#define PER_CPU_ATTRIBUTES
+#define PER_CPU_DEF_ATTRIBUTES
 #else
 /*
  * To calculate addresses of locally defined variables, GCC uses 32-bit
@@ -49,7 +49,7 @@ extern unsigned long __per_cpu_offset[NR_CPUS];
 		: "=&r"(__ptr), "=&r"(tmp_gp));		\
 	(typeof(&per_cpu_var(var)))(__ptr + (offset)); })
 
-#define PER_CPU_ATTRIBUTES	__used
+#define PER_CPU_DEF_ATTRIBUTES	__used
 
 #endif /* MODULE */
 
@@ -71,7 +71,7 @@ extern unsigned long __per_cpu_offset[NR_CPUS];
 #define __get_cpu_var(var)		per_cpu_var(var)
 #define __raw_get_cpu_var(var)		per_cpu_var(var)
 
-#define PER_CPU_ATTRIBUTES
+#define PER_CPU_DEF_ATTRIBUTES
 
 #endif /* SMP */
 
diff --git a/include/asm-generic/percpu.h b/include/asm-generic/percpu.h
index d7d50d7..aa00800 100644
--- a/include/asm-generic/percpu.h
+++ b/include/asm-generic/percpu.h
@@ -97,4 +97,8 @@ extern void setup_per_cpu_areas(void);
 #define PER_CPU_ATTRIBUTES
 #endif
 
+#ifndef PER_CPU_DEF_ATTRIBUTES
+#define PER_CPU_DEF_ATTRIBUTES
+#endif
+
 #endif /* _ASM_GENERIC_PERCPU_H_ */
diff --git a/include/linux/percpu-defs.h b/include/linux/percpu-defs.h
index 8f921d7..68438e1 100644
--- a/include/linux/percpu-defs.h
+++ b/include/linux/percpu-defs.h
@@ -24,7 +24,8 @@
 
 #define DEFINE_PER_CPU_SECTION(type, name, section)			\
 	__attribute__((__section__(PER_CPU_BASE_SECTION section)))	\
-	PER_CPU_ATTRIBUTES __typeof__(type) per_cpu__##name
+	PER_CPU_ATTRIBUTES PER_CPU_DEF_ATTRIBUTES			\
+	__typeof__(type) per_cpu__##name
 
 /*
  * Variant on the per-CPU variable declaration/definition theme used for

Re: [PATCH] alpha: fix percpu build breakage

[Ivan Kokshaysky]

On Sun, Jun 28, 2009 at 09:49:56AM +0900, Tejun Heo wrote:
> alpha percpu access requires custom SHIFT_PERCPU_PTR() definition for
> modules to work around addressing range limitation.  This is done via
> generating inline assembly using C preprocessing which forces the
> assembler to generate external reference.  This happens behind the
> compiler's back and makes the compiler think that static percpu
> variables in modules are unused.
> 
> This used to be worked around by using __unused attribute for percpu
> variables which prevent the compiler from omitting the variable;
> however, recent declare/definition attribute unification change broke
> this as __used can't be used for declaration.  Also, in the process,
> PER_CPU_ATTRIBUTES definition in alpha percpu.h got broken.
> 
> This patch adds PER_CPU_DEF_ATTRIBUTES which is only used for
> definitions and make alpha use it to add __used for percpu variables
> in modules.  This also fixes the PER_CPU_ATTRIBUTES double definition
> bug.

Thanks! I was going to suggest similar patch, but you've beaten
me to it ;-)

> Signed-off-by: Tejun Heo <tj@kernel.org>

Acked-by: Ivan Kokshaysky <ink@jurassic.park.msu.ru>

As for upcoming percpu patchset, can we similarly move the __weak attribute
to PER_CPU_DEF_ATTRIBUTES? I don't feel really comfortable with an
"extern __weak" combination - it may not work with future compilers...

Ivan.

Re: [PATCH] alpha: fix percpu build breakage

[Tejun Heo]

Hello, Ivan.

Ivan Kokshaysky wrote:
> As for upcoming percpu patchset, can we similarly move the __weak attribute
> to PER_CPU_DEF_ATTRIBUTES? I don't feel really comfortable with an
> "extern __weak" combination - it may not work with future compilers...

Oh... the newest incarnation looks like the following.

#if defined(ARCH_NEEDS_WEAK_PER_CPU) || defined(CONFIG_DEBUG_FORCE_WEAK_PER_CPU)
#define DECLARE_PER_CPU_SECTION(type, name, sec)			\
	extern __PCPU_DUMMY_ATTRS char __pcpu_scope_##name;		\
	extern __PCPU_ATTRS(sec) __weak __typeof__(type) per_cpu__##name

#define DEFINE_PER_CPU_SECTION(type, name, sec)				\
	__PCPU_DUMMY_ATTRS char __pcpu_scope_##name;			\
	__PCPU_DUMMY_ATTRS char __pcpu_unique_##name;			\
	__PCPU_ATTRS(sec) __weak __typeof__(type) per_cpu__##name
#else
#define DECLARE_PER_CPU_SECTION(type, name, sec)			\
	extern __PCPU_ATTRS(sec) __typeof__(type) per_cpu__##name

#define DEFINE_PER_CPU_SECTION(type, name, sec)				\
	__PCPU_ATTRS(sec) __typeof__(type) per_cpu__##name
#endif

So, I can simply drop __weak from the declaration like the following.
Looks good?

 include/linux/percpu-defs.h |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/include/linux/percpu-defs.h b/include/linux/percpu-defs.h
index cf32838..9b7a53c 100644
--- a/include/linux/percpu-defs.h
+++ b/include/linux/percpu-defs.h
@@ -56,7 +56,7 @@
  */
 #define DECLARE_PER_CPU_SECTION(type, name, sec)			\
 	extern __PCPU_DUMMY_ATTRS char __pcpu_scope_##name;		\
-	extern __PCPU_ATTRS(sec) __weak __typeof__(type) per_cpu__##name
+	extern __PCPU_ATTRS(sec) __typeof__(type) per_cpu__##name
 
 #define DEFINE_PER_CPU_SECTION(type, name, sec)				\
 	__PCPU_DUMMY_ATTRS char __pcpu_scope_##name;			\

-- 
tejun

Re: [PATCH] alpha: fix percpu build breakage

[Ivan Kokshaysky]

On Sun, Jun 28, 2009 at 09:21:17PM +0900, Tejun Heo wrote:
> Oh... the newest incarnation looks like the following.
> 
> #if defined(ARCH_NEEDS_WEAK_PER_CPU) || defined(CONFIG_DEBUG_FORCE_WEAK_PER_CPU)
> #define DECLARE_PER_CPU_SECTION(type, name, sec)			\
> 	extern __PCPU_DUMMY_ATTRS char __pcpu_scope_##name;		\
> 	extern __PCPU_ATTRS(sec) __weak __typeof__(type) per_cpu__##name
> 
> #define DEFINE_PER_CPU_SECTION(type, name, sec)				\
> 	__PCPU_DUMMY_ATTRS char __pcpu_scope_##name;			\
> 	__PCPU_DUMMY_ATTRS char __pcpu_unique_##name;			\
> 	__PCPU_ATTRS(sec) __weak __typeof__(type) per_cpu__##name
> #else
> #define DECLARE_PER_CPU_SECTION(type, name, sec)			\
> 	extern __PCPU_ATTRS(sec) __typeof__(type) per_cpu__##name
> 
> #define DEFINE_PER_CPU_SECTION(type, name, sec)				\
> 	__PCPU_ATTRS(sec) __typeof__(type) per_cpu__##name
> #endif
> 
> So, I can simply drop __weak from the declaration like the following.
> Looks good?

Yeah, thanks, Tejun.

Ivan.