generic rwsem [Re: Alpha "process table hang"]
Quoted 3 lines
I am sure ppc couldn't race (at least unless up_read/up_write were excuted from irq/softnet context and that never happens in 2.4.4pre3, see below ;).
This is not actually using the rwsem code I wrote at the moment.
Quoted 5 lines
And incidentally the above is what (I guess Richard) did on the alpha and that should really go into common code instead of having
asm-i386/rwsem-xadd.hasm-alpha/rwsem.hetcc.etc... just implementatomic_inc_returnusing xadd inasm-i386/atomic.h, that's much better design IMHO.
I disagree... you want such primitives to be as efficient as possible. The whole point of having asm/xxxx.h files is that you can stuff them full of dirty tricks specific to certain architectures.
Quoted 6 lines
That can obviously be done for example with C code like this:
count = atomic_inc_return(&sem->count); if (__builtin_expect(count == 0, 0)) slow_path()The above is the perfect C implementation IMHO
But not so efficient since it _has_ to take a jump unless the compiler can emit code in alternative text sections when it seems appropriate. Plus, you have to test count's value. XADD sets EFLAGS based on the result in memory, something that allows all but one fastpath to be two instructions in length (the one that isn't is three).
But, yes, there should probably be two generic cases: one implemented with a spinlock in the rwsem struct (as I supply) and one implemented using atomic_add_return(). Note, however! atomic_add_return() is not necessarily implemented efficiently: if it involves a cmpxchg loop (as many seem to), then that is really quite inefficient, and may be better done as a spinlock anyway.
I've had a look at your implementation... It seems to hold the spinlocks for an awfully long time... specifically around the local variable initialisation in the 'failed' functions. Don't forget that the compiler can't reorder these because they're inside the spinlock. I would, if I were you, fold the 'failed' functions into the main ones to avoid this problem.
Your rw_semaphore structure is also rather large: 46 bytes without debugging stuff (16 bytes apiece for the waitqueues and 12 bytes for the rest). Contrast that with mine: generic is 24 bytes and the i386-xadd optimised is 20.
Admittedly, though, yours is extremely simple and easy to follow, but I don't think it's going to be very fast.
Of course, I still prefer mine... smaller, faster, more efficient:-)
David
Quoted 3 lines · D . W . Howells
I disagree... you want such primitives to be as efficient as possible. The whole point of having
asm/xxxx.hfiles is that you can stuff them full of dirty tricks specific to certain architectures.
Of course you always have the option to override completly and you should on x86 (providing an API for total override is the main object of my patch).
Quoted 2 lines
I've had a look at your implementation... It seems to hold the spinlocks for an awfully long time... specifically around the local variable initialisation
My point for not unlocking is that unlocking and locking back another spinlock for the waitqueue and using the wait_even interface for serializing the slow path is expensive and generates more cacheline ping pong between cpus. And quite frankly I don't care about the scalability of the slow path so if the slow path is simpler and slower I'm happy with it.
Quoted 1 line
Your
rw_semaphorestructure is also rather large: 46 bytes without debugging
It is 36bytes. and on 64bit archs the difference is going to be less.
Quoted 1 line
stuff (16 bytes apiece for the waitqueues and 12 bytes for the rest).
The real waste is the lock of the waitqueue that I don't need, so I should probably keep two list_head in the waitqueue instead of using the wait_queue_head_t and wake_up_process by hand.
Quoted 2 lines
Admittedly, though, yours is extremely simple and easy to follow, but I don't think it's going to be very fast.
The fast path has to be as fast as yours, if not then the only variable that can make difference is the fact I'm not inlining the fast path because it's not that small, in such a case I should simply inline the fast path, I don't care about the scalability of the slow path and I think the slow path may even be faster than yours because I don't run additional unlock/lock and memory barriers and the other cpus will stop dirtifying my stuff after their first trylock until I unlock.
If you have time to benchmark I'd be interested to see some number. But anyways my implementation was mostly meant to be obviously right and possible to ovverride with per-arch algorithms.
Andrea