Re: [PATCH] usbhid: Fix lockdep unannotated irqs-off warning

[Sedat Dilek <sedat.dilek@gmail.com>]

[-- Attachment #1: Type: text/plain, Size: 1585 bytes --]

> What shall I do... play with lockdep (print_irqtrace_events) in
> del_timer_sync()?

I recalled that when Jiri was thinking towards a "compiler bug" that
the part in del_timer_sync() emebedded in the "ifdef CONFIG_LOCKDEP"
is somehow "mis-compiled".
Furthermore, I see that try_to_del_timer_sync() is invoked within
del_timer_sync() which does a spin_unlock_irqrestore().

[ kernel/time/timer.c ]

int del_timer_sync(struct timer_list *timer)
{
#ifdef CONFIG_LOCKDEP
unsigned long flags;

/*
* If lockdep gives a backtrace here, please reference
* the synchronization rules above.
*/
local_irq_save(flags);
lock_map_acquire(&timer->lockdep_map);
lock_map_release(&timer->lockdep_map);
local_irq_restore(flags);
#endif
/*
* don't use it in hardirq context, because it
* could lead to deadlock.
*/
WARN_ON(in_irq() && !(timer->flags & TIMER_IRQSAFE));
for (;;) {
int ret = try_to_del_timer_sync(timer);
if (ret >= 0)
return ret;
cpu_relax();
}
}
EXPORT_SYMBOL(del_timer_sync);
#endif
...
int try_to_del_timer_sync(struct timer_list *timer)
{
struct tvec_base *base;
unsigned long flags;
int ret = -1;

debug_assert_init(timer);

base = lock_timer_base(timer, &flags);

if (base->running_timer != timer) {
timer_stats_timer_clear_start_info(timer);
ret = detach_if_pending(timer, base, true);
}
spin_unlock_irqrestore(&base->lock, flags);

return ret;
}
EXPORT_SYMBOL(try_to_del_timer_sync);
...

Is the attached patch suitable for a test?

I remember I tried to turn lockdep kernel-config for amd64 but that
was somehow tricky.

Shall I ask timer folks?

Thoughts?

Thanks!

- Sedat -

[-- Attachment #2: 0001-timer-lockdep-Add-WARN_ON-irqs_disabled-to-del_timer.patch --]
[-- Type: text/x-diff, Size: 1241 bytes --]

From edb4cb72c631c5e588af40794830c5bb5d6a927a Mon Sep 17 00:00:00 2001
From: Sedat Dilek <sedat.dilek@gmail.com>
Date: Wed, 30 Sep 2015 08:20:15 +0200
Subject: [PATCH] timer: lockdep: Add WARN_ON(irqs_disabled()) to
 del_timer_sync()

---
 kernel/locking/lockdep.c | 1 +
 kernel/time/timer.c      | 3 +++
 2 files changed, 4 insertions(+)

diff --git a/kernel/locking/lockdep.c b/kernel/locking/lockdep.c
index 8acfbf773e06..8b29b3dd518f 100644
--- a/kernel/locking/lockdep.c
+++ b/kernel/locking/lockdep.c
@@ -2410,6 +2410,7 @@ void print_irqtrace_events(struct task_struct *curr)
 	printk("softirqs last disabled at (%u): ", curr->softirq_disable_event);
 	print_ip_sym(curr->softirq_disable_ip);
 }
+EXPORT_SYMBOL(print_irqtrace_events);
 
 static int HARDIRQ_verbose(struct lock_class *class)
 {
diff --git a/kernel/time/timer.c b/kernel/time/timer.c
index 84190f02b521..f434b2dce642 100644
--- a/kernel/time/timer.c
+++ b/kernel/time/timer.c
@@ -1082,6 +1082,9 @@ int del_timer_sync(struct timer_list *timer)
 #ifdef CONFIG_LOCKDEP
 	unsigned long flags;
 
+	if(WARN_ON(irqs_disabled()))
+		print_irqtrace_events(current);
+
 	/*
 	 * If lockdep gives a backtrace here, please reference
 	 * the synchronization rules above.
-- 
2.5.3