Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]


Groups > linux.kernel > #1272695

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

From Josh Cartwright <joshc@ni.com>
Newsgroups linux.kernel
Subject Re: [PATCH] hid: usbhid: hid-core: fix recursive deadlock
Date 2015-11-19 01:00 +0100
Message-ID <qwkNQ-1An-19@gated-at.bofh.it> (permalink)
References <qwgAz-7es-25@gated-at.bofh.it> <qwhGh-7Ze-3@gated-at.bofh.it> <qwi9j-8vZ-7@gated-at.bofh.it>
Organization linux.* mail to news gateway

Show all headers | View raw


[Multipart message — attachments visible in raw view] - view raw

On Wed, Nov 18, 2015 at 11:05:44PM +0200, Ioan-Adrian Ratiu wrote:
> On Wed, 18 Nov 2015 21:37:42 +0100 (CET)
> Jiri Kosina <jikos@kernel.org> wrote:
> 
> > On Wed, 18 Nov 2015, Ioan-Adrian Ratiu wrote:
> > 
> > > The critical section protected by usbhid->lock in hid_ctrl() is too
> > > big and in rare cases causes a recursive deadlock because of its call
> > > to hid_input_report().
> > > 
> > > This deadlock reproduces on newer wacom tablets like 056a:033c because
> > > the wacom driver in its irq handler ends up calling hid_hw_request()
> > > from wacom_intuos_schedule_prox_event() in wacom_wac.c. What this means
> > > is that it submits a report to reschedule a proximity read through a
> > > sync ctrl call which grabs the lock in hid_ctrl(struct urb *urb)
> > > before calling hid_input_report(). When the irq kicks in on the same
> > > cpu, it also tries to grab the lock resulting in a recursive deadlock.
> > > 
> > > The proper fix is to shrink the critical section in hid_ctrl() to
> > > protect only the instructions which modify usbhid, thus move the lock
> > > after the hid_input_report() call and the deadlock dissapears.  
> > 
> > I think the proper fix actually is to spin_lock_irqsave() in hid_ctrl(), 
> > isn't it?
> > 
> 
> That was my first attempt, yes, but the deadlock still happens with interrupts
> disabled. It is very weird, I know.

I think your best course of action is to figure out why this is the
case, instead of continuing with trying to solve the symptoms.  Do you
have actual callstacks showing the cases where you hit?  That might be
useful to share (your lockdep picture cuts out the callstacks).

Also, have you tried without the PREEMPT_RT patch in the picture at all?

  Josh

Back to linux.kernel | Previous | NextPrevious in thread | Next in thread | Find similar | Unroll thread


Thread

[PATCH] hid: usbhid: hid-core: fix recursive deadlock Ioan-Adrian Ratiu <adi@adirat.com> - 2015-11-18 20:30 +0100
  Re: [PATCH] hid: usbhid: hid-core: fix recursive deadlock Jiri Kosina <jikos@kernel.org> - 2015-11-18 21:40 +0100
    Re: [PATCH] hid: usbhid: hid-core: fix recursive deadlock Ioan-Adrian Ratiu <adi@adirat.com> - 2015-11-18 22:10 +0100
      Re: [PATCH] hid: usbhid: hid-core: fix recursive deadlock Josh Cartwright <joshc@ni.com> - 2015-11-19 01:00 +0100
        Re: [PATCH] hid: usbhid: hid-core: fix recursive deadlock Ioan-Adrian Ratiu <adi@adirat.com> - 2015-11-19 07:50 +0100
          Re: [PATCH] hid: usbhid: hid-core: fix recursive deadlock Jiri Kosina <jikos@kernel.org> - 2015-11-19 10:20 +0100
            Re: [PATCH] hid: usbhid: hid-core: fix recursive deadlock Ioan-Adrian Ratiu <adi@adirat.com> - 2015-11-19 17:40 +0100
              Re: [PATCH] hid: usbhid: hid-core: fix recursive deadlock Jiri Kosina <jikos@kernel.org> - 2015-11-19 22:40 +0100
                Re: [PATCH] hid: usbhid: hid-core: fix recursive deadlock Ioan-Adrian Ratiu <adi@adirat.com> - 2015-11-20 21:10 +0100
      Re: [PATCH] hid: usbhid: hid-core: fix recursive deadlock Jiri Kosina <jikos@kernel.org> - 2015-11-19 10:00 +0100
  [PATCH v2] hid: usbhid: hid-core: fix recursive deadlock Ioan-Adrian Ratiu <adi@adirat.com> - 2015-11-20 21:20 +0100
    Re: [PATCH v2] hid: usbhid: hid-core: fix recursive deadlock Ioan-Adrian Ratiu <adi@adirat.com> - 2015-11-29 11:30 +0100

csiph-web