Re: [PATCH 2/2] KVM: Convert read-only users of vm_list to RCU

[Avi Kivity <avi@redhat.com>]

On 02/10/2011 02:45 PM, Jan Kiszka wrote:
> >>>
> >>>   There is no list_move_tail_rcu().
> >>
> >>  ...specifically not for this one.
> >
> >  Well, we can add one if needed (and if possible).
>
> I can have a look, at least at the lower hanging fruits.

Please keep rcu->parent in the loop.

> >
> >>>
> >>>   Why check kvm->deleted?  it's in the process of being torn down anyway,
> >>>   it doesn't matter if mmu_shrink or kvm_destroy_vm pulls the trigger.
> >>
> >>  kvm_destroy_vm removes a vm from the list while mmu_shrink is running.
> >>  Then mmu_shrink's list_move_tail will re-add that vm to the list tail
> >>  again (unless already the removal in move_tail produces a crash).
> >
> >  It's too subtle.  Communication across threads with a variable needs
> >  memory barriers (even though they're nops on x86) and documentation.
>
> The barriers are provided by this spin lock we acquire for testing are
> modifying deleted.

Right.

I'm not thrilled with adding ->deleted though.

> >
> >  btw, not even sure if it's legal: you have a mutating call within an rcu
> >  read critical section for the same object.  If synchronize_rcu() were
> >  called there, would it ever terminate?
>
> Why not? kvm_destroy_vm is not preventing blocking mmu_shrink to acquire
> the kvm_lock where we then find the vm deleted and release both kvm_lock
> and the rcu read "lock" afterwards.

synchronize_rcu() waits until all currently running rcu read-side 
critical sections are completed.  But we are in the middle of one, which 
isn't going to complete until it synchronize_rcu() returns.

> >
> >  (not that synchronize_rcu() is a good thing there, better do it with
> >  call_rcu()).
>
> What's the benefit? The downside is a bit more complexity as you need an
> additional callback handler.

synchronize_rcu() can be very slow (its a systemwide operation), and 
mmu_shrink() can be called often on a loaded system.

-- 
error compiling committee.c: too many arguments to function