* [LTP] [PATCH v2 0/6] cfs_bandwidth01 and CGroup API
@ 2021-05-21 10:25 Richard Palethorpe
2021-05-21 10:25 ` [LTP] [PATCH v2 1/6] API/cgroups: Allow fetching of CGroup name Richard Palethorpe
` (5 more replies)
0 siblings, 6 replies; 15+ messages in thread
From: Richard Palethorpe @ 2021-05-21 10:25 UTC (permalink / raw)
To: ltp
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
^ permalink raw reply [flat|nested] 15+ messages in thread
* [LTP] [PATCH v2 1/6] API/cgroups: Allow fetching of CGroup name
2021-05-21 10:25 [LTP] [PATCH v2 0/6] cfs_bandwidth01 and CGroup API Richard Palethorpe
@ 2021-05-21 10:25 ` Richard Palethorpe
2021-05-21 10:25 ` [LTP] [PATCH v2 2/6] API/cgroups: Remove obsolete function in API Richard Palethorpe
` (4 subsequent siblings)
5 siblings, 0 replies; 15+ messages in thread
From: Richard Palethorpe @ 2021-05-21 10:25 UTC (permalink / raw)
To: ltp
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
^ permalink raw reply related [flat|nested] 15+ messages in thread
* [LTP] [PATCH v2 2/6] API/cgroups: Remove obsolete function in API
2021-05-21 10:25 [LTP] [PATCH v2 0/6] cfs_bandwidth01 and CGroup API Richard Palethorpe
2021-05-21 10:25 ` [LTP] [PATCH v2 1/6] API/cgroups: Allow fetching of CGroup name Richard Palethorpe
@ 2021-05-21 10:25 ` Richard Palethorpe
2021-05-21 10:25 ` [LTP] [PATCH v2 3/6] API/cgroups: Add cpu controller Richard Palethorpe
` (3 subsequent siblings)
5 siblings, 0 replies; 15+ messages in thread
From: Richard Palethorpe @ 2021-05-21 10:25 UTC (permalink / raw)
To: ltp
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
^ permalink raw reply related [flat|nested] 15+ messages in thread
* [LTP] [PATCH v2 3/6] API/cgroups: Add cpu controller
2021-05-21 10:25 [LTP] [PATCH v2 0/6] cfs_bandwidth01 and CGroup API Richard Palethorpe
2021-05-21 10:25 ` [LTP] [PATCH v2 1/6] API/cgroups: Allow fetching of CGroup name Richard Palethorpe
2021-05-21 10:25 ` [LTP] [PATCH v2 2/6] API/cgroups: Remove obsolete function in API Richard Palethorpe
@ 2021-05-21 10:25 ` Richard Palethorpe
2021-05-21 10:25 ` [LTP] [PATCH v2 4/6] API/cgroups: Auto add controllers to subtree_control in new subgroup Richard Palethorpe
` (2 subsequent siblings)
5 siblings, 0 replies; 15+ messages in thread
From: Richard Palethorpe @ 2021-05-21 10:25 UTC (permalink / raw)
To: ltp
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
^ permalink raw reply related [flat|nested] 15+ messages in thread
* [LTP] [PATCH v2 4/6] API/cgroups: Auto add controllers to subtree_control in new subgroup
2021-05-21 10:25 [LTP] [PATCH v2 0/6] cfs_bandwidth01 and CGroup API Richard Palethorpe
` (2 preceding siblings ...)
2021-05-21 10:25 ` [LTP] [PATCH v2 3/6] API/cgroups: Add cpu controller Richard Palethorpe
@ 2021-05-21 10:25 ` Richard Palethorpe
2021-05-21 10:25 ` [LTP] [PATCH v2 5/6] API/cgroups: tst_require fail gracefully with unknown controller Richard Palethorpe
2021-05-21 10:25 ` [LTP] [PATCH v2 6/6] sched/cgroup: Add cfs_bandwidth01 Richard Palethorpe
5 siblings, 0 replies; 15+ messages in thread
From: Richard Palethorpe @ 2021-05-21 10:25 UTC (permalink / raw)
To: ltp
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
^ permalink raw reply related [flat|nested] 15+ messages in thread
* [LTP] [PATCH v2 5/6] API/cgroups: tst_require fail gracefully with unknown controller
2021-05-21 10:25 [LTP] [PATCH v2 0/6] cfs_bandwidth01 and CGroup API Richard Palethorpe
` (3 preceding siblings ...)
2021-05-21 10:25 ` [LTP] [PATCH v2 4/6] API/cgroups: Auto add controllers to subtree_control in new subgroup Richard Palethorpe
@ 2021-05-21 10:25 ` Richard Palethorpe
2021-05-27 13:18 ` Li Wang
2021-05-21 10:25 ` [LTP] [PATCH v2 6/6] sched/cgroup: Add cfs_bandwidth01 Richard Palethorpe
5 siblings, 1 reply; 15+ messages in thread
From: Richard Palethorpe @ 2021-05-21 10:25 UTC (permalink / raw)
To: ltp
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
^ permalink raw reply related [flat|nested] 15+ messages in thread
* [LTP] [PATCH v2 6/6] sched/cgroup: Add cfs_bandwidth01
2021-05-21 10:25 [LTP] [PATCH v2 0/6] cfs_bandwidth01 and CGroup API Richard Palethorpe
` (4 preceding siblings ...)
2021-05-21 10:25 ` [LTP] [PATCH v2 5/6] API/cgroups: tst_require fail gracefully with unknown controller Richard Palethorpe
@ 2021-05-21 10:25 ` Richard Palethorpe
2021-05-27 13:26 ` Li Wang
5 siblings, 1 reply; 15+ messages in thread
From: Richard Palethorpe @ 2021-05-21 10:25 UTC (permalink / raw)
To: ltp
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
^ permalink raw reply related [flat|nested] 15+ messages in thread
* [LTP] [PATCH v2 5/6] API/cgroups: tst_require fail gracefully with unknown controller
2021-05-21 10:25 ` [LTP] [PATCH v2 5/6] API/cgroups: tst_require fail gracefully with unknown controller Richard Palethorpe
@ 2021-05-27 13:18 ` Li Wang
2021-05-27 15:14 ` Richard Palethorpe
0 siblings, 1 reply; 15+ messages in thread
From: Li Wang @ 2021-05-27 13:18 UTC (permalink / raw)
To: ltp
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
^ permalink raw reply [flat|nested] 15+ messages in thread
* [LTP] [PATCH v2 6/6] sched/cgroup: Add cfs_bandwidth01
2021-05-21 10:25 ` [LTP] [PATCH v2 6/6] sched/cgroup: Add cfs_bandwidth01 Richard Palethorpe
@ 2021-05-27 13:26 ` Li Wang
2021-05-28 9:37 ` Richard Palethorpe
0 siblings, 1 reply; 15+ messages in thread
From: Li Wang @ 2021-05-27 13:26 UTC (permalink / raw)
To: ltp
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
^ permalink raw reply [flat|nested] 15+ messages in thread
* [LTP] [PATCH v2 5/6] API/cgroups: tst_require fail gracefully with unknown controller
2021-05-27 13:18 ` Li Wang
@ 2021-05-27 15:14 ` Richard Palethorpe
2021-05-28 8:22 ` Li Wang
0 siblings, 1 reply; 15+ messages in thread
From: Richard Palethorpe @ 2021-05-27 15:14 UTC (permalink / raw)
To: ltp
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.
^ permalink raw reply [flat|nested] 15+ messages in thread
* [LTP] [PATCH v2 5/6] API/cgroups: tst_require fail gracefully with unknown controller
2021-05-27 15:14 ` Richard Palethorpe
@ 2021-05-28 8:22 ` Li Wang
0 siblings, 0 replies; 15+ messages in thread
From: Li Wang @ 2021-05-28 8:22 UTC (permalink / raw)
To: ltp
> >> + 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
^ permalink raw reply [flat|nested] 15+ messages in thread
* [LTP] [PATCH v2 6/6] sched/cgroup: Add cfs_bandwidth01
2021-05-27 13:26 ` Li Wang
@ 2021-05-28 9:37 ` Richard Palethorpe
2021-05-31 5:33 ` Li Wang
0 siblings, 1 reply; 15+ messages in thread
From: Richard Palethorpe @ 2021-05-28 9:37 UTC (permalink / raw)
To: ltp
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.
^ permalink raw reply [flat|nested] 15+ messages in thread
* [LTP] [PATCH v2 6/6] sched/cgroup: Add cfs_bandwidth01
2021-05-28 9:37 ` Richard Palethorpe
@ 2021-05-31 5:33 ` Li Wang
2021-05-31 6:02 ` Li Wang
0 siblings, 1 reply; 15+ messages in thread
From: Li Wang @ 2021-05-31 5:33 UTC (permalink / raw)
To: ltp
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
^ permalink raw reply [flat|nested] 15+ messages in thread
* [LTP] [PATCH v2 6/6] sched/cgroup: Add cfs_bandwidth01
2021-05-31 5:33 ` Li Wang
@ 2021-05-31 6:02 ` Li Wang
2021-06-01 10:42 ` Richard Palethorpe
0 siblings, 1 reply; 15+ messages in thread
From: Li Wang @ 2021-05-31 6:02 UTC (permalink / raw)
To: ltp
> > > [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
^ permalink raw reply [flat|nested] 15+ messages in thread
* [LTP] [PATCH v2 6/6] sched/cgroup: Add cfs_bandwidth01
2021-05-31 6:02 ` Li Wang
@ 2021-06-01 10:42 ` Richard Palethorpe
0 siblings, 0 replies; 15+ messages in thread
From: Richard Palethorpe @ 2021-06-01 10:42 UTC (permalink / raw)
To: ltp
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.
^ permalink raw reply [flat|nested] 15+ messages in thread
end of thread, other threads:[~2021-06-01 10:42 UTC | newest]
Thread overview: 15+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-05-21 10:25 [LTP] [PATCH v2 0/6] cfs_bandwidth01 and CGroup API Richard Palethorpe
2021-05-21 10:25 ` [LTP] [PATCH v2 1/6] API/cgroups: Allow fetching of CGroup name Richard Palethorpe
2021-05-21 10:25 ` [LTP] [PATCH v2 2/6] API/cgroups: Remove obsolete function in API Richard Palethorpe
2021-05-21 10:25 ` [LTP] [PATCH v2 3/6] API/cgroups: Add cpu controller Richard Palethorpe
2021-05-21 10:25 ` [LTP] [PATCH v2 4/6] API/cgroups: Auto add controllers to subtree_control in new subgroup Richard Palethorpe
2021-05-21 10:25 ` [LTP] [PATCH v2 5/6] API/cgroups: tst_require fail gracefully with unknown controller Richard Palethorpe
2021-05-27 13:18 ` Li Wang
2021-05-27 15:14 ` Richard Palethorpe
2021-05-28 8:22 ` Li Wang
2021-05-21 10:25 ` [LTP] [PATCH v2 6/6] sched/cgroup: Add cfs_bandwidth01 Richard Palethorpe
2021-05-27 13:26 ` Li Wang
2021-05-28 9:37 ` Richard Palethorpe
2021-05-31 5:33 ` Li Wang
2021-05-31 6:02 ` Li Wang
2021-06-01 10:42 ` Richard Palethorpe
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.