linux-kernel - Re: [patch 02/15] sched: validate CFS quota hierarchies

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: <4D9182F0.2020404@jp.fujitsu.com>
Date:	Tue, 29 Mar 2011 15:57:52 +0900
From:	Hidetoshi Seto <seto.hidetoshi@...fujitsu.com>
To:	Paul Turner <pjt@...gle.com>
CC:	linux-kernel@...r.kernel.org,
	Peter Zijlstra <a.p.zijlstra@...llo.nl>,
	Bharata B Rao <bharata@...ux.vnet.ibm.com>,
	Dhaval Giani <dhaval.giani@...il.com>,
	Balbir Singh <balbir@...ux.vnet.ibm.com>,
	Vaidyanathan Srinivasan <svaidy@...ux.vnet.ibm.com>,
	Srivatsa Vaddagiri <vatsa@...ibm.com>,
	Kamalesh Babulal <kamalesh@...ux.vnet.ibm.com>,
	Ingo Molnar <mingo@...e.hu>, Pavel Emelyanov <xemul@...nvz.org>
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@...alhost group0]# cat cpu.cfs_*
500000
500000
[root@...alhost group0]# cat sub0/cpu.cfs_*
500000
100000
[root@...alhost group0]# cat sub1/cpu.cfs_*
500000
100000
[root@...alhost 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@...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

--
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/