From patchwork Fri Jan 9 06:34:38 2009 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Sheng Yang X-Patchwork-Id: 1454 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 n096V21S026003 for ; Thu, 8 Jan 2009 22:31:03 -0800 Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752728AbZAIGen (ORCPT ); Fri, 9 Jan 2009 01:34:43 -0500 Received: (majordomo@vger.kernel.org) by vger.kernel.org id S1752249AbZAIGen (ORCPT ); Fri, 9 Jan 2009 01:34:43 -0500 Received: from mga09.intel.com ([134.134.136.24]:15240 "EHLO mga09.intel.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751400AbZAIGem (ORCPT ); Fri, 9 Jan 2009 01:34:42 -0500 Received: from orsmga002.jf.intel.com ([10.7.209.21]) by orsmga102.jf.intel.com with ESMTP; 08 Jan 2009 22:26:40 -0800 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="4.37,238,1231142400"; d="scan'208";a="376987957" Received: from syang10-desktop.sh.intel.com (HELO syang10-desktop.localnet) ([10.239.13.176]) by orsmga002.jf.intel.com with ESMTP; 08 Jan 2009 22:33:09 -0800 From: Sheng Yang Organization: Intel Opensource Technology Center To: kvm@vger.kernel.org Subject: Re: [PATCH] Fix almost infinite loop in APIC Date: Fri, 9 Jan 2009 14:34:38 +0800 User-Agent: KMail/1.10.3 (Linux/2.6.27-9-generic; KDE/4.1.3; x86_64; ; ) Cc: Alexander Graf , avi@redhat.com, Kevin Wolf , Marcelo Tosatti References: <1231432566-9864-1-git-send-email-agraf@suse.de> In-Reply-To: <1231432566-9864-1-git-send-email-agraf@suse.de> MIME-Version: 1.0 Content-Disposition: inline Message-Id: <200901091434.39200.sheng@linux.intel.com> Sender: kvm-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: kvm@vger.kernel.org On Friday 09 January 2009 00:36:06 Alexander Graf wrote: > While booting Linux in VMware ESX, I encountered a strange effect > in the in-kernel lapic implementation: time went backwards! > > While this should never occur, because of that the while loop that > is done after the invalid calculations caused my host system to hang. > > In order to make debugging easier, let's replace this as suggested > with a modulo function and not run into the danger of looping forever. > > To replace the nice hint this bug gave me that the values are broken, > I added a printk message so people encountering this can at least > see that something is fishy. > > Of course, the real issue needs to be fixed as well! I'm open to ideas > why now < last_update! > > (Thanks to Kevin for his help in debugging this) > > Signed-off-by: Alexander Graf > Signed-off-by: Kevin Wolf > --- Hi Alexander I'm a little suspect here: > if (unlikely(ktime_to_ns(now) <= > ktime_to_ns(apic->timer.last_update))) { > /* Wrap around */ > passed = ktime_add(( { > (ktime_t) { > .tv64 = KTIME_MAX - > (apic->timer.last_update).tv64}; } > ), now); > apic_debug("time elapsed\n"); > } else > passed = ktime_sub(now, apic->timer.last_update); And now apic timer base is hr_timer with CLOCK_MONOTONIC, and get_time() is really ktime_get() which is almost impossible to wrap around. If it's overflow, at least we need a warning. I think this piece of code due to clock source change. So I doubt: due to some reason, now <= apic->timer.last_update, which cause a big wrap around operation. And the most suspect: > void kvm_apic_timer_intr_post(struct kvm_vcpu *vcpu, int vec) > { > struct kvm_lapic *apic = vcpu->arch.apic; > > if (apic && apic_lvt_vector(apic, APIC_LVTT) == vec) > apic->timer.last_update = ktime_add_ns( > apic->timer.last_update, > apic->timer.period); > } Not sure what's happening, have you tried this? (In fact, I am little willing to replace all apic->timer.dev.base->get_time() with more explicit ktime_get() as in pit.) --- int kvm_get_apic_interrupt(struct kvm_vcpu *vcpu) diff --git a/arch/x86/kvm/lapic.c b/arch/x86/kvm/lapic.c index afac68c..414e7e0 100644 --- a/arch/x86/kvm/lapic.c +++ b/arch/x86/kvm/lapic.c @@ -1115,9 +1115,7 @@ void kvm_apic_timer_intr_post(struct kvm_vcpu *vcpu, int vec) struct kvm_lapic *apic = vcpu->arch.apic; if (apic && apic_lvt_vector(apic, APIC_LVTT) == vec) - apic->timer.last_update = ktime_add_ns( - apic->timer.last_update, - apic->timer.period); + apic->timer.last_update = apic->timer.dev.base->get_time(); }