Makefile race between jobs

Makefile race between jobs

[Noam Camus]

Hello Michael,

Here is my patch for dealing with race between 2 jobs of primary Makefile:

From 697f6ffa0d67b676fc2d75f65541c6e12108829d Mon Sep 17 00:00:00 2001
From: Noam Camus <noamc@ezchip.com>
Date: Fri, 16 Nov 2012 15:12:48 +0200
Subject: [PATCH] Makefile: solve race between make jobs.

Fix occasionally compilation failures when source tree is distcleaned.
The compilation is failing over file scripts/mod/empty.c

The error is because empty.o depends on some headers from:
	 arch/<arch>/include/generated/asm/

I see that there is a race between two targets in main Makefile (j=2):
	1) asm-generic
	2) scripts_basic

asm-generic:
	generates all wrappers in atch/<arch>/include/generated/asm/

scripts_basic:
	calls scripts/Makefile.build
	--> performs target __build
		--> depends on targets defined at variable $(always)
			--> compile scripts/mod/empty.c

Occasionally compilation of empty.c starts before all its required
headers created by asm-generic are available.

This dependency will make sure that this situation can not happen.

Signed-off-by: Noam Camus <noamc@ezchip.com>
---
 Makefile |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/Makefile b/Makefile
index 5be2ee8..00330a1 100644
--- a/Makefile
+++ b/Makefile
@@ -415,7 +415,7 @@ export RCS_TAR_IGNORE := --exclude SCCS --exclude BitKeeper --exclude .svn \
 
 # Basic helpers built in scripts/
 PHONY += scripts_basic
-scripts_basic:
+scripts_basic: asm-generic
 	$(Q)$(MAKE) $(build)=scripts/basic
 	$(Q)rm -f .tmp_quiet_recordmcount
 
-- 
1.7.1

Please comment.

Noam Camus

RE: Makefile race between jobs

[Noam Camus]

Hello Michal,

Here is my patch for dealing with race between 2 jobs of main Makefile:

From 697f6ffa0d67b676fc2d75f65541c6e12108829d Mon Sep 17 00:00:00 2001
From: Noam Camus <noamc@ezchip.com>
Date: Fri, 16 Nov 2012 15:12:48 +0200
Subject: [PATCH] Makefile: solve race between make jobs.

Fix occasionally compilation failures when source tree is distcleaned.
The compilation is failing over file scripts/mod/empty.c

The error is because empty.o depends on some headers from:
         arch/<arch>/include/generated/asm/

I see that there is a race between two targets in main Makefile (j=2):
        1) asm-generic
        2) scripts_basic

asm-generic:
        generates all wrappers in arch/<arch>/include/generated/asm/

scripts_basic:
        calls scripts/Makefile.build
        --> performs target __build
                --> depends on targets defined at variable $(always)
                        --> compile scripts/mod/empty.c

Occasionally compilation of empty.c starts before all its required
headers created by asm-generic are available.

This dependency will make sure that this situation can not happen.

Signed-off-by: Noam Camus <noamc@ezchip.com>
---
 Makefile |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/Makefile b/Makefile
index 5be2ee8..00330a1 100644
--- a/Makefile
+++ b/Makefile
@@ -415,7 +415,7 @@ export RCS_TAR_IGNORE := --exclude SCCS --exclude BitKeeper --exclude .svn \

 # Basic helpers built in scripts/
 PHONY += scripts_basic
-scripts_basic:
+scripts_basic: asm-generic
        $(Q)$(MAKE) $(build)=scripts/basic
        $(Q)rm -f .tmp_quiet_recordmcount

--
1.7.1

Please comment.

Noam Camus

RE: Makefile race between jobs

[Noam Camus]

Hello Michal,

Below is my patch for dealing with race between 2 jobs of main Makefile.
This is the third time I try to get any response from you.
Will appreciate your comment.

From 697f6ffa0d67b676fc2d75f65541c6e12108829d Mon Sep 17 00:00:00 2001
From: Noam Camus <noamc@ezchip.com>
Date: Fri, 16 Nov 2012 15:12:48 +0200
Subject: [PATCH] Makefile: solve race between make jobs.

Fix occasionally compilation failures when source tree is distcleaned.
The compilation is failing over file scripts/mod/empty.c

The error is because empty.o depends on some headers from:
         arch/<arch>/include/generated/asm/

I see that there is a race between two targets in main Makefile (j=2):
        1) asm-generic
        2) scripts_basic

asm-generic:
        generates all wrappers in arch/<arch>/include/generated/asm/

scripts_basic:
        calls scripts/Makefile.build
        --> performs target __build
                --> depends on targets defined at variable $(always)
                        --> compile scripts/mod/empty.c

Occasionally compilation of empty.c starts before all its required headers created by asm-generic are available.

This dependency will make sure that this situation can not happen.

Signed-off-by: Noam Camus <noamc@ezchip.com>
---
 Makefile |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/Makefile b/Makefile
index 5be2ee8..00330a1 100644
--- a/Makefile
+++ b/Makefile
@@ -415,7 +415,7 @@ export RCS_TAR_IGNORE := --exclude SCCS --exclude BitKeeper --exclude .svn \

 # Basic helpers built in scripts/
 PHONY += scripts_basic
-scripts_basic:
+scripts_basic: asm-generic
        $(Q)$(MAKE) $(build)=scripts/basic
        $(Q)rm -f .tmp_quiet_recordmcount

--
1.7.1

Noam Camus

Re: Makefile race between jobs

[Michal Marek]

On 16.11.2012 14:40, Noam Camus wrote:
> Hello Michael,
> 
> Here is my patch for dealing with race between 2 jobs of primary Makefile:
> 
> From 697f6ffa0d67b676fc2d75f65541c6e12108829d Mon Sep 17 00:00:00 2001
> From: Noam Camus <noamc@ezchip.com>
> Date: Fri, 16 Nov 2012 15:12:48 +0200
> Subject: [PATCH] Makefile: solve race between make jobs.
> 
> Fix occasionally compilation failures when source tree is distcleaned.
> The compilation is failing over file scripts/mod/empty.c
> 
> The error is because empty.o depends on some headers from:
> 	 arch/<arch>/include/generated/asm/
> 
> I see that there is a race between two targets in main Makefile (j=2):
> 	1) asm-generic
> 	2) scripts_basic
> 
> asm-generic:
> 	generates all wrappers in atch/<arch>/include/generated/asm/
> 
> scripts_basic:
> 	calls scripts/Makefile.build
> 	--> performs target __build
> 		--> depends on targets defined at variable $(always)
> 			--> compile scripts/mod/empty.c

Can you explain this dependency? scripts/basic/fixdep.c is supposed to
not depend on _anything_, because the script is subsequently used for
any compilation. And scripts/mod/empty.c is only built when visiting
scripts/mod:

$ arch
x86_64
$ mkdir /dev/shm/_build
$ make O=/dev/shm/_build defconfig
  HOSTCC  scripts/basic/fixdep
...
# At this point, scripts_basic has already been done, but just for sure
$ make O=/dev/shm/_build scripts_basic
  GEN     /dev/shm/_build/Makefile
scripts/kconfig/conf --silentoldconfig Kconfig
# Yet
$ ls /dev/shm/_build/scripts/mod
ls: cannot access /dev/shm/_build/scripts/mod: No such file or directory

If you are seeing something different, please tell which architecture
and which configuration.

Michal

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
>