Re: svn commit: r272372 - stable/10/bin/rm
On Sat, 4 Oct 2014, Bruce Evans wrote: On Fri, 3 Oct 2014, Garrett Cooper wrote: ? as filtering out these errors would handle the case that -f should handle according to the manpage: -f Attempt to remove the files without prompting for confirmation, regardless of the file's permissions. If the file does not exist, do not display a diagnostic message or modify the exit status to reflect an error. The -f option overrides any previous -i options. I didn't realize theat the man page was that broken. It doesn't even mention the main effect of -f -- to ignore errors for nonexistent files. This is discussed in the COMPATIBILITY section, with quite bad wording: Oops, I missed the second sentence in the above. % COMPATIBILITY % The rm utility differs from historical implementations in that the -f % option only masks attempts to remove non-existent files instead of mask- % ing a large variety of errors. The -v option is non-standard and its use % in scripts is not recommended. masks attempts to remove is strange wording. rm doesn't attempt to remove nonexistent files; it attempts to remove existent files ... The code doesn't do anything like that. It is close to POSIX, and just tries to remove existent files and then ignores the ENOENT error for nonexistent files iff -f. I misread this too. There are only minor wording problems. Bruce___ svn-src-all@freebsd.org mailing list http://lists.freebsd.org/mailman/listinfo/svn-src-all To unsubscribe, send any mail to svn-src-all-unsubscr...@freebsd.org
Re: svn commit: r272372 - stable/10/bin/rm
On Oct 2, 2014, at 16:34, Bruce Evans b...@optusnet.com.au wrote: There is still the larger problem with fts_read(). Applications like rm are specified to do a complete tree walk, with special handling for files that do not exist. If fts_read() is going to abort in the middle of a tree walk, then it is unusable for implementing applications like rm. +1. In this case I was doing a du -sh /* while I was running rm -Rf /rel (an old make release directory tree). This stopped the du -sh: $ sudo du -sh /* 8.0K/COPYRIGHT 996K/bin 218M/boot 0B/boot.config 2.5K/dev 4.0K/entropy 2.2M/etc 1.0G/home 16M/lib 280K/libexec 4.0K/media 4.0K/mnt 4.0K/proc du: /rel/usr/ports/net/samba36: No such file or directory du: /rel/usr/ports/net/aslookup: No such file or directory du: /rel/usr/ports/net/p5-Net-Amazon-AWSSign: No such file or directory du: /rel/usr/ports/net/p5-Net-Server-SS-PreFork: No such file or directory du: /rel/usr/ports/net/lla: No such file or directory du: /rel/usr/ports/net/sslh: No such file or directory du: /rel/usr/ports/net/wmlj: No such file or directory du: /rel/usr/ports/net/sendsms: No such file or directory du: /rel/usr/ports/net/uriparser: No such file or directory du: /rel/usr/ports/net/p5-Data-IPV4-Range-Parse: No such file or directory du: /rel/usr/ports/net/cagibi: No such file or directory du: /rel/usr/ports/net/fsplib: No such file or directory du: fts_read: No such file or directory The problem with changing fts_read to ignore ENOENT or other errors will break compatibility with Linux [and other OSes potentially], and is not the correct solution for this issue. I do however think that the errnos to ignore with -f should be... - EACCES - ENOENT - EPERM … as filtering out these errors would handle the case that -f should handle according to the manpage: -f Attempt to remove the files without prompting for confirmation, regardless of the file's permissions. If the file does not exist, do not display a diagnostic message or modify the exit status to reflect an error. The -f option overrides any previous -i options. Thanks, -Garrett signature.asc Description: Message signed with OpenPGP using GPGMail
Re: svn commit: r272372 - stable/10/bin/rm
On Wed, 2014-10-01 at 23:25 -0700, NGie Cooper wrote: On Wed, Oct 1, 2014 at 11:16 PM, Glen Barber g...@freebsd.org wrote: On Thu, Oct 02, 2014 at 02:56:05PM +1000, Bruce Evans wrote: On Wed, 1 Oct 2014, Glen Barber wrote: Log: MFC r268376 (imp): rm -rf can fail sometimes with an error from fts_read. Make it honor fflag to ignore fts_read errors, but stop deleting from that directory because no further progress can be made. I asked for this to be backed out in -current. It is not suitable for MFC. It fixes an immediate issue that prevents high-concurrent make(1) jobs from stomping over each other. The real problem is noted in this bug: https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=192490 ; the SUBDIR_PARALLEL logic is executing multiple instances of the clean/cleandir .PHONY targets. I agree with bde@ that this commit is papering over a bigger problem, but it's annoying enough and causes enough false positives that I understand why gjb@ MFCed the commit imp@ did in head. Thank you.. I agree that the change to rm only papers over the real problem. I think bug 192490 should be re-opened so that we can address the actual build system problem, but I don't know if that's allowed for in the new bugzilla workflows, or if we need to open a new report now. I've been digging into the actual build system problem this evening, and I'm starting to think that all the reported failures that contain enough of the log to be useful show that the build failed in a directory that has subdirectories. That is, one of the failures appeared to be caused by rm -rf running concurrently in usr.bin/lex and usr.bin/lex/lib. Another failure involves modules/aic7xxx and modules/aic7xxx/ahc. In another log it appeared that ata/atapci/chipsets was being deleted simulataneously with ata/atapci/chipsets/ataacard and several other subdirs under chipsets/. If this is indeed the cause of the problem I'm too tired right now to think of a fix, but I at least wanted to get what I found in writing before I sleep and completely forget it. :) BTW, I didn't see any evidence that the exact same path was being multiply deleted at the same time. That is, no duplicated entries in SUBDIR lists or accidentally processing the entire sys/modules hiearchy twice in parallel somehow through two different parent paths or anything like that. -- Ian ___ svn-src-all@freebsd.org mailing list http://lists.freebsd.org/mailman/listinfo/svn-src-all To unsubscribe, send any mail to svn-src-all-unsubscr...@freebsd.org
Re: svn commit: r272372 - stable/10/bin/rm
On Fri, 3 Oct 2014, Garrett Cooper wrote: On Oct 2, 2014, at 16:34, Bruce Evans b...@optusnet.com.au wrote: There is still the larger problem with fts_read(). Applications like rm are specified to do a complete tree walk, with special handling for files that do not exist. If fts_read() is going to abort in the middle of a tree walk, then it is unusable for implementing applications like rm. +1. In this case I was doing a du -sh /* while I was running rm -Rf /rel (an old make release directory tree). This stopped the du -sh: $ sudo du -sh /* 8.0K/COPYRIGHT 996K/bin ... du: /rel/usr/ports/net/fsplib: No such file or directory du: fts_read: No such file or directory It is fine garbage collection to delete itself, but that should be more like du: du: No such file or directory :-). The problem with changing fts_read to ignore ENOENT or other errors will break compatibility with Linux [and other OSes potentially], and is not the correct solution for this issue. I do however think that the errnos to ignore with -f should be... - EACCES - ENOENT - EPERM No, the only error that can be safely ignored in most cases is ENOENT. EACCES and EPERM mean that rm has failed to remove the requested files. More errors could be ignored under a different option. ? as filtering out these errors would handle the case that -f should handle according to the manpage: -f Attempt to remove the files without prompting for confirmation, regardless of the file's permissions. If the file does not exist, do not display a diagnostic message or modify the exit status to reflect an error. The -f option overrides any previous -i options. I didn't realize theat the man page was that broken. It doesn't even mention the main effect of -f -- to ignore errors for nonexistent files. This is discussed in the COMPATIBILITY section, with quite bad wording: % COMPATIBILITY % The rm utility differs from historical implementations in that the -f % option only masks attempts to remove non-existent files instead of mask- % ing a large variety of errors. The -v option is non-standard and its use % in scripts is not recommended. masks attempts to remove is strange wording. rm doesn't attempt to remove nonexistent files; it attempts to remove existent files ... The code doesn't do anything like that. It is close to POSIX, and just tries to remove existent files and then ignores the ENOENT error for nonexistent files iff -f. These bugs haven't changed since FreeBSD-1 (Net/2?). They are only in the man page. The change to POSIX conformance is documented in the FreeBSD-1 version: %%% /* * rm -- * This rm is different from historic rm's, but is expected to match * POSIX 1003.2 behavior. The most visible difference is that -f * has two specific effects now, ignore non-existent files and force * file removal. */ %%% This hasn't changed, and neither has the man page which apparently documents a version before this. I think it is misleading to say force file removal. I still tend to think of the f in rm -f being force. That seems to have been last correct in 1988 (?) before Net/2. All f does is to force is turn off some checking and interactivity. It doesn't do things like clobbering immutable flags so that unlink() can work. Bruce___ svn-src-all@freebsd.org mailing list http://lists.freebsd.org/mailman/listinfo/svn-src-all To unsubscribe, send any mail to svn-src-all-unsubscr...@freebsd.org
Re: svn commit: r272372 - stable/10/bin/rm
On Thu, Oct 02, 2014 at 02:56:05PM +1000, Bruce Evans wrote: On Wed, 1 Oct 2014, Glen Barber wrote: Log: MFC r268376 (imp): rm -rf can fail sometimes with an error from fts_read. Make it honor fflag to ignore fts_read errors, but stop deleting from that directory because no further progress can be made. I asked for this to be backed out in -current. It is not suitable for MFC. It fixes an immediate issue that prevents high-concurrent make(1) jobs from stomping over each other. If there is a more suitable solution, it needs to be introduced in head/ first, pending MFC for 10.1-RELEASE. In the meantime, we lack any alternate fix, and this resolves the immediate issue at hand. See old mail for more details. I saw the original email. I do not see a proposed patch. Glen pgpIs4bKfJbpR.pgp Description: PGP signature
Re: svn commit: r272372 - stable/10/bin/rm
On Wed, Oct 1, 2014 at 11:16 PM, Glen Barber g...@freebsd.org wrote: On Thu, Oct 02, 2014 at 02:56:05PM +1000, Bruce Evans wrote: On Wed, 1 Oct 2014, Glen Barber wrote: Log: MFC r268376 (imp): rm -rf can fail sometimes with an error from fts_read. Make it honor fflag to ignore fts_read errors, but stop deleting from that directory because no further progress can be made. I asked for this to be backed out in -current. It is not suitable for MFC. It fixes an immediate issue that prevents high-concurrent make(1) jobs from stomping over each other. The real problem is noted in this bug: https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=192490 ; the SUBDIR_PARALLEL logic is executing multiple instances of the clean/cleandir .PHONY targets. I agree with bde@ that this commit is papering over a bigger problem, but it's annoying enough and causes enough false positives that I understand why gjb@ MFCed the commit imp@ did in head. Thank you.. ___ svn-src-all@freebsd.org mailing list http://lists.freebsd.org/mailman/listinfo/svn-src-all To unsubscribe, send any mail to svn-src-all-unsubscr...@freebsd.org
Re: svn commit: r272372 - stable/10/bin/rm
On Thursday, October 02, 2014 2:16:28 am Glen Barber wrote: On Thu, Oct 02, 2014 at 02:56:05PM +1000, Bruce Evans wrote: On Wed, 1 Oct 2014, Glen Barber wrote: Log: MFC r268376 (imp): rm -rf can fail sometimes with an error from fts_read. Make it honor fflag to ignore fts_read errors, but stop deleting from that directory because no further progress can be made. I asked for this to be backed out in -current. It is not suitable for MFC. It fixes an immediate issue that prevents high-concurrent make(1) jobs from stomping over each other. If there is a more suitable solution, it needs to be introduced in head/ first, pending MFC for 10.1-RELEASE. In the meantime, we lack any alternate fix, and this resolves the immediate issue at hand. See old mail for more details. I saw the original email. I do not see a proposed patch. I think the proposed patch is to revert this and fix the broken Makefiles instead. The problem with changing rm to workaround a bug in FreeBSD's Makefiles is we are breaking rm(1) for everyone else in the world to cater to a bug in our build system. Are you happy that we are invoking the same commands in parallel for a make target? In some cases it may be ok, but there are many others where it is not (e.g. if two jobs are both compiling the same .o file, the second one might replace an object file out from under a subsequent link that starts after the first one finishes). Instead of breaking rm, you should be harassing whoever broke make (or added the SUBDIR_PARALLEL logic). Presumably you can build without -j x as a workaround. -- John Baldwin ___ svn-src-all@freebsd.org mailing list http://lists.freebsd.org/mailman/listinfo/svn-src-all To unsubscribe, send any mail to svn-src-all-unsubscr...@freebsd.org
Re: svn commit: r272372 - stable/10/bin/rm
On Thu, 2 Oct 2014, John Baldwin wrote: On Thursday, October 02, 2014 2:16:28 am Glen Barber wrote: On Thu, Oct 02, 2014 at 02:56:05PM +1000, Bruce Evans wrote: On Wed, 1 Oct 2014, Glen Barber wrote: Log: MFC r268376 (imp): rm -rf can fail sometimes with an error from fts_read. Make it honor fflag to ignore fts_read errors, but stop deleting from that directory because no further progress can be made. I asked for this to be backed out in -current. It is not suitable for MFC. It fixes an immediate issue that prevents high-concurrent make(1) jobs from stomping over each other. If there is a more suitable solution, it needs to be introduced in head/ first, pending MFC for 10.1-RELEASE. In the meantime, we lack any alternate fix, and this resolves the immediate issue at hand. See old mail for more details. I saw the original email. I do not see a proposed patch. My mail gave hints for hundreds of patches. None of them are good enough to propose. You could at least limit the special case to the errno that fts_read() returns when it gets confused instead of ignoring all errors. I think the proposed patch is to revert this and fix the broken Makefiles instead. The problem with changing rm to workaround a bug in FreeBSD's Makefiles is we are breaking rm(1) for everyone else in the world to cater to a bug in our build system. There is still the larger problem with fts_read(). Applications like rm are specified to do a complete tree walk, with special handling for files that do not exist. If fts_read() is going to abort in the middle of a tree walk, then it is unusable for implementing applications like rm. Are you happy that we are invoking the same commands in parallel for a make target? In some cases it may be ok, but there are many others where it is not (e.g. if two jobs are both compiling the same .o file, the second one might replace an object file out from under a subsequent link that starts after the first one finishes). Instead of breaking rm, you should be harassing whoever broke make (or added the SUBDIR_PARALLEL logic). Presumably you can build without -j x as a workaround. Even multiple rm -f's for cleaning an otherwise static tree aren't safe unless rm -f works as specified. It's hard to think of a single safe example using standard utilities (except make itself, used more carefully). Bruce ___ svn-src-all@freebsd.org mailing list http://lists.freebsd.org/mailman/listinfo/svn-src-all To unsubscribe, send any mail to svn-src-all-unsubscr...@freebsd.org
svn commit: r272372 - stable/10/bin/rm
Author: gjb Date: Wed Oct 1 16:18:40 2014 New Revision: 272372 URL: https://svnweb.freebsd.org/changeset/base/272372 Log: MFC r268376 (imp): rm -rf can fail sometimes with an error from fts_read. Make it honor fflag to ignore fts_read errors, but stop deleting from that directory because no further progress can be made. When building a kernel with a high -j value on a high core count machine, during the cleanobj phase we can wind up doing multiple rm -rf at the same time for modules that have subdirectories. This exposed this race (sometimes) as fts_read can return an error if the directory is removed by another rm -rf. Since the intent of the -f flag was to ignore errors, even if this was a bug in fts_read, we should ignore the error like we've been instructed to do. Approved by: re (kib) Sponsored by: The FreeBSD Foundation Modified: stable/10/bin/rm/rm.c Directory Properties: stable/10/ (props changed) Modified: stable/10/bin/rm/rm.c == --- stable/10/bin/rm/rm.c Wed Oct 1 16:16:01 2014(r272371) +++ stable/10/bin/rm/rm.c Wed Oct 1 16:18:40 2014(r272372) @@ -335,7 +335,7 @@ err: warn(%s, p-fts_path); eval = 1; } - if (errno) + if (!fflag errno) err(1, fts_read); fts_close(fts); } ___ svn-src-all@freebsd.org mailing list http://lists.freebsd.org/mailman/listinfo/svn-src-all To unsubscribe, send any mail to svn-src-all-unsubscr...@freebsd.org
Re: svn commit: r272372 - stable/10/bin/rm
On Wed, 1 Oct 2014, Glen Barber wrote: Log: MFC r268376 (imp): rm -rf can fail sometimes with an error from fts_read. Make it honor fflag to ignore fts_read errors, but stop deleting from that directory because no further progress can be made. I asked for this to be backed out in -current. It is not suitable for MFC. When building a kernel with a high -j value on a high core count machine, during the cleanobj phase we can wind up doing multiple rm -rf at the same time for modules that have subdirectories. This exposed this race (sometimes) as fts_read can return an error if the directory is removed by another rm -rf. Since the intent of the -f flag was to ignore errors, even if this was a bug in fts_read, we should ignore the error like we've been instructed to do. That may be the intent for sloppy uses, but the meaning of the -f flag is to suppress confirmation and diagnostic messages and modification of the exit status in the case of nonexistent files. (Note that POSIX's OPTIONS section is buggy and only says that this applies to nonexistent operands. Only the main part of the specification says that -f applies recursively.) It is not for breaking reporting of all detected errors. == --- stable/10/bin/rm/rm.c Wed Oct 1 16:16:01 2014(r272371) +++ stable/10/bin/rm/rm.c Wed Oct 1 16:18:40 2014(r272372) @@ -335,7 +335,7 @@ err: warn(%s, p-fts_path); eval = 1; } - if (errno) + if (!fflag errno) err(1, fts_read); fts_close(fts); } All other checks of errno in limit the effect of fflag to ENOENT errors. Changing the above to to the same might give conformance to POSIX, depending on whether fts_read() sets errno to ENOENT only when there is an ENOENT error relevant to rm. But I don't like that either. It is only correct to ignore the error for races between multiple rm's where the the interference is limited enough to allow one of the rm's to complete the removal, and moreover, all of the rm's that fail early wait to synchronize with one that actually completes. But the races are caused in the first place by lack of synchronization. See old mail for more details. Bruce ___ svn-src-all@freebsd.org mailing list http://lists.freebsd.org/mailman/listinfo/svn-src-all To unsubscribe, send any mail to svn-src-all-unsubscr...@freebsd.org
