LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
* [PATCH] Remove bogus WARN_ON in futex_wait
@ 2004-05-19 10:23 Andi Kleen
  2004-05-19 10:43 ` Muli Ben-Yehuda
  0 siblings, 1 reply; 8+ messages in thread
From: Andi Kleen @ 2004-05-19 10:23 UTC (permalink / raw)
  To: linux-kernel, akpm, rusty


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:

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] Remove bogus WARN_ON in futex_wait
  2004-05-19 10:23 [PATCH] Remove bogus WARN_ON in futex_wait Andi Kleen
@ 2004-05-19 10:43 ` Muli Ben-Yehuda
  2004-05-19 10:50   ` Andi Kleen
  0 siblings, 1 reply; 8+ messages in thread
From: Muli Ben-Yehuda @ 2004-05-19 10:43 UTC (permalink / raw)
  To: Andi Kleen; +Cc: linux-kernel, akpm, rusty

[-- 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 --]

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] Remove bogus WARN_ON in futex_wait
  2004-05-19 10:43 ` Muli Ben-Yehuda
@ 2004-05-19 10:50   ` Andi Kleen
  2004-05-19 10:54     ` Muli Ben-Yehuda
                       ` (3 more replies)
  0 siblings, 4 replies; 8+ messages in thread
From: Andi Kleen @ 2004-05-19 10:50 UTC (permalink / raw)
  To: Muli Ben-Yehuda; +Cc: linux-kernel, akpm, rusty

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

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] Remove bogus WARN_ON in futex_wait
  2004-05-19 10:50   ` Andi Kleen
@ 2004-05-19 10:54     ` Muli Ben-Yehuda
  2004-05-19 11:07     ` Nick Piggin
                       ` (2 subsequent siblings)
  3 siblings, 0 replies; 8+ messages in thread
From: Muli Ben-Yehuda @ 2004-05-19 10:54 UTC (permalink / raw)
  To: Andi Kleen; +Cc: linux-kernel, akpm, rusty

[-- 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 --]

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] Remove bogus WARN_ON in futex_wait
  2004-05-19 10:50   ` Andi Kleen
  2004-05-19 10:54     ` Muli Ben-Yehuda
@ 2004-05-19 11:07     ` Nick Piggin
  2004-05-19 14:42     ` Daniel Jacobowitz
  2004-05-20  0:52     ` Rusty Russell
  3 siblings, 0 replies; 8+ messages in thread
From: Nick Piggin @ 2004-05-19 11:07 UTC (permalink / raw)
  To: Andi Kleen; +Cc: Muli Ben-Yehuda, linux-kernel, akpm, rusty

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...

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] Remove bogus WARN_ON in futex_wait
  2004-05-19 10:50   ` Andi Kleen
  2004-05-19 10:54     ` Muli Ben-Yehuda
  2004-05-19 11:07     ` Nick Piggin
@ 2004-05-19 14:42     ` Daniel Jacobowitz
  2004-05-20  0:52     ` Rusty Russell
  3 siblings, 0 replies; 8+ messages in thread
From: Daniel Jacobowitz @ 2004-05-19 14:42 UTC (permalink / raw)
  To: Andi Kleen; +Cc: Muli Ben-Yehuda, linux-kernel, akpm, rusty

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

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] Remove bogus WARN_ON in futex_wait
  2004-05-19 10:50   ` Andi Kleen
                       ` (2 preceding siblings ...)
  2004-05-19 14:42     ` Daniel Jacobowitz
@ 2004-05-20  0:52     ` Rusty Russell
  2004-05-20  8:41       ` Andi Kleen
  3 siblings, 1 reply; 8+ messages in thread
From: Rusty Russell @ 2004-05-20  0:52 UTC (permalink / raw)
  To: Andi Kleen; +Cc: Muli Ben-Yehuda, lkml - Kernel Mailing List, Andrew Morton

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


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] Remove bogus WARN_ON in futex_wait
  2004-05-20  0:52     ` Rusty Russell
@ 2004-05-20  8:41       ` Andi Kleen
  0 siblings, 0 replies; 8+ messages in thread
From: Andi Kleen @ 2004-05-20  8:41 UTC (permalink / raw)
  To: Rusty Russell; +Cc: mulix, linux-kernel, akpm

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

^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2004-05-20  8:43 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2004-05-19 10:23 [PATCH] Remove bogus WARN_ON in futex_wait Andi Kleen
2004-05-19 10:43 ` Muli Ben-Yehuda
2004-05-19 10:50   ` Andi Kleen
2004-05-19 10:54     ` Muli Ben-Yehuda
2004-05-19 11:07     ` Nick Piggin
2004-05-19 14:42     ` Daniel Jacobowitz
2004-05-20  0:52     ` Rusty Russell
2004-05-20  8:41       ` Andi Kleen

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).