On Mon, Jan 21, 2019 at 01:29:11PM +0100, Dmitry Vyukov wrote: > On Mon, Jan 21, 2019 at 12:45 PM Andrea Parri > <andrea.pa...@amarulasolutions.com> wrote: > > > > On Mon, Jan 21, 2019 at 10:52:37AM +0100, Dmitry Vyukov wrote: > > > > [...] > > > > > > Am I missing something or refcount_dec_and_test does not in fact > > > > provide ACQUIRE ordering? > > > > > > > > +case 5) - decrement-based RMW ops that return a value > > > > +----------------------------------------------------- > > > > + > > > > +Function changes: > > > > + atomic_dec_and_test() --> refcount_dec_and_test() > > > > + atomic_sub_and_test() --> refcount_sub_and_test() > > > > + no atomic counterpart --> refcount_dec_if_one() > > > > + atomic_add_unless(&var, -1, 1) --> > > > > refcount_dec_not_one(&var) > > > > + > > > > +Memory ordering guarantees changes: > > > > + fully ordered --> RELEASE ordering + control dependency > > > > > > > > I think that's against the expected refcount guarantees. When I > > > > privatize an atomic_dec_and_test I would expect that not only stores, > > > > but also loads act on a quiescent object. But loads can hoist outside > > > > of the control dependency. > > > > > > > > Consider the following example, is it the case that the BUG_ON can > > > > still fire? > > > > Can't it fire in an SC world? (wrong example, or a Monday morning? ;D) > > I don't see how. Maybe there is a stupid off-by-one, but overall > that's the example I wanted to show. refcount is 2, each thread sets > own done flag, drops refcount, last thread checks done flag of the > other thread.
You're right: looking at the example again, I think that the BUG_ON() in your example can indeed trigger with a CTRL+RELEASE semantics (but _not with the fully-ordered semantics). I apologize for the confusion (it must have been _my Monday... ;-/). Andrea > > > > > > > struct X { > > > > refcount_t rc; // == 2 > > > > int done1, done2; // == 0 > > > > }; > > > > > > > > // thread 1: > > > > x->done1 = 1; > > > > if (refcount_dec_and_test(&x->rc)) > > > > BUG_ON(!x->done2); > > > > > > > > // thread 2: > > > > x->done2 = 1; > > > > if (refcount_dec_and_test(&x->rc)) > > > > BUG_ON(!x->done1);