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 -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
