Re: [PATCH v2] introduce atomic_pointer to fix a race condition in cancelable mcs spinlocks

[Linus Torvalds]

On Mon, Jun 2, 2014 at 3:08 PM, Paul E. McKenney
<paulmck@xxxxxxxxxxxxxxxxxx> 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
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/