Re: [PATCH] usbhid: Fix lockdep unannotated irqs-off warning
From: Jiri Kosina <jikos@kernel.org>
Date: 2015-09-28 11:33:22
On Mon, 28 Sep 2015, Sedat Dilek wrote:
When compiling Linux v4.2+ and v4.3-rc2+ with a llvmlinux patchset and CLANG v3.7 I see a BUG line like this: [ 24.705463] BUG: sleeping function called from invalid context at kernel/workqueue.c:2680 [ 24.705576] in_atomic(): 0, irqs_disabled(): 1, pid: 1447, name: acpid After some vital help from workqueue and hid folks it turned out to be a problem in the hid area. Jiri encouraged me to look into del_timer-sync()/cancel_work_sync(). So, I disassembled kernel/time/timer.o. This looked good. Both functions are called in hid_cancel_delayed_stuff().
Yeah, but we're enabling IRQs before calling hid_cancel_delayed_stuff() (or, to be more precise, we're restoring original flags, and I don't see usbhid_close() being called with IRQs off). Therefore ... [ ... snip ... ]
quoted hunk ↗ jump to hunk
diff --git a/drivers/hid/usbhid/hid-core.c b/drivers/hid/usbhid/hid-core.c index 36712e9f56c2..188f59348ec5 100644 --- a/drivers/hid/usbhid/hid-core.c +++ b/drivers/hid/usbhid/hid-core.c@@ -729,16 +729,16 @@ void usbhid_close(struct hid_device *hid) * data acquistion due to a resumption we no longer * care about */ - spin_lock_irq(&usbhid->lock); + spin_lock_bh(&usbhid->lock); if (!--hid->open) { - spin_unlock_irq(&usbhid->lock); + spin_unlock_bh(&usbhid->lock); hid_cancel_delayed_stuff(usbhid);
I still don't understand how this should be improving anything. I believe spin_unlock_irq() should just re-enable interrupts, because we've been called with them enabled as well. Now if you are able to see how usbhid_close() can be called with IRQs off, that would be a completely different story. But if that's not the case, the warning is bogus, and gcc-compiled kernels are right about not issuing it. But without that, I so far fail to see how this is a correct thing to do. Thanks, -- Jiri Kosina SUSE Labs