On Mon, Jul 02, 2018 at 01:30:14AM +0800, Guo Ren wrote:
> +static inline void arch_spin_lock(arch_spinlock_t *lock)
> +{
> + unsigned int *p = &lock->lock;
> + unsigned int tmp;
> +
> + asm volatile (
> + "1: ldex.w %0, (%1) \n"
> + " bnez %0, 1b \n"
> + " movi %0, 1 \n"
> + " stex.w %0, (%1) \n"
> + " bez %0, 1b \n"
> + : "=&r" (tmp)
> + : "r"(p)
> + : "memory");
> + smp_mb();
> +}
Test-and-set with MB acting as ACQUIRE, ok.
> +static inline void arch_spin_unlock(arch_spinlock_t *lock)
> +{
> + unsigned int *p = &lock->lock;
> + unsigned int tmp;
> +
> + smp_mb();
> + asm volatile (
> + "1: ldex.w %0, (%1) \n"
> + " movi %0, 0 \n"
> + " stex.w %0, (%1) \n"
> + " bez %0, 1b \n"
> + : "=&r" (tmp)
> + : "r"(p)
> + : "memory");
> +}
MB acting for RELEASE, but _why_ are you using a LDEX/STEX to clear the
lock word? Would not a normal store work?
Also, the fact that you need MB for release implies your LDEX does not
in fact imply anything and your xchg/cmpxchg implementation is broken.
> +static inline int arch_spin_trylock(arch_spinlock_t *lock)
> +{
> + unsigned int *p = &lock->lock;
> + unsigned int tmp;
> +
> + asm volatile (
> + "1: ldex.w %0, (%1) \n"
> + " bnez %0, 2f \n"
> + " movi %0, 1 \n"
> + " stex.w %0, (%1) \n"
> + " bez %0, 1b \n"
> + " movi %0, 0 \n"
> + "2: \n"
> + : "=&r" (tmp)
> + : "r"(p)
> + : "memory");
> + smp_mb();
> +
> + return !tmp;
> +}
Strictly speaking you can avoid the MB on failure. You only need to
provide ACQUIRE semantics on success.
That said, I would really suggest you implement a ticket lock instead of
a test-and-set lock. They're not really all that complicated and do
provide better worst case behaviour.
> +/****** read lock/unlock/trylock ******/
Please have a look at using qrwlock -- esp. if you implement a ticket
lock, then the rwlock comes for 'free'.
