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

From: Linus Torvalds
Date: Mon Mar 07 2016 - 13:30:30 EST


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.

Linus