Re: [CFS Bandwidth Control v4 4/7] sched: unthrottle cfs_rq(s) whoran out of quota at period refresh

[Peter Zijlstra]

On Thu, 2011-02-24 at 12:34 +0530, Bharata B Rao wrote:
> On Wed, Feb 23, 2011 at 02:32:12PM +0100, Peter Zijlstra wrote:
> > On Tue, 2011-02-15 at 19:18 -0800, Paul Turner wrote:
> > 
> > > +static void unthrottle_cfs_rq(struct cfs_rq *cfs_rq)
> > > +{
> > > +	struct rq *rq = rq_of(cfs_rq);
> > > +	struct sched_entity *se;
> > > +
> > > +	se = cfs_rq->tg->se[cpu_of(rq_of(cfs_rq))];
> > > +
> > > +	update_rq_clock(rq);
> > > +	/* (Try to) avoid maintaining share statistics for idle time */
> > > +	cfs_rq->load_stamp = cfs_rq->load_last = rq->clock_task;
> > 
> > Ok, so here you try to compensate for some of the weirdness from the
> > previous patch.. wouldn't it be much saner to fully consider the
> > throttled things dequeued for the load calculation etc.?
> > 
> > > +
> > > +	cfs_rq->throttled = 0;
> > > +	for_each_sched_entity(se) {
> > > +		if (se->on_rq)
> > > +			break;
> > > +
> > > +		cfs_rq = cfs_rq_of(se);
> > > +		enqueue_entity(cfs_rq, se, ENQUEUE_WAKEUP);
> > > +		if (cfs_rq_throttled(cfs_rq))
> > > +			break;
> > 
> > That's just weird, it was throttled, you enqueued it but find it
> > throttled.
> 
> se got enqueued to cfs_rq, but we find that cfs_rq is throttled and hence
> refrain from enqueueing cfs_rq futher.
> 
> So essentially enqueing to a throttled cfs_rq is allowed, but a throttled 
> group entitiy can't be enqueued further.

Argh, so this is about the total trainwreck you have for hierarchy
semantics (which you've basically inherited from the RT bits I guess,
which are similarly broken).

Do you want to support per-cgroup throttle periods? If so we need to sit
down and work this out, if not the above should not be possible because
each cgroup can basically run from the same refresh timer and everybody
will get throttled at the same time.


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/