Re: [tip:core/locking] locking, x86: Slightly shorten __ticket_spin_trylock()

From: Jan Beulich
Date: Wed Dec 02 2009 - 09:57:53 EST


>>> Ingo Molnar <mingo@xxxxxxx> 02.12.09 15:21 >>>
792a99a9 <_raw_spin_lock>:
...
>792a99f3: 89 d8 mov %ebx,%eax
>792a99f5: ff 15 d0 6c f2 79 call *0x79f26cd0
>792a99fb: 85 c0 test %eax,%eax
...
>792a9a2e: 89 f8 mov %edi,%eax
>792a9a30: ff 15 d0 6c f2 79 call *0x79f26cd0
>792a9a36: 85 c0 test %eax,%eax

Assuming that these are the calls to __raw_spin_trylock, it is clear that
the generated code isn't what we want: It should be test %al, %al in
both cases.

This isn't a compiler bug, though: ____PVOP_CALL() doesn't properly
deal with rettype being bool. Other than all integer types, casting
to bool isn't just a truncation (or extension), but is an actual
conversion. With the compiler seeing that __eax is declared as
unsigned long, it can optimize the whole operation and avoid the
conversion to bool (and hence test the 32-bit register directly).

While I would think these macros should be fixed accordingly (as it
would be quite easy for others to run into the same trap), for the
case at hand I'm not too certain it would be worthwhile as I believe
the goal is to eliminate at least pv-ops spinlocks anyway due to
their overhead on native (and I have a replacement basically
ready, just that for now it only aims at fully virtualized Xen guests).

In any case, please drop the patch until either paravirt.h gets
fixed (I'll try to find time to look into this), or pv-ops spinlocks gets
eliminated. And I'm sorry for not noticing this before submission.

Jan

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/