From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753414AbcCGRSU (ORCPT ); Mon, 7 Mar 2016 12:18:20 -0500 Received: from mail-vk0-f44.google.com ([209.85.213.44]:33966 "EHLO mail-vk0-f44.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753360AbcCGRSL (ORCPT ); Mon, 7 Mar 2016 12:18:11 -0500 MIME-Version: 1.0 Reply-To: sedat.dilek@gmail.com In-Reply-To: References: Date: Mon, 7 Mar 2016 18:18:09 +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 5:41 PM, Alan Stern wrote: > On Mon, 7 Mar 2016, Sedat Dilek wrote: > >> 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 ! > > Yes, Jiri was looking more or less at the right place but his > conclusions were wrong. > >> *** Part one: ObjectDump of hid-core.o *** >> >> $ objdump -D drivers/hid/usbhid/hid-core.o | awk '/<[^>]*>:$/ { p=0; } >> /:/ { p=1; } { if (p) print $0; }' > >> ../objdump-D_hid-core_o_usbhid_close_$(uname -r).txt > > This reveals the problem, at last... > >> $ cat ../objdump-D_hid-core_o_usbhid_close_4.4.4-1-iniza-small.txt >> 00000000000002e0 : >> 2e0: 55 push %rbp >> 2e1: 48 89 e5 mov %rsp,%rbp >> 2e4: 41 57 push %r15 >> 2e6: 41 56 push %r14 >> 2e8: 41 54 push %r12 >> 2ea: 53 push %rbx >> 2eb: 49 89 ff mov %rdi,%r15 >> 2ee: 4d 8b b7 e8 1e 00 00 mov 0x1ee8(%r15),%r14 >> 2f5: 48 c7 c7 00 00 00 00 mov $0x0,%rdi >> 2fc: 31 f6 xor %esi,%esi >> 2fe: e8 00 00 00 00 callq 303 > > mutex_lock(&hid_open_mut); > >> 303: 49 8d 9e 88 28 00 00 lea 0x2888(%r14),%rbx >> 30a: 48 89 df mov %rbx,%rdi >> 30d: e8 00 00 00 00 callq 312 > > spin_lock_irq(&usbhid->lock); > >> 312: 41 ff 8f e4 1d 00 00 decl 0x1de4(%r15) > > --hid->open > >> 319: 9c pushfq >> 31a: 41 5c pop %r12 >> 31c: 48 89 df mov %rbx,%rdi >> 31f: e8 00 00 00 00 callq 324 >> 324: 41 54 push %r12 >> 326: 9d popfq > > spin_unlock_irq(&usbhid->lock); while attempting to preserve the Z > flag. The problem is that this code sequence will also preserve the > Interrupt Flag! > >> 327: 75 23 jne 34c > > if (!--hid->open), testing the Z flag from the decl. > >> 329: 4c 89 f7 mov %r14,%rdi >> 32c: e8 3f 00 00 00 callq 370 > > But now hid_cancel_delayed_stuff(usbhid) gets called with interrupts > disabled. > > It's hard to call this a compiler bug, but perhaps it is -- I don't > know how programmers are supposed to tell CLANG that a subroutine > modifies the Interrupt Flag in a way that the compiler shouldn't mess > up. > So, if Clang is producing wrong X86 code here, is it possible to turn interrupts on/off manually? But, hmm that affects other places as well in the Linux sources, so. - Sedat -