From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <linux-kernel-owner@vger.kernel.org>
Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand
	id S1753445AbcCGQlo (ORCPT <rfc822;w@1wt.eu>);
	Mon, 7 Mar 2016 11:41:44 -0500
Received: from iolanthe.rowland.org ([192.131.102.54]:40862 "HELO
	iolanthe.rowland.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org
	with SMTP id S1753423AbcCGQli (ORCPT
	<rfc822;linux-kernel@vger.kernel.org>);
	Mon, 7 Mar 2016 11:41:38 -0500
Date: Mon, 7 Mar 2016 11:41:37 -0500 (EST)
From: Alan Stern <stern@rowland.harvard.edu>
X-X-Sender: stern@iolanthe.rowland.org
To: Sedat Dilek <sedat.dilek@gmail.com>
cc: Steven Rostedt <rostedt@goodmis.org>, Jiri Kosina <jikos@kernel.org>,
        Tejun Heo <tj@kernel.org>, Lai Jiangshan <jiangshanlai@gmail.com>,
        Benjamin Tissoires <benjamin.tissoires@redhat.com>,
        Paul McKenney <paulmck@linux.vnet.ibm.com>,
        Andy Lutomirski <luto@kernel.org>, LKML <linux-kernel@vger.kernel.org>,
        USB list <linux-usb@vger.kernel.org>,
        Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
        Peter Zijlstra <peterz@infradead.org>, Ingo Molnar <mingo@redhat.com>
Subject: Re: [PATCH] usbhid: Fix lockdep unannotated irqs-off warning
In-Reply-To: <CA+icZUWaFhN09TBVkS4kDxVbe65GgGR4VMVaa2zcVL=fs=ioSA@mail.gmail.com>
Message-ID: <Pine.LNX.4.44L0.1603071106030.1611-100000@iolanthe.rowland.org>
MIME-Version: 1.0
Content-Type: TEXT/PLAIN; charset=US-ASCII
Sender: linux-kernel-owner@vger.kernel.org
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

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; }
> /<usbhid_close>:/ { 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 <usbhid_close>:
>      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 <usbhid_close+0x23>

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 <usbhid_close+0x32>

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 <usbhid_close+0x44>
>      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 <usbhid_close+0x6c>

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 <hid_cancel_delayed_stuff>

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.

Alan Stern