[U-Boot-Users] [PATCH] Use `ln -sf` rather than `rm -f && ln -s`
Signed-off-by: Mike Frysinger vapier@gentoo.org --- tools/Makefile | 9 +++------ 1 files changed, 3 insertions(+), 6 deletions(-)
diff --git a/tools/Makefile b/tools/Makefile index af0de47..22d9dae 100644 --- a/tools/Makefile +++ b/tools/Makefile @@ -203,19 +203,16 @@ else endif
$(obj)environment.c: - @rm -f $(obj)environment.c - ln -s $(src)../common/environment.c $(obj)environment.c + ln -s -f $(src)../common/environment.c $(obj)environment.c
$(obj)environment.o: $(obj)environment.c $(CC) -g $(HOST_ENVIRO_CFLAGS) $(CPPFLAGS) -c -o $@ $<
$(obj)crc32.c: - @rm -f $(obj)crc32.c - ln -s $(src)../lib_generic/crc32.c $(obj)crc32.c + ln -s -f $(src)../lib_generic/crc32.c $(obj)crc32.c
$(obj)sha1.c: - @rm -f $(obj)sha1.c - ln -s $(src)../lib_generic/sha1.c $(obj)sha1.c + ln -s -f $(src)../lib_generic/sha1.c $(obj)sha1.c
$(LOGO_H): $(obj)bmp_logo $(LOGO_BMP) $(obj)./bmp_logo $(LOGO_BMP) >$@
odd ... git-send-email ate the explanatory text ... --- The -f option to `ln` should give the same behavior as the -f option to the `rm` command. It is better to do this in one shot so as to avoid race conditions when building in parallel. I build on a quad G5 and without this change, it isn't uncommon for the build to fail when using -j8 due to this small window where the files don't actually exist.
Signed-off-by: Mike Frysinger vapier@gentoo.org --- tools/Makefile | 9 +++------ 1 files changed, 3 insertions(+), 6 deletions(-)
diff --git a/tools/Makefile b/tools/Makefile index af0de47..22d9dae 100644 --- a/tools/Makefile +++ b/tools/Makefile @@ -203,19 +203,16 @@ else endif
$(obj)environment.c: - @rm -f $(obj)environment.c - ln -s $(src)../common/environment.c $(obj)environment.c + ln -s -f $(src)../common/environment.c $(obj)environment.c
$(obj)environment.o: $(obj)environment.c $(CC) -g $(HOST_ENVIRO_CFLAGS) $(CPPFLAGS) -c -o $@ $<
$(obj)crc32.c: - @rm -f $(obj)crc32.c - ln -s $(src)../lib_generic/crc32.c $(obj)crc32.c + ln -s -f $(src)../lib_generic/crc32.c $(obj)crc32.c
$(obj)sha1.c: - @rm -f $(obj)sha1.c - ln -s $(src)../lib_generic/sha1.c $(obj)sha1.c + ln -s -f $(src)../lib_generic/sha1.c $(obj)sha1.c
$(LOGO_H): $(obj)bmp_logo $(LOGO_BMP) $(obj)./bmp_logo $(LOGO_BMP) >$@
In message 200801280638.59012.vapier@gentoo.org you wrote:
odd ... git-send-email ate the explanatory text ...
The -f option to `ln` should give the same behavior as the -f option to the `rm` command. It is better to do this in one shot so as to avoid race conditions when building in parallel. I build on a quad G5 and without this change, it isn't uncommon for the build to fail when using -j8 due to this small window where the files don't actually exist.
Note that "ln -s -f" will come down to two separate system calls as well:
... unlink(); symlink(); ...
So you don't avoid the race condition; you're just making it a little less likely at the cost of reduced portability.
Best regards,
Wolfgang Denk
On Monday 28 January 2008, Wolfgang Denk wrote:
In message 200801280638.59012.vapier@gentoo.org you wrote:
odd ... git-send-email ate the explanatory text ...
The -f option to `ln` should give the same behavior as the -f option to the `rm` command. It is better to do this in one shot so as to avoid race conditions when building in parallel. I build on a quad G5 and without this change, it isn't uncommon for the build to fail when using -j8 due to this small window where the files don't actually exist.
Note that "ln -s -f" will come down to two separate system calls as well:
... unlink(); symlink(); ...
So you don't avoid the race condition; you're just making it a little less likely at the cost of reduced portability.
yes, i know it's much less likely, but that difference on my system has been from 1-in-20 build failures to 1-in-none-so-far-out-of-hundreds. the real fix is to overhaul the u-boot build system, but that'll take quite a bit of time and this change is for all practical purposes, Good Enough. i dont know what portability issues you refer to considering the -f flag is in POSIX and has been supported on all Linux systems since before that (pre-2000). -mike
Mike Frysinger wrote:
On Monday 28 January 2008, Wolfgang Denk wrote:
In message 200801280638.59012.vapier@gentoo.org you wrote:
odd ... git-send-email ate the explanatory text ...
The -f option to `ln` should give the same behavior as the -f option to the `rm` command. It is better to do this in one shot so as to avoid race conditions when building in parallel. I build on a quad G5 and without this change, it isn't uncommon for the build to fail when using -j8 due to this small window where the files don't actually exist.
Note that "ln -s -f" will come down to two separate system calls as well:
... unlink(); symlink(); ...
So you don't avoid the race condition; you're just making it a little less likely at the cost of reduced portability.
yes, i know it's much less likely, but that difference on my system has been from 1-in-20 build failures to 1-in-none-so-far-out-of-hundreds. the real fix is to overhaul the u-boot build system, but that'll take quite a bit of time and this change is for all practical purposes, Good Enough. i dont know what portability issues you refer to considering the -f flag is in POSIX and has been supported on all Linux systems since before that (pre-2000). -mike
I don't think there is a race condition at all in the "ln -s -f" case. The race condition exists in the first approach because the shell is allowed to execute the rm and the ln in parallel, while in the "ln -s -f" case the two system calls are sequential in the same thread of execution (inside ln) and therefore happen in a defined order. It is not just that the window is small, there is no window. -Bill
This SF.net email is sponsored by: Microsoft Defy all challenges. Microsoft(R) Visual Studio 2008. http://clk.atdmt.com/MRT/go/vse0120000070mrt/direct/01/
U-Boot-Users mailing list U-Boot-Users@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/u-boot-users
On Mon, Jan 28, 2008 at 06:38:58AM -0500, Mike Frysinger wrote:
The -f option to `ln` should give the same behavior as the -f option to the `rm` command. It is better to do this in one shot so as to avoid race conditions when building in parallel. I build on a quad G5 and without this change, it isn't uncommon for the build to fail when using -j8 due to this small window where the files don't actually exist.
Since there is a clear dependency of environment.o on environment.c there is no way for make to run the rm/ln and $(CC) commands in parallel, so there should be no race condition. For illustrative purposes: you should be able to put a sleep between the rm and the ln -s, and it should still build.
It seems the real problem is one level above, the commands were run twice in parallel, once for "depend" and once for the SUBDIRS "all". Please test the patch below.
The
$(obj)u-boot: depend $(SUBDIRS) $(OBJS) $(LIBS) $(LDSCRIPT)
line tells make it can run commands to update each dependency in parallel, unless there are further dependencies which restrict this.
Thanks, Johannes
--- Add dependencies to avoid race conditions with parallel make.
Signed-off-by: Johannes Stezenbach js@sig21.net
diff --git a/Makefile b/Makefile index 0f6cc59..464db34 100644 --- a/Makefile +++ b/Makefile @@ -317,13 +317,13 @@ $(obj)u-boot: depend $(SUBDIRS) $(OBJS) $(LIBS) $(LDSCRIPT) --start-group $(__LIBS) --end-group $(PLATFORM_LIBS) \ -Map u-boot.map -o u-boot
-$(OBJS): $(obj)include/autoconf.mk +$(OBJS): depend $(obj)include/autoconf.mk $(MAKE) -C cpu/$(CPU) $(if $(REMOTE_BUILD),$@,$(notdir $@))
-$(LIBS): $(obj)include/autoconf.mk +$(LIBS): depend $(obj)include/autoconf.mk $(MAKE) -C $(dir $(subst $(obj),,$@))
-$(SUBDIRS): $(obj)include/autoconf.mk +$(SUBDIRS): depend $(obj)include/autoconf.mk $(MAKE) -C $@ all
$(NAND_SPL): $(VERSION_FILE) $(obj)include/autoconf.mk
On Monday 28 January 2008, Johannes Stezenbach wrote:
On Mon, Jan 28, 2008 at 06:38:58AM -0500, Mike Frysinger wrote:
The -f option to `ln` should give the same behavior as the -f option to the `rm` command. It is better to do this in one shot so as to avoid race conditions when building in parallel. I build on a quad G5 and without this change, it isn't uncommon for the build to fail when using -j8 due to this small window where the files don't actually exist.
Since there is a clear dependency of environment.o on environment.c there is no way for make to run the rm/ln and $(CC) commands in parallel, so there should be no race condition. For illustrative purposes: you should be able to put a sleep between the rm and the ln -s, and it should still build.
right, i know the issue isnt between the link and the compile, it was that the rm/ln was run multiple times. but i never dug deeper into the issue to find the true cause of the problem.
It seems the real problem is one level above, the commands were run twice in parallel, once for "depend" and once for the SUBDIRS "all". Please test the patch below.
thanks for poking deeper
-$(OBJS): $(obj)include/autoconf.mk +$(OBJS): depend $(obj)include/autoconf.mk $(MAKE) -C cpu/$(CPU) $(if $(REMOTE_BUILD),$@,$(notdir $@))
-$(LIBS): $(obj)include/autoconf.mk +$(LIBS): depend $(obj)include/autoconf.mk $(MAKE) -C $(dir $(subst $(obj),,$@))
-$(SUBDIRS): $(obj)include/autoconf.mk +$(SUBDIRS): depend $(obj)include/autoconf.mk $(MAKE) -C $@ all
perhaps depend should get its own line ? u-boot $(OBJS) $(LIBS) $(SUBDIRS): depend or not, doesnt matter to me ... as long as it works :) -mike
In message 20080128231125.GA27063@sig21.net you wrote:
On Mon, Jan 28, 2008 at 06:38:58AM -0500, Mike Frysinger wrote:
The -f option to `ln` should give the same behavior as the -f option to the `rm` command. It is better to do this in one shot so as to avoid race conditions when building in parallel. I build on a quad G5 and without this change, it isn't uncommon for the build to fail when using -j8 due to this small window where the files don't actually exist.
Since there is a clear dependency of environment.o on environment.c there is no way for make to run the rm/ln and $(CC) commands in parallel, so there should be no race condition. For illustrative purposes: you should be able to put a sleep between the rm and the ln -s, and it should still build.
It seems the real problem is one level above, the commands were run twice in parallel, once for "depend" and once for the SUBDIRS "all". Please test the patch below.
The
$(obj)u-boot: depend $(SUBDIRS) $(OBJS) $(LIBS) $(LDSCRIPT)
line tells make it can run commands to update each dependency in parallel, unless there are further dependencies which restrict this.
Applied, thanks.
Thanks, Johannes
Add dependencies to avoid race conditions with parallel make.
Signed-off-by: Johannes Stezenbach js@sig21.net
Note that these 3 lines belong *above* the "---" line.
Best regards,
Wolfgang Denk
In message 1201520254-25593-1-git-send-email-vapier@gentoo.org you wrote:
Signed-off-by: Mike Frysinger vapier@gentoo.org
tools/Makefile | 9 +++------ 1 files changed, 3 insertions(+), 6 deletions(-)
diff --git a/tools/Makefile b/tools/Makefile index af0de47..22d9dae 100644 --- a/tools/Makefile +++ b/tools/Makefile @@ -203,19 +203,16 @@ else endif
$(obj)environment.c:
@rm -f $(obj)environment.cln -s $(src)../common/environment.c $(obj)environment.c
ln -s -f $(src)../common/environment.c $(obj)environment.c
Be careful here. Are you 100% sure that all systems in the field will behave exactly as your's is doing?
What is the exact problem you are trying to solve? If it's not a bug, I recomment to leave the code as is.
Best regards,
Wolfgang Denk
On Monday 28 January 2008, Wolfgang Denk wrote:
In message 1201520254-25593-1-git-send-email-vapier@gentoo.org you wrote:
Signed-off-by: Mike Frysinger vapier@gentoo.org
tools/Makefile | 9 +++------ 1 files changed, 3 insertions(+), 6 deletions(-)
diff --git a/tools/Makefile b/tools/Makefile index af0de47..22d9dae 100644 --- a/tools/Makefile +++ b/tools/Makefile @@ -203,19 +203,16 @@ else endif
$(obj)environment.c:
@rm -f $(obj)environment.cln -s $(src)../common/environment.c $(obj)environment.c
ln -s -f $(src)../common/environment.c $(obj)environment.cBe careful here. Are you 100% sure that all systems in the field will behave exactly as your's is doing?
i really dont understand what you mean. any POSIX compliant system will behave exactly as it's supposed to: if the destination exists already, unlink it just before creating the link.
What is the exact problem you are trying to solve? If it's not a bug, I recomment to leave the code as is.
i wouldnt be posting a change if it werent causing me a problem. as i said, the time between executing `rm` and `ln` is too long and can cause problems when building u-boot in parallel. moving the unlink() from `rm` to `ln` just shortens this window drastically. -mike
In message 200801281557.55976.vapier@gentoo.org you wrote:
@rm -f $(obj)environment.cln -s $(src)../common/environment.c $(obj)environment.c
ln -s -f $(src)../common/environment.c $(obj)environment.cBe careful here. Are you 100% sure that all systems in the field will behave exactly as your's is doing?
i really dont understand what you mean. any POSIX compliant system will behave exactly as it's supposed to: if the destination exists already, unlink it just before creating the link.
I finally fund such a situation again.
# cat /proc/version Linux version 2.6.22.14-72.fc6 (brewbuilder@ls20-bc1-14.build.redhat.com) (gcc version 4.1.2 20070626 (Red Hat 4.1.2-13)) #1 SMP Wed Nov 21 14:10:25 EST 2007 # cat /etc/issue Fedora Core release 6 (Zod) Kernel \r on an \m
# id uid=0(root) gid=0(root) groups=0(root),1(bin),2(daemon),3(sys),4(adm),6(disk),10(wheel) # cd /opt # ls -ld eldk eldk-4.2-2008-03-27 eldk-4.2-2008-04-01 lrwxrwxrwx 1 root root 19 Apr 4 04:01 eldk -> eldk-4.2-2008-03-27 drwxr-xr-x 14 root root 4096 Apr 8 23:37 eldk-4.2-2008-03-27 drwxr-xr-x 14 root root 4096 Apr 3 13:38 eldk-4.2-2008-04-01
Now we try:
# ln -sf eldk-4.2-2008-04-01 eldk # echo $? 0
And what do you think we got?
# ls -ld eldk eldk-4.2-2008-03-27 eldk-4.2-2008-04-01 lrwxrwxrwx 1 root root 19 Apr 4 04:01 eldk -> eldk-4.2-2008-03-27 drwxr-xr-x 14 root root 4096 Apr 14 21:27 eldk-4.2-2008-03-27 drwxr-xr-x 14 root root 4096 Apr 3 13:38 eldk-4.2-2008-04-01
This is what I mean when I say that "rm -f foo ; ln -s bar foo" is *NOT* exactly equivalent to "ln -sf bar foo". The former will always work, the latter won't.
Best regards,
Wolfgang Denk
participants (4)
-
J. William Campbell -
Johannes Stezenbach -
Mike Frysinger -
Wolfgang Denk