On Sun, Aug 28, 2016 at 01:56:15PM +0200, Manfred Spraul wrote: > +++ b/net/netfilter/nf_conntrack_core.c > @@ -80,20 +80,29 @@ static __read_mostly bool nf_conntrack_locks_all; > > void nf_conntrack_lock(spinlock_t *lock) __acquires(lock) > { > + /* Step 1: Acquire the lock */ > spin_lock(lock); > - while (unlikely(nf_conntrack_locks_all)) { > - spin_unlock(lock); > > - /* > - * Order the 'nf_conntrack_locks_all' load vs. the > - * spin_unlock_wait() loads below, to ensure > - * that 'nf_conntrack_locks_all_lock' is indeed held: > - */ > - smp_rmb(); /* spin_lock(&nf_conntrack_locks_all_lock) */ > - spin_unlock_wait(&nf_conntrack_locks_all_lock); > - spin_lock(lock); > - } > + /* Step 2: make it visible to all CPUs that we hold the lock */ > + smp_mb__after_spin_lock();
I hate this comment. A barrier does _not_ make visible anything. A barrier _orders_ things.