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
next prev parent 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®