On Fri, Aug 30, 2013 at 12:38 PM, Paolo Bonzini <
address@hidden> wrote:
>
> > @@ -867,7 +879,12 @@ static int ram_load(QEMUFile *f, void *opaque, int version_id)
> > if (version_id < 4 || version_id > 4) {
> > return -EINVAL;
> > }
> > -
> > + /* this implements a long-running RCU critical section.
> > + * When rcu reclaims in the code start to become numerous
> > + * it will be necessary to reduce the granularity of this critical
> > + * section.
> > + */
>
> Please add the same comment (and a rcu_read_lock/unlock pair replacing
> the ramlist mutex) in ram_save_iterate, too.
Just double checking on this particular change. In practice ram_save manipulates the ram_list indirectly through ram_save_block. But I'm assuming you want this change because of the ram state info that persists between calls to ram_save (ram_list version in particular). Also, there is potential for the callback functions ram_control_*_iterate to manipulate the ram_list.
I'm adding the rcu_read_lock/unlock pair in ram_load. It will be recursive with the same calls in ram_save_block, but as you pointed out this is low overhead.
With this change in my working code, ram_control_*_iterate are called from within an rcu critical section.
Mike