* [PATCH v3 2/3] KVM: send IPI to vcpu only when it's in guest mode
2011-01-12 7:39 [PATCH v3 1/3] KVM: fix rcu usage warning in kvm_arch_vcpu_ioctl_set_sregs() Xiao Guangrong
@ 2011-01-12 7:40 ` Xiao Guangrong
2011-01-12 9:54 ` Nadav Har'El
2011-01-12 7:41 ` [PATCH v3 3/3] KVM: make make_all_cpus_request() lockless Xiao Guangrong
2011-01-19 5:16 ` [PATCH v3 1/3] KVM: fix rcu usage warning in kvm_arch_vcpu_ioctl_set_sregs() Xiao Guangrong
2 siblings, 1 reply; 11+ messages in thread
From: Xiao Guangrong @ 2011-01-12 7:40 UTC (permalink / raw)
To: Avi Kivity; +Cc: Marcelo Tosatti, LKML, KVM
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
^ permalink raw reply related [flat|nested] 11+ messages in thread* Re: [PATCH v3 2/3] KVM: send IPI to vcpu only when it's in guest mode
2011-01-12 7:40 ` [PATCH v3 2/3] KVM: send IPI to vcpu only when it's in guest mode Xiao Guangrong
@ 2011-01-12 9:54 ` Nadav Har'El
2011-01-12 9:59 ` Avi Kivity
0 siblings, 1 reply; 11+ messages in thread
From: Nadav Har'El @ 2011-01-12 9:54 UTC (permalink / raw)
To: Xiao Guangrong; +Cc: Avi Kivity, Marcelo Tosatti, LKML, KVM
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!
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v3 2/3] KVM: send IPI to vcpu only when it's in guest mode
2011-01-12 9:54 ` Nadav Har'El
@ 2011-01-12 9:59 ` Avi Kivity
0 siblings, 0 replies; 11+ messages in thread
From: Avi Kivity @ 2011-01-12 9:59 UTC (permalink / raw)
To: Nadav Har'El; +Cc: Xiao Guangrong, Marcelo Tosatti, LKML, KVM
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
^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH v3 3/3] KVM: make make_all_cpus_request() lockless
2011-01-12 7:39 [PATCH v3 1/3] KVM: fix rcu usage warning in kvm_arch_vcpu_ioctl_set_sregs() Xiao Guangrong
2011-01-12 7:40 ` [PATCH v3 2/3] KVM: send IPI to vcpu only when it's in guest mode Xiao Guangrong
@ 2011-01-12 7:41 ` Xiao Guangrong
2011-01-19 5:16 ` [PATCH v3 1/3] KVM: fix rcu usage warning in kvm_arch_vcpu_ioctl_set_sregs() Xiao Guangrong
2 siblings, 0 replies; 11+ messages in thread
From: Xiao Guangrong @ 2011-01-12 7:41 UTC (permalink / raw)
To: Avi Kivity; +Cc: Marcelo Tosatti, LKML, KVM
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
^ permalink raw reply related [flat|nested] 11+ messages in thread* Re: [PATCH v3 1/3] KVM: fix rcu usage warning in kvm_arch_vcpu_ioctl_set_sregs()
2011-01-12 7:39 [PATCH v3 1/3] KVM: fix rcu usage warning in kvm_arch_vcpu_ioctl_set_sregs() Xiao Guangrong
2011-01-12 7:40 ` [PATCH v3 2/3] KVM: send IPI to vcpu only when it's in guest mode Xiao Guangrong
2011-01-12 7:41 ` [PATCH v3 3/3] KVM: make make_all_cpus_request() lockless Xiao Guangrong
@ 2011-01-19 5:16 ` Xiao Guangrong
2011-01-19 18:13 ` Marcelo Tosatti
2 siblings, 1 reply; 11+ messages in thread
From: Xiao Guangrong @ 2011-01-19 5:16 UTC (permalink / raw)
To: Avi Kivity; +Cc: Marcelo Tosatti, LKML, KVM
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 ...?
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v3 1/3] KVM: fix rcu usage warning in kvm_arch_vcpu_ioctl_set_sregs()
2011-01-19 5:16 ` [PATCH v3 1/3] KVM: fix rcu usage warning in kvm_arch_vcpu_ioctl_set_sregs() Xiao Guangrong
@ 2011-01-19 18:13 ` Marcelo Tosatti
2011-01-20 3:03 ` Xiao Guangrong
0 siblings, 1 reply; 11+ messages in thread
From: Marcelo Tosatti @ 2011-01-19 18:13 UTC (permalink / raw)
To: Xiao Guangrong; +Cc: Avi Kivity, LKML, KVM
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.
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v3 1/3] KVM: fix rcu usage warning in kvm_arch_vcpu_ioctl_set_sregs()
2011-01-19 18:13 ` Marcelo Tosatti
@ 2011-01-20 3:03 ` Xiao Guangrong
2011-01-20 15:27 ` Marcelo Tosatti
0 siblings, 1 reply; 11+ messages in thread
From: Xiao Guangrong @ 2011-01-20 3:03 UTC (permalink / raw)
To: Marcelo Tosatti; +Cc: Avi Kivity, LKML, KVM
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
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v3 1/3] KVM: fix rcu usage warning in kvm_arch_vcpu_ioctl_set_sregs()
2011-01-20 3:03 ` Xiao Guangrong
@ 2011-01-20 15:27 ` Marcelo Tosatti
2011-01-27 2:35 ` Xiao Guangrong
2011-01-27 13:15 ` Avi Kivity
0 siblings, 2 replies; 11+ messages in thread
From: Marcelo Tosatti @ 2011-01-20 15:27 UTC (permalink / raw)
To: Xiao Guangrong; +Cc: Avi Kivity, LKML, KVM
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.
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v3 1/3] KVM: fix rcu usage warning in kvm_arch_vcpu_ioctl_set_sregs()
2011-01-20 15:27 ` Marcelo Tosatti
@ 2011-01-27 2:35 ` Xiao Guangrong
2011-01-27 13:15 ` Avi Kivity
1 sibling, 0 replies; 11+ messages in thread
From: Xiao Guangrong @ 2011-01-27 2:35 UTC (permalink / raw)
To: Marcelo Tosatti; +Cc: Avi Kivity, LKML, KVM
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. :-)
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v3 1/3] KVM: fix rcu usage warning in kvm_arch_vcpu_ioctl_set_sregs()
2011-01-20 15:27 ` Marcelo Tosatti
2011-01-27 2:35 ` Xiao Guangrong
@ 2011-01-27 13:15 ` Avi Kivity
1 sibling, 0 replies; 11+ messages in thread
From: Avi Kivity @ 2011-01-27 13:15 UTC (permalink / raw)
To: Marcelo Tosatti; +Cc: Xiao Guangrong, LKML, KVM
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
^ permalink raw reply [flat|nested] 11+ messages in thread