Messages in this thread |  | | | Date | Thu, 27 Jan 2011 15:53:59 -0800 | | From | Jeremy Fitzhardinge <> | | Subject | Re: [PATCH 6/6] x86/ticketlock: make __ticket_spin_trylock common |
| |
On 01/24/2011 05:58 PM, Nick Piggin wrote: >> >> The C version can't take advantage of the fact that the cmpxchg directly >> sets the flags, so it ends up re-comparing the old and swapped-out >> values to set the return. And it doesn't re-use the same sete to set >> the return value in the quick failed-to-acquire path. > Hm.
Adding a "cmpxchg_flag" which does its own sete and returns a boolean "success" flag, the whole thing goes to:
__ticket_spin_trylock: movw (%rdi), %ax # lock_2(D)->D.5950.tickets, old xorl %edx, %edx # D.13949 movzbl %ah, %ecx # old, tmp69 cmpb %al, %cl # old, tmp69 jne .L5 #, leal 256(%rax), %edx #, D.13951 lock; cmpxchgw %dx,(%rdi); sete %al # D.13951,* lock, __ret movzbl %al, %edx # __ret, D.13949 .L5: movl %edx, %eax # D.13949, ret
The eax/edx shuffle is a bit unfortunate I can't see it hurting very much.
>> It might be worth having a generic cmpxchg() variant which returns a >> succeed/fail flag rather than the fetched value, to avoid comparison in >> this case - since many (most?) cmpxchg() callers end up doing that >> comparison. >> >> How performance critical is trylock? I guess the ones in fs/dcache.c >> are the ones looming large in your mind. > Well they are on on the reclaim/free path rather than the _hottest_ > paths, but yes they are performance critical.
I think that code looks pretty reasonable.
J
|  |