Messages in this thread | | | Date | Mon, 15 Aug 2016 13:28:39 -0700 | From | "Paul E. McKenney" <> | Subject | Re: spin_lock implicit/explicit memory barrier |
| |
On Mon, Aug 15, 2016 at 10:06:39PM +0200, Manfred Spraul wrote: > Hi Paul, > > On 08/10/2016 11:00 PM, Paul E. McKenney wrote: > >On Wed, Aug 10, 2016 at 12:17:57PM -0700, Davidlohr Bueso wrote: > >>[...] > >> CPU0 CPU1 > >> complex_mode = true spin_lock(l) > >> smp_mb() <--- do we want a smp_mb() here? > >> spin_unlock_wait(l) if (!smp_load_acquire(complex_mode)) > >> foo() foo() > >> > >>We should not be doing an smp_mb() right after a spin_lock(), makes no sense. The > >>spinlock machinery should guarantee us the barriers in the unorthodox locking cases, > >>such as this. > >In this case, from what I can see, we do need a store-load fence. > >That said, yes, it really should be smp_mb__after_unlock_lock() rather > >than smp_mb(). So if this code pattern is both desired and legitimate, > >the smp_mb__after_unlock_lock() definitions probably need to move out > >of kernel/rcu/tree.h to barrier.h or some such. > Can you explain the function name, why smp_mb__after_unlock_lock()?
When placed after a locking function, it provides full ordering for all the accesses within this critical section against all the accesses in the previous critical section for this lock. In addition, it provides full ordering for all accesses within this critical section against all previous critical sections for all locks acquired by this task/CPU.
In short, it acts on the prior lock in combination with some earlier unlock, hence the name.
> I would have called it smp_mb__after_spin_lock().
It works on mutexes as well as spinlocks, for whatever that is worth.
> For ipc/sem.c, the use case is: > [sorry, I only now notice that the mailer ate the formatting]: > > cpu 1: complex_mode_enter(): > smp_store_mb(sma->complex_mode, true); > > for (i = 0; i < sma->sem_nsems; i++) { > sem = sma->sem_base + i; > spin_unlock_wait(&sem->lock); > } > > cpu 2: sem_lock(): > spin_lock(&sem->lock); > smp_mb(); > if (!smp_load_acquire(&sma->complex_mode)) { > > > What is forbidden is that both cpu1 and cpu2 proceed.
It looks to me that CPU 2's smp_mb() could be an smp_mb__after_unlock_lock() in this case, although that does mean defining its relationship to spin_unlock_wait() in general. Use of smp_mb__after_unlock_lock() would get rid of a memory barrier on many architectures, while still guaranteeing full ordering. Probably not measurable at the system level, though.
Thanx, Paul
| |