[LTP] [PATCH v2 2/5] API/cgroup: Declare required controllers and version in test struct
Li Wang
liwang@redhat.com
Fri Feb 4 08:37:09 CET 2022
Hi Richard,
This is a good improvement for the CG API. Nice work!!
On Thu, Feb 3, 2022 at 4:20 PM Richard Palethorpe via ltp <
ltp@lists.linux.it> wrote:
> ...
--- a/include/tst_test.h
> +++ b/include/tst_test.h
> @@ -133,6 +133,14 @@ extern unsigned int tst_variant;
>
> #define TST_NO_HUGEPAGES ((unsigned long)-1)
>
> +/* CGroups Kernel API version */
> +enum tst_cgroup_ver {
> + TST_CGROUP_V1 = 1,
> + TST_CGROUP_V2 = 2,
> +};
>
We can move this into 'tst_cgroup.h' if included that header in tst_test.h.
(As we decide to integrate the CG structure in tst_test, it
seems better to include that tst_cgorup.h file though that
makes some binaries become larger.)
+
> +struct tst_cgroup_group;
>
As Cyril pointed out, this is useless.
+
> struct tst_test {
> /* number of tests available in test() function */
> unsigned int tcnt;
> @@ -280,6 +288,12 @@ struct tst_test {
>
> /* NULL terminated array of required commands */
> const char *const *needs_cmds;
> +
> + /* Requires a particular CGroup API version. */
> + const enum tst_cgroup_ver needs_cgroup_ver;
> +
> + /* {} terminated array of required CGroup controllers */
> + const char *const *needs_cgroup_controllers;
>
Can we use abbreviations for both? just to keep them consistently :).
const enum tst_cgroup_ver needs_cgroup_ver;
const char *const *needs_cgroup_ctrls;
or,
const enum tst_cgroup_ver needs_cgroup_version;
const char *const *needs_cgroup_controllers;
> };
>
> /*
> diff --git a/lib/newlib_tests/tst_cgroup01.c
> b/lib/newlib_tests/tst_cgroup01.c
> index 54a370362..62df9aab2 100644
> --- a/lib/newlib_tests/tst_cgroup01.c
> +++ b/lib/newlib_tests/tst_cgroup01.c
> @@ -22,7 +22,7 @@ static void do_test(void)
>
> static void setup(void)
> {
> - cgopts.only_mount_v1 = !!only_mount_v1,
> + cgopts.needs_ver = !!only_mount_v1 ? TST_CGROUP_V1 : 0;
>
> tst_cgroup_scan();
> tst_cgroup_print_config();
> diff --git a/lib/newlib_tests/tst_cgroup02.c
> b/lib/newlib_tests/tst_cgroup02.c
> index 64b0a1e94..d048c720a 100644
> --- a/lib/newlib_tests/tst_cgroup02.c
> +++ b/lib/newlib_tests/tst_cgroup02.c
> @@ -15,8 +15,6 @@ static struct tst_option opts[] = {
> {NULL, NULL, NULL},
> };
> static struct tst_cgroup_opts cgopts;
> -static const struct tst_cgroup_group *cg;
> -static const struct tst_cgroup_group *cg_drain;
> static struct tst_cgroup_group *cg_child;
>
> static void do_test(void)
> @@ -24,12 +22,12 @@ static void do_test(void)
> char buf[BUFSIZ];
> size_t mem;
>
> - if (!TST_CGROUP_VER_IS_V1(cg, "memory"))
> - SAFE_CGROUP_PRINT(cg, "cgroup.subtree_control", "+memory");
> - if (!TST_CGROUP_VER_IS_V1(cg, "cpuset"))
> - SAFE_CGROUP_PRINT(cg, "cgroup.subtree_control", "+cpuset");
> + if (!TST_CGROUP_VER_IS_V1(tst_cgroup, "memory"))
> + SAFE_CGROUP_PRINT(tst_cgroup, "cgroup.subtree_control",
> "+memory");
> + if (!TST_CGROUP_VER_IS_V1(tst_cgroup, "cpuset"))
> + SAFE_CGROUP_PRINT(tst_cgroup, "cgroup.subtree_control",
> "+cpuset");
>
> - cg_child = tst_cgroup_group_mk(cg, "child");
> + cg_child = tst_cgroup_group_mk(tst_cgroup, "child");
> if (!SAFE_FORK()) {
> SAFE_CGROUP_PRINTF(cg_child, "cgroup.procs", "%d",
> getpid());
>
> @@ -47,19 +45,19 @@ static void do_test(void)
> exit(0);
> }
>
> - SAFE_CGROUP_PRINTF(cg, "memory.max", "%zu", (1UL << 24) - 1);
> + SAFE_CGROUP_PRINTF(tst_cgroup, "memory.max", "%zu", (1UL << 24) -
> 1);
> SAFE_CGROUP_PRINTF(cg_child, "cgroup.procs", "%d", getpid());
> - SAFE_CGROUP_SCANF(cg, "memory.current", "%zu", &mem);
> + SAFE_CGROUP_SCANF(tst_cgroup, "memory.current", "%zu", &mem);
> tst_res(TPASS, "memory.current = %zu", mem);
>
> tst_reap_children();
> - SAFE_CGROUP_PRINTF(cg_drain, "cgroup.procs", "%d", getpid());
> + SAFE_CGROUP_PRINTF(tst_cgroup_drain, "cgroup.procs", "%d",
> getpid());
> cg_child = tst_cgroup_group_rm(cg_child);
> }
>
> static void setup(void)
> {
> - cgopts.only_mount_v1 = !!only_mount_v1,
> + cgopts.needs_ver = !!only_mount_v1 ? TST_CGROUP_V1 : 0;
>
> tst_cgroup_scan();
> tst_cgroup_print_config();
> @@ -67,14 +65,14 @@ static void setup(void)
> tst_cgroup_require("memory", &cgopts);
> tst_cgroup_require("cpuset", &cgopts);
>
> - cg = tst_cgroup_get_test_group();
> - cg_drain = tst_cgroup_get_drain_group();
> + tst_cgroup_init();
> }
>
> static void cleanup(void)
> {
> if (cg_child) {
> - SAFE_CGROUP_PRINTF(cg_drain, "cgroup.procs", "%d",
> getpid());
> + SAFE_CGROUP_PRINTF(tst_cgroup_drain,
> + "cgroup.procs", "%d", getpid());
> cg_child = tst_cgroup_group_rm(cg_child);
> }
> if (!no_cleanup)
> diff --git a/lib/tst_cgroup.c b/lib/tst_cgroup.c
> index 10b65364b..e694d353b 100644
> --- a/lib/tst_cgroup.c
> +++ b/lib/tst_cgroup.c
> @@ -138,6 +138,14 @@ struct tst_cgroup_group {
> struct cgroup_dir *dirs[ROOTS_MAX + 1];
> };
>
> +/* If controllers are required via the tst_test struct then this is
> + * populated with the test's CGroup.
> + */
> +static struct tst_cgroup_group test_group;
> +static struct tst_cgroup_group drain_group;
> +const struct tst_cgroup_group *const tst_cgroup = &test_group;
> +const struct tst_cgroup_group *const tst_cgroup_drain = &drain_group;
> +
> /* Always use first item for unified hierarchy */
> static struct cgroup_root roots[ROOTS_MAX + 1];
>
> @@ -196,7 +204,7 @@ static struct cgroup_ctrl controllers[] = {
> { }
> };
>
> -static const struct tst_cgroup_opts default_opts = { 0 };
> +static const struct tst_cgroup_opts default_opts;
>
I'm wondering is there still a need this 'default_opts'?
Since we have defined 'cg_opts' in do_cgroup_requires for the
whole library. So that 'default_opts' is redundant from now on.
--
Regards,
Li Wang
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.linux.it/pipermail/ltp/attachments/20220204/2c3c9c3f/attachment-0001.htm>
More information about the ltp
mailing list