Hi, Martin,
I tried to build ipv6 as a module in 2.6.7, and it bombs, producing a
module which wants per_cpu____icmpv6_socket (obviously, undefined).
The problem appears to be caused by this:
#define __get_got_cpu_var(var,offset) \
(*({ unsigned long *__ptr; \
asm ( "larl %0,per_cpu__"#var"@GOTENT" : "=a" (__ptr) ); \
((typeof(&per_cpu__##var))((*__ptr) + offset)); \
}))
The net/ipv6/icmp.c has this:
static DEFINE_PER_CPU(struct socket *, __icmpv6_socket) = NULL;
It is a static variable, and all references to it come from asm code.
The gcc does not know that they are present and optimizes the variable
away. It simply emits no code to define per_cpu____icmpv6_socket
whatsoever, as verified with gcc -S.
If the DEFINE_PER_CPU() is used without static, or if a bogus
reference is added to the C code (a printk), everything works.
I tried to add a fake input argument to the asm, like this:
--- linux-2.6.7-1.459/include/asm-s390/percpu.h 2004-06-16 01:20:04.000000000 -0400
+++ linux-2.6.7-1.459.z1/include/asm-s390/percpu.h 2004-06-29 22:03:23.000000000 -0400
@@ -15,7 +15,7 @@
#if defined(__s390x__) && defined(MODULE)
#define __get_got_cpu_var(var,offset) \
(*({ unsigned long *__ptr; \
- asm ( "larl %0,per_cpu__"#var"@GOTENT" : "=a" (__ptr) ); \
+ asm ( "larl %0,per_cpu__"#var"@GOTENT" : "=a" (__ptr) : "a" (&per_cpu__##var) ); \
((typeof(&per_cpu__##var))((*__ptr) + offset)); \
}))
#undef __get_cpu_var
It seems to work fine, but I'm wondering if a better fix can be found.
Ideas?
-- Pete
On Tue, Jun 29, 2004 at 11:35:37PM -0700, Pete Zaitcev wrote:
> It seems to work fine, but I'm wondering if a better fix can be found.
> Ideas?
> -- Pete
How does __attribute__((used)) fare?
-- wli
Pete> It seems to work fine, but I'm wondering if a better fix can
Pete> be found. Ideas?
Not sure if it will work (and I don't have an s390 toolchain handy!)
but this (static variable used only by asm) seems to be exactly the
situation that __attribute_used__ is intended for.
- Roland
> > It seems to work fine, but I'm wondering if a better fix can
> > be found.
>
> How does __attribute__((used)) fare?
__attribute_used__ isn't really what we want. If a statically
defined per cpu variable isn't used in the C file then gcc
should be allowed to remove it. It's not used after all.
What we need is a way to tell the compiler that an inline
assembly uses a variable without passing any kind of address
of the variable to it. The solution is the "X" constraint.
While I was at it I cleaned things up a bit, I don't like
the #undefs in percpu.h. Patch is attached, I will include
it in my next update for Andrew.
blue skies,
Martin.
diff -urN linux-2.6/include/asm-s390/percpu.h linux-2.6-s390/include/asm-s390/percpu.h
--- linux-2.6/include/asm-s390/percpu.h Wed Jun 16 07:20:04 2004
+++ linux-2.6-s390/include/asm-s390/percpu.h Wed Jun 30 14:37:45 2004
@@ -1,30 +1,70 @@
#ifndef __ARCH_S390_PERCPU__
#define __ARCH_S390_PERCPU__
-#include <asm-generic/percpu.h>
+#include <linux/compiler.h>
#include <asm/lowcore.h>
+#define __GENERIC_PER_CPU
+
/*
- * For builtin kernel code s390 uses the generic implementation for
- * per cpu data, with the exception that the offset of the cpu local
- * data area is cached in the cpu's lowcore memory
+ * s390 uses its own implementation for per cpu data, the offset of
+ * the cpu local data area is cached in the cpu's lowcore memory.
* For 64 bit module code s390 forces the use of a GOT slot for the
* address of the per cpu variable. This is needed because the module
* may be more than 4G above the per cpu area.
*/
#if defined(__s390x__) && defined(MODULE)
-#define __get_got_cpu_var(var,offset) \
+
+#define __reloc_hide(var,offset) \
(*({ unsigned long *__ptr; \
- asm ( "larl %0,per_cpu__"#var"@GOTENT" : "=a" (__ptr) ); \
- ((typeof(&per_cpu__##var))((*__ptr) + offset)); \
- }))
-#undef __get_cpu_var
-#define __get_cpu_var(var) __get_got_cpu_var(var,S390_lowcore.percpu_offset)
-#undef per_cpu
-#define per_cpu(var,cpu) __get_got_cpu_var(var,__per_cpu_offset[cpu])
+ asm ( "larl %0,per_cpu__"#var"@GOTENT" \
+ : "=a" (__ptr) : "X" (per_cpu__##var) ); \
+ (typeof(&per_cpu__##var))((*__ptr) + (offset)); }))
+
#else
-#undef __get_cpu_var
-#define __get_cpu_var(var) (*RELOC_HIDE(&per_cpu__##var, S390_lowcore.percpu_offset))
+
+#define __reloc_hide(var, offset) \
+ (*({ unsigned long __ptr; \
+ asm ( "" : "=a" (__ptr) : "0" (&per_cpu__##var) ); \
+ (typeof(&per_cpu__##var)) (__ptr + (offset)); }))
+
#endif
+#ifdef CONFIG_SMP
+
+extern unsigned long __per_cpu_offset[NR_CPUS];
+
+/* Separate out the type, so (int[3], foo) works. */
+#define DEFINE_PER_CPU(type, name) \
+ __attribute__((__section__(".data.percpu"))) \
+ __typeof__(type) per_cpu__##name
+
+#define __get_cpu_var(var) __reloc_hide(var,S390_lowcore.percpu_offset)
+#define per_cpu(var,cpu) __reloc_hide(var,__per_cpu_offset[cpu])
+
+/* A macro to avoid #include hell... */
+#define percpu_modcopy(pcpudst, src, size) \
+do { \
+ unsigned int __i; \
+ for (__i = 0; __i < NR_CPUS; __i++) \
+ if (cpu_possible(__i)) \
+ memcpy((pcpudst)+__per_cpu_offset[__i], \
+ (src), (size)); \
+} while (0)
+
+#else /* ! SMP */
+
+#define DEFINE_PER_CPU(type, name) \
+ __typeof__(type) per_cpu__##name
+
+#define __get_cpu_var(var) __reloc_hide(var,0)
+#define per_cpu(var,cpu) __reloc_hide(var,0)
+
+#endif /* SMP */
+
+#define DECLARE_PER_CPU(type, name) extern __typeof__(type) per_cpu__##name
+
+#define EXPORT_PER_CPU_SYMBOL(var) EXPORT_SYMBOL(per_cpu__##var)
+#define EXPORT_PER_CPU_SYMBOL_GPL(var) EXPORT_SYMBOL_GPL(per_cpu__##var)
+
#endif /* __ARCH_S390_PERCPU__ */
Martin> __attribute_used__ isn't really what we want. If a
Martin> statically defined per cpu variable isn't used in the C
Martin> file then gcc should be allowed to remove it. It's not
Martin> used after all. What we need is a way to tell the
Martin> compiler that an inline assembly uses a variable without
Martin> passing any kind of address of the variable to it.
Actually my understanding is that __attribute_used__ is intended for
exactly that: to let the compiler know that something that is
apparently unused by the C code is actually used by inline assembly.
I'm sure your solution is fine as well but I think this type of
situation is exactly what __attribute_used__ is for. For example it
is attached to the modversions ____versions array so that it is not
discarded by the compiler.
Best,
Roland
On Wed, 30 Jun 2004 15:04:42 +0200
Martin Schwidefsky <[email protected]> wrote:
> The solution is the "X" constraint.
What is the minimum gcc version which supports "X" constraint?
Thanks,
-- Pete
On Wed, 2004-06-30 at 16:35, Pete Zaitcev wrote:
> Hi, Martin,
>
> I tried to build ipv6 as a module in 2.6.7, and it bombs, producing a
> module which wants per_cpu____icmpv6_socket (obviously, undefined).
>
> The problem appears to be caused by this:
>
> #define __get_got_cpu_var(var,offset) \
> (*({ unsigned long *__ptr; \
> asm ( "larl %0,per_cpu__"#var"@GOTENT" : "=a" (__ptr) ); \
> ((typeof(&per_cpu__##var))((*__ptr) + offset)); \
> }))
Heh, I ran into the same problem trying to do this trick for PPC64. You
really need to use __thread and make GCC do the work, AFAICT.
The worse problem is that a (static) per-cpu var declared *inside* a
function gets renamed by gcc; IIRC some generic code used to do this.
Rusty.
--
Anyone who quotes me in their signature is an idiot -- Rusty Russell
On Fri, 2004-10-15 at 19:15, Martin Schwidefsky wrote:
>
>
> Rusty Russell <[email protected]> wrote on 15/10/2004 03:41:40 AM:
> > The worse problem is that a (static) per-cpu var declared *inside* a
> > function gets renamed by gcc; IIRC some generic code used to do this.
>
> __thread in the kernel would be a real innovation, but I fear it isn't easy.
> The problem with the per_cpu__x variables in modules is solved for s390x
> by the way.
Sure, but it doesn't solve this case, AFAICT:
void func(void)
{
static DEFINE_PER_CPU(x, int);
__get_per_cpu(x)++;
}
The compiler will create a variable called "per_cpu__x.0" and your asm
reference to "per_cpu__x" will cause a link failure, no? Obviously, you
would have noticed this, so I'm wondering what I'm missing.
I hit this in mm/page-writeback.c:balance_dirty_pages_ratelimited().
Confused,
Rusty.
--
Anyone who quotes me in their signature is an idiot -- Rusty Russell