From 1ed0ce000a6c20c36ec649e32fc24393ef418ed8 Mon Sep 17 00:00:00 2001 From: Gleb Natapov Date: Tue, 9 Jun 2009 15:56:27 +0300 Subject: KVM: Use pointer to vcpu instead of vcpu_id in timer code. Signed-off-by: Gleb Natapov Signed-off-by: Avi Kivity --- arch/x86/kvm/timer.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'arch/x86/kvm/timer.c') diff --git a/arch/x86/kvm/timer.c b/arch/x86/kvm/timer.c index 86dbac072d0c..85cc743a8203 100644 --- a/arch/x86/kvm/timer.c +++ b/arch/x86/kvm/timer.c @@ -33,7 +33,7 @@ enum hrtimer_restart kvm_timer_fn(struct hrtimer *data) struct kvm_vcpu *vcpu; struct kvm_timer *ktimer = container_of(data, struct kvm_timer, timer); - vcpu = ktimer->kvm->vcpus[ktimer->vcpu_id]; + vcpu = ktimer->vcpu; if (!vcpu) return HRTIMER_NORESTART; -- cgit v1.2.2 From f7104db26ab2bc5f642892774ac8fb0f15400969 Mon Sep 17 00:00:00 2001 From: Jan Kiszka Date: Tue, 9 Jun 2009 15:37:01 +0200 Subject: KVM: Fix racy event propagation in timer Minor issue that likely had no practical relevance: the kvm timer function so far incremented the pending counter and then may reset it again to 1 in case reinjection was disabled. This opened a small racy window with the corresponding VCPU loop that may have happened to run on another (real) CPU and already consumed the value. Fix it by skipping the incrementation in case pending is already > 0. This opens a different race windows, but may only rarely cause lost events in case we do not care about them anyway (!reinject). Signed-off-by: Jan Kiszka Signed-off-by: Avi Kivity --- arch/x86/kvm/timer.c | 16 ++++++++++------ 1 file changed, 10 insertions(+), 6 deletions(-) (limited to 'arch/x86/kvm/timer.c') diff --git a/arch/x86/kvm/timer.c b/arch/x86/kvm/timer.c index 85cc743a8203..1baed414b57a 100644 --- a/arch/x86/kvm/timer.c +++ b/arch/x86/kvm/timer.c @@ -9,12 +9,16 @@ static int __kvm_timer_fn(struct kvm_vcpu *vcpu, struct kvm_timer *ktimer) int restart_timer = 0; wait_queue_head_t *q = &vcpu->wq; - /* FIXME: this code should not know anything about vcpus */ - if (!atomic_inc_and_test(&ktimer->pending)) - set_bit(KVM_REQ_PENDING_TIMER, &vcpu->requests); - - if (!ktimer->reinject) - atomic_set(&ktimer->pending, 1); + /* + * There is a race window between reading and incrementing, but we do + * not care about potentially loosing timer events in the !reinject + * case anyway. + */ + if (ktimer->reinject || !atomic_read(&ktimer->pending)) { + /* FIXME: this code should not know anything about vcpus */ + if (!atomic_inc_and_test(&ktimer->pending)) + set_bit(KVM_REQ_PENDING_TIMER, &vcpu->requests); + } if (waitqueue_active(q)) wake_up_interruptible(q); -- cgit v1.2.2 From 681405bfc73a2717ae9b03b2bad465b009106f31 Mon Sep 17 00:00:00 2001 From: Jan Kiszka Date: Tue, 9 Jun 2009 15:37:03 +0200 Subject: KVM: Drop useless atomic test from timer function The current code tries to optimize the setting of KVM_REQ_PENDING_TIMER but used atomic_inc_and_test - which always returns true unless pending had the invalid value of -1 on entry. This patch drops the test part preserving the original semantic but expressing it less confusingly. Signed-off-by: Jan Kiszka Signed-off-by: Avi Kivity --- arch/x86/kvm/timer.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'arch/x86/kvm/timer.c') diff --git a/arch/x86/kvm/timer.c b/arch/x86/kvm/timer.c index 1baed414b57a..eea40439066c 100644 --- a/arch/x86/kvm/timer.c +++ b/arch/x86/kvm/timer.c @@ -15,9 +15,9 @@ static int __kvm_timer_fn(struct kvm_vcpu *vcpu, struct kvm_timer *ktimer) * case anyway. */ if (ktimer->reinject || !atomic_read(&ktimer->pending)) { + atomic_inc(&ktimer->pending); /* FIXME: this code should not know anything about vcpus */ - if (!atomic_inc_and_test(&ktimer->pending)) - set_bit(KVM_REQ_PENDING_TIMER, &vcpu->requests); + set_bit(KVM_REQ_PENDING_TIMER, &vcpu->requests); } if (waitqueue_active(q)) -- cgit v1.2.2