Date: Thu, 2 Oct 2014 14:56:05 +1000 (EST) From: Bruce Evans <brde@optusnet.com.au> To: Glen Barber <gjb@freebsd.org> Cc: svn-src-stable@freebsd.org, svn-src-all@freebsd.org, src-committers@freebsd.org, svn-src-stable-10@freebsd.org Subject: Re: svn commit: r272372 - stable/10/bin/rm Message-ID: <20141002141656.Y1807@besplex.bde.org> In-Reply-To: <201410011618.s91GIfR5071251@svn.freebsd.org> References: <201410011618.s91GIfR5071251@svn.freebsd.org>
next in thread | previous in thread | raw e-mail | index | archive | help
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
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20141002141656.Y1807>