From mboxrd@z Thu Jan  1 00:00:00 1970
From: Marcelo Tosatti <mtosatti@redhat.com>
Subject: Re: [PATCH] Fix almost infinite loop in APIC
Date: Tue, 20 Jan 2009 16:51:39 -0200
Message-ID: <20090120185139.GA28746@amt.cnet>
References: <1231432566-9864-1-git-send-email-agraf@suse.de> <20090116050143.GA13032@amt.cnet> <4975AA51.2060705@suse.de> <200901202143.16125.sheng@linux.intel.com>
Mime-Version: 1.0
Content-Type: text/plain; charset=iso-8859-1
Content-Transfer-Encoding: QUOTED-PRINTABLE
Cc: kvm@vger.kernel.org, Alexander Graf <agraf@suse.de>,
	avi@redhat.com, Kevin Wolf <kwolf@suse.de>
To: Sheng Yang <sheng@linux.intel.com>
Return-path: <kvm-owner@vger.kernel.org>
Received: from mx2.redhat.com ([66.187.237.31]:43651 "EHLO mx2.redhat.com"
	rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP
	id S1759699AbZATSwD (ORCPT <rfc822;kvm@vger.kernel.org>);
	Tue, 20 Jan 2009 13:52:03 -0500
Content-Disposition: inline
In-Reply-To: <200901202143.16125.sheng@linux.intel.com>
Sender: kvm-owner@vger.kernel.org
List-ID: <kvm.vger.kernel.org>

Hi Sheng,

On Tue, Jan 20, 2009 at 09:43:15PM +0800, Sheng Yang wrote:
> Marcelo, I realize some interesting things.
>=20
> In fact, on my machine, when I measured the delta of now() and last_u=
pdate at=20
> kvm_apic_timer_intr_post(), the delta was bigger and bigger...(and no=
w() is=20
> always bigger for me, so still no clue for why data can be like above=
,=20
> last_update always ahead of when for about one period...).
>=20
> Then I found something not good in original design - it ignored the i=
nterval=20
> between time fire and injection, so we got:
>=20
> last_update =3D now() + n * period;
>=20
> And the time we update last_update:
>=20
> time =3D now() + n * period + n * interval.
>=20
> So last_update time is more and more inaccurate... Though it was revi=
sed by=20
> tmcct function, it's still not a good way to go.
>=20
> Then I understand your purpose more.
>=20
> +=A0=A0=A0=A0=A0=A0=A0if (unlikely(atomic_read(&apic->timer.pending) =
> 0)) {
> +=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0remaining =3D apic->tim=
er.injection_delay;
> +=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0if (ktime_to_ns(remaini=
ng) > apic->timer.period)
> +=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=
remaining =3D ns_to_ktime(apic->timer.period);
> + =A0 =A0 =A0 =A0} else
> +=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0remaining =3D hrtimer_e=
xpires_remaining(&apic->timer.dev);
>=20
> And about your patch, how about take interval between intr_post() and=
 read=20
> tmcct in to account as well?=20

You mean to sum up interval between intr_post() and read tmcct into
injection_delay?=20

I don't get it. Can you be more descriptive please?

> That can keep the consistent with hrtimer_get_remaining() in the read=
=20
> tmcct.

> And I think if remaining > period,=20
> remaining =3D remain % period maybe more reasonable here.

Yes, thats better.

> How do you think?
>=20
> --=20
> regards
> Yang, Sheng
>=20
>=20