2013/3/15 Oleg Nesterov <o...@redhat.com>: > On 03/15, Frederic Weisbecker wrote: >> >> > The lack of the barrier? >> > >> > I thought about this, this should be fine? atomic_add_unless() has the same >> > "problem", but this is documented in atomic_ops.txt: >> > >> > atomic_add_unless requires explicit memory barriers around the >> > operation >> > unless it fails (returns 0). >> > >> > I thought that atomic_add_unless_negative() should have the same >> > guarantees? >> >> I feel very uncomfortable with that. The memory barrier is needed >> anyway to make sure we don't deal with a stale value of the atomic val >> (wrt. ordering against another object). >> The following should really be expected to work without added barrier: >> >> void put_object(foo *obj) >> { >> if (atomic_dec_return(obj->ref) == -1) >> free_rcu(obj); >> } >> >> bool try_get_object(foo *obj) >> { >> if (atomic_add_unless_negative(obj, 1)) >> return true; >> return false; >> } >> >> = CPU 0 = = CPU 1 >> rcu_read_lock() >> put_object(obj0); >> obj = rcu_derefr(obj0); >> rcu_assign_ptr(obj0, NULL); > > (I guess you meant rcu_assign_ptr() then put_object())
Right. > >> if (try_get_object(obj)) >> do_something... >> else >> object is dying >> rcu_read_unlock() > > I must have missed something. > > do_something() looks fine, if atomic_add_unless_negative() succeeds > we do have a barrier? Ok, I guess the guarantee of a barrier in case of failure is probably not needed. But since the only way to safely read the atomic value is a cmpxchg like operation, I guess a barrier must be involved in any case. Using atomic_read() may return some stale value. > > Anyway, I understand that it is possible to write the code which > won't work without the uncoditional mb(). Yeah that's my fear. > > My point was: should we fix atomic_add_unless() then? If not, why > should atomic_add_unless_negative() differ? They shouldn't differ I guess. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/