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

[Sedat Dilek]

On Mon, Jun 27, 2016 at 9:50 PM, Sedat Dilek <sedat.dilek@xxxxxxxxx> wrote:
> On Mon, Mar 7, 2016 at 7:30 PM, Linus Torvalds
> <torvalds@xxxxxxxxxxxxxxxxxxxx> wrote:
>> On Mon, Mar 7, 2016 at 10:07 AM, Alan Stern <stern@xxxxxxxxxxxxxxxxxxx> 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 -