* [RESEND] [RFC][PATCH X86_32 1/2]: Call do_notify_resume() with interrupts enabled
@ 2011-10-25 14:18 Srikar Dronamraju
2011-10-25 14:21 ` [RFC] [PATCH x86 2/2] Cleanup do_int3 Srikar Dronamraju
` (2 more replies)
0 siblings, 3 replies; 13+ messages in thread
From: Srikar Dronamraju @ 2011-10-25 14:18 UTC (permalink / raw)
To: Peter Zijlstra, H. Peter Anvin
Cc: Steven Rostedt, Linus Torvalds, Christoph Hellwig,
Ananth N Mavinakayanahalli, Thomas Gleixner, Andi Kleen,
Oleg Nesterov, Andrew Morton, Jim Keniston, Roland McGrath,
Shuah Khan, Ingo Molnar, Alexander van Heukelum, Russell King,
LKML
do_notify_resume() gets called with interrupts disabled on x86_32. This
is different from the x86_64 behavior, where interrupts are enabled at
the time.
Queries on lkml on this issue hasn't yielded any clear answer. Lets make
x86_32 behave the same as x86_64, unless there is a real reason to
maintain status quo.
Please refer https://lkml.org/lkml/2011/9/27/130 for more details
A similar change was suggested in arm
https://lkml.org/lkml/2011/8/25/231
My 32-bit machine works fine (tm) with the patch below
Signed-off-by: Srikar Dronamraju <srikar@linux.vnet.ibm.com>
---
arch/x86/kernel/entry_32.S | 4 ++++
1 files changed, 4 insertions(+)
diff --git a/arch/x86/kernel/entry_32.S b/arch/x86/kernel/entry_32.S
index f3f6f53..22d0e21 100644
--- a/arch/x86/kernel/entry_32.S
+++ b/arch/x86/kernel/entry_32.S
@@ -625,6 +625,8 @@ work_notifysig: # deal with pending signals and
movl %esp, %eax
jne work_notifysig_v86 # returning to kernel-space or
# vm86-space
+ TRACE_IRQS_ON
+ ENABLE_INTERRUPTS(CLBR_NONE)
xorl %edx, %edx
call do_notify_resume
jmp resume_userspace_sig
@@ -638,6 +640,8 @@ work_notifysig: # deal with pending signals and
#else
movl %esp, %eax
#endif
+ TRACE_IRQS_ON
+ ENABLE_INTERRUPTS(CLBR_NONE)
xorl %edx, %edx
call do_notify_resume
jmp resume_userspace_sig
^ permalink raw reply related [flat|nested] 13+ messages in thread
* [RFC] [PATCH x86 2/2] Cleanup do_int3
2011-10-25 14:18 [RESEND] [RFC][PATCH X86_32 1/2]: Call do_notify_resume() with interrupts enabled Srikar Dronamraju
@ 2011-10-25 14:21 ` Srikar Dronamraju
2011-10-25 15:52 ` Oleg Nesterov
` (2 more replies)
2011-10-25 16:14 ` [RESEND] [RFC][PATCH X86_32 1/2]: Call do_notify_resume() with interrupts enabled Oleg Nesterov
2011-12-06 9:42 ` [tip:x86/asm] x86: " tip-bot for Srikar Dronamraju
2 siblings, 3 replies; 13+ messages in thread
From: Srikar Dronamraju @ 2011-10-25 14:21 UTC (permalink / raw)
To: Peter Zijlstra, H. Peter Anvin
Cc: Steven Rostedt, Linus Torvalds, Christoph Hellwig,
Ananth N Mavinakayanahalli, Thomas Gleixner, Andi Kleen,
Oleg Nesterov, Andrew Morton, Jim Keniston, Roland McGrath,
Ingo Molnar, Alexander van Heukelum, LKML
Since there is a possibility of !KPROBES int3 listeners and since
DIE_TRAP is currently not being used by anybody, notify all listeners
with DIE_INT3.
Signed-off-by: Ananth N Mavinakayanahalli <ananth@in.ibm.com>
Signed-off-by: Srikar Dronamraju <srikar@linux.vnet.ibm.com>
---
arch/x86/kernel/traps.c | 7 +------
1 files changed, 1 insertions(+), 6 deletions(-)
diff --git a/arch/x86/kernel/traps.c b/arch/x86/kernel/traps.c
index 6913369..c9a4be5 100644
--- a/arch/x86/kernel/traps.c
+++ b/arch/x86/kernel/traps.c
@@ -461,15 +461,10 @@ dotraplinkage void __kprobes do_int3(struct pt_regs *regs, long error_code)
== NOTIFY_STOP)
return;
#endif /* CONFIG_KGDB_LOW_LEVEL_TRAP */
-#ifdef CONFIG_KPROBES
+
if (notify_die(DIE_INT3, "int3", regs, error_code, 3, SIGTRAP)
== NOTIFY_STOP)
return;
-#else
- if (notify_die(DIE_TRAP, "int3", regs, error_code, 3, SIGTRAP)
- == NOTIFY_STOP)
- return;
-#endif
preempt_conditional_sti(regs);
do_trap(3, SIGTRAP, "int3", regs, error_code, NULL);
^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [RFC] [PATCH x86 2/2] Cleanup do_int3
2011-10-25 14:21 ` [RFC] [PATCH x86 2/2] Cleanup do_int3 Srikar Dronamraju
@ 2011-10-25 15:52 ` Oleg Nesterov
2011-10-28 1:49 ` Masami Hiramatsu
2011-12-06 9:42 ` [tip:x86/asm] x86: Clean up and extend do_int3() tip-bot for Srikar Dronamraju
2 siblings, 0 replies; 13+ messages in thread
From: Oleg Nesterov @ 2011-10-25 15:52 UTC (permalink / raw)
To: Srikar Dronamraju
Cc: Peter Zijlstra, H. Peter Anvin, Steven Rostedt, Linus Torvalds,
Christoph Hellwig, Ananth N Mavinakayanahalli, Thomas Gleixner,
Andi Kleen, Andrew Morton, Jim Keniston, Roland McGrath,
Ingo Molnar, Alexander van Heukelum, LKML
On 10/25, Srikar Dronamraju wrote:
>
> Since there is a possibility of !KPROBES int3 listeners and since
> DIE_TRAP is currently not being used by anybody, notify all listeners
> with DIE_INT3.
Obviously I can't ack this change, but it looks like the bugfix to me.
The current code uses DIE_INT3 or DIE_TRAP depending on CONFIG_KPROBES,
this can't be right.
> Signed-off-by: Ananth N Mavinakayanahalli <ananth@in.ibm.com>
> Signed-off-by: Srikar Dronamraju <srikar@linux.vnet.ibm.com>
> ---
> arch/x86/kernel/traps.c | 7 +------
> 1 files changed, 1 insertions(+), 6 deletions(-)
>
> diff --git a/arch/x86/kernel/traps.c b/arch/x86/kernel/traps.c
> index 6913369..c9a4be5 100644
> --- a/arch/x86/kernel/traps.c
> +++ b/arch/x86/kernel/traps.c
> @@ -461,15 +461,10 @@ dotraplinkage void __kprobes do_int3(struct pt_regs *regs, long error_code)
> == NOTIFY_STOP)
> return;
> #endif /* CONFIG_KGDB_LOW_LEVEL_TRAP */
> -#ifdef CONFIG_KPROBES
> +
> if (notify_die(DIE_INT3, "int3", regs, error_code, 3, SIGTRAP)
> == NOTIFY_STOP)
> return;
> -#else
> - if (notify_die(DIE_TRAP, "int3", regs, error_code, 3, SIGTRAP)
> - == NOTIFY_STOP)
> - return;
> -#endif
>
> preempt_conditional_sti(regs);
> do_trap(3, SIGTRAP, "int3", regs, error_code, NULL);
>
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [RESEND] [RFC][PATCH X86_32 1/2]: Call do_notify_resume() with interrupts enabled
2011-10-25 14:18 [RESEND] [RFC][PATCH X86_32 1/2]: Call do_notify_resume() with interrupts enabled Srikar Dronamraju
2011-10-25 14:21 ` [RFC] [PATCH x86 2/2] Cleanup do_int3 Srikar Dronamraju
@ 2011-10-25 16:14 ` Oleg Nesterov
2011-10-26 12:38 ` Linus Torvalds
2011-12-06 9:42 ` [tip:x86/asm] x86: " tip-bot for Srikar Dronamraju
2 siblings, 1 reply; 13+ messages in thread
From: Oleg Nesterov @ 2011-10-25 16:14 UTC (permalink / raw)
To: Srikar Dronamraju
Cc: Peter Zijlstra, H. Peter Anvin, Steven Rostedt, Linus Torvalds,
Christoph Hellwig, Ananth N Mavinakayanahalli, Thomas Gleixner,
Andi Kleen, Andrew Morton, Jim Keniston, Roland McGrath,
Shuah Khan, Ingo Molnar, Alexander van Heukelum, Russell King,
LKML
On 10/25, Srikar Dronamraju wrote:
>
> do_notify_resume() gets called with interrupts disabled on x86_32. This
> is different from the x86_64 behavior, where interrupts are enabled at
> the time.
And note that do_notify_resume() paths assume that irqs are enabled.
key_replace_session_keyring/get_signal_to_deliver start with _lock_irq.
IOW, I vote for this change even if I can't ack it (although it looks
"obviously correct" to me). In fact refrigerator() looks buggy without
this change. Yes, it enables irqs but only "by accident", via unlock_irq().
And we are going to remove this recalc_sigpending() from freezer.
> Queries on lkml on this issue hasn't yielded any clear answer. Lets make
> x86_32 behave the same as x86_64, unless there is a real reason to
> maintain status quo.
>
> Please refer https://lkml.org/lkml/2011/9/27/130 for more details
>
> A similar change was suggested in arm
> https://lkml.org/lkml/2011/8/25/231
>
> My 32-bit machine works fine (tm) with the patch below
>
> Signed-off-by: Srikar Dronamraju <srikar@linux.vnet.ibm.com>
> ---
> arch/x86/kernel/entry_32.S | 4 ++++
> 1 files changed, 4 insertions(+)
>
> diff --git a/arch/x86/kernel/entry_32.S b/arch/x86/kernel/entry_32.S
> index f3f6f53..22d0e21 100644
> --- a/arch/x86/kernel/entry_32.S
> +++ b/arch/x86/kernel/entry_32.S
> @@ -625,6 +625,8 @@ work_notifysig: # deal with pending signals and
> movl %esp, %eax
> jne work_notifysig_v86 # returning to kernel-space or
> # vm86-space
> + TRACE_IRQS_ON
> + ENABLE_INTERRUPTS(CLBR_NONE)
> xorl %edx, %edx
> call do_notify_resume
> jmp resume_userspace_sig
> @@ -638,6 +640,8 @@ work_notifysig: # deal with pending signals and
> #else
> movl %esp, %eax
> #endif
> + TRACE_IRQS_ON
> + ENABLE_INTERRUPTS(CLBR_NONE)
> xorl %edx, %edx
> call do_notify_resume
> jmp resume_userspace_sig
>
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [RESEND] [RFC][PATCH X86_32 1/2]: Call do_notify_resume() with interrupts enabled
2011-10-25 16:14 ` [RESEND] [RFC][PATCH X86_32 1/2]: Call do_notify_resume() with interrupts enabled Oleg Nesterov
@ 2011-10-26 12:38 ` Linus Torvalds
2011-10-26 15:10 ` Russell King
2011-11-03 4:43 ` Srikar Dronamraju
0 siblings, 2 replies; 13+ messages in thread
From: Linus Torvalds @ 2011-10-26 12:38 UTC (permalink / raw)
To: Oleg Nesterov
Cc: Srikar Dronamraju, Peter Zijlstra, H. Peter Anvin, Steven Rostedt,
Christoph Hellwig, Ananth N Mavinakayanahalli, Thomas Gleixner,
Andi Kleen, Andrew Morton, Jim Keniston, Roland McGrath,
Shuah Khan, Ingo Molnar, Alexander van Heukelum, Russell King,
LKML
Ingo, Thomas, I think this is your call, but it seems valid,
Linus
On Tue, Oct 25, 2011 at 6:14 PM, Oleg Nesterov <oleg@redhat.com> wrote:
> On 10/25, Srikar Dronamraju wrote:
>>
>> do_notify_resume() gets called with interrupts disabled on x86_32. This
>> is different from the x86_64 behavior, where interrupts are enabled at
>> the time.
>
> And note that do_notify_resume() paths assume that irqs are enabled.
> key_replace_session_keyring/get_signal_to_deliver start with _lock_irq.
>
> IOW, I vote for this change even if I can't ack it (although it looks
> "obviously correct" to me). In fact refrigerator() looks buggy without
> this change. Yes, it enables irqs but only "by accident", via unlock_irq().
> And we are going to remove this recalc_sigpending() from freezer.
>
>
>> Queries on lkml on this issue hasn't yielded any clear answer. Lets make
>> x86_32 behave the same as x86_64, unless there is a real reason to
>> maintain status quo.
>>
>> Please refer https://lkml.org/lkml/2011/9/27/130 for more details
>>
>> A similar change was suggested in arm
>> https://lkml.org/lkml/2011/8/25/231
>>
>> My 32-bit machine works fine (tm) with the patch below
>>
>> Signed-off-by: Srikar Dronamraju <srikar@linux.vnet.ibm.com>
>> ---
>> arch/x86/kernel/entry_32.S | 4 ++++
>> 1 files changed, 4 insertions(+)
>>
>> diff --git a/arch/x86/kernel/entry_32.S b/arch/x86/kernel/entry_32.S
>> index f3f6f53..22d0e21 100644
>> --- a/arch/x86/kernel/entry_32.S
>> +++ b/arch/x86/kernel/entry_32.S
>> @@ -625,6 +625,8 @@ work_notifysig: # deal with pending signals and
>> movl %esp, %eax
>> jne work_notifysig_v86 # returning to kernel-space or
>> # vm86-space
>> + TRACE_IRQS_ON
>> + ENABLE_INTERRUPTS(CLBR_NONE)
>> xorl %edx, %edx
>> call do_notify_resume
>> jmp resume_userspace_sig
>> @@ -638,6 +640,8 @@ work_notifysig: # deal with pending signals and
>> #else
>> movl %esp, %eax
>> #endif
>> + TRACE_IRQS_ON
>> + ENABLE_INTERRUPTS(CLBR_NONE)
>> xorl %edx, %edx
>> call do_notify_resume
>> jmp resume_userspace_sig
>>
>
>
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [RESEND] [RFC][PATCH X86_32 1/2]: Call do_notify_resume() with interrupts enabled
2011-10-26 12:38 ` Linus Torvalds
@ 2011-10-26 15:10 ` Russell King
2011-10-26 17:34 ` Roland McGrath
2011-10-26 18:58 ` Oleg Nesterov
2011-11-03 4:43 ` Srikar Dronamraju
1 sibling, 2 replies; 13+ messages in thread
From: Russell King @ 2011-10-26 15:10 UTC (permalink / raw)
To: Linus Torvalds
Cc: Oleg Nesterov, Srikar Dronamraju, Peter Zijlstra, H. Peter Anvin,
Steven Rostedt, Christoph Hellwig, Ananth N Mavinakayanahalli,
Thomas Gleixner, Andi Kleen, Andrew Morton, Jim Keniston,
Roland McGrath, Shuah Khan, Ingo Molnar, Alexander van Heukelum,
LKML
On Wed, Oct 26, 2011 at 02:38:39PM +0200, Linus Torvalds wrote:
> Ingo, Thomas, I think this is your call, but it seems valid,
Hi Linus,
I guess I should've talked to you about this during a moment during the
kernel summit, but as I'm now back home it'll have to be email.
I've been toying with a similar patch for ARM, but I keep feeling uneasy
about having interrupts enabled in this path (even though they get enabled
in the depths of the signal handling code.)
I worry about are race condition like the following:
syscall enter
...
syscall returns -ERESTARTNOHAND
check for signal
signal pending, but no handler, setup for restart
interrupt happens, sets need_resched
need_resched set
switch to another thread
...
something happens which queues SIGIO
switch back to this thread
check for signal
signal pending, has handler, but we've setup for a restart
return to userspace
run SIGIO handler
restart syscall
This feels like it violates the expectations of the syscall being
restarted - which explicitly asks to be restarted only if there wasn't
a handler run.
I've been working on the assumption that this is a problem and we should
do something about it - but it's non-trivial to solve all the corner cases.
We can do a lot better with the restarting if we delay setting up for a
restart until either we setup the user stack for the sig handler or
immediately before returning to userspace (with a TIF flag.)
If you're interested in seeing where I got to, the patch is available at:
https://lkml.org/lkml/2011/8/25/231
However, that doesn't solve the (probably unsolvable) case where an
ERESTARTSYS syscall is interrupted by a SA_RESTART-marked handler, and
while that handler is running it is then interrupted by a non-SA_RESTART-
marked handler. I think that is far too an obscure case to care about
though.
--
Russell King
Linux kernel 2.6 ARM Linux - http://www.arm.linux.org.uk/
maintainer of:
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [RESEND] [RFC][PATCH X86_32 1/2]: Call do_notify_resume() with interrupts enabled
2011-10-26 15:10 ` Russell King
@ 2011-10-26 17:34 ` Roland McGrath
2011-10-26 18:58 ` Oleg Nesterov
1 sibling, 0 replies; 13+ messages in thread
From: Roland McGrath @ 2011-10-26 17:34 UTC (permalink / raw)
To: Russell King
Cc: Linus Torvalds, Oleg Nesterov, Srikar Dronamraju, Peter Zijlstra,
H. Peter Anvin, Steven Rostedt, Christoph Hellwig,
Ananth N Mavinakayanahalli, Thomas Gleixner, Andi Kleen,
Andrew Morton, Jim Keniston, Shuah Khan, Ingo Molnar,
Alexander van Heukelum, LKML
> I worry about are race condition like the following:
>
> syscall enter
> ...
> syscall returns -ERESTARTNOHAND
> check for signal
> signal pending, but no handler, setup for restart
> interrupt happens, sets need_resched
> need_resched set
> switch to another thread
> ...
> something happens which queues SIGIO
> switch back to this thread
> check for signal
> signal pending, has handler, but we've setup for a restart
> return to userspace
> run SIGIO handler
> restart syscall
I don't think this is a violation. That ERESTARTNOHAND referred to
restarting for the signal that interrupted that first syscall. That
decision has already been made. You can consider that restart already to
have begun before the second signal arrives. In essence, it's like the
first syscall had never been entered, because the user state is already
rolled back to where the syscall instruction is about to execute (correct
me if I'm wrong about that). Then the second signal comes along and
prevents that instruction from executing. It's really no different from if
you had gotten all the way back to user mode, but not yet executed the
first user instruction (i.e. the syscall instruction), when the second
signal interrupted the user task.
> However, that doesn't solve the (probably unsolvable) case where an
> ERESTARTSYS syscall is interrupted by a SA_RESTART-marked handler, and
> while that handler is running it is then interrupted by a non-SA_RESTART-
> marked handler. I think that is far too an obscure case to care about
> though.
I'm personally for caring about all cases, no matter how obscure. For that
particular one, I don't think I see the problem. I'm probably overlooking
something. ERESTARTSYS says that the user state should be rolled back to
the syscall instruction before the handler setup. So it's just as if the
signal had arrived before user mode executed the syscall instruction in the
first place. That just affects the sigcontext saved before the handler
runs. Nothing about that interacts with further interruptions during the
handler. What am I missing?
However, we already have the fundamental constraint that
ERESTART_RESTARTBLOCK states are lost when a handler runs.
Thanks,
Roland
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [RESEND] [RFC][PATCH X86_32 1/2]: Call do_notify_resume() with interrupts enabled
2011-10-26 15:10 ` Russell King
2011-10-26 17:34 ` Roland McGrath
@ 2011-10-26 18:58 ` Oleg Nesterov
1 sibling, 0 replies; 13+ messages in thread
From: Oleg Nesterov @ 2011-10-26 18:58 UTC (permalink / raw)
To: Russell King
Cc: Linus Torvalds, Srikar Dronamraju, Peter Zijlstra, H. Peter Anvin,
Steven Rostedt, Christoph Hellwig, Ananth N Mavinakayanahalli,
Thomas Gleixner, Andi Kleen, Andrew Morton, Jim Keniston,
Roland McGrath, Shuah Khan, Ingo Molnar, Alexander van Heukelum,
LKML
On 10/26, Russell King wrote:
>
> I've been toying with a similar patch for ARM, but I keep feeling uneasy
> about having interrupts enabled in this path (even though they get enabled
> in the depths of the signal handling code.)
>
> I worry about are race condition like the following:
>
> syscall enter
> ...
> syscall returns -ERESTARTNOHAND
> check for signal
> signal pending, but no handler, setup for restart
> interrupt happens, sets need_resched
> need_resched set
> switch to another thread
> ...
> something happens which queues SIGIO
> switch back to this thread
I don't understand how "interrupts disabled" can help... A signal
can come without preempt_schedule().
> check for signal
> signal pending, has handler, but we've setup for a restart
> return to userspace
> run SIGIO handler
> restart syscall
>
> This feels like it violates the expectations of the syscall being
> restarted - which explicitly asks to be restarted only if there wasn't
> a handler run.
But this doesn't differ from the case when this signal comes after
the sycall was already restarted?
> However, that doesn't solve the (probably unsolvable) case where an
> ERESTARTSYS syscall is interrupted by a SA_RESTART-marked handler, and
> while that handler is running it is then interrupted by a non-SA_RESTART-
> marked handler. I think that is far too an obscure case to care about
> though.
If I understand correctly, this was already discussed:
Re: HR timers prevent an itimer from generating EINTR?
http://marc.info/?l=linux-kernel&m=125384722012869
Unfortunately, marc.info doesn't show the authoritative reply from
Roland, but he agreed with "not a problem".
Or I misunderstood?
Oleg.
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [RFC] [PATCH x86 2/2] Cleanup do_int3
2011-10-25 14:21 ` [RFC] [PATCH x86 2/2] Cleanup do_int3 Srikar Dronamraju
2011-10-25 15:52 ` Oleg Nesterov
@ 2011-10-28 1:49 ` Masami Hiramatsu
2011-11-18 9:41 ` Srikar Dronamraju
2011-12-06 9:42 ` [tip:x86/asm] x86: Clean up and extend do_int3() tip-bot for Srikar Dronamraju
2 siblings, 1 reply; 13+ messages in thread
From: Masami Hiramatsu @ 2011-10-28 1:49 UTC (permalink / raw)
To: Srikar Dronamraju
Cc: Peter Zijlstra, H. Peter Anvin, Steven Rostedt, Linus Torvalds,
Christoph Hellwig, Ananth N Mavinakayanahalli, Thomas Gleixner,
Andi Kleen, Oleg Nesterov, Andrew Morton, Jim Keniston,
Roland McGrath, Ingo Molnar, Alexander van Heukelum, LKML
(2011/10/25 23:21), Srikar Dronamraju wrote:
> Since there is a possibility of !KPROBES int3 listeners and since
> DIE_TRAP is currently not being used by anybody, notify all listeners
> with DIE_INT3.
I'm not sure why this CONFIG_KPROBES dependency had been introduced,
but it's the time to fix it. do_int3 should send DIE_INT3 always.
Acked-by: Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com>
Thank you!
>
> Signed-off-by: Ananth N Mavinakayanahalli <ananth@in.ibm.com>
> Signed-off-by: Srikar Dronamraju <srikar@linux.vnet.ibm.com>
> ---
> arch/x86/kernel/traps.c | 7 +------
> 1 files changed, 1 insertions(+), 6 deletions(-)
>
> diff --git a/arch/x86/kernel/traps.c b/arch/x86/kernel/traps.c
> index 6913369..c9a4be5 100644
> --- a/arch/x86/kernel/traps.c
> +++ b/arch/x86/kernel/traps.c
> @@ -461,15 +461,10 @@ dotraplinkage void __kprobes do_int3(struct pt_regs *regs, long error_code)
> == NOTIFY_STOP)
> return;
> #endif /* CONFIG_KGDB_LOW_LEVEL_TRAP */
> -#ifdef CONFIG_KPROBES
> +
> if (notify_die(DIE_INT3, "int3", regs, error_code, 3, SIGTRAP)
> == NOTIFY_STOP)
> return;
> -#else
> - if (notify_die(DIE_TRAP, "int3", regs, error_code, 3, SIGTRAP)
> - == NOTIFY_STOP)
> - return;
> -#endif
>
> preempt_conditional_sti(regs);
> do_trap(3, SIGTRAP, "int3", regs, error_code, NULL);
>
> --
> 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/
--
Masami HIRAMATSU
Software Platform Research Dept. Linux Technology Center
Hitachi, Ltd., Yokohama Research Laboratory
E-mail: masami.hiramatsu.pt@hitachi.com
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [RESEND] [RFC][PATCH X86_32 1/2]: Call do_notify_resume() with interrupts enabled
2011-10-26 12:38 ` Linus Torvalds
2011-10-26 15:10 ` Russell King
@ 2011-11-03 4:43 ` Srikar Dronamraju
1 sibling, 0 replies; 13+ messages in thread
From: Srikar Dronamraju @ 2011-11-03 4:43 UTC (permalink / raw)
To: Linus Torvalds, Thomas Gleixner, Ingo Molnar
Cc: Oleg Nesterov, Peter Zijlstra, H. Peter Anvin, Steven Rostedt,
Christoph Hellwig, Ananth N Mavinakayanahalli, Andi Kleen,
Andrew Morton, Jim Keniston, Roland McGrath, Shuah Khan,
Alexander van Heukelum, Russell King, LKML
> Ingo, Thomas, I think this is your call, but it seems valid,
>
> Linus
>
Hey Ingo, Thomas,
Can you please let me know if you have taken a look at this patch and if
you have any reservations?
I would also be happy to know your thoughts on the other patch in the
series which makes sure that notify_die is called irrespective of
kprobes.
--
Thanks and Regards
Srikar
> On Tue, Oct 25, 2011 at 6:14 PM, Oleg Nesterov <oleg@redhat.com> wrote:
> > On 10/25, Srikar Dronamraju wrote:
> >>
> >> do_notify_resume() gets called with interrupts disabled on x86_32. This
> >> is different from the x86_64 behavior, where interrupts are enabled at
> >> the time.
> >
> > And note that do_notify_resume() paths assume that irqs are enabled.
> > key_replace_session_keyring/get_signal_to_deliver start with _lock_irq.
> >
> > IOW, I vote for this change even if I can't ack it (although it looks
> > "obviously correct" to me). In fact refrigerator() looks buggy without
> > this change. Yes, it enables irqs but only "by accident", via unlock_irq().
> > And we are going to remove this recalc_sigpending() from freezer.
> >
> >
> >> Queries on lkml on this issue hasn't yielded any clear answer. Lets make
> >> x86_32 behave the same as x86_64, unless there is a real reason to
> >> maintain status quo.
> >>
> >> Please refer https://lkml.org/lkml/2011/9/27/130 for more details
> >>
> >> A similar change was suggested in arm
> >> https://lkml.org/lkml/2011/8/25/231
> >>
> >> My 32-bit machine works fine (tm) with the patch below
> >>
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [RFC] [PATCH x86 2/2] Cleanup do_int3
2011-10-28 1:49 ` Masami Hiramatsu
@ 2011-11-18 9:41 ` Srikar Dronamraju
0 siblings, 0 replies; 13+ messages in thread
From: Srikar Dronamraju @ 2011-11-18 9:41 UTC (permalink / raw)
To: Masami Hiramatsu, Linus Torvalds, Ingo Molnar
Cc: Peter Zijlstra, H. Peter Anvin, Steven Rostedt, Christoph Hellwig,
Ananth N Mavinakayanahalli, Thomas Gleixner, Andi Kleen,
Oleg Nesterov, Andrew Morton, Jim Keniston, Roland McGrath,
Alexander van Heukelum, LKML
> (2011/10/25 23:21), Srikar Dronamraju wrote:
> > Since there is a possibility of !KPROBES int3 listeners and since
> > DIE_TRAP is currently not being used by anybody, notify all listeners
> > with DIE_INT3.
>
> I'm not sure why this CONFIG_KPROBES dependency had been introduced,
> but it's the time to fix it. do_int3 should send DIE_INT3 always.
>
> Acked-by: Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com>
>
> Thank you!
>
Hi Ingo,
Is there a reason for not taking this patch?
Oleg, Masami, Alexander(who was the last to modify this code) have
all been in favour of this patch.
I didnt get any response from you for the other patch in this series
too.
--
Thanks and Regards
Srikar
> >
> > Signed-off-by: Ananth N Mavinakayanahalli <ananth@in.ibm.com>
> > Signed-off-by: Srikar Dronamraju <srikar@linux.vnet.ibm.com>
> > ---
> > arch/x86/kernel/traps.c | 7 +------
> > 1 files changed, 1 insertions(+), 6 deletions(-)
> >
> > diff --git a/arch/x86/kernel/traps.c b/arch/x86/kernel/traps.c
> > index 6913369..c9a4be5 100644
> > --- a/arch/x86/kernel/traps.c
> > +++ b/arch/x86/kernel/traps.c
> > @@ -461,15 +461,10 @@ dotraplinkage void __kprobes do_int3(struct pt_regs *regs, long error_code)
> > == NOTIFY_STOP)
> > return;
> > #endif /* CONFIG_KGDB_LOW_LEVEL_TRAP */
> > -#ifdef CONFIG_KPROBES
> > +
> > if (notify_die(DIE_INT3, "int3", regs, error_code, 3, SIGTRAP)
> > == NOTIFY_STOP)
> > return;
> > -#else
> > - if (notify_die(DIE_TRAP, "int3", regs, error_code, 3, SIGTRAP)
> > - == NOTIFY_STOP)
> > - return;
> > -#endif
> >
> > preempt_conditional_sti(regs);
> > do_trap(3, SIGTRAP, "int3", regs, error_code, NULL);
> >
> > --
> > 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/
>
>
> --
> Masami HIRAMATSU
> Software Platform Research Dept. Linux Technology Center
> Hitachi, Ltd., Yokohama Research Laboratory
> E-mail: masami.hiramatsu.pt@hitachi.com
>
^ permalink raw reply [flat|nested] 13+ messages in thread
* [tip:x86/asm] x86: Call do_notify_resume() with interrupts enabled
2011-10-25 14:18 [RESEND] [RFC][PATCH X86_32 1/2]: Call do_notify_resume() with interrupts enabled Srikar Dronamraju
2011-10-25 14:21 ` [RFC] [PATCH x86 2/2] Cleanup do_int3 Srikar Dronamraju
2011-10-25 16:14 ` [RESEND] [RFC][PATCH X86_32 1/2]: Call do_notify_resume() with interrupts enabled Oleg Nesterov
@ 2011-12-06 9:42 ` tip-bot for Srikar Dronamraju
2 siblings, 0 replies; 13+ messages in thread
From: tip-bot for Srikar Dronamraju @ 2011-12-06 9:42 UTC (permalink / raw)
To: linux-tip-commits
Cc: linux-kernel, hpa, mingo, torvalds, a.p.zijlstra,
masami.hiramatsu.pt, srikar, tglx, mingo
Commit-ID: 3596ff4e6b2aff8a28c69af389d5046090a53330
Gitweb: http://git.kernel.org/tip/3596ff4e6b2aff8a28c69af389d5046090a53330
Author: Srikar Dronamraju <srikar@linux.vnet.ibm.com>
AuthorDate: Tue, 25 Oct 2011 19:48:12 +0530
Committer: Ingo Molnar <mingo@elte.hu>
CommitDate: Tue, 6 Dec 2011 08:20:34 +0100
x86: Call do_notify_resume() with interrupts enabled
do_notify_resume() gets called with interrupts disabled on x86_32. This
is different from the x86_64 behavior, where interrupts are enabled at
the time.
Queries on lkml on this issue hasn't yielded any clear answer. Lets make
x86_32 behave the same as x86_64, unless there is a real reason to
maintain status quo.
Please refer https://lkml.org/lkml/2011/9/27/130 for more
details.
A similar change was suggested in ARM:
https://lkml.org/lkml/2011/8/25/231
My 32-bit machine works fine (tm) with this patch.
Signed-off-by: Srikar Dronamraju <srikar@linux.vnet.ibm.com>
Acked-by: Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com>
Signed-off-by: Peter Zijlstra <a.p.zijlstra@chello.nl>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Link: http://lkml.kernel.org/r/20111025141812.GA21225@linux.vnet.ibm.com
Signed-off-by: Ingo Molnar <mingo@elte.hu>
---
arch/x86/kernel/entry_32.S | 4 ++++
1 files changed, 4 insertions(+), 0 deletions(-)
diff --git a/arch/x86/kernel/entry_32.S b/arch/x86/kernel/entry_32.S
index f3f6f53..22d0e21 100644
--- a/arch/x86/kernel/entry_32.S
+++ b/arch/x86/kernel/entry_32.S
@@ -625,6 +625,8 @@ work_notifysig: # deal with pending signals and
movl %esp, %eax
jne work_notifysig_v86 # returning to kernel-space or
# vm86-space
+ TRACE_IRQS_ON
+ ENABLE_INTERRUPTS(CLBR_NONE)
xorl %edx, %edx
call do_notify_resume
jmp resume_userspace_sig
@@ -638,6 +640,8 @@ work_notifysig_v86:
#else
movl %esp, %eax
#endif
+ TRACE_IRQS_ON
+ ENABLE_INTERRUPTS(CLBR_NONE)
xorl %edx, %edx
call do_notify_resume
jmp resume_userspace_sig
^ permalink raw reply related [flat|nested] 13+ messages in thread
* [tip:x86/asm] x86: Clean up and extend do_int3()
2011-10-25 14:21 ` [RFC] [PATCH x86 2/2] Cleanup do_int3 Srikar Dronamraju
2011-10-25 15:52 ` Oleg Nesterov
2011-10-28 1:49 ` Masami Hiramatsu
@ 2011-12-06 9:42 ` tip-bot for Srikar Dronamraju
2 siblings, 0 replies; 13+ messages in thread
From: tip-bot for Srikar Dronamraju @ 2011-12-06 9:42 UTC (permalink / raw)
To: linux-tip-commits
Cc: linux-kernel, hpa, mingo, torvalds, a.p.zijlstra, ananth, srikar,
tglx, mingo
Commit-ID: cc3a1bf52a9d2808c7cd6e8f413b02b650b6b84b
Gitweb: http://git.kernel.org/tip/cc3a1bf52a9d2808c7cd6e8f413b02b650b6b84b
Author: Srikar Dronamraju <srikar@linux.vnet.ibm.com>
AuthorDate: Tue, 25 Oct 2011 19:51:59 +0530
Committer: Ingo Molnar <mingo@elte.hu>
CommitDate: Tue, 6 Dec 2011 08:20:37 +0100
x86: Clean up and extend do_int3()
Since there is a possibility of !KPROBES int3 listeners
(such as kgdb) and since DIE_TRAP is currently not being
used by anybody, notify all listeners with DIE_INT3.
Signed-off-by: Ananth N Mavinakayanahalli <ananth@in.ibm.com>
Signed-off-by: Srikar Dronamraju <srikar@linux.vnet.ibm.com>
Signed-off-by: Peter Zijlstra <a.p.zijlstra@chello.nl>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Link: http://lkml.kernel.org/r/20111025142159.GB21225@linux.vnet.ibm.com
Signed-off-by: Ingo Molnar <mingo@elte.hu>
---
arch/x86/kernel/traps.c | 7 +------
1 files changed, 1 insertions(+), 6 deletions(-)
diff --git a/arch/x86/kernel/traps.c b/arch/x86/kernel/traps.c
index a8e3eb8..fa1191f 100644
--- a/arch/x86/kernel/traps.c
+++ b/arch/x86/kernel/traps.c
@@ -306,15 +306,10 @@ dotraplinkage void __kprobes do_int3(struct pt_regs *regs, long error_code)
== NOTIFY_STOP)
return;
#endif /* CONFIG_KGDB_LOW_LEVEL_TRAP */
-#ifdef CONFIG_KPROBES
+
if (notify_die(DIE_INT3, "int3", regs, error_code, 3, SIGTRAP)
== NOTIFY_STOP)
return;
-#else
- if (notify_die(DIE_TRAP, "int3", regs, error_code, 3, SIGTRAP)
- == NOTIFY_STOP)
- return;
-#endif
preempt_conditional_sti(regs);
do_trap(3, SIGTRAP, "int3", regs, error_code, NULL);
^ permalink raw reply related [flat|nested] 13+ messages in thread
end of thread, other threads:[~2011-12-06 9:43 UTC | newest]
Thread overview: 13+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2011-10-25 14:18 [RESEND] [RFC][PATCH X86_32 1/2]: Call do_notify_resume() with interrupts enabled Srikar Dronamraju
2011-10-25 14:21 ` [RFC] [PATCH x86 2/2] Cleanup do_int3 Srikar Dronamraju
2011-10-25 15:52 ` Oleg Nesterov
2011-10-28 1:49 ` Masami Hiramatsu
2011-11-18 9:41 ` Srikar Dronamraju
2011-12-06 9:42 ` [tip:x86/asm] x86: Clean up and extend do_int3() tip-bot for Srikar Dronamraju
2011-10-25 16:14 ` [RESEND] [RFC][PATCH X86_32 1/2]: Call do_notify_resume() with interrupts enabled Oleg Nesterov
2011-10-26 12:38 ` Linus Torvalds
2011-10-26 15:10 ` Russell King
2011-10-26 17:34 ` Roland McGrath
2011-10-26 18:58 ` Oleg Nesterov
2011-11-03 4:43 ` Srikar Dronamraju
2011-12-06 9:42 ` [tip:x86/asm] x86: " tip-bot for Srikar Dronamraju
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).