Hi,

I it looks like the example in the Documentation/atomic_ops.txt
file at line 232 is not quite right. The obj->active = 0 will
be delayed, but not further than spin_unlock() in obj_timeout().
Becaus spin_unlock() has a memory barrier.

I guess you would need to move spin_lock(&global_list_lock) to
obj_list_del() to make the example valid.

This confused me when I read the file.

-- 
Best Regards,
Artem Bityutskiy (Артём Битюцкий)
--

[ message continues ]

David,

do you have any comments on this? I paste the example below for
convenience.


static void obj_list_add(struct obj *obj)
{
        obj->active = 1;
        list_add(&obj->list);
}

static void obj_list_del(struct obj *obj)
{
        list_del(&obj->list);
        obj->active = 0;
}

static void obj_destroy(struct obj *obj)
{
        BUG_ON(obj->active);
        kfree(obj);
}

struct obj *obj_list_peek(struct list_head *head)
{
        if (!list_empty(head)) {
                struct obj *obj;

                obj = list_entry(head->next, struct obj, list);
                atomic_inc(&obj->refcnt);
                return obj;
        }
        return NULL;
}

void obj_poke(void)
{
        struct obj *obj;

        spin_lock(&global_list_lock);
        obj = obj_list_peek(&global_list);
        spin_unlock(&global_list_lock);

        if (obj) {
                obj->ops->poke(obj);
                if (atomic_dec_and_test(&obj->refcnt))
                        obj_destroy(obj);
        }
}

void obj_timeout(struct obj *obj)
{
        spin_lock(&global_list_lock);
        obj_list_del(obj);
        spin_unlock(&global_list_lock);

        if (atomic_dec_and_test(&obj->refcnt))
                obj_destroy(obj);
}

(This is a simplification of the ARP queue management in the
 generic neighbour discover code of the networking.  Olaf Kirch
 found a bug wrt. memory barriers in kfree_skb() that exposed
 the atomic_t memory barrier requirements quite clearly.)

Given the above scheme, it must be the case that the obj->active
update done by the obj list deletion be visible to other processors
before the atomic counter decrement is performed.

Otherwise, the counter could fall to zero, yet obj->active would still
be set, thus triggering the assertion in obj_destroy().  The error
sequence looks like this:

        cpu 0                           cpu 1
        obj_poke()                      obj_timeout()
        obj = obj_list_peek();
       ...
[ message continues ]