Re: [PATCH] hid: usbhid: hid-core: fix recursive deadlock

On Thu, 19 Nov 2015 10:10:19 +0100 (CET)
Jiri Kosina [off-list ref] wrote:

On Thu, 19 Nov 2015, Ioan-Adrian Ratiu wrote:

quoted

First part of lockdep report:
http://imgur.com/clLsCWe

Second part:
http://imgur.com/Wa2PzRl

Here are some printk's of mine while reproducing + debugging the issue:
http://imgur.com/SETOHT7  

So the real problem is that Intuos driver is calling hid_hw_request() 
(which tries to grab the lock in usbhid_submit_report()) while handling 
the CTRL IRQ (lock gets acquired there).

So the proper way to fix seems to be delaying the scheduling of the 
proximity read event in wacom_intuos_inout() to workqueue.

quoted

I'll continue to research this more in depth, but progress is slow 
because I don't have much time, I'm doing this in my spare time because 
it's my girlfriend's tablet.  

Oh, now I understand the level of severity of this bug! :-)

Thanks,

Yes, exactly, you are beginning to understand! :)  When I've put my 2 variants
above to solve this deadlock, by "removing the call from wacom" at 1) I was
trying to say exactly this, removing it from the irq to a workqueue.

But please understand further my reasoning for submitting this patch. Consider
if this is a bug in the wacom driver or in the usbhid core? IMO
this is a usbhid bug: the critical region in hid_ctrl() is too big, there
is no reason for the call to hid_input_report() to be protected by
usbhid->lock.

The correct way to fix this deadlock is to fix the critical section in
usbhid, not remove the call from the wacom irq. If wacom wants to
reschedule in the irq, it should not deadlock on usbhid. "Fixing" the wacom call
would just work around the critical region bug inside usbhid.

I hope I've made myself clear this time; I really needed to explain this
patch better :( sorry.