From c403e32526bc99e886d58a09e11a4f3043af8040 Mon Sep 17 00:00:00 2001 From: Denys Vlasenko Date: Thu, 21 Oct 2010 18:19:35 +0200 Subject: sleep: code shrink Use less stack by using same "sigset_t set" object for new and saved signal set, remove redundant clearing of set, and do not save/restore errno around sigprocmask(SIG_SETMASK) - it never changes it. While at it, improve comments and make code style consistent across sleep.c file. text data bss dec hex filename - 242 0 0 242 f2 libc/unistd/sleep.o + 197 0 0 197 c5 libc/unistd/sleep.o Signed-off-by: Denys Vlasenko --- libc/unistd/sleep.c | 47 +++++++++++++++++++++-------------------------- libc/unistd/usleep.c | 14 +++++++------- 2 files changed, 28 insertions(+), 33 deletions(-) (limited to 'libc/unistd') diff --git a/libc/unistd/sleep.c b/libc/unistd/sleep.c index b0031f022..d844d5b11 100644 --- a/libc/unistd/sleep.c +++ b/libc/unistd/sleep.c @@ -49,50 +49,45 @@ unsigned int sleep (unsigned int sec) unsigned int sleep (unsigned int seconds) { struct timespec ts = { .tv_sec = (long int) seconds, .tv_nsec = 0 }; - sigset_t set, oset; + sigset_t set; unsigned int result; /* This is not necessary but some buggy programs depend on this. */ - if (seconds == 0) - { + if (seconds == 0) { # ifdef CANCELLATION_P - CANCELLATION_P (THREAD_SELF); + CANCELLATION_P (THREAD_SELF); # endif - return 0; - } + return 0; + } /* Linux will wake up the system call, nanosleep, when SIGCHLD arrives even if SIGCHLD is ignored. We have to deal with it in libc. We block SIGCHLD first. */ __sigemptyset (&set); __sigaddset (&set, SIGCHLD); - sigprocmask (SIG_BLOCK, &set, &oset); /* can't fail */ + sigprocmask (SIG_BLOCK, &set, &set); /* never fails */ - /* If SIGCHLD is already blocked, we don't have to do anything. */ - if (!__sigismember (&oset, SIGCHLD)) - { - int saved_errno; + /* If SIGCHLD was already blocked, no need to check SIG_IGN. Else... */ + if (!__sigismember (&set, SIGCHLD)) { struct sigaction oact; - __sigemptyset (&set); - __sigaddset (&set, SIGCHLD); - + /* Is SIGCHLD set to SIG_IGN? */ sigaction (SIGCHLD, NULL, &oact); /* never fails */ + if (oact.sa_handler == SIG_IGN) { + //int saved_errno; - if (oact.sa_handler == SIG_IGN) - { - /* We should leave SIGCHLD blocked. */ + /* Yes, run nanosleep with SIGCHLD blocked. */ result = nanosleep (&ts, &ts); - saved_errno = errno; - /* Restore the original signal mask. */ - sigprocmask (SIG_SETMASK, &oset, NULL); - __set_errno (saved_errno); - } - else - { - /* We should unblock SIGCHLD. Restore the original signal mask. */ - sigprocmask (SIG_SETMASK, &oset, NULL); + /* Unblock SIGCHLD by restoring signal mask. */ + /* this sigprocmask call never fails, thus never updates errno, + and therefore we don't need to save/restore it. */ + //saved_errno = errno; + sigprocmask (SIG_SETMASK, &set, NULL); + //__set_errno (saved_errno); + } else { + /* No workaround needed, unblock SIGCHLD by restoring signal mask. */ + sigprocmask (SIG_SETMASK, &set, NULL); result = nanosleep (&ts, &ts); } } diff --git a/libc/unistd/usleep.c b/libc/unistd/usleep.c index 61ddb900a..8d01703ec 100644 --- a/libc/unistd/usleep.c +++ b/libc/unistd/usleep.c @@ -14,19 +14,19 @@ int usleep (__useconds_t usec) { - const struct timespec ts = { - .tv_sec = (long int) (usec / 1000000), - .tv_nsec = (long int) (usec % 1000000) * 1000ul - }; - return(nanosleep(&ts, NULL)); + const struct timespec ts = { + .tv_sec = (long int) (usec / 1000000), + .tv_nsec = (long int) (usec % 1000000) * 1000ul + }; + return nanosleep(&ts, NULL); } #else /* __UCLIBC_HAS_REALTIME__ */ int usleep (__useconds_t usec) { struct timeval tv; - tv.tv_sec = 0; - tv.tv_usec = usec; + tv.tv_sec = (long int) (usec / 1000000); + tv.tv_usec = (long int) (usec % 1000000); return select(0, NULL, NULL, NULL, &tv); } #endif /* __UCLIBC_HAS_REALTIME__ */ -- cgit v1.2.3 From 5e8d3975e2cb45df16038e2a99c74abfb252d44a Mon Sep 17 00:00:00 2001 From: Denys Vlasenko Date: Fri, 22 Oct 2010 13:48:57 +0200 Subject: sleep: remove commented-out code. no code changes It can be easily reconstructed, since it's obvious Signed-off-by: Denys Vlasenko --- libc/unistd/sleep.c | 4 ---- 1 file changed, 4 deletions(-) (limited to 'libc/unistd') diff --git a/libc/unistd/sleep.c b/libc/unistd/sleep.c index d844d5b11..55e646380 100644 --- a/libc/unistd/sleep.c +++ b/libc/unistd/sleep.c @@ -74,17 +74,13 @@ unsigned int sleep (unsigned int seconds) /* Is SIGCHLD set to SIG_IGN? */ sigaction (SIGCHLD, NULL, &oact); /* never fails */ if (oact.sa_handler == SIG_IGN) { - //int saved_errno; - /* Yes, run nanosleep with SIGCHLD blocked. */ result = nanosleep (&ts, &ts); /* Unblock SIGCHLD by restoring signal mask. */ /* this sigprocmask call never fails, thus never updates errno, and therefore we don't need to save/restore it. */ - //saved_errno = errno; sigprocmask (SIG_SETMASK, &set, NULL); - //__set_errno (saved_errno); } else { /* No workaround needed, unblock SIGCHLD by restoring signal mask. */ sigprocmask (SIG_SETMASK, &set, NULL); -- cgit v1.2.3 From 62fb840ea00d9d10446959d290d93a45065220f4 Mon Sep 17 00:00:00 2001 From: Denys Vlasenko Date: Fri, 22 Oct 2010 15:22:40 +0200 Subject: sleep: check "SIGCHLD is SIG_IGN'ed" first. Saves two syscalls in common case text data bss dec hex filename - 197 0 0 197 c5 libc/unistd/sleep.o + 168 0 0 168 a8 libc/unistd/sleep.o Signed-off-by: Denys Vlasenko --- libc/unistd/sleep.c | 40 ++++++++++++++++++---------------------- 1 file changed, 18 insertions(+), 22 deletions(-) (limited to 'libc/unistd') diff --git a/libc/unistd/sleep.c b/libc/unistd/sleep.c index 55e646380..481e635a2 100644 --- a/libc/unistd/sleep.c +++ b/libc/unistd/sleep.c @@ -50,6 +50,7 @@ unsigned int sleep (unsigned int seconds) { struct timespec ts = { .tv_sec = (long int) seconds, .tv_nsec = 0 }; sigset_t set; + struct sigaction oact; unsigned int result; /* This is not necessary but some buggy programs depend on this. */ @@ -62,33 +63,28 @@ unsigned int sleep (unsigned int seconds) /* Linux will wake up the system call, nanosleep, when SIGCHLD arrives even if SIGCHLD is ignored. We have to deal with it - in libc. We block SIGCHLD first. */ + in libc. */ + __sigemptyset (&set); __sigaddset (&set, SIGCHLD); - sigprocmask (SIG_BLOCK, &set, &set); /* never fails */ - /* If SIGCHLD was already blocked, no need to check SIG_IGN. Else... */ + /* Is SIGCHLD set to SIG_IGN? */ + sigaction (SIGCHLD, NULL, &oact); /* never fails */ + if (oact.sa_handler == SIG_IGN) { + /* Yes. Block SIGCHLD, save old mask. */ + sigprocmask (SIG_BLOCK, &set, &set); /* never fails */ + } + + /* Run nanosleep, with SIGCHLD blocked if SIGCHLD is SIG_IGNed. */ + result = nanosleep (&ts, &ts); + if (!__sigismember (&set, SIGCHLD)) { - struct sigaction oact; - - /* Is SIGCHLD set to SIG_IGN? */ - sigaction (SIGCHLD, NULL, &oact); /* never fails */ - if (oact.sa_handler == SIG_IGN) { - /* Yes, run nanosleep with SIGCHLD blocked. */ - result = nanosleep (&ts, &ts); - - /* Unblock SIGCHLD by restoring signal mask. */ - /* this sigprocmask call never fails, thus never updates errno, - and therefore we don't need to save/restore it. */ - sigprocmask (SIG_SETMASK, &set, NULL); - } else { - /* No workaround needed, unblock SIGCHLD by restoring signal mask. */ - sigprocmask (SIG_SETMASK, &set, NULL); - result = nanosleep (&ts, &ts); - } + /* We did block SIGCHLD, and old mask had no SIGCHLD bit. + IOW: we need to unblock SIGCHLD now. Do it. */ + /* this sigprocmask call never fails, thus never updates errno, + and therefore we don't need to save/restore it. */ + sigprocmask (SIG_SETMASK, &set, NULL); /* never fails */ } - else - result = nanosleep (&ts, &ts); if (result != 0) /* Round remaining time. */ -- cgit v1.2.3 From 251a3c19cb4bba47fcd38c697b3d7679b3edb137 Mon Sep 17 00:00:00 2001 From: Denys Vlasenko Date: Fri, 22 Oct 2010 15:24:13 +0200 Subject: sleep: employ __USE_EXTERN_INLINES (with necessary fixes) __USE_EXTERN_INLINES was unused and had bit-rotted, had to fix it when it didn't work as intended at first. text data bss dec hex filename - 168 0 0 168 a8 libc/unistd/sleep.o + 146 0 0 146 92 libc/unistd/sleep.o Signed-off-by: Denys Vlasenko --- libc/unistd/sleep.c | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) (limited to 'libc/unistd') diff --git a/libc/unistd/sleep.c b/libc/unistd/sleep.c index 481e635a2..438f5e282 100644 --- a/libc/unistd/sleep.c +++ b/libc/unistd/sleep.c @@ -20,8 +20,13 @@ #include #include -#include #include +/* Want fast and small __sigemptyset/__sigaddset/__sigismember: */ +/* TODO: make them available (to everybody) without this hack */ +#ifndef __USE_EXTERN_INLINES +# define __USE_EXTERN_INLINES 1 +#endif +#include -- cgit v1.2.3 From e9b9c97c33c52e9eafaf6bf6d682e43ecfa3aea7 Mon Sep 17 00:00:00 2001 From: Denys Vlasenko Date: Fri, 22 Oct 2010 15:46:04 +0200 Subject: sleep: tiny code shrink ...or rather, it WILL BE code shrink when gcc become clever enough to not emit a second, useless XORing of ebx: 31 db xor %ebx,%ebx 85 c0 test %eax,%eax 74 11 je 73 <__GI_sleep+0x73> 31 db xor %ebx,%ebx <=== ?! Signed-off-by: Denys Vlasenko --- libc/unistd/sleep.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) (limited to 'libc/unistd') diff --git a/libc/unistd/sleep.c b/libc/unistd/sleep.c index 438f5e282..6a237e3f9 100644 --- a/libc/unistd/sleep.c +++ b/libc/unistd/sleep.c @@ -82,6 +82,10 @@ unsigned int sleep (unsigned int seconds) /* Run nanosleep, with SIGCHLD blocked if SIGCHLD is SIG_IGNed. */ result = nanosleep (&ts, &ts); + if (result != 0) { + /* Got EINTR. Return remaining time. */ + result = (unsigned int) ts.tv_sec + (ts.tv_nsec >= 500000000L); + } if (!__sigismember (&set, SIGCHLD)) { /* We did block SIGCHLD, and old mask had no SIGCHLD bit. @@ -91,10 +95,6 @@ unsigned int sleep (unsigned int seconds) sigprocmask (SIG_SETMASK, &set, NULL); /* never fails */ } - if (result != 0) - /* Round remaining time. */ - result = (unsigned int) ts.tv_sec + (ts.tv_nsec >= 500000000L); - return result; } #endif -- cgit v1.2.3 From 162cfaea20d807f0ae329efe39292a9b22593b41 Mon Sep 17 00:00:00 2001 From: Denys Vlasenko Date: Fri, 22 Oct 2010 17:01:05 +0200 Subject: *: inline constant __sig{add,del}set and __sigismember text data bss dec hex filename - 318 4 0 322 142 libc/pwd_grp/lckpwdf.o + 312 4 0 316 13c libc/pwd_grp/lckpwdf.o - 166 0 1 167 a7 libc/stdlib/abort.o + 157 0 1 158 9e libc/stdlib/abort.o - 42 0 0 42 2a libc/sysdeps/linux/common/pause.o + 27 0 0 27 1b libc/sysdeps/linux/common/pause.o Signed-off-by: Denys Vlasenko --- libc/unistd/sleep.c | 8 +------- 1 file changed, 1 insertion(+), 7 deletions(-) (limited to 'libc/unistd') diff --git a/libc/unistd/sleep.c b/libc/unistd/sleep.c index 6a237e3f9..c4b8a4812 100644 --- a/libc/unistd/sleep.c +++ b/libc/unistd/sleep.c @@ -20,14 +20,8 @@ #include #include -#include -/* Want fast and small __sigemptyset/__sigaddset/__sigismember: */ -/* TODO: make them available (to everybody) without this hack */ -#ifndef __USE_EXTERN_INLINES -# define __USE_EXTERN_INLINES 1 -#endif #include - +#include /* version perusing nanosleep */ -- cgit v1.2.3 From fed8640bceb8fe937efaceaf2ca506b03f76f621 Mon Sep 17 00:00:00 2001 From: Denys Vlasenko Date: Fri, 29 Oct 2010 03:44:57 +0200 Subject: sleep: add comment with test program for SIG_IGNed SIGCHLD Signed-off-by: Denys Vlasenko --- libc/unistd/sleep.c | 43 ++++++++++++++++++++++++++++++++++++++----- 1 file changed, 38 insertions(+), 5 deletions(-) (limited to 'libc/unistd') diff --git a/libc/unistd/sleep.c b/libc/unistd/sleep.c index c4b8a4812..5bd3755c4 100644 --- a/libc/unistd/sleep.c +++ b/libc/unistd/sleep.c @@ -27,7 +27,35 @@ /* version perusing nanosleep */ #if defined __UCLIBC_HAS_REALTIME__ -#if 0 +/* I am unable to reproduce alleged "Linux quirk". + * I used the following test program: +#include +#include +#include +static void dummy(int sig) {} +int main() { + struct timespec t = { 2, 0 }; + if (fork() == 0) { + sleep(1); + return 0; + } + signal(SIGCHLD, SIG_DFL); // + signal(SIGCHLD, dummy); // Pick one + signal(SIGCHLD, SIG_IGN); // + nanosleep(&t, &t); + return 0; +} + * On 2.6.35-rc4: + * With SIG_DFL, nanosleep() is not interrupted by SIGCHLD. Ok. + * With dummy handler, nanosleep() is interrupted by SIGCHLD. Ok. + * With SIG_IGN, nanosleep() is NOT interrupted by SIGCHLD. + * It looks like sleep's workaround for SIG_IGN is no longer needed? + * The only emails I can find are from 1998 - + * google for "sleep ignore sigchld". + */ + +# if 0 + /* This is a quick and dirty, but not 100% compliant with * the stupid SysV SIGCHLD vs. SIG_IGN behaviour. It is * fine unless you are messing with SIGCHLD... */ @@ -40,7 +68,7 @@ unsigned int sleep (unsigned int sec) return res; } -#else +# else /* We are going to use the `nanosleep' syscall of the kernel. But the kernel does not implement the sstupid SysV SIGCHLD vs. SIG_IGN @@ -54,9 +82,9 @@ unsigned int sleep (unsigned int seconds) /* This is not necessary but some buggy programs depend on this. */ if (seconds == 0) { -# ifdef CANCELLATION_P +# ifdef CANCELLATION_P CANCELLATION_P (THREAD_SELF); -# endif +# endif return 0; } @@ -91,8 +119,11 @@ unsigned int sleep (unsigned int seconds) return result; } -#endif + +# endif + #else /* __UCLIBC_HAS_REALTIME__ */ + /* no nanosleep, use signals and alarm() */ static void sleep_alarm_handler(int attribute_unused sig) { @@ -140,5 +171,7 @@ unsigned int sleep (unsigned int seconds) return result > seconds ? 0 : seconds - result; } + #endif /* __UCLIBC_HAS_REALTIME__ */ + libc_hidden_def(sleep) -- cgit v1.2.3 From 18e0f7e66e06b3b128e3d0a8837909f7f474ab69 Mon Sep 17 00:00:00 2001 From: Denys Vlasenko Date: Fri, 29 Oct 2010 04:01:27 +0200 Subject: sleep: document testing result on 2.4.x kernels Signed-off-by: Denys Vlasenko --- libc/unistd/sleep.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'libc/unistd') diff --git a/libc/unistd/sleep.c b/libc/unistd/sleep.c index 5bd3755c4..211c8434e 100644 --- a/libc/unistd/sleep.c +++ b/libc/unistd/sleep.c @@ -45,7 +45,7 @@ int main() { nanosleep(&t, &t); return 0; } - * On 2.6.35-rc4: + * Testing on 2.4.20 and on 2.6.35-rc4: * With SIG_DFL, nanosleep() is not interrupted by SIGCHLD. Ok. * With dummy handler, nanosleep() is interrupted by SIGCHLD. Ok. * With SIG_IGN, nanosleep() is NOT interrupted by SIGCHLD. -- cgit v1.2.3 From 69c3616ade8e268708ef24e5b091f94f2ffe08d4 Mon Sep 17 00:00:00 2001 From: Denys Vlasenko Date: Sat, 30 Oct 2010 15:06:20 +0200 Subject: sleep: include Linus' email in the comment Signed-off-by: Denys Vlasenko --- libc/unistd/sleep.c | 26 +++++++++++++++++++++----- 1 file changed, 21 insertions(+), 5 deletions(-) (limited to 'libc/unistd') diff --git a/libc/unistd/sleep.c b/libc/unistd/sleep.c index 211c8434e..9db115a29 100644 --- a/libc/unistd/sleep.c +++ b/libc/unistd/sleep.c @@ -46,12 +46,28 @@ int main() { return 0; } * Testing on 2.4.20 and on 2.6.35-rc4: - * With SIG_DFL, nanosleep() is not interrupted by SIGCHLD. Ok. - * With dummy handler, nanosleep() is interrupted by SIGCHLD. Ok. - * With SIG_IGN, nanosleep() is NOT interrupted by SIGCHLD. + * With SIG_DFL, nanosleep is not interrupted by SIGCHLD. Ok. + * With dummy handler, nanosleep is interrupted by SIGCHLD. Ok. + * With SIG_IGN, nanosleep is NOT interrupted by SIGCHLD. * It looks like sleep's workaround for SIG_IGN is no longer needed? - * The only emails I can find are from 1998 - - * google for "sleep ignore sigchld". + * The only emails I can find are from 1998 (!): + * ---------- + * Subject: Re: sleep ignore sigchld + * From: Linus Torvalds + * Date: Mon, 16 Nov 1998 11:02:15 -0800 (PST) + * + * On Mon, 16 Nov 1998, H. J. Lu wrote: + * > That is a kernel bug. SIGCHLD is a special one. Usually it cannot + * > be ignored. [snip...] + * + * No can do. + * + * "nanosleep()" is implemented in a bad way that makes it impossible to + * restart it cleanly. It was done that way because glibc wanted it that way, + * not because it's a good idea. [snip...] + * ---------- + * I assume that in the passed twelve+ years, nanosleep got fixed, + * but the hack in sleep to work around broken nanosleep was never removed. */ # if 0 -- cgit v1.2.3