Re: [RFC v3 1/6] Track the active utilisation

From: luca abeni
Date: Wed Nov 09 2016 - 11:29:43 EST


On Tue, 8 Nov 2016 17:56:35 +0000
Juri Lelli <juri.lelli@xxxxxxx> wrote:
[...]
> > > > @@ -947,14 +965,19 @@ static void enqueue_task_dl(struct rq *rq, struct task_struct *p, int flags)
> > > > return;
> > > > }
> > > >
> > > > + if (p->on_rq == TASK_ON_RQ_MIGRATING)
> > > > + add_running_bw(&p->dl, &rq->dl);
> > > > +
> > > > /*
> > > > * If p is throttled, we do nothing. In fact, if it exhausted
> > > > * its budget it needs a replenishment and, since it now is on
> > > > * its rq, the bandwidth timer callback (which clearly has not
> > > > * run yet) will take care of this.
> > > > */
> > > > - if (p->dl.dl_throttled && !(flags & ENQUEUE_REPLENISH))
> > > > + if (p->dl.dl_throttled && !(flags & ENQUEUE_REPLENISH)) {
> > > > + add_running_bw(&p->dl, &rq->dl);
> > >
> > > Don't rememeber if we discussed this already, but do we need to add the bw here
> > > even if the task is not actually enqueued until after the replenishment timer
> > > fires?
> > I think yes... The active utilization does not depend on the fact that the task
> > is on the runqueue or not, but depends on the task's state (in GRUB parlance,
> > "inactive" vs "active contending"). In other words, even when a task is throttled
> > its utilization must be counted in the active utilization.
> >
>
> OK. Could you add a comment about this point please (so that I don't
> forget again :)?
So, I just changed the comment in

/*
* If p is throttled, we do not enqueue it. In fact, if it exhausted
* its budget it needs a replenishment and, since it now is on
* its rq, the bandwidth timer callback (which clearly has not
* run yet) will take care of this.
* However, the active utilization does not depend on the fact
* that the task is on the runqueue or not (but depends on the
* task's state - in GRUB parlance, "inactive" vs "active contending").
* In other words, even if a task is throttled its utilization must
* be counted in the active utilization; hence, we need to call
* add_running_bw().
*/

Is this ok?


Thanks,
Luca