Re: Makefile race between jobs

Re: Makefile race between jobs

[Michal Marek]

On Mon, Dec 10, 2012 at 11:02:25AM +0200, Noam Camus wrote:
> Target "scripts_basic" by itself is indeed not creating the dependency issue.
> The relevant target is "scripts".
> Sorry for the confusion, thanks for the example.
> So I call target "defconfig" and then I call target "vmlinux" which by dependency calls target "scripts", e.g.:
> $ make ARCH=arc O=/dev/shm/_build/ defconfig scripts
>   HOSTCC  scripts/basic/fixdep
>   GEN     /dev/shm/_build/Makefile
>   HOSTCC  scripts/kconfig/conf.o
>   SHIPPED scripts/kconfig/zconf.tab.c
>   SHIPPED scripts/kconfig/zconf.lex.c
>   SHIPPED scripts/kconfig/zconf.hash.c
>   HOSTCC  scripts/kconfig/zconf.tab.o
>   HOSTLD  scripts/kconfig/conf
> #
> # configuration written to .config
> #
>   GEN     /dev/shm/_build/Makefile
> scripts/kconfig/conf --silentoldconfig Kconfig
>   CC      scripts/mod/empty.o
> In file included from /tmp/linux/include/linux/thread_info.h:10,
>                  from /tmp/linux/arch/arc/include/asm/current.h:24,
>                  from <command-line>:0:
> /tmp/linux/include/linux/types.h:4:23: error: asm/types.h: No such file or directory
> 
> The inclusion of asm/current.h comes from use of LINUXINCLUDE in my
> architecture Makefile.
> And asm/types.h is missing since it suppose to be created by target
> "asm-generic".

OK, so there is a new architecture 'arc' that adds '-include asm/current.h'
to global cflags in arch/arc/Makefile. Added Arnd to CC.


> I think the point here that I try to emphasize is that target
> "asm-generic" should be completed before any non-host source is
> compiled.
> In our case scripts/mod/empty.c is the first non-host source to be
> compiled and by bad coincidence it depends upon auto generated
> headers.

I see the problem now. However, so far, no other architecture has been
doing this. 'grep -e -include arch/*/Makefile' shows only one instance
in arch/arm, but that include is a static file with no dependencies. Is
it really necessary to include asm/current.h when compiling _every_
file? Sure, you get it via many common headers, but then you don't need
to add it explicitly :). But if Arnd thinks this is OK, I will merge a
patch that serializes scripts/mod/empty.o and asm-generic.

Michal

RE: Makefile race between jobs

[Noam Camus]

-----Original Message-----
> From: Michal Marek [mailto:mmarek@suse.cz] 
> Sent: Monday, December 10, 2012 11:45 AM
> To: Noam Camus
> Cc: linux-kbuild@vger.kernel.org; linux-kernel@vger.kernel.org; Arnd Bergmann
> Subject: Re: Makefile race between jobs

> I see the problem now. However, so far, no other architecture has been doing this. 'grep -e -include arch/*/Makefile' shows only one instance in arch/arm, but that include is a static file with no dependencies. Is it really necessary to include asm/current.h when compiling _every_ file? Sure, you get it via many common headers, but then you don't need to add it explicitly :). But if Arnd thinks this is OK, I will merge a patch that serializes scripts/mod/empty.o and asm-generic.

> Michal


According to my architecture maintainer it is mandatory.

Re: Makefile race between jobs

[Vineet Gupta]

On Monday 10 December 2012 03:15 PM, Michal Marek wrote:
> On Mon, Dec 10, 2012 at 11:02:25AM +0200, Noam Camus wrote:
>> Target "scripts_basic" by itself is indeed not creating the dependency issue.
>> The relevant target is "scripts".
>> Sorry for the confusion, thanks for the example.
>> So I call target "defconfig" and then I call target "vmlinux" which by dependency calls target "scripts", e.g.:
>> $ make ARCH=arc O=/dev/shm/_build/ defconfig scripts
>>   HOSTCC  scripts/basic/fixdep
>>   GEN     /dev/shm/_build/Makefile
>>   HOSTCC  scripts/kconfig/conf.o
>>   SHIPPED scripts/kconfig/zconf.tab.c
>>   SHIPPED scripts/kconfig/zconf.lex.c
>>   SHIPPED scripts/kconfig/zconf.hash.c
>>   HOSTCC  scripts/kconfig/zconf.tab.o
>>   HOSTLD  scripts/kconfig/conf
>> #
>> # configuration written to .config
>> #
>>   GEN     /dev/shm/_build/Makefile
>> scripts/kconfig/conf --silentoldconfig Kconfig
>>   CC      scripts/mod/empty.o
>> In file included from /tmp/linux/include/linux/thread_info.h:10,
>>                  from /tmp/linux/arch/arc/include/asm/current.h:24,
>>                  from <command-line>:0:
>> /tmp/linux/include/linux/types.h:4:23: error: asm/types.h: No such file or directory
>>
>> The inclusion of asm/current.h comes from use of LINUXINCLUDE in my
>> architecture Makefile.
>> And asm/types.h is missing since it suppose to be created by target
>> "asm-generic".
> 
> OK, so there is a new architecture 'arc' that adds '-include asm/current.h'
> to global cflags in arch/arc/Makefile. Added Arnd to CC.
> 
> 
>> I think the point here that I try to emphasize is that target
>> "asm-generic" should be completed before any non-host source is
>> compiled.
>> In our case scripts/mod/empty.c is the first non-host source to be
>> compiled and by bad coincidence it depends upon auto generated
>> headers.
> 
> I see the problem now. However, so far, no other architecture has been
> doing this. 'grep -e -include arch/*/Makefile' shows only one instance
> in arch/arm, but that include is a static file with no dependencies. Is
> it really necessary to include asm/current.h when compiling _every_
> file?

ARC Port caches current task pointer in a register - thus we have a
global asm register definition in current.h
In the past, a customer ran into issue when porting some "really
portable" code to kernel - such that asm/current.h didn't make it into
the build of their module - via normal header includes - strange but
true. Thus forcing current.h via way of -include seemed like a
safe/sensible way.


 Sure, you get it via many common headers, but then you don't need
> to add it explicitly :). But if Arnd thinks this is OK, I will merge a
> patch that serializes scripts/mod/empty.o and asm-generic.
> 
> Michal
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kbuild" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

Re: Makefile race between jobs

[Michal Marek]

On 10.12.2012 11:11, Vineet Gupta wrote:
> ARC Port caches current task pointer in a register - thus we have a
> global asm register definition in current.h
> In the past, a customer ran into issue when porting some "really
> portable" code to kernel - such that asm/current.h didn't make it into
> the build of their module - via normal header includes - strange but
> true. Thus forcing current.h via way of -include seemed like a
> safe/sensible way.

To me that sounds like either an arc header is using the define but
lacking an include of asm/current.h, or the code is lacking asm/current.h.

Michal

Re: Makefile race between jobs

[Vineet Gupta]

On Monday 10 December 2012 04:00 PM, Michal Marek wrote:
> On 10.12.2012 11:11, Vineet Gupta wrote:
>> ARC Port caches current task pointer in a register - thus we have a
>> global asm register definition in current.h
>> In the past, a customer ran into issue when porting some "really
>> portable" code to kernel - such that asm/current.h didn't make it into
>> the build of their module - via normal header includes - strange but
>> true. Thus forcing current.h via way of -include seemed like a
>> safe/sensible way.
> 
> To me that sounds like either an arc header is using the define but
> lacking an include of asm/current.h, or the code is lacking asm/current.h.
> 

It was latter - customer code was lacking include of asm/current.h
At any rate, independent of above, since we are dealing with a global
reg definition, IMHO, forcing the -include for each file built ensures
the generated code correctness (gcc reg allocator not fiddling with that
reg) - w/o "assuming" it would.

-Vineet

> Michal

Re: Makefile race between jobs

[Arnd Bergmann]

On Monday 10 December 2012, Vineet Gupta wrote:
> 
> On Monday 10 December 2012 04:00 PM, Michal Marek wrote:
> > On 10.12.2012 11:11, Vineet Gupta wrote:
> >> ARC Port caches current task pointer in a register - thus we have a
> >> global asm register definition in current.h
> >> In the past, a customer ran into issue when porting some "really
> >> portable" code to kernel - such that asm/current.h didn't make it into
> >> the build of their module - via normal header includes - strange but
> >> true. Thus forcing current.h via way of -include seemed like a
> >> safe/sensible way.
> > 
> > To me that sounds like either an arc header is using the define but
> > lacking an include of asm/current.h, or the code is lacking asm/current.h.
> > 
> 
> It was latter - customer code was lacking include of asm/current.h
> At any rate, independent of above, since we are dealing with a global
> reg definition, IMHO, forcing the -include for each file built ensures
> the generated code correctness (gcc reg allocator not fiddling with that
> reg) - w/o "assuming" it would.

I'm not convinced that adding the -include to the kernel Makefile
actually helps here: If a third party module is built outside of
the kernel sources, it probably also does not use the kernel Makefile,
and it may not have access to the kernel header files at all.

That aside, can't you just have an arch specific header file that
reservers the register but does not rely on asm-generic/types.h?
That would eliminate the need to serialize the build process.

	Arnd

Re: Makefile race between jobs

[Vineet Gupta]

On Monday 10 December 2012 04:29 PM, Arnd Bergmann wrote:
> On Monday 10 December 2012, Vineet Gupta wrote:
>>
>> On Monday 10 December 2012 04:00 PM, Michal Marek wrote:
>>> On 10.12.2012 11:11, Vineet Gupta wrote:
>>>> ARC Port caches current task pointer in a register - thus we have a
>>>> global asm register definition in current.h
>>>> In the past, a customer ran into issue when porting some "really
>>>> portable" code to kernel - such that asm/current.h didn't make it into
>>>> the build of their module - via normal header includes - strange but
>>>> true. Thus forcing current.h via way of -include seemed like a
>>>> safe/sensible way.
>>>
>>> To me that sounds like either an arc header is using the define but
>>> lacking an include of asm/current.h, or the code is lacking asm/current.h.
>>>
>>
>> It was latter - customer code was lacking include of asm/current.h
>> At any rate, independent of above, since we are dealing with a global
>> reg definition, IMHO, forcing the -include for each file built ensures
>> the generated code correctness (gcc reg allocator not fiddling with that
>> reg) - w/o "assuming" it would.
> 
> I'm not convinced that adding the -include to the kernel Makefile
> actually helps here: If a third party module is built outside of
> the kernel sources, it probably also does not use the kernel Makefile,
> and it may not have access to the kernel header files at all.

There might be NDA issues otherwise I would have uploaded the customer
code. It was third party code alright - but built in tree - and
interestingly not using any kernel headers - even linux/types.h. I was
dazzled myself when I found the root cause (due to r25 getting clobbered
mysteriously after some code from that driver had run).
So seriously it was really portable code.


> That aside, can't you just have an arch specific header file that
> reservers the register but does not rely on asm-generic/types.h?
> That would eliminate the need to serialize the build process.

The code in asm/current.h is as follows:

#ifdef CONFIG_ARC_CURR_IN_REG
register struct task_struct *curr_arc asm("r25");
#define current (curr_arc)
#else
#include <asm-generic/current.h>
#endif

The Makefile snippet in 3.2 (which Noam is using) is
LINUXINCLUDE	+=  -include ${src}/arch/arc/include/asm/current.h

I'm ready to change it to anything which helps alleviate the need for
build system change - w/o terribly uglifying this code.

However, in the 3.7 port which I posted to lists, the Makefile fragment
has already been changed due to other reasons.

ifdef CONFIG_ARC_CURR_IN_REG
# Can't do unconditionally because of recursive include
# issues due to <linux/thread_info.h>
LINUXINCLUDE	+=  -include ${src}/arch/arc/include/asm/current.h
endif

Does that make the problem report moot already - since the issue only
seems to happen for !CONFIG_ARC_CURR_IN_REG in which case the header
won't be -include ed

-Vineet

> 
> 	Arnd
>