lkml.org 
[lkml]   [2009]   [Dec]   [2]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [tip:core/locking] locking, x86: Slightly shorten __ticket_spin_trylock()
>>> Ingo Molnar <mingo@elte.hu> 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



\
 
 \ /
  Last update: 2009-12-02 16:01    [W:0.057 / U:0.704 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site