From mboxrd@z Thu Jan  1 00:00:00 1970
From: Sheng Yang <sheng@linux.intel.com>
Subject: Re: [PATCH] Fix almost infinite loop in APIC
Date: Wed, 21 Jan 2009 10:40:49 +0800
Message-ID: <200901211040.49733.sheng@linux.intel.com>
References: <1231432566-9864-1-git-send-email-agraf@suse.de> <200901202143.16125.sheng@linux.intel.com> <20090120185139.GA28746@amt.cnet>
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: Marcelo Tosatti <mtosatti@redhat.com>
Return-path: <kvm-owner@vger.kernel.org>
Received: from mga05.intel.com ([192.55.52.89]:61014 "EHLO
	fmsmga101.fm.intel.com" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org
	with ESMTP id S1754656AbZAUCkw convert rfc822-to-8bit (ORCPT
	<rfc822;kvm@vger.kernel.org>); Tue, 20 Jan 2009 21:40:52 -0500
In-Reply-To: <20090120185139.GA28746@amt.cnet>
Content-Disposition: inline
Sender: kvm-owner@vger.kernel.org
List-ID: <kvm.vger.kernel.org>

On Wednesday 21 January 2009 02:51:39 Marcelo Tosatti wrote:
> Hi Sheng,
>
> On Tue, Jan 20, 2009 at 09:43:15PM +0800, Sheng Yang wrote:
> > Marcelo, I realize some interesting things.
> >
> > In fact, on my machine, when I measured the delta of now() and
> > last_update at kvm_apic_timer_intr_post(), the delta was bigger and
> > bigger...(and now() is always bigger for me, so still no clue for w=
hy
> > data can be like above, last_update always ahead of when for about =
one
> > period...).
> >
> > Then I found something not good in original design - it ignored the
> > interval between time fire and injection, so we got:
> >
> > last_update =3D now() + n * period;
> >
> > And the time we update last_update:
> >
> > time =3D now() + n * period + n * interval.
> >
> > So last_update time is more and more inaccurate... Though it was re=
vised
> > by tmcct function, it's still not a good way to go.
> >
> > Then I understand your purpose more.
> >
> > +=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->t=
imer.injection_delay;
> > +=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0if (ktime_to_ns(remai=
ning) > 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=
=A0remaining =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=
_expires_remaining(&apic->timer.dev);
> >
> > And about your patch, how about take interval between intr_post() a=
nd
> > read tmcct in to account as well?
>
> You mean to sum up interval between intr_post() and read tmcct into
> injection_delay?
>
> I don't get it. Can you be more descriptive please?

Mind continue to change...

A: time_fire
B: intr_post
C: read TMCCT

I think the sequence can be AABC or AACB(I just consider AABC last nigh=
t...).=20
We have determined to return A->C as TMCCT, and the interval between di=
fferent=20
A is period which can be ignored. So how about simply return=20
hrtimer_expires_remaining() for all tmcct reading including pending one=
s?

(And sorry for I still can't get why you return injection delay for TMC=
CT...)

--=20
regards
Yang, Sheng