2.6-test6: nanosleep+SIGCONT weirdness

I observe some strange behaviour in 2.6-test6 with this small program:

n.c
===
#include <sys/time.h>
#include <errno.h>

int main(int argc, char* argv[]) {
    struct timespec t = { 5000,0};
    while(nanosleep(&t,&t)<0) {
        puts("Yeah");
        if(errno!=EINTR) break;
    }
    return 0;
}

In 2.4 stracing it while doing killall -CONT ./n works fine:

# strace ./n
execve("./n", ["./n", "5000"], [/* 23 vars */]) = 0
nanosleep({5000, 0}, 0xbffffd54)        = -1 EINTR (Interrupted system 
call)
--- SIGCONT (Continued) ---
write(1, "Yeah", 4Yeah)                     = 4
write(1, "\n", 1
)                       = 1
nanosleep({4994, 730000000}, 0xbffffd54) = -1 EINTR (Interrupted system 
call)
--- SIGCONT (Continued) ---
write(1, "Yeah", 4Yeah)                     = 4
write(1, "\n", 1
)                       = 1
nanosleep({4994, 280000000}, 0xbffffd54) = -1 EINTR (Interrupted system 
call)
--- SIGCONT (Continued) ---
write(1, "Yeah", 4Yeah)                     = 4
write(1, "\n", 1
)                       = 1
nanosleep({4993, 930000000}, 

But in 2.6 it does this:

# strace ./n
execve("./n", ["./n", "400"], [/* 26 vars */]) = 0
nanosleep({5000, 0}, 0xbffffc44)        = -1 ERRNO_516 (errno 516)
--- SIGCONT (Continued) ---
setup()                                 = -1 EFAULT (Bad address)
--- SIGCONT (Continued) ---
write(1, "Yeah", 4Yeah)                     = 4
write(1, "\n", 1
)                       = 1
_exit(0)                                = ?

vda

Good catch.

That nanosleep restart seems to be broken, and quite frankly, looking at the mess in kernel/posix-timers.c I'm not all that surprised. The code is total and absolute crap. I have no idea how it's even supposed to work.

I suspect that it might just work right if you disable the nanosleep stuff by undefining FOLD_NANO_SLEEP_INTO_CLOCK_NANO_SLEEP in <linux/signal.h>. Because unlike the "folded" version in posix-timers.c, the original version at least looks sane.

Linus

I'd suggest just removing the regular nanosleep() emulation from there. The clock_nanosleep() restart is likely still broken, but at least this way the _regular_ nanosleep() system call works correctly, and fixing clock_nanosleep() is likely easier since the restart stuff doesn't have to worry about _which_ system call it should restart.

Denis, does this work for you?

Linus

-----
===== include/linux/signal.h 1.13 vs edited =====

@@ -214,7 +214,7 @@
 struct pt_regs;
 extern int get_signal_to_deliver(siginfo_t *info, struct pt_regs *regs, void *cookie);
 #endif
-#define FOLD_NANO_SLEEP_INTO_CLOCK_NANO_SLEEP
+
 #endif /* __KERNEL__ */
 
 #endif /* _LINUX_SIGNAL_H */

===== kernel/posix-timers.c 1.21 vs edited =====

@@ -1104,29 +1104,6 @@
 extern long do_clock_nanosleep(clockid_t which_clock, int flags,
 			       struct timespec *t);
 
-#ifdef FOLD_NANO_SLEEP_INTO_CLOCK_NANO_SLEEP
-
-asmlinkage long
-sys_nanosleep(struct timespec __user *rqtp, struct timespec __user *rmtp)
-{
-	struct timespec t;
-	long ret;
-
-	if (copy_from_user(&t, rqtp, sizeof (t)))
-		return -EFAULT;
-
-	if ((unsigned) t.tv_nsec >= NSEC_PER_SEC || t.tv_sec < 0)
-		return -EINVAL;
-
-	ret = do_clock_nanosleep(CLOCK_REALTIME, 0, &t);
-
-	if (ret == -ERESTART_RESTARTBLOCK && rmtp &&
-					copy_to_user(rmtp, &t, sizeof (t)))
-		return -EFAULT;
-	return ret;
-}
-#endif				// ! FOLD_NANO_SLEEP_INTO_CLOCK_NANO_SLEEP
-
 asmlinkage long
 sys_clock_nanosleep(clockid_t which_clock, int flags,
 		    const struct timespec __user *rqtp,

@@ -1244,7 +1221,7 @@
 	return 0;
 }
 /*
- * This will restart either clock_nanosleep or clock_nanosleep
+ * This will restart clock_nanosleep
  */
 long
 clock_nanosleep_restart(struct restart_block *restart_block)

===== kernel/timer.c 1.72 vs edited =====

@@ -1059,7 +1059,6 @@
 {
 	return current->pid;
 }
-#ifndef FOLD_NANO_SLEEP_INTO_CLOCK_NANO_SLEEP
 
 static long nanosleep_restart(struct restart_block *restart)
 {

@@ -1118,7 +1117,6 @@
 	}
 	return ret;
 }
-#endif // ! FOLD_NANO_SLEEP_INTO_CLOCK_NANO_SLEEP
 
 /*
  * sys_sysinfo - fill in sysinfo struct

Does not seem to work, same symptoms 8(

Although I did not do make clean since build system is supposed to be fixed. I just patched the tree where kernel was already built, and rebuild it (make bzImage modules modules_install).

Will apply to pristine test6 tree (or test9 - need to check whether it is downloaded or not yet) and retest
--
vda

Ok I retested with pristine test9+patch and it works.

vda

The problem was that the address of the users return timespec was being saved at the wrong place. In needs to be saved in the sys call interface code rather than the do_clock_nanosleep(). My tests were a bit weak as they only did one signal rather than two or more which were required to break it.

The attached patch fixes the problem and reverts the removal of the FOLD_NANO_SLEEP_INTO_CLOCK_NANO_SLEEP code. This code meets the standard WRT signal interruption AND WRT being able to sleep for longer than MAX_LONG jiffies (something the move to HZ=1000 makes more pressing).

Change log:

Revert the FOLD_NANO_SLEEP_INTO_CLOCK_NANO_SLEEP code. Fix both nanosleep() and clock_nanosleep() signal restart code.

Added a few comments about how restart works.

Add my name to the MAINTAINERS list.

George Anzinger   [email protected]
High-res-timers:  http://sourceforge.net/projects/high-res-timers/
Preemption patch: http://www.kernel.org/pub/linux/kernel/people/rml

diff -urP -I '\$Id:.*Exp \$' -X /usr/src/patch.exclude linux-2.6.0-test9-kb/MAINTAINERS linux/MAINTAINERS
@@ -1575,6 +1575,12 @@
 L:	[email protected]
 S:	Maintained
 
+POSIX CLOCKS and TIMERS
+P:	George Anzinger
+M:	[email protected]
+L:	[email protected]
+S:	Supported
+
 PNP SUPPORT
 P:	Adam Belay
 M:	[email protected]

diff -urP -I '\$Id:.*Exp \$' -X /usr/src/patch.exclude linux-2.6.0-test9-kb/include/linux/signal.h linux/include/linux/signal.h
@@ -214,7 +214,7 @@
 struct pt_regs;
 extern int get_signal_to_deliver(siginfo_t *info, struct pt_regs *regs, void *cookie);
 #endif
-
+#define FOLD_NANO_SLEEP_INTO_CLOCK_NANO_SLEEP
 #endif /* __KERNEL__ */
 
 #endif /* _LINUX_SIGNAL_H */

diff -urP -I '\$Id:.*Exp \$' -X /usr/src/patch.exclude linux-2.6.0-test9-kb/kernel/timer.c linux/kernel/timer.c
@@ -1059,6 +1059,7 @@
 {
 	return current->pid;
 }
+#ifndef FOLD_NANO_SLEEP_INTO_CLOCK_NANO_SLEEP
 
 static long nanosleep_restart(struct restart_block *restart)
 {

@@ -1117,6 +1118,7 @@
 	}
 	return ret;
 }
+#endif // ! FOLD_NANO_SLEEP_INTO_CLOCK_NANO_SLEEP
 
 /*
  * sys_sysinfo - fill in sysinfo struct

Binary files linux-2.6.0-test9-kb/scripts/bin2c and linux/scripts/bin2c differ

Something looks wrong with the syscall restart stuff in 2.6, I noticed it when doing:

sleep 600

Then doing ctrl z; fg twice.

Happens on x86 and ppc32.

Anton

No, the restart code is fine. But the posix timer code looks fundamentally broken. Try the patch I just sent, I bet it fixes it.

I can try to fix the posix-timer.c code too, but it looks like even modern glibc doesn't even export the clock_nanosleep() function. So it might not be worth fixing at this point..

Linus

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Of course it's supported. the clock_* functions are in librt. And they are supported and, when working, greatly increase the usability. The old user-level implementation is as good as we get it but really not up to the job.

- --
- --------------. ,-. 444 Castro Street
Ulrich Drepper \ ,-----------------' \ Mountain View, CA 94041 USA
Red Hat --' drepper at redhat.com ---------------------------
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.2.2 (GNU/Linux)

iD8DBQE/rUGN2ijCOnn/RHQRAtV2AKCTXNl4dAW2OXzxmmvox6uEXxBSBwCfboRq Ml2plaRE9tY7YzB19auWmUQ=
=sZrM
-----END PGP SIGNATURE-----

Okey-dokey - I verified that the clock_nanosleep() function is also broken.

I'll see if I can see what the mess is all about. That posix-timer "retry" code really is a pretty horrible thing.

Linus

Yep, things work fine with your patch.

Anton