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

[Stefano Stabellini <stefano.stabellini@xxxxxxxxxxxxx>]

On Thu, 15 Mar 2012, Jan Beulich wrote:
> >>> 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).

I agree with Jan: we could use a good comment there.
Other then that, ack.

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