Patchwork : powerpc: Fix build breakage due to incorrect location of autoconf.h

login
register
mail settings
Submitter Joakim Tjernlund
Date Jan. 12, 2010, 11:59 a.m.
Message ID <OFF0BABDB9.D573B5F9-ONC12576A9.0041BA69-C12576A9.0041D76A@transmode.se>
Download mbox | patch
Permalink /patch/42708/
State Superseded
Headers show

Comments

Joakim Tjernlund - Jan. 12, 2010, 11:59 a.m.
>
> Hi Anton,
>
> On Tue, 12 Jan 2010 13:21:51 +1100 Anton Blanchard <anton@samba.org> wrote:
> >
> > commit ac4c2a3bbe5db5fc570b1d0ee1e474db7cb22585 (zlib: optimize inffast when
> > copying direct from output) referenced include/linux/autoconf.h which
> > is now called include/generated/autoconf.h.
>
> Even with this fix, you cannot build with a separate object directory.
> See my other posting "linux-next: origin tree build failure" ...
> --

How does this work for you?

From 044f40d169bf5fe189d5cb058f56b7cd72675ca4 Mon Sep 17 00:00:00 2001
From: Joakim Tjernlund <Joakim.Tjernlund@transmode.se>
Date: Tue, 12 Jan 2010 11:20:36 +0100
Subject: [PATCH] powerpc: Fix build breakage due to incorrect location of autoconf.h

commit ac4c2a3bbe5db5fc570b1d0ee1e474db7cb22585 (zlib: optimize inffast when
copying direct from output) referenced include/linux/autoconf.h which
is now called include/generated/autoconf.h.
Also, -I include paths needs to be prefixed with $(srctree)

Signed-off-by: Joakim Tjernlund <Joakim.Tjernlund@transmode.se>
---
 arch/powerpc/boot/Makefile |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

--
1.6.4.4
Kumar Gala - Jan. 13, 2010, 6:54 p.m.
On Jan 12, 2010, at 5:59 AM, Joakim Tjernlund wrote:

>> 
>> Hi Anton,
>> 
>> On Tue, 12 Jan 2010 13:21:51 +1100 Anton Blanchard <anton@samba.org> wrote:
>>> 
>>> commit ac4c2a3bbe5db5fc570b1d0ee1e474db7cb22585 (zlib: optimize inffast when
>>> copying direct from output) referenced include/linux/autoconf.h which
>>> is now called include/generated/autoconf.h.
>> 
>> Even with this fix, you cannot build with a separate object directory.
>> See my other posting "linux-next: origin tree build failure" ...
>> --
> 
> How does this work for you?
> 
>> From 044f40d169bf5fe189d5cb058f56b7cd72675ca4 Mon Sep 17 00:00:00 2001
> From: Joakim Tjernlund <Joakim.Tjernlund@transmode.se>
> Date: Tue, 12 Jan 2010 11:20:36 +0100
> Subject: [PATCH] powerpc: Fix build breakage due to incorrect location of autoconf.h
> 
> commit ac4c2a3bbe5db5fc570b1d0ee1e474db7cb22585 (zlib: optimize inffast when
> copying direct from output) referenced include/linux/autoconf.h which
> is now called include/generated/autoconf.h.
> Also, -I include paths needs to be prefixed with $(srctree)
> 
> Signed-off-by: Joakim Tjernlund <Joakim.Tjernlund@transmode.se>
> ---
> arch/powerpc/boot/Makefile |    4 ++--
> 1 files changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/powerpc/boot/Makefile b/arch/powerpc/boot/Makefile
> index 826a30a..79ebd6f 100644
> --- a/arch/powerpc/boot/Makefile
> +++ b/arch/powerpc/boot/Makefile
> @@ -34,8 +34,8 @@ BOOTCFLAGS	+= -fno-stack-protector
> endif
> 
> BOOTCFLAGS	+= -I$(obj) -I$(srctree)/$(obj)
> -BOOTCFLAGS	+= -include include/linux/autoconf.h -Iarch/powerpc/include
> -BOOTCFLAGS	+= -Iinclude
> +BOOTCFLAGS	+= -include include/generated/autoconf.h
> +BOOTCFLAGS	+= -I$(srctree)/arch/powerpc/include -I$(srctree)/include
> 
> DTS_FLAGS	?= -p 1024
> 

Ack, this works for me (seeing as -rc4 doesn't generate uImages w/o it :)

- k
Benjamin Herrenschmidt - Jan. 13, 2010, 8:02 p.m.
On Wed, 2010-01-13 at 12:54 -0600, Kumar Gala wrote:

> > 
> > BOOTCFLAGS	+= -I$(obj) -I$(srctree)/$(obj)
> > -BOOTCFLAGS	+= -include include/linux/autoconf.h -Iarch/powerpc/include
> > -BOOTCFLAGS	+= -Iinclude
> > +BOOTCFLAGS	+= -include include/generated/autoconf.h
> > +BOOTCFLAGS	+= -I$(srctree)/arch/powerpc/include -I$(srctree)/include
> > 
> > DTS_FLAGS	?= -p 1024
> > 
> 
> Ack, this works for me (seeing as -rc4 doesn't generate uImages w/o it :)
> 

I sent a different patch to Linus yesterday for that.

Cheers,
Ben.
Joakim Tjernlund - Jan. 14, 2010, 7:47 a.m.
Benjamin Herrenschmidt <benh@kernel.crashing.org> wrote on 13/01/2010 21:02:43:
>
> On Wed, 2010-01-13 at 12:54 -0600, Kumar Gala wrote:
>
> > >
> > > BOOTCFLAGS   += -I$(obj) -I$(srctree)/$(obj)
> > > -BOOTCFLAGS   += -include include/linux/autoconf.h -Iarch/powerpc/include
> > > -BOOTCFLAGS   += -Iinclude
> > > +BOOTCFLAGS   += -include include/generated/autoconf.h
> > > +BOOTCFLAGS   += -I$(srctree)/arch/powerpc/include -I$(srctree)/include
> > >
> > > DTS_FLAGS   ?= -p 1024
> > >
> >
> > Ack, this works for me (seeing as -rc4 doesn't generate uImages w/o it :)
> >
>
> I sent a different patch to Linus yesterday for that.

Seen it now as it is in Linus tree:

1) IMHO it would have been nicer to use #ifdef __KERNEL__
   instead of CONFIG_HAVE_EFFICIENT_UNALIGNED_ACCESS
   as then arches that don't define CONFIG_HAVE_EFFICIENT_UNALIGNED_ACCESS
   at all will never use the new optimization or was that what you intended?

2) You really should add an comment in the Makefile about not using
   autoconf.h/-D__KERNEL__
Benjamin Herrenschmidt - Jan. 14, 2010, 8:57 a.m.
> Seen it now as it is in Linus tree:
> 
> 1) IMHO it would have been nicer to use #ifdef __KERNEL__
>    instead of CONFIG_HAVE_EFFICIENT_UNALIGNED_ACCESS
>    as then arches that don't define CONFIG_HAVE_EFFICIENT_UNALIGNED_ACCESS
>    at all will never use the new optimization or was that what you intended?

No, that was on purpose. If an arch doesn't have efficient unaligned
accesses, then they should not use the optimization since it will result
in a lot of unaligned accesses :-) In which case they are better off
falling back to the old byte-by-byte method.

The advantage also of doing it this way is that x86 will benefit from
the optimisation at boot time since it does include autoconf.h in its
boot wrapper (and deals with unaligned accesses just fine at any time)
though something tells me that it won't make much of a difference in
performances on any recent x86 (it might on some of the newer low power
embedded ones, I don't know for sure).

> 2) You really should add an comment in the Makefile about not using
>    autoconf.h/-D__KERNEL__

That's true :-)

Cheers,
Ben.
Joakim Tjernlund - Jan. 14, 2010, 9:12 a.m.
Benjamin Herrenschmidt <benh@kernel.crashing.org> wrote on 14/01/2010 09:57:11:
> Subject: Re: [PATCH]: powerpc: Fix build breakage due to incorrect location ofautoconf.h
>
>
> > Seen it now as it is in Linus tree:
> >
> > 1) IMHO it would have been nicer to use #ifdef __KERNEL__
> >    instead of CONFIG_HAVE_EFFICIENT_UNALIGNED_ACCESS
> >    as then arches that don't define CONFIG_HAVE_EFFICIENT_UNALIGNED_ACCESS
> >    at all will never use the new optimization or was that what you intended?
>
> No, that was on purpose. If an arch doesn't have efficient unaligned
> accesses, then they should not use the optimization since it will result
> in a lot of unaligned accesses :-) In which case they are better off
> falling back to the old byte-by-byte method.

Not quite, it is only 1 of 4 accesses that uses unaligned and
that accesses is only unaligned 50% in average, it might still
be faster. We will never know now.

>
> The advantage also of doing it this way is that x86 will benefit from
> the optimisation at boot time since it does include autoconf.h in its

But so does my suggestion, doesn't it?

> boot wrapper (and deals with unaligned accesses just fine at any time)
> though something tells me that it won't make much of a difference in
> performances on any recent x86 (it might on some of the newer low power
> embedded ones, I don't know for sure).
Benjamin Herrenschmidt - Jan. 14, 2010, 9:43 a.m.
On Thu, 2010-01-14 at 10:12 +0100, Joakim Tjernlund wrote:
> > No, that was on purpose. If an arch doesn't have efficient unaligned
> > accesses, then they should not use the optimization since it will
> result
> > in a lot of unaligned accesses :-) In which case they are better off
> > falling back to the old byte-by-byte method.
> 
> Not quite, it is only 1 of 4 accesses that uses unaligned and
> that accesses is only unaligned 50% in average, it might still
> be faster. We will never know now.

Why ? If you think it's a win, then it's easy to make a patch to turn
it to __KERNEL__ and ask some people from ARM and MIPS or even sparc
land for example to give it a spin. If it's indeed a win, then submit it
to Linus and/or Andrew and there's no reason for it not to go in.

I simply took a more conservative approach for post -rc4

Cheers,
Ben.
Joakim Tjernlund - Jan. 14, 2010, 10:22 a.m.
Benjamin Herrenschmidt <benh@kernel.crashing.org> wrote on 14/01/2010 10:43:44:

> On Thu, 2010-01-14 at 10:12 +0100, Joakim Tjernlund wrote:
> > > No, that was on purpose. If an arch doesn't have efficient unaligned
> > > accesses, then they should not use the optimization since it will
> > result
> > > in a lot of unaligned accesses :-) In which case they are better off
> > > falling back to the old byte-by-byte method.
> >
> > Not quite, it is only 1 of 4 accesses that uses unaligned and
> > that accesses is only unaligned 50% in average, it might still
> > be faster. We will never know now.
>
> Why ? If you think it's a win, then it's easy to make a patch to turn
> it to __KERNEL__ and ask some people from ARM and MIPS or even sparc
> land for example to give it a spin. If it's indeed a win, then submit it
> to Linus and/or Andrew and there's no reason for it not to go in.
>
> I simply took a more conservative approach for post -rc4

Perhaps for the best this late. I will just leave it as is. If ARM/MIPS et. all
wants, they can test it whenever they want.

    Jocke
Joakim Tjernlund - Jan. 14, 2010, 1:27 p.m.
Benjamin Herrenschmidt <benh@kernel.crashing.org> wrote on 14/01/2010 10:43:44:
>
> On Thu, 2010-01-14 at 10:12 +0100, Joakim Tjernlund wrote:
> > > No, that was on purpose. If an arch doesn't have efficient unaligned
> > > accesses, then they should not use the optimization since it will
> > result
> > > in a lot of unaligned accesses :-) In which case they are better off
> > > falling back to the old byte-by-byte method.
> >
> > Not quite, it is only 1 of 4 accesses that uses unaligned and
> > that accesses is only unaligned 50% in average, it might still
> > be faster. We will never know now.
>
> Why ? If you think it's a win, then it's easy to make a patch to turn
> it to __KERNEL__ and ask some people from ARM and MIPS or even sparc
> land for example to give it a spin. If it's indeed a win, then submit it
> to Linus and/or Andrew and there's no reason for it not to go in.
>
> I simply took a more conservative approach for post -rc4

It would probably be a good idea to redefine UP_UNALIGNED macro to
do 2 byte accesses in the non CONFIG_HAVE_EFFICIENT_UNALIGNED_ACCESS case.

I don't think I will revisit this any time soon so I figured I should
mention it in case someone else wants to try it.

         Jocke
Benjamin Herrenschmidt - Jan. 14, 2010, 7:59 p.m.
On Thu, 2010-01-14 at 14:27 +0100, Joakim Tjernlund wrote:
> 
> It would probably be a good idea to redefine UP_UNALIGNED macro to
> do 2 byte accesses in the non CONFIG_HAVE_EFFICIENT_UNALIGNED_ACCESS
> case.
> 
> I don't think I will revisit this any time soon so I figured I should
> mention it in case someone else wants to try it. 

Well, get_unaligned() does just that...

Ben.

Patch

diff --git a/arch/powerpc/boot/Makefile b/arch/powerpc/boot/Makefile
index 826a30a..79ebd6f 100644
--- a/arch/powerpc/boot/Makefile
+++ b/arch/powerpc/boot/Makefile
@@ -34,8 +34,8 @@  BOOTCFLAGS	+= -fno-stack-protector
 endif

 BOOTCFLAGS	+= -I$(obj) -I$(srctree)/$(obj)
-BOOTCFLAGS	+= -include include/linux/autoconf.h -Iarch/powerpc/include
-BOOTCFLAGS	+= -Iinclude
+BOOTCFLAGS	+= -include include/generated/autoconf.h
+BOOTCFLAGS	+= -I$(srctree)/arch/powerpc/include -I$(srctree)/include

 DTS_FLAGS	?= -p 1024