2002-12-10 23:49:39

by Marco d'Itri

[permalink] [raw]
Subject: 2.5.51 nanosleep fails

nanosleep fails after being interrupted:

[...]
fstat64(3, {st_mode=S_IFREG|0644, st_size=5444, ...}) = 0
gettimeofday({1039564416, 703895}, NULL) = 0
nanosleep({1, 0}, NULL) = 0
fstat64(3, {st_mode=S_IFREG|0644, st_size=5444, ...}) = 0
gettimeofday({1039564417, 777452}, NULL) = 0
nanosleep({1, 0},
[1]+ Stopped strace tail -f /var/log/uucp/Log
md@wonderland:~$ fg
strace tail -f /var/log/uucp/Log
<unfinished ...>
--- SIGCONT (Continued) ---
<... nanosleep resumed> 0) = -1 ENOSYS (Function not implemented)


This can be reliably reproduced.


Linux wonderland 2.5.51 #13 Tue Dec 10 14:15:49 CET 2002 i686 unknown unknown GNU/Linux

--
ciao,
Marco


Attachments:
(No filename) (700.00 B)
(No filename) (189.00 B)
Download all attachments

2002-12-11 20:57:38

by Jim Houston

[permalink] [raw]
Subject: Re: 2.5.51 nanosleep fails

Marco d'Itri wrote:
> nanosleep fails after being interrupted:
>
> [...]
> nanosleep({1, 0},
> [1]+ Stopped strace tail -f /var/log/uucp/Log
> md@wonderland:~$ fg
> strace tail -f /var/log/uucp/Log
> <unfinished ...>
> --- SIGCONT (Continued) ---
> <... nanosleep resumed> 0) = -1 ENOSYS (Function not implemented)>
>
> This can be reliably reproduced.

Hi Marco, Everyone,

I was able to reproduce this issue. It happens on all the
kernels I tried including a stock Redhat kernel. This is just
an idiosyncrasy of strace. In this case both the strace and
tail are sent a SIGTSTP when they are put into the background.
Its not suprising that this might confuse strace.

I also tried this with a program which just does a long
nanosleep. Strace still shows a return of ENOSYS but the
program actually gets the correct EINTR.

Jim Houston - Concurrent Computer Corp.

2002-12-12 19:51:21

by Fleischer, Julie N

[permalink] [raw]
Subject: RE: 2.5.51 nanosleep fails

Jim Houston wrote:
> Marco d'Itri wrote:
> > nanosleep fails after being interrupted:
> >
> > [...]
> > nanosleep({1, 0},
> > [1]+ Stopped strace tail -f /var/log/uucp/Log
> > md@wonderland:~$ fg
> > strace tail -f /var/log/uucp/Log
> > <unfinished ...>
> > --- SIGCONT (Continued) ---
> > <... nanosleep resumed> 0) = -1 ENOSYS
> (Function not implemented)>
> >
> > This can be reliably reproduced.
> I was able to reproduce this issue. It happens on all the
> kernels I tried including a stock Redhat kernel. This is just
> an idiosyncrasy of strace. In this case both the strace and
> tail are sent a SIGTSTP when they are put into the background.
> Its not suprising that this might confuse strace.

I'm not sure this helps or is irrelevant (or already known), but I did try a
similar thing on a 2.5.50 kernel with the high-res-timers patches (all 4),
and it worked. That same test did fail (as described above) in 2.5.51 (no
patches).

I have a feeling I may be missing something as I think (?) similar things
were discussed in the "[PATCH] compatibility syscall layer (lets try again)"
thread, but I know I didn't keep up. I also may have misinterpreted the
original report.

In my test case, I set up a nanosleep to sleep for 5 seconds while I
interrupted it after 1 second with SIGSTOP and then SIGCONT.
On 2.5.50 w/HRT, it output:
nanosleep() returned success [i.e., exitted w/no errors]
Start 1039720777 sec; End 1039720782 sec
Test PASSED

On 2.5.51 w/no HRT, it output:
nanosleep() did not return success [i.e., exitted w/-1 and set errno]
Start 1039721052 sec; End 1039721053 sec
nanosleep() did not sleep long enough

>From strace of the 2.5.51 run:
nanosleep({1, 0}, {1, 0}) = 0
kill(1233, SIGSTOP) = 0
--- SIGCHLD (Child exited) ---
kill(1233, SIGCONTnanosleep() did not return success
) = 0
--- SIGCHLD (Child exited) ---
wait4(-1, [WIFEXITED(s) && WEXITSTATUS(s) == 1], 0, NULL) = 1233
gettimeofday({1039721104, 153936}, NULL) = 0

The test case was a variant of nanosleep/3-2.c on posixtest.sf.net.

- Julie
**These views are not necessarily those of my employer.**

2002-12-12 20:50:45

by Marco d'Itri

[permalink] [raw]
Subject: Re: 2.5.51 nanosleep fails

On Dec 11, Jim Houston <[email protected]> wrote:

>I was able to reproduce this issue. It happens on all the
>kernels I tried including a stock Redhat kernel. This is just
>an idiosyncrasy of strace. In this case both the strace and
No, it is not. It does not happen with 2.4.x kernel and probably did not
happen with 2.5.50.
It *does* happen with 2.5.51 even when strace is not used:

md@wonderland:~$LANG= tail -f /var/log/uucp/Log
[...]
uucico bofh - (2002-12-12 21:50:27.54 4484) Call complete (4 seconds 2336 bytes 584 bps)

[2]+ Stopped LANG= tail -f /var/log/uucp/Log
md@wonderland:~$ fg
LANG= tail -f /var/log/uucp/Log
tail: cannot read realtime clock: Bad address
[Exit 1]
md@wonderland:~$

--
ciao,
Marco

2002-12-12 22:21:06

by Jim Houston

[permalink] [raw]
Subject: Re: 2.5.51 nanosleep fails


Hi Linus, Marco, Julie, Everyone,

It really is broken. I just didn't try enough combinations.

Thanks Julie, the test suite really helps.

The problem is that nanosleep didn't check for the NULL pointer
case and always tries to copy out the remaining time. The attached
patch will fix this problem.

Jim Houston - Concurrent Computer Corp.

--- linux-2.5.51.orig/kernel/timer.c Thu Dec 12 17:08:30 2002
+++ linux-2.5.51.new/kernel/timer.c Thu Dec 12 16:59:10 2002
@@ -1040,7 +1040,7 @@
jiffies_to_timespec(expire, &t);

ret = -ERESTART_RESTARTBLOCK;
- if (copy_to_user(rmtp, &t, sizeof(t)))
+ if (rmtp && copy_to_user(rmtp, &t, sizeof(t)))
ret = -EFAULT;
/* The 'restart' block is already filled in */
}
@@ -1067,7 +1067,7 @@
if (expire) {
struct restart_block *restart;
jiffies_to_timespec(expire, &t);
- if (copy_to_user(rmtp, &t, sizeof(t)))
+ if (rmtp && copy_to_user(rmtp, &t, sizeof(t)))
return -EFAULT;

restart = &current_thread_info()->restart_block;

2002-12-13 01:21:54

by Stephen Rothwell

[permalink] [raw]
Subject: Re: 2.5.51 nanosleep fails

Hi All,

On Thu, 12 Dec 2002 17:27:46 -0500 Jim Houston <[email protected]> wrote:
>
> The problem is that nanosleep didn't check for the NULL pointer
> case and always tries to copy out the remaining time. The attached
> patch will fix this problem.

Equivalent fix for the compatibility layer ...

Not compiled, not tested (I need to get a 64 bit machine ...)

Obviously correct (hopefully) :-)
--
Cheers,
Stephen Rothwell [email protected]
http://www.canb.auug.org.au/~sfr/

diff -ruN 2.5.51/kernel/compat.c 2.5.51-ns/kernel/compat.c
--- 2.5.51/kernel/compat.c 2002-12-10 15:10:41.000000000 +1100
+++ 2.5.51-ns/kernel/compat.c 2002-12-13 12:12:00.000000000 +1100
@@ -21,8 +21,7 @@
static long compat_nanosleep_restart(struct restart_block *restart)
{
unsigned long expire = restart->arg0, now = jiffies;
- struct timespec *rmtp = (struct timespec *) restart->arg1;
- long ret;
+ struct compat_timespec *rmtp;

/* Did it expire while we handled signals? */
if (!time_after(expire, now))
@@ -30,22 +29,22 @@

current->state = TASK_INTERRUPTIBLE;
expire = schedule_timeout(expire - now);
+ if (expire == 0)
+ return 0;

- ret = 0;
- if (expire) {
+ rmtp = (struct compat_timespec *)restart->arg1;
+ if (rmtp) {
struct compat_timespec ct;
struct timespec t;

jiffies_to_timespec(expire, &t);
ct.tv_sec = t.tv_sec;
ct.tv_nsec = t.tv_nsec;
-
- ret = -ERESTART_RESTARTBLOCK;
if (copy_to_user(rmtp, &ct, sizeof(ct)))
- ret = -EFAULT;
- /* The 'restart' block is already filled in */
+ return -EFAULT;
}
- return ret;
+ /* The 'restart' block is already filled in */
+ return -ERESTART_RESTARTBLOCK;
}

asmlinkage long compat_sys_nanosleep(struct compat_timespec *rqtp,
@@ -53,8 +52,8 @@
{
struct compat_timespec ct;
struct timespec t;
+ struct restart_block *restart;
unsigned long expire;
- s32 ret;

if (copy_from_user(&ct, rqtp, sizeof(ct)))
return -EFAULT;
@@ -67,25 +66,21 @@
expire = timespec_to_jiffies(&t) + (t.tv_sec || t.tv_nsec);
current->state = TASK_INTERRUPTIBLE;
expire = schedule_timeout(expire);
+ if (expire == 0)
+ return 0;

- ret = 0;
- if (expire) {
- struct restart_block *restart;
-
+ if (rmtp) {
jiffies_to_timespec(expire, &t);
ct.tv_sec = t.tv_sec;
ct.tv_nsec = t.tv_nsec;
-
if (copy_to_user(rmtp, &ct, sizeof(ct)))
return -EFAULT;
-
- restart = &current_thread_info()->restart_block;
- restart->fn = compat_nanosleep_restart;
- restart->arg0 = jiffies + expire;
- restart->arg1 = (unsigned long) rmtp;
- ret = -ERESTART_RESTARTBLOCK;
}
- return ret;
+ restart = &current_thread_info()->restart_block;
+ restart->fn = compat_nanosleep_restart;
+ restart->arg0 = jiffies + expire;
+ restart->arg1 = (unsigned long) rmtp;
+ return -ERESTART_RESTARTBLOCK;
}

/*