Re: [Qemu-devel] [RFC PATCH 2/2] mem: prepare address_space listener rcu style

[Paolo Bonzini <pbonzini@redhat.com>]

Il 15/05/2013 03:29, liu ping fan ha scritto:
>>>> Pointers are quite expensive here.  With RCU we can fetch a consistent
>>>> root/table pair like this:
>>>>
>>>>     rcu_read_lock();
>>>>     do {
>>>>         pgtbl = d->cur_pgtbl;
>>>>         smp_rmb();
>>>>         root = d->cur_root;
>>>>
>>>>         /* RCU ensures that d->cur_pgtbl remains alive, thus it cannot
>>>>          * be recycled while this loop is running.  If
>>>>          * d->cur_pgtbl == pgtbl, the root is the right one for this
>>>>          * pgtable.
>>>>          */
>>>>         smp_rmb();
>>>>     } while (d->cur_pgtbl == pgtbl);
>>
>> Ouch, != of course.
>>
>>>>     ...
>>>>     rcu_read_unlock();
>>>>
>>> It seems to break the semantic of rcu_dereference() and rcu_assign().
>>
>> It doesn't.  In fact it is even stronger, I'm using a "full" rmb instead
>> of read_barrier_depends.
>>
> rcu_dereference()/rcu_assign() ensure the switch from prev to next
> version, based on atomic-ops.

rcu_dereference()/rcu_assign() are not magic, they are simply
read+read_barrier_depends and wmb+write.

> I think your method _does_ work based on
> read+check+barrier skill, but it is not the standard RCU method, and
> export some concept (barrier) outside RCU.

It is a standard method to load 2 words and ensure it is consistent.  If
you want to use rcu_dereference(&d->cur_pgtbl) and
rcu_dereference(&d->cur_root), that's fine.  But you still need the read
barrier.

>>> If pointers are expensive, how about this:
>>> if (unlikely(d->prev_map!=d->cur_map)) {
>>>     d->root = d->cur_map->root;
>>>     d->pgtbl = d->cur_map->root;
>>>     d->prev_map = d->cur_map;
>>> }
>>> So usually, we use cache value.
>>
> rcu_read_lock();
> map = rcu_derefenrence(d->cur_map)
> if (unlikely(d->prev_map!=map) {
>     d->root = map->root;
>     d->pgtbl = map->pgtbl;
> }
> ......
> rcu_read_unlock();
> 
> Then it can avoid ABA problem.

I don't see the assignment of prev_map, which is where the ABA problem
arises.

Paolo