From patchwork Tue Jun 9 13:37:01 2009 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Jan Kiszka X-Patchwork-Id: 29015 Received: from vger.kernel.org (vger.kernel.org [209.132.176.167]) by demeter.kernel.org (8.14.2/8.14.2) with ESMTP id n59DbSiR007733 for ; Tue, 9 Jun 2009 13:37:29 GMT Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753134AbZFINhW (ORCPT ); Tue, 9 Jun 2009 09:37:22 -0400 Received: (majordomo@vger.kernel.org) by vger.kernel.org id S1753560AbZFINhW (ORCPT ); Tue, 9 Jun 2009 09:37:22 -0400 Received: from lizzard.sbs.de ([194.138.37.39]:22415 "EHLO lizzard.sbs.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753049AbZFINhV (ORCPT ); Tue, 9 Jun 2009 09:37:21 -0400 Received: from mail1.sbs.de (localhost [127.0.0.1]) by lizzard.sbs.de (8.12.11.20060308/8.12.11) with ESMTP id n59Db29d030619; Tue, 9 Jun 2009 15:37:02 +0200 Received: from [139.25.109.167] (mchn012c.mchp.siemens.de [139.25.109.167] (may be forged)) by mail1.sbs.de (8.12.11.20060308/8.12.11) with ESMTP id n59Db1gf031736; Tue, 9 Jun 2009 15:37:01 +0200 Message-ID: <4A2E657D.5090405@siemens.com> Date: Tue, 09 Jun 2009 15:37:01 +0200 From: Jan Kiszka User-Agent: Mozilla/5.0 (X11; U; Linux i686 (x86_64); de; rv:1.8.1.12) Gecko/20080226 SUSE/2.0.0.12-1.1 Thunderbird/2.0.0.12 Mnenhy/0.7.5.666 MIME-Version: 1.0 To: Avi Kivity CC: kvm-devel , Marcelo Tosatti Subject: [PATCH v2 1/2] kvm: x86: Fix racy event propagation in kmv timer References: <4A2E2B3B.5020408@siemens.com> In-Reply-To: <4A2E2B3B.5020408@siemens.com> Sender: kvm-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: kvm@vger.kernel.org v2 as requested in private discussion: Broken into two pieces, and the second one will not change the original semantic. --------------> 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 --- arch/x86/kvm/timer.c | 16 ++++++++++------ 1 files changed, 10 insertions(+), 6 deletions(-) -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html diff --git a/arch/x86/kvm/timer.c b/arch/x86/kvm/timer.c index 86dbac0..36054e9 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);