From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753138AbcCGQ2g (ORCPT ); Mon, 7 Mar 2016 11:28:36 -0500 Received: from mail-vk0-f41.google.com ([209.85.213.41]:36687 "EHLO mail-vk0-f41.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752957AbcCGQ20 (ORCPT ); Mon, 7 Mar 2016 11:28:26 -0500 MIME-Version: 1.0 Reply-To: sedat.dilek@gmail.com In-Reply-To: References: Date: Mon, 7 Mar 2016 17:28:25 +0100 Message-ID: Subject: Re: [PATCH] usbhid: Fix lockdep unannotated irqs-off warning From: Sedat Dilek To: Alan Stern Cc: Steven Rostedt , Jiri Kosina , Tejun Heo , Lai Jiangshan , Benjamin Tissoires , Paul McKenney , Andy Lutomirski , LKML , USB list , Greg Kroah-Hartman , Peter Zijlstra , Ingo Molnar Content-Type: text/plain; charset=UTF-8 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Mon, Mar 7, 2016 at 4:59 PM, Sedat Dilek wrote: > On Sun, Mar 6, 2016 at 6:23 PM, Alan Stern wrote: >> On Sat, 5 Mar 2016, Sedat Dilek wrote: >> >>> On Fri, Mar 4, 2016 at 5:04 PM, Alan Stern wrote: >>> > On Wed, 2 Mar 2016, Sedat Dilek wrote: >>> > >>> >> On 3/1/16, Alan Stern wrote: >>> >> > On Tue, 1 Mar 2016, Sedat Dilek wrote: >>> >> > >>> >> >> On Tue, Oct 13, 2015 at 2:57 AM, Steven Rostedt >>> >> >> wrote: >>> >> >> > On Sat, 3 Oct 2015 12:05:42 +0200 >>> >> >> > Sedat Dilek wrote: >>> >> >> > >>> >> >> >> So, at the beginning... dunno WTF is causing the problems - no >>> >> >> >> workaround for CLANG. >>> >> >> > >>> >> >> > Probably need to compile with gcc and with clang and look at the binary >>> >> >> > differences. Or at least what objdump shows. >>> >> >> > >>> >> >> >>> >> >> [ Hope to address this issue to the correct people - CCed some people >>> >> >> I taped on their nerves ] >>> >> >> >>> >> >> Not sure if I should open a new thread? >>> >> >> Please, some clear statements on this. >>> >> >> Thanks. >>> >> >> >>> >> >> The issue is still visible and alive. >>> > >>> > I think it would be worthwhile to doublecheck the time at which >>> > interrupts get disabled. Sedat, please try your plug/unplug the USB >>> > mouse test with the patch below. >>> > >>> > Alan Stern >>> > >>> > >>> > >>> > Index: usb-4.4/drivers/hid/usbhid/hid-core.c >>> > =================================================================== >>> > --- usb-4.4.orig/drivers/hid/usbhid/hid-core.c >>> > +++ usb-4.4/drivers/hid/usbhid/hid-core.c >>> > @@ -1393,8 +1393,11 @@ static void usbhid_disconnect(struct usb >>> > >>> > static void hid_cancel_delayed_stuff(struct usbhid_device *usbhid) >>> > { >>> > + if (raw_irqs_disabled()) pr_info("usbhid irqs disabled A\n"); >>> > del_timer_sync(&usbhid->io_retry); >>> > + if (raw_irqs_disabled()) pr_info("usbhid irqs disabled B\n"); >>> > cancel_work_sync(&usbhid->reset_work); >>> > + if (raw_irqs_disabled()) pr_info("usbhid irqs disabled C\n"); >>> > } >>> > >>> > static void hid_cease_io(struct usbhid_device *usbhid) >>> > >>> >>> With your patch I get the dmesg attached. >> >>> [ 22.234758] usbhid irqs disabled A >>> [ 22.234857] usbhid irqs disabled B >>> [ 22.234912] BUG: sleeping function called from invalid context atkernel/workqueue.c:2688 >> >> That's a smoking gun. It means everyone has been looking in the wrong >> place. Can you provide an objdump listing of usbhid_close()? The >> routine starts like this: >> >> void usbhid_close(struct hid_device *hid) >> { >> struct usbhid_device *usbhid = hid->driver_data; >> >> mutex_lock(&hid_open_mut); >> >> /* protecting hid->open to make sure we don't restart >> * data acquistion due to a resumption we no longer >> * care about >> */ >> spin_lock_irq(&usbhid->lock); >> if (!--hid->open) { >> spin_unlock_irq(&usbhid->lock); >> hid_cancel_delayed_stuff(usbhid); >> >> It appears that the spin_unlock_irq() call isn't working. >> >> For extra thoroughness, try putting one of those raw_irqs_disabled() >> checks just before and one just after the spin_lock_irq() line above. >> Maybe also before the mutex_lock() line. >> >> Alan Stern >> > > Hmm, we are there where I was looking at... > > Please, read the reply of Jiri [1], we did some tweaking. > With CONFIG_FTRACE=n and CONFIG_PROVE_LOCKING=n ! > Shall I enable CONFIG_TRACE_IRQFLAGS (CONFIG_PROVE_LOCKING=n disables it)? - Sedat -