On Thu, Oct 01, 2020 at 12:15:29PM -0400, Alan Stern wrote:
> > <viro> CPU1:
> > <viro> to_free = NULL
> > <viro> spin_lock(&LOCK)
> > <viro> if (!smp_load_acquire(&V->B))
> > <viro> to_free = V
> > <viro> V->A = 0
> > <viro> spin_unlock(&LOCK)
> > <viro> kfree(to_free)
> > <viro>
> > <viro> CPU2:
> > <viro> to_free = V;
> > <viro> if (READ_ONCE(V->A)) {
> > <viro> spin_lock(&LOCK)
> > <viro> if (V->A)
> > <viro> to_free = NULL
> > <viro> smp_store_release(&V->B, 0);
> > <viro> spin_unlock(&LOCK)
> > <viro> }
> > <viro> kfree(to_free);
> > <viro> 1) is it guaranteed that V will be freed exactly once and that
> > no accesses to *V will happen after freeing it?
> > <viro> 2) do we need smp_store_release() there? I.e. will anything
> > break if it's replaced with plain V->B = 0?
>
> Here are my answers to Al's questions:
>
> 1) It is guaranteed that V will be freed exactly once. It is not
> guaranteed that no accesses to *V will occur after it is freed, because
> the test contains a data race. CPU1's plain "V->A = 0" write races with
> CPU2's READ_ONCE;
What will that READ_ONCE() yield in that case? If it's non-zero, we should
be fine - we won't get to kfree() until after we are done with the spinlock.
And if it's zero... What will CPU1 do with *V accesses _after_ it has issued
the store to V->A?
Confused...
> if the plain write were replaced with
> "WRITE_ONCE(V->A, 0)" then the guarantee would hold. Equally well,
> CPU1's smp_load_acquire could be replaced with a plain read while the
> plain write is replaced with smp_store_release.
Er... Do you mean the write to ->A on CPU1?
