[LTP] [PATCH v2 0/6] cfs_bandwidth01 and CGroup API

[LTP] [PATCH v2 0/6] cfs_bandwidth01 and CGroup API

[Richard Palethorpe]

Hello,

This adds a test scheduler test which uses the cpu controller.

Also included are some additions and fixes to the CGroup API related
to the test.

V2:
* Simpler check for unrecognised controller name
* Dropped whitespace patch which was already merged
* Moved subgroup creation to setup. Otherwise it leaks memory between iterations.
  Although the test still worked because it doesn't error if the group already
  exists.

Richard Palethorpe (6):
  API/cgroups: Allow fetching of CGroup name
  API/cgroups: Remove obsolete function in API
  API/cgroups: Add cpu controller
  API/cgroups: Auto add controllers to subtree_control in new subgroup
  API/cgroups: tst_require fail gracefully with unknown controller
  sched/cgroup: Add cfs_bandwidth01

 include/tst_cgroup.h                          |   4 +
 lib/tst_cgroup.c                              |  58 ++++--
 runtest/sched                                 |   1 +
 .../kernel/sched/cfs-scheduler/.gitignore     |   1 +
 testcases/kernel/sched/cfs-scheduler/Makefile |   4 +-
 .../sched/cfs-scheduler/cfs_bandwidth01.c     | 175 ++++++++++++++++++
 6 files changed, 227 insertions(+), 16 deletions(-)
 create mode 100644 testcases/kernel/sched/cfs-scheduler/cfs_bandwidth01.c

-- 
2.31.1

[LTP] [PATCH v2 1/6] API/cgroups: Allow fetching of CGroup name

[Richard Palethorpe]

Lets the test author get the name of a CGroup.

Signed-off-by: Richard Palethorpe <rpalethorpe@suse.com>
---
 include/tst_cgroup.h | 4 ++++
 lib/tst_cgroup.c     | 5 +++++
 2 files changed, 9 insertions(+)

diff --git a/include/tst_cgroup.h b/include/tst_cgroup.h
index d4c93db79..de72645bc 100644
--- a/include/tst_cgroup.h
+++ b/include/tst_cgroup.h
@@ -133,6 +133,10 @@ struct tst_cgroup_group *
 tst_cgroup_group_mk(const struct tst_cgroup_group *const parent,
 		    const char *const group_name)
 		    __attribute__ ((nonnull, warn_unused_result));
+const char *
+tst_cgroup_group_name(const struct tst_cgroup_group *const cg)
+		      __attribute__ ((nonnull, warn_unused_result));
+
 /* Remove a descendant CGroup */
 struct tst_cgroup_group *
 tst_cgroup_group_rm(struct tst_cgroup_group *const cg)
diff --git a/lib/tst_cgroup.c b/lib/tst_cgroup.c
index 1e036d3c3..793a712e1 100644
--- a/lib/tst_cgroup.c
+++ b/lib/tst_cgroup.c
@@ -855,6 +855,11 @@ tst_cgroup_group_mk(const struct tst_cgroup_group *const parent,
 	return cg;
 }
 
+const char *tst_cgroup_group_name(const struct tst_cgroup_group *const cg)
+{
+	return cg->group_name;
+}
+
 struct tst_cgroup_group *tst_cgroup_group_rm(struct tst_cgroup_group *const cg)
 {
 	struct cgroup_dir **dir;
-- 
2.31.1

[LTP] [PATCH v2 2/6] API/cgroups: Remove obsolete function in API

[Richard Palethorpe]

Signed-off-by: Richard Palethorpe <rpalethorpe@suse.com>
---
 lib/tst_cgroup.c | 6 ------
 1 file changed, 6 deletions(-)

diff --git a/lib/tst_cgroup.c b/lib/tst_cgroup.c
index 793a712e1..8fd5ab288 100644
--- a/lib/tst_cgroup.c
+++ b/lib/tst_cgroup.c
@@ -225,12 +225,6 @@ static void add_ctrl(uint32_t *const ctrl_field,
 	*ctrl_field |= 1 << ctrl->ctrl_indx;
 }
 
-__attribute__ ((warn_unused_result))
-struct cgroup_root *tst_cgroup_root_get(void)
-{
-	return roots[0].ver ? roots : roots + 1;
-}
-
 static int cgroup_v2_mounted(void)
 {
 	return !!roots[0].ver;
-- 
2.31.1

[LTP] [PATCH v2 3/6] API/cgroups: Add cpu controller

[Richard Palethorpe]

Signed-off-by: Richard Palethorpe <rpalethorpe@suse.com>
---
 lib/tst_cgroup.c | 18 +++++++++++++++++-
 1 file changed, 17 insertions(+), 1 deletion(-)

diff --git a/lib/tst_cgroup.c b/lib/tst_cgroup.c
index 8fd5ab288..0b71d4eed 100644
--- a/lib/tst_cgroup.c
+++ b/lib/tst_cgroup.c
@@ -82,7 +82,8 @@ struct cgroup_root {
 /* Controller sub-systems */
 enum cgroup_ctrl_indx {
 	CTRL_MEMORY = 1,
-	CTRL_CPUSET = 2,
+	CTRL_CPU,
+	CTRL_CPUSET,
 };
 #define CTRLS_MAX CTRL_CPUSET
 
@@ -162,6 +163,18 @@ static const files_t memory_ctrl_files = {
 	{ }
 };
 
+static const files_t cpu_ctrl_files = {
+	/* The V1 quota and period files were combined in the V2 max
+	 * file. The quota is in the first column and if we just print
+	 * a single value to the file, it will be treated as the
+	 * quota. To get or set the period we need to branch on the
+	 * API version.
+	 */
+	{ "cpu.max", "cpu.cfs_quota_us", CTRL_CPU },
+	{ "cpu.cfs_period_us", "cpu.cfs_period_us", CTRL_CPU },
+	{ }
+};
+
 static const files_t cpuset_ctrl_files = {
 	{ "cpuset.cpus", "cpuset.cpus", CTRL_CPUSET },
 	{ "cpuset.mems", "cpuset.mems", CTRL_CPUSET },
@@ -174,6 +187,9 @@ static struct cgroup_ctrl controllers[] = {
 	[CTRL_MEMORY] = {
 		"memory", memory_ctrl_files, CTRL_MEMORY, NULL, 0
 	},
+	[CTRL_CPU] = {
+		"cpu", cpu_ctrl_files, CTRL_CPU, NULL, 0
+	},
 	[CTRL_CPUSET] = {
 		"cpuset", cpuset_ctrl_files, CTRL_CPUSET, NULL, 0
 	},
-- 
2.31.1

[LTP] [PATCH v2 4/6] API/cgroups: Auto add controllers to subtree_control in new subgroup

[Richard Palethorpe]

This is what we have always wanted so far. If we do not want to do it
on a new test then a new config option can be added during require.

Signed-off-by: Richard Palethorpe <rpalethorpe@suse.com>
---
 lib/tst_cgroup.c | 23 ++++++++++++++++-------
 1 file changed, 16 insertions(+), 7 deletions(-)

diff --git a/lib/tst_cgroup.c b/lib/tst_cgroup.c
index 0b71d4eed..74746f13e 100644
--- a/lib/tst_cgroup.c
+++ b/lib/tst_cgroup.c
@@ -826,19 +826,28 @@ static void cgroup_group_init(struct tst_cgroup_group *const cg,
 	strcpy(cg->group_name, group_name);
 }
 
-__attribute__ ((nonnull))
-static void cgroup_group_add_dir(struct tst_cgroup_group *const cg,
+__attribute__((nonnull (2, 3)))
+static void cgroup_group_add_dir(const struct tst_cgroup_group *const parent,
+				 struct tst_cgroup_group *const cg,
 				 struct cgroup_dir *const dir)
 {
 	const struct cgroup_ctrl *ctrl;
 	int i;
 
-	if (dir->dir_root->ver == TST_CGROUP_V2)
+	if (dir->dir_root->ver != TST_CGROUP_V1)
 		cg->dirs_by_ctrl[0] = dir;
 
 	for_each_ctrl(ctrl) {
-		if (has_ctrl(dir->ctrl_field, ctrl))
-			cg->dirs_by_ctrl[ctrl->ctrl_indx] = dir;
+		if (!has_ctrl(dir->ctrl_field, ctrl))
+			continue;
+
+		cg->dirs_by_ctrl[ctrl->ctrl_indx] = dir;
+
+		if (!parent || dir->dir_root->ver == TST_CGROUP_V1)
+			continue;
+
+		SAFE_CGROUP_PRINTF(parent, "cgroup.subtree_control",
+				   "+%s", ctrl->ctrl_name);
 	}
 
 	for (i = 0; cg->dirs[i]; i++);
@@ -859,7 +868,7 @@ tst_cgroup_group_mk(const struct tst_cgroup_group *const parent,
 	for_each_dir(parent, 0, dir) {
 		new_dir = SAFE_MALLOC(sizeof(*new_dir));
 		cgroup_dir_mk(*dir, group_name, new_dir);
-		cgroup_group_add_dir(cg, new_dir);
+		cgroup_group_add_dir(parent, cg, new_dir);
 	}
 
 	return cg;
@@ -1023,7 +1032,7 @@ static struct tst_cgroup_group *cgroup_group_from_roots(const size_t tree_off)
 		dir = (typeof(dir))(((char *)root) + tree_off);
 
 		if (dir->ctrl_field)
-			cgroup_group_add_dir(cg, dir);
+			cgroup_group_add_dir(NULL, cg, dir);
 	}
 
 	if (cg->dirs[0]) {
-- 
2.31.1

[LTP] [PATCH v2 5/6] API/cgroups: tst_require fail gracefully with unknown controller

[Richard Palethorpe]

Signed-off-by: Richard Palethorpe <rpalethorpe@suse.com>
---
 lib/tst_cgroup.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/lib/tst_cgroup.c b/lib/tst_cgroup.c
index 74746f13e..6d94ea41c 100644
--- a/lib/tst_cgroup.c
+++ b/lib/tst_cgroup.c
@@ -599,6 +599,12 @@ void tst_cgroup_require(const char *const ctrl_name,
 	struct cgroup_ctrl *const ctrl = cgroup_find_ctrl(ctrl_name);
 	struct cgroup_root *root;
 
+	if (!ctrl) {
+		tst_brk(TBROK, "'%s' controller is unknown to LTP", ctrl_name);
+		tst_brk(TBROK, "Calling %s in cleanup?", __func__);
+		return;
+	}
+
 	if (!options)
 		options = &default_opts;
 
-- 
2.31.1

[LTP] [PATCH v2 5/6] API/cgroups: tst_require fail gracefully with unknown controller

[Li Wang]

Hi Richard,

On Fri, May 21, 2021 at 6:26 PM Richard Palethorpe via ltp
<ltp@lists.linux.it> wrote:
>
> Signed-off-by: Richard Palethorpe <rpalethorpe@suse.com>
> ---
>  lib/tst_cgroup.c | 6 ++++++
>  1 file changed, 6 insertions(+)
>
> diff --git a/lib/tst_cgroup.c b/lib/tst_cgroup.c
> index 74746f13e..6d94ea41c 100644
> --- a/lib/tst_cgroup.c
> +++ b/lib/tst_cgroup.c
> @@ -599,6 +599,12 @@ void tst_cgroup_require(const char *const ctrl_name,
>         struct cgroup_ctrl *const ctrl = cgroup_find_ctrl(ctrl_name);
>         struct cgroup_root *root;
>
> +       if (!ctrl) {
> +               tst_brk(TBROK, "'%s' controller is unknown to LTP", ctrl_name);
> +               tst_brk(TBROK, "Calling %s in cleanup?", __func__);
> +               return;

It'd never go here to perform a return because the first tst_brk
will break the test directly. And, I don't know why we need the
second tst_brk to show calling in cleanup, is that possible?


> +       }
> +
>         if (!options)
>                 options = &default_opts;
>
> --
> 2.31.1
>
>
> --
> Mailing list info: https://lists.linux.it/listinfo/ltp
>


--
Regards,
Li Wang

[LTP] [PATCH v2 5/6] API/cgroups: tst_require fail gracefully with unknown controller

[Richard Palethorpe]

Hello Li,

Li Wang <liwang@redhat.com> writes:

> Hi Richard,
>
> On Fri, May 21, 2021 at 6:26 PM Richard Palethorpe via ltp
> <ltp@lists.linux.it> wrote:
>>
>> Signed-off-by: Richard Palethorpe <rpalethorpe@suse.com>
>> ---
>>  lib/tst_cgroup.c | 6 ++++++
>>  1 file changed, 6 insertions(+)
>>
>> diff --git a/lib/tst_cgroup.c b/lib/tst_cgroup.c
>> index 74746f13e..6d94ea41c 100644
>> --- a/lib/tst_cgroup.c
>> +++ b/lib/tst_cgroup.c
>> @@ -599,6 +599,12 @@ void tst_cgroup_require(const char *const ctrl_name,
>>         struct cgroup_ctrl *const ctrl = cgroup_find_ctrl(ctrl_name);
>>         struct cgroup_root *root;
>>
>> +       if (!ctrl) {
>> +               tst_brk(TBROK, "'%s' controller is unknown to LTP", ctrl_name);
>> +               tst_brk(TBROK, "Calling %s in cleanup?", __func__);
>> +               return;
>
> It'd never go here to perform a return because the first tst_brk
> will break the test directly. And, I don't know why we need the
> second tst_brk to show calling in cleanup, is that possible?

It can return if it is called during cleanup. tst_cgroup_require should
not be called from cleanup. However someone can do it by accident.

We probably need two versions of tst_brk. One which can return if called
from cleanup and one which does not. I suspect most tst_brk callers
assume it will not return. It is really only some safe library functions
which can handle that.



>
>
>> +       }
>> +
>>         if (!options)
>>                 options = &default_opts;
>>
>> --
>> 2.31.1
>>
>>
>> --
>> Mailing list info: https://lists.linux.it/listinfo/ltp
>>


-- 
Thank you,
Richard.

[LTP] [PATCH v2 5/6] API/cgroups: tst_require fail gracefully with unknown controller

[Li Wang]

> >> +       if (!ctrl) {
> >> +               tst_brk(TBROK, "'%s' controller is unknown to LTP", ctrl_name);
> >> +               tst_brk(TBROK, "Calling %s in cleanup?", __func__);
> >> +               return;
> >
> > It'd never go here to perform a return because the first tst_brk
> > will break the test directly. And, I don't know why we need the
> > second tst_brk to show calling in cleanup, is that possible?
>
> It can return if it is called during cleanup. tst_cgroup_require should
> not be called from cleanup. However someone can do it by accident.

Well, I see. But TBH, here worries too much about the unexpected situation:).

> We probably need two versions of tst_brk. One which can return if called
> from cleanup and one which does not. I suspect most tst_brk callers
> assume it will not return. It is really only some safe library functions
> which can handle that.

Yes, sounds reasonable to me.

[Cc Cyril if he has more advice on this]

--
Regards,
Li Wang

[LTP] [PATCH v2 6/6] sched/cgroup: Add cfs_bandwidth01

[Richard Palethorpe]

Signed-off-by: Richard Palethorpe <rpalethorpe@suse.com>
---
 runtest/sched                                 |   1 +
 .../kernel/sched/cfs-scheduler/.gitignore     |   1 +
 testcases/kernel/sched/cfs-scheduler/Makefile |   4 +-
 .../sched/cfs-scheduler/cfs_bandwidth01.c     | 175 ++++++++++++++++++
 4 files changed, 179 insertions(+), 2 deletions(-)
 create mode 100644 testcases/kernel/sched/cfs-scheduler/cfs_bandwidth01.c

diff --git a/runtest/sched b/runtest/sched
index bfc4f2711..592898723 100644
--- a/runtest/sched
+++ b/runtest/sched
@@ -6,6 +6,7 @@ pth_str03 pth_str03
 time-schedule01		time-schedule
 trace_sched01		trace_sched -c 1
 
+cfs_bandwidth01 cfs_bandwidth01 -i 5
 hackbench01 hackbench 50 process 1000
 hackbench02 hackbench 20 thread 1000
 
diff --git a/testcases/kernel/sched/cfs-scheduler/.gitignore b/testcases/kernel/sched/cfs-scheduler/.gitignore
index db2759e4f..c5dacd6ef 100644
--- a/testcases/kernel/sched/cfs-scheduler/.gitignore
+++ b/testcases/kernel/sched/cfs-scheduler/.gitignore
@@ -1 +1,2 @@
 /hackbench
+cfs_bandwidth01
diff --git a/testcases/kernel/sched/cfs-scheduler/Makefile b/testcases/kernel/sched/cfs-scheduler/Makefile
index aa3bf8459..2ffe1f7f9 100644
--- a/testcases/kernel/sched/cfs-scheduler/Makefile
+++ b/testcases/kernel/sched/cfs-scheduler/Makefile
@@ -18,8 +18,8 @@
 
 top_srcdir		?= ../../../..
 
-include $(top_srcdir)/include/mk/env_pre.mk
+include $(top_srcdir)/include/mk/testcases.mk
 
-LDLIBS			+= -lpthread
+hackbench: LDLIBS			+= -lpthread
 
 include $(top_srcdir)/include/mk/generic_leaf_target.mk
diff --git a/testcases/kernel/sched/cfs-scheduler/cfs_bandwidth01.c b/testcases/kernel/sched/cfs-scheduler/cfs_bandwidth01.c
new file mode 100644
index 000000000..7c988730e
--- /dev/null
+++ b/testcases/kernel/sched/cfs-scheduler/cfs_bandwidth01.c
@@ -0,0 +1,175 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/* Copyright (c) 2021 SUSE LLC <rpalethorpe@suse.com> */
+/*\
+ *
+ * [Description]
+ *
+ * Creates a multi-level CGroup hierarchy with the cpu controller
+ * enabled. The leaf groups are populated with "busy" processes which
+ * simulate intermittent cpu load. They spin for some time then sleep
+ * then repeat.
+ *
+ * Both the trunk and leaf groups are set cpu bandwidth limits. The
+ * busy processes will intermittently exceed these limits. Causing
+ * them to be throttled. When they begin sleeping this will then cause
+ * them to be unthrottle.
+ *
+ * The test is known to reproduce an issue with an update to
+ * SLE-15-SP1 (kernel 4.12.14-197.64, bsc#1179093).
+ */
+
+#include <stdlib.h>
+
+#include "tst_test.h"
+#include "tst_cgroup.h"
+#include "tst_timer.h"
+
+static const struct tst_cgroup_group *cg_test;
+static struct tst_cgroup_group *cg_level2, *cg_level3a, *cg_level3b;
+static struct tst_cgroup_group *cg_workers[3];
+
+static void set_cpu_quota(const struct tst_cgroup_group *const cg,
+			  const float quota_percent)
+{
+	const unsigned int period_us = 10000;
+	const unsigned int quota_us = (quota_percent / 100) * (float)period_us;
+
+	if (TST_CGROUP_VER(cg, "cpu") != TST_CGROUP_V1) {
+		SAFE_CGROUP_PRINTF(cg, "cpu.max",
+				   "%u %u", quota_us, period_us);
+	} else {
+		SAFE_CGROUP_PRINTF(cg, "cpu.max",
+				   "%u", quota_us);
+		SAFE_CGROUP_PRINTF(cg, "cpu.cfs_period_us",
+				  "%u", period_us);
+	}
+
+	tst_res(TINFO, "Set '%s/cpu.max' = '%d %d'",
+		tst_cgroup_group_name(cg), quota_us, period_us);
+}
+
+static struct tst_cgroup_group *
+mk_cpu_cgroup(const struct tst_cgroup_group *const cg_parent,
+	      const char *const cg_child_name,
+	      const float quota_percent)
+{
+	struct tst_cgroup_group *const cg =
+		tst_cgroup_group_mk(cg_parent, cg_child_name);
+
+	set_cpu_quota(cg, quota_percent);
+
+	return cg;
+}
+
+static void busy_loop(const unsigned int sleep_ms)
+{
+	for (;;) {
+		tst_timer_start(CLOCK_MONOTONIC_RAW);
+		while (!tst_timer_expired_ms(20))
+			;
+
+		const int ret = tst_checkpoint_wait(0, sleep_ms);
+
+		if (!ret)
+			exit(0);
+
+		if (errno != ETIMEDOUT)
+			tst_brk(TBROK | TERRNO, "tst_checkpoint_wait");
+	}
+}
+
+static void fork_busy_procs_in_cgroup(const struct tst_cgroup_group *const cg)
+{
+	const unsigned int sleeps_ms[] = {3000, 1000, 10};
+	const pid_t worker_pid = SAFE_FORK();
+	size_t i;
+
+	if (worker_pid)
+		return;
+
+	for (i = 0; i < ARRAY_SIZE(sleeps_ms); i++) {
+		const pid_t busy_pid = SAFE_FORK();
+
+		if (!busy_pid)
+			busy_loop(sleeps_ms[i]);
+
+		SAFE_CGROUP_PRINTF(cg, "cgroup.procs", "%d", busy_pid);
+	}
+
+	tst_reap_children();
+
+	exit(0);
+}
+
+static void do_test(void)
+{
+	size_t i;
+
+	for (i = 0; i < ARRAY_SIZE(cg_workers); i++)
+		fork_busy_procs_in_cgroup(cg_workers[i]);
+
+	tst_res(TPASS, "Scheduled bandwidth constrained workers");
+
+	sleep(1);
+
+	set_cpu_quota(cg_level2, 50);
+
+	sleep(2);
+
+	TST_CHECKPOINT_WAKE2(0, 3 * 3);
+	tst_reap_children();
+
+	tst_res(TPASS, "Workers exited");
+}
+
+static void setup(void)
+{
+	tst_cgroup_require("cpu", NULL);
+
+	cg_test = tst_cgroup_get_test_group();
+
+	cg_level2 = tst_cgroup_group_mk(cg_test, "level2");
+
+	cg_level3a = tst_cgroup_group_mk(cg_level2, "level3a");
+	cg_workers[0] = mk_cpu_cgroup(cg_level3a, "worker1", 30);
+	cg_workers[1] = mk_cpu_cgroup(cg_level3a, "worker2", 20);
+
+	cg_level3b = tst_cgroup_group_mk(cg_level2, "level3b");
+	cg_workers[2] = mk_cpu_cgroup(cg_level3b, "worker3", 30);
+}
+
+static void cleanup(void)
+{
+	size_t i;
+
+	for (i = 0; i < ARRAY_SIZE(cg_workers); i++) {
+		if (cg_workers[i])
+			cg_workers[i] = tst_cgroup_group_rm(cg_workers[i]);
+	}
+
+	if (cg_level3a)
+		cg_level3a = tst_cgroup_group_rm(cg_level3a);
+	if (cg_level3b)
+		cg_level3b = tst_cgroup_group_rm(cg_level3b);
+	if (cg_level2)
+		cg_level2 = tst_cgroup_group_rm(cg_level2);
+
+	tst_cgroup_cleanup();
+}
+
+static struct tst_test test = {
+	.test_all = do_test,
+	.setup = setup,
+	.cleanup = cleanup,
+	.forks_child = 1,
+	.needs_checkpoints = 1,
+	.taint_check = TST_TAINT_W | TST_TAINT_D,
+	.tags = (const struct tst_tag[]) {
+		{"linux-git", "39f23ce07b93"},
+		{"linux-git", "b34cb07dde7c"},
+		{"linux-git", "fe61468b2cbc"},
+		{"linux-git", "5ab297bab984"},
+		{"linux-git", "6d4d22468dae"},
+		{ }
+	}
+};
-- 
2.31.1

[LTP] [PATCH v2 6/6] sched/cgroup: Add cfs_bandwidth01

[Li Wang]

On Fri, May 21, 2021 at 6:26 PM Richard Palethorpe via ltp
<ltp@lists.linux.it> wrote:
>
> Signed-off-by: Richard Palethorpe <rpalethorpe@suse.com>
> ---
>  runtest/sched                                 |   1 +
>  .../kernel/sched/cfs-scheduler/.gitignore     |   1 +
>  testcases/kernel/sched/cfs-scheduler/Makefile |   4 +-
>  .../sched/cfs-scheduler/cfs_bandwidth01.c     | 175 ++++++++++++++++++
>  4 files changed, 179 insertions(+), 2 deletions(-)
>  create mode 100644 testcases/kernel/sched/cfs-scheduler/cfs_bandwidth01.c
>
> diff --git a/runtest/sched b/runtest/sched
> index bfc4f2711..592898723 100644
> --- a/runtest/sched
> +++ b/runtest/sched
> @@ -6,6 +6,7 @@ pth_str03 pth_str03
>  time-schedule01                time-schedule
>  trace_sched01          trace_sched -c 1
>
> +cfs_bandwidth01 cfs_bandwidth01 -i 5
>  hackbench01 hackbench 50 process 1000
>  hackbench02 hackbench 20 thread 1000
>
> diff --git a/testcases/kernel/sched/cfs-scheduler/.gitignore b/testcases/kernel/sched/cfs-scheduler/.gitignore
> index db2759e4f..c5dacd6ef 100644
> --- a/testcases/kernel/sched/cfs-scheduler/.gitignore
> +++ b/testcases/kernel/sched/cfs-scheduler/.gitignore
> @@ -1 +1,2 @@
>  /hackbench
> +cfs_bandwidth01
> diff --git a/testcases/kernel/sched/cfs-scheduler/Makefile b/testcases/kernel/sched/cfs-scheduler/Makefile
> index aa3bf8459..2ffe1f7f9 100644
> --- a/testcases/kernel/sched/cfs-scheduler/Makefile
> +++ b/testcases/kernel/sched/cfs-scheduler/Makefile
> @@ -18,8 +18,8 @@
>
>  top_srcdir             ?= ../../../..
>
> -include $(top_srcdir)/include/mk/env_pre.mk
> +include $(top_srcdir)/include/mk/testcases.mk
>
> -LDLIBS                 += -lpthread
> +hackbench: LDLIBS                      += -lpthread
>
>  include $(top_srcdir)/include/mk/generic_leaf_target.mk
> diff --git a/testcases/kernel/sched/cfs-scheduler/cfs_bandwidth01.c b/testcases/kernel/sched/cfs-scheduler/cfs_bandwidth01.c
> new file mode 100644
> index 000000000..7c988730e
> --- /dev/null
> +++ b/testcases/kernel/sched/cfs-scheduler/cfs_bandwidth01.c
> @@ -0,0 +1,175 @@
> +// SPDX-License-Identifier: GPL-2.0-or-later
> +/* Copyright (c) 2021 SUSE LLC <rpalethorpe@suse.com> */
> +/*\
> + *
> + * [Description]
> + *
> + * Creates a multi-level CGroup hierarchy with the cpu controller
> + * enabled. The leaf groups are populated with "busy" processes which
> + * simulate intermittent cpu load. They spin for some time then sleep
> + * then repeat.
> + *
> + * Both the trunk and leaf groups are set cpu bandwidth limits. The
> + * busy processes will intermittently exceed these limits. Causing
> + * them to be throttled. When they begin sleeping this will then cause
> + * them to be unthrottle.
> + *
> + * The test is known to reproduce an issue with an update to
> + * SLE-15-SP1 (kernel 4.12.14-197.64, bsc#1179093).
> + */
> +
> +#include <stdlib.h>
> +
> +#include "tst_test.h"
> +#include "tst_cgroup.h"
> +#include "tst_timer.h"
> +
> +static const struct tst_cgroup_group *cg_test;
> +static struct tst_cgroup_group *cg_level2, *cg_level3a, *cg_level3b;
> +static struct tst_cgroup_group *cg_workers[3];
> +
> +static void set_cpu_quota(const struct tst_cgroup_group *const cg,
> +                         const float quota_percent)
> +{
> +       const unsigned int period_us = 10000;
> +       const unsigned int quota_us = (quota_percent / 100) * (float)period_us;
> +
> +       if (TST_CGROUP_VER(cg, "cpu") != TST_CGROUP_V1) {
> +               SAFE_CGROUP_PRINTF(cg, "cpu.max",
> +                                  "%u %u", quota_us, period_us);
> +       } else {
> +               SAFE_CGROUP_PRINTF(cg, "cpu.max",
> +                                  "%u", quota_us);
> +               SAFE_CGROUP_PRINTF(cg, "cpu.cfs_period_us",
> +                                 "%u", period_us);
> +       }
> +
> +       tst_res(TINFO, "Set '%s/cpu.max' = '%d %d'",
> +               tst_cgroup_group_name(cg), quota_us, period_us);
> +}
> +
> +static struct tst_cgroup_group *
> +mk_cpu_cgroup(const struct tst_cgroup_group *const cg_parent,
> +             const char *const cg_child_name,
> +             const float quota_percent)
> +{
> +       struct tst_cgroup_group *const cg =
> +               tst_cgroup_group_mk(cg_parent, cg_child_name);
> +
> +       set_cpu_quota(cg, quota_percent);
> +
> +       return cg;
> +}
> +
> +static void busy_loop(const unsigned int sleep_ms)
> +{
> +       for (;;) {
> +               tst_timer_start(CLOCK_MONOTONIC_RAW);
> +               while (!tst_timer_expired_ms(20))
> +                       ;
> +
> +               const int ret = tst_checkpoint_wait(0, sleep_ms);
> +
> +               if (!ret)
> +                       exit(0);
> +
> +               if (errno != ETIMEDOUT)
> +                       tst_brk(TBROK | TERRNO, "tst_checkpoint_wait");
> +       }
> +}
> +
> +static void fork_busy_procs_in_cgroup(const struct tst_cgroup_group *const cg)
> +{
> +       const unsigned int sleeps_ms[] = {3000, 1000, 10};
> +       const pid_t worker_pid = SAFE_FORK();
> +       size_t i;
> +
> +       if (worker_pid)
> +               return;
> +
> +       for (i = 0; i < ARRAY_SIZE(sleeps_ms); i++) {
> +               const pid_t busy_pid = SAFE_FORK();
> +
> +               if (!busy_pid)
> +                       busy_loop(sleeps_ms[i]);
> +
> +               SAFE_CGROUP_PRINTF(cg, "cgroup.procs", "%d", busy_pid);
> +       }
> +
> +       tst_reap_children();
> +
> +       exit(0);
> +}
> +
> +static void do_test(void)
> +{
> +       size_t i;
> +
> +       for (i = 0; i < ARRAY_SIZE(cg_workers); i++)
> +               fork_busy_procs_in_cgroup(cg_workers[i]);
> +
> +       tst_res(TPASS, "Scheduled bandwidth constrained workers");
> +
> +       sleep(1);
> +
> +       set_cpu_quota(cg_level2, 50);

This test itself looks good.
But I got a series of warnings when testing on CGroup V1:

# uname -r
4.18.0-296.el8.x86_64

[root@dhcp-66-83-181 cfs-scheduler]# ./cfs_bandwidth01
tst_test.c:1313: TINFO: Timeout per run is 0h 05m 00s
tst_buffers.c:55: TINFO: Test is using guarded buffers
cfs_bandwidth01.c:48: TINFO: Set 'worker1/cpu.max' = '3000 10000'
cfs_bandwidth01.c:48: TINFO: Set 'worker2/cpu.max' = '2000 10000'
cfs_bandwidth01.c:48: TINFO: Set 'worker3/cpu.max' = '3000 10000'
cfs_bandwidth01.c:111: TPASS: Scheduled bandwidth constrained workers
cfs_bandwidth01.c:42: TBROK:
vdprintf(10</sys/fs/cgroup/cpu,cpuacct/ltp/test-8450/level2>,
'cpu.cfs_quota_us', '%u'<5000>): EINVAL (22)
tst_cgroup.c:896: TWARN:
unlinkat(11</sys/fs/cgroup/cpu,cpuacct/ltp/test-8450/level2/level3a>,
'worker1', AT_REMOVEDIR): EBUSY (16)
tst_cgroup.c:896: TWARN:
unlinkat(11</sys/fs/cgroup/cpu,cpuacct/ltp/test-8450/level2/level3a>,
'worker2', AT_REMOVEDIR): EBUSY (16)
tst_cgroup.c:896: TWARN:
unlinkat(14</sys/fs/cgroup/cpu,cpuacct/ltp/test-8450/level2/level3b>,
'worker3', AT_REMOVEDIR): EBUSY (16)
tst_cgroup.c:896: TWARN:
unlinkat(10</sys/fs/cgroup/cpu,cpuacct/ltp/test-8450/level2>,
'level3a', AT_REMOVEDIR): EBUSY (16)
tst_cgroup.c:896: TWARN:
unlinkat(10</sys/fs/cgroup/cpu,cpuacct/ltp/test-8450/level2>,
'level3b', AT_REMOVEDIR): EBUSY (16)
tst_cgroup.c:896: TWARN:
unlinkat(9</sys/fs/cgroup/cpu,cpuacct/ltp/test-8450>, 'level2',
AT_REMOVEDIR): EBUSY (16)
tst_cgroup.c:766: TWARN: unlinkat(7</sys/fs/cgroup/cpu,cpuacct/ltp>,
'test-8450', AT_REMOVEDIR): EBUSY (16)

-- 
Regards,
Li Wang

[LTP] [PATCH v2 6/6] sched/cgroup: Add cfs_bandwidth01

[Richard Palethorpe]

Hello, Li,

Li Wang <liwang@redhat.com> writes:

> On Fri, May 21, 2021 at 6:26 PM Richard Palethorpe via ltp
> <ltp@lists.linux.it> wrote:
>>
>> Signed-off-by: Richard Palethorpe <rpalethorpe@suse.com>
>> ---
>>  runtest/sched                                 |   1 +
>>  .../kernel/sched/cfs-scheduler/.gitignore     |   1 +
>>  testcases/kernel/sched/cfs-scheduler/Makefile |   4 +-
>>  .../sched/cfs-scheduler/cfs_bandwidth01.c     | 175 ++++++++++++++++++
>>  4 files changed, 179 insertions(+), 2 deletions(-)
>>  create mode 100644 testcases/kernel/sched/cfs-scheduler/cfs_bandwidth01.c
>>
>> diff --git a/runtest/sched b/runtest/sched
>> index bfc4f2711..592898723 100644
>> --- a/runtest/sched
>> +++ b/runtest/sched
>> @@ -6,6 +6,7 @@ pth_str03 pth_str03
>>  time-schedule01                time-schedule
>>  trace_sched01          trace_sched -c 1
>>
>> +cfs_bandwidth01 cfs_bandwidth01 -i 5
>>  hackbench01 hackbench 50 process 1000
>>  hackbench02 hackbench 20 thread 1000
>>
>> diff --git a/testcases/kernel/sched/cfs-scheduler/.gitignore b/testcases/kernel/sched/cfs-scheduler/.gitignore
>> index db2759e4f..c5dacd6ef 100644
>> --- a/testcases/kernel/sched/cfs-scheduler/.gitignore
>> +++ b/testcases/kernel/sched/cfs-scheduler/.gitignore
>> @@ -1 +1,2 @@
>>  /hackbench
>> +cfs_bandwidth01
>> diff --git a/testcases/kernel/sched/cfs-scheduler/Makefile b/testcases/kernel/sched/cfs-scheduler/Makefile
>> index aa3bf8459..2ffe1f7f9 100644
>> --- a/testcases/kernel/sched/cfs-scheduler/Makefile
>> +++ b/testcases/kernel/sched/cfs-scheduler/Makefile
>> @@ -18,8 +18,8 @@
>>
>>  top_srcdir             ?= ../../../..
>>
>> -include $(top_srcdir)/include/mk/env_pre.mk
>> +include $(top_srcdir)/include/mk/testcases.mk
>>
>> -LDLIBS                 += -lpthread
>> +hackbench: LDLIBS                      += -lpthread
>>
>>  include $(top_srcdir)/include/mk/generic_leaf_target.mk
>> diff --git a/testcases/kernel/sched/cfs-scheduler/cfs_bandwidth01.c b/testcases/kernel/sched/cfs-scheduler/cfs_bandwidth01.c
>> new file mode 100644
>> index 000000000..7c988730e
>> --- /dev/null
>> +++ b/testcases/kernel/sched/cfs-scheduler/cfs_bandwidth01.c
>> @@ -0,0 +1,175 @@
>> +// SPDX-License-Identifier: GPL-2.0-or-later
>> +/* Copyright (c) 2021 SUSE LLC <rpalethorpe@suse.com> */
>> +/*\
>> + *
>> + * [Description]
>> + *
>> + * Creates a multi-level CGroup hierarchy with the cpu controller
>> + * enabled. The leaf groups are populated with "busy" processes which
>> + * simulate intermittent cpu load. They spin for some time then sleep
>> + * then repeat.
>> + *
>> + * Both the trunk and leaf groups are set cpu bandwidth limits. The
>> + * busy processes will intermittently exceed these limits. Causing
>> + * them to be throttled. When they begin sleeping this will then cause
>> + * them to be unthrottle.
>> + *
>> + * The test is known to reproduce an issue with an update to
>> + * SLE-15-SP1 (kernel 4.12.14-197.64, bsc#1179093).
>> + */
>> +
>> +#include <stdlib.h>
>> +
>> +#include "tst_test.h"
>> +#include "tst_cgroup.h"
>> +#include "tst_timer.h"
>> +
>> +static const struct tst_cgroup_group *cg_test;
>> +static struct tst_cgroup_group *cg_level2, *cg_level3a, *cg_level3b;
>> +static struct tst_cgroup_group *cg_workers[3];
>> +
>> +static void set_cpu_quota(const struct tst_cgroup_group *const cg,
>> +                         const float quota_percent)
>> +{
>> +       const unsigned int period_us = 10000;
>> +       const unsigned int quota_us = (quota_percent / 100) * (float)period_us;
>> +
>> +       if (TST_CGROUP_VER(cg, "cpu") != TST_CGROUP_V1) {
>> +               SAFE_CGROUP_PRINTF(cg, "cpu.max",
>> +                                  "%u %u", quota_us, period_us);
>> +       } else {
>> +               SAFE_CGROUP_PRINTF(cg, "cpu.max",
>> +                                  "%u", quota_us);
>> +               SAFE_CGROUP_PRINTF(cg, "cpu.cfs_period_us",
>> +                                 "%u", period_us);
>> +       }
>> +
>> +       tst_res(TINFO, "Set '%s/cpu.max' = '%d %d'",
>> +               tst_cgroup_group_name(cg), quota_us, period_us);
>> +}
>> +
>> +static struct tst_cgroup_group *
>> +mk_cpu_cgroup(const struct tst_cgroup_group *const cg_parent,
>> +             const char *const cg_child_name,
>> +             const float quota_percent)
>> +{
>> +       struct tst_cgroup_group *const cg =
>> +               tst_cgroup_group_mk(cg_parent, cg_child_name);
>> +
>> +       set_cpu_quota(cg, quota_percent);
>> +
>> +       return cg;
>> +}
>> +
>> +static void busy_loop(const unsigned int sleep_ms)
>> +{
>> +       for (;;) {
>> +               tst_timer_start(CLOCK_MONOTONIC_RAW);
>> +               while (!tst_timer_expired_ms(20))
>> +                       ;
>> +
>> +               const int ret = tst_checkpoint_wait(0, sleep_ms);
>> +
>> +               if (!ret)
>> +                       exit(0);
>> +
>> +               if (errno != ETIMEDOUT)
>> +                       tst_brk(TBROK | TERRNO, "tst_checkpoint_wait");
>> +       }
>> +}
>> +
>> +static void fork_busy_procs_in_cgroup(const struct tst_cgroup_group *const cg)
>> +{
>> +       const unsigned int sleeps_ms[] = {3000, 1000, 10};
>> +       const pid_t worker_pid = SAFE_FORK();
>> +       size_t i;
>> +
>> +       if (worker_pid)
>> +               return;
>> +
>> +       for (i = 0; i < ARRAY_SIZE(sleeps_ms); i++) {
>> +               const pid_t busy_pid = SAFE_FORK();
>> +
>> +               if (!busy_pid)
>> +                       busy_loop(sleeps_ms[i]);
>> +
>> +               SAFE_CGROUP_PRINTF(cg, "cgroup.procs", "%d", busy_pid);
>> +       }
>> +
>> +       tst_reap_children();
>> +
>> +       exit(0);
>> +}
>> +
>> +static void do_test(void)
>> +{
>> +       size_t i;
>> +
>> +       for (i = 0; i < ARRAY_SIZE(cg_workers); i++)
>> +               fork_busy_procs_in_cgroup(cg_workers[i]);
>> +
>> +       tst_res(TPASS, "Scheduled bandwidth constrained workers");
>> +
>> +       sleep(1);
>> +
>> +       set_cpu_quota(cg_level2, 50);
>
> This test itself looks good.
> But I got a series of warnings when testing on CGroup V1:

Thanks for testing it.

>
> # uname -r
> 4.18.0-296.el8.x86_64
>
> [root@dhcp-66-83-181 cfs-scheduler]# ./cfs_bandwidth01
> tst_test.c:1313: TINFO: Timeout per run is 0h 05m 00s
> tst_buffers.c:55: TINFO: Test is using guarded buffers
> cfs_bandwidth01.c:48: TINFO: Set 'worker1/cpu.max' = '3000 10000'
> cfs_bandwidth01.c:48: TINFO: Set 'worker2/cpu.max' = '2000 10000'
> cfs_bandwidth01.c:48: TINFO: Set 'worker3/cpu.max' = '3000 10000'
> cfs_bandwidth01.c:111: TPASS: Scheduled bandwidth constrained workers
> cfs_bandwidth01.c:42: TBROK:
> vdprintf(10</sys/fs/cgroup/cpu,cpuacct/ltp/test-8450/level2>,
> 'cpu.cfs_quota_us', '%u'<5000>): EINVAL (22)

I wonder if your kernel disallows setting this on a trunk node after it
has been set on leaf nodes (with or without procs in)?

I'm not sure whether this should be considered a fail or it is just how
older kernels work.

> tst_cgroup.c:896: TWARN:
> unlinkat(11</sys/fs/cgroup/cpu,cpuacct/ltp/test-8450/level2/level3a>,
> 'worker1', AT_REMOVEDIR): EBUSY (16)
> tst_cgroup.c:896: TWARN:
> unlinkat(11</sys/fs/cgroup/cpu,cpuacct/ltp/test-8450/level2/level3a>,
> 'worker2', AT_REMOVEDIR): EBUSY (16)
> tst_cgroup.c:896: TWARN:
> unlinkat(14</sys/fs/cgroup/cpu,cpuacct/ltp/test-8450/level2/level3b>,
> 'worker3', AT_REMOVEDIR): EBUSY (16)
> tst_cgroup.c:896: TWARN:
> unlinkat(10</sys/fs/cgroup/cpu,cpuacct/ltp/test-8450/level2>,
> 'level3a', AT_REMOVEDIR): EBUSY (16)
> tst_cgroup.c:896: TWARN:
> unlinkat(10</sys/fs/cgroup/cpu,cpuacct/ltp/test-8450/level2>,
> 'level3b', AT_REMOVEDIR): EBUSY (16)
> tst_cgroup.c:896: TWARN:
> unlinkat(9</sys/fs/cgroup/cpu,cpuacct/ltp/test-8450>, 'level2',
> AT_REMOVEDIR): EBUSY (16)
> tst_cgroup.c:766: TWARN: unlinkat(7</sys/fs/cgroup/cpu,cpuacct/ltp>,
> 'test-8450', AT_REMOVEDIR): EBUSY (16)

This happens because the child processes are still running at cleanup
because we skipped stopping them. I guess I should fix that.

-- 
Thank you,
Richard.

[LTP] [PATCH v2 6/6] sched/cgroup: Add cfs_bandwidth01

[Li Wang]

Hi Richard,

> >> +static void do_test(void)
> >> +{
> >> +       size_t i;
> >> +
> >> +       for (i = 0; i < ARRAY_SIZE(cg_workers); i++)
> >> +               fork_busy_procs_in_cgroup(cg_workers[i]);
> >> +
> >> +       tst_res(TPASS, "Scheduled bandwidth constrained workers");
> >> +
> >> +       sleep(1);
> >> +
> >> +       set_cpu_quota(cg_level2, 50);
> >
> > This test itself looks good.
> > But I got a series of warnings when testing on CGroup V1:
>
> Thanks for testing it.
>
> >
> > # uname -r
> > 4.18.0-296.el8.x86_64
> >
> > [root@dhcp-66-83-181 cfs-scheduler]# ./cfs_bandwidth01
> > tst_test.c:1313: TINFO: Timeout per run is 0h 05m 00s
> > tst_buffers.c:55: TINFO: Test is using guarded buffers
> > cfs_bandwidth01.c:48: TINFO: Set 'worker1/cpu.max' = '3000 10000'
> > cfs_bandwidth01.c:48: TINFO: Set 'worker2/cpu.max' = '2000 10000'
> > cfs_bandwidth01.c:48: TINFO: Set 'worker3/cpu.max' = '3000 10000'
> > cfs_bandwidth01.c:111: TPASS: Scheduled bandwidth constrained workers
> > cfs_bandwidth01.c:42: TBROK:
> > vdprintf(10</sys/fs/cgroup/cpu,cpuacct/ltp/test-8450/level2>,
> > 'cpu.cfs_quota_us', '%u'<5000>): EINVAL (22)
>
> I wonder if your kernel disallows setting this on a trunk node after it
> has been set on leaf nodes (with or without procs in)?

After looking a while, I think the CGrup V1 disallows the parent quota
less than the max value of its children.

This means we should set in level2 at least '3000/10000', just like what
we did for level3.

  cfs_bandwidth01.c:48: TINFO: Set 'worker1/cpu.max' = '3000 10000'
  cfs_bandwidth01.c:48: TINFO: Set 'worker2/cpu.max' = '2000 10000'
  cfs_bandwidth01.c:48: TINFO: Set 'worker3/cpu.max' = '3000 10000'

But in the failure, it shows level2 only set to 5000/100000 (far less than
3000/10000), that's because function set_cpu_quota changes the system
default value 'cpu.cfs_period_us' from 100000 to 10000.

To verify my suppose, I got all PASS when changing it back to default 100000.

--- a/testcases/kernel/sched/cfs-scheduler/cfs_bandwidth01.c
+++ b/testcases/kernel/sched/cfs-scheduler/cfs_bandwidth01.c
@@ -31,7 +31,7 @@ static struct tst_cgroup_group *cg_workers[3];
 static void set_cpu_quota(const struct tst_cgroup_group *const cg,
                          const float quota_percent)
 {
-       const unsigned int period_us = 10000;
+       const unsigned int period_us = 100000;
        const unsigned int quota_us = (quota_percent / 100) * (float)period_us;

        if (TST_CGROUP_VER(cg, "cpu") != TST_CGROUP_V1) {


# ./cfs_bandwidth01
tst_test.c:1313: TINFO: Timeout per run is 0h 05m 00s
tst_buffers.c:55: TINFO: Test is using guarded buffers
cfs_bandwidth01.c:48: TINFO: Set 'worker1/cpu.max' = '30000 100000'
cfs_bandwidth01.c:48: TINFO: Set 'worker2/cpu.max' = '20000 100000'
cfs_bandwidth01.c:48: TINFO: Set 'worker3/cpu.max' = '30000 100000'
cfs_bandwidth01.c:111: TPASS: Scheduled bandwidth constrained workers
cfs_bandwidth01.c:48: TINFO: Set 'level2/cpu.max' = '50000 100000'
cfs_bandwidth01.c:122: TPASS: Workers exited

Summary:
passed   2
failed   0
broken   0
skipped  0
warnings 0


> > unlinkat(10</sys/fs/cgroup/cpu,cpuacct/ltp/test-8450/level2>,
> > 'level3b', AT_REMOVEDIR): EBUSY (16)
> > tst_cgroup.c:896: TWARN:
> > unlinkat(9</sys/fs/cgroup/cpu,cpuacct/ltp/test-8450>, 'level2',
> > AT_REMOVEDIR): EBUSY (16)
> > tst_cgroup.c:766: TWARN: unlinkat(7</sys/fs/cgroup/cpu,cpuacct/ltp>,
> > 'test-8450', AT_REMOVEDIR): EBUSY (16)
>
> This happens because the child processes are still running at cleanup
> because we skipped stopping them. I guess I should fix that.

+1

Patchset looks good with adding the above two fixes.

Reviewed-by: Li Wang <liwang@redhat.com>

-- 
Regards,
Li Wang

[LTP] [PATCH v2 6/6] sched/cgroup: Add cfs_bandwidth01

[Li Wang]

> > > [root@dhcp-66-83-181 cfs-scheduler]# ./cfs_bandwidth01
> > > tst_test.c:1313: TINFO: Timeout per run is 0h 05m 00s
> > > tst_buffers.c:55: TINFO: Test is using guarded buffers
> > > cfs_bandwidth01.c:48: TINFO: Set 'worker1/cpu.max' = '3000 10000'
> > > cfs_bandwidth01.c:48: TINFO: Set 'worker2/cpu.max' = '2000 10000'
> > > cfs_bandwidth01.c:48: TINFO: Set 'worker3/cpu.max' = '3000 10000'
> > > cfs_bandwidth01.c:111: TPASS: Scheduled bandwidth constrained workers
> > > cfs_bandwidth01.c:42: TBROK:
> > > vdprintf(10</sys/fs/cgroup/cpu,cpuacct/ltp/test-8450/level2>,
> > > 'cpu.cfs_quota_us', '%u'<5000>): EINVAL (22)
> >
> > I wonder if your kernel disallows setting this on a trunk node after it
> > has been set on leaf nodes (with or without procs in)?
>
> After looking a while, I think the CGrup V1 disallows the parent quota
> less than the max value of its children.
>
> This means we should set in level2 at least '3000/10000', just like what
> we did for level3.
>
>   cfs_bandwidth01.c:48: TINFO: Set 'worker1/cpu.max' = '3000 10000'
>   cfs_bandwidth01.c:48: TINFO: Set 'worker2/cpu.max' = '2000 10000'
>   cfs_bandwidth01.c:48: TINFO: Set 'worker3/cpu.max' = '3000 10000'
>
> But in the failure, it shows level2 only set to 5000/100000 (far less than
> 3000/10000), that's because function set_cpu_quota changes the system
> default value 'cpu.cfs_period_us' from 100000 to 10000.

Or, just reverse the code order to set cfs_period_us first, that also works.

--- a/testcases/kernel/sched/cfs-scheduler/cfs_bandwidth01.c
+++ b/testcases/kernel/sched/cfs-scheduler/cfs_bandwidth01.c
@@ -38,10 +38,10 @@ static void set_cpu_quota(const struct
tst_cgroup_group *const cg,
                SAFE_CGROUP_PRINTF(cg, "cpu.max",
                                   "%u %u", quota_us, period_us);
        } else {
-               SAFE_CGROUP_PRINTF(cg, "cpu.max",
-                                  "%u", quota_us);
                SAFE_CGROUP_PRINTF(cg, "cpu.cfs_period_us",
                                  "%u", period_us);
+               SAFE_CGROUP_PRINTF(cg, "cpu.max",
+                                  "%u", quota_us);
        }


-- 
Regards,
Li Wang

[LTP] [PATCH v2 6/6] sched/cgroup: Add cfs_bandwidth01

[Richard Palethorpe]

Hello Li,

Li Wang <liwang@redhat.com> writes:

>> > > [root@dhcp-66-83-181 cfs-scheduler]# ./cfs_bandwidth01
>> > > tst_test.c:1313: TINFO: Timeout per run is 0h 05m 00s
>> > > tst_buffers.c:55: TINFO: Test is using guarded buffers
>> > > cfs_bandwidth01.c:48: TINFO: Set 'worker1/cpu.max' = '3000 10000'
>> > > cfs_bandwidth01.c:48: TINFO: Set 'worker2/cpu.max' = '2000 10000'
>> > > cfs_bandwidth01.c:48: TINFO: Set 'worker3/cpu.max' = '3000 10000'
>> > > cfs_bandwidth01.c:111: TPASS: Scheduled bandwidth constrained workers
>> > > cfs_bandwidth01.c:42: TBROK:
>> > > vdprintf(10</sys/fs/cgroup/cpu,cpuacct/ltp/test-8450/level2>,
>> > > 'cpu.cfs_quota_us', '%u'<5000>): EINVAL (22)
>> >
>> > I wonder if your kernel disallows setting this on a trunk node after it
>> > has been set on leaf nodes (with or without procs in)?
>>
>> After looking a while, I think the CGrup V1 disallows the parent quota
>> less than the max value of its children.
>>
>> This means we should set in level2 at least '3000/10000', just like what
>> we did for level3.
>>
>>   cfs_bandwidth01.c:48: TINFO: Set 'worker1/cpu.max' = '3000 10000'
>>   cfs_bandwidth01.c:48: TINFO: Set 'worker2/cpu.max' = '2000 10000'
>>   cfs_bandwidth01.c:48: TINFO: Set 'worker3/cpu.max' = '3000 10000'
>>
>> But in the failure, it shows level2 only set to 5000/100000 (far less than
>> 3000/10000), that's because function set_cpu_quota changes the system
>> default value 'cpu.cfs_period_us' from 100000 to 10000.
>
> Or, just reverse the code order to set cfs_period_us first, that also works.
>
> --- a/testcases/kernel/sched/cfs-scheduler/cfs_bandwidth01.c
> +++ b/testcases/kernel/sched/cfs-scheduler/cfs_bandwidth01.c
> @@ -38,10 +38,10 @@ static void set_cpu_quota(const struct
> tst_cgroup_group *const cg,
>                 SAFE_CGROUP_PRINTF(cg, "cpu.max",
>                                    "%u %u", quota_us, period_us);
>         } else {
> -               SAFE_CGROUP_PRINTF(cg, "cpu.max",
> -                                  "%u", quota_us);
>                 SAFE_CGROUP_PRINTF(cg, "cpu.cfs_period_us",
>                                   "%u", period_us);
> +               SAFE_CGROUP_PRINTF(cg, "cpu.max",
> +                                  "%u", quota_us);
>         }

Thanks, that is a nice fix.


-- 
Thank you,
Richard.