Re: link error : 2.6.21-rc6-mm1 for s390

Re: link error : 2.6.21-rc6-mm1 for s390

[Andrew Morton]

On Tue, 10 Apr 2007 20:56:16 -0400
Mathieu Desnoyers <mathieu.desnoyers@polymtl.ca> wrote:

> The last for today : link error of 2.6.21-rc6-mm1 for s390 :
> 
> 
>   /opt/crosstool/gcc-4.1.1-glibc-2.3.6/s390-unknown-linux-gnu/bin/s390-unknown-linux-gnu-ld -m elf_s390 -e start -o .tmp_vmlinux1 -T arch/s390/kernel/vmlinux.lds arch/s390/kernel/head.o arch/s390/kernel/init_task.o  init/built-in.o --start-group  usr/built-in.o  arch/s390/mm/built-in.o  arch/s390/kernel/built-in.o  arch/s390/crypto/built-in.o  arch/s390/appldata/built-in.o  arch/s390/hypfs/built-in.o  kernel/built-in.o  mm/built-in.o  fs/built-in.o  ipc/built-in.o  security/built-in.o  crypto/built-in.o  block/built-in.o  ltt/built-in.o  lib/lib.a  arch/s390/lib/lib.a  lib/built-in.o  arch/s390/lib/built-in.o  drivers/built-in.o  sound/built-in.o  drivers/s390/built-in.o  arch/s390/math-emu/built-in.o  net/built-in.o --end-group
> lib/built-in.o: In function `__div64_32':
> : multiple definition of `__div64_32'
> arch/s390/lib/lib.a(div64.o):div64.c:(.text+0x0): first defined here
> /opt/crosstool/gcc-4.1.1-glibc-2.3.6/s390-unknown-linux-gnu/bin/s390-unknown-linux-gnu-ld: Warning: size of symbol `__div64_32' changed from 218 in arch/s390/lib/lib.a(div64.o) to 260 in lib/built-in.o

git-net.patch implements generic lib/div64.c, but s390 also has a private one.

Presumably the appropriate fix is to remove s390's private implementation within
davem's tree.

Re: link error : 2.6.21-rc6-mm1 for s390

[David Miller]

From: Andrew Morton <akpm@linux-foundation.org>
Date: Tue, 10 Apr 2007 18:29:37 -0700

> git-net.patch implements generic lib/div64.c, but s390 also has a
> private one.  Presumably the appropriate fix is to remove s390's
> private implementation within davem's tree.

The s390 version seems to be optimized in assembler for that
processor, therefore we should probably instead elide the
generic version on s390.

How about something like this?

diff --git a/include/asm-s390/div64.h b/include/asm-s390/div64.h
index 6cd978c..21aea15 100644
--- a/include/asm-s390/div64.h
+++ b/include/asm-s390/div64.h
@@ -1 +1,2 @@
 #include <asm-generic/div64.h>
+#define HAVE_ARCH_DIV64_32
diff --git a/lib/div64.c b/lib/div64.c
index 74f0c8c..5b480fa 100644
--- a/lib/div64.c
+++ b/lib/div64.c
@@ -23,6 +23,8 @@
 /* Not needed on 64bit architectures */
 #if BITS_PER_LONG == 32
 
+#ifndef HAVE_ARCH_DIV64_32
+
 uint32_t __div64_32(uint64_t *n, uint32_t base)
 {
 	uint64_t rem = *n;
@@ -58,6 +60,8 @@ uint32_t __div64_32(uint64_t *n, uint32_t base)
 
 EXPORT_SYMBOL(__div64_32);
 
+#endif /* !(HAVE_ARCH_DIV64_32) */
+
 /* 64bit divisor, dividend and result. dynamic precision */
 uint64_t div64_64(uint64_t dividend, uint64_t divisor)
 {

Re: link error : 2.6.21-rc6-mm1 for s390

[Andrew Morton]

On Tue, 10 Apr 2007 18:36:29 -0700 (PDT)
David Miller <davem@davemloft.net> wrote:

> From: Andrew Morton <akpm@linux-foundation.org>
> Date: Tue, 10 Apr 2007 18:29:37 -0700
> 
> > git-net.patch implements generic lib/div64.c, but s390 also has a
> > private one.  Presumably the appropriate fix is to remove s390's
> > private implementation within davem's tree.
> 
> The s390 version seems to be optimized in assembler for that
> processor, therefore we should probably instead elide the
> generic version on s390.

We're sure that it has the same API?

> How about something like this?
> 
> diff --git a/include/asm-s390/div64.h b/include/asm-s390/div64.h
> index 6cd978c..21aea15 100644
> --- a/include/asm-s390/div64.h
> +++ b/include/asm-s390/div64.h
> @@ -1 +1,2 @@
>  #include <asm-generic/div64.h>
> +#define HAVE_ARCH_DIV64_32
> diff --git a/lib/div64.c b/lib/div64.c
> index 74f0c8c..5b480fa 100644
> --- a/lib/div64.c
> +++ b/lib/div64.c
> @@ -23,6 +23,8 @@
>  /* Not needed on 64bit architectures */
>  #if BITS_PER_LONG == 32
>  
> +#ifndef HAVE_ARCH_DIV64_32
> +
>  uint32_t __div64_32(uint64_t *n, uint32_t base)
>  {
>  	uint64_t rem = *n;
> @@ -58,6 +60,8 @@ uint32_t __div64_32(uint64_t *n, uint32_t base)
>  
>  EXPORT_SYMBOL(__div64_32);
>  
> +#endif /* !(HAVE_ARCH_DIV64_32) */
> +
>  /* 64bit divisor, dividend and result. dynamic precision */
>  uint64_t div64_64(uint64_t dividend, uint64_t divisor)
>  {

attribute(weak) would give a nicer result?

We'd also need to remove s390's EXPORT_SYMBOL(__div64_32), so s390 ends up
using lib/div64.c's EXPORT_SYMBOL().

Re: link error : 2.6.21-rc6-mm1 for s390

[David Miller]

From: Andrew Morton <akpm@linux-foundation.org>
Date: Tue, 10 Apr 2007 18:47:38 -0700

> On Tue, 10 Apr 2007 18:36:29 -0700 (PDT)
> David Miller <davem@davemloft.net> wrote:
> 
> > From: Andrew Morton <akpm@linux-foundation.org>
> > Date: Tue, 10 Apr 2007 18:29:37 -0700
> > 
> > > git-net.patch implements generic lib/div64.c, but s390 also has a
> > > private one.  Presumably the appropriate fix is to remove s390's
> > > private implementation within davem's tree.
> > 
> > The s390 version seems to be optimized in assembler for that
> > processor, therefore we should probably instead elide the
> > generic version on s390.
> 
> We're sure that it has the same API?

Yes, I read over it, I'm pretty sure it does.

> attribute(weak) would give a nicer result?

I'm not so sure.

> We'd also need to remove s390's EXPORT_SYMBOL(__div64_32), so s390 ends up
> using lib/div64.c's EXPORT_SYMBOL().

It shouldn't matter if we use s390's or the generic version's....

Oh, I see, s390 uses lib-y for it's div64.o object, that's a bug.
I'll fix that up, thanks Andrew.

Re: link error : 2.6.21-rc6-mm1 for s390

[David Miller]

From: Andrew Morton <akpm@linux-foundation.org>
Date: Tue, 10 Apr 2007 18:47:38 -0700

> attribute(weak) would give a nicer result?
> 
> We'd also need to remove s390's EXPORT_SYMBOL(__div64_32), so s390 ends up
> using lib/div64.c's EXPORT_SYMBOL().

Ok, here is the version of the fix I'll use for now:

commit c3abb3b8d41814ce4691cc4cc3998b0f5242c8d0
Author: David S. Miller <davem@sunset.davemloft.net>
Date:   Tue Apr 10 22:10:39 2007 -0700

    [S390]: Fix build on 31-bit.
    
    Allow s390 to properly override the generic
    __div64_32() implementation by:
    
    1) Using obj-y for div64.o in s390's makefile instead
       of lib-y
    
    2) Adding the weak attribute to the generic implementation.
    
    Signed-off-by: David S. Miller <davem@davemloft.net>

diff --git a/arch/s390/lib/Makefile b/arch/s390/lib/Makefile
index 7a44fed..59aea65 100644
--- a/arch/s390/lib/Makefile
+++ b/arch/s390/lib/Makefile
@@ -5,6 +5,6 @@
 EXTRA_AFLAGS := -traditional
 
 lib-y += delay.o string.o uaccess_std.o uaccess_pt.o qrnnd.o
-lib-$(CONFIG_32BIT) += div64.o
+obj-$(CONFIG_32BIT) += div64.o
 lib-$(CONFIG_64BIT) += uaccess_mvcos.o
 lib-$(CONFIG_SMP) += spinlock.o
diff --git a/arch/s390/lib/div64.c b/arch/s390/lib/div64.c
index 0481f34..a5f8300 100644
--- a/arch/s390/lib/div64.c
+++ b/arch/s390/lib/div64.c
@@ -147,5 +147,3 @@ uint32_t __div64_32(uint64_t *n, uint32_t base)
 }
 
 #endif /* MARCH_G5 */
-
-EXPORT_SYMBOL(__div64_32);
diff --git a/lib/div64.c b/lib/div64.c
index 74f0c8c..b71cf93 100644
--- a/lib/div64.c
+++ b/lib/div64.c
@@ -23,7 +23,7 @@
 /* Not needed on 64bit architectures */
 #if BITS_PER_LONG == 32
 
-uint32_t __div64_32(uint64_t *n, uint32_t base)
+uint32_t __attribute__((weak)) __div64_32(uint64_t *n, uint32_t base)
 {
 	uint64_t rem = *n;
 	uint64_t b = base;

Re: link error : 2.6.21-rc6-mm1 for s390

[Andrew Morton]

On Tue, 10 Apr 2007 22:11:01 -0700 (PDT) David Miller <davem@davemloft.net> wrote:

> diff --git a/arch/s390/lib/Makefile b/arch/s390/lib/Makefile
> index 7a44fed..59aea65 100644
> --- a/arch/s390/lib/Makefile
> +++ b/arch/s390/lib/Makefile
> @@ -5,6 +5,6 @@
>  EXTRA_AFLAGS := -traditional
>  
>  lib-y += delay.o string.o uaccess_std.o uaccess_pt.o qrnnd.o
> -lib-$(CONFIG_32BIT) += div64.o
> +obj-$(CONFIG_32BIT) += div64.o
>  lib-$(CONFIG_64BIT) += uaccess_mvcos.o
>  lib-$(CONFIG_SMP) += spinlock.o
> diff --git a/arch/s390/lib/div64.c b/arch/s390/lib/div64.c
> index 0481f34..a5f8300 100644
> --- a/arch/s390/lib/div64.c
> +++ b/arch/s390/lib/div64.c
> @@ -147,5 +147,3 @@ uint32_t __div64_32(uint64_t *n, uint32_t base)
>  }
>  
>  #endif /* MARCH_G5 */
> -
> -EXPORT_SYMBOL(__div64_32);
> diff --git a/lib/div64.c b/lib/div64.c
> index 74f0c8c..b71cf93 100644
> --- a/lib/div64.c
> +++ b/lib/div64.c
> @@ -23,7 +23,7 @@
>  /* Not needed on 64bit architectures */
>  #if BITS_PER_LONG == 32
>  
> -uint32_t __div64_32(uint64_t *n, uint32_t base)
> +uint32_t __attribute__((weak)) __div64_32(uint64_t *n, uint32_t base)
>  {
>  	uint64_t rem = *n;
>  	uint64_t b = base;

I think this means that if CONFIG_32BIT=y, s390 networking gets the whizzy
assembly version and if CONFIG_32BIT=n, it gets to use the generic version.

Possibly the whizzy version could be used if CONFIG_32BIT=n, too.  But
I'd let the s390 people worry about that ;)

Re: link error : 2.6.21-rc6-mm1 for s390

[Martin Schwidefsky]

On 4/11/07, Andrew Morton <akpm@linux-foundation.org> wrote:
> On Tue, 10 Apr 2007 22:11:01 -0700 (PDT) David Miller <davem@davemloft.net> wrote:
> I think this means that if CONFIG_32BIT=y, s390 networking gets the whizzy
> assembly version and if CONFIG_32BIT=n, it gets to use the generic version.
>
> Possibly the whizzy version could be used if CONFIG_32BIT=n, too.  But
> I'd let the s390 people worry about that ;)

If CONFIG_32BIT=n we don't need __div64_32 at all, do we? Just use the
real 64 / 64 bit division.

-- 
blue skies,
  Martin

Re: link error : 2.6.21-rc6-mm1 for s390

[David Miller]

From: "Martin Schwidefsky" <schwidefsky@googlemail.com>
Date: Wed, 11 Apr 2007 10:15:42 +0200

> On 4/11/07, Andrew Morton <akpm@linux-foundation.org> wrote:
> > On Tue, 10 Apr 2007 22:11:01 -0700 (PDT) David Miller <davem@davemloft.net> wrote:
> > I think this means that if CONFIG_32BIT=y, s390 networking gets the whizzy
> > assembly version and if CONFIG_32BIT=n, it gets to use the generic version.
> >
> > Possibly the whizzy version could be used if CONFIG_32BIT=n, too.  But
> > I'd let the s390 people worry about that ;)
> 
> If CONFIG_32BIT=n we don't need __div64_32 at all, do we? Just use the
> real 64 / 64 bit division.

That's right, Andrew missed the BITS_PER_LONG check in lib/div64.c
:-)