linuxppc-dev.lists.ozlabs.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] powerpc: remove meaningless KBUILD_ARFLAGS addition
@ 2019-07-13  3:21 Masahiro Yamada
  2019-07-13 12:47 ` Segher Boessenkool
  2019-08-28  4:24 ` Michael Ellerman
  0 siblings, 2 replies; 19+ messages in thread
From: Masahiro Yamada @ 2019-07-13  3:21 UTC (permalink / raw)
  To: Michael Ellerman, linuxppc-dev
  Cc: Stephen Rothwell, linux-kernel, Nicholas Piggin, Masahiro Yamada,
	Paul Mackerras

The KBUILD_ARFLAGS addition in arch/powerpc/Makefile has never worked
in a useful way because it is always overridden by the following code
in the top Makefile:

  # use the deterministic mode of AR if available
  KBUILD_ARFLAGS := $(call ar-option,D)

The code in the top Makefile was added in 2011, by commit 40df759e2b9e
("kbuild: Fix build with binutils <= 2.19").

The KBUILD_ARFLAGS addition for ppc has always been dead code from the
beginning.

Nobody has reported a problem since 43c9127d94d6 ("powerpc: Add option
to use thin archives"), so this code was unneeded.

Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>
---

 arch/powerpc/Makefile | 1 -
 1 file changed, 1 deletion(-)

diff --git a/arch/powerpc/Makefile b/arch/powerpc/Makefile
index c345b79414a9..46ed198a3aa3 100644
--- a/arch/powerpc/Makefile
+++ b/arch/powerpc/Makefile
@@ -112,7 +112,6 @@ ifeq ($(HAS_BIARCH),y)
 KBUILD_CFLAGS	+= -m$(BITS)
 KBUILD_AFLAGS	+= -m$(BITS) -Wl,-a$(BITS)
 KBUILD_LDFLAGS	+= -m elf$(BITS)$(LDEMULATION)
-KBUILD_ARFLAGS	+= --target=elf$(BITS)-$(GNUTARGET)
 endif
 
 cflags-$(CONFIG_STACKPROTECTOR)	+= -mstack-protector-guard=tls
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 19+ messages in thread

* Re: [PATCH] powerpc: remove meaningless KBUILD_ARFLAGS addition
  2019-07-13  3:21 [PATCH] powerpc: remove meaningless KBUILD_ARFLAGS addition Masahiro Yamada
@ 2019-07-13 12:47 ` Segher Boessenkool
  2019-07-13 13:16   ` Segher Boessenkool
  2019-08-28  4:24 ` Michael Ellerman
  1 sibling, 1 reply; 19+ messages in thread
From: Segher Boessenkool @ 2019-07-13 12:47 UTC (permalink / raw)
  To: Masahiro Yamada
  Cc: Stephen Rothwell, linux-kernel, Nicholas Piggin, Paul Mackerras,
	linuxppc-dev

On Sat, Jul 13, 2019 at 12:21:06PM +0900, Masahiro Yamada wrote:
> The KBUILD_ARFLAGS addition in arch/powerpc/Makefile has never worked
> in a useful way because it is always overridden by the following code
> in the top Makefile:
> 
>   # use the deterministic mode of AR if available
>   KBUILD_ARFLAGS := $(call ar-option,D)
> 
> The code in the top Makefile was added in 2011, by commit 40df759e2b9e
> ("kbuild: Fix build with binutils <= 2.19").
> 
> The KBUILD_ARFLAGS addition for ppc has always been dead code from the
> beginning.

That was added in 43c9127d94d6 to replace my 8995ac870273 from 2007.

Is it no longer supported to build a 64-bit kernel with a toolchain
that defaults to 32-bit, or the other way around?  And with non-native
toolchains (this one didn't run on Linux, even).


Segher

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH] powerpc: remove meaningless KBUILD_ARFLAGS addition
  2019-07-13 12:47 ` Segher Boessenkool
@ 2019-07-13 13:16   ` Segher Boessenkool
  2019-07-13 22:45     ` Masahiro Yamada
  0 siblings, 1 reply; 19+ messages in thread
From: Segher Boessenkool @ 2019-07-13 13:16 UTC (permalink / raw)
  To: Masahiro Yamada
  Cc: Stephen Rothwell, linuxppc-dev, Paul Mackerras, linux-kernel,
	Nicholas Piggin

On Sat, Jul 13, 2019 at 07:47:44AM -0500, Segher Boessenkool wrote:
> On Sat, Jul 13, 2019 at 12:21:06PM +0900, Masahiro Yamada wrote:
> > The KBUILD_ARFLAGS addition in arch/powerpc/Makefile has never worked
> > in a useful way because it is always overridden by the following code
> > in the top Makefile:
> > 
> >   # use the deterministic mode of AR if available
> >   KBUILD_ARFLAGS := $(call ar-option,D)
> > 
> > The code in the top Makefile was added in 2011, by commit 40df759e2b9e
> > ("kbuild: Fix build with binutils <= 2.19").
> > 
> > The KBUILD_ARFLAGS addition for ppc has always been dead code from the
> > beginning.
> 
> That was added in 43c9127d94d6 to replace my 8995ac870273 from 2007.
> 
> Is it no longer supported to build a 64-bit kernel with a toolchain
> that defaults to 32-bit, or the other way around?  And with non-native
> toolchains (this one didn't run on Linux, even).

It was an --enable-targets=all toolchain, somewhat common for crosses,
if that matters.


Segher

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH] powerpc: remove meaningless KBUILD_ARFLAGS addition
  2019-07-13 13:16   ` Segher Boessenkool
@ 2019-07-13 22:45     ` Masahiro Yamada
  2019-07-13 23:54       ` Segher Boessenkool
  0 siblings, 1 reply; 19+ messages in thread
From: Masahiro Yamada @ 2019-07-13 22:45 UTC (permalink / raw)
  To: Segher Boessenkool
  Cc: Stephen Rothwell, linuxppc-dev, Paul Mackerras,
	Linux Kernel Mailing List, Nicholas Piggin

On Sat, Jul 13, 2019 at 10:17 PM Segher Boessenkool
<segher@kernel.crashing.org> wrote:
>
> On Sat, Jul 13, 2019 at 07:47:44AM -0500, Segher Boessenkool wrote:
> > On Sat, Jul 13, 2019 at 12:21:06PM +0900, Masahiro Yamada wrote:
> > > The KBUILD_ARFLAGS addition in arch/powerpc/Makefile has never worked
> > > in a useful way because it is always overridden by the following code
> > > in the top Makefile:
> > >
> > >   # use the deterministic mode of AR if available
> > >   KBUILD_ARFLAGS := $(call ar-option,D)
> > >
> > > The code in the top Makefile was added in 2011, by commit 40df759e2b9e
> > > ("kbuild: Fix build with binutils <= 2.19").
> > >
> > > The KBUILD_ARFLAGS addition for ppc has always been dead code from the
> > > beginning.
> >
> > That was added in 43c9127d94d6 to replace my 8995ac870273 from 2007.
> >
> > Is it no longer supported to build a 64-bit kernel with a toolchain
> > that defaults to 32-bit, or the other way around?  And with non-native
> > toolchains (this one didn't run on Linux, even).
>
> It was an --enable-targets=all toolchain, somewhat common for crosses,
> if that matters.


I always use the same toolchain
for compile-testing PPC32/64.

I have never been hit by the issue you mention.
Somebody would have reported it if it were still a problem.

Moreover, commit 43c9127d94d6
translated the environment variable "GNUTARGET"
into the command option "--target="

My powerpc-linux-ar does not know it:

powerpc-linux-ar: -t: No such file or directory




-- 
Best Regards
Masahiro Yamada

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH] powerpc: remove meaningless KBUILD_ARFLAGS addition
  2019-07-13 22:45     ` Masahiro Yamada
@ 2019-07-13 23:54       ` Segher Boessenkool
  2019-07-15  7:05         ` Michael Ellerman
  0 siblings, 1 reply; 19+ messages in thread
From: Segher Boessenkool @ 2019-07-13 23:54 UTC (permalink / raw)
  To: Masahiro Yamada
  Cc: Stephen Rothwell, linuxppc-dev, Paul Mackerras,
	Linux Kernel Mailing List, Nicholas Piggin

On Sun, Jul 14, 2019 at 07:45:15AM +0900, Masahiro Yamada wrote:
> On Sat, Jul 13, 2019 at 10:17 PM Segher Boessenkool
> <segher@kernel.crashing.org> wrote:
> > On Sat, Jul 13, 2019 at 07:47:44AM -0500, Segher Boessenkool wrote:
> > > On Sat, Jul 13, 2019 at 12:21:06PM +0900, Masahiro Yamada wrote:
> > > > The KBUILD_ARFLAGS addition in arch/powerpc/Makefile has never worked
> > > > in a useful way because it is always overridden by the following code
> > > > in the top Makefile:
> > > >
> > > >   # use the deterministic mode of AR if available
> > > >   KBUILD_ARFLAGS := $(call ar-option,D)
> > > >
> > > > The code in the top Makefile was added in 2011, by commit 40df759e2b9e
> > > > ("kbuild: Fix build with binutils <= 2.19").
> > > >
> > > > The KBUILD_ARFLAGS addition for ppc has always been dead code from the
> > > > beginning.
> > >
> > > That was added in 43c9127d94d6 to replace my 8995ac870273 from 2007.
> > >
> > > Is it no longer supported to build a 64-bit kernel with a toolchain
> > > that defaults to 32-bit, or the other way around?  And with non-native
> > > toolchains (this one didn't run on Linux, even).
> >
> > It was an --enable-targets=all toolchain, somewhat common for crosses,
> > if that matters.
> 
> I always use the same toolchain
> for compile-testing PPC32/64.
> 
> I have never been hit by the issue you mention.
> Somebody would have reported it if it were still a problem.

But did you use --enable-targets=all?

The problem was empty archives IIRC.  Not a problem anymore with thin
archives, maybe?

> Moreover, commit 43c9127d94d6
> translated the environment variable "GNUTARGET"
> into the command option "--target="
> 
> My powerpc-linux-ar does not know it:
> 
> powerpc-linux-ar: -t: No such file or directory

Yes, that is why I used the environment variable, all binutils work
with that.  There was no --target option in GNU ar before 2.22.


Segher

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH] powerpc: remove meaningless KBUILD_ARFLAGS addition
  2019-07-13 23:54       ` Segher Boessenkool
@ 2019-07-15  7:05         ` Michael Ellerman
  2019-07-15  7:29           ` Segher Boessenkool
  0 siblings, 1 reply; 19+ messages in thread
From: Michael Ellerman @ 2019-07-15  7:05 UTC (permalink / raw)
  To: Segher Boessenkool, Masahiro Yamada
  Cc: Stephen Rothwell, linuxppc-dev, Paul Mackerras,
	Linux Kernel Mailing List, Nicholas Piggin

Segher Boessenkool <segher@kernel.crashing.org> writes:
> On Sun, Jul 14, 2019 at 07:45:15AM +0900, Masahiro Yamada wrote:
>> On Sat, Jul 13, 2019 at 10:17 PM Segher Boessenkool
>> <segher@kernel.crashing.org> wrote:
>> > On Sat, Jul 13, 2019 at 07:47:44AM -0500, Segher Boessenkool wrote:
>> > > On Sat, Jul 13, 2019 at 12:21:06PM +0900, Masahiro Yamada wrote:
>> > > > The KBUILD_ARFLAGS addition in arch/powerpc/Makefile has never worked
>> > > > in a useful way because it is always overridden by the following code
>> > > > in the top Makefile:
>> > > >
>> > > >   # use the deterministic mode of AR if available
>> > > >   KBUILD_ARFLAGS := $(call ar-option,D)
>> > > >
>> > > > The code in the top Makefile was added in 2011, by commit 40df759e2b9e
>> > > > ("kbuild: Fix build with binutils <= 2.19").
>> > > >
>> > > > The KBUILD_ARFLAGS addition for ppc has always been dead code from the
>> > > > beginning.
>> > >
>> > > That was added in 43c9127d94d6 to replace my 8995ac870273 from 2007.
>> > >
>> > > Is it no longer supported to build a 64-bit kernel with a toolchain
>> > > that defaults to 32-bit, or the other way around?  And with non-native
>> > > toolchains (this one didn't run on Linux, even).
>> >
>> > It was an --enable-targets=all toolchain, somewhat common for crosses,
>> > if that matters.
>> 
>> I always use the same toolchain
>> for compile-testing PPC32/64.
>> 
>> I have never been hit by the issue you mention.
>> Somebody would have reported it if it were still a problem.
>
> But did you use --enable-targets=all?

I do. And I don't see any errors with this patch applied.

> The problem was empty archives IIRC.  Not a problem anymore with thin
> archives, maybe?

Maybe? Though I can't get it to break even before we switched to them.

>> Moreover, commit 43c9127d94d6
>> translated the environment variable "GNUTARGET"
>> into the command option "--target="
>> 
>> My powerpc-linux-ar does not know it:
>> 
>> powerpc-linux-ar: -t: No such file or directory
>
> Yes, that is why I used the environment variable, all binutils work
> with that.  There was no --target option in GNU ar before 2.22.

Yeah, we're not very good at testing with really old binutils, so I
guess we broke that.

I'm inclined to merge this, it doesn't seem to break anything, and it
fixes using --target on old binutils that don't have it.

cheers

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH] powerpc: remove meaningless KBUILD_ARFLAGS addition
  2019-07-15  7:05         ` Michael Ellerman
@ 2019-07-15  7:29           ` Segher Boessenkool
  2019-07-15 12:03             ` Masahiro Yamada
  2019-07-16 12:15             ` Michael Ellerman
  0 siblings, 2 replies; 19+ messages in thread
From: Segher Boessenkool @ 2019-07-15  7:29 UTC (permalink / raw)
  To: Michael Ellerman
  Cc: Stephen Rothwell, Linux Kernel Mailing List, Nicholas Piggin,
	Masahiro Yamada, Paul Mackerras, linuxppc-dev

On Mon, Jul 15, 2019 at 05:05:34PM +1000, Michael Ellerman wrote:
> Segher Boessenkool <segher@kernel.crashing.org> writes:
> > Yes, that is why I used the environment variable, all binutils work
> > with that.  There was no --target option in GNU ar before 2.22.
> 
> Yeah, we're not very good at testing with really old binutils, so I
> guess we broke that.
> 
> I'm inclined to merge this, it doesn't seem to break anything, and it
> fixes using --target on old binutils that don't have it.

But we don't set the target any other way either.  I don't think this
will work with a 32-bit toolchain (default target 32 bit) and a 64-bit
kernel, or the other way around.

Then again, does that work at *all* nowadays?  Do we even consider that
important, *should* it work?


Segher

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH] powerpc: remove meaningless KBUILD_ARFLAGS addition
  2019-07-15  7:29           ` Segher Boessenkool
@ 2019-07-15 12:03             ` Masahiro Yamada
  2019-07-15 18:16               ` Segher Boessenkool
  2019-07-16 12:15             ` Michael Ellerman
  1 sibling, 1 reply; 19+ messages in thread
From: Masahiro Yamada @ 2019-07-15 12:03 UTC (permalink / raw)
  To: Segher Boessenkool
  Cc: Stephen Rothwell, Linux Kernel Mailing List, Nicholas Piggin,
	Paul Mackerras, linuxppc-dev

On Mon, Jul 15, 2019 at 4:30 PM Segher Boessenkool
<segher@kernel.crashing.org> wrote:
>
> On Mon, Jul 15, 2019 at 05:05:34PM +1000, Michael Ellerman wrote:
> > Segher Boessenkool <segher@kernel.crashing.org> writes:
> > > Yes, that is why I used the environment variable, all binutils work
> > > with that.  There was no --target option in GNU ar before 2.22.

I use binutils 2.30
It does not understand --target option.

$ powerpc-linux-ar --version
GNU ar (GNU Binutils) 2.30
Copyright (C) 2018 Free Software Foundation, Inc.
This program is free software; you may redistribute it under the terms of
the GNU General Public License version 3 or (at your option) any later version.
This program has absolutely no warranty.

If I give --target=elf$(BITS)-$(GNUTARGET) option, I see this:
powerpc-linux-ar: -t: No such file or directory



> > Yeah, we're not very good at testing with really old binutils, so I
> > guess we broke that.
> >
> > I'm inclined to merge this, it doesn't seem to break anything, and it
> > fixes using --target on old binutils that don't have it.
>
> But we don't set the target any other way either.  I don't think this
> will work with a 32-bit toolchain (default target 32 bit) and a 64-bit
> kernel, or the other way around.
>
> Then again, does that work at *all* nowadays?  Do we even consider that
> important, *should* it work?


Let me confirm if I understood this discussion.


[1] KBUILD_ARFLAGS += --target=elf$(BITS)-$(GNUTARGET)
    is pointless since it is always overridden by another
    KBUILD_ARFLAGS assignment.

[2] If we stop overriding it, it would cause build errors.
    So, --target is not only useless, but it is rather harmful.


So, we all agreed with this patch, right?


We are discussing whether or not to revive
GNUTARGET=elf$(BITS)-$(GNUTARGET)
in a *separate* patch, correct?


-- 
Best Regards
Masahiro Yamada

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH] powerpc: remove meaningless KBUILD_ARFLAGS addition
  2019-07-15 12:03             ` Masahiro Yamada
@ 2019-07-15 18:16               ` Segher Boessenkool
  2019-07-16  7:14                 ` Masahiro Yamada
  0 siblings, 1 reply; 19+ messages in thread
From: Segher Boessenkool @ 2019-07-15 18:16 UTC (permalink / raw)
  To: Masahiro Yamada
  Cc: Stephen Rothwell, Linux Kernel Mailing List, Nicholas Piggin,
	Paul Mackerras, linuxppc-dev

On Mon, Jul 15, 2019 at 09:03:46PM +0900, Masahiro Yamada wrote:
> On Mon, Jul 15, 2019 at 4:30 PM Segher Boessenkool
> <segher@kernel.crashing.org> wrote:
> >
> > On Mon, Jul 15, 2019 at 05:05:34PM +1000, Michael Ellerman wrote:
> > > Segher Boessenkool <segher@kernel.crashing.org> writes:
> > > > Yes, that is why I used the environment variable, all binutils work
> > > > with that.  There was no --target option in GNU ar before 2.22.
> 
> I use binutils 2.30
> It does not understand --target option.
> 
> $ powerpc-linux-ar --version
> GNU ar (GNU Binutils) 2.30
> Copyright (C) 2018 Free Software Foundation, Inc.
> This program is free software; you may redistribute it under the terms of
> the GNU General Public License version 3 or (at your option) any later version.
> This program has absolutely no warranty.
> 
> If I give --target=elf$(BITS)-$(GNUTARGET) option, I see this:
> powerpc-linux-ar: -t: No such file or directory

You need to provide a valid command line, like

$ powerpc-linux-ar tv smth.a --target=elf32-powerpc

ar is a bit weird.


Segher

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH] powerpc: remove meaningless KBUILD_ARFLAGS addition
  2019-07-15 18:16               ` Segher Boessenkool
@ 2019-07-16  7:14                 ` Masahiro Yamada
  0 siblings, 0 replies; 19+ messages in thread
From: Masahiro Yamada @ 2019-07-16  7:14 UTC (permalink / raw)
  To: Segher Boessenkool
  Cc: Stephen Rothwell, Linux Kernel Mailing List, Nicholas Piggin,
	Paul Mackerras, linuxppc-dev

On Tue, Jul 16, 2019 at 3:16 AM Segher Boessenkool
<segher@kernel.crashing.org> wrote:
>
> On Mon, Jul 15, 2019 at 09:03:46PM +0900, Masahiro Yamada wrote:
> > On Mon, Jul 15, 2019 at 4:30 PM Segher Boessenkool
> > <segher@kernel.crashing.org> wrote:
> > >
> > > On Mon, Jul 15, 2019 at 05:05:34PM +1000, Michael Ellerman wrote:
> > > > Segher Boessenkool <segher@kernel.crashing.org> writes:
> > > > > Yes, that is why I used the environment variable, all binutils work
> > > > > with that.  There was no --target option in GNU ar before 2.22.
> >
> > I use binutils 2.30
> > It does not understand --target option.
> >
> > $ powerpc-linux-ar --version
> > GNU ar (GNU Binutils) 2.30
> > Copyright (C) 2018 Free Software Foundation, Inc.
> > This program is free software; you may redistribute it under the terms of
> > the GNU General Public License version 3 or (at your option) any later version.
> > This program has absolutely no warranty.
> >
> > If I give --target=elf$(BITS)-$(GNUTARGET) option, I see this:
> > powerpc-linux-ar: -t: No such file or directory
>
> You need to provide a valid command line, like
>
> $ powerpc-linux-ar tv smth.a --target=elf32-powerpc
>
> ar is a bit weird.


Ah, I see!

I had missed the space being required.

Since I cannot test old binutils,
I will leave this to ppc people.






--
Best Regards
Masahiro Yamada

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH] powerpc: remove meaningless KBUILD_ARFLAGS addition
  2019-07-15  7:29           ` Segher Boessenkool
  2019-07-15 12:03             ` Masahiro Yamada
@ 2019-07-16 12:15             ` Michael Ellerman
  2019-07-17 14:38               ` Segher Boessenkool
  1 sibling, 1 reply; 19+ messages in thread
From: Michael Ellerman @ 2019-07-16 12:15 UTC (permalink / raw)
  To: Segher Boessenkool
  Cc: Stephen Rothwell, Linux Kernel Mailing List, Nicholas Piggin,
	Masahiro Yamada, Paul Mackerras, linuxppc-dev

Segher Boessenkool <segher@kernel.crashing.org> writes:
> On Mon, Jul 15, 2019 at 05:05:34PM +1000, Michael Ellerman wrote:
>> Segher Boessenkool <segher@kernel.crashing.org> writes:
>> > Yes, that is why I used the environment variable, all binutils work
>> > with that.  There was no --target option in GNU ar before 2.22.
>> 
>> Yeah, we're not very good at testing with really old binutils, so I
>> guess we broke that.
>> 
>> I'm inclined to merge this, it doesn't seem to break anything, and it
>> fixes using --target on old binutils that don't have it.
>
> But we don't set the target any other way either.  I don't think this
> will work with a 32-bit toolchain (default target 32 bit) and a 64-bit
> kernel, or the other way around.

I think it does, but maybe I'm misunderstanding.

My test setup is:

  ~/linux$ export PATH=/home/toolchains/ppc/gcc-8-branch/powerpc-linux/bin/:$PATH
  ~/linux$ echo "int test(void) { return 2; }" > test.c
  ~/linux$ powerpc-linux-gcc -c test.c 
  ~/linux$ file test.o 
  test.o: ELF 32-bit MSB relocatable, PowerPC or cisco 4500, version 1 (SYSV), not stripped
  ~/linux$ make CROSS_COMPILE=powerpc-linux- -s ppc64le_defconfig
  ~/linux$ make CROSS_COMPILE=powerpc-linux- -s -j 320
  ~/linux$ echo $?
  0

And it's definitely calling ar with no flags, eg:

  rm -f init/built-in.a; powerpc-linux-ar rcSTPD init/built-in.a init/main.o init/version.o init/do_mounts.o init/do_mounts_rd.o init/do_mounts_initrd.o init/do_mounts_md.o init/initramfs.o init/init_task.o

So presumably at some point ar learnt to cope with objects that don't
match its default? (how do I ask it what its default is?)

> Then again, does that work at *all* nowadays?  Do we even consider that
> important, *should* it work?

Yes and yes. There were a lot of bugs in the kernel makefiles after we
added LE support which prevented a biarch/biendian compiler from working.
But now it does work and we want it to keep working because it means you
can have a single compiler for building 32-bit, 64-bit BE & 64-bit LE.

cheers

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH] powerpc: remove meaningless KBUILD_ARFLAGS addition
  2019-07-16 12:15             ` Michael Ellerman
@ 2019-07-17 14:38               ` Segher Boessenkool
  2019-07-17 15:19                 ` Masahiro Yamada
  0 siblings, 1 reply; 19+ messages in thread
From: Segher Boessenkool @ 2019-07-17 14:38 UTC (permalink / raw)
  To: Michael Ellerman
  Cc: Stephen Rothwell, Linux Kernel Mailing List, Nicholas Piggin,
	Masahiro Yamada, Paul Mackerras, linuxppc-dev

On Tue, Jul 16, 2019 at 10:15:47PM +1000, Michael Ellerman wrote:
> Segher Boessenkool <segher@kernel.crashing.org> writes:
> And it's definitely calling ar with no flags, eg:
> 
>   rm -f init/built-in.a; powerpc-linux-ar rcSTPD init/built-in.a init/main.o init/version.o init/do_mounts.o init/do_mounts_rd.o init/do_mounts_initrd.o init/do_mounts_md.o init/initramfs.o init/init_task.o

This uses thin archives.  Those will work fine.

The failing case was empty files IIRC, stuff created from no inputs.

> So presumably at some point ar learnt to cope with objects that don't
> match its default? (how do I ask it what its default is?)

The first supported target in the --help list is the default, I think?

$ powerpc-linux-ar --help
 [...]
powerpc-linux-ar: supported targets: elf32-powerpc aixcoff-rs6000 elf32-powerpcle ppcboot elf64-powerpc elf64-powerpcle elf64-little elf64-big elf32-little elf32-big plugin srec symbolsrec verilog tekhex binary ihex

$ powerpc64-linux-ar --help
 [...]
powerpc64-linux-ar: supported targets: elf64-powerpc elf64-powerpcle elf32-powerpc elf32-powerpcle aixcoff-rs6000 aixcoff64-rs6000 aix5coff64-rs6000 elf64-little elf64-big elf32-little elf32-big plugin srec symbolsrec verilog tekhex binary ihex

$ powerpc64le-linux-ar --help
 [...]
powerpc64le-linux-ar: supported targets: elf64-powerpcle elf64-powerpc elf32-powerpcle elf32-powerpc aixcoff-rs6000 aixcoff64-rs6000 aix5coff64-rs6000 elf64-little elf64-big elf32-little elf32-big plugin srec symbolsrec verilog tekhex binary ihex

$ powerpcle-linux-ar --help
 [...]
powerpcle-linux-ar: supported targets: elf32-powerpcle aixcoff-rs6000 elf32-powerpc ppcboot elf64-powerpc elf64-powerpcle elf64-little elf64-big elf32-little elf32-big plugin srec symbolsrec verilog tekhex binary ihex

> > Then again, does that work at *all* nowadays?  Do we even consider that
> > important, *should* it work?
> 
> Yes and yes. There were a lot of bugs in the kernel makefiles after we
> added LE support which prevented a biarch/biendian compiler from working.
> But now it does work and we want it to keep working because it means you
> can have a single compiler for building 32-bit, 64-bit BE & 64-bit LE.

Good to hear :-)


Segher

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH] powerpc: remove meaningless KBUILD_ARFLAGS addition
  2019-07-17 14:38               ` Segher Boessenkool
@ 2019-07-17 15:19                 ` Masahiro Yamada
  2019-07-17 16:46                   ` Segher Boessenkool
  0 siblings, 1 reply; 19+ messages in thread
From: Masahiro Yamada @ 2019-07-17 15:19 UTC (permalink / raw)
  To: Segher Boessenkool
  Cc: Stephen Rothwell, Linux Kernel Mailing List, Nicholas Piggin,
	Paul Mackerras, linuxppc-dev

On Wed, Jul 17, 2019 at 11:38 PM Segher Boessenkool
<segher@kernel.crashing.org> wrote:
>
> On Tue, Jul 16, 2019 at 10:15:47PM +1000, Michael Ellerman wrote:
> > Segher Boessenkool <segher@kernel.crashing.org> writes:
> > And it's definitely calling ar with no flags, eg:
> >
> >   rm -f init/built-in.a; powerpc-linux-ar rcSTPD init/built-in.a init/main.o init/version.o init/do_mounts.o init/do_mounts_rd.o init/do_mounts_initrd.o init/do_mounts_md.o init/initramfs.o init/init_task.o
>
> This uses thin archives.  Those will work fine.
>
> The failing case was empty files IIRC, stuff created from no inputs.

Actually, empty files are created everywhere.
I do not see any problems whether the target is 32-bit or 64-bit.


Just try allnoconfig, for example.
You can apply the following to debug.


diff --git a/scripts/Makefile.build b/scripts/Makefile.build
index be38198d98b2..9d6b30e50663 100644
--- a/scripts/Makefile.build
+++ b/scripts/Makefile.build
@@ -413,6 +413,7 @@ quiet_cmd_ar_builtin = AR      $@
       cmd_ar_builtin = rm -f $@; $(AR) rcSTP$(KBUILD_ARFLAGS) $@
$(real-prereqs)

 $(builtin-target): $(real-obj-y) FORCE
+       @$(if $(real-prereqs),,echo creating empty archive: $@)
        $(call if_changed,ar_builtin)

 targets += $(builtin-target)

The following are all empty files created from no input.

creating empty archive: usr/built-in.a
creating empty archive: arch/powerpc/crypto/built-in.a
creating empty archive: arch/powerpc/math-emu/built-in.a
  CHK     include/generated/compile.h
creating empty archive: arch/powerpc/net/built-in.a
creating empty archive: arch/powerpc/platforms/built-in.a
creating empty archive: arch/powerpc/sysdev/built-in.a
creating empty archive: certs/built-in.a
creating empty archive: ipc/built-in.a
creating empty archive: block/built-in.a
creating empty archive: drivers/amba/built-in.a
creating empty archive: sound/built-in.a
creating empty archive: drivers/auxdisplay/built-in.a
creating empty archive: crypto/built-in.a
creating empty archive: drivers/block/built-in.a
creating empty archive: net/built-in.a
creating empty archive: drivers/bus/built-in.a
creating empty archive: drivers/cdrom/built-in.a
creating empty archive: drivers/char/ipmi/built-in.a
creating empty archive: arch/powerpc/kernel/trace/built-in.a
creating empty archive: drivers/char/agp/built-in.a
creating empty archive: virt/lib/built-in.a
creating empty archive: drivers/clk/actions/built-in.a
creating empty archive: drivers/clocksource/built-in.a
creating empty archive: drivers/clk/analogbits/built-in.a
creating empty archive: drivers/firewire/built-in.a
creating empty archive: drivers/clk/bcm/built-in.a
creating empty archive: drivers/clk/imx/built-in.a
creating empty archive: drivers/clk/imgtec/built-in.a
creating empty archive: drivers/clk/ingenic/built-in.a
creating empty archive: drivers/clk/mediatek/built-in.a
creating empty archive: drivers/gpu/drm/arm/built-in.a
creating empty archive: drivers/clk/mvebu/built-in.a
creating empty archive: drivers/firmware/broadcom/built-in.a
creating empty archive: drivers/firmware/imx/built-in.a
creating empty archive: drivers/gpu/drm/bridge/synopsys/built-in.a
creating empty archive: drivers/clk/renesas/built-in.a
creating empty archive: drivers/firmware/meson/built-in.a
creating empty archive: drivers/firmware/psci/built-in.a
creating empty archive: drivers/clk/ti/built-in.a
creating empty archive: drivers/gpu/drm/hisilicon/built-in.a
creating empty archive: drivers/firmware/tegra/built-in.a
creating empty archive: drivers/gpu/drm/i2c/built-in.a
creating empty archive: drivers/gpu/drm/omapdrm/displays/built-in.a
creating empty archive: drivers/gpu/drm/omapdrm/dss/built-in.a
creating empty archive: drivers/firmware/xilinx/built-in.a
creating empty archive: drivers/gpu/drm/panel/built-in.a
creating empty archive: drivers/gpu/drm/rcar-du/built-in.a
creating empty archive: drivers/hwtracing/intel_th/built-in.a
creating empty archive: kernel/livepatch/built-in.a
creating empty archive: drivers/gpu/drm/tilcdc/built-in.a
creating empty archive: drivers/base/firmware_loader/builtin/built-in.a
creating empty archive: drivers/base/power/built-in.a
creating empty archive: drivers/gpu/vga/built-in.a
creating empty archive: drivers/base/test/built-in.a
creating empty archive: drivers/i2c/algos/built-in.a
creating empty archive: drivers/i3c/built-in.a
creating empty archive: drivers/i2c/busses/built-in.a
creating empty archive: drivers/idle/built-in.a
creating empty archive: drivers/i2c/muxes/built-in.a
creating empty archive: drivers/iommu/built-in.a
creating empty archive: fs/devpts/built-in.a
creating empty archive: drivers/macintosh/built-in.a
creating empty archive: fs/notify/dnotify/built-in.a
creating empty archive: fs/notify/fanotify/built-in.a
creating empty archive: fs/notify/inotify/built-in.a
creating empty archive: drivers/media/common/b2c2/built-in.a
creating empty archive: drivers/media/common/saa7146/built-in.a
creating empty archive: fs/quota/built-in.a
creating empty archive: drivers/media/firewire/built-in.a
creating empty archive: drivers/media/i2c/built-in.a
creating empty archive: drivers/mfd/built-in.a
creating empty archive: drivers/media/common/siano/built-in.a
creating empty archive: drivers/media/common/v4l2-tpg/built-in.a
creating empty archive: drivers/misc/cardreader/built-in.a
creating empty archive: drivers/media/mmc/siano/built-in.a
creating empty archive: drivers/media/common/videobuf2/built-in.a
creating empty archive: drivers/media/pci/b2c2/built-in.a
creating empty archive: drivers/misc/cb710/built-in.a
creating empty archive: drivers/misc/eeprom/built-in.a
creating empty archive: drivers/media/pci/ddbridge/built-in.a
creating empty archive: drivers/misc/lis3lv02d/built-in.a
creating empty archive: drivers/media/platform/cros-ec-cec/built-in.a
creating empty archive: drivers/media/pci/dm1105/built-in.a
creating empty archive: lib/crypto/built-in.a
creating empty archive: drivers/media/rc/keymaps/built-in.a
creating empty archive: drivers/misc/mic/bus/built-in.a
creating empty archive: drivers/misc/ti-st/built-in.a
creating empty archive: drivers/media/platform/davinci/built-in.a
creating empty archive: drivers/media/pci/intel/ipu3/built-in.a
creating empty archive: drivers/media/platform/meson/built-in.a
creating empty archive: drivers/media/platform/omap/built-in.a
creating empty archive: drivers/media/pci/mantis/built-in.a
creating empty archive: drivers/mmc/built-in.a
creating empty archive: drivers/media/pci/netup_unidvb/built-in.a
creating empty archive: drivers/nfc/built-in.a
creating empty archive: drivers/media/pci/ngene/built-in.a
creating empty archive: drivers/net/dsa/b53/built-in.a
creating empty archive: drivers/media/pci/pluto2/built-in.a
creating empty archive: drivers/media/platform/stm32/built-in.a
creating empty archive: drivers/nvme/host/built-in.a
creating empty archive: drivers/nvme/target/built-in.a
creating empty archive: drivers/media/spi/built-in.a
creating empty archive: drivers/net/dsa/microchip/built-in.a
creating empty archive: drivers/net/dsa/mv88e6xxx/built-in.a
creating empty archive: drivers/media/pci/pt1/built-in.a
creating empty archive: drivers/media/tuners/built-in.a
creating empty archive: drivers/media/pci/pt3/built-in.a
creating empty archive: drivers/media/usb/b2c2/built-in.a
creating empty archive: drivers/net/phy/built-in.a
creating empty archive: drivers/media/usb/dvb-usb/built-in.a
creating empty archive: drivers/media/pci/saa7146/built-in.a
creating empty archive: drivers/net/dsa/sja1105/built-in.a
creating empty archive: drivers/media/usb/s2255/built-in.a
creating empty archive: drivers/media/pci/smipcie/built-in.a
creating empty archive: drivers/media/usb/dvb-usb-v2/built-in.a
creating empty archive: drivers/pci/controller/dwc/built-in.a
creating empty archive: drivers/media/usb/siano/built-in.a
creating empty archive: drivers/media/pci/ttpci/built-in.a
creating empty archive: drivers/pci/switch/built-in.a
creating empty archive: drivers/media/usb/stkwebcam/built-in.a
creating empty archive: drivers/platform/built-in.a
creating empty archive: drivers/power/built-in.a
creating empty archive: drivers/media/usb/ttusb-dec/built-in.a
creating empty archive: drivers/media/usb/zr364xx/built-in.a
creating empty archive: drivers/media/usb/ttusb-budget/built-in.a
creating empty archive: drivers/ptp/built-in.a
creating empty archive: drivers/pwm/built-in.a
creating empty archive: drivers/scsi/built-in.a
creating empty archive: drivers/usb/built-in.a
creating empty archive: drivers/soc/amlogic/built-in.a
creating empty archive: drivers/soc/fsl/built-in.a
creating empty archive: drivers/tty/ipwireless/built-in.a
creating empty archive: drivers/soc/bcm/built-in.a
creating empty archive: drivers/soc/mediatek/built-in.a
creating empty archive: drivers/tty/serial/built-in.a
creating empty archive: drivers/video/backlight/built-in.a
creating empty archive: drivers/tty/vt/built-in.a
creating empty archive: drivers/soc/qcom/built-in.a
creating empty archive: drivers/soc/renesas/built-in.a
creating empty archive: drivers/soc/sunxi/built-in.a
creating empty archive: drivers/soc/ti/built-in.a
creating empty archive: drivers/video/fbdev/core/built-in.a
creating empty archive: drivers/soc/xilinx/built-in.a
creating empty archive: drivers/video/fbdev/omap2/omapfb/displays/built-in.a
creating empty archive: drivers/video/fbdev/omap2/omapfb/dss/built-in.a



BTW, your commit 8995ac8702737147115e1c75879a1a2d75627b9e
dates back to 2008.

At that time, thin archive was not used.


--
Best Regards
Masahiro Yamada

^ permalink raw reply related	[flat|nested] 19+ messages in thread

* Re: [PATCH] powerpc: remove meaningless KBUILD_ARFLAGS addition
  2019-07-17 15:19                 ` Masahiro Yamada
@ 2019-07-17 16:46                   ` Segher Boessenkool
  2019-07-18  2:19                     ` Masahiro Yamada
  0 siblings, 1 reply; 19+ messages in thread
From: Segher Boessenkool @ 2019-07-17 16:46 UTC (permalink / raw)
  To: Masahiro Yamada
  Cc: Stephen Rothwell, Linux Kernel Mailing List, Nicholas Piggin,
	Paul Mackerras, linuxppc-dev

On Thu, Jul 18, 2019 at 12:19:36AM +0900, Masahiro Yamada wrote:
> On Wed, Jul 17, 2019 at 11:38 PM Segher Boessenkool
> <segher@kernel.crashing.org> wrote:
> >
> > On Tue, Jul 16, 2019 at 10:15:47PM +1000, Michael Ellerman wrote:
> > > Segher Boessenkool <segher@kernel.crashing.org> writes:
> > > And it's definitely calling ar with no flags, eg:
> > >
> > >   rm -f init/built-in.a; powerpc-linux-ar rcSTPD init/built-in.a init/main.o init/version.o init/do_mounts.o init/do_mounts_rd.o init/do_mounts_initrd.o init/do_mounts_md.o init/initramfs.o init/init_task.o
> >
> > This uses thin archives.  Those will work fine.
> >
> > The failing case was empty files IIRC, stuff created from no inputs.
> 
> Actually, empty files are created everywhere.

>        cmd_ar_builtin = rm -f $@; $(AR) rcSTP$(KBUILD_ARFLAGS) $@
> $(real-prereqs)

You use thin archives.

Does every config use thin archives always nowadays?

> BTW, your commit 8995ac8702737147115e1c75879a1a2d75627b9e
> dates back to 2008.
> 
> At that time, thin archive was not used.

Yes, I know.  This isn't about built-in.[oa], it is about *other*
archives we at least *used to* create.  If we *know* we do not anymore,
then this workaround can of course be removed (and good riddance).

If ar creates an archive file (a real one, not a thin archive), and it
has no input files, it uses its default object format as destination
format, if it isn't told to use something else.  And that doesn't work,
it needs to use some format compatible with what that archive later is
linked with.


Segher

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH] powerpc: remove meaningless KBUILD_ARFLAGS addition
  2019-07-17 16:46                   ` Segher Boessenkool
@ 2019-07-18  2:19                     ` Masahiro Yamada
  2019-07-18 20:46                       ` Segher Boessenkool
  0 siblings, 1 reply; 19+ messages in thread
From: Masahiro Yamada @ 2019-07-18  2:19 UTC (permalink / raw)
  To: Segher Boessenkool
  Cc: Stephen Rothwell, Linux Kernel Mailing List, Nicholas Piggin,
	Paul Mackerras, linuxppc-dev

On Thu, Jul 18, 2019 at 1:46 AM Segher Boessenkool
<segher@kernel.crashing.org> wrote:
>
> On Thu, Jul 18, 2019 at 12:19:36AM +0900, Masahiro Yamada wrote:
> > On Wed, Jul 17, 2019 at 11:38 PM Segher Boessenkool
> > <segher@kernel.crashing.org> wrote:
> > >
> > > On Tue, Jul 16, 2019 at 10:15:47PM +1000, Michael Ellerman wrote:
> > > > Segher Boessenkool <segher@kernel.crashing.org> writes:
> > > > And it's definitely calling ar with no flags, eg:
> > > >
> > > >   rm -f init/built-in.a; powerpc-linux-ar rcSTPD init/built-in.a init/main.o init/version.o init/do_mounts.o init/do_mounts_rd.o init/do_mounts_initrd.o init/do_mounts_md.o init/initramfs.o init/init_task.o
> > >
> > > This uses thin archives.  Those will work fine.
> > >
> > > The failing case was empty files IIRC, stuff created from no inputs.
> >
> > Actually, empty files are created everywhere.
>
> >        cmd_ar_builtin = rm -f $@; $(AR) rcSTP$(KBUILD_ARFLAGS) $@
> > $(real-prereqs)
>
> You use thin archives.
>
> Does every config use thin archives always nowadays?

Kbuild always uses thin archives as far as vmlinux is concerned.

But, there are some other call-sites.

masahiro@pug:~/ref/linux$ git grep  '$(AR)' -- :^Documentation :^tools
arch/powerpc/boot/Makefile:    BOOTAR := $(AR)
arch/unicore32/lib/Makefile:    $(Q)$(AR) p $(GNU_LIBC_A) $(notdir $@) > $@
arch/unicore32/lib/Makefile:    $(Q)$(AR) p $(GNU_LIBGCC_A) $(notdir $@) > $@
lib/raid6/test/Makefile:         $(AR) cq $@ $^
scripts/Kbuild.include:ar-option = $(call try-run, $(AR) rc$(1)
"$$TMP",$(1),$(2))
scripts/Makefile.build:      cmd_ar_builtin = rm -f $@; $(AR)
rcSTP$(KBUILD_ARFLAGS) $@ $(real-prereqs)
scripts/Makefile.lib:      cmd_ar = rm -f $@; $(AR)
rcsTP$(KBUILD_ARFLAGS) $@ $(real-prereqs)


Probably, you are interested in arch/powerpc/boot/Makefile.
This does not seem a thin archive.


> > BTW, your commit 8995ac8702737147115e1c75879a1a2d75627b9e
> > dates back to 2008.
> >
> > At that time, thin archive was not used.
>
> Yes, I know.  This isn't about built-in.[oa], it is about *other*
> archives we at least *used to* create.  If we *know* we do not anymore,
> then this workaround can of course be removed (and good riddance).

If it is not about built-in.[oa],
which archive are you talking about?

Can you pin-point the one?

masahiro@pug:~/ref/linux$ git log --oneline  -1
8995ac870273 (HEAD) [POWERPC] Specify GNUTARGET on $(AR) invocations
masahiro@pug:~/ref/linux$ git grep '(AR)'
Documentation/kbuild/makefiles.txt:     per-directory options to $(LD)
and $(AR).
arch/powerpc/Makefile:CROSS32AR := GNUTARGET=elf32-powerpc $(AR)
arch/powerpc/Makefile:override AR       := GNUTARGET=elf$(SZ)-powerpc $(AR)
drivers/md/raid6test/Makefile:   $(AR) cq $@ $^
scripts/Makefile.build:               rm -f $@; $(AR) rcs $@)
scripts/Makefile.build:cmd_link_l_target = rm -f $@; $(AR)
$(EXTRA_ARFLAGS) rcs $@ $(lib-y)
masahiro@pug:~/ref/linux$ git grep '(CROSS32AR)'
arch/powerpc/boot/Makefile:      cmd_bootar = $(CROSS32AR) -cr $@.$$$$
$(filter-out FORCE,$^); mv $@.$$$$ $@



> If ar creates an archive file (a real one, not a thin archive), and it
> has no input files, it uses its default object format as destination
> format, if it isn't told to use something else.  And that doesn't work,
> it needs to use some format compatible with what that archive later is
> linked with.

I compile-tested v4.10, which was before the thin-archive migration,
but I did not see any problem for building ppc32/64.

Whether or not it is a thin archive,
an empty archive is always 8-byte file.

masahiro@pug:~/ref/linux$ cat kernel/livepatch/built-in.o
!<arch>

Is there a room for caring about the under-lying architecture?


-- 
Best Regards
Masahiro Yamada

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH] powerpc: remove meaningless KBUILD_ARFLAGS addition
  2019-07-18  2:19                     ` Masahiro Yamada
@ 2019-07-18 20:46                       ` Segher Boessenkool
  2019-07-19  3:39                         ` Michael Ellerman
  0 siblings, 1 reply; 19+ messages in thread
From: Segher Boessenkool @ 2019-07-18 20:46 UTC (permalink / raw)
  To: Masahiro Yamada
  Cc: Stephen Rothwell, Linux Kernel Mailing List, Nicholas Piggin,
	Paul Mackerras, linuxppc-dev

Hi!

On Thu, Jul 18, 2019 at 11:19:58AM +0900, Masahiro Yamada wrote:
> On Thu, Jul 18, 2019 at 1:46 AM Segher Boessenkool
> <segher@kernel.crashing.org> wrote:
> Kbuild always uses thin archives as far as vmlinux is concerned.
> 
> But, there are some other call-sites.
> 
> masahiro@pug:~/ref/linux$ git grep  '$(AR)' -- :^Documentation :^tools
> arch/powerpc/boot/Makefile:    BOOTAR := $(AR)
> arch/unicore32/lib/Makefile:    $(Q)$(AR) p $(GNU_LIBC_A) $(notdir $@) > $@
> arch/unicore32/lib/Makefile:    $(Q)$(AR) p $(GNU_LIBGCC_A) $(notdir $@) > $@
> lib/raid6/test/Makefile:         $(AR) cq $@ $^
> scripts/Kbuild.include:ar-option = $(call try-run, $(AR) rc$(1)
> "$$TMP",$(1),$(2))
> scripts/Makefile.build:      cmd_ar_builtin = rm -f $@; $(AR)
> rcSTP$(KBUILD_ARFLAGS) $@ $(real-prereqs)
> scripts/Makefile.lib:      cmd_ar = rm -f $@; $(AR)
> rcsTP$(KBUILD_ARFLAGS) $@ $(real-prereqs)
> 
> Probably, you are interested in arch/powerpc/boot/Makefile.

That one seems fine actually.  The raid6 one I don't know.


My original commit message was

    Without this, some versions of GNU ar fail to create
    an archive index if the object files it is packing
    together are of a different object format than ar's
    default format (for example, binutils compiled to
    default to 64-bit, with 32-bit objects).

but I cannot reproduce the problem anymore.  Shortly after my patch the
thin archive code happened to binutils, and that overhauled some other
things, which might have fixed it already?

> > Yes, I know.  This isn't about built-in.[oa], it is about *other*
> > archives we at least *used to* create.  If we *know* we do not anymore,
> > then this workaround can of course be removed (and good riddance).
> 
> If it is not about built-in.[oa],
> which archive are you talking about?
> 
> Can you pin-point the one?

No, not anymore.  Lost in the mists of time, I guess?  I think we'll
just have to file it as "it seems to work fine now".

Thank you (and everyone else) for the time looking at this!


Segher

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH] powerpc: remove meaningless KBUILD_ARFLAGS addition
  2019-07-18 20:46                       ` Segher Boessenkool
@ 2019-07-19  3:39                         ` Michael Ellerman
  2019-08-19  4:25                           ` Masahiro Yamada
  0 siblings, 1 reply; 19+ messages in thread
From: Michael Ellerman @ 2019-07-19  3:39 UTC (permalink / raw)
  To: Segher Boessenkool, Masahiro Yamada
  Cc: Stephen Rothwell, linuxppc-dev, Paul Mackerras,
	Linux Kernel Mailing List, Nicholas Piggin

Segher Boessenkool <segher@kernel.crashing.org> writes:
> On Thu, Jul 18, 2019 at 11:19:58AM +0900, Masahiro Yamada wrote:
>> On Thu, Jul 18, 2019 at 1:46 AM Segher Boessenkool
>> <segher@kernel.crashing.org> wrote:
>> Kbuild always uses thin archives as far as vmlinux is concerned.
>> 
>> But, there are some other call-sites.
>> 
>> masahiro@pug:~/ref/linux$ git grep  '$(AR)' -- :^Documentation :^tools
>> arch/powerpc/boot/Makefile:    BOOTAR := $(AR)
>> arch/unicore32/lib/Makefile:    $(Q)$(AR) p $(GNU_LIBC_A) $(notdir $@) > $@
>> arch/unicore32/lib/Makefile:    $(Q)$(AR) p $(GNU_LIBGCC_A) $(notdir $@) > $@
>> lib/raid6/test/Makefile:         $(AR) cq $@ $^
>> scripts/Kbuild.include:ar-option = $(call try-run, $(AR) rc$(1)
>> "$$TMP",$(1),$(2))
>> scripts/Makefile.build:      cmd_ar_builtin = rm -f $@; $(AR)
>> rcSTP$(KBUILD_ARFLAGS) $@ $(real-prereqs)
>> scripts/Makefile.lib:      cmd_ar = rm -f $@; $(AR)
>> rcsTP$(KBUILD_ARFLAGS) $@ $(real-prereqs)
>> 
>> Probably, you are interested in arch/powerpc/boot/Makefile.
>
> That one seems fine actually.  The raid6 one I don't know.
>
>
> My original commit message was
>
>     Without this, some versions of GNU ar fail to create
>     an archive index if the object files it is packing
>     together are of a different object format than ar's
>     default format (for example, binutils compiled to
>     default to 64-bit, with 32-bit objects).
>
> but I cannot reproduce the problem anymore.  Shortly after my patch the
> thin archive code happened to binutils, and that overhauled some other
> things, which might have fixed it already?
>
>> > Yes, I know.  This isn't about built-in.[oa], it is about *other*
>> > archives we at least *used to* create.  If we *know* we do not anymore,
>> > then this workaround can of course be removed (and good riddance).
>> 
>> If it is not about built-in.[oa],
>> which archive are you talking about?
>> 
>> Can you pin-point the one?
>
> No, not anymore.  Lost in the mists of time, I guess?  I think we'll
> just have to file it as "it seems to work fine now".

Yeah I think so. If someone finds a case it breaks we can fix it then.

> Thank you (and everyone else) for the time looking at this!

Likewise.

cheers

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH] powerpc: remove meaningless KBUILD_ARFLAGS addition
  2019-07-19  3:39                         ` Michael Ellerman
@ 2019-08-19  4:25                           ` Masahiro Yamada
  0 siblings, 0 replies; 19+ messages in thread
From: Masahiro Yamada @ 2019-08-19  4:25 UTC (permalink / raw)
  To: Michael Ellerman
  Cc: Stephen Rothwell, Linux Kernel Mailing List, Nicholas Piggin,
	Paul Mackerras, linuxppc-dev

Hi,

On Fri, Jul 19, 2019 at 12:43 PM Michael Ellerman <mpe@ellerman.id.au> wrote:
>
> Segher Boessenkool <segher@kernel.crashing.org> writes:
> > On Thu, Jul 18, 2019 at 11:19:58AM +0900, Masahiro Yamada wrote:
> >> On Thu, Jul 18, 2019 at 1:46 AM Segher Boessenkool
> >> <segher@kernel.crashing.org> wrote:
> >> Kbuild always uses thin archives as far as vmlinux is concerned.
> >>
> >> But, there are some other call-sites.
> >>
> >> masahiro@pug:~/ref/linux$ git grep  '$(AR)' -- :^Documentation :^tools
> >> arch/powerpc/boot/Makefile:    BOOTAR := $(AR)
> >> arch/unicore32/lib/Makefile:    $(Q)$(AR) p $(GNU_LIBC_A) $(notdir $@) > $@
> >> arch/unicore32/lib/Makefile:    $(Q)$(AR) p $(GNU_LIBGCC_A) $(notdir $@) > $@
> >> lib/raid6/test/Makefile:         $(AR) cq $@ $^
> >> scripts/Kbuild.include:ar-option = $(call try-run, $(AR) rc$(1)
> >> "$$TMP",$(1),$(2))
> >> scripts/Makefile.build:      cmd_ar_builtin = rm -f $@; $(AR)
> >> rcSTP$(KBUILD_ARFLAGS) $@ $(real-prereqs)
> >> scripts/Makefile.lib:      cmd_ar = rm -f $@; $(AR)
> >> rcsTP$(KBUILD_ARFLAGS) $@ $(real-prereqs)
> >>
> >> Probably, you are interested in arch/powerpc/boot/Makefile.
> >
> > That one seems fine actually.  The raid6 one I don't know.
> >
> >
> > My original commit message was
> >
> >     Without this, some versions of GNU ar fail to create
> >     an archive index if the object files it is packing
> >     together are of a different object format than ar's
> >     default format (for example, binutils compiled to
> >     default to 64-bit, with 32-bit objects).
> >
> > but I cannot reproduce the problem anymore.  Shortly after my patch the
> > thin archive code happened to binutils, and that overhauled some other
> > things, which might have fixed it already?
> >
> >> > Yes, I know.  This isn't about built-in.[oa], it is about *other*
> >> > archives we at least *used to* create.  If we *know* we do not anymore,
> >> > then this workaround can of course be removed (and good riddance).
> >>
> >> If it is not about built-in.[oa],
> >> which archive are you talking about?
> >>
> >> Can you pin-point the one?
> >
> > No, not anymore.  Lost in the mists of time, I guess?  I think we'll
> > just have to file it as "it seems to work fine now".
>
> Yeah I think so. If someone finds a case it breaks we can fix it then.
>
> > Thank you (and everyone else) for the time looking at this!
>
> Likewise.
>
> cheers


So, we agreed to apply this patch, right?

Please let me know if there is some improvement that should be get done.


-- 
Best Regards
Masahiro Yamada

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH] powerpc: remove meaningless KBUILD_ARFLAGS addition
  2019-07-13  3:21 [PATCH] powerpc: remove meaningless KBUILD_ARFLAGS addition Masahiro Yamada
  2019-07-13 12:47 ` Segher Boessenkool
@ 2019-08-28  4:24 ` Michael Ellerman
  1 sibling, 0 replies; 19+ messages in thread
From: Michael Ellerman @ 2019-08-28  4:24 UTC (permalink / raw)
  To: Masahiro Yamada, linuxppc-dev
  Cc: Stephen Rothwell, Paul Mackerras, linux-kernel, Nicholas Piggin,
	Masahiro Yamada

On Sat, 2019-07-13 at 03:21:06 UTC, Masahiro Yamada wrote:
> The KBUILD_ARFLAGS addition in arch/powerpc/Makefile has never worked
> in a useful way because it is always overridden by the following code
> in the top Makefile:
> 
>   # use the deterministic mode of AR if available
>   KBUILD_ARFLAGS := $(call ar-option,D)
> 
> The code in the top Makefile was added in 2011, by commit 40df759e2b9e
> ("kbuild: Fix build with binutils <= 2.19").
> 
> The KBUILD_ARFLAGS addition for ppc has always been dead code from the
> beginning.
> 
> Nobody has reported a problem since 43c9127d94d6 ("powerpc: Add option
> to use thin archives"), so this code was unneeded.
> 
> Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>

Applied to powerpc next, thanks.

https://git.kernel.org/powerpc/c/56347074c5307d7bca5db38eb2c764c64ae57514

cheers

^ permalink raw reply	[flat|nested] 19+ messages in thread

end of thread, other threads:[~2019-08-28  4:28 UTC | newest]

Thread overview: 19+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-07-13  3:21 [PATCH] powerpc: remove meaningless KBUILD_ARFLAGS addition Masahiro Yamada
2019-07-13 12:47 ` Segher Boessenkool
2019-07-13 13:16   ` Segher Boessenkool
2019-07-13 22:45     ` Masahiro Yamada
2019-07-13 23:54       ` Segher Boessenkool
2019-07-15  7:05         ` Michael Ellerman
2019-07-15  7:29           ` Segher Boessenkool
2019-07-15 12:03             ` Masahiro Yamada
2019-07-15 18:16               ` Segher Boessenkool
2019-07-16  7:14                 ` Masahiro Yamada
2019-07-16 12:15             ` Michael Ellerman
2019-07-17 14:38               ` Segher Boessenkool
2019-07-17 15:19                 ` Masahiro Yamada
2019-07-17 16:46                   ` Segher Boessenkool
2019-07-18  2:19                     ` Masahiro Yamada
2019-07-18 20:46                       ` Segher Boessenkool
2019-07-19  3:39                         ` Michael Ellerman
2019-08-19  4:25                           ` Masahiro Yamada
2019-08-28  4:24 ` Michael Ellerman

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).