Messages in this thread | | | Date | Mon, 21 Jan 2019 15:44:46 +0100 | From | Andrea Parri <> | Subject | Re: [PATCH] kcov: convert kcov.refcount to refcount_t |
| |
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.parri@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);
| |