Currently, the max expiration time is 2147483647ns(INT32_MAX ns). This is enough
when guest is busy, but when guest is idle, the next timer will be later than
INT32_MAX ns. And those meaningless alarm will harm the pkg C-state.
Signed-off-by: Yang Zhang <yang.z.zhang@Intel.com>
---
vl.c | 6 +++---
1 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/vl.c b/vl.c
index be8587a..40486eb 100644
--- a/vl.c
+++ b/vl.c
@@ -1410,8 +1410,8 @@ static int64_t qemu_next_deadline(void)
delta = active_timers[QEMU_TIMER_VIRTUAL]->expire_time -
qemu_get_clock(vm_clock);
} else {
- /* To avoid problems with overflow limit this to 2^32. */
- delta = INT32_MAX;
+ /* To avoid problems with overflow limit this to 2^64 - 1000. */
+ delta = INT64_MAX - 1000;
}
if (delta < 0)
@@ -1427,7 +1427,7 @@ static uint64_t qemu_next_deadline_dyntick(void)
int64_t rtdelta;
if (use_icount)
- delta = INT32_MAX;
+ delta = INT64_MAX;
else
delta = (qemu_next_deadline() + 999) / 1000;
--
1.7.1
>>> On 14.03.12 at 03:54, "Zhang, Yang Z" <yang.z.zhang@intel.com> wrote: > Currently, the max expiration time is 2147483647ns(INT32_MAX ns). This is > enough when guest is busy, but when guest is idle, the next timer will be > later than INT32_MAX ns. And those meaningless alarm will harm the pkg > C-state.A wakeup every 2s can''t be that harmful.> Signed-off-by: Yang Zhang <yang.z.zhang@Intel.com> > --- > vl.c | 6 +++--- > 1 files changed, 3 insertions(+), 3 deletions(-) > > diff --git a/vl.c b/vl.c > index be8587a..40486eb 100644 > --- a/vl.c > +++ b/vl.c > @@ -1410,8 +1410,8 @@ static int64_t qemu_next_deadline(void) > delta = active_timers[QEMU_TIMER_VIRTUAL]->expire_time - > qemu_get_clock(vm_clock); > } else { > - /* To avoid problems with overflow limit this to 2^32. */ > - delta = INT32_MAX; > + /* To avoid problems with overflow limit this to 2^64 - 1000. */ > + delta = INT64_MAX - 1000;This looks rather arbitrary. Jan> } > > if (delta < 0) > @@ -1427,7 +1427,7 @@ static uint64_t qemu_next_deadline_dyntick(void) > int64_t rtdelta; > > if (use_icount) > - delta = INT32_MAX; > + delta = INT64_MAX; > else > delta = (qemu_next_deadline() + 999) / 1000; > > -- > 1.7.1 > > _______________________________________________ > Xen-devel mailing list > Xen-devel@lists.xen.org > http://lists.xen.org/xen-devel
Zhang, Yang Z writes ("[PATCH] use INT64_MAX as max
expiration"):> Currently, the max expiration time is 2147483647ns(INT32_MAX ns). This is
enough when guest is busy, but when guest is idle, the next timer will be later
than INT32_MAX ns. And those meaningless alarm will harm the pkg C-state.
This is a power performance improvement, yes ? Should we be accepting
these into qemu-xen-unstable at this point ? Although TBH it''s quite
a small change so it might be worth making an exception.
Ian.
On Wed, 14 Mar 2012, Jan Beulich wrote:> >>> On 14.03.12 at 03:54, "Zhang, Yang Z" <yang.z.zhang@intel.com> wrote: > > Currently, the max expiration time is 2147483647ns(INT32_MAX ns). This is > > enough when guest is busy, but when guest is idle, the next timer will be > > later than INT32_MAX ns. And those meaningless alarm will harm the pkg > > C-state. > > A wakeup every 2s can''t be that harmful.Right, but it is still a very good idea to avoid it if we can> > Signed-off-by: Yang Zhang <yang.z.zhang@Intel.com> > > --- > > vl.c | 6 +++--- > > 1 files changed, 3 insertions(+), 3 deletions(-) > > > > diff --git a/vl.c b/vl.c > > index be8587a..40486eb 100644 > > --- a/vl.c > > +++ b/vl.c > > @@ -1410,8 +1410,8 @@ static int64_t qemu_next_deadline(void) > > delta = active_timers[QEMU_TIMER_VIRTUAL]->expire_time - > > qemu_get_clock(vm_clock); > > } else { > > - /* To avoid problems with overflow limit this to 2^32. */ > > - delta = INT32_MAX; > > + /* To avoid problems with overflow limit this to 2^64 - 1000. */ > > + delta = INT64_MAX - 1000; > > This looks rather arbitrary.Give a look at qemu_next_deadline_dyntick, it adds 999 to whatever qemu_next_deadline returns. I think we should set delta to INT64_MAX in qemu_next_deadline and fix the calculation of delta in qemu_next_deadline_dyntick so that it doesn''t overflow even if qemu_next_deadline returns INT64_MAX. Something like: delta = qemu_next_deadline(); delta = (delta / 1000) + (delta % 1000 > 0 ? 1 : 0);
On Wed, 14 Mar 2012, Ian Jackson wrote:> Zhang, Yang Z writes ("[PATCH] use INT64_MAX as max expiration"): > > Currently, the max expiration time is 2147483647ns(INT32_MAX ns). This is enough when guest is busy, but when guest is idle, the next timer will be later than INT32_MAX ns. And those meaningless alarm will harm the pkg C-state. > > This is a power performance improvement, yes ? Should we be accepting > these into qemu-xen-unstable at this point ? Although TBH it''s quite > a small change so it might be worth making an exception.I am in favor of making an exception for this one
Stefano Stabellini writes ("Re: [Xen-devel] [PATCH] use INT64_MAX as max
expiration"):> On Wed, 14 Mar 2012, Ian Jackson wrote:
> > This is a power performance improvement, yes ? Should we be accepting
> > these into qemu-xen-unstable at this point ? Although TBH
it''s quite
> > a small change so it might be worth making an exception.
>
> I am in favor of making an exception for this one
OK. I''ll await a revised version addressing your comments in the
other subthread, thenk.
Thanks,
Ian.
Thanks for all your comments. Will send out the modified patch according to stefano''s suggestion. best regards yang> -----Original Message----- > From: Ian Jackson [mailto:Ian.Jackson@eu.citrix.com] > Sent: Wednesday, March 14, 2012 7:42 PM > To: Stefano Stabellini > Cc: Zhang, Yang Z; xen-devel@lists.xensource.com > Subject: Re: [Xen-devel] [PATCH] use INT64_MAX as max expiration > > Stefano Stabellini writes ("Re: [Xen-devel] [PATCH] use INT64_MAX as max > expiration"): > > On Wed, 14 Mar 2012, Ian Jackson wrote: > > > This is a power performance improvement, yes ? Should we be accepting > > > these into qemu-xen-unstable at this point ? Although TBH it''s quite > > > a small change so it might be worth making an exception. > > > > I am in favor of making an exception for this one > > OK. I''ll await a revised version addressing your comments in the > other subthread, thenk. > > Thanks, > Ian.