[PATCH v3 1/3] KVM: fix rcu usage warning in kvm_arch_vcpu_ioctl_set_sregs()

[PATCH v3 1/3] KVM: fix rcu usage warning in kvm_arch_vcpu_ioctl_set_sregs()

[Xiao Guangrong]

Fix:

[ 1001.499596] ===================================================
[ 1001.499599] [ INFO: suspicious rcu_dereference_check() usage. ]
[ 1001.499601] ---------------------------------------------------
[ 1001.499604] include/linux/kvm_host.h:301 invoked rcu_dereference_check() without protection!
	......
[ 1001.499636] Pid: 6035, comm: qemu-system-x86 Not tainted 2.6.37-rc6+ #62
[ 1001.499638] Call Trace:
[ 1001.499644]  [] lockdep_rcu_dereference+0x9d/0xa5
[ 1001.499653]  [] gfn_to_memslot+0x8d/0xc8 [kvm]
[ 1001.499661]  [] gfn_to_hva+0x16/0x3f [kvm]
[ 1001.499669]  [] kvm_read_guest_page+0x1e/0x5e [kvm]
[ 1001.499681]  [] kvm_read_guest_page_mmu+0x53/0x5e [kvm]
[ 1001.499699]  [] load_pdptrs+0x3f/0x9c [kvm]
[ 1001.499705]  [] ? vmx_set_cr0+0x507/0x517 [kvm_intel]
[ 1001.499717]  [] kvm_arch_vcpu_ioctl_set_sregs+0x1f3/0x3c0 [kvm]
[ 1001.499727]  [] kvm_vcpu_ioctl+0x6a5/0xbc5 [kvm]

Signed-off-by: Xiao Guangrong <xiaoguangrong@cn.fujitsu.com>
---
 arch/x86/kvm/x86.c |    5 ++++-
 1 files changed, 4 insertions(+), 1 deletions(-)

diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index fa708c9..0b77e9c 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -5571,7 +5571,7 @@ int kvm_arch_vcpu_ioctl_set_sregs(struct kvm_vcpu *vcpu,
 				  struct kvm_sregs *sregs)
 {
 	int mmu_reset_needed = 0;
-	int pending_vec, max_bits;
+	int pending_vec, max_bits, idx;
 	struct desc_ptr dt;
 
 	dt.size = sregs->idt.limit;
@@ -5600,10 +5600,13 @@ int kvm_arch_vcpu_ioctl_set_sregs(struct kvm_vcpu *vcpu,
 	kvm_x86_ops->set_cr4(vcpu, sregs->cr4);
 	if (sregs->cr4 & X86_CR4_OSXSAVE)
 		update_cpuid(vcpu);
+
+	idx = srcu_read_lock(&vcpu->kvm->srcu);
 	if (!is_long_mode(vcpu) && is_pae(vcpu)) {
 		load_pdptrs(vcpu, vcpu->arch.walk_mmu, kvm_read_cr3(vcpu));
 		mmu_reset_needed = 1;
 	}
+	srcu_read_unlock(&vcpu->kvm->srcu, idx);
 
 	if (mmu_reset_needed)
 		kvm_mmu_reset_context(vcpu);
-- 
1.7.3.4

[PATCH v3 2/3] KVM: send IPI to vcpu only when it's in guest mode

[Xiao Guangrong]

We can interrupt the vcpu only when it's running in guest mode
to reduce IPI

Also
1: No need atomically to read/write ->mode in vcpu's thread

2: reorganize struct kvm_vcpu to make ->mode and ->requests
   in the same cache line explicitly

Signed-off-by: Xiao Guangrong <xiaoguangrong@cn.fujitsu.com>
---
 arch/ia64/kvm/kvm-ia64.c |    2 ++
 arch/x86/kvm/x86.c       |   16 ++++++++++------
 include/linux/kvm_host.h |   22 +++++++++++++++++-----
 virt/kvm/kvm_main.c      |    7 ++++++-
 4 files changed, 35 insertions(+), 12 deletions(-)

diff --git a/arch/ia64/kvm/kvm-ia64.c b/arch/ia64/kvm/kvm-ia64.c
index 70d224d..8213efe 100644
--- a/arch/ia64/kvm/kvm-ia64.c
+++ b/arch/ia64/kvm/kvm-ia64.c
@@ -662,6 +662,7 @@ again:
 		goto vcpu_run_fail;
 
 	srcu_read_unlock(&vcpu->kvm->srcu, idx);
+	vcpu->mode = IN_GUEST_MODE;
 	kvm_guest_enter();
 
 	/*
@@ -683,6 +684,7 @@ again:
 	 */
 	barrier();
 	kvm_guest_exit();
+	vcpu->mode = OUTSIDE_GUEST_MODE;
 	preempt_enable();
 
 	idx = srcu_read_lock(&vcpu->kvm->srcu);
diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index 0b77e9c..434363e 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -5213,14 +5213,18 @@ static int vcpu_enter_guest(struct kvm_vcpu *vcpu)
 		kvm_load_guest_fpu(vcpu);
 	kvm_load_guest_xcr0(vcpu);
 
-	atomic_set(&vcpu->guest_mode, 1);
-	smp_wmb();
+	vcpu->mode = IN_GUEST_MODE;
+
+	/* We should set ->mode before check ->requests,
+	 * see the comment in make_all_cpus_request.
+	 */
+	smp_mb();
 
 	local_irq_disable();
 
-	if (!atomic_read(&vcpu->guest_mode) || vcpu->requests
+	if (vcpu->mode == EXITING_GUEST_MODE || vcpu->requests
 	    || need_resched() || signal_pending(current)) {
-		atomic_set(&vcpu->guest_mode, 0);
+		vcpu->mode = OUTSIDE_GUEST_MODE;
 		smp_wmb();
 		local_irq_enable();
 		preempt_enable();
@@ -5256,7 +5260,7 @@ static int vcpu_enter_guest(struct kvm_vcpu *vcpu)
 
 	kvm_get_msr(vcpu, MSR_IA32_TSC, &vcpu->arch.last_guest_tsc);
 
-	atomic_set(&vcpu->guest_mode, 0);
+	vcpu->mode = OUTSIDE_GUEST_MODE;
 	smp_wmb();
 	local_irq_enable();
 
@@ -6157,7 +6161,7 @@ void kvm_vcpu_kick(struct kvm_vcpu *vcpu)
 
 	me = get_cpu();
 	if (cpu != me && (unsigned)cpu < nr_cpu_ids && cpu_online(cpu))
-		if (atomic_xchg(&vcpu->guest_mode, 0))
+		if (kvm_vcpu_exiting_guest_mode(vcpu) == IN_GUEST_MODE)
 			smp_send_reschedule(cpu);
 	put_cpu();
 }
diff --git a/include/linux/kvm_host.h b/include/linux/kvm_host.h
index b5021db..b99eacd 100644
--- a/include/linux/kvm_host.h
+++ b/include/linux/kvm_host.h
@@ -98,19 +98,26 @@ int kvm_setup_async_pf(struct kvm_vcpu *vcpu, gva_t gva, gfn_t gfn,
 int kvm_async_pf_wakeup_all(struct kvm_vcpu *vcpu);
 #endif
 
+enum {
+	OUTSIDE_GUEST_MODE,
+	IN_GUEST_MODE,
+	EXITING_GUEST_MODE
+};
+
 struct kvm_vcpu {
 	struct kvm *kvm;
 #ifdef CONFIG_PREEMPT_NOTIFIERS
 	struct preempt_notifier preempt_notifier;
 #endif
+	int cpu;
 	int vcpu_id;
-	struct mutex mutex;
-	int   cpu;
-	atomic_t guest_mode;
-	struct kvm_run *run;
+	int srcu_idx;
+	int mode;
 	unsigned long requests;
 	unsigned long guest_debug;
-	int srcu_idx;
+
+	struct mutex mutex;
+	struct kvm_run *run;
 
 	int fpu_active;
 	int guest_fpu_loaded, guest_xcr0_loaded;
@@ -140,6 +147,11 @@ struct kvm_vcpu {
 	struct kvm_vcpu_arch arch;
 };
 
+static inline int kvm_vcpu_exiting_guest_mode(struct kvm_vcpu *vcpu)
+{
+	return cmpxchg(&vcpu->mode, IN_GUEST_MODE, EXITING_GUEST_MODE);
+}
+
 /*
  * Some of the bitops functions do not support too long bitmaps.
  * This number must be determined not to exceed such limits.
diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
index b1b6cbb..71a4eaf 100644
--- a/virt/kvm/kvm_main.c
+++ b/virt/kvm/kvm_main.c
@@ -153,7 +153,12 @@ static bool make_all_cpus_request(struct kvm *kvm, unsigned int req)
 		if (kvm_make_check_request(req, vcpu))
 			continue;
 		cpu = vcpu->cpu;
-		if (cpus != NULL && cpu != -1 && cpu != me)
+
+		/* Set ->requests bit before we read ->mode */
+		smp_mb();
+
+		if (cpus != NULL && cpu != -1 && cpu != me &&
+		      kvm_vcpu_exiting_guest_mode(vcpu) != OUTSIDE_GUEST_MODE)
 			cpumask_set_cpu(cpu, cpus);
 	}
 	if (unlikely(cpus == NULL))
-- 
1.7.3.4

[PATCH v3 3/3] KVM: make make_all_cpus_request() lockless

[Xiao Guangrong]

Now, we have 'vcpu->mode' to judge whether need to send ipi to other
cpus, this way is very exact, so checking request bit is needless,
then we can drop the spinlock let it's collateral

Signed-off-by: Xiao Guangrong <xiaoguangrong@cn.fujitsu.com>
---
 include/linux/kvm_host.h |    6 ------
 virt/kvm/kvm_main.c      |    9 +++------
 2 files changed, 3 insertions(+), 12 deletions(-)

diff --git a/include/linux/kvm_host.h b/include/linux/kvm_host.h
index b99eacd..c8dee22 100644
--- a/include/linux/kvm_host.h
+++ b/include/linux/kvm_host.h
@@ -224,7 +224,6 @@ struct kvm_memslots {
 
 struct kvm {
 	spinlock_t mmu_lock;
-	raw_spinlock_t requests_lock;
 	struct mutex slots_lock;
 	struct mm_struct *mm; /* userspace tied to this vm */
 	struct kvm_memslots *memslots;
@@ -731,11 +730,6 @@ static inline void kvm_make_request(int req, struct kvm_vcpu *vcpu)
 	set_bit(req, &vcpu->requests);
 }
 
-static inline bool kvm_make_check_request(int req, struct kvm_vcpu *vcpu)
-{
-	return test_and_set_bit(req, &vcpu->requests);
-}
-
 static inline bool kvm_check_request(int req, struct kvm_vcpu *vcpu)
 {
 	if (test_bit(req, &vcpu->requests)) {
diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
index 71a4eaf..c9eed53 100644
--- a/virt/kvm/kvm_main.c
+++ b/virt/kvm/kvm_main.c
@@ -147,11 +147,9 @@ static bool make_all_cpus_request(struct kvm *kvm, unsigned int req)
 
 	zalloc_cpumask_var(&cpus, GFP_ATOMIC);
 
-	raw_spin_lock(&kvm->requests_lock);
-	me = smp_processor_id();
+	me = get_cpu();
 	kvm_for_each_vcpu(i, vcpu, kvm) {
-		if (kvm_make_check_request(req, vcpu))
-			continue;
+		kvm_make_request(req, vcpu);
 		cpu = vcpu->cpu;
 
 		/* Set ->requests bit before we read ->mode */
@@ -167,7 +165,7 @@ static bool make_all_cpus_request(struct kvm *kvm, unsigned int req)
 		smp_call_function_many(cpus, ack_flush, NULL, 1);
 	else
 		called = false;
-	raw_spin_unlock(&kvm->requests_lock);
+	put_cpu();
 	free_cpumask_var(cpus);
 	return called;
 }
@@ -433,7 +431,6 @@ static struct kvm *kvm_create_vm(void)
 	kvm->mm = current->mm;
 	atomic_inc(&kvm->mm->mm_count);
 	spin_lock_init(&kvm->mmu_lock);
-	raw_spin_lock_init(&kvm->requests_lock);
 	kvm_eventfd_init(kvm);
 	mutex_init(&kvm->lock);
 	mutex_init(&kvm->irq_lock);
-- 
1.7.3.4

Re: [PATCH v3 2/3] KVM: send IPI to vcpu only when it's in guest mode

[Nadav Har'El]

On Wed, Jan 12, 2011, Xiao Guangrong wrote about "[PATCH v3 2/3] KVM: send IPI to vcpu only when it's in guest mode":
> We can interrupt the vcpu only when it's running in guest mode
> to reduce IPI

Hi,

I am afraid there's a risk of confusion between the new

	vcpu->mode = IN_GUEST_MODE;

and the existing

	is_guest_mode()   (i.e., vcpu->arch.hflags & HF_GUEST_MASK)

The latter says that the virtual cpu is in guest mode (i.e., the guest used
VMLAUNCH and is runnning a nested guest), while the former says that the
physical CPU that this vcpu is currently being run on, is in guest mode - or
in other words, this vcpu is currently running.

I'm not sure what is the best way to resolve this potential for confusion.
Maybe on of them is better renamed (e.g., instead of vcpu->mode = IN_GUEST_MODE
have something like vcpu->running = NOW_RUNNING). Or maybe some good comments
need to to be written to explain the situation.

Actually, I just noticed that there's already a vcpu->guest_mode which you
are apparently replacing, so the potential for confusion is already in the
current code... I.e., in the current code, is_guest_mode(vcpu) does NOT check
vcpu->guest_mode...


-- 
Nadav Har'El                        |    Wednesday, Jan 12 2011, 7 Shevat 5771
nyh@math.technion.ac.il             |-----------------------------------------
Phone +972-523-790466, ICQ 13349191 |Hi! I'm a signature virus! Copy me into
http://nadav.harel.org.il           |your signature to help me spread!

Re: [PATCH v3 2/3] KVM: send IPI to vcpu only when it's in guest mode

[Avi Kivity]

On 01/12/2011 11:54 AM, Nadav Har'El wrote:
> On Wed, Jan 12, 2011, Xiao Guangrong wrote about "[PATCH v3 2/3] KVM: send IPI to vcpu only when it's in guest mode":
> >  We can interrupt the vcpu only when it's running in guest mode
> >  to reduce IPI
>
> Hi,
>
> I am afraid there's a risk of confusion between the new
>
> 	vcpu->mode = IN_GUEST_MODE;
>
> and the existing
>
> 	is_guest_mode()   (i.e., vcpu->arch.hflags&  HF_GUEST_MASK)
>
> The latter says that the virtual cpu is in guest mode (i.e., the guest used
> VMLAUNCH and is runnning a nested guest), while the former says that the
> physical CPU that this vcpu is currently being run on, is in guest mode - or
> in other words, this vcpu is currently running.
>
> I'm not sure what is the best way to resolve this potential for confusion.
> Maybe on of them is better renamed (e.g., instead of vcpu->mode = IN_GUEST_MODE
> have something like vcpu->running = NOW_RUNNING). Or maybe some good comments
> need to to be written to explain the situation.
>
> Actually, I just noticed that there's already a vcpu->guest_mode which you
> are apparently replacing, so the potential for confusion is already in the
> current code... I.e., in the current code, is_guest_mode(vcpu) does NOT check
> vcpu->guest_mode...
>

Right.  One is from the host's point of view, the other is from the 
guest's point of view.  I'll rename is_guest_mode() to is_nested_guest().

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

Re: [PATCH v3 1/3] KVM: fix rcu usage warning in kvm_arch_vcpu_ioctl_set_sregs()

[Xiao Guangrong]

On 01/12/2011 03:39 PM, Xiao Guangrong wrote:
> Fix:
> 
> [ 1001.499596] ===================================================
> [ 1001.499599] [ INFO: suspicious rcu_dereference_check() usage. ]
> [ 1001.499601] ---------------------------------------------------
> [ 1001.499604] include/linux/kvm_host.h:301 invoked rcu_dereference_check() without protection!
> 	......
> [ 1001.499636] Pid: 6035, comm: qemu-system-x86 Not tainted 2.6.37-rc6+ #62
> [ 1001.499638] Call Trace:
> [ 1001.499644]  [] lockdep_rcu_dereference+0x9d/0xa5
> [ 1001.499653]  [] gfn_to_memslot+0x8d/0xc8 [kvm]
> [ 1001.499661]  [] gfn_to_hva+0x16/0x3f [kvm]
> [ 1001.499669]  [] kvm_read_guest_page+0x1e/0x5e [kvm]
> [ 1001.499681]  [] kvm_read_guest_page_mmu+0x53/0x5e [kvm]
> [ 1001.499699]  [] load_pdptrs+0x3f/0x9c [kvm]
> [ 1001.499705]  [] ? vmx_set_cr0+0x507/0x517 [kvm_intel]
> [ 1001.499717]  [] kvm_arch_vcpu_ioctl_set_sregs+0x1f3/0x3c0 [kvm]
> [ 1001.499727]  [] kvm_vcpu_ioctl+0x6a5/0xbc5 [kvm]
> 
> Signed-off-by: Xiao Guangrong <xiaoguangrong@cn.fujitsu.com>

Ping ...?

Re: [PATCH v3 1/3] KVM: fix rcu usage warning in kvm_arch_vcpu_ioctl_set_sregs()

[Marcelo Tosatti]

On Wed, Jan 19, 2011 at 01:16:23PM +0800, Xiao Guangrong wrote:
> On 01/12/2011 03:39 PM, Xiao Guangrong wrote:
> > Fix:
> > 
> > [ 1001.499596] ===================================================
> > [ 1001.499599] [ INFO: suspicious rcu_dereference_check() usage. ]
> > [ 1001.499601] ---------------------------------------------------
> > [ 1001.499604] include/linux/kvm_host.h:301 invoked rcu_dereference_check() without protection!
> > 	......
> > [ 1001.499636] Pid: 6035, comm: qemu-system-x86 Not tainted 2.6.37-rc6+ #62
> > [ 1001.499638] Call Trace:
> > [ 1001.499644]  [] lockdep_rcu_dereference+0x9d/0xa5
> > [ 1001.499653]  [] gfn_to_memslot+0x8d/0xc8 [kvm]
> > [ 1001.499661]  [] gfn_to_hva+0x16/0x3f [kvm]
> > [ 1001.499669]  [] kvm_read_guest_page+0x1e/0x5e [kvm]
> > [ 1001.499681]  [] kvm_read_guest_page_mmu+0x53/0x5e [kvm]
> > [ 1001.499699]  [] load_pdptrs+0x3f/0x9c [kvm]
> > [ 1001.499705]  [] ? vmx_set_cr0+0x507/0x517 [kvm_intel]
> > [ 1001.499717]  [] kvm_arch_vcpu_ioctl_set_sregs+0x1f3/0x3c0 [kvm]
> > [ 1001.499727]  [] kvm_vcpu_ioctl+0x6a5/0xbc5 [kvm]
> > 
> > Signed-off-by: Xiao Guangrong <xiaoguangrong@cn.fujitsu.com>
> 
> Ping ...?

Applied this fix. For the make_all_cpus_request optimization, can you
show numbers with this new version? Because now there is LOCK# similarly
to the spinlock.

Re: [PATCH v3 1/3] KVM: fix rcu usage warning in kvm_arch_vcpu_ioctl_set_sregs()

[Xiao Guangrong]

On 01/20/2011 02:13 AM, Marcelo Tosatti wrote:
> On Wed, Jan 19, 2011 at 01:16:23PM +0800, Xiao Guangrong wrote:
>> On 01/12/2011 03:39 PM, Xiao Guangrong wrote:
>>> Fix:
>>>
>>> [ 1001.499596] ===================================================
>>> [ 1001.499599] [ INFO: suspicious rcu_dereference_check() usage. ]
>>> [ 1001.499601] ---------------------------------------------------
>>> [ 1001.499604] include/linux/kvm_host.h:301 invoked rcu_dereference_check() without protection!
>>> 	......
>>> [ 1001.499636] Pid: 6035, comm: qemu-system-x86 Not tainted 2.6.37-rc6+ #62
>>> [ 1001.499638] Call Trace:
>>> [ 1001.499644]  [] lockdep_rcu_dereference+0x9d/0xa5
>>> [ 1001.499653]  [] gfn_to_memslot+0x8d/0xc8 [kvm]
>>> [ 1001.499661]  [] gfn_to_hva+0x16/0x3f [kvm]
>>> [ 1001.499669]  [] kvm_read_guest_page+0x1e/0x5e [kvm]
>>> [ 1001.499681]  [] kvm_read_guest_page_mmu+0x53/0x5e [kvm]
>>> [ 1001.499699]  [] load_pdptrs+0x3f/0x9c [kvm]
>>> [ 1001.499705]  [] ? vmx_set_cr0+0x507/0x517 [kvm_intel]
>>> [ 1001.499717]  [] kvm_arch_vcpu_ioctl_set_sregs+0x1f3/0x3c0 [kvm]
>>> [ 1001.499727]  [] kvm_vcpu_ioctl+0x6a5/0xbc5 [kvm]
>>>
>>> Signed-off-by: Xiao Guangrong <xiaoguangrong@cn.fujitsu.com>
>>
>> Ping ...?
> 
> Applied this fix. For the make_all_cpus_request optimization, can you
> show numbers with this new version? Because now there is LOCK# similarly
> to the spinlock.

Marcelo,

Sure :-), there is the simply test result of kernbench:

Before patch:

real    5m6.493s                                                                
user    3m57.847s                                                               
sys     9m7.115s 

real    5m1.750s                                                                
user    4m0.109s                                                                
sys     9m10.192s       


After patch:
real    5m0.140s                                                                
user    3m57.956s                                                               
sys     8m58.339s  

real    4m56.314s                                                               
user    4m0.303s                                                                
sys     8m55.774s  

 

Re: [PATCH v3 1/3] KVM: fix rcu usage warning in kvm_arch_vcpu_ioctl_set_sregs()

[Marcelo Tosatti]

On Thu, Jan 20, 2011 at 11:03:31AM +0800, Xiao Guangrong wrote:
> On 01/20/2011 02:13 AM, Marcelo Tosatti wrote:
> > On Wed, Jan 19, 2011 at 01:16:23PM +0800, Xiao Guangrong wrote:
> >> On 01/12/2011 03:39 PM, Xiao Guangrong wrote:
> >>> Fix:
> >>>
> >>> [ 1001.499596] ===================================================
> >>> [ 1001.499599] [ INFO: suspicious rcu_dereference_check() usage. ]
> >>> [ 1001.499601] ---------------------------------------------------
> >>> [ 1001.499604] include/linux/kvm_host.h:301 invoked rcu_dereference_check() without protection!
> >>> 	......
> >>> [ 1001.499636] Pid: 6035, comm: qemu-system-x86 Not tainted 2.6.37-rc6+ #62
> >>> [ 1001.499638] Call Trace:
> >>> [ 1001.499644]  [] lockdep_rcu_dereference+0x9d/0xa5
> >>> [ 1001.499653]  [] gfn_to_memslot+0x8d/0xc8 [kvm]
> >>> [ 1001.499661]  [] gfn_to_hva+0x16/0x3f [kvm]
> >>> [ 1001.499669]  [] kvm_read_guest_page+0x1e/0x5e [kvm]
> >>> [ 1001.499681]  [] kvm_read_guest_page_mmu+0x53/0x5e [kvm]
> >>> [ 1001.499699]  [] load_pdptrs+0x3f/0x9c [kvm]
> >>> [ 1001.499705]  [] ? vmx_set_cr0+0x507/0x517 [kvm_intel]
> >>> [ 1001.499717]  [] kvm_arch_vcpu_ioctl_set_sregs+0x1f3/0x3c0 [kvm]
> >>> [ 1001.499727]  [] kvm_vcpu_ioctl+0x6a5/0xbc5 [kvm]
> >>>
> >>> Signed-off-by: Xiao Guangrong <xiaoguangrong@cn.fujitsu.com>
> >>
> >> Ping ...?
> > 
> > Applied this fix. For the make_all_cpus_request optimization, can you
> > show numbers with this new version? Because now there is LOCK# similarly
> > to the spinlock.
> 
> Marcelo,
> 
> Sure :-), there is the simply test result of kernbench:
> 
> Before patch:
> 
> real    5m6.493s                                                                
> user    3m57.847s                                                               
> sys     9m7.115s 
> 
> real    5m1.750s                                                                
> user    4m0.109s                                                                
> sys     9m10.192s       
> 
> 
> After patch:
> real    5m0.140s                                                                
> user    3m57.956s                                                               
> sys     8m58.339s  
> 
> real    4m56.314s                                                               
> user    4m0.303s                                                                
> sys     8m55.774s  

Nice. One disadvantageous side effect for the kvm_vcpu_kick path 
is that it can race with make_all_cpus_request, which is possibly
doing unrelated, slower work (IPI'ing other vcpus, waiting for
response).

Looks ok, but lets wait for more careful reviews before applying.

Re: [PATCH v3 1/3] KVM: fix rcu usage warning in kvm_arch_vcpu_ioctl_set_sregs()

[Xiao Guangrong]

On 01/20/2011 11:27 PM, Marcelo Tosatti wrote:

>> Sure :-), there is the simply test result of kernbench:
>>
>> Before patch:
>>
>> real    5m6.493s                                                                
>> user    3m57.847s                                                               
>> sys     9m7.115s 
>>
>> real    5m1.750s                                                                
>> user    4m0.109s                                                                
>> sys     9m10.192s       
>>
>>
>> After patch:
>> real    5m0.140s                                                                
>> user    3m57.956s                                                               
>> sys     8m58.339s  
>>
>> real    4m56.314s                                                               
>> user    4m0.303s                                                                
>> sys     8m55.774s  
> 
> Nice. One disadvantageous side effect for the kvm_vcpu_kick path 
> is that it can race with make_all_cpus_request, which is possibly
> doing unrelated, slower work (IPI'ing other vcpus, waiting for
> response).
> 
> Looks ok, but lets wait for more careful reviews before applying.
> 

Hi Avi, Marcelo,

I shall on vacation from 1.25 to 2.9, if you have comments on it i'll
replay after coming back. :-)

Re: [PATCH v3 1/3] KVM: fix rcu usage warning in kvm_arch_vcpu_ioctl_set_sregs()

[Avi Kivity]

On 01/20/2011 05:27 PM, Marcelo Tosatti wrote:
> >  Before patch:
> >
> >  real    5m6.493s
> >  user    3m57.847s
> >  sys     9m7.115s
> >
> >  real    5m1.750s
> >  user    4m0.109s
> >  sys     9m10.192s
> >
> >
> >  After patch:
> >  real    5m0.140s
> >  user    3m57.956s
> >  sys     8m58.339s
> >
> >  real    4m56.314s
> >  user    4m0.303s
> >  sys     8m55.774s
>
> Nice. One disadvantageous side effect for the kvm_vcpu_kick path
> is that it can race with make_all_cpus_request, which is possibly
> doing unrelated, slower work (IPI'ing other vcpus, waiting for
> response).

I think we're fine here.  The kvm_vcpu_kick() check is

  	me = get_cpu();
  	if (cpu != me&&  (unsigned)cpu<  nr_cpu_ids&&  cpu_online(cpu))
-		if (atomic_xchg(&vcpu->guest_mode, 0))
+		if (kvm_vcpu_exiting_guest_mode(vcpu) == IN_GUEST_MODE)
  			smp_send_reschedule(cpu);
  	put_cpu();


even if it did race, ->mode becomes EXITING_GUEST_MODE and we still 
avoid the IPI.  Note that make_all_cpus_request() cleverly checks for != 
OUTSIDE_GUEST_MODE, so if it loses the race with kvm_vcpu_kick(), it 
still sends the IPI to be sure the vcpu loop sees vcpu->requests in time.

> Looks ok, but lets wait for more careful reviews before applying.

Patches applied.

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