[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: sort link failure due to sleep
From: |
Eric Blake |
Subject: |
Re: sort link failure due to sleep |
Date: |
Mon, 23 Nov 2009 22:20:56 +0000 (UTC) |
User-agent: |
Loom/3.14 (http://gmane.org/) |
Jim Meyering <jim <at> meyering.net> writes:
> > Or should I go with an alternative patch of making sort.c use xnanosleep
> > instead of sleep? (For that matter, if we went with xnanosleep, we could
make
> > sort's child wait loop start with something smaller, like 0.25 seconds,
instead
> > of a minimum granularity of 1 second.)
>
> Hi Eric,
>
> Your alternative patch sounds better still.
How about this? Note there is a slight change in semantics to forking done by
sort - I chose to cut the minimum wait time by a factor of 4, but increase the
number of retries only by 2, for a net result that we give up twice as fast.
Is this discrepancy okay, or do I still need to tweak the scale of one (or
both) of the changes? There is also the potential of a subtle change of
behavior of how sort reacts if invoked with SIGALRM blocked or ignored, since
POSIX requires that nanosleep work independently of SIGALRM, while sleep may be
hampered by interference with the SIGALRM signal (although for really old
platforms that lack nanosleep, the gnulib xnanosleep fallback calls usleep or
even sleep, so we can't quite guarantee no SIGALRM interference).
From: Eric Blake <address@hidden>
Date: Mon, 23 Nov 2009 14:59:18 -0700
Subject: [PATCH] build: fix link failure on cygwin
Cygwin 1.5 has a broken sleep, and the gnulib tests dragged in
rpl_sleep which then caused a link failure because it wasn't in
libcoreutils.a. We could solve it by using the gnulib sleep module.
However, sleep and usleep may interact poorly with SIGALRM, and they
have less granularity; so it is better to adopt a policy that if we
must sleep, prefer xnanosleep.
* src/sort.c (pipe_fork): Use xnanosleep, to avoid the need for
rpl_sleep on cygwin, and to reduce granularity.
(MAX_FORK_TRIES_COMPRESS, MAX_FORK_TRIES_DECOMPRESS): Double,
to account for reduction in granularity.
* src/tail.c (tail_file): Use xnanosleep in debug code.
* cfg.mk (sc_prohibit_sleep): New rule.
---
cfg.mk | 6 ++++++
src/sort.c | 9 +++++----
src/tail.c | 2 +-
3 files changed, 12 insertions(+), 5 deletions(-)
diff --git a/cfg.mk b/cfg.mk
index babd99b..c57c0d3 100644
--- a/cfg.mk
+++ b/cfg.mk
@@ -239,4 +239,10 @@ sc_require_stdio_safer:
else :; \
fi
+# Prefer xnanosleep over other less-precise sleep methods
+sc_prohibit_sleep:
+ @re='\<(nano|u)?sleep \(' \
+ msg='prefer xnanosleep over other sleep interfaces' \
+ $(_prohibit_regexp)
+
include $(srcdir)/dist-check.mk
diff --git a/src/sort.c b/src/sort.c
index 0213fee..b2ee8db 100644
--- a/src/sort.c
+++ b/src/sort.c
@@ -44,6 +44,7 @@
#include "strnumcmp.h"
#include "xmemcoll.h"
#include "xmemxfrm.h"
+#include "xnanosleep.h"
#include "xstrtol.h"
#if HAVE_SYS_RESOURCE_H
@@ -108,12 +109,12 @@ enum
(we retry if the fork call fails). We don't _need_ to compress
temp files, this is just to reduce disk access, so this number
can be small. */
- MAX_FORK_TRIES_COMPRESS = 2,
+ MAX_FORK_TRIES_COMPRESS = 4,
/* The number of times we should try to fork a decompression process.
If we can't fork a decompression process, we can't sort, so this
number should be big. */
- MAX_FORK_TRIES_DECOMPRESS = 8
+ MAX_FORK_TRIES_DECOMPRESS = 16
};
/* The representation of the decimal point in the current locale. */
@@ -868,7 +869,7 @@ pipe_fork (int pipefds[2], size_t tries)
#if HAVE_WORKING_FORK
struct tempnode *saved_temphead;
int saved_errno;
- unsigned int wait_retry = 1;
+ double wait_retry = 0.25;
pid_t pid IF_LINT (= -1);
struct cs_status cs;
@@ -895,7 +896,7 @@ pipe_fork (int pipefds[2], size_t tries)
break;
else
{
- sleep (wait_retry);
+ xnanosleep (wait_retry);
wait_retry *= 2;
reap_some ();
}
diff --git a/src/tail.c b/src/tail.c
index 9a2f5ae..2bd9e3d 100644
--- a/src/tail.c
+++ b/src/tail.c
@@ -1619,7 +1619,7 @@ tail_file (struct File_spec *f, uintmax_t n_units)
/* Before the tail function provided `read_pos', there was
a race condition described in the URL below. This sleep
call made the window big enough to exercise the problem. */
- sleep (1);
+ xnanosleep (1);
#endif
f->errnum = ok - 1;
if (fstat (fd, &stats) < 0)
--
1.6.4.2