2012-08-02 12:26:03

by Will Deacon

[permalink] [raw]
Subject: Re: [PATCH] kbuild: kvm: make export of linux/kvm_para.h unconditional

On Thu, Jul 26, 2012 at 02:44:14PM +0100, Will Deacon wrote:
> The asm-generic version of kvm_para.h is always exported, confusing the
> Kbuild wildcarding that tries to detect whether the source architecture
> is exporting the header, since asm-* matches the generic version.
>
> This patch unconditionally exports linux/kvm_para.h and fixes the few
> remaining architectures without asm/kvm_para.h to use the generic
> version. I also took the liberty of removing some dead lines from the
> wildcarding which was searcing for asm-$(SRCARCH) directores under
> $(srctree).
>
> Cc: Arnd Bergmann <[email protected]>
> Cc: Avi Kivity <[email protected]>
> Tested-by: Geert Uytterhoeven <[email protected]>
> Signed-off-by: Will Deacon <[email protected]>
> ---

Any further comments on this? It fixes header generation for me, so it would
be nice to see it merged.

Cheers,

Will

> arch/cris/include/asm/Kbuild | 2 ++
> arch/m32r/include/asm/Kbuild | 2 ++
> include/asm-generic/Kbuild.asm | 12 +++---------
> include/linux/Kbuild | 8 +-------
> 4 files changed, 8 insertions(+), 16 deletions(-)
>
> diff --git a/arch/cris/include/asm/Kbuild b/arch/cris/include/asm/Kbuild
> index 04d02a5..2fde49c 100644
> --- a/arch/cris/include/asm/Kbuild
> +++ b/arch/cris/include/asm/Kbuild
> @@ -7,3 +7,5 @@ header-y += ethernet.h
> header-y += etraxgpio.h
> header-y += rs485.h
> header-y += sync_serial.h
> +
> +generic-y += kvm_para.h
> diff --git a/arch/m32r/include/asm/Kbuild b/arch/m32r/include/asm/Kbuild
> index c68e168..78c505e 100644
> --- a/arch/m32r/include/asm/Kbuild
> +++ b/arch/m32r/include/asm/Kbuild
> @@ -1 +1,3 @@
> include include/asm-generic/Kbuild.asm
> +
> +generic-y += kvm_para.h
> diff --git a/include/asm-generic/Kbuild.asm b/include/asm-generic/Kbuild.asm
> index c5d2e5d..f180c99 100644
> --- a/include/asm-generic/Kbuild.asm
> +++ b/include/asm-generic/Kbuild.asm
> @@ -1,15 +1,8 @@
> -ifneq ($(wildcard $(srctree)/arch/$(SRCARCH)/include/asm/kvm.h \
> - $(srctree)/include/asm-$(SRCARCH)/kvm.h),)
> +ifneq ($(wildcard $(srctree)/arch/$(SRCARCH)/include/asm/kvm.h),)
> header-y += kvm.h
> endif
>
> -ifneq ($(wildcard $(srctree)/arch/$(SRCARCH)/include/asm/kvm_para.h \
> - $(srctree)/include/asm-$(SRCARCH)/kvm_para.h),)
> -header-y += kvm_para.h
> -endif
> -
> -ifneq ($(wildcard $(srctree)/arch/$(SRCARCH)/include/asm/a.out.h \
> - $(srctree)/include/asm-$(SRCARCH)/a.out.h),)
> +ifneq ($(wildcard $(srctree)/arch/$(SRCARCH)/include/asm/a.out.h),)
> header-y += a.out.h
> endif
>
> @@ -21,6 +14,7 @@ header-y += fcntl.h
> header-y += ioctl.h
> header-y += ioctls.h
> header-y += ipcbuf.h
> +header-y += kvm_para.h
> header-y += mman.h
> header-y += msgbuf.h
> header-y += param.h
> diff --git a/include/linux/Kbuild b/include/linux/Kbuild
> index 8760be3..048abc6 100644
> --- a/include/linux/Kbuild
> +++ b/include/linux/Kbuild
> @@ -23,20 +23,13 @@ header-y += wimax/
> objhdr-y += version.h
>
> ifneq ($(wildcard $(srctree)/arch/$(SRCARCH)/include/asm/a.out.h \
> - $(srctree)/include/asm-$(SRCARCH)/a.out.h \
> $(INSTALL_HDR_PATH)/include/asm-*/a.out.h),)
> header-y += a.out.h
> endif
> ifneq ($(wildcard $(srctree)/arch/$(SRCARCH)/include/asm/kvm.h \
> - $(srctree)/include/asm-$(SRCARCH)/kvm.h \
> $(INSTALL_HDR_PATH)/include/asm-*/kvm.h),)
> header-y += kvm.h
> endif
> -ifneq ($(wildcard $(srctree)/arch/$(SRCARCH)/include/asm/kvm_para.h \
> - $(srctree)/include/asm-$(SRCARCH)/kvm_para.h \
> - $(INSTALL_HDR_PATH)/include/asm-*/kvm_para.h),)
> -header-y += kvm_para.h
> -endif
>
> header-y += acct.h
> header-y += adb.h
> @@ -229,6 +222,7 @@ header-y += kernel-page-flags.h
> header-y += kexec.h
> header-y += keyboard.h
> header-y += keyctl.h
> +header-y += kvm_para.h
> header-y += l2tp.h
> header-y += limits.h
> header-y += llc.h
> --
> 1.7.4.1
>


2012-08-02 14:19:34

by Avi Kivity

[permalink] [raw]
Subject: Re: [PATCH] kbuild: kvm: make export of linux/kvm_para.h unconditional

On 08/02/2012 03:25 PM, Will Deacon wrote:
> On Thu, Jul 26, 2012 at 02:44:14PM +0100, Will Deacon wrote:
>> The asm-generic version of kvm_para.h is always exported, confusing the
>> Kbuild wildcarding that tries to detect whether the source architecture
>> is exporting the header, since asm-* matches the generic version.
>>
>> This patch unconditionally exports linux/kvm_para.h and fixes the few
>> remaining architectures without asm/kvm_para.h to use the generic
>> version. I also took the liberty of removing some dead lines from the
>> wildcarding which was searcing for asm-$(SRCARCH) directores under
>> $(srctree).
>>
>> Cc: Arnd Bergmann <[email protected]>
>> Cc: Avi Kivity <[email protected]>
>> Tested-by: Geert Uytterhoeven <[email protected]>
>> Signed-off-by: Will Deacon <[email protected]>
>> ---
>
> Any further comments on this? It fixes header generation for me, so it would
> be nice to see it merged.

Can you get it reviewed by someone who is familiar with this? This is
probably the third fix for the this issue.

Arnd?

--
error compiling committee.c: too many arguments to function

2012-08-02 20:29:15

by Sam Ravnborg

[permalink] [raw]
Subject: Re: [PATCH] kbuild: kvm: make export of linux/kvm_para.h unconditional

On Thu, Aug 02, 2012 at 05:19:20PM +0300, Avi Kivity wrote:
> On 08/02/2012 03:25 PM, Will Deacon wrote:
> > On Thu, Jul 26, 2012 at 02:44:14PM +0100, Will Deacon wrote:
> >> The asm-generic version of kvm_para.h is always exported, confusing the
> >> Kbuild wildcarding that tries to detect whether the source architecture
> >> is exporting the header, since asm-* matches the generic version.
> >>
> >> This patch unconditionally exports linux/kvm_para.h and fixes the few
> >> remaining architectures without asm/kvm_para.h to use the generic
> >> version. I also took the liberty of removing some dead lines from the
> >> wildcarding which was searcing for asm-$(SRCARCH) directores under
> >> $(srctree).
> >>
> >> Cc: Arnd Bergmann <[email protected]>
> >> Cc: Avi Kivity <[email protected]>
> >> Tested-by: Geert Uytterhoeven <[email protected]>
> >> Signed-off-by: Will Deacon <[email protected]>
> >> ---
> >
> > Any further comments on this? It fixes header generation for me, so it would
> > be nice to see it merged.
>
> Can you get it reviewed by someone who is familiar with this? This is
> probably the third fix for the this issue.

IMO the patch is wrong.
Any use of wildcards in include/asm-generic/Kbuild.asm is wrong.

IMO include/asm-generic/Kbuild.asm is for generic header files
that ALL archs can use.

If not ALL archs can use it, then the individual arch shall
specify the file explicit.

I know we here hit a bad thing with the generic-y support,
where we miss a good way to add a file to a lot of archs
without editing a lot of files.

Sam

2012-08-03 12:52:00

by Will Deacon

[permalink] [raw]
Subject: Re: [PATCH] kbuild: kvm: make export of linux/kvm_para.h unconditional

On Thu, Aug 02, 2012 at 09:29:11PM +0100, Sam Ravnborg wrote:
> On Thu, Aug 02, 2012 at 05:19:20PM +0300, Avi Kivity wrote:
> > Can you get it reviewed by someone who is familiar with this? This is
> > probably the third fix for the this issue.
>
> IMO the patch is wrong.
> Any use of wildcards in include/asm-generic/Kbuild.asm is wrong.

I agree that the wildcard stuff is horrible but that's not something
introduced by this patch. In fact, I'm removing those where there is an
asm-generic header that can be used instead.

> IMO include/asm-generic/Kbuild.asm is for generic header files
> that ALL archs can use.
>
> If not ALL archs can use it, then the individual arch shall
> specify the file explicit.

In which case, we should probably fix the rules for kvm.h and a.out.h as
well. Neither of these headers have asm-generic versions and I doubt this
is possible, so the header-y lines should be done for each architecture
exporting such a header, no?

> I know we here hit a bad thing with the generic-y support,
> where we miss a good way to add a file to a lot of archs
> without editing a lot of files.

If there isn't a generic version of the header, then yes, I guess you have
to modify the architectures that want to export it.

I'm happy to post an extra patch sorting out kvm.h and a.out.h if that's
what's required to get headers building again.

Will

2012-08-03 13:02:33

by Sam Ravnborg

[permalink] [raw]
Subject: Re: [PATCH] kbuild: kvm: make export of linux/kvm_para.h unconditional

On Fri, Aug 03, 2012 at 01:51:44PM +0100, Will Deacon wrote:
> On Thu, Aug 02, 2012 at 09:29:11PM +0100, Sam Ravnborg wrote:
> > On Thu, Aug 02, 2012 at 05:19:20PM +0300, Avi Kivity wrote:
> > > Can you get it reviewed by someone who is familiar with this? This is
> > > probably the third fix for the this issue.
> >
> > IMO the patch is wrong.
> > Any use of wildcards in include/asm-generic/Kbuild.asm is wrong.
>
> I agree that the wildcard stuff is horrible but that's not something
> introduced by this patch. In fact, I'm removing those where there is an
> asm-generic header that can be used instead.
>
> > IMO include/asm-generic/Kbuild.asm is for generic header files
> > that ALL archs can use.
> >
> > If not ALL archs can use it, then the individual arch shall
> > specify the file explicit.
>
> In which case, we should probably fix the rules for kvm.h and a.out.h as
> well. Neither of these headers have asm-generic versions and I doubt this
> is possible, so the header-y lines should be done for each architecture
> exporting such a header, no?
>
> > I know we here hit a bad thing with the generic-y support,
> > where we miss a good way to add a file to a lot of archs
> > without editing a lot of files.
>
> If there isn't a generic version of the header, then yes, I guess you have
> to modify the architectures that want to export it.
>
> I'm happy to post an extra patch sorting out kvm.h and a.out.h if that's
> what's required to get headers building again.

IMO this is required to sort this out for good.
I will be happy to review your patches.

Sam

2012-08-03 13:11:41

by Arnd Bergmann

[permalink] [raw]
Subject: Re: [PATCH] kbuild: kvm: make export of linux/kvm_para.h unconditional

On Friday 03 August 2012, Sam Ravnborg wrote:
> > > I know we here hit a bad thing with the generic-y support,
> > > where we miss a good way to add a file to a lot of archs
> > > without editing a lot of files.
> >
> > If there isn't a generic version of the header, then yes, I guess you have
> > to modify the architectures that want to export it.
> >
> > I'm happy to post an extra patch sorting out kvm.h and a.out.h if that's
> > what's required to get headers building again.
>
> IMO this is required to sort this out for good.
> I will be happy to review your patches.

I also owe Mark Brown a solution for the asm/clkdev.h redirect,
as by http://lkml.indiana.edu/hypermail/linux/kernel/1207.0/01141.html

I would very much like to avoid adding another wildcard logic for
generic-y, but I could not figure out how to do that better.

Arnd