[ the original mail was sent by Zdenek to me offlist ]
On Wed, 13 Feb 2008, Zdenek Kabelac wrote:
> [ 68.379022] gnome-settings-[2941] trap int3 ip:3d2c840f34
> sp:7fff36f5d100 error:0<3>BUG: sleeping function called from invalid
> context at kernel/rwsem.c:21
> [ 68.379039] in_atomic():1, irqs_disabled():0
> [ 68.379044] no locks held by gnome-settings-/2941.
> [ 68.379050] Pid: 2941, comm: gnome-settings- Not tainted 2.6.25-rc1 #30
> [ 68.379054]
> [ 68.379056] Call Trace:
> [ 68.379061] <#DB> [<ffffffff81064883>] ? __debug_show_held_locks+0x13/0x30
> [ 68.379109] [<ffffffff81036765>] __might_sleep+0xe5/0x110
> [ 68.379123] [<ffffffff812f2240>] down_read+0x20/0x70
> [ 68.379137] [<ffffffff8109cdca>] print_vma_addr+0x3a/0x110
> [ 68.379152] [<ffffffff8100f435>] do_trap+0xf5/0x170
> [ 68.379168] [<ffffffff8100f52b>] do_int3+0x7b/0xe0
> [ 68.379180] [<ffffffff812f4a6f>] int3+0x9f/0xd0
> [ 68.379203] <<EOE>>
> [ 68.379229] in libglib-2.0.so.0.1505.0[3d2c800000+dc000]
Andi,
in commit 03252919 you introduced print_vma_addr(), but this funcion for
obvious reasons takes mmap_sem, so it is not safe to call it from atomic
context (i.e. do_trap(), for example), which is behavior your patch
introduced.
--
Jiri Kosina
SUSE Labs
[ sorry for forgetting to change subject properly :) ]
On Wed, 13 Feb 2008, Jiri Kosina wrote:
> [ the original mail was sent by Zdenek to me offlist ]
>
> On Wed, 13 Feb 2008, Zdenek Kabelac wrote:
>
> > [ 68.379022] gnome-settings-[2941] trap int3 ip:3d2c840f34
> > sp:7fff36f5d100 error:0<3>BUG: sleeping function called from invalid
> > context at kernel/rwsem.c:21
> > [ 68.379039] in_atomic():1, irqs_disabled():0
> > [ 68.379044] no locks held by gnome-settings-/2941.
> > [ 68.379050] Pid: 2941, comm: gnome-settings- Not tainted 2.6.25-rc1 #30
> > [ 68.379054]
> > [ 68.379056] Call Trace:
> > [ 68.379061] <#DB> [<ffffffff81064883>] ? __debug_show_held_locks+0x13/0x30
> > [ 68.379109] [<ffffffff81036765>] __might_sleep+0xe5/0x110
> > [ 68.379123] [<ffffffff812f2240>] down_read+0x20/0x70
> > [ 68.379137] [<ffffffff8109cdca>] print_vma_addr+0x3a/0x110
> > [ 68.379152] [<ffffffff8100f435>] do_trap+0xf5/0x170
> > [ 68.379168] [<ffffffff8100f52b>] do_int3+0x7b/0xe0
> > [ 68.379180] [<ffffffff812f4a6f>] int3+0x9f/0xd0
> > [ 68.379203] <<EOE>>
> > [ 68.379229] in libglib-2.0.so.0.1505.0[3d2c800000+dc000]
>
> Andi,
>
> in commit 03252919 you introduced print_vma_addr(), but this funcion for
> obvious reasons takes mmap_sem, so it is not safe to call it from atomic
> context (i.e. do_trap(), for example), which is behavior your patch
> introduced.
>
>
> > in commit 03252919 you introduced print_vma_addr(), but this funcion for
> > obvious reasons takes mmap_sem, so it is not safe to call it from atomic
> > context (i.e. do_trap(), for example), which is behavior your patch
> > introduced.
Ah yes -- this behaviour of int3 do_trap was always a source of bugs. I remember
fixing such things in this area several times (last time in the RT kernel), but
I keep forgetting it. Sorry.
The correct fix is to run the int3 and debug handlers on the process stack
when the fault originated from user space. Then they can run preemptive
and it's ok to schedule for the lock too.
I'll fix that tomorrow.
-Andi
* Jiri Kosina <[email protected]> wrote:
> > > [ 68.379054]
> > > [ 68.379056] Call Trace:
> > > [ 68.379061] <#DB> [<ffffffff81064883>] ? __debug_show_held_locks+0x13/0x30
> > > [ 68.379109] [<ffffffff81036765>] __might_sleep+0xe5/0x110
> > > [ 68.379123] [<ffffffff812f2240>] down_read+0x20/0x70
> > > [ 68.379137] [<ffffffff8109cdca>] print_vma_addr+0x3a/0x110
> > > [ 68.379152] [<ffffffff8100f435>] do_trap+0xf5/0x170
> > > [ 68.379168] [<ffffffff8100f52b>] do_int3+0x7b/0xe0
> > > [ 68.379180] [<ffffffff812f4a6f>] int3+0x9f/0xd0
> > > [ 68.379203] <<EOE>>
> > > [ 68.379229] in libglib-2.0.so.0.1505.0[3d2c800000+dc000]
ouch. Could you try the fix below? This makes print_vma_addr() more
robust and should fix the immediate bug.
The longer-term fix will be to not run int3 exception handlers in a
non-preemptible context (like 32-bit does) - but that will need more
testing.
Ingo
---------------->
Subject: x86: fix "BUG: sleeping function called from invalid context" in print_vma_addr()
From: Ingo Molnar <[email protected]>
Date: Wed Feb 13 20:21:06 CET 2008
Jiri Kosina reported the following deadlock scenario with
show_unhandled_signals enabled:
[ 68.379022] gnome-settings-[2941] trap int3 ip:3d2c840f34
sp:7fff36f5d100 error:0<3>BUG: sleeping function called from invalid
context at kernel/rwsem.c:21
[ 68.379039] in_atomic():1, irqs_disabled():0
[ 68.379044] no locks held by gnome-settings-/2941.
[ 68.379050] Pid: 2941, comm: gnome-settings- Not tainted 2.6.25-rc1 #30
[ 68.379054]
[ 68.379056] Call Trace:
[ 68.379061] <#DB> [<ffffffff81064883>] ? __debug_show_held_locks+0x13/0x30
[ 68.379109] [<ffffffff81036765>] __might_sleep+0xe5/0x110
[ 68.379123] [<ffffffff812f2240>] down_read+0x20/0x70
[ 68.379137] [<ffffffff8109cdca>] print_vma_addr+0x3a/0x110
[ 68.379152] [<ffffffff8100f435>] do_trap+0xf5/0x170
[ 68.379168] [<ffffffff8100f52b>] do_int3+0x7b/0xe0
[ 68.379180] [<ffffffff812f4a6f>] int3+0x9f/0xd0
[ 68.379203] <<EOE>>
[ 68.379229] in libglib-2.0.so.0.1505.0[3d2c800000+dc000]
and tracked it down to:
commit 03252919b79891063cf99145612360efbdf9500b
Author: Andi Kleen <[email protected]>
Date: Wed Jan 30 13:33:18 2008 +0100
x86: print which shared library/executable faulted in segfault etc. messages
the problem is that we call down_read() from an atomic context.
Solve this by returning from print_vma_addr() if the preempt count is
elevated. Update preempt_conditional_sti / preempt_conditional_cli to
unconditionally lift the preempt count even on !CONFIG_PREEMPT.
Reported-by: Jiri Kosina <[email protected]>
Signed-off-by: Ingo Molnar <[email protected]>
---
arch/x86/kernel/traps_64.c | 4 ++--
mm/memory.c | 7 +++++++
2 files changed, 9 insertions(+), 2 deletions(-)
Index: linux-x86.q/arch/x86/kernel/traps_64.c
===================================================================
--- linux-x86.q.orig/arch/x86/kernel/traps_64.c
+++ linux-x86.q/arch/x86/kernel/traps_64.c
@@ -84,7 +84,7 @@ static inline void conditional_sti(struc
static inline void preempt_conditional_sti(struct pt_regs *regs)
{
- preempt_disable();
+ inc_preempt_count();
if (regs->flags & X86_EFLAGS_IF)
local_irq_enable();
}
@@ -95,7 +95,7 @@ static inline void preempt_conditional_c
local_irq_disable();
/* Make sure to not schedule here because we could be running
on an exception stack. */
- preempt_enable_no_resched();
+ dec_preempt_count();
}
int kstack_depth_to_print = 12;
Index: linux-x86.q/mm/memory.c
===================================================================
--- linux-x86.q.orig/mm/memory.c
+++ linux-x86.q/mm/memory.c
@@ -2711,6 +2711,13 @@ void print_vma_addr(char *prefix, unsign
struct mm_struct *mm = current->mm;
struct vm_area_struct *vma;
+ /*
+ * Do not print if we are in atomic
+ * contexts (in exception stacks, etc.):
+ */
+ if (preempt_count())
+ return;
+
down_read(&mm->mmap_sem);
vma = find_vma(mm, ip);
if (vma && vma->vm_file) {
On Wed, 13 Feb 2008, Ingo Molnar wrote:
> The longer-term fix will be to not run int3 exception handlers in a
> non-preemptible context (like 32-bit does) - but that will need more
> testing.
Yup, exactly.
> Jiri Kosina reported the following deadlock scenario with
> show_unhandled_signals enabled:
Actually the kernel error message was reported to me by Zdenek. Zdenek,
could you please try the fix?
Thanks,
--
Jiri Kosina
SUSE Labs
2008/2/13, Jiri Kosina <[email protected]>:
> On Wed, 13 Feb 2008, Ingo Molnar wrote:
>
> > The longer-term fix will be to not run int3 exception handlers in a
> > non-preemptible context (like 32-bit does) - but that will need more
> > testing.
>
> Yup, exactly.
>
> > Jiri Kosina reported the following deadlock scenario with
> > show_unhandled_signals enabled:
>
> Actually the kernel error message was reported to me by Zdenek. Zdenek,
> could you please try the fix?
Ok - great I do not get this BUG log
So this has fixed 2 problems a 3 other remains :)
I'll open them tomorrow I guess :) at my two were fixed pretty quickly.
PS: Ingo did you have a chance to check my kernel IPI problem ?
Zdenek
On Wednesday 13 February 2008 20:35:38 Ingo Molnar wrote:
> + /*
> + * Do not print if we are in atomic
> + * contexts (in exception stacks, etc.):
> + */
> + if (preempt_count())
> + return;
It should print the address even in this case; just not resolve the VMA
name.
-Andi