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

[luca abeni]

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