Messages in this thread | | | Date | Fri, 7 Feb 2014 18:19:00 +0100 | From | Peter Zijlstra <> | Subject | Re: [PATCH] Convert powerpc simple spinlocks into ticket locks |
| |
On Fri, Feb 07, 2014 at 06:08:45PM +0100, Torsten Duwe wrote: > > static inline unsigned int xadd(unsigned int *v, unsigned int i) > > { > > int t, ret; > > > > __asm__ __volatile__ ( > > "1: lwarx %0, 0, %4\n" > > " mr %1, %0\n" > > " add %0, %3, %0\n" > > " stwcx. %0, %0, %4\n" > > " bne- 1b\n" > > : "=&r" (t), "=&r" (ret), "+m" (*v) > > : "r" (i), "r" (v) > > : "cc"); > > > > return ret; > > } > > > I don't like this xadd thing -- it's so x86 ;) > x86 has its LOCK prefix, ppc has ll/sc. > That should be reflected somehow IMHO.
Its the operational semantics I care about; this version is actually nicer in that it doesn't actually imply all sorts of barriers :-)
> Maybe if xadd became mandatory for some kernel library.
call it fetch_add() its not an uncommon operation and many people understand the semantics.
But you can simply include the asm bits in ticket_lock() and be done with it. In that case you can also replace the add with an addi which might be a little more efficient.
> > void ticket_unlock(tickets_t *lock) > > { > > ticket_t tail = lock->tail + 1; > > > > /* > > * The store is save against the xadd for it will make the ll/sc fail > > * and try again. Aside from that PowerISA guarantees single-copy > > * atomicy for half-word writes. > > * > > * And since only the lock owner will ever write the tail, we're good. > > */ > > smp_store_release(&lock->tail, tail); > > } > > Yeah, let's try that on top of v2 (just posted). > First, I want to see v2 work as nicely as v1 -- > compiling a debug kernel takes a while...
Use a faster machine... it can be done < 1 minute :-)
| |