On Sun, Mar 13, 2022 at 10:27 AM Visa Hankala <[email protected]> wrote:
> On Sun, Mar 13, 2022 at 04:29:44PM +0100, Mark Kettenis wrote:
>
...
> > Under what circumstances does memory ordering matter for these
> > interfaces?
>
> Consider the following scenario:
>
> struct widget {
> struct refcnt w_refcnt;
> /* more fields spanning many cache lines */
> ...
> int w_var;
> };
>
> First, CPU 1 executes:
>
> w->w_var = 1;
> refcnt_rele(&w->w_refcnt); /* remains above zero */
>
Having incremented the refcnt previous does not give this thread exclusive
access to 'w', so if it's writing to w->w_var then it must either
a) have some sort of write lock taken, which it will release after this and
which will contain the necessary member, OR
b) have the only access patch to this structure (i.e, it's not yet
'published' into structures which can be seen by other threads), in which
case the operations which do that 'publishing' of the access to 'w' (adding
it to a global list, etc) must include the necessary membar.
Next, CPU 2 executes:
>
> if (refcnt_rele(&w->w_refcnt)) /* refcnt drops to zero */
> free(w);
>
How did CPU 2 get what is now exclusive access to 'w' without any membars?
If that's possible then it was just accessing 'w' and possibly not seeing
the update to w->w_var even _before_ the refcnt_rele(), so putting a membar
in refcnt_rele() hides the incorrect code by suppressing the later crash!
If these membars appear to help then the code is and remains broken. This
change should not be done.
Philip Guenther
