2013/3/15 Oleg Nesterov <o...@redhat.com>:
> On 03/15, Ming Lei wrote:
>>
>> On Fri, Mar 15, 2013 at 9:46 PM, Oleg Nesterov <o...@redhat.com> wrote:
>> > On 03/15, Ming Lei wrote:
>> >>
>> >> On Fri, Mar 15, 2013 at 12:24 AM, Oleg Nesterov <o...@redhat.com> wrote:
>> >> > static inline int atomic_inc_unless_negative(atomic_t *p)
>> >> > {
>> >> > int v, v1;
>> >> > - for (v = 0; v >= 0; v = v1) {
>> >> > + for (v = atomic_read(p); v >= 0; v = v1) {
>> >> > v1 = atomic_cmpxchg(p, v, v + 1);
>> >>
>> >> Unfortunately, the above will exchange the current value even though
>> >> it is negative, so it isn't correct.
>> >
>> > Hmm, why? We always check "v >= 0" before we try to do
>> > atomic_cmpxchg(old => v) ?
>>
>> Sorry, yes, you are right. But then your patch is basically same with the
>> previous one, isn't it?
>
> Sure, the logic is the same, just the patch (and the code) looks simpler
> and more understandable.
>
>> And has same problem, see below discussion:
>>
>> http://marc.info/?t=136284366900001&r=1&w=2
>
> 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);
if (try_get_object(obj))
do_something...
else
object is dying
rcu_read_unlock()
But anyway I must defer on Paul, he's the specialist here.
--
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/
