Re: [Xen-devel] [PATCH v2] use INT64_MAX as max expiration

["Zhang, Yang Z" <yang.z.zhang@xxxxxxxxx>]

> -----Original Message-----
> From: Jan Beulich [mailto:JBeulich@xxxxxxxx]
> Sent: Thursday, March 15, 2012 4:41 PM
> To: Zhang, Yang Z
> Cc: Ian Jackson; 'Stefano Stabellini(stefano.stabellini@xxxxxxxxxxxxx)'; 
> xen-devel
> Subject: Re: [PATCH v2] use INT64_MAX as max expiration
> 
> >>> On 15.03.12 at 01:57, "Zhang, Yang Z" <yang.z.zhang@xxxxxxxxx> wrote:
> > Change from v1:
> > According to Jan and Stefano's comments, drop the previous logic which
> > setting the value in an arbitrary way and use Stefano's suggestion instead.
> >
> > Currently, the max expiration time is 2147483647ns(INT32_MAX ns)n 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@xxxxxxxxx>
> > ---
> >  vl.c |   11 ++++++-----
> >  1 files changed, 6 insertions(+), 5 deletions(-)
> >
> > diff --git a/vl.c b/vl.c
> > index be8587a..a9b7a0d 100644
> > --- a/vl.c
> > +++ b/vl.c
> > @@ -1410,8 +1410,7 @@ 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;
> > +        delta = INT64_MAX;
> 
> You're silently removing the comment - was it wrong (i.e. is there no
> potential for overflow)? If so, this should be explained in the commit
> message. If not, the change is wrong (quite possibly, using e.g.
> 1 << 48 might be an alternative then, but it needs to be clarified
> what particular overflow is possible here, and that whatever new
> value gets chosen doesn't trigger any).
> 
In fact, the original patch is using INT64_MAX too. When introducing 
instruction count support, it try to round up the next tick(like: (delta + 999) 
/ 1000), and it will overflow if still using INT64_MAX, so it choose INT32_MAX 
instead.  
Now, we use ( (delta / 1000) + (delta % 1000 > 0 ? 1 : 0) and the overflow will 
not happen with this, so I removed the comments. 

best regards
yang

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel