Messages in this thread | | | Date | Wed, 23 Mar 2011 11:39:06 +0100 | From | torbenh <> | Subject | Re: [patch 02/15] sched: validate CFS quota hierarchies |
| |
On Tue, Mar 22, 2011 at 08:03:28PM -0700, Paul Turner wrote: > Add constraints validation for CFS bandwidth hierachies. > > It is checked that: > sum(child bandwidth) <= parent_bandwidth > > In a quota limited hierarchy, an unconstrainted entity > (e.g. bandwidth==RUNTIME_INF) inherits the bandwidth of its parent. > > Since bandwidth periods may be non-uniform we normalize to the maximum allowed > period, 5 seconds. > > This behavior may be disabled (allowing child bandwidth to exceed parent) via > kernel.sched_cfs_bandwidth_consistent=0 > > Signed-off-by: Paul Turner <pjt@google.com> > > --- > include/linux/sched.h | 8 +++ > kernel/sched.c | 127 +++++++++++++++++++++++++++++++++++++++++++++++--- > kernel/sched_fair.c | 8 +++ > kernel/sysctl.c | 11 ++++ > 4 files changed, 147 insertions(+), 7 deletions(-) > > Index: tip/kernel/sched.c > =================================================================== > --- tip.orig/kernel/sched.c > +++ tip/kernel/sched.c > +static int tg_cfs_schedulable_down(struct task_group *tg, void *data) > +{ > + struct cfs_schedulable_data *d = data; > + struct cfs_bandwidth *cfs_b = tg_cfs_bandwidth(tg); > + s64 quota = 0, parent_quota = -1; > + > + quota = normalize_cfs_quota(tg, d); > + if (!tg->parent) { > + quota = RUNTIME_INF; > + } else { > + struct cfs_bandwidth *parent_b = tg_cfs_bandwidth(tg->parent); > + > + parent_quota = parent_b->hierarchal_quota; > + if (parent_quota != RUNTIME_INF) { > + parent_quota -= quota; > + /* invalid hierarchy, child bandwidth exceeds parent */ > + if (parent_quota < 0) > + return -EINVAL; > + } > + > + /* if no inherent limit then inherit parent quota */ > + if (quota == RUNTIME_INF) > + quota = parent_quota; > + parent_b->hierarchal_quota = parent_quota; > + } > + cfs_b->hierarchal_quota = quota; > + > + return 0; > +}
I find this logic pretty weird. As long as quota == INF i can overcommit, but as soon as there is some quota, i can not ?
Its clear, that one needs to be able to overcommit runtime, or the default runtime for a new cgroup would need to be 0. The root problem imo is that runtime and shares should not be in the same cgroup subsystem. The semantics are too different.
> + > +static int __cfs_schedulable(struct task_group *tg, u64 period, u64 quota) > +{ > + int ret; > + struct cfs_schedulable_data data = { > + .tg = tg, > + .period = period / NSEC_PER_USEC, > + .quota = quota / NSEC_PER_USEC, > + }; > + > + if (!sysctl_sched_cfs_bandwidth_consistent) > + return 0; > + > + rcu_read_lock(); > + ret = walk_tg_tree(tg_cfs_schedulable_down, tg_nop, > + &data); > + rcu_read_unlock();
walk_tg_tree does the rcu_read_lock itself. not necessary to do that here. look at __rt_schedulable there is no rcu.
> + > + return ret; > +}
-- torben Hohn
| |