mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: Linus Torvalds <torvalds@linux-foundation.org>
To: Roland McGrath <roland@redhat.com>
Cc: Ingo Molnar <mingo@elte.hu>, Thomas Gleixner <tglx@linutronix.de>,
	Andrew Morton <akpm@linux-foundation.org>,
	linux-kernel@vger.kernel.org
Subject: Re: [PATCH] x86_64: fix delayed signals
Date: Thu, 10 Jul 2008 19:22:54 -0700 (PDT)	[thread overview]
Message-ID: <alpine.LFD.1.10.0807101908330.2936@woody.linux-foundation.org> (raw)
In-Reply-To: <alpine.LFD.1.10.0807101855020.2936@woody.linux-foundation.org>



On Thu, 10 Jul 2008, Linus Torvalds wrote:
> 
> So now I'm considering just putting it in before the 2.6.26 release after 
> all ;)

.. and having looked at the code, and thought about it some more, I'm 
definitely off the patch again.

The reason is actually exactly the same bug that showed up when you did 
this for x86-32 three years ago, and that may in fact still be lurking.

The endless loop of "call do_notify_resume until all the work flags are 
zero" is very fragile: it will immediately cause a hard lockup if there is 
some circumstance where do_notify_resume will not clear the flag.

And when it comes to signals, there are several cases that can cause 
TIF_SIGPENDING to not be cleared:

 - confusion about user/kernel mode, where "do_signal()" will return 
   without doing anything at all if we're in user mode.

   This was the bug we hit back in 2005 with a out-of-tree kernel-based 
   vm86 model (which hopefully has since died a painful death).

 - get_signal_to_deliver() returning and not handling the signal. 
   dequeue_signal() will do this for that collect_signal() case and for 
   the whole DRI notifier thing. The DRI notifier() case actually clears 
   TIF_SIGPENDING, but then we do "recalc_sigpending()" in the caller, so 
   it might get set again.

   I do hate that code (I know you do too), and the code _should_ block 
   the signal that gets ignored (so recalc_sigpending() should keep it 
   cleared), but it's not entirely obvious. Maybe it gets into an endless 
   loop of calling the notifier if this case ever triggers?

 - recalc_sigpending() expressly does not clear the TIF_SIGPENDING flag if 
   we hit the "freezing(current)" case. So TIF_SIGPENDING stays set for 
   freezing() processes. I think (and *hope*) they all get caught by other 
   means anyway in that whole do_notify_resume() loop, but this is another 
   of those "the freezer code is insane, I'm not going to try to think it 
   through" cases.

In short, I think your patch is fine now, but I'm also nervous enough 
about it that I'm not going to apply it. Any bugs it could expose look 
very unlikely, and if they exist they are probably bugs on 32-bit as we 
speak, but call me a worry-wart.

		Linus

  reply	other threads:[~2008-07-11  2:23 UTC|newest]

Thread overview: 46+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2008-07-10 21:50 Roland McGrath
2008-07-10 22:06 ` Linus Torvalds
2008-07-10 22:42   ` Roland McGrath
2008-07-10 22:51     ` Linus Torvalds
2008-07-10 23:02       ` Linus Torvalds
2008-07-11  0:52       ` Roland McGrath
2008-07-11  1:18         ` Linus Torvalds
2008-07-11  1:27           ` Roland McGrath
2008-07-11  1:48         ` Linus Torvalds
2008-07-11  2:02           ` Linus Torvalds
2008-07-11  2:22             ` Linus Torvalds [this message]
2008-07-11  2:26               ` Linus Torvalds
2008-07-12 12:24             ` Andi Kleen
2008-07-11  5:46 ` Ingo Molnar
2008-07-11 11:13   ` Török Edwin
2008-07-11 12:24   ` Elias Oltmanns
2008-07-11 17:58   ` Linus Torvalds
2008-07-11 18:07     ` Roland McGrath
2008-07-11 18:16       ` Linus Torvalds
2008-07-11 18:17         ` Linus Torvalds
2008-07-11 18:10     ` Linus Torvalds
2008-07-11 18:31       ` Linus Torvalds
2008-07-11 22:53         ` Arjan van de Ven
2008-07-12 10:33           ` Török Edwin
2008-07-11 20:37       ` Linus Torvalds
2008-07-11 23:22         ` Linus Torvalds
2008-07-12 10:32           ` Török Edwin
2008-07-12 13:42             ` Török Edwin
2008-07-12 14:55               ` Arjan van de Ven
2008-07-12 18:00                 ` Linus Torvalds
2008-07-12 18:15                   ` Arjan van de Ven
2008-07-12 18:28                     ` Linus Torvalds
2008-07-12 17:29             ` Linus Torvalds
2008-07-12 20:26               ` Török Edwin
2008-07-12 20:47                 ` Linus Torvalds
2008-07-12 20:57                 ` Denys Vlasenko
2008-07-13 10:46                   ` Oleg Nesterov
2008-07-13 12:34                     ` Denys Vlasenko
2008-07-13 18:36                     ` Linus Torvalds
2008-07-13 18:45                       ` Peter T. Breuer
2008-07-12 12:27     ` Andi Kleen
2008-07-12 17:41       ` Linus Torvalds
2008-07-13  9:38         ` Andi Kleen
2008-07-13 17:32           ` Linus Torvalds
2008-07-13 18:59             ` Andi Kleen
2008-07-13 19:08               ` Linus Torvalds

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=alpine.LFD.1.10.0807101908330.2936@woody.linux-foundation.org \
    --to=torvalds@linux-foundation.org \
    --cc=akpm@linux-foundation.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mingo@elte.hu \
    --cc=roland@redhat.com \
    --cc=tglx@linutronix.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®