On 27 July 2016 at 11:15, Ingo Molnar <[email protected]> wrote:
>
> * Vegard Nossum <[email protected]> wrote:
[...]
> These two blocks could be merged trivially, avoiding an #ifdef pair ...
>> @@ -7541,6 +7550,9 @@ EXPORT_SYMBOL(__might_sleep);
>> void ___might_sleep(const char *file, int line, int preempt_offset)
>> {
>> static unsigned long prev_jiffy; /* ratelimiting */
>> +#ifdef CONFIG_DEBUG_PREEMPT
>> + unsigned long preempt_disable_ip;
>> +#endif
>>
>> rcu_sleep_check(); /* WARN_ON_ONCE() by default, no rate limit reqd. */
>> if ((preempt_count_equals(preempt_offset) && !irqs_disabled() &&
>> @@ -7551,6 +7563,11 @@ void ___might_sleep(const char *file, int line, int
>> preempt_offset)
>> return;
>> prev_jiffy = jiffies;
>>
>> +#ifdef CONFIG_DEBUG_PREEMPT
>> + /* Save this before calling printk(), since that will clobber it */
>> + preempt_disable_ip = current->preempt_disable_ip;
>> +#endif
>
> Ditto.
I'm assuming you want to declare and initialise preempt_disable_ip at
once here, but it generates slightly worse code since it dereferences
current->preempt_disable_ip in the "fast path" (i.e. a sleeping
function is NOT called from an invalid context).
If you really prefer that I've attached an updated patch.
Let me know if I misunderstood :-)
Vegard
From 332d49d4da90f00304de73c7412311576e2e7f78 Mon Sep 17 00:00:00 2001
From: Vegard Nossum <[email protected]>
Date: Sat, 23 Jul 2016 09:46:39 +0200
Subject: [PATCH] sched/core: make "Preemption disabled at" message more useful
This message is currently really useless since it always prints a value
that comes from the printk() we just did, e.g.:
BUG: sleeping function called from invalid context at mm/slab.h:388
in_atomic(): 0, irqs_disabled(): 0, pid: 31996, name: trinity-c1
Preemption disabled at:[<ffffffff8119db33>] down_trylock+0x13/0x80
BUG: sleeping function called from invalid context at include/linux/freezer.h:56
in_atomic(): 0, irqs_disabled(): 0, pid: 31996, name: trinity-c1
Preemption disabled at:[<ffffffff811aaa37>] console_unlock+0x2f7/0x930
Here, both down_trylock() and console_unlock() is somewhere in the
printk() path.
We should save the value before calling printk() and use the saved value
instead. That immediately reveals the offending callsite:
BUG: sleeping function called from invalid context at mm/slab.h:388
in_atomic(): 0, irqs_disabled(): 0, pid: 14971, name: trinity-c2
Preemption disabled at:[<ffffffff819bcd46>] rhashtable_walk_start+0x46/0x150
(Bug report: http://marc.info/?l=linux-netdev&m=146925979821849&w=2)
Cc: Peter Zijlstra <[email protected]>
Cc: Paul E. McKenney <[email protected]>
Cc: Thomas Gleixner <[email protected]>
Cc: Ingo Molnar <[email protected]>
Cc: Rusty Russel <[email protected]>
Signed-off-by: Vegard Nossum <[email protected]>
---
kernel/sched/core.c | 13 +++++++++++--
1 file changed, 11 insertions(+), 2 deletions(-)
diff --git a/kernel/sched/core.c b/kernel/sched/core.c
index 7171cf9..e06354e 100644
--- a/kernel/sched/core.c
+++ b/kernel/sched/core.c
@@ -3144,6 +3144,11 @@ static inline void preempt_latency_stop(int val) { }
*/
static noinline void __schedule_bug(struct task_struct *prev)
{
+#ifdef CONFIG_DEBUG_PREEMPT
+ /* Save this before calling printk(), since that will clobber it */
+ unsigned long preempt_disable_ip = current->preempt_disable_ip;
+#endif
+
if (oops_in_progress)
return;
@@ -3157,7 +3162,7 @@ static noinline void __schedule_bug(struct task_struct *prev)
#ifdef CONFIG_DEBUG_PREEMPT
if (in_atomic_preempt_off()) {
pr_err("Preemption disabled at:");
- print_ip_sym(current->preempt_disable_ip);
+ print_ip_sym(preempt_disable_ip);
pr_cont("\n");
}
#endif
@@ -7541,6 +7546,10 @@ EXPORT_SYMBOL(__might_sleep);
void ___might_sleep(const char *file, int line, int preempt_offset)
{
static unsigned long prev_jiffy; /* ratelimiting */
+#ifdef CONFIG_DEBUG_PREEMPT
+ /* Save this before calling printk(), since that will clobber it */
+ unsigned long preempt_disable_ip = current->preempt_disable_ip;
+#endif
rcu_sleep_check(); /* WARN_ON_ONCE() by default, no rate limit reqd. */
if ((preempt_count_equals(preempt_offset) && !irqs_disabled() &&
@@ -7568,7 +7577,7 @@ void ___might_sleep(const char *file, int line, int preempt_offset)
#ifdef CONFIG_DEBUG_PREEMPT
if (!preempt_count_equals(preempt_offset)) {
pr_err("Preemption disabled at:");
- print_ip_sym(current->preempt_disable_ip);
+ print_ip_sym(preempt_disable_ip);
pr_cont("\n");
}
#endif
--
1.9.1
