Messages in this thread Patch in this message | | | Date | Tue, 29 Mar 2011 15:57:52 +0900 | From | Hidetoshi Seto <> | Subject | Re: [patch 02/15] sched: validate CFS quota hierarchies |
| |
(2011/03/23 12:03), Paul Turner wrote: > @@ -9251,7 +9255,13 @@ static int tg_set_cfs_bandwidth(struct t > if (period > max_cfs_quota_period) > return -EINVAL; > > - mutex_lock(&mutex); > + mutex_lock(&cfs_constraints_mutex); > + if (sysctl_sched_cfs_bandwidth_consistent) { > + ret = __cfs_schedulable(tg, period, quota);
At this point: period => scale in ns unit quota => scale in ns unit, or RUNTIME_INF
And both are unsigned. But...
> @@ -9339,6 +9350,108 @@ static int cpu_cfs_period_write_u64(stru > return tg_set_cfs_period(cgroup_tg(cgrp), cfs_period_us); > } > > + > +struct cfs_schedulable_data { > + struct task_group *tg; > + u64 period, quota; > +}; > + > +/* > + * 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; > + struct load_weight lw; > + > + if (tg == d->tg) { > + period = d->period; > + quota = d->quota; > + } else { > + period = tg_get_cfs_period(tg); > + quota = tg_get_cfs_quota(tg); > + }
... at this point: period => scale in us unit quota => scale in us unit, or -1 Moreover: d->period => (scale in ns unit) / NSEC_PER_USEC d->quota => (scale in ns unit, or RUNTIME_INF) / NSEC_PER_USEC
Therefore, ...
> + > + if (quota == RUNTIME_INF) > + return RUNTIME_INF;
This check doesn't work properly.
I found this problem because I could not get child group back to be unconstrained:
[root@localhost group0]# cat cpu.cfs_* 500000 500000 [root@localhost group0]# cat sub0/cpu.cfs_* 500000 100000 [root@localhost group0]# cat sub1/cpu.cfs_* 500000 100000 [root@localhost group0]# echo -1 > sub1/cpu.cfs_quota_us bash: echo: write error: Invalid argument
I confirmed that this write error is removed by the following change. I'm looking forward to seeing your V6 soon.
Reviewed-by: Hidetoshi Seto <seto.hidetoshi@jp.fujitsu.com>
Thanks, H.Seto
--- kernel/sched.c | 8 ++++---- 1 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/kernel/sched.c b/kernel/sched.c index 6d764b5..c8f9820 100644 --- a/kernel/sched.c +++ b/kernel/sched.c @@ -9467,8 +9467,8 @@ static u64 normalize_cfs_quota(struct task_group *tg, period = d->period; quota = d->quota; } else { - period = tg_get_cfs_period(tg); - quota = tg_get_cfs_quota(tg); + period = ktime_to_ns(tg_cfs_bandwidth(tg)->period); + quota = tg_cfs_bandwidth(tg)->quota; } if (quota == RUNTIME_INF) @@ -9515,8 +9515,8 @@ 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, + .period = period, + .quota = quota, }; if (!sysctl_sched_cfs_bandwidth_consistent) -- 1.7.4
| |