Re: Litmus test for question from Al Viro

From: Jon Masters
Date: Fri Oct 02 2020 - 22:36:20 EST


On 10/1/20 12:15 PM, Alan Stern wrote:
On Wed, Sep 30, 2020 at 09:51:16PM -0700, Paul E. McKenney wrote:
Hello!

Al Viro posted the following query:

------------------------------------------------------------------------

<viro> fun question regarding barriers, if you have time for that
<viro> V->A = V->B = 1;
<viro>
<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; 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.

2) The smp_store_release in CPU2 is not needed. Replacing it with a
plain V->B = 0 will not break anything.

This was my interpretation also. I made the mistake of reading this right before trying to go to bed the other night and ended up tweeting at Paul that I'd regret it if he gave me scary dreams. Thought about it and read your write up and it is still exactly how I see it.

Jon.

--
Computer Architect