Messages in this thread | | | From | Paul Turner <> | Date | Wed, 11 May 2011 02:37:38 -0700 | Subject | Re: [patch 04/15] sched: validate CFS quota hierarchies |
| |
On Tue, May 10, 2011 at 12:20 AM, Hidetoshi Seto <seto.hidetoshi@jp.fujitsu.com> wrote: > Description typos + one bug. > > (2011/05/03 18:28), Paul Turner wrote: >> Add constraints validation for CFS bandwidth hierachies. > > hierarchies > >> >> Validate that: >> sum(child bandwidth) <= parent_bandwidth >> >> In a quota limited hierarchy, an unconstrainted entity > > unconstrained > >> (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, 1 second. >> >> 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> >> >> --- > (snip) >> +/* >> + * normalize group quota/period to be quota/max_period >> + * note: units are usecs >> + */ >> +static u64 normalize_cfs_quota(struct task_group *tg, >> + struct cfs_schedulable_data *d) >> +{ >> + u64 quota, period; >> + >> + if (tg == d->tg) { >> + period = d->period; >> + quota = d->quota; >> + } else { >> + period = tg_get_cfs_period(tg); >> + quota = tg_get_cfs_quota(tg); >> + } >> + >> + if (quota == RUNTIME_INF) >> + return RUNTIME_INF; >> + >> + return to_ratio(period, quota); >> +} > > Since tg_get_cfs_quota() doesn't return RUNTIME_INF but -1, > this function needs a fix like following. > > For fixed version, feel free to add: > > Reviewed-by: Hidetoshi Seto <seto.hidetoshi@jp.fujitsu.com> > > Thanks, > H.Seto > > --- > kernel/sched.c | 7 ++++--- > 1 files changed, 4 insertions(+), 3 deletions(-) > > diff --git a/kernel/sched.c b/kernel/sched.c > index d2562aa..f171ba5 100644 > --- a/kernel/sched.c > +++ b/kernel/sched.c > @@ -9465,16 +9465,17 @@ static u64 normalize_cfs_quota(struct task_group *tg, > u64 quota, period; > > if (tg == d->tg) { > + if (d->quota == RUNTIME_INF) > + return RUNTIME_INF; > period = d->period; > quota = d->quota; > } else { > + if (tg_cfs_bandwidth(tg)->quota == RUNTIME_INF) > + return RUNTIME_INF; > period = tg_get_cfs_period(tg); > quota = tg_get_cfs_quota(tg); > } >
Good catch!
Just modifying: +if (quota == RUNTIME_INF || quota == -1) + return RUNTIME_INF;
Seems simpler.
Although really there's no reason for tg_get_cfs_runtime (and sched_group_rt_runtime from which it's cloned) not to be returning RUNTIME_INF and then doing the conversion within the cgroupfs handler.
Fixing both is probably a better clean-up.
> - if (quota == RUNTIME_INF) > - return RUNTIME_INF; > - > return to_ratio(period, quota); > } > > > -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
| |