All of lore.kernel.org
 help / color / mirror / Atom feed
From: Andi Kleen <andi@firstfloor.org>
To: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Roland McGrath <roland@redhat.com>, 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: Sat, 12 Jul 2008 14:24:59 +0200	[thread overview]
Message-ID: <87r69z2stg.fsf@basil.nowhere.org> (raw)
In-Reply-To: <alpine.LFD.1.10.0807101855020.2936@woody.linux-foundation.org> (Linus Torvalds's message of "Thu, 10 Jul 2008 19:02:25 -0700 (PDT)")

Linus Torvalds <torvalds@linux-foundation.org> writes:
>
> That said, seeing the full history of this same thing on the x86-32 side 
> actually makes me _much_ happier about the patch. Because now I can tell 
> when we did it, and what problems it seems to have caused (answer: 
> "apparently none that are possibly relevant on x86-64").

FWIW I agree with Roland that the 64bit behaviour has a race window
that can lead to delayed signals. I had actually fixed this some
months ago myself in my tree because this was noticed by David (who
was comparing to sparc I thin) during code review, but never pushed it
out because it didn't get enough testing (it only ran on my
workstation for some time)

I think his patch is actually doing way too much work though, especially
it is not needed to repeat all checks for the full work mask. I'm
appending the patch I was using, which only rechecks signals.

Obviously it's still not something that should be used without a lot 
of testing.

But in theory it's ok.

> So now I'm considering just putting it in before the 2.6.26 release after 
> all ;)

That would seem rather hazardous.

-Andi

---

Fix lost signal race on x86-64

Originally noticed by Dave Miller during code review

Signed-off-by: Andi Kleen <ak@linux.intel.com>

---
 arch/x86/kernel/entry_64.S |    6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

Index: linux/arch/x86/kernel/entry_64.S
===================================================================
--- linux.orig/arch/x86/kernel/entry_64.S
+++ linux/arch/x86/kernel/entry_64.S
@@ -305,7 +305,7 @@ sysret_signal:
 	leaq -ARGOFFSET(%rsp),%rdi # &pt_regs -> arg1
 	xorl %esi,%esi # oldset -> arg2
 	call ptregscall_common
-1:	movl $_TIF_NEED_RESCHED,%edi
+1:	movl $_TIF_NEED_RESCHED|_TIF_SIGPENDING,%edi
 	/* Use IRET because user could have changed frame. This
 	   works because ptregscall_common has called FIXUP_TOP_OF_STACK. */
 	DISABLE_INTERRUPTS(CLBR_NONE)
@@ -393,7 +393,7 @@ int_signal:
 	movq %rsp,%rdi		# &ptregs -> arg1
 	xorl %esi,%esi		# oldset -> arg2
 	call do_notify_resume
-1:	movl $_TIF_NEED_RESCHED,%edi	
+1:	movl $_TIF_NEED_RESCHED|_TIF_SIGPENDING,%edi
 int_restore_rest:
 	RESTORE_REST
 	DISABLE_INTERRUPTS(CLBR_NONE)
@@ -647,7 +647,7 @@ retint_signal:
 	RESTORE_REST
 	DISABLE_INTERRUPTS(CLBR_NONE)
 	TRACE_IRQS_OFF
-	movl $_TIF_NEED_RESCHED,%edi
+	movl $_TIF_NEED_RESCHED|_TIF_SIGPENDING,%edi
 	GET_THREAD_INFO(%rcx)
 	jmp retint_check
 

  parent reply	other threads:[~2008-07-12 12:25 UTC|newest]

Thread overview: 46+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2008-07-10 21:50 [PATCH] x86_64: fix delayed signals 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
2008-07-11  2:26               ` Linus Torvalds
2008-07-12 12:24             ` Andi Kleen [this message]
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=87r69z2stg.fsf@basil.nowhere.org \
    --to=andi@firstfloor.org \
    --cc=akpm@linux-foundation.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mingo@elte.hu \
    --cc=roland@redhat.com \
    --cc=tglx@linutronix.de \
    --cc=torvalds@linux-foundation.org \
    /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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.