Messages in this thread | | | Date | Mon, 2 Jun 2014 15:55:57 -0700 | Subject | Re: [PATCH v2] introduce atomic_pointer to fix a race condition in cancelable mcs spinlocks | From | Linus Torvalds <> |
| |
On Mon, Jun 2, 2014 at 3:08 PM, Paul E. McKenney <paulmck@linux.vnet.ibm.com> wrote: > > rcu: Eliminate read-modify-write ACCESS_ONCE() calls > > preempt_disable(); > - ACCESS_ONCE(this_cpu_ptr(sp->per_cpu_ref)->c[idx]) += 1; > + lp = this_cpu_ptr(&sp->per_cpu_ref->c[idx]); > + ACCESS_ONCE(*lp) = *lp + 1; > smp_mb(); /* B */ /* Avoid leaking the critical section. */ > - ACCESS_ONCE(this_cpu_ptr(sp->per_cpu_ref)->seq[idx]) += 1; > + lp = this_cpu_ptr(&sp->per_cpu_ref->seq[idx]); > + ACCESS_ONCE(*lp) = *lp + 1; > preempt_enable(); > return idx;
What Eric said. This should just use "this_cpu_inc()" instead. Particularly with the smp_mb() and the preempt_enable(), there's no way that could/should leak, and the ACCESS_ONCE() seems pointless and ugly.
And the good news is, gcc _will_ generate good code for that.
Linus
| |