[PATCH] Remove bogus WARN_ON in futex_wait

[PATCH] Remove bogus WARN_ON in futex_wait

[Andi Kleen]

futex_wait goes to an interruptible sleep, but does a WARN_ON later
if it wakes up early. But waking up early is totally legal, since
the sleep is interruptible and any signal can wake it up.

Remove the WARN_ON checking for that.

diff -u linux/kernel/Makefile-o linux/kernel/Makefile
diff -u linux/kernel/futex.c-o linux/kernel/futex.c
--- linux/kernel/futex.c-o	2004-03-21 21:12:13.000000000 +0100
+++ linux/kernel/futex.c	2004-05-19 10:01:02.000000000 +0200
@@ -504,8 +504,6 @@
 		return 0;
 	if (time == 0)
 		return -ETIMEDOUT;
-	/* A spurious wakeup should never happen. */
-	WARN_ON(!signal_pending(current));
 	return -EINTR;
 
  out_unqueue:

Re: [PATCH] Remove bogus WARN_ON in futex_wait

[Muli Ben-Yehuda]

[-- Attachment #1: Type: text/plain, Size: 1283 bytes --]

On Wed, May 19, 2004 at 12:23:50PM +0200, Andi Kleen wrote:
> 
> futex_wait goes to an interruptible sleep, but does a WARN_ON later
> if it wakes up early. But waking up early is totally legal, since
> the sleep is interruptible and any signal can wake it up.

That's not what the WARN_ON is saynig, unless I'm missing
something. It's checking if we were woken up early and there's no
signal pending for us. 

Cheers, 
Muli 

> Remove the WARN_ON checking for that.
> 
> diff -u linux/kernel/Makefile-o linux/kernel/Makefile
> diff -u linux/kernel/futex.c-o linux/kernel/futex.c
> --- linux/kernel/futex.c-o	2004-03-21 21:12:13.000000000 +0100
> +++ linux/kernel/futex.c	2004-05-19 10:01:02.000000000 +0200
> @@ -504,8 +504,6 @@
>  		return 0;
>  	if (time == 0)
>  		return -ETIMEDOUT;
> -	/* A spurious wakeup should never happen. */
> -	WARN_ON(!signal_pending(current));
>  	return -EINTR;
>  
>   out_unqueue:
> -
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/

-- 
Muli Ben-Yehuda
http://www.mulix.org | http://mulix.livejournal.com/


[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 189 bytes --]

Re: [PATCH] Remove bogus WARN_ON in futex_wait

[Andi Kleen]

On Wed, 19 May 2004 13:43:40 +0300
Muli Ben-Yehuda <mulix@mulix.org> wrote:

> On Wed, May 19, 2004 at 12:23:50PM +0200, Andi Kleen wrote:
> > 
> > futex_wait goes to an interruptible sleep, but does a WARN_ON later
> > if it wakes up early. But waking up early is totally legal, since
> > the sleep is interruptible and any signal can wake it up.
> 
> That's not what the WARN_ON is saynig, unless I'm missing
> something. It's checking if we were woken up early and there's no
> signal pending for us. 

True. Anyways, it seems to happen in practice.

-Andi

Re: [PATCH] Remove bogus WARN_ON in futex_wait

[Muli Ben-Yehuda]

[-- Attachment #1: Type: text/plain, Size: 875 bytes --]

On Wed, May 19, 2004 at 12:50:01PM +0200, Andi Kleen wrote:
> On Wed, 19 May 2004 13:43:40 +0300
> Muli Ben-Yehuda <mulix@mulix.org> wrote:
> 
> > On Wed, May 19, 2004 at 12:23:50PM +0200, Andi Kleen wrote:
> > > 
> > > futex_wait goes to an interruptible sleep, but does a WARN_ON later
> > > if it wakes up early. But waking up early is totally legal, since
> > > the sleep is interruptible and any signal can wake it up.
> > 
> > That's not what the WARN_ON is saynig, unless I'm missing
> > something. It's checking if we were woken up early and there's no
> > signal pending for us. 
> 
> True. Anyways, it seems to happen in practice.

Granted; the interesting question is whether this is harmless or
something to worry about. Any ideas why it happens? 

Cheers, 
Muli 
-- 
Muli Ben-Yehuda
http://www.mulix.org | http://mulix.livejournal.com/


[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 189 bytes --]

Re: [PATCH] Remove bogus WARN_ON in futex_wait

[Nick Piggin]

Andi Kleen wrote:
> On Wed, 19 May 2004 13:43:40 +0300
> Muli Ben-Yehuda <mulix@mulix.org> wrote:
> 
> 
>>On Wed, May 19, 2004 at 12:23:50PM +0200, Andi Kleen wrote:
>>
>>>futex_wait goes to an interruptible sleep, but does a WARN_ON later
>>>if it wakes up early. But waking up early is totally legal, since
>>>the sleep is interruptible and any signal can wake it up.
>>
>>That's not what the WARN_ON is saynig, unless I'm missing
>>something. It's checking if we were woken up early and there's no
>>signal pending for us. 
> 
> 
> True. Anyways, it seems to happen in practice.
> 

Somebody thought an early wakeup there was buggy (but harmless).
There was a patch for kernel/sched.c that was supposed to print
the source of the wakeup. I think the WARN_ON in the futex is
pretty uninteresting without the sched.c patch...

Re: [PATCH] Remove bogus WARN_ON in futex_wait

[Daniel Jacobowitz]

On Wed, May 19, 2004 at 12:50:01PM +0200, Andi Kleen wrote:
> On Wed, 19 May 2004 13:43:40 +0300
> Muli Ben-Yehuda <mulix@mulix.org> wrote:
> 
> > On Wed, May 19, 2004 at 12:23:50PM +0200, Andi Kleen wrote:
> > > 
> > > futex_wait goes to an interruptible sleep, but does a WARN_ON later
> > > if it wakes up early. But waking up early is totally legal, since
> > > the sleep is interruptible and any signal can wake it up.
> > 
> > That's not what the WARN_ON is saynig, unless I'm missing
> > something. It's checking if we were woken up early and there's no
> > signal pending for us. 
> 
> True. Anyways, it seems to happen in practice.

I'm guessing that at least one of the ways to trigger this is with
ptrace.  Linux has a nasty habit of waking "wake on signal" sleeps
without a signal if ptrace cancels the signal.

-- 
Daniel Jacobowitz

Re: [PATCH] Remove bogus WARN_ON in futex_wait

[Rusty Russell]

On Wed, 2004-05-19 at 20:50, Andi Kleen wrote:
> On Wed, 19 May 2004 13:43:40 +0300
> Muli Ben-Yehuda <mulix@mulix.org> wrote:
> 
> > On Wed, May 19, 2004 at 12:23:50PM +0200, Andi Kleen wrote:
> > > 
> > > futex_wait goes to an interruptible sleep, but does a WARN_ON later
> > > if it wakes up early. But waking up early is totally legal, since
> > > the sleep is interruptible and any signal can wake it up.
> > 
> > That's not what the WARN_ON is saynig, unless I'm missing
> > something. It's checking if we were woken up early and there's no
> > signal pending for us. 
> 
> True. Anyways, it seems to happen in practice.

Which we've been trying to figure out.  We return -EINTR in this case
even though it's a lie.  Don't know if it breaks anything, but I
*really* want to know who the buggy waker is before pronouncing it
harmless.

Rusty.
-- 
Anyone who quotes me in their signature is an idiot -- Rusty Russell

Re: [PATCH] Remove bogus WARN_ON in futex_wait

[Andi Kleen]

On Thu, 20 May 2004 10:52:46 +1000
Rusty Russell <rusty@rustcorp.com.au> wrote:

> Which we've been trying to figure out.  We return -EINTR in this case
> even though it's a lie.  Don't know if it breaks anything, but I
> *really* want to know who the buggy waker is before pronouncing it
> harmless.

e.g. from ptrace, like Daniel said. I don't think it was from ptrace
in the case where I saw a report, but it shows that the WARN_ON is bogus.
If you really want to know what causes it you would need more debugging code
as Nick pointed out too. So, the WARN_ON in its current form
is known to trigger in a valid situation and not sufficient to find 
possible problems. I would remove it.

-Andi