linux-kernel - Re: [CFS Bandwidth Control v4 4/7] sched: unthrottle cfs_rq(s) who ran out of quota at period refresh

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <1298546043.2428.24.camel@twins>
Date:	Thu, 24 Feb 2011 12:14:03 +0100
From:	Peter Zijlstra <a.p.zijlstra@...llo.nl>
To:	bharata@...ux.vnet.ibm.com
Cc:	Paul Turner <pjt@...gle.com>, linux-kernel@...r.kernel.org,
	Dhaval Giani <dhaval@...ux.vnet.ibm.com>,
	Balbir Singh <balbir@...ux.vnet.ibm.com>,
	Vaidyanathan Srinivasan <svaidy@...ux.vnet.ibm.com>,
	Gautham R Shenoy <ego@...ibm.com>,
	Srivatsa Vaddagiri <vatsa@...ibm.com>,
	Kamalesh Babulal <kamalesh@...ux.vnet.ibm.com>,
	Ingo Molnar <mingo@...e.hu>,
	Pavel Emelyanov <xemul@...nvz.org>,
	Herbert Poetzl <herbert@...hfloor.at>,
	Avi Kivity <avi@...hat.com>,
	Chris Friesen <cfriesen@...tel.com>,
	Nikhil Rao <ncrao@...gle.com>
Subject: Re: [CFS Bandwidth Control v4 4/7] sched: unthrottle cfs_rq(s) who
 ran out of quota at period refresh

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@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/