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?