Re: [PATCH v5 3/3] sched/cputime: Add steal time support to full dynticks CPU time accounting

[Rik van Riel <riel@redhat.com>]

[-- Attachment #1: Type: text/plain, Size: 1601 bytes --]

On Thu, 2016-06-09 at 07:57 +0800, Wanpeng Li wrote:
> 2016-06-09 3:05 GMT+08:00 Rik van Riel <riel@redhat.com>:
> > 
> > On Wed, 2016-06-08 at 11:05 +0800, Wanpeng Li wrote:
> > > 
> > > 
> > > @@ -681,12 +681,17 @@ static cputime_t vtime_delta(struct
> > > task_struct
> > > *tsk)
> > >  static cputime_t get_vtime_delta(struct task_struct *tsk)
> > >  {
> > >       unsigned long now = READ_ONCE(jiffies);
> > > -     unsigned long delta = now - tsk->vtime_snap;
> > > +     cputime_t delta_time, steal_time;
> > > 
> > > +     steal_time =
> > > jiffies_to_cputime(steal_account_process_tick());
> > > +     delta_time = jiffies_to_cputime(now - tsk->vtime_snap);
> > >       WARN_ON_ONCE(tsk->vtime_snap_whence == VTIME_INACTIVE);
> > >       tsk->vtime_snap = now;
> > > 
> > > -     return jiffies_to_cputime(delta);
> > > +     if (steal_time < delta_time)
> > > +             delta_time -= steal_time;
> > > +
> > > +     return delta_time;
> > >  }
> > This isn't right.
> > 
> > If steal_time is equal to or larger than delta_time,
> > get_vtime_delta needs to return 0, not delta_time.
> > 
> > Otherwise the same time will be counted twice.
> Paolo also pointed out this yesterday, so his proposal looks good to
> you, right?
> 
Yes it does.

I can build the irqtime rework on top of your patches,
taking irq and softirq time out of the vtime delta as
well.

With Paolo's proposal, no time will ever be accounted
double, which is a good thing.

-- 
All Rights Reversed.


[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 473 bytes --]