Skip to contentSkip to conversation

Use `ln -sf` rather than `rm -f && ln -s`

PATCH New Mike Frysinger 4 participants 11 messages Jan 28, 2008 – Apr 14, 2008 Kbuild

lore
Share & tools
lore
#0 lore raw

PATCH 1/1 Use `ln -sf` rather than `rm -f && ln -s`

---
 tools/Makefile |    9 +++------
 1 files changed, 3 insertions(+), 6 deletions(-)
tools/Makefile +3 -6
index af0de47..22d9dae 100644
@@ -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) >$@
Signature · 1 line
1.5.3.8
#1 lore raw

PATCH 1/1 Use `ln -sf` rather than `rm -f && ln -s`

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.

---
 tools/Makefile |    9 +++------
 1 files changed, 3 insertions(+), 6 deletions(-)
tools/Makefile +3 -6
index af0de47..22d9dae 100644
@@ -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) >$@
Signature · 1 line
1.5.3.8
#2 lore raw

PATCH 1/1 Use `ln -sf` rather than `rm -f && ln -s`

Quoted 7 lines · In message <[email protected]> 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

Signature · 7 lines
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
Software suppliers are trying to make their  software  packages  more
``user-friendly''.  .  .  .  Their best approach, so far, has been to
take all the old brochures, and stamp the words, ``user-friendly'' on
the cover.                                               - Bill Gates
#3 lore raw

PATCH 1/1 Use `ln -sf` rather than `rm -f && ln -s`

Quoted 12 lines · Wolfgang Denk
Quoted 7 lines · In message <[email protected]> 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:

Quoted 7 lines
	...
	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
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 827 bytes
Desc: This is a digitally signed message part.
Url : http://lists.denx.de/pipermail/u-boot/attachments/20080128/8f778de0/attachment.pgp

#4 lore raw

PATCH 1/1 Use `ln -sf` rather than `rm -f && ln -s`

Quoted 24 lines · Mike Frysinger
Quoted 13 lines · Wolfgang Denk
Quoted 8 lines · In message <[email protected]> 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:

Quoted 8 lines
	...
	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.

Quoted 9 lines

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

Quoted 13 lines

------------------------------------------------------------------------

-------------------------------------------------------------------------
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 at lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/u-boot-users

#5 lore raw

PATCH 1/1 Use `ln -sf` rather than `rm -f && ln -s`

In reply to Mike Frysinger
Quoted 5 lines · Mike Frysinger

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.

Makefile +3 -3
index 0f6cc59..464db34 100644
@@ -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
#6 lore raw

PATCH 1/1 Use `ln -sf` rather than `rm -f && ln -s`

Quoted 7 lines · Johannes Stezenbach
Quoted 5 lines · Mike Frysinger

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.

Quoted 5 lines

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.

Quoted 3 lines

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

Makefile $(obj)u-boot: depend $(SUBDIRS) $(OBJS) $(LIBS) $(LDSCRIPT) · line 327

Quoted 11 lines

-$(OBJS): $(obj)include/autoconf.mk
+$(OBJS): depend $(obj)include/autoconf.mk

Makefile +2 -2
 		$(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
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 827 bytes
Desc: This is a digitally signed message part.
Url : http://lists.denx.de/pipermail/u-boot/attachments/20080128/1687ce1f/attachment.pgp

#7 lore raw

PATCH 1/1 Use `ln -sf` rather than `rm -f && ln -s`

In reply to Johannes Stezenbach
Quoted 17 lines · In message <[email protected]> you wrote:
Quoted 5 lines · Mike Frysinger

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.

Quoted 6 lines

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.

Quoted 8 lines

Thanks,
Johannes

---
Add dependencies to avoid race conditions with parallel make.

Note that these 3 lines belong *above* the "---" line.

Best regards,

Wolfgang Denk

Signature · 5 lines
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
There are three ways to get something  done:  do  it  yourself,  hire
someone, or forbid your kids to do it.
#8 lore raw

PATCH 1/1 Use `ln -sf` rather than `rm -f && ln -s`

In reply to Mike Frysinger
Quoted 11 lines · In message <[email protected]> you wrote:
---
 tools/Makefile |    9 +++------
 1 files changed, 3 insertions(+), 6 deletions(-)
tools/Makefile +0 -0
index af0de47..22d9dae 100644

tools/Makefile else · line 206

Quoted 6 lines
tools/Makefile +1 -2
@@ -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

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

Signature · 5 lines
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
If you can't beat it or corrupt it, you pretend it was your  idea  in
the first place.                 - Terry Pratchett, _Guards! Guards!_
#9 lore raw

PATCH 1/1 Use `ln -sf` rather than `rm -f && ln -s`

Quoted 13 lines · Wolfgang Denk
Quoted 12 lines · In message <[email protected]> you wrote:
---
 tools/Makefile |    9 +++------
 1 files changed, 3 insertions(+), 6 deletions(-)
tools/Makefile +0 -0
index af0de47..22d9dae 100644
@@ -203,19 +203,16 @@ else
 endif
 
Quoted 7 lines

tools/Makefile else · line 206

Quoted 4 lines
tools/Makefile +1 -2
 $(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

Be 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.

Quoted 2 lines

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
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 827 bytes
Desc: This is a digitally signed message part.
Url : http://lists.denx.de/pipermail/u-boot/attachments/20080128/d5583c8c/attachment.pgp

#10 lore raw

PATCH 1/1 Use `ln -sf` rather than `rm -f && ln -s`

Quoted 11 lines · In message <[email protected]> you wrote:
Quoted 7 lines

tools/Makefile else · line 206

Quoted 4 lines
tools/Makefile +1 -2
-		@rm -f $(obj)environment.c
-		ln -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?

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 at 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

Signature · 4 lines
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
"Wagner's music is better than it sounds."               - Mark Twain