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

2011-02-15 Thread Jan Kiszka
On 2011-02-15 13:32, Marcelo Tosatti wrote: > On Tue, Feb 08, 2011 at 12:55:39PM +0100, Jan Kiszka wrote: >> Only for walking the list of VMs, we do not need to hold the preemption >> disabling kvm_lock. Convert stat services, the cpufreq callback and >> mmu_shrink to RCU. For the latter, special c

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

2011-02-15 Thread Marcelo Tosatti
On Tue, Feb 08, 2011 at 12:55:39PM +0100, Jan Kiszka wrote: > Only for walking the list of VMs, we do not need to hold the preemption > disabling kvm_lock. Convert stat services, the cpufreq callback and > mmu_shrink to RCU. For the latter, special care is required to > synchronize its list_move_ta

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

2011-02-10 Thread Jan Kiszka
On 2011-02-10 15:47, Avi Kivity wrote: > On 02/10/2011 04:34 PM, Jan Kiszka wrote: >> On 2011-02-10 15:26, Avi Kivity wrote: >>> On 02/10/2011 03:47 PM, Jan Kiszka wrote: >> >> Accept for mmu_shrink, which is write but not delete, thus works >> without >> that slow synchronize

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

2011-02-10 Thread Avi Kivity
On 02/10/2011 04:34 PM, Jan Kiszka wrote: On 2011-02-10 15:26, Avi Kivity wrote: > On 02/10/2011 03:47 PM, Jan Kiszka wrote: Accept for mmu_shrink, which is write but not delete, thus works without that slow synchronize_rcu. >>> >>> I don't really see how you can implement l

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

2011-02-10 Thread Jan Kiszka
On 2011-02-10 15:26, Avi Kivity wrote: > On 02/10/2011 03:47 PM, Jan Kiszka wrote: Accept for mmu_shrink, which is write but not delete, thus works without that slow synchronize_rcu. >>> >>> I don't really see how you can implement list_move_rcu(), it has to be >>> atomic or othe

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

2011-02-10 Thread Avi Kivity
On 02/10/2011 03:47 PM, Jan Kiszka wrote: >> >> Accept for mmu_shrink, which is write but not delete, thus works without >> that slow synchronize_rcu. > > I don't really see how you can implement list_move_rcu(), it has to be > atomic or other users will see a partial vm_list. Right, even if

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

2011-02-10 Thread Jan Kiszka
On 2011-02-10 14:19, Avi Kivity wrote: > On 02/10/2011 03:14 PM, Jan Kiszka wrote: >> On 2011-02-10 13:57, Avi Kivity wrote: >>> On 02/10/2011 02:56 PM, Avi Kivity wrote: > What's the benefit? The downside is a bit more complexity as you need an > additional callback handler. >

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

2011-02-10 Thread Avi Kivity
On 02/10/2011 03:14 PM, Jan Kiszka wrote: On 2011-02-10 13:57, Avi Kivity wrote: > On 02/10/2011 02:56 PM, Avi Kivity wrote: >>> 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 systemw

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

2011-02-10 Thread Jan Kiszka
On 2011-02-10 13:57, Avi Kivity wrote: > On 02/10/2011 02:56 PM, Avi Kivity wrote: >>> 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 call

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

2011-02-10 Thread Avi Kivity
On 02/10/2011 02:56 PM, Avi Kivity wrote: 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. In fact this just shows

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

2011-02-10 Thread Avi Kivity
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

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

2011-02-10 Thread Jan Kiszka
On 2011-02-10 13:34, Avi Kivity wrote: > On 02/10/2011 01:31 PM, Jan Kiszka wrote: >>> @@ -3607,10 +3607,14 @@ static int mmu_shrink(struct shrinker *shrink, int nr_to_scan, gfp_t gfp_mask) spin_unlock(&kvm->mmu_lock); srcu_read_unloc

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

2011-02-10 Thread Avi Kivity
On 02/10/2011 01:31 PM, Jan Kiszka wrote: > >> >> @@ -3607,10 +3607,14 @@ static int mmu_shrink(struct shrinker *shrink, int nr_to_scan, gfp_t gfp_mask) >>spin_unlock(&kvm->mmu_lock); >>srcu_read_unlock(&kvm->srcu, idx); >>} >> - if (kvm_freed) >> -

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

2011-02-10 Thread Jan Kiszka
On 2011-02-10 11:16, Avi Kivity wrote: > On 02/08/2011 01:55 PM, Jan Kiszka wrote: >> Only for walking the list of VMs, we do not need to hold the preemption >> disabling kvm_lock. Convert stat services, the cpufreq callback and >> mmu_shrink to RCU. For the latter, special care is required to >> s

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

2011-02-10 Thread Avi Kivity
On 02/08/2011 01:55 PM, Jan Kiszka wrote: Only for walking the list of VMs, we do not need to hold the preemption disabling kvm_lock. Convert stat services, the cpufreq callback and mmu_shrink to RCU. For the latter, special care is required to synchronize its list_move_tail with kvm_destroy_vm.