Re: [PATCH] sched/core: add taint on "BUG: sleeping function called from invalid context"
On Fri, 29 Jul 2016, Rusty Russell wrote: > Vegard Nossumwrites: > > Seeing this, it occurs to me that we should probably add a taint here: > > Taint has traditionally meant "the user did something unsupported, take > the bug report with a grain of salt". Such as force removing a module. We have changed the meaning of taint quite some time ago. It also denotes e.g. that there was a bug, warning etc. So it's not necessarily 'user did something wrong'. It's very helpful to see that there was already a warnon/bug before the backtrace you get from the reporter. Thanks, tglx
Re: [PATCH] sched/core: add taint on "BUG: sleeping function called from invalid context"
On Fri, 29 Jul 2016, Rusty Russell wrote: > Vegard Nossum writes: > > Seeing this, it occurs to me that we should probably add a taint here: > > Taint has traditionally meant "the user did something unsupported, take > the bug report with a grain of salt". Such as force removing a module. We have changed the meaning of taint quite some time ago. It also denotes e.g. that there was a bug, warning etc. So it's not necessarily 'user did something wrong'. It's very helpful to see that there was already a warnon/bug before the backtrace you get from the reporter. Thanks, tglx
Re: [PATCH] sched/core: add taint on "BUG: sleeping function called from invalid context"
On 07/29/2016 03:22 AM, Rusty Russell wrote: Vegard Nossumwrites: Seeing this, it occurs to me that we should probably add a taint here: Taint has traditionally meant "the user did something unsupported, take the bug report with a grain of salt". Such as force removing a module. So this seems wrong... There are many types of taints. This is the one you mentioned: 2 - A module was force loaded by insmod -f. This is the one I'd like to set: 512 - A kernel warning has occurred. (from Documentation/sysctl/kernel.txt). The flag I want to set is also set on any kind of WARN(). BUG: sleeping function called from invalid context at mm/slab.h:388 in_atomic(): 0, irqs_disabled(): 0, pid: 32211, name: trinity-c3 Preemption disabled at:[] console_unlock+0x2f7/0x930 I am not proposing this patch on a whim: it was actually hiding real bugs because I only saw an oops that happened later when in fact the real bug happened where the "sleeping function called from invalid context" was called. Vegard
Re: [PATCH] sched/core: add taint on "BUG: sleeping function called from invalid context"
On 07/29/2016 03:22 AM, Rusty Russell wrote: Vegard Nossum writes: Seeing this, it occurs to me that we should probably add a taint here: Taint has traditionally meant "the user did something unsupported, take the bug report with a grain of salt". Such as force removing a module. So this seems wrong... There are many types of taints. This is the one you mentioned: 2 - A module was force loaded by insmod -f. This is the one I'd like to set: 512 - A kernel warning has occurred. (from Documentation/sysctl/kernel.txt). The flag I want to set is also set on any kind of WARN(). BUG: sleeping function called from invalid context at mm/slab.h:388 in_atomic(): 0, irqs_disabled(): 0, pid: 32211, name: trinity-c3 Preemption disabled at:[] console_unlock+0x2f7/0x930 I am not proposing this patch on a whim: it was actually hiding real bugs because I only saw an oops that happened later when in fact the real bug happened where the "sleeping function called from invalid context" was called. Vegard
Re: [PATCH] sched/core: add taint on "BUG: sleeping function called from invalid context"
Vegard Nossumwrites: > Seeing this, it occurs to me that we should probably add a taint here: Taint has traditionally meant "the user did something unsupported, take the bug report with a grain of salt". Such as force removing a module. So this seems wrong... Cheers, Rusty. > > BUG: sleeping function called from invalid context at mm/slab.h:388 > in_atomic(): 0, irqs_disabled(): 0, pid: 32211, name: trinity-c3 > Preemption disabled at:[] console_unlock+0x2f7/0x930 > > CPU: 3 PID: 32211 Comm: trinity-c3 Not tainted 4.7.0-rc7+ #19 >^^^ > Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS > Ubuntu-1.8.2-1ubuntu1 04/01/2014 > 8800b8a17160 81971441 88011a3c4c80 > 88011a3c4c80 8800b8a17198 81158067 0de6 > 88011a3c4c80 8390e07c 0184 > Call Trace: > [...] > > BUG: sleeping function called from invalid context at > arch/x86/mm/fault.c:1309 > in_atomic(): 0, irqs_disabled(): 0, pid: 32211, name: trinity-c3 > Preemption disabled at:[] down_trylock+0x13/0x80 > > CPU: 3 PID: 32211 Comm: trinity-c3 Not tainted 4.7.0-rc7+ #19 >^^^ > Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS > Ubuntu-1.8.2-1ubuntu1 04/01/2014 > 8800b8a17e08 81971441 88011a3c4c80 > 88011a3c4c80 8800b8a17e40 81158067 > 88011a3c4c80 83437b20 051d > Call Trace: > [...] > > Cc: Peter Zijlstra > Cc: Paul E. McKenney > Cc: Thomas Gleixner > Cc: Ingo Molnar > Cc: Rusty Russel > Signed-off-by: Vegard Nossum > --- > kernel/sched/core.c | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/kernel/sched/core.c b/kernel/sched/core.c > index 97ee9ac..7171cf9 100644 > --- a/kernel/sched/core.c > +++ b/kernel/sched/core.c > @@ -7573,6 +7573,7 @@ void ___might_sleep(const char *file, int line, int > preempt_offset) > } > #endif > dump_stack(); > + add_taint(TAINT_WARN, LOCKDEP_STILL_OK); > } > EXPORT_SYMBOL(___might_sleep); > #endif > -- > 1.9.1
Re: [PATCH] sched/core: add taint on "BUG: sleeping function called from invalid context"
Vegard Nossum writes: > Seeing this, it occurs to me that we should probably add a taint here: Taint has traditionally meant "the user did something unsupported, take the bug report with a grain of salt". Such as force removing a module. So this seems wrong... Cheers, Rusty. > > BUG: sleeping function called from invalid context at mm/slab.h:388 > in_atomic(): 0, irqs_disabled(): 0, pid: 32211, name: trinity-c3 > Preemption disabled at:[] console_unlock+0x2f7/0x930 > > CPU: 3 PID: 32211 Comm: trinity-c3 Not tainted 4.7.0-rc7+ #19 >^^^ > Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS > Ubuntu-1.8.2-1ubuntu1 04/01/2014 > 8800b8a17160 81971441 88011a3c4c80 > 88011a3c4c80 8800b8a17198 81158067 0de6 > 88011a3c4c80 8390e07c 0184 > Call Trace: > [...] > > BUG: sleeping function called from invalid context at > arch/x86/mm/fault.c:1309 > in_atomic(): 0, irqs_disabled(): 0, pid: 32211, name: trinity-c3 > Preemption disabled at:[] down_trylock+0x13/0x80 > > CPU: 3 PID: 32211 Comm: trinity-c3 Not tainted 4.7.0-rc7+ #19 >^^^ > Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS > Ubuntu-1.8.2-1ubuntu1 04/01/2014 > 8800b8a17e08 81971441 88011a3c4c80 > 88011a3c4c80 8800b8a17e40 81158067 > 88011a3c4c80 83437b20 051d > Call Trace: > [...] > > Cc: Peter Zijlstra > Cc: Paul E. McKenney > Cc: Thomas Gleixner > Cc: Ingo Molnar > Cc: Rusty Russel > Signed-off-by: Vegard Nossum > --- > kernel/sched/core.c | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/kernel/sched/core.c b/kernel/sched/core.c > index 97ee9ac..7171cf9 100644 > --- a/kernel/sched/core.c > +++ b/kernel/sched/core.c > @@ -7573,6 +7573,7 @@ void ___might_sleep(const char *file, int line, int > preempt_offset) > } > #endif > dump_stack(); > + add_taint(TAINT_WARN, LOCKDEP_STILL_OK); > } > EXPORT_SYMBOL(___might_sleep); > #endif > -- > 1.9.1