Re: Clang and X86-EFlags (was Re: [PATCH] usbhid: Fix lockdep unannotated irqs-off warning)

[Sedat Dilek]

On Tue, Apr 24, 2018 at 5:22 PM, Linus Torvalds
 wrote:
> On Tue, Apr 24, 2018 at 6:28 AM, Sedat Dilek  wrote:
>>
>> $ objdump -S clang-eflag.o
>>
>> Does this now look good?
>
> Looks fine to me. The instruction choice is still pretty odd:
>
>   1a:   f6 c3 fftest   $0xff,%bl
>   1d:   74 02   je 21 
>
> that "test   $0xff,%bl" is a rather odd way of testing the byte for zero, 
> since
>
>   1a:   84 dbtest   %bl,%bl
>   1c:   74 02   je 20 
>
> would have been a byte shorter and is the canonical way on x86 to test
> a register against zero.
>
> But that's just a "looks odd to somebody who is used to x86 asm", not
> a bug or anything really noticeable.
>
>   Linus

Thanks for the quick reply and the confirmation.

Personally, I do not speak x86/asm.
I hope CCed LLVM folks know on how to improve things.

- Sedat -
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: Clang and X86-EFlags (was Re: [PATCH] usbhid: Fix lockdep unannotated irqs-off warning)

[Linus Torvalds]

On Tue, Apr 24, 2018 at 6:28 AM, Sedat Dilek  wrote:
>
> $ objdump -S clang-eflag.o
>
> Does this now look good?

Looks fine to me. The instruction choice is still pretty odd:

  1a:   f6 c3 fftest   $0xff,%bl
  1d:   74 02   je 21 

that "test   $0xff,%bl" is a rather odd way of testing the byte for zero, since

  1a:   84 dbtest   %bl,%bl
  1c:   74 02   je 20 

would have been a byte shorter and is the canonical way on x86 to test
a register against zero.

But that's just a "looks odd to somebody who is used to x86 asm", not
a bug or anything really noticeable.

  Linus
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: Clang and X86-EFlags (was Re: [PATCH] usbhid: Fix lockdep unannotated irqs-off warning)

[Sedat Dilek]

On Tue, Apr 24, 2018 at 3:28 PM, Sedat Dilek  wrote:
> On Mon, Jun 27, 2016 at 10:14 PM, Linus Torvalds
>  wrote:
>> On Mon, Jun 27, 2016 at 12:50 PM, Sedat Dilek  wrote:
>>>
>>> $ objdump -S clang-eflag.o
>>>
>>> clang-eflag.o: file format elf64-x86-64
>>>
>>>
>>> Disassembly of section .text:
>>>
>>>  :
>>>0:   55  push   %rbp
>>>1:   48 89 e5mov%rsp,%rbp
>>>4:   53  push   %rbx
>>>5:   50  push   %rax
>>>6:   e8 00 00 00 00  callq  b 
>>>b:   ff 0d 00 00 00 00   decl   0x0(%rip)# 11 
>>>   11:   9c  pushfq
>>>   12:   5b  pop%rbx
>>>   13:   e8 00 00 00 00  callq  18 
>>>   18:   b8 01 00 00 00  mov$0x1,%eax
>>>   1d:   53  push   %rbx
>>>   1e:   9d  popfq
>>>   1f:   75 07   jne28 
>>
>>
>> Yeah, the above is pure garbage.
>>
>>> So, the issue is still alive.
>>>
>>> What do you mean by "for the kernel we at a minimum need a way to
>>> disable that code generation"?
>>> Can this be fixed in the Linux-kernel?
>>
>> No. This will never be fixed in the kernel. It's a compiler bug.
>>
>> The compiler generates shit code. It's absolutely atrociously bad even
>> if you ignore any kernel issues, because that kind of code just
>> performs badly (the compiler should have used "setcc" or something
>> similar to just set the comparison value, not save and restore eflags.
>>
>> And quite frankly, any compiler writer that thinks it is good code is
>> not somebody I want touching a compiler that the kernel depends on
>> anyway.
>>
>> But it is not just bad code for the kernel, it's actively buggy code,
>> since it corrupts the IF.
>>
>> Until this gets fixed in LLVM, there's no way in hell that we will
>> ever have a kernel compiled with that piece of shit.
>>
>> Really. If the LLVM developers cannot fix their crap code generation,
>> it's not worth touching that shit with a ten-foot pole.
>>
>> I'd love to be able to compile the kernel with LLVM, but the fact that
>> the broken eflags code apparently _still_ hasn't been fixed makes me
>> just go "not worth it".
>>
>> And if the LLVM developers don't see this as an obvious bug, it's even
>> less worth it - because that shows not just that the compiler is
>> broken, but that the developers involved with it are broken too.
>>
>>   Linus
>
> [ Changed Subject ]
> [ CC Matthias ]
> [ CC Michael test-case ]
> [ CC Chandler ]
>
> Hi Linus,
>
> Matthias pointed me in [0] to [1] in the LLVM-BTS.
>
> ...and I tried again the test-case from Michael from [3] "[LLVMdev]
> optimizer clobber EFLAGS"...
>
> ...with clang-7 (version:
> 7~svn330207-1~exp1+0~20180417201234.1709~1.gbp6fb10d) from
> .
>
> [ TEST-CASE ]
>
> [ clang-eflag.c ]
> #include 
> #include 
>
> void foo(void);
> int a;
>
> int bar(void)
> {
>  foo();
>
>  bool const zero = a -= 1;
>
>  asm volatile ("" : : : "cc");
>  foo();
>
>  if (zero) {
>  return EXIT_FAILURE;
>  }
>
>  foo();
>
>  return EXIT_SUCCESS;
> }
> - EOF -
>
> $ clang-7 -O2 -c -o clang-eflag.o clang-eflag.c
>
> [ OBJDUMP ]
>
> $ objdump -S clang-eflag.o
>
> clang-eflag.o: file format elf64-x86-64
>
>
> Disassembly of section .text:
>
>  :
>0:   53  push   %rbx
>1:   e8 00 00 00 00  callq  6 
>6:   83 05 00 00 00 00 ffaddl   $0x,0x0(%rip)#
> d 
>d:   0f 95 c3setne  %bl
>   10:   e8 00 00 00 00  callq  15 
>   15:   b8 01 00 00 00  mov$0x1,%eax
>   1a:   f6 c3 fftest   $0xff,%bl
>   1d:   74 02   je 21 
>   1f:   5b  pop%rbx
>   20:   c3  retq
>   21:   e8 00 00 00 00  callq  26 
>   26:   31 c0   xor%eax,%eax
>   28:   5b  pop%rbx
>   29:   c3  retq
>
> Does this now look good?
>
> Thanks.
>
> Kind regards,
> - Sedat -
>
> [0] https://marc.info/?l=linux-kernel=152450535720279=2
> [1] https://bugs.llvm.org/show_bug.cgi?id=36028
> [2] http://lists.llvm.org/pipermail/llvm-dev/2015-July/088766.html
> [3] https://marc.info/?l=linux-kernel=152457089205170=2

For the sake of completeness the two fixes in LLVM upstream.

>From [4]:

Chandler Carruth 2018-04-09 23:41:58 PDT

This should finally be resolved in its entirety with r329657 (and r329673).

"[x86] Introduce a pass to begin more systematically fixing PR36028
and similar issues"
https://github.com/llvm-mirror/llvm/commit/21a0c18174343502c9f2b546a01333d1c351d9c0
svn -r329657

"[x86] Model 

Clang and X86-EFlags (was Re: [PATCH] usbhid: Fix lockdep unannotated irqs-off warning)

[Sedat Dilek]

On Mon, Jun 27, 2016 at 10:14 PM, Linus Torvalds
 wrote:
> On Mon, Jun 27, 2016 at 12:50 PM, Sedat Dilek  wrote:
>>
>> $ objdump -S clang-eflag.o
>>
>> clang-eflag.o: file format elf64-x86-64
>>
>>
>> Disassembly of section .text:
>>
>>  :
>>0:   55  push   %rbp
>>1:   48 89 e5mov%rsp,%rbp
>>4:   53  push   %rbx
>>5:   50  push   %rax
>>6:   e8 00 00 00 00  callq  b 
>>b:   ff 0d 00 00 00 00   decl   0x0(%rip)# 11 
>>   11:   9c  pushfq
>>   12:   5b  pop%rbx
>>   13:   e8 00 00 00 00  callq  18 
>>   18:   b8 01 00 00 00  mov$0x1,%eax
>>   1d:   53  push   %rbx
>>   1e:   9d  popfq
>>   1f:   75 07   jne28 
>
>
> Yeah, the above is pure garbage.
>
>> So, the issue is still alive.
>>
>> What do you mean by "for the kernel we at a minimum need a way to
>> disable that code generation"?
>> Can this be fixed in the Linux-kernel?
>
> No. This will never be fixed in the kernel. It's a compiler bug.
>
> The compiler generates shit code. It's absolutely atrociously bad even
> if you ignore any kernel issues, because that kind of code just
> performs badly (the compiler should have used "setcc" or something
> similar to just set the comparison value, not save and restore eflags.
>
> And quite frankly, any compiler writer that thinks it is good code is
> not somebody I want touching a compiler that the kernel depends on
> anyway.
>
> But it is not just bad code for the kernel, it's actively buggy code,
> since it corrupts the IF.
>
> Until this gets fixed in LLVM, there's no way in hell that we will
> ever have a kernel compiled with that piece of shit.
>
> Really. If the LLVM developers cannot fix their crap code generation,
> it's not worth touching that shit with a ten-foot pole.
>
> I'd love to be able to compile the kernel with LLVM, but the fact that
> the broken eflags code apparently _still_ hasn't been fixed makes me
> just go "not worth it".
>
> And if the LLVM developers don't see this as an obvious bug, it's even
> less worth it - because that shows not just that the compiler is
> broken, but that the developers involved with it are broken too.
>
>   Linus

[ Changed Subject ]
[ CC Matthias ]
[ CC Michael test-case ]
[ CC Chandler ]

Hi Linus,

Matthias pointed me in [0] to [1] in the LLVM-BTS.

...and I tried again the test-case from Michael from [3] "[LLVMdev]
optimizer clobber EFLAGS"...

...with clang-7 (version:
7~svn330207-1~exp1+0~20180417201234.1709~1.gbp6fb10d) from
.

[ TEST-CASE ]

[ clang-eflag.c ]
#include 
#include 

void foo(void);
int a;

int bar(void)
{
 foo();

 bool const zero = a -= 1;

 asm volatile ("" : : : "cc");
 foo();

 if (zero) {
 return EXIT_FAILURE;
 }

 foo();

 return EXIT_SUCCESS;
}
- EOF -

$ clang-7 -O2 -c -o clang-eflag.o clang-eflag.c

[ OBJDUMP ]

$ objdump -S clang-eflag.o

clang-eflag.o: file format elf64-x86-64


Disassembly of section .text:

 :
   0:   53  push   %rbx
   1:   e8 00 00 00 00  callq  6 
   6:   83 05 00 00 00 00 ffaddl   $0x,0x0(%rip)#
d 
   d:   0f 95 c3setne  %bl
  10:   e8 00 00 00 00  callq  15 
  15:   b8 01 00 00 00  mov$0x1,%eax
  1a:   f6 c3 fftest   $0xff,%bl
  1d:   74 02   je 21 
  1f:   5b  pop%rbx
  20:   c3  retq
  21:   e8 00 00 00 00  callq  26 
  26:   31 c0   xor%eax,%eax
  28:   5b  pop%rbx
  29:   c3  retq

Does this now look good?

Thanks.

Kind regards,
- Sedat -

[0] https://marc.info/?l=linux-kernel=152450535720279=2
[1] https://bugs.llvm.org/show_bug.cgi?id=36028
[2] http://lists.llvm.org/pipermail/llvm-dev/2015-July/088766.html
[3] https://marc.info/?l=linux-kernel=152457089205170=2
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] usbhid: Fix lockdep unannotated irqs-off warning

[Linus Torvalds]

On Mon, Jun 27, 2016 at 1:27 PM, Sedat Dilek  wrote:
>
> I just grepped for some "buzzwords" people gave me in this
> email-thread and I was looking at (llvm.git HEAD - upcoming v3.9
> release) and found these comments in [1]
>
>
> [1] 
> https://github.com/llvm-mirror/llvm/blob/master/lib/Target/X86/X86InstrInfo.cpp#L4516

Christ. That's still pretty bad. Using LAHF/SAHF is just wrong.

But at least it's not semantically buggy any more, it's just stupid and slow.

Apparently the problem is that LLVM doesn't actually track flags as
different conditions, but as a single register, and doesn't know which
bits of it matter.

I guess the SETO + LAHF/SAHF is the best llvm can do then. But it
doesn't speak well of the code generation quality.

Linus
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] usbhid: Fix lockdep unannotated irqs-off warning

[Sedat Dilek]

On Mon, Jun 27, 2016 at 10:14 PM, Linus Torvalds
 wrote:
> On Mon, Jun 27, 2016 at 12:50 PM, Sedat Dilek  wrote:
>>
>> $ objdump -S clang-eflag.o
>>
>> clang-eflag.o: file format elf64-x86-64
>>
>>
>> Disassembly of section .text:
>>
>>  :
>>0:   55  push   %rbp
>>1:   48 89 e5mov%rsp,%rbp
>>4:   53  push   %rbx
>>5:   50  push   %rax
>>6:   e8 00 00 00 00  callq  b 
>>b:   ff 0d 00 00 00 00   decl   0x0(%rip)# 11 
>>   11:   9c  pushfq
>>   12:   5b  pop%rbx
>>   13:   e8 00 00 00 00  callq  18 
>>   18:   b8 01 00 00 00  mov$0x1,%eax
>>   1d:   53  push   %rbx
>>   1e:   9d  popfq
>>   1f:   75 07   jne28 
>
>
> Yeah, the above is pure garbage.
>
>> So, the issue is still alive.
>>
>> What do you mean by "for the kernel we at a minimum need a way to
>> disable that code generation"?
>> Can this be fixed in the Linux-kernel?
>
> No. This will never be fixed in the kernel. It's a compiler bug.
>
> The compiler generates shit code. It's absolutely atrociously bad even
> if you ignore any kernel issues, because that kind of code just
> performs badly (the compiler should have used "setcc" or something
> similar to just set the comparison value, not save and restore eflags.
>
> And quite frankly, any compiler writer that thinks it is good code is
> not somebody I want touching a compiler that the kernel depends on
> anyway.
>
> But it is not just bad code for the kernel, it's actively buggy code,
> since it corrupts the IF.
>
> Until this gets fixed in LLVM, there's no way in hell that we will
> ever have a kernel compiled with that piece of shit.
>
> Really. If the LLVM developers cannot fix their crap code generation,
> it's not worth touching that shit with a ten-foot pole.
>
> I'd love to be able to compile the kernel with LLVM, but the fact that
> the broken eflags code apparently _still_ hasn't been fixed makes me
> just go "not worth it".
>
> And if the LLVM developers don't see this as an obvious bug, it's even
> less worth it - because that shows not just that the compiler is
> broken, but that the developers involved with it are broken too.
>

Thanks for the quick answer.

I just grepped for some "buzzwords" people gave me in this
email-thread and I was looking at (llvm.git HEAD - upcoming v3.9
release) and found these comments in [1]

[ lib/Target/X86/X86InstrInfo.cpp ]

void X86InstrInfo::copyPhysReg()
...
// PUSHF/POPF is also potentially incorrect because it affects other flags
// such as TF/IF/DF, which LLVM doesn't model.
...

- Sedat -

[1] 
https://github.com/llvm-mirror/llvm/blob/master/lib/Target/X86/X86InstrInfo.cpp#L4516
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] usbhid: Fix lockdep unannotated irqs-off warning

[Linus Torvalds]

On Mon, Jun 27, 2016 at 12:50 PM, Sedat Dilek  wrote:
>
> $ objdump -S clang-eflag.o
>
> clang-eflag.o: file format elf64-x86-64
>
>
> Disassembly of section .text:
>
>  :
>0:   55  push   %rbp
>1:   48 89 e5mov%rsp,%rbp
>4:   53  push   %rbx
>5:   50  push   %rax
>6:   e8 00 00 00 00  callq  b 
>b:   ff 0d 00 00 00 00   decl   0x0(%rip)# 11 
>   11:   9c  pushfq
>   12:   5b  pop%rbx
>   13:   e8 00 00 00 00  callq  18 
>   18:   b8 01 00 00 00  mov$0x1,%eax
>   1d:   53  push   %rbx
>   1e:   9d  popfq
>   1f:   75 07   jne28 


Yeah, the above is pure garbage.

> So, the issue is still alive.
>
> What do you mean by "for the kernel we at a minimum need a way to
> disable that code generation"?
> Can this be fixed in the Linux-kernel?

No. This will never be fixed in the kernel. It's a compiler bug.

The compiler generates shit code. It's absolutely atrociously bad even
if you ignore any kernel issues, because that kind of code just
performs badly (the compiler should have used "setcc" or something
similar to just set the comparison value, not save and restore eflags.

And quite frankly, any compiler writer that thinks it is good code is
not somebody I want touching a compiler that the kernel depends on
anyway.

But it is not just bad code for the kernel, it's actively buggy code,
since it corrupts the IF.

Until this gets fixed in LLVM, there's no way in hell that we will
ever have a kernel compiled with that piece of shit.

Really. If the LLVM developers cannot fix their crap code generation,
it's not worth touching that shit with a ten-foot pole.

I'd love to be able to compile the kernel with LLVM, but the fact that
the broken eflags code apparently _still_ hasn't been fixed makes me
just go "not worth it".

And if the LLVM developers don't see this as an obvious bug, it's even
less worth it - because that shows not just that the compiler is
broken, but that the developers involved with it are broken too.

  Linus
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] usbhid: Fix lockdep unannotated irqs-off warning

[Sedat Dilek]

On Mon, Jun 27, 2016 at 9:50 PM, Sedat Dilek  wrote:
> On Mon, Mar 7, 2016 at 7:30 PM, Linus Torvalds
>  wrote:
>> On Mon, Mar 7, 2016 at 10:07 AM, Alan Stern  
>> wrote:
>>>
>>> Of course, there are other ways to save a single flag value (such as
>>> setz).  It's up to the compiler developers to decide what they think is
>>> best.
>>
>> Using 'setcc' to save eflags somewhere is definitely the right thing to do.
>>
>> Using pushf/popf in generated code is completely insane (unless done
>> very localized in a controlled area).
>>
>> It is, in fact, insane and wrong even in user space, since eflags does
>> contain bits that user space itself might be modifying.
>>
>> In fact, even IF may be modified with iopl 3 (thing old X server
>> setups), but ignoring that flag entirely, you have AC that acts in
>> very similar ways (system-wide alignment control) that user space
>> might be using to make sure it doesn't have unaligned accesses.
>>
>> It's rare, yes. But still - this isn't really limited to just the kernel.
>>
>> But perhaps more importantly, I suspect using pushf/popf isn't just
>> semantically the wrong thing to do, it's just plain stupid. It's
>> likely slower than the obvious 'setcc' model. Agner Fog's table shows
>> it "popf" as being 25-30 uops on several microarchitectures. Looks
>> like it's often microcode.
>>
>> Now, pushf/popf may well be fairly cheap on *some* uarchitectures, but
>> it really sounds like a bad idea to use it when not absolutely
>> required. And that is completely independent of the fact that is
>> screws up the IF bit.
>>
>> But yeah, for the kernel we at a minimum need a way to disable that
>> code generation, even if the clang guys might have some insane reason
>> to keep it for other cases.
>>
>
> I am testing my new llvm-toolchain v3.8.1 and a pending x86/hweight
> fix [1] encouraged me to look at this again.

Just for the sake of completeness:

I use the latest Linux v4.4.y LTS for testing (here: v4.4.14) with a
custom llvmlinux-amd64 patchset (on demand I can send it to you).
( With CONFIG_TRACING_SUPPORT=n and CONFIG_PARAVIRT=n )

- Sedat -
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] usbhid: Fix lockdep unannotated irqs-off warning

[Sedat Dilek]

On Mon, Mar 7, 2016 at 7:30 PM, Linus Torvalds
 wrote:
> On Mon, Mar 7, 2016 at 10:07 AM, Alan Stern  wrote:
>>
>> Of course, there are other ways to save a single flag value (such as
>> setz).  It's up to the compiler developers to decide what they think is
>> best.
>
> Using 'setcc' to save eflags somewhere is definitely the right thing to do.
>
> Using pushf/popf in generated code is completely insane (unless done
> very localized in a controlled area).
>
> It is, in fact, insane and wrong even in user space, since eflags does
> contain bits that user space itself might be modifying.
>
> In fact, even IF may be modified with iopl 3 (thing old X server
> setups), but ignoring that flag entirely, you have AC that acts in
> very similar ways (system-wide alignment control) that user space
> might be using to make sure it doesn't have unaligned accesses.
>
> It's rare, yes. But still - this isn't really limited to just the kernel.
>
> But perhaps more importantly, I suspect using pushf/popf isn't just
> semantically the wrong thing to do, it's just plain stupid. It's
> likely slower than the obvious 'setcc' model. Agner Fog's table shows
> it "popf" as being 25-30 uops on several microarchitectures. Looks
> like it's often microcode.
>
> Now, pushf/popf may well be fairly cheap on *some* uarchitectures, but
> it really sounds like a bad idea to use it when not absolutely
> required. And that is completely independent of the fact that is
> screws up the IF bit.
>
> But yeah, for the kernel we at a minimum need a way to disable that
> code generation, even if the clang guys might have some insane reason
> to keep it for other cases.
>

I am testing my new llvm-toolchain v3.8.1 and a pending x86/hweight
fix [1] encouraged me to look at this again.

In [2] I found a simple test program from Michael Hordijk.
( See thread "[LLVMdev] optimizer clobber EFLAGS". )

This is what I see...

$ objdump -S clang-eflag.o

clang-eflag.o: file format elf64-x86-64


Disassembly of section .text:

 :
   0:   55  push   %rbp
   1:   48 89 e5mov%rsp,%rbp
   4:   53  push   %rbx
   5:   50  push   %rax
   6:   e8 00 00 00 00  callq  b 
   b:   ff 0d 00 00 00 00   decl   0x0(%rip)# 11 
  11:   9c  pushfq
  12:   5b  pop%rbx
  13:   e8 00 00 00 00  callq  18 
  18:   b8 01 00 00 00  mov$0x1,%eax
  1d:   53  push   %rbx
  1e:   9d  popfq
  1f:   75 07   jne28 
  21:   e8 00 00 00 00  callq  26 
  26:   31 c0   xor%eax,%eax
  28:   48 83 c4 08 add$0x8,%rsp
  2c:   5b  pop%rbx
  2d:   5d  pop%rbp
  2e:   c3  retq

So, the issue is still alive.

What do you mean by "for the kernel we at a minimum need a way to
disable that code generation"?
Can this be fixed in the Linux-kernel?

I asked parallelly the people involved in [2] if there are any news on that.

- Sedat -

[1] 
http://git.kernel.org/cgit/linux/kernel/git/tip/tip.git/commit/?id=f5967101e9de12addcda4510dfbac66d7c5779c3
[2] http://lists.llvm.org/pipermail/llvm-dev/2015-July/088766.html
// Using Clang/LLVM 3.6.0 we are observing a case where the optimizations
// are clobbering EFLAGS on x86_64.  This is inconvenient when the status of
// bit 9 (IF), which controls interrupts, changes.
//
// Here's a simple test program.  Assume that the external function foo()
// modifies the IF bit in EFLAGS.
//
// And it's compiled using the following command line:
//
// $ clang -O2 -c -o clang-eflag.o clang-eflag.c
//
// Check objdump output using the following command line:
//
// $ objdump -S clang-eflag.o
//
// For more details see "[LLVMdev] optimizer clobber EFLAGS"
// 

#include 
#include 

void foo(void);
int a;

int bar(void)
{
foo();

bool const zero = a -= 1;

asm volatile ("" : : : "cc");
foo();

if (zero) {
return EXIT_FAILURE;
}

foo();

return EXIT_SUCCESS;
}


clang-eflag.o
Description: application/object

Re: [PATCH] usbhid: Fix lockdep unannotated irqs-off warning

[Steven Rostedt]

On Mon, 7 Mar 2016 10:04:21 -0800
Andy Lutomirski  wrote:


> > Exactly. The compiler may get away with this in userspace (maybe), but
> > for the kernel, it is definitely a show stopper. Especially if it knows
> > that an asm() may be called.  
> 
> It's broken for user code that fiddles with AC, too.

Which is why I added the "(maybe)" ;-)

-- Steve
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] usbhid: Fix lockdep unannotated irqs-off warning

[Linus Torvalds]

On Mon, Mar 7, 2016 at 10:07 AM, Alan Stern  wrote:
>
> Of course, there are other ways to save a single flag value (such as
> setz).  It's up to the compiler developers to decide what they think is
> best.

Using 'setcc' to save eflags somewhere is definitely the right thing to do.

Using pushf/popf in generated code is completely insane (unless done
very localized in a controlled area).

It is, in fact, insane and wrong even in user space, since eflags does
contain bits that user space itself might be modifying.

In fact, even IF may be modified with iopl 3 (thing old X server
setups), but ignoring that flag entirely, you have AC that acts in
very similar ways (system-wide alignment control) that user space
might be using to make sure it doesn't have unaligned accesses.

It's rare, yes. But still - this isn't really limited to just the kernel.

But perhaps more importantly, I suspect using pushf/popf isn't just
semantically the wrong thing to do, it's just plain stupid. It's
likely slower than the obvious 'setcc' model. Agner Fog's table shows
it "popf" as being 25-30 uops on several microarchitectures. Looks
like it's often microcode.

Now, pushf/popf may well be fairly cheap on *some* uarchitectures, but
it really sounds like a bad idea to use it when not absolutely
required. And that is completely independent of the fact that is
screws up the IF bit.

But yeah, for the kernel we at a minimum need a way to disable that
code generation, even if the clang guys might have some insane reason
to keep it for other cases.

  Linus
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

RE: [PATCH] usbhid: Fix lockdep unannotated irqs-off warning

[Alan Stern]

On Mon, 7 Mar 2016, David Laight wrote:

> From: Sedat Dilek
> ...
> > Did someone look at the next/follow-ups in this thread?
> > For example: D6629 "x86: Emit LAHF/SAHF instead of PUSHF/POPF" [2]?
> 
> LAHF and SAHF come with the following note:
> 
> This instruction executes as described above in compatibility mode and legacy 
> mode.
> It is valid in 64-bit mode only if CPUID.8001H:ECX.LAHF-SAHF[bit 0] = 1.
> 
> So I suspect they can't be used.

Of course, there are other ways to save a single flag value (such as
setz).  It's up to the compiler developers to decide what they think is
best.

Alan Stern

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] usbhid: Fix lockdep unannotated irqs-off warning

[Andy Lutomirski]

On Mon, Mar 7, 2016 at 9:30 AM, Steven Rostedt  wrote:
> On Mon, 7 Mar 2016 18:24:12 +0100 (CET)
> Jiri Kosina  wrote:
>
>> > 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.
>>
>> This issue needs to be handled in the compiler.
>>
>
> Exactly. The compiler may get away with this in userspace (maybe), but
> for the kernel, it is definitely a show stopper. Especially if it knows
> that an asm() may be called.

It's broken for user code that fiddles with AC, too.

--Andy
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

RE: [PATCH] usbhid: Fix lockdep unannotated irqs-off warning

[David Laight]

From: Sedat Dilek
...
> Did someone look at the next/follow-ups in this thread?
> For example: D6629 "x86: Emit LAHF/SAHF instead of PUSHF/POPF" [2]?

LAHF and SAHF come with the following note:

This instruction executes as described above in compatibility mode and legacy 
mode.
It is valid in 64-bit mode only if CPUID.8001H:ECX.LAHF-SAHF[bit 0] = 1.

So I suspect they can't be used.

David

N�r��yb�X��ǧv�^�)޺{.n�+{��^n�r���z���h�&���G���h�(�階�ݢj"���m��z�ޖ���f���h���~�m�

Re: [PATCH] usbhid: Fix lockdep unannotated irqs-off warning

[Steven Rostedt]

On Mon, 7 Mar 2016 18:24:12 +0100 (CET)
Jiri Kosina  wrote:

> > 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.  
> 
> This issue needs to be handled in the compiler.
> 

Exactly. The compiler may get away with this in userspace (maybe), but
for the kernel, it is definitely a show stopper. Especially if it knows
that an asm() may be called.

-- Steve
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] usbhid: Fix lockdep unannotated irqs-off warning

[Jiri Kosina]

On Mon, 7 Mar 2016, Sedat Dilek wrote:

> Did someone look at the next/follow-ups in this thread?
> For example: D6629 "x86: Emit LAHF/SAHF instead of PUSHF/POPF" [2]?

Using LAHF/SAHF would "solve" it, as IF is at bit #9. The question is 
whether they need to play with flags here at all.

On Mon, 7 Mar 2016, Sedat Dilek wrote:

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

This issue needs to be handled in the compiler.

-- 
Jiri Kosina
SUSE Labs

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] usbhid: Fix lockdep unannotated irqs-off warning

[Sedat Dilek]

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
>> 02e0 :
>>  2e0:   55  push   %rbp
>>  2e1:   48 89 e5mov%rsp,%rbp
>>  2e4:   41 57   push   %r15
>>  2e6:   41 56   push   %r14
>>  2e8:   41 54   push   %r12
>>  2ea:   53  push   %rbx
>>  2eb:   49 89 ffmov%rdi,%r15
>>  2ee:   4d 8b b7 e8 1e 00 00mov0x1ee8(%r15),%r14
>>  2f5:   48 c7 c7 00 00 00 00mov$0x0,%rdi
>>  2fc:   31 f6   xor%esi,%esi
>>  2fe:   e8 00 00 00 00  callq  303 
>
> mutex_lock(_open_mut);
>
>>  303:   49 8d 9e 88 28 00 00lea0x2888(%r14),%rbx
>>  30a:   48 89 dfmov%rbx,%rdi
>>  30d:   e8 00 00 00 00  callq  312 
>
> spin_lock_irq(>lock);
>
>>  312:   41 ff 8f e4 1d 00 00decl   0x1de4(%r15)
>
> --hid->open
>
>>  319:   9c  pushfq
>>  31a:   41 5c   pop%r12
>>  31c:   48 89 dfmov%rbx,%rdi
>>  31f:   e8 00 00 00 00  callq  324 
>>  324:   41 54   push   %r12
>>  326:   9d  popfq
>
> spin_unlock_irq(>lock); while attempting to preserve the Z
> flag.  The problem is that this code sequence will also preserve the
> Interrupt Flag!
>
>>  327:   75 23   jne34c 
>
> if (!--hid->open), testing the Z flag from the decl.
>
>>  329:   4c 89 f7mov%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 -
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] usbhid: Fix lockdep unannotated irqs-off warning

[Sedat Dilek]

On Mon, Mar 7, 2016 at 6:05 PM, Jiri Kosina  wrote:
> On Mon, 7 Mar 2016, Alan Stern wrote:
>
>> >  319:   9c  pushfq
>> >  31a:   41 5c   pop%r12
>> >  31c:   48 89 dfmov%rbx,%rdi
>> >  31f:   e8 00 00 00 00  callq  324 
>> >  324:   41 54   push   %r12
>> >  326:   9d  popfq
>>
>> spin_unlock_irq(>lock); while attempting to preserve the Z
>> flag.  The problem is that this code sequence will also preserve the
>> Interrupt Flag!
>
> You are right Alan, thanks a lot, for reason I could not understand I
> completely missed the pushf/popf last time I was looking at the generated
> assembly!
>
> OK, a little bit of googling revealed related discussion on LLVM
> mailinglist:
>
> http://lists.llvm.org/pipermail/llvm-dev/2015-July/088780.html
>
> Seems like it has been reported already, but noone dared to fix it yet.
>
> This basically makes LLVM unusable for compiling the kernel.
>

OK, OK.

Did someone look at the next/follow-ups in this thread?
For example: D6629 "x86: Emit LAHF/SAHF instead of PUSHF/POPF" [2]?

- Sedat -

[1] http://lists.llvm.org/pipermail/llvm-dev/2015-July/088874.html
[2] http://reviews.llvm.org/D6629
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] usbhid: Fix lockdep unannotated irqs-off warning

[Steven Rostedt]

On Mon, 7 Mar 2016 11:41:37 -0500 (EST)
Alan Stern  wrote:


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


I would state that this is a compiler bug for any kernel development.
Because it's modifying a global variable (IF) that can be modified by
asm(). Clang is assuming that this is userspace where IF can't change.
But because this is kernel space, the IF can (and does here), which
makes this "feature" incompatible with any (Linux or otherwise) kernel
programming.

-- Steve
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] usbhid: Fix lockdep unannotated irqs-off warning

[Jiri Kosina]

On Mon, 7 Mar 2016, Alan Stern wrote:

> >  319:   9c  pushfq
> >  31a:   41 5c   pop%r12
> >  31c:   48 89 dfmov%rbx,%rdi
> >  31f:   e8 00 00 00 00  callq  324 
> >  324:   41 54   push   %r12
> >  326:   9d  popfq
> 
> spin_unlock_irq(>lock); while attempting to preserve the Z
> flag.  The problem is that this code sequence will also preserve the
> Interrupt Flag!

You are right Alan, thanks a lot, for reason I could not understand I 
completely missed the pushf/popf last time I was looking at the generated 
assembly!

OK, a little bit of googling revealed related discussion on LLVM 
mailinglist:

http://lists.llvm.org/pipermail/llvm-dev/2015-July/088780.html

Seems like it has been reported already, but noone dared to fix it yet. 

This basically makes LLVM unusable for compiling the kernel.

-- 
Jiri Kosina
SUSE Labs

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] usbhid: Fix lockdep unannotated irqs-off warning

[Steven Rostedt]

On Mon, 7 Mar 2016 11:41:37 -0500 (EST)
Alan Stern  wrote:

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


Really! This is what's is happening??

Clang takes this:

if (!--hid->open) {
spin_unlock_irq(X);
do_something();
} else {
spin_unlock_irq(X);
}

Thus it's basically doing:

FLAG = !--hid->open;
push flags;
spin_unlock_irq(X)
pop flags;
if (FLAG zero set) {
do_something();
}


OUCH!!! There's gotta be a way to turn that off, otherwise Clang can
not be used to compile the kernel.

Nice detective work.

-- Steve
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] usbhid: Fix lockdep unannotated irqs-off warning

[Alan Stern]

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
> 02e0 :
>  2e0:   55  push   %rbp
>  2e1:   48 89 e5mov%rsp,%rbp
>  2e4:   41 57   push   %r15
>  2e6:   41 56   push   %r14
>  2e8:   41 54   push   %r12
>  2ea:   53  push   %rbx
>  2eb:   49 89 ffmov%rdi,%r15
>  2ee:   4d 8b b7 e8 1e 00 00mov0x1ee8(%r15),%r14
>  2f5:   48 c7 c7 00 00 00 00mov$0x0,%rdi
>  2fc:   31 f6   xor%esi,%esi
>  2fe:   e8 00 00 00 00  callq  303 

mutex_lock(_open_mut);

>  303:   49 8d 9e 88 28 00 00lea0x2888(%r14),%rbx
>  30a:   48 89 dfmov%rbx,%rdi
>  30d:   e8 00 00 00 00  callq  312 

spin_lock_irq(>lock);

>  312:   41 ff 8f e4 1d 00 00decl   0x1de4(%r15)

--hid->open

>  319:   9c  pushfq
>  31a:   41 5c   pop%r12
>  31c:   48 89 dfmov%rbx,%rdi
>  31f:   e8 00 00 00 00  callq  324 
>  324:   41 54   push   %r12
>  326:   9d  popfq

spin_unlock_irq(>lock); while attempting to preserve the Z
flag.  The problem is that this code sequence will also preserve the
Interrupt Flag!

>  327:   75 23   jne34c 

if (!--hid->open), testing the Z flag from the decl.

>  329:   4c 89 f7mov%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.

Alan Stern

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] usbhid: Fix lockdep unannotated irqs-off warning

[Sedat Dilek]

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(>io_retry);
>>> > +   if (raw_irqs_disabled())  pr_info("usbhid irqs disabled B\n");
>>> > cancel_work_sync(>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(_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(>lock);
>> if (!--hid->open) {
>> spin_unlock_irq(>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 -
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] usbhid: Fix lockdep unannotated irqs-off warning

[Sedat Dilek]

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(>io_retry);
>> > +   if (raw_irqs_disabled())  pr_info("usbhid irqs disabled B\n");
>> > cancel_work_sync(>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(_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(>lock);
> if (!--hid->open) {
> spin_unlock_irq(>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 !

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

$ cat ../objdump-D_hid-core_o_usbhid_close_4.4.4-1-iniza-small.txt
02e0 :
 2e0:   55  push   %rbp
 2e1:   48 89 e5mov%rsp,%rbp
 2e4:   41 57   push   %r15
 2e6:   41 56   push   %r14
 2e8:   41 54   push   %r12
 2ea:   53  push   %rbx
 2eb:   49 89 ffmov%rdi,%r15
 2ee:   4d 8b b7 e8 1e 00 00mov0x1ee8(%r15),%r14
 2f5:   48 c7 c7 00 00 00 00mov$0x0,%rdi
 2fc:   31 f6   xor%esi,%esi
 2fe:   e8 00 00 00 00  callq  303 
 303:   49 8d 9e 88 28 00 00lea0x2888(%r14),%rbx
 30a:   48 89 dfmov%rbx,%rdi
 30d:   e8 00 00 00 00  callq  312 
 312:   41 ff 8f e4 1d 00 00decl   0x1de4(%r15)
 319:   9c  pushfq
 31a:   41 5c   pop%r12
 31c:   48 89 dfmov%rbx,%rdi
 31f:   e8 00 00 00 00  callq  324 
 324:   41 54   push   %r12
 326:   9d  popfq
 327:   75 23   jne34c 

Re: [PATCH] usbhid: Fix lockdep unannotated irqs-off warning

[Alan Stern]

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(>io_retry);
> > +   if (raw_irqs_disabled())  pr_info("usbhid irqs disabled B\n");
> > cancel_work_sync(>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(_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(>lock);
if (!--hid->open) {
spin_unlock_irq(>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

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] usbhid: Fix lockdep unannotated irqs-off warning

[Alan Stern]

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(>io_retry);
+   if (raw_irqs_disabled())  pr_info("usbhid irqs disabled B\n");
cancel_work_sync(>reset_work);
+   if (raw_irqs_disabled())  pr_info("usbhid irqs disabled C\n");
 }
 
 static void hid_cease_io(struct usbhid_device *usbhid)

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] usbhid: Fix lockdep unannotated irqs-off warning

[Sedat Dilek]

On 3/2/16, Sedat Dilek  wrote:
> On 3/2/16, Peter Zijlstra  wrote:
>> On Wed, Mar 02, 2016 at 04:53:36PM +0100, Sedat Dilek wrote:
>>> 8110f570 :
>>> 8110f570:   55  push   %rbp
>>> 8110f571:   48 89 e5mov%rsp,%rbp
>>> 8110f574:   41 57   push   %r15
>>> 8110f576:   41 56   push   %r14
>>> 8110f578:   53  push   %rbx
>>> 8110f579:   48 83 ec 28 sub$0x28,%rsp
>>
>> stack offset is 0x28 bytes [*]
>>
>>> 8110f57d:   48 89 fbmov%rdi,%rbx
>>> 8110f580:   e8 6b 6e 80 00  callq  819163f0 
>>> 8110f585:   e8 66 6e 80 00  callq  819163f0 
>>> 8110f58a:   e8 61 6e 80 00  callq  819163f0 
>>> 8110f58f:   e8 5c 6e 80 00  callq  819163f0 
>>
>> Your compiler is on drugs!
>>
>>> 8110f594:   9c  pushfq
>>> 8110f595:   8f 45 e0popq   -0x20(%rbp)
>>
>> Saves flags in -0x20(%rbp)
>>
>>> 8110f598:   4c 8b 7d e0 mov-0x20(%rbp),%r15
>>
>> And in %r15
>>
>> /me wonders what's wrong with: popf %r15
>>
>>> 8110f59c:   e8 4f 6e 80 00  callq  819163f0 
>>> 8110f5a1:   e8 4a 6e 80 00  callq  819163f0 
>>> 8110f5a6:   fa  cli
>>> 8110f5a7:   e8 84 cb fc ff  callq  810dc130
>>> 
>>> 8110f5ac:   4c 8d 73 50 lea0x50(%rbx),%r14
>>> 8110f5b0:   48 c7 04 24 b0 f5 10movq
>>> $0x8110f5b0,(%rsp)
>>> 8110f5b7:   81
>>> 8110f5b8:   31 f6   xor%esi,%esi
>>> 8110f5ba:   31 d2   xor%edx,%edx
>>> 8110f5bc:   31 c9   xor%ecx,%ecx
>>> 8110f5be:   41 b8 01 00 00 00   mov$0x1,%r8d
>>> 8110f5c4:   45 31 c9xor%r9d,%r9d
>>> 8110f5c7:   4c 89 f7mov%r14,%rdi
>>> 8110f5ca:   e8 c1 e5 fc ff  callq  810ddb90
>>> 
>>> 8110f5cf:   be 01 00 00 00  mov$0x1,%esi
>>> 8110f5d4:   48 c7 c2 cf f5 10 81mov$0x8110f5cf,%rdx
>>> 8110f5db:   4c 89 f7mov%r14,%rdi
>>> 8110f5de:   e8 8d 08 fd ff  callq  810dfe70
>>> 
>>> 8110f5e3:   e8 08 6e 80 00  callq  819163f0 
>>
>>> 8110f5e8:   4c 89 f8mov%r15,%rax
>>> 8110f5eb:   49 89 c6mov%rax,%r14
>>
>> Moves r15 into r14 through rax
>>
>>
>>> 8110f5ee:   f6 c4 02test   $0x2,%ah
>>> 8110f5f1:   75 19   jne8110f60c
>>> 
>>> 8110f5f3:   e8 f8 6d 80 00  callq  819163f0 
>>> 8110f5f8:   e8 f3 6d 80 00  callq  819163f0 
>>
>>
>>> 8110f5fd:   4c 89 75 d0 mov%r14,-0x30(%rbp)
>>> 8110f601:   ff 75 d0pushq  -0x30(%rbp)
>>> 8110f604:   9d  popfq
>>
>> put r14 into -0x30(rbp) and pushes/pops that, see [*] this is 8 bytes
>> over stack ?!
>>
>>> 8110f605:   e8 26 cb fc ff  callq  810dc130
>>> 
>>> 8110f60a:   eb 17   jmp8110f623
>>> 
>>> 8110f60c:   e8 2f cb fc ff  callq  810dc140
>>> 
>>> 8110f611:   e8 da 6d 80 00  callq  819163f0 
>>> 8110f616:   e8 d5 6d 80 00  callq  819163f0 
>>> 8110f61b:   4c 89 75 d8 mov%r14,-0x28(%rbp)
>>> 8110f61f:   ff 75 d8pushq  -0x28(%rbp)
>>> 8110f622:   9d  popfq
>>
>> puts r14 into -0x28(rbp) and pushes/pops that
>>
>>> 8110f623:   e8 c8 6d 80 00  callq  819163f0 
>>> 8110f628:   65 8b 04 25 d4 ae 00mov%gs:0xaed4,%eax
>>> 8110f62f:   00
>>> 8110f630:   a9 00 00 0f 00  test   $0xf,%eax
>>> 8110f635:   74 25   je 8110f65c
>>> 
>>> 8110f637:   f6 43 2a 20 testb  $0x20,0x2a(%rbx)
>>> 8110f63b:   75 1f   jne8110f65c
>>> 
>>> 8110f63d:   48 c7 c7 04 54 c5 81mov$0x81c55404,%rdi
>>> 8110f644:   be 61 04 00 00  mov$0x461,%esi
>>> 8110f649:   e8 12 c4 f6 ff  callq  8107ba60
>>> 
>>> 8110f64e:   eb 0c   jmp8110f65c
>>> 
>>> 8110f650:   e8 9b 6d 80 00  callq  819163f0 
>>> 8110f655:   e8 96 6d 80 00  callq  819163f0 
>>> 

Re: [PATCH] usbhid: Fix lockdep unannotated irqs-off warning

[Peter Zijlstra]

On Wed, Mar 02, 2016 at 11:35:35AM -0500, Steven Rostedt wrote:
> On Wed, 2 Mar 2016 17:24:12 +0100
> Peter Zijlstra  wrote:
> 
> > On Wed, Mar 02, 2016 at 04:53:36PM +0100, Sedat Dilek wrote:
> > > 8110f570 :
> > > 8110f570: 55  push   %rbp
> > > 8110f571: 48 89 e5mov%rsp,%rbp
> > > 8110f574: 41 57   push   %r15
> > > 8110f576: 41 56   push   %r14
> > > 8110f578: 53  push   %rbx
> > > 8110f579: 48 83 ec 28 sub$0x28,%rsp  
> > 
> > stack offset is 0x28 bytes [*]
> 
> Actually, isn't it really 0x40 bytes? The stack pushed 3 words (8 bytes
> each) before doing the subtract. 0x28 == 40 bytes, 3 * 8 = 24,
> 40 + 24 = 64 == 0x40.

> > > 8110f5fd: 4c 89 75 d0 mov%r14,-0x30(%rbp)
> > > 8110f601: ff 75 d0pushq  -0x30(%rbp)
> > > 8110f604: 9d  popfq
> > 
> > put r14 into -0x30(rbp) and pushes/pops that, see [*] this is 8 bytes
> > over stack ?!
> 
> But from rbp, the stack is 0x40 bytes. This may be fine.

Ah indeed.
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] usbhid: Fix lockdep unannotated irqs-off warning

[Sedat Dilek]

On 3/2/16, Peter Zijlstra  wrote:
> On Wed, Mar 02, 2016 at 04:53:36PM +0100, Sedat Dilek wrote:
>> 8110f570 :
>> 8110f570:55  push   %rbp
>> 8110f571:48 89 e5mov%rsp,%rbp
>> 8110f574:41 57   push   %r15
>> 8110f576:41 56   push   %r14
>> 8110f578:53  push   %rbx
>> 8110f579:48 83 ec 28 sub$0x28,%rsp
>
> stack offset is 0x28 bytes [*]
>
>> 8110f57d:48 89 fbmov%rdi,%rbx
>> 8110f580:e8 6b 6e 80 00  callq  819163f0 
>> 8110f585:e8 66 6e 80 00  callq  819163f0 
>> 8110f58a:e8 61 6e 80 00  callq  819163f0 
>> 8110f58f:e8 5c 6e 80 00  callq  819163f0 
>
> Your compiler is on drugs!
>
>> 8110f594:9c  pushfq
>> 8110f595:8f 45 e0popq   -0x20(%rbp)
>
> Saves flags in -0x20(%rbp)
>
>> 8110f598:4c 8b 7d e0 mov-0x20(%rbp),%r15
>
> And in %r15
>
> /me wonders what's wrong with: popf %r15
>
>> 8110f59c:e8 4f 6e 80 00  callq  819163f0 
>> 8110f5a1:e8 4a 6e 80 00  callq  819163f0 
>> 8110f5a6:fa  cli
>> 8110f5a7:e8 84 cb fc ff  callq  810dc130
>> 
>> 8110f5ac:4c 8d 73 50 lea0x50(%rbx),%r14
>> 8110f5b0:48 c7 04 24 b0 f5 10movq   
>> $0x8110f5b0,(%rsp)
>> 8110f5b7:81
>> 8110f5b8:31 f6   xor%esi,%esi
>> 8110f5ba:31 d2   xor%edx,%edx
>> 8110f5bc:31 c9   xor%ecx,%ecx
>> 8110f5be:41 b8 01 00 00 00   mov$0x1,%r8d
>> 8110f5c4:45 31 c9xor%r9d,%r9d
>> 8110f5c7:4c 89 f7mov%r14,%rdi
>> 8110f5ca:e8 c1 e5 fc ff  callq  810ddb90
>> 
>> 8110f5cf:be 01 00 00 00  mov$0x1,%esi
>> 8110f5d4:48 c7 c2 cf f5 10 81mov$0x8110f5cf,%rdx
>> 8110f5db:4c 89 f7mov%r14,%rdi
>> 8110f5de:e8 8d 08 fd ff  callq  810dfe70
>> 
>> 8110f5e3:e8 08 6e 80 00  callq  819163f0 
>
>> 8110f5e8:4c 89 f8mov%r15,%rax
>> 8110f5eb:49 89 c6mov%rax,%r14
>
> Moves r15 into r14 through rax
>
>
>> 8110f5ee:f6 c4 02test   $0x2,%ah
>> 8110f5f1:75 19   jne8110f60c
>> 
>> 8110f5f3:e8 f8 6d 80 00  callq  819163f0 
>> 8110f5f8:e8 f3 6d 80 00  callq  819163f0 
>
>
>> 8110f5fd:4c 89 75 d0 mov%r14,-0x30(%rbp)
>> 8110f601:ff 75 d0pushq  -0x30(%rbp)
>> 8110f604:9d  popfq
>
> put r14 into -0x30(rbp) and pushes/pops that, see [*] this is 8 bytes
> over stack ?!
>
>> 8110f605:e8 26 cb fc ff  callq  810dc130
>> 
>> 8110f60a:eb 17   jmp8110f623
>> 
>> 8110f60c:e8 2f cb fc ff  callq  810dc140
>> 
>> 8110f611:e8 da 6d 80 00  callq  819163f0 
>> 8110f616:e8 d5 6d 80 00  callq  819163f0 
>> 8110f61b:4c 89 75 d8 mov%r14,-0x28(%rbp)
>> 8110f61f:ff 75 d8pushq  -0x28(%rbp)
>> 8110f622:9d  popfq
>
> puts r14 into -0x28(rbp) and pushes/pops that
>
>> 8110f623:e8 c8 6d 80 00  callq  819163f0 
>> 8110f628:65 8b 04 25 d4 ae 00mov%gs:0xaed4,%eax
>> 8110f62f:00
>> 8110f630:a9 00 00 0f 00  test   $0xf,%eax
>> 8110f635:74 25   je 8110f65c
>> 
>> 8110f637:f6 43 2a 20 testb  $0x20,0x2a(%rbx)
>> 8110f63b:75 1f   jne8110f65c
>> 
>> 8110f63d:48 c7 c7 04 54 c5 81mov$0x81c55404,%rdi
>> 8110f644:be 61 04 00 00  mov$0x461,%esi
>> 8110f649:e8 12 c4 f6 ff  callq  8107ba60
>> 
>> 8110f64e:eb 0c   jmp8110f65c
>> 
>> 8110f650:e8 9b 6d 80 00  callq  819163f0 
>> 8110f655:e8 96 6d 80 00  callq  819163f0 
>> 8110f65a:f3 90   pause
>> 8110f65c:48 89 dfmov   

Re: [PATCH] usbhid: Fix lockdep unannotated irqs-off warning

[Steven Rostedt]

On Wed, 2 Mar 2016 17:24:12 +0100
Peter Zijlstra  wrote:

> On Wed, Mar 02, 2016 at 04:53:36PM +0100, Sedat Dilek wrote:
> > 8110f570 :
> > 8110f570:   55  push   %rbp
> > 8110f571:   48 89 e5mov%rsp,%rbp
> > 8110f574:   41 57   push   %r15
> > 8110f576:   41 56   push   %r14
> > 8110f578:   53  push   %rbx
> > 8110f579:   48 83 ec 28 sub$0x28,%rsp  
> 
> stack offset is 0x28 bytes [*]

Actually, isn't it really 0x40 bytes? The stack pushed 3 words (8 bytes
each) before doing the subtract. 0x28 == 40 bytes, 3 * 8 = 24,
40 + 24 = 64 == 0x40.

> 
> > 8110f57d:   48 89 fbmov%rdi,%rbx
> > 8110f580:   e8 6b 6e 80 00  callq  819163f0 
> > 8110f585:   e8 66 6e 80 00  callq  819163f0 
> > 8110f58a:   e8 61 6e 80 00  callq  819163f0 
> > 8110f58f:   e8 5c 6e 80 00  callq  819163f0 
> >   
> 
> Your compiler is on drugs!

Totally agree!


[..]

> 
> > 8110f5fd:   4c 89 75 d0 mov%r14,-0x30(%rbp)
> > 8110f601:   ff 75 d0pushq  -0x30(%rbp)
> > 8110f604:   9d  popfq
> 
> put r14 into -0x30(rbp) and pushes/pops that, see [*] this is 8 bytes
> over stack ?!

But from rbp, the stack is 0x40 bytes. This may be fine.

-- Steve
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] usbhid: Fix lockdep unannotated irqs-off warning

[Peter Zijlstra]

On Wed, Mar 02, 2016 at 04:53:36PM +0100, Sedat Dilek wrote:
> 8110f570 :
> 8110f570: 55  push   %rbp
> 8110f571: 48 89 e5mov%rsp,%rbp
> 8110f574: 41 57   push   %r15
> 8110f576: 41 56   push   %r14
> 8110f578: 53  push   %rbx
> 8110f579: 48 83 ec 28 sub$0x28,%rsp

stack offset is 0x28 bytes [*]

> 8110f57d: 48 89 fbmov%rdi,%rbx
> 8110f580: e8 6b 6e 80 00  callq  819163f0 
> 8110f585: e8 66 6e 80 00  callq  819163f0 
> 8110f58a: e8 61 6e 80 00  callq  819163f0 
> 8110f58f: e8 5c 6e 80 00  callq  819163f0 

Your compiler is on drugs!

> 8110f594: 9c  pushfq 
> 8110f595: 8f 45 e0popq   -0x20(%rbp)

Saves flags in -0x20(%rbp)

> 8110f598: 4c 8b 7d e0 mov-0x20(%rbp),%r15

And in %r15

/me wonders what's wrong with: popf %r15

> 8110f59c: e8 4f 6e 80 00  callq  819163f0 
> 8110f5a1: e8 4a 6e 80 00  callq  819163f0 
> 8110f5a6: fa  cli
> 8110f5a7: e8 84 cb fc ff  callq  810dc130 
> 
> 8110f5ac: 4c 8d 73 50 lea0x50(%rbx),%r14
> 8110f5b0: 48 c7 04 24 b0 f5 10movq   
> $0x8110f5b0,(%rsp)
> 8110f5b7: 81 
> 8110f5b8: 31 f6   xor%esi,%esi
> 8110f5ba: 31 d2   xor%edx,%edx
> 8110f5bc: 31 c9   xor%ecx,%ecx
> 8110f5be: 41 b8 01 00 00 00   mov$0x1,%r8d
> 8110f5c4: 45 31 c9xor%r9d,%r9d
> 8110f5c7: 4c 89 f7mov%r14,%rdi
> 8110f5ca: e8 c1 e5 fc ff  callq  810ddb90 
> 
> 8110f5cf: be 01 00 00 00  mov$0x1,%esi
> 8110f5d4: 48 c7 c2 cf f5 10 81mov$0x8110f5cf,%rdx
> 8110f5db: 4c 89 f7mov%r14,%rdi
> 8110f5de: e8 8d 08 fd ff  callq  810dfe70 
> 
> 8110f5e3: e8 08 6e 80 00  callq  819163f0 

> 8110f5e8: 4c 89 f8mov%r15,%rax
> 8110f5eb: 49 89 c6mov%rax,%r14

Moves r15 into r14 through rax


> 8110f5ee: f6 c4 02test   $0x2,%ah
> 8110f5f1: 75 19   jne8110f60c 
> 
> 8110f5f3: e8 f8 6d 80 00  callq  819163f0 
> 8110f5f8: e8 f3 6d 80 00  callq  819163f0 


> 8110f5fd: 4c 89 75 d0 mov%r14,-0x30(%rbp)
> 8110f601: ff 75 d0pushq  -0x30(%rbp)
> 8110f604: 9d  popfq  

put r14 into -0x30(rbp) and pushes/pops that, see [*] this is 8 bytes
over stack ?!

> 8110f605: e8 26 cb fc ff  callq  810dc130 
> 
> 8110f60a: eb 17   jmp8110f623 
> 
> 8110f60c: e8 2f cb fc ff  callq  810dc140 
> 
> 8110f611: e8 da 6d 80 00  callq  819163f0 
> 8110f616: e8 d5 6d 80 00  callq  819163f0 
> 8110f61b: 4c 89 75 d8 mov%r14,-0x28(%rbp)
> 8110f61f: ff 75 d8pushq  -0x28(%rbp)
> 8110f622: 9d  popfq  

puts r14 into -0x28(rbp) and pushes/pops that

> 8110f623: e8 c8 6d 80 00  callq  819163f0 
> 8110f628: 65 8b 04 25 d4 ae 00mov%gs:0xaed4,%eax
> 8110f62f: 00 
> 8110f630: a9 00 00 0f 00  test   $0xf,%eax
> 8110f635: 74 25   je 8110f65c 
> 
> 8110f637: f6 43 2a 20 testb  $0x20,0x2a(%rbx)
> 8110f63b: 75 1f   jne8110f65c 
> 
> 8110f63d: 48 c7 c7 04 54 c5 81mov$0x81c55404,%rdi
> 8110f644: be 61 04 00 00  mov$0x461,%esi
> 8110f649: e8 12 c4 f6 ff  callq  8107ba60 
> 
> 8110f64e: eb 0c   jmp8110f65c 
> 
> 8110f650: e8 9b 6d 80 00  callq  819163f0 
> 8110f655: e8 96 6d 80 00  callq  819163f0 
> 8110f65a: f3 90   pause  
> 8110f65c: 48 89 dfmov%rbx,%rdi
> 8110f65f: e8 4c fe ff ff  callq  8110f4b0 

Re: [PATCH] usbhid: Fix lockdep unannotated irqs-off warning

[Sedat Dilek]

On 3/2/16, Sedat Dilek  wrote:
> On 3/2/16, Sedat Dilek  wrote:
>> On 3/2/16, Steven Rostedt  wrote:
>>>
>>> [ Resend with reply all, instead of just "reply" ]
>>>
>>> On Wed, 2 Mar 2016 16:53:36 +0100
>>> Sedat Dilek  wrote:
>>>
 8110f3f0 :
 8110f3f0:  55  push   %rbp
 8110f3f1:  48 89 e5mov%rsp,%rbp
 8110f3f4:  41 57   push   %r15
 8110f3f6:  41 56   push   %r14
 8110f3f8:  53  push   %rbx
 8110f3f9:  50  push   %rax
 8110f3fa:  48 89 fbmov%rdi,%rbx
 8110f3fd:  e8 ee 6f 80 00  callq  819163f0
 
 8110f402:  e8 e9 6f 80 00  callq  819163f0
 
 8110f407:  e8 e4 6f 80 00  callq  819163f0
 
 8110f40c:  e8 df 6f 80 00  callq  819163f0
 
>>>
>>> What is this about?
>>>
>>
>> Dunno.
>>
>> Not sure if this is relevant or not...
>>
>> [ LINUX-CONFIG ]
>>
>> $ ./scripts/diffconfig /boot/config-4.4.3-3-iniza-small
>> /boot/config-4.4.3-3-llvmlinux-amd64
>>  ARCH_HWEIGHT_CFLAGS "-fcall-saved-rdi -fcall-saved-rsi
>> -fcall-saved-rdx -fcall-saved-rcx -fcall-saved-r8 -fcall-saved-r9
>> -fcall-saved-r10 -fcall-saved-r11" -> ""
>>
>> ...and my patchset on top of Linux v4.4.3 is attached.
>>
>
> That was my full linux-config and now and really the patchset.
> ( If you are ill, stay in your bed. )
>

For the sake of (in)completeness my dmesg after Xorg crashed and my
usbmouse un-/re-plugged.

- Sedat -
[0.00] Initializing cgroup subsys cpuset
[0.00] Initializing cgroup subsys cpu
[0.00] Initializing cgroup subsys cpuacct
[0.00] Linux version 4.4.3-3-llvmlinux-amd64 
(sedat.di...@gmail.com@fambox) (clang version 3.8.0 ) #1 SMP Wed Mar 2 15:36:44 
CET 2016
[0.00] Command line: BOOT_IMAGE=/boot/vmlinuz-4.4.3-3-llvmlinux-amd64 
root=UUID=001AADA61AAD9964 loop=/ubuntu/disks/root.disk ro
[0.00] KERNEL supported cpus:
[0.00]   Intel GenuineIntel
[0.00]   AMD AuthenticAMD
[0.00]   Centaur CentaurHauls
[0.00] Disabled fast string operations
[0.00] x86/fpu: xstate_offset[2]:  576, xstate_sizes[2]:  256
[0.00] x86/fpu: Supporting XSAVE feature 0x01: 'x87 floating point 
registers'
[0.00] x86/fpu: Supporting XSAVE feature 0x02: 'SSE registers'
[0.00] x86/fpu: Supporting XSAVE feature 0x04: 'AVX registers'
[0.00] x86/fpu: Enabled xstate features 0x7, context size is 832 bytes, 
using 'standard' format.
[0.00] x86/fpu: Using 'eager' FPU context switches.
[0.00] e820: BIOS-provided physical RAM map:
[0.00] BIOS-e820: [mem 0x-0x0009d7ff] usable
[0.00] BIOS-e820: [mem 0x0009d800-0x0009] reserved
[0.00] BIOS-e820: [mem 0x000e-0x000f] reserved
[0.00] BIOS-e820: [mem 0x0010-0x1fff] usable
[0.00] BIOS-e820: [mem 0x2000-0x201f] reserved
[0.00] BIOS-e820: [mem 0x2020-0x3fff] usable
[0.00] BIOS-e820: [mem 0x4000-0x401f] reserved
[0.00] BIOS-e820: [mem 0x4020-0xd9c9efff] usable
[0.00] BIOS-e820: [mem 0xd9c9f000-0xdae7efff] reserved
[0.00] BIOS-e820: [mem 0xdae7f000-0xdaf9efff] ACPI NVS
[0.00] BIOS-e820: [mem 0xdaf9f000-0xdaffefff] ACPI data
[0.00] BIOS-e820: [mem 0xdafff000-0xdaff] usable
[0.00] BIOS-e820: [mem 0xdb00-0xdf9f] reserved
[0.00] BIOS-e820: [mem 0xf800-0xfbff] reserved
[0.00] BIOS-e820: [mem 0xfec0-0xfec00fff] reserved
[0.00] BIOS-e820: [mem 0xfed08000-0xfed08fff] reserved
[0.00] BIOS-e820: [mem 0xfed1-0xfed19fff] reserved
[0.00] BIOS-e820: [mem 0xfed1c000-0xfed1] reserved
[0.00] BIOS-e820: [mem 0xfee0-0xfee00fff] reserved
[0.00] BIOS-e820: [mem 0xffd8-0x] reserved
[0.00] BIOS-e820: [mem 0x0001-0x00011fdf] usable
[0.00] NX (Execute Disable) protection: active
[0.00] SMBIOS 2.6 present.
[0.00] DMI: SAMSUNG ELECTRONICS CO., LTD. 
530U3BI/530U4BI/530U4BH/530U3BI/530U4BI/530U4BH, BIOS 13XK 03/28/2013
[0.00] e820: update [mem 0x-0x0fff] usable ==> reserved
[0.00] e820: remove [mem 0x000a-0x000f] usable
[0.00] e820: last_pfn = 0x11fe00 max_arch_pfn = 0x4
[0.00] MTRR default type: 

Re: [PATCH] usbhid: Fix lockdep unannotated irqs-off warning

[Sedat Dilek]

On 3/2/16, Sedat Dilek  wrote:
> On 3/2/16, Steven Rostedt  wrote:
>>
>> [ Resend with reply all, instead of just "reply" ]
>>
>> On Wed, 2 Mar 2016 16:53:36 +0100
>> Sedat Dilek  wrote:
>>
>>> 8110f3f0 :
>>> 8110f3f0:   55  push   %rbp
>>> 8110f3f1:   48 89 e5mov%rsp,%rbp
>>> 8110f3f4:   41 57   push   %r15
>>> 8110f3f6:   41 56   push   %r14
>>> 8110f3f8:   53  push   %rbx
>>> 8110f3f9:   50  push   %rax
>>> 8110f3fa:   48 89 fbmov%rdi,%rbx
>>> 8110f3fd:   e8 ee 6f 80 00  callq  819163f0 
>>> 8110f402:   e8 e9 6f 80 00  callq  819163f0 
>>> 8110f407:   e8 e4 6f 80 00  callq  819163f0 
>>> 8110f40c:   e8 df 6f 80 00  callq  819163f0 
>>
>> What is this about?
>>
>
> Dunno.
>
> Not sure if this is relevant or not...
>
> [ LINUX-CONFIG ]
>
> $ ./scripts/diffconfig /boot/config-4.4.3-3-iniza-small
> /boot/config-4.4.3-3-llvmlinux-amd64
>  ARCH_HWEIGHT_CFLAGS "-fcall-saved-rdi -fcall-saved-rsi
> -fcall-saved-rdx -fcall-saved-rcx -fcall-saved-r8 -fcall-saved-r9
> -fcall-saved-r10 -fcall-saved-r11" -> ""
>
> ...and my patchset on top of Linux v4.4.3 is attached.
>

That was my full linux-config and now and really the patchset.
( If you are ill, stay in your bed. )

- Sedat -
Al Viro (1):
  use ->d_seq to get coherency between ->d_inode and ->d_flags

Sedat Dilek (23):
  kbuild: llvmlinux: Add cross compilation support
  kbuild: llvmlinux: Add support for integrated-assembler (IA)
  kbuild: llvmlinux: Add LLVM bitcode support
  kbuild: llvmlinux: Add some more clang compiler-flags
  kbuild: llvmlinux: Add -Werror compiler-flag to cc-options
  kbuild: llvmlinux: Fix ASM defines
  kbuild: llvmlinux: Fix unsupported -fno-delete-null-pointer-checks compiler-flag
  kbuild: llvmlinux: Fix unsupported -fcatch-undefined-behavior compiler-flag
  compiler-gcc: llvmlinux: Add __maybe_unused attribute for inlining
  fs/compat: llvmlinux: Fix warning in COMPATIBLE_IOCTL define
  bcache: llvmlinux: Replace nested function with __bch_cache_cmp()
  megaraid_sas: llvmlinux: Remove inline from megasas_return_cmd()
  scsi: libosd: llvmlinux: Remove __weak and add __maybe_unused attribute
  mpilib: llvmlinux: Fix compilation with clang
  md/raid10: llvmlinux: Remove VLAIS from handle_reshape_read_error()
  apparmor: llvmlinux: Remove VLAIS from aa_calc_profile_hash()
  x86: llvmlinux: Fix unsupported -falign-{jumps,loops} compiler-flags
  xen: llvmlinux: Remove VLAIS from xen_flush_tlb_others()
  um: llvmlinux: Check for clang compiler in memcpy export
  x86: boot: llvmlinux: Workaround LLVM Bug PR3997
  x86/hweight: boot: llvmlinux: Workaround LLVM Bug PR9457
  Merge branch 'for-4.4/vfs-fixes' into 4.4.3-3-iniza-small
  Merge branch 'for-4.4/llvmlinux-amd64-fixes' into 4.4.3-3-llvmlinux-amd64

 .gitignore|  1 +
 Kbuild|  8 +++
 Makefile  | 36 +++
 arch/x86/Kconfig  |  4 ++--
 arch/x86/Makefile |  6 --
 arch/x86/boot/memory.c|  6 ++
 arch/x86/boot/string.h|  3 +++
 arch/x86/include/asm/arch_hweight.h   | 18 
 arch/x86/um/ksyms.c   |  2 +-
 arch/x86/xen/mmu.c| 35 +++---
 drivers/md/bcache/sysfs.c | 10 +
 drivers/md/raid10.c   |  8 +++
 drivers/scsi/megaraid/megaraid_sas_base.c |  2 +-
 fs/compat_ioctl.c |  2 +-
 fs/dcache.c   | 20 +
 include/linux/compiler-gcc.h  | 12 +--
 include/linux/dcache.h|  4 +---
 include/linux/kbuild.h|  6 +++---
 include/scsi/osd_types.h  |  2 +-
 lib/mpi/Makefile  |  2 ++
 lib/mpi/longlong.h|  9 +++-
 lib/mpi/mpi-inline.h  |  2 +-
 lib/mpi/mpi-internal.h| 10 +
 scripts/Kbuild.include|  6 +++---
 scripts/Makefile.build| 14 
 scripts/mod/Makefile  |  8 +++
 security/apparmor/crypto.c| 17 ++-
 27 files changed, 137 insertions(+), 116 deletions(-)

diff --git a/.gitignore b/.gitignore
index fd3a35592543..34fe1346aa87 100644
--- a/.gitignore
+++ b/.gitignore
@@ -33,6 +33,7 @@
 *.lzo
 *.patch
 *.gcno
+*.ll
 modules.builtin
 Module.symvers
 *.dwo
diff --git a/Kbuild 

Re: [PATCH] usbhid: Fix lockdep unannotated irqs-off warning

[Steven Rostedt]

[ Resend with reply all, instead of just "reply" ]

On Wed, 2 Mar 2016 16:53:36 +0100
Sedat Dilek  wrote:

> 8110f3f0 :
> 8110f3f0: 55  push   %rbp
> 8110f3f1: 48 89 e5mov%rsp,%rbp
> 8110f3f4: 41 57   push   %r15
> 8110f3f6: 41 56   push   %r14
> 8110f3f8: 53  push   %rbx
> 8110f3f9: 50  push   %rax
> 8110f3fa: 48 89 fbmov%rdi,%rbx
> 8110f3fd: e8 ee 6f 80 00  callq  819163f0 
> 8110f402: e8 e9 6f 80 00  callq  819163f0 
> 8110f407: e8 e4 6f 80 00  callq  819163f0 
> 8110f40c: e8 df 6f 80 00  callq  819163f0 

What is this about?

-- Steve
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] usbhid: Fix lockdep unannotated irqs-off warning

[Sedat Dilek]

On 3/2/16, Peter Zijlstra  wrote:
> On Wed, Mar 02, 2016 at 04:00:49PM +0100, Sedat Dilek wrote:
>> >
>> > Right, most odd. Sedat, could you provide objdump -D of the relevant
>> > sections of vmlinux ?
>> >
>>
>> Can you give some clear instructions - for what shall I look for in
>> special?
>
> objdump -D vmlinux | awk '/<[^>]*>:$/ { p=0; } /:/ { p=1; }
> { if (p) print $0; }'
>
> might be a good start, esp. if the output differs between the LLVM and
> GCC cases (+- address muck).
>

Here the most relevant objdumps as single files.

- Sedat -
8109b5d0 :
8109b5d0:   55  push   %rbp
8109b5d1:   48 89 e5mov%rsp,%rbp
8109b5d4:   53  push   %rbx
8109b5d5:   50  push   %rax
8109b5d6:   48 89 fbmov%rdi,%rbx
8109b5d9:   e8 12 ae 87 00  callq  819163f0 
8109b5de:   31 f6   xor%esi,%esi
8109b5e0:   48 89 dfmov%rbx,%rdi
8109b5e3:   e8 08 00 00 00  callq  8109b5f0 
<__cancel_work_timer>
8109b5e8:   48 83 c4 08 add$0x8,%rsp
8109b5ec:   5b  pop%rbx
8109b5ed:   5d  pop%rbp
8109b5ee:   c3  retq   
8109b5ef:   90  nop

8109b5f0 <__cancel_work_timer>:
8109b5f0:   55  push   %rbp
8109b5f1:   48 89 e5mov%rsp,%rbp
8109b5f4:   41 57   push   %r15
8109b5f6:   41 56   push   %r14
8109b5f8:   41 55   push   %r13
8109b5fa:   41 54   push   %r12
8109b5fc:   53  push   %rbx
8109b5fd:   48 83 ec 48 sub$0x48,%rsp
8109b601:   89 f3   mov%esi,%ebx
8109b603:   49 89 fdmov%rdi,%r13
8109b606:   e8 e5 ad 87 00  callq  819163f0 
8109b60b:   4c 8d 65 b8 lea-0x48(%rbp),%r12
8109b60f:   44 0f b6 fb movzbl %bl,%r15d
8109b613:   48 8d 5d a0 lea-0x60(%rbp),%rbx
8109b617:   eb 17   jmp8109b630 
<__cancel_work_timer+0x40>
8109b619:   48 c7 c7 a0 42 e4 81mov$0x81e442a0,%rdi
8109b620:   48 89 demov%rbx,%rsi
8109b623:   e8 28 84 03 00  callq  810d3a50 

8109b628:   0f 1f 84 00 00 00 00nopl   0x0(%rax,%rax,1)
8109b62f:   00 
8109b630:   4c 89 efmov%r13,%rdi
8109b633:   44 89 femov%r15d,%esi
8109b636:   48 8d 55 d0 lea-0x30(%rbp),%rdx
8109b63a:   e8 91 ed ff ff  callq  8109a3d0 

8109b63f:   41 89 c6mov%eax,%r14d
8109b642:   41 83 fe fe cmp$0xfffe,%r14d
8109b646:   74 07   je 8109b64f 
<__cancel_work_timer+0x5f>
8109b648:   45 85 f6test   %r14d,%r14d
8109b64b:   79 7f   jns8109b6cc 
<__cancel_work_timer+0xdc>
8109b64d:   eb e1   jmp8109b630 
<__cancel_work_timer+0x40>
8109b64f:   e8 9c ad 87 00  callq  819163f0 
8109b654:   65 48 8b 04 25 c0 aemov%gs:0xaec0,%rax
8109b65b:   00 00 
8109b65d:   48 89 45 a8 mov%rax,-0x58(%rbp)
8109b661:   48 c7 45 b0 d0 39 0dmovq   
$0x810d39d0,-0x50(%rbp)
8109b668:   81 
8109b669:   e8 82 ad 87 00  callq  819163f0 
8109b66e:   4c 89 65 b8 mov%r12,-0x48(%rbp)
8109b672:   4c 89 65 c0 mov%r12,-0x40(%rbp)
8109b676:   c7 45 a0 00 00 00 00movl   $0x0,-0x60(%rbp)
8109b67d:   48 c7 45 b0 70 f4 09movq   
$0x8109f470,-0x50(%rbp)
8109b684:   81 
8109b685:   4c 89 6d c8 mov%r13,-0x38(%rbp)
8109b689:   48 c7 c7 a0 42 e4 81mov$0x81e442a0,%rdi
8109b690:   ba 02 00 00 00  mov$0x2,%edx
8109b695:   48 89 demov%rbx,%rsi
8109b698:   e8 e3 80 03 00  callq  810d3780 

8109b69d:   e8 4e ad 87 00  callq  819163f0 
8109b6a2:   e8 49 ad 87 00  callq  819163f0 
8109b6a7:   e8 44 ad 87 00  callq  819163f0 

Re: [PATCH] usbhid: Fix lockdep unannotated irqs-off warning

[Peter Zijlstra]

On Wed, Mar 02, 2016 at 04:00:49PM +0100, Sedat Dilek wrote:
> >
> > Right, most odd. Sedat, could you provide objdump -D of the relevant
> > sections of vmlinux ?
> >
> 
> Can you give some clear instructions - for what shall I look for in special?

objdump -D vmlinux | awk '/<[^>]*>:$/ { p=0; } /:/ { p=1; } { 
if (p) print $0; }'

might be a good start, esp. if the output differs between the LLVM and
GCC cases (+- address muck).
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] usbhid: Fix lockdep unannotated irqs-off warning

[Sedat Dilek]

On 3/1/16, Peter Zijlstra  wrote:
> On Tue, Mar 01, 2016 at 10:07:40AM -0500, Steven Rostedt wrote:
>> On Tue, 1 Mar 2016 11:05:42 +0100
>> Sedat Dilek  wrote:
>>
>>
>> > [ FACT #3: TEST-CASE #2 ]
>> >
>> > The most reliable test-case is to simply unplug my external Logitech
>> > USB mouse - saw this by accident.
>> > YES, it was so simple.
>>
>> Just to clarify, this happens on gcc and clang?
>
> Just clang from what I gather.
>

YES, gcc, but I can crash my Xorg, but do not see a pile of nothing in
my dmesg-log.

>> > --- dmesg_4.5.0-rc6-2-llvmlinux-amd64.txt   2016-02-29
>> > 21:23:56.399691975 +0100
>> > +++ dmesg_4.5.0-rc6-2-llvmlinux-amd64_usbmouse-unplugged.txt
>> > 2016-02-29 21:28:14.401832240 +0100
>> > @@ -832,3 +832,75 @@
>> >  [   66.529779] PPP BSD Compression module registered
>> >  [   66.563013] PPP Deflate Compression module registered
>> >  [   66.978977] usb 2-1.5: USB disconnect, device number 4
>> > +[  321.937369] usb 2-1.4: USB disconnect, device number 3
>> > +[  321.950810] BUG: sleeping function called from invalid context at
>> > kernel/workqueue.c:2785
>> > +[  321.950816] in_atomic(): 0, irqs_disabled(): 1, pid: 44, name:
>> > kworker/2:1
>> > +[  321.950819] 9 locks held by kworker/2:1/44:
>> > +[  321.950820]  #0:  ("usb_hub_wq"){.+.+.+}, at: []
>> > process_one_work+0x30f/0x8d0
>> > +[  321.950830]  #1:  ((>events)){+.+.+.}, at:
>> > [] process_one_work+0x33c/0x8d0
>> > +[  321.950836]  #2:  (>mutex){..}, at: []
>> > hub_event+0x50/0x15b0
>> > +[  321.950844]  #3:  (>mutex){..}, at: []
>> > usb_disconnect+0x5f/0x2c0
>> > +[  321.950849]  #4:  (>mutex){..}, at: []
>> > device_release_driver+0x22/0x40
>> > +[  321.950856]  #5:  (>mutex){..}, at: []
>> > device_release_driver+0x22/0x40
>> > +[  321.950862]  #6:  (input_mutex){+.+.+.}, at: []
>> > __input_unregister_device+0x9a/0x190
>> > +[  321.950869]  #7:  (>mutex#2){+.+...}, at:
>> > [] input_close_device+0x27/0x70
>> > +[  321.950875]  #8:  (hid_open_mut){+.+...}, at: []
>> > usbhid_close+0x28/0xb0 [usbhid]
>> > +[  321.950883] irq event stamp: 47770
>> > +[  321.950885] hardirqs last  enabled at (47769):
>> > [] _raw_spin_unlock_irq+0x32/0x60
>> > +[  321.950889] hardirqs last disabled at (47770):
>> > [] del_timer_sync+0x3c/0x110
>>
>> According to lockdep, interrupts were last disabled in del_timer_sync,
>> and they were never enabled. The numbers in parenthesis show the order
>> of events. _raw_spin_unlock_irq() at 47769, then del_timer_sync at
>> 47770.
>>
>> But why did they not get enabled again? We have:
>>
>>  local_irq_save(flags);
>>  lock_map_acquire(>lockdep_map);
>>  lock_map_release(>lockdep_map);
>>  local_irq_restore(flags);
>>
>> If this caused an issue, then you would have a lockdep splat. But
>> perhaps something corrupted "flags", and interrupts were not re-enabled?
>
> Right, most odd. Sedat, could you provide objdump -D of the relevant
> sections of vmlinux ?
>

Can you give some clear instructions - for what shall I look for in special?

I can send you a 300MiB mail-bomb if you like :-).

$ cd linux-git
$ objdump -D vmlinux > ../objdump-D_vmlinux.txt
$ du -m ../objdump-D_vmlinux.txt
294 ../objdump-D_vmlinux.txt

I have now built with the same kernel-config for gcc and clang and
archived the most interesting stuff.
Before sending the stuff out, I would like to have some clear instructions.

Thanks.

- Sedat -
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] usbhid: Fix lockdep unannotated irqs-off warning

[Sedat Dilek]

On 3/2/16, Jiri Kosina  wrote:
> On Wed, 2 Mar 2016, Sedat Dilek wrote:
>>
>> static bool start_flush_work(struct work_struct *work, struct wq_barrier
>> *barr)
>> {
>> struct worker *worker = NULL;
>> struct worker_pool *pool;
>> struct pool_workqueue *pwq;
>>
>> might_sleep();
>>
>> local_irq_disable();
>> pool = get_work_pool(work);
>> if (!pool) {
>> local_irq_enable();
>> return false;
>> }
>>
>> spin_lock(>lock); <--- XXX: spin_lock_irq() ???
>
> No, this is fine. IRQs are unconditionally disabled a few lines above.
>

You are right, I tried with a substitution and that does not matter.

What about passing flags to local_irq_XXX?
And how do I do that?

- Sedat -

> --
> Jiri Kosina
> SUSE Labs
>
>
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] usbhid: Fix lockdep unannotated irqs-off warning

[Jiri Kosina]

On Wed, 2 Mar 2016, Sedat Dilek wrote:
> 
> static bool start_flush_work(struct work_struct *work, struct wq_barrier 
> *barr)
> {
> struct worker *worker = NULL;
> struct worker_pool *pool;
> struct pool_workqueue *pwq;
> 
> might_sleep();
> 
> local_irq_disable();
> pool = get_work_pool(work);
> if (!pool) {
> local_irq_enable();
> return false;
> }
> 
> spin_lock(>lock); <--- XXX: spin_lock_irq() ???

No, this is fine. IRQs are unconditionally disabled a few lines above.

-- 
Jiri Kosina
SUSE Labs

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] usbhid: Fix lockdep unannotated irqs-off warning

[Sedat Dilek]

On 3/2/16, 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.
>>
>> ...
>>
>>> [ FACT #3: TEST-CASE #2 ]
>>>
>>> The most reliable test-case is to simply unplug my external Logitech
>>> USB mouse - saw this by accident.
>>> YES, it was so simple.
>>>
>>> --- dmesg_4.5.0-rc6-2-llvmlinux-amd64.txt   2016-02-29
>>> 21:23:56.399691975 +0100
>>> +++ dmesg_4.5.0-rc6-2-llvmlinux-amd64_usbmouse-unplugged.txt
>>> 2016-02-29 21:28:14.401832240 +0100
>>> @@ -832,3 +832,75 @@
>>>  [   66.529779] PPP BSD Compression module registered
>>>  [   66.563013] PPP Deflate Compression module registered
>>>  [   66.978977] usb 2-1.5: USB disconnect, device number 4
>>> +[  321.937369] usb 2-1.4: USB disconnect, device number 3
>>> +[  321.950810] BUG: sleeping function called from invalid context at
>>> kernel/workqueue.c:2785
>>> +[  321.950816] in_atomic(): 0, irqs_disabled(): 1, pid: 44, name:
>>> kworker/2:1
>>
>>> +[  321.950885] hardirqs last  enabled at (47769):
>>> [] _raw_spin_unlock_irq+0x32/0x60
>>> +[  321.950889] hardirqs last disabled at (47770):
>>> [] del_timer_sync+0x3c/0x110
>>> +[  321.950894] softirqs last  enabled at (47112):
>>> [] __do_softirq+0x5a2/0x610
>>> +[  321.950898] softirqs last disabled at (47097):
>>> [] irq_exit+0x9c/0x120
>>> +[  321.950903] CPU: 2 PID: 44 Comm: kworker/2:1 Not tainted
>>> 4.5.0-rc6-2-llvmlinux-amd64 #1
>>> +[  321.950905] Hardware name: SAMSUNG ELECTRONICS CO., LTD.
>>> 530U3BI/530U4BI/530U4BH/530U3BI/530U4BI/530U4BH, BIOS 13XK 03/28/2013
>>> +[  321.950908] Workqueue: usb_hub_wq hub_event
>>> +[  321.950911]  8800d3326948 0092 
>>> 8800d4347568
>>> +[  321.950915]  814ba7d4 8800d43474f8 8800d4340cc0
>>> 8800d4347568
>>> +[  321.950919]  810e28fd 0092 0096
>>> 8800d43475a8
>>> +[  321.950923] Call Trace:
>>> +[  321.950927]  [] dump_stack+0xa4/0xf0
>>> +[  321.950931]  [] ? print_irqtrace_events+0xcd/0xe0
>>> +[  321.950936]  [] ___might_sleep+0x295/0x2b0
>>> +[  321.950939]  [] __might_sleep+0x4f/0xc0
>>> +[  321.950943]  [] start_flush_work+0x2f/0x2a0
>>> +[  321.950946]  [] flush_work+0x5c/0x80
>>> +[  321.950949]  [] ? flush_work+0x1a/0x80
>>> +[  321.950953]  [] ? trace_hardirqs_off+0xd/0x10
>>> +[  321.950956]  [] ? try_to_grab_pending+0x4a/0x260
>>> +[  321.950960]  [] __cancel_work_timer+0x197/0x290
>>> +[  321.950963]  [] ? try_to_del_timer_sync+0xad/0xc0
>>> +[  321.950966]  [] cancel_work_sync+0x18/0x20
>>
>> It's possible that this could be a compiler-related error connected
>> with local_irq_save().  __cancel_work_timer() calls
>>
>>  ret = try_to_grab_pending(work, is_dwork, );
>>
>> and try_to_grab_pending() starts this way:
>>
>> static int try_to_grab_pending(struct work_struct *work, bool is_dwork,
>> unsigned long *flags)
>> {
>>  struct worker_pool *pool;
>>  struct pool_workqueue *pwq;
>>
>>  local_irq_save(*flags);
>>
>> Then later on, __cancel_work_timer() does
>>
>>  local_irq_restore(flags);
>>
>> Maybe CLANG doesn't like local_irq_save() applied to a pointer as
>> opposed to a stack variable.
>>
>> As a quick test, try applying the patch below.  (Note that there is a
>> similar construction in kernel/signal.c.)
>>
>> Alan Stern
>>
>>
>>
>> Index: usb-4.4/kernel/workqueue.c
>> ===
>> --- usb-4.4.orig/kernel/workqueue.c
>> +++ usb-4.4/kernel/workqueue.c
>> @@ -1175,12 +1175,14 @@ out_put:
>>   * This function is safe to call from any context including IRQ handler.
>>   */
>>  static int try_to_grab_pending(struct work_struct *work, bool is_dwork,
>> -   unsigned long *flags)
>> +   unsigned long *pflags)
>>  {
>>  struct worker_pool *pool;
>>  struct pool_workqueue *pwq;
>> +unsigned long flags;
>>
>> -local_irq_save(*flags);
>> +local_irq_save(flags);
>> +*pflags = flags;
>>
>>  /* try to steal the timer if it exists */
>>  if (is_dwork) {
>> @@ -1241,7 +1243,7 @@ static int try_to_grab_pending(struct wo
>>  }
>>  spin_unlock(>lock);
>>  fail:
>> -local_irq_restore(*flags);
>> +

Re: [PATCH] usbhid: Fix lockdep unannotated irqs-off warning

[Sedat Dilek]

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.
>
> ...
>
>> [ FACT #3: TEST-CASE #2 ]
>>
>> The most reliable test-case is to simply unplug my external Logitech
>> USB mouse - saw this by accident.
>> YES, it was so simple.
>>
>> --- dmesg_4.5.0-rc6-2-llvmlinux-amd64.txt   2016-02-29
>> 21:23:56.399691975 +0100
>> +++ dmesg_4.5.0-rc6-2-llvmlinux-amd64_usbmouse-unplugged.txt
>> 2016-02-29 21:28:14.401832240 +0100
>> @@ -832,3 +832,75 @@
>>  [   66.529779] PPP BSD Compression module registered
>>  [   66.563013] PPP Deflate Compression module registered
>>  [   66.978977] usb 2-1.5: USB disconnect, device number 4
>> +[  321.937369] usb 2-1.4: USB disconnect, device number 3
>> +[  321.950810] BUG: sleeping function called from invalid context at
>> kernel/workqueue.c:2785
>> +[  321.950816] in_atomic(): 0, irqs_disabled(): 1, pid: 44, name:
>> kworker/2:1
>
>> +[  321.950885] hardirqs last  enabled at (47769):
>> [] _raw_spin_unlock_irq+0x32/0x60
>> +[  321.950889] hardirqs last disabled at (47770):
>> [] del_timer_sync+0x3c/0x110
>> +[  321.950894] softirqs last  enabled at (47112):
>> [] __do_softirq+0x5a2/0x610
>> +[  321.950898] softirqs last disabled at (47097):
>> [] irq_exit+0x9c/0x120
>> +[  321.950903] CPU: 2 PID: 44 Comm: kworker/2:1 Not tainted
>> 4.5.0-rc6-2-llvmlinux-amd64 #1
>> +[  321.950905] Hardware name: SAMSUNG ELECTRONICS CO., LTD.
>> 530U3BI/530U4BI/530U4BH/530U3BI/530U4BI/530U4BH, BIOS 13XK 03/28/2013
>> +[  321.950908] Workqueue: usb_hub_wq hub_event
>> +[  321.950911]  8800d3326948 0092 
>> 8800d4347568
>> +[  321.950915]  814ba7d4 8800d43474f8 8800d4340cc0
>> 8800d4347568
>> +[  321.950919]  810e28fd 0092 0096
>> 8800d43475a8
>> +[  321.950923] Call Trace:
>> +[  321.950927]  [] dump_stack+0xa4/0xf0
>> +[  321.950931]  [] ? print_irqtrace_events+0xcd/0xe0
>> +[  321.950936]  [] ___might_sleep+0x295/0x2b0
>> +[  321.950939]  [] __might_sleep+0x4f/0xc0
>> +[  321.950943]  [] start_flush_work+0x2f/0x2a0
>> +[  321.950946]  [] flush_work+0x5c/0x80
>> +[  321.950949]  [] ? flush_work+0x1a/0x80
>> +[  321.950953]  [] ? trace_hardirqs_off+0xd/0x10
>> +[  321.950956]  [] ? try_to_grab_pending+0x4a/0x260
>> +[  321.950960]  [] __cancel_work_timer+0x197/0x290
>> +[  321.950963]  [] ? try_to_del_timer_sync+0xad/0xc0
>> +[  321.950966]  [] cancel_work_sync+0x18/0x20
>
> It's possible that this could be a compiler-related error connected
> with local_irq_save().  __cancel_work_timer() calls
>
>   ret = try_to_grab_pending(work, is_dwork, );
>
> and try_to_grab_pending() starts this way:
>
> static int try_to_grab_pending(struct work_struct *work, bool is_dwork,
>  unsigned long *flags)
> {
>   struct worker_pool *pool;
>   struct pool_workqueue *pwq;
>
>   local_irq_save(*flags);
>
> Then later on, __cancel_work_timer() does
>
>   local_irq_restore(flags);
>
> Maybe CLANG doesn't like local_irq_save() applied to a pointer as
> opposed to a stack variable.
>
> As a quick test, try applying the patch below.  (Note that there is a
> similar construction in kernel/signal.c.)
>
> Alan Stern
>
>
>
> Index: usb-4.4/kernel/workqueue.c
> ===
> --- usb-4.4.orig/kernel/workqueue.c
> +++ usb-4.4/kernel/workqueue.c
> @@ -1175,12 +1175,14 @@ out_put:
>   * This function is safe to call from any context including IRQ handler.
>   */
>  static int try_to_grab_pending(struct work_struct *work, bool is_dwork,
> -unsigned long *flags)
> +unsigned long *pflags)
>  {
>   struct worker_pool *pool;
>   struct pool_workqueue *pwq;
> + unsigned long flags;
>
> - local_irq_save(*flags);
> + local_irq_save(flags);
> + *pflags = flags;
>
>   /* try to steal the timer if it exists */
>   if (is_dwork) {
> @@ -1241,7 +1243,7 @@ static int try_to_grab_pending(struct wo
>   }
>   spin_unlock(>lock);
>  fail:
> - local_irq_restore(*flags);
> + local_irq_restore(flags);
>   if (work_is_canceling(work))
>   return -ENOENT;
>   cpu_relax();
>
>

might_sleep() is invoked in start_flush_work(), but 

Re: [PATCH] usbhid: Fix lockdep unannotated irqs-off warning

[Sedat Dilek]

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.
>
> ...
>
>> [ FACT #3: TEST-CASE #2 ]
>>
>> The most reliable test-case is to simply unplug my external Logitech
>> USB mouse - saw this by accident.
>> YES, it was so simple.
>>
>> --- dmesg_4.5.0-rc6-2-llvmlinux-amd64.txt   2016-02-29
>> 21:23:56.399691975 +0100
>> +++ dmesg_4.5.0-rc6-2-llvmlinux-amd64_usbmouse-unplugged.txt
>> 2016-02-29 21:28:14.401832240 +0100
>> @@ -832,3 +832,75 @@
>>  [   66.529779] PPP BSD Compression module registered
>>  [   66.563013] PPP Deflate Compression module registered
>>  [   66.978977] usb 2-1.5: USB disconnect, device number 4
>> +[  321.937369] usb 2-1.4: USB disconnect, device number 3
>> +[  321.950810] BUG: sleeping function called from invalid context at
>> kernel/workqueue.c:2785
>> +[  321.950816] in_atomic(): 0, irqs_disabled(): 1, pid: 44, name:
>> kworker/2:1
>
>> +[  321.950885] hardirqs last  enabled at (47769):
>> [] _raw_spin_unlock_irq+0x32/0x60
>> +[  321.950889] hardirqs last disabled at (47770):
>> [] del_timer_sync+0x3c/0x110
>> +[  321.950894] softirqs last  enabled at (47112):
>> [] __do_softirq+0x5a2/0x610
>> +[  321.950898] softirqs last disabled at (47097):
>> [] irq_exit+0x9c/0x120
>> +[  321.950903] CPU: 2 PID: 44 Comm: kworker/2:1 Not tainted
>> 4.5.0-rc6-2-llvmlinux-amd64 #1
>> +[  321.950905] Hardware name: SAMSUNG ELECTRONICS CO., LTD.
>> 530U3BI/530U4BI/530U4BH/530U3BI/530U4BI/530U4BH, BIOS 13XK 03/28/2013
>> +[  321.950908] Workqueue: usb_hub_wq hub_event
>> +[  321.950911]  8800d3326948 0092 
>> 8800d4347568
>> +[  321.950915]  814ba7d4 8800d43474f8 8800d4340cc0
>> 8800d4347568
>> +[  321.950919]  810e28fd 0092 0096
>> 8800d43475a8
>> +[  321.950923] Call Trace:
>> +[  321.950927]  [] dump_stack+0xa4/0xf0
>> +[  321.950931]  [] ? print_irqtrace_events+0xcd/0xe0
>> +[  321.950936]  [] ___might_sleep+0x295/0x2b0
>> +[  321.950939]  [] __might_sleep+0x4f/0xc0
>> +[  321.950943]  [] start_flush_work+0x2f/0x2a0
>> +[  321.950946]  [] flush_work+0x5c/0x80
>> +[  321.950949]  [] ? flush_work+0x1a/0x80
>> +[  321.950953]  [] ? trace_hardirqs_off+0xd/0x10
>> +[  321.950956]  [] ? try_to_grab_pending+0x4a/0x260
>> +[  321.950960]  [] __cancel_work_timer+0x197/0x290
>> +[  321.950963]  [] ? try_to_del_timer_sync+0xad/0xc0
>> +[  321.950966]  [] cancel_work_sync+0x18/0x20
>
> It's possible that this could be a compiler-related error connected
> with local_irq_save().  __cancel_work_timer() calls
>
>   ret = try_to_grab_pending(work, is_dwork, );
>
> and try_to_grab_pending() starts this way:
>
> static int try_to_grab_pending(struct work_struct *work, bool is_dwork,
>  unsigned long *flags)
> {
>   struct worker_pool *pool;
>   struct pool_workqueue *pwq;
>
>   local_irq_save(*flags);
>
> Then later on, __cancel_work_timer() does
>
>   local_irq_restore(flags);
>
> Maybe CLANG doesn't like local_irq_save() applied to a pointer as
> opposed to a stack variable.
>
> As a quick test, try applying the patch below.  (Note that there is a
> similar construction in kernel/signal.c.)
>
> Alan Stern
>
>
>
> Index: usb-4.4/kernel/workqueue.c
> ===
> --- usb-4.4.orig/kernel/workqueue.c
> +++ usb-4.4/kernel/workqueue.c
> @@ -1175,12 +1175,14 @@ out_put:
>   * This function is safe to call from any context including IRQ handler.
>   */
>  static int try_to_grab_pending(struct work_struct *work, bool is_dwork,
> -unsigned long *flags)
> +unsigned long *pflags)
>  {
>   struct worker_pool *pool;
>   struct pool_workqueue *pwq;
> + unsigned long flags;
>
> - local_irq_save(*flags);
> + local_irq_save(flags);
> + *pflags = flags;
>
>   /* try to steal the timer if it exists */
>   if (is_dwork) {
> @@ -1241,7 +1243,7 @@ static int try_to_grab_pending(struct wo
>   }
>   spin_unlock(>lock);
>  fail:
> - local_irq_restore(*flags);
> + local_irq_restore(flags);
>   if (work_is_canceling(work))
>   return -ENOENT;
>   cpu_relax();
>

Hi Alan,

thanks for the quick reply and test-patch.
I 

Re: [PATCH] usbhid: Fix lockdep unannotated irqs-off warning

[Alan Stern]

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.

...

> [ FACT #3: TEST-CASE #2 ]
> 
> The most reliable test-case is to simply unplug my external Logitech
> USB mouse - saw this by accident.
> YES, it was so simple.
> 
> --- dmesg_4.5.0-rc6-2-llvmlinux-amd64.txt   2016-02-29
> 21:23:56.399691975 +0100
> +++ dmesg_4.5.0-rc6-2-llvmlinux-amd64_usbmouse-unplugged.txt
> 2016-02-29 21:28:14.401832240 +0100
> @@ -832,3 +832,75 @@
>  [   66.529779] PPP BSD Compression module registered
>  [   66.563013] PPP Deflate Compression module registered
>  [   66.978977] usb 2-1.5: USB disconnect, device number 4
> +[  321.937369] usb 2-1.4: USB disconnect, device number 3
> +[  321.950810] BUG: sleeping function called from invalid context at
> kernel/workqueue.c:2785
> +[  321.950816] in_atomic(): 0, irqs_disabled(): 1, pid: 44, name: kworker/2:1

> +[  321.950885] hardirqs last  enabled at (47769):
> [] _raw_spin_unlock_irq+0x32/0x60
> +[  321.950889] hardirqs last disabled at (47770):
> [] del_timer_sync+0x3c/0x110
> +[  321.950894] softirqs last  enabled at (47112):
> [] __do_softirq+0x5a2/0x610
> +[  321.950898] softirqs last disabled at (47097):
> [] irq_exit+0x9c/0x120
> +[  321.950903] CPU: 2 PID: 44 Comm: kworker/2:1 Not tainted
> 4.5.0-rc6-2-llvmlinux-amd64 #1
> +[  321.950905] Hardware name: SAMSUNG ELECTRONICS CO., LTD.
> 530U3BI/530U4BI/530U4BH/530U3BI/530U4BI/530U4BH, BIOS 13XK 03/28/2013
> +[  321.950908] Workqueue: usb_hub_wq hub_event
> +[  321.950911]  8800d3326948 0092 
> 8800d4347568
> +[  321.950915]  814ba7d4 8800d43474f8 8800d4340cc0
> 8800d4347568
> +[  321.950919]  810e28fd 0092 0096
> 8800d43475a8
> +[  321.950923] Call Trace:
> +[  321.950927]  [] dump_stack+0xa4/0xf0
> +[  321.950931]  [] ? print_irqtrace_events+0xcd/0xe0
> +[  321.950936]  [] ___might_sleep+0x295/0x2b0
> +[  321.950939]  [] __might_sleep+0x4f/0xc0
> +[  321.950943]  [] start_flush_work+0x2f/0x2a0
> +[  321.950946]  [] flush_work+0x5c/0x80
> +[  321.950949]  [] ? flush_work+0x1a/0x80
> +[  321.950953]  [] ? trace_hardirqs_off+0xd/0x10
> +[  321.950956]  [] ? try_to_grab_pending+0x4a/0x260
> +[  321.950960]  [] __cancel_work_timer+0x197/0x290
> +[  321.950963]  [] ? try_to_del_timer_sync+0xad/0xc0
> +[  321.950966]  [] cancel_work_sync+0x18/0x20

It's possible that this could be a compiler-related error connected 
with local_irq_save().  __cancel_work_timer() calls 

ret = try_to_grab_pending(work, is_dwork, );

and try_to_grab_pending() starts this way:

static int try_to_grab_pending(struct work_struct *work, bool is_dwork,
   unsigned long *flags)
{
struct worker_pool *pool;
struct pool_workqueue *pwq;

local_irq_save(*flags);

Then later on, __cancel_work_timer() does

local_irq_restore(flags);

Maybe CLANG doesn't like local_irq_save() applied to a pointer as 
opposed to a stack variable.

As a quick test, try applying the patch below.  (Note that there is a 
similar construction in kernel/signal.c.)

Alan Stern



Index: usb-4.4/kernel/workqueue.c
===
--- usb-4.4.orig/kernel/workqueue.c
+++ usb-4.4/kernel/workqueue.c
@@ -1175,12 +1175,14 @@ out_put:
  * This function is safe to call from any context including IRQ handler.
  */
 static int try_to_grab_pending(struct work_struct *work, bool is_dwork,
-  unsigned long *flags)
+  unsigned long *pflags)
 {
struct worker_pool *pool;
struct pool_workqueue *pwq;
+   unsigned long flags;
 
-   local_irq_save(*flags);
+   local_irq_save(flags);
+   *pflags = flags;
 
/* try to steal the timer if it exists */
if (is_dwork) {
@@ -1241,7 +1243,7 @@ static int try_to_grab_pending(struct wo
}
spin_unlock(>lock);
 fail:
-   local_irq_restore(*flags);
+   local_irq_restore(flags);
if (work_is_canceling(work))
return -ENOENT;
cpu_relax();

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] usbhid: Fix lockdep unannotated irqs-off warning

[Peter Zijlstra]

On Tue, Mar 01, 2016 at 10:07:40AM -0500, Steven Rostedt wrote:
> On Tue, 1 Mar 2016 11:05:42 +0100
> Sedat Dilek  wrote:
> 
> 
> > [ FACT #3: TEST-CASE #2 ]
> > 
> > The most reliable test-case is to simply unplug my external Logitech
> > USB mouse - saw this by accident.
> > YES, it was so simple.
> 
> Just to clarify, this happens on gcc and clang?

Just clang from what I gather.

> > --- dmesg_4.5.0-rc6-2-llvmlinux-amd64.txt   2016-02-29
> > 21:23:56.399691975 +0100
> > +++ dmesg_4.5.0-rc6-2-llvmlinux-amd64_usbmouse-unplugged.txt
> > 2016-02-29 21:28:14.401832240 +0100
> > @@ -832,3 +832,75 @@
> >  [   66.529779] PPP BSD Compression module registered
> >  [   66.563013] PPP Deflate Compression module registered
> >  [   66.978977] usb 2-1.5: USB disconnect, device number 4
> > +[  321.937369] usb 2-1.4: USB disconnect, device number 3
> > +[  321.950810] BUG: sleeping function called from invalid context at
> > kernel/workqueue.c:2785
> > +[  321.950816] in_atomic(): 0, irqs_disabled(): 1, pid: 44, name: 
> > kworker/2:1
> > +[  321.950819] 9 locks held by kworker/2:1/44:
> > +[  321.950820]  #0:  ("usb_hub_wq"){.+.+.+}, at: []
> > process_one_work+0x30f/0x8d0
> > +[  321.950830]  #1:  ((>events)){+.+.+.}, at:
> > [] process_one_work+0x33c/0x8d0
> > +[  321.950836]  #2:  (>mutex){..}, at: []
> > hub_event+0x50/0x15b0
> > +[  321.950844]  #3:  (>mutex){..}, at: []
> > usb_disconnect+0x5f/0x2c0
> > +[  321.950849]  #4:  (>mutex){..}, at: []
> > device_release_driver+0x22/0x40
> > +[  321.950856]  #5:  (>mutex){..}, at: []
> > device_release_driver+0x22/0x40
> > +[  321.950862]  #6:  (input_mutex){+.+.+.}, at: []
> > __input_unregister_device+0x9a/0x190
> > +[  321.950869]  #7:  (>mutex#2){+.+...}, at:
> > [] input_close_device+0x27/0x70
> > +[  321.950875]  #8:  (hid_open_mut){+.+...}, at: []
> > usbhid_close+0x28/0xb0 [usbhid]
> > +[  321.950883] irq event stamp: 47770
> > +[  321.950885] hardirqs last  enabled at (47769):
> > [] _raw_spin_unlock_irq+0x32/0x60
> > +[  321.950889] hardirqs last disabled at (47770):
> > [] del_timer_sync+0x3c/0x110
> 
> According to lockdep, interrupts were last disabled in del_timer_sync,
> and they were never enabled. The numbers in parenthesis show the order
> of events. _raw_spin_unlock_irq() at 47769, then del_timer_sync at
> 47770.
> 
> But why did they not get enabled again? We have:
> 
>   local_irq_save(flags);
>   lock_map_acquire(>lockdep_map);
>   lock_map_release(>lockdep_map);
>   local_irq_restore(flags);
> 
> If this caused an issue, then you would have a lockdep splat. But
> perhaps something corrupted "flags", and interrupts were not re-enabled?

Right, most odd. Sedat, could you provide objdump -D of the relevant
sections of vmlinux ?
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] usbhid: Fix lockdep unannotated irqs-off warning

[Steven Rostedt]

On Tue, 1 Mar 2016 11:05:42 +0100
Sedat Dilek  wrote:


> [ FACT #3: TEST-CASE #2 ]
> 
> The most reliable test-case is to simply unplug my external Logitech
> USB mouse - saw this by accident.
> YES, it was so simple.

Just to clarify, this happens on gcc and clang?


> 
> --- dmesg_4.5.0-rc6-2-llvmlinux-amd64.txt   2016-02-29
> 21:23:56.399691975 +0100
> +++ dmesg_4.5.0-rc6-2-llvmlinux-amd64_usbmouse-unplugged.txt
> 2016-02-29 21:28:14.401832240 +0100
> @@ -832,3 +832,75 @@
>  [   66.529779] PPP BSD Compression module registered
>  [   66.563013] PPP Deflate Compression module registered
>  [   66.978977] usb 2-1.5: USB disconnect, device number 4
> +[  321.937369] usb 2-1.4: USB disconnect, device number 3
> +[  321.950810] BUG: sleeping function called from invalid context at
> kernel/workqueue.c:2785
> +[  321.950816] in_atomic(): 0, irqs_disabled(): 1, pid: 44, name: kworker/2:1
> +[  321.950819] 9 locks held by kworker/2:1/44:
> +[  321.950820]  #0:  ("usb_hub_wq"){.+.+.+}, at: []
> process_one_work+0x30f/0x8d0
> +[  321.950830]  #1:  ((>events)){+.+.+.}, at:
> [] process_one_work+0x33c/0x8d0
> +[  321.950836]  #2:  (>mutex){..}, at: []
> hub_event+0x50/0x15b0
> +[  321.950844]  #3:  (>mutex){..}, at: []
> usb_disconnect+0x5f/0x2c0
> +[  321.950849]  #4:  (>mutex){..}, at: []
> device_release_driver+0x22/0x40
> +[  321.950856]  #5:  (>mutex){..}, at: []
> device_release_driver+0x22/0x40
> +[  321.950862]  #6:  (input_mutex){+.+.+.}, at: []
> __input_unregister_device+0x9a/0x190
> +[  321.950869]  #7:  (>mutex#2){+.+...}, at:
> [] input_close_device+0x27/0x70
> +[  321.950875]  #8:  (hid_open_mut){+.+...}, at: []
> usbhid_close+0x28/0xb0 [usbhid]
> +[  321.950883] irq event stamp: 47770
> +[  321.950885] hardirqs last  enabled at (47769):
> [] _raw_spin_unlock_irq+0x32/0x60
> +[  321.950889] hardirqs last disabled at (47770):
> [] del_timer_sync+0x3c/0x110

According to lockdep, interrupts were last disabled in del_timer_sync,
and they were never enabled. The numbers in parenthesis show the order
of events. _raw_spin_unlock_irq() at 47769, then del_timer_sync at
47770.

But why did they not get enabled again? We have:

local_irq_save(flags);
lock_map_acquire(>lockdep_map);
lock_map_release(>lockdep_map);
local_irq_restore(flags);

If this caused an issue, then you would have a lockdep splat. But
perhaps something corrupted "flags", and interrupts were not re-enabled?

> +[  321.950894] softirqs last  enabled at (47112):
> [] __do_softirq+0x5a2/0x610
> +[  321.950898] softirqs last disabled at (47097):
> [] irq_exit+0x9c/0x120
> +[  321.950903] CPU: 2 PID: 44 Comm: kworker/2:1 Not tainted
> 4.5.0-rc6-2-llvmlinux-amd64 #1
> +[  321.950905] Hardware name: SAMSUNG ELECTRONICS CO., LTD.
> 530U3BI/530U4BI/530U4BH/530U3BI/530U4BI/530U4BH, BIOS 13XK 03/28/2013
> +[  321.950908] Workqueue: usb_hub_wq hub_event
> +[  321.950911]  8800d3326948 0092 
> 8800d4347568
> +[  321.950915]  814ba7d4 8800d43474f8 8800d4340cc0
> 8800d4347568
> +[  321.950919]  810e28fd 0092 0096
> 8800d43475a8
> +[  321.950923] Call Trace:
> +[  321.950927]  [] dump_stack+0xa4/0xf0
> +[  321.950931]  [] ? print_irqtrace_events+0xcd/0xe0
> +[  321.950936]  [] ___might_sleep+0x295/0x2b0
> +[  321.950939]  [] __might_sleep+0x4f/0xc0

This triggered on the might_sleep() in start_flush_work() because
something left interrupts enable.

Just strange.

-- Steve


> +[  321.950943]  [] start_flush_work+0x2f/0x2a0
> +[  321.950946]  [] flush_work+0x5c/0x80
> +[  321.950949]  [] ? flush_work+0x1a/0x80
> +[  321.950953]  [] ? trace_hardirqs_off+0xd/0x10
> +[  321.950956]  [] ? try_to_grab_pending+0x4a/0x260
> +[  321.950960]  [] __cancel_work_timer+0x197/0x290
> +[  321.950963]  [] ? try_to_del_timer_sync+0xad/0xc0
> +[  321.950966]  [] cancel_work_sync+0x18/0x20
> +[  321.950970]  [] usbhid_close+0x75/0xb0 [usbhid]
> +[  321.950977]  [] hidinput_close+0x31/0x40 [hid]
> +[  321.950982]  [] ? hidinput_open+0x40/0x40 [hid]
> +[  321.950985]  [] input_close_device+0x48/0x70
> +[  321.950988]  [] evdev_cleanup+0xd5/0xe0
> +[  321.950990]  [] evdev_disconnect+0x2c/0x60
> +[  321.950993]  [] __input_unregister_device+0xbe/0x190
> +[  321.950996]  [] input_unregister_device+0x4a/0x80
> +[  321.951001]  [] hidinput_disconnect+0x8f/0xc0 [hid]
> +[  321.951007]  [] hid_device_remove+0xb0/0x130 [hid]
> +[  321.951010]  [] __device_release_driver+0xfd/0x190
> +[  321.951014]  [] device_release_driver+0x2a/0x40
> +[  321.951017]  [] bus_remove_device+0x153/0x190
> +[  321.951020]  [] device_del+0x1db/0x2b0
> +[  321.951025]  [] hid_destroy_device+0x2c/0x60 [hid]
> +[  321.951029]  [] usbhid_disconnect+0x67/0x90 [usbhid]
> +[  321.951033]  [] usb_unbind_interface+0xbf/0x2b0
> +[  321.951037]  [] __device_release_driver+0xfd/0x190
> +[  321.951040]  [] device_release_driver+0x2a/0x40
> +[