s390(64) per_cpu in modules (ipv6)

s390(64) per_cpu in modules (ipv6)

[Pete Zaitcev]

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

Re: s390(64) per_cpu in modules (ipv6)

[William Lee Irwin III]

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

Re: s390(64) per_cpu in modules (ipv6)

[Roland Dreier]

    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

Re: s390(64) per_cpu in modules (ipv6)

[Martin Schwidefsky]

> > 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__ */

Re: s390(64) per_cpu in modules (ipv6)

[Roland Dreier]

    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

Re: s390(64) per_cpu in modules (ipv6)

[Pete Zaitcev]

On Wed, 30 Jun 2004 15:04:42 +0200
Martin Schwidefsky <schwidefsky@de.ibm.com> wrote:

> The solution is the "X" constraint.

What is the minimum gcc version which supports "X" constraint?

Thanks,
-- Pete

Re: s390(64) per_cpu in modules (ipv6)

[Arnd Bergmann]

[-- Attachment #1: Type: text/plain, Size: 216 bytes --]

On Mittwoch, 30. Juni 2004 22:55, Pete Zaitcev wrote:
> What is the minimum gcc version which supports "X" constraint?
> 
It's already there in 2.95, which was the oldest version with
s390 support.

	Arnd <><

[-- Attachment #2: signature --]
[-- Type: application/pgp-signature, Size: 189 bytes --]

Re: s390(64) per_cpu in modules (ipv6)

[Rusty Russell]

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

Re: s390(64) per_cpu in modules (ipv6)

[Rusty Russell]

On Fri, 2004-10-15 at 19:15, Martin Schwidefsky wrote:
> 
> 
> Rusty Russell <rusty@rustcorp.com.au> 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