[LTP] [PATCH v8 2/2] tst_cgroup.c: Add a cgroup pseudo controller
Li Wang
liwang@redhat.com
Fri Apr 21 06:33:31 CEST 2023
On Fri, Apr 21, 2023 at 9:27 AM Wei Gao via ltp <ltp@lists.linux.it> wrote:
> For new test case such as kill01.c no need specific controller, it just
> need LTP cgroup library start work, so we need add a "cgroup" pseudo
> controller.
>
> Signed-off-by: Wei Gao <wegao@suse.com>
> ---
> lib/tst_cgroup.c | 54 +++++++++++++++++++++++++++++++-----------------
> 1 file changed, 35 insertions(+), 19 deletions(-)
>
> diff --git a/lib/tst_cgroup.c b/lib/tst_cgroup.c
> index 77575431d..6a70bf4b4 100644
> --- a/lib/tst_cgroup.c
> +++ b/lib/tst_cgroup.c
> @@ -94,9 +94,10 @@ enum cgroup_ctrl_indx {
> CTRL_MISC,
> CTRL_PERFEVENT,
> CTRL_DEBUG,
> - CTRL_RDMA
> + CTRL_RDMA,
> + CTRL_PSEUDO
> };
> -#define CTRLS_MAX CTRL_RDMA
> +#define CTRLS_MAX CTRL_PSEUDO
>
> /* At most we can have one cgroup V1 tree for each controller and one
> * (empty) v2 tree.
> @@ -259,6 +260,10 @@ static const struct cgroup_file rdma_ctrl_files[] = {
> { }
> };
>
> +static const struct cgroup_file cgroup_pseudo_ctrl_files[] = {
>
As the controller defined CTRL_PSEUDO here may be better to
use pseudo_ctrl_files without "cgroup_" prefix.
> + { }
> +};
> +
> #define CTRL_NAME_MAX 31
> #define CGROUP_CTRL_MEMBER(x, y)[y] = { .ctrl_name = #x, .files = \
> x ## _ctrl_files, .ctrl_indx = y, NULL, 0 }
> @@ -282,6 +287,7 @@ static struct cgroup_ctrl controllers[] = {
> CGROUP_CTRL_MEMBER(perf_event, CTRL_PERFEVENT),
> CGROUP_CTRL_MEMBER(debug, CTRL_DEBUG),
> CGROUP_CTRL_MEMBER(rdma, CTRL_RDMA),
> + CGROUP_CTRL_MEMBER(cgroup_pseudo, CTRL_PSEUDO),
>
use pseudo directly without "cgroup_" prefix.
> { }
> };
>
> @@ -798,6 +804,7 @@ void tst_cg_require(const char *const ctrl_name,
> const char *const cgsc = "cgroup.subtree_control";
> struct cgroup_ctrl *const ctrl = cgroup_find_ctrl(ctrl_name);
> struct cgroup_root *root;
> + int cgroup_pseudo = !strcmp(ctrl->ctrl_name, "cgroup_pseudo");
>
int cgroup_pseudo = !strcmp(ctrl->ctrl_name, "pseudo");
>
> if (!ctrl) {
> tst_brk(TBROK, "'%s' controller is unknown to LTP",
> ctrl_name);
> @@ -827,6 +834,9 @@ void tst_cg_require(const char *const ctrl_name,
> if (options->needs_ver != TST_CG_V2)
> cgroup_mount_v1(ctrl);
>
> + if (cgroup_pseudo)
> + ctrl->ctrl_root = roots;
> +
> if (!ctrl->ctrl_root) {
> tst_brk(TCONF,
> "'%s' controller required, but not available",
> @@ -849,13 +859,15 @@ mkdirs:
> ctrl->ctrl_name);
> }
>
> - if (cgroup_ctrl_on_v2(ctrl)) {
> - if (root->we_mounted_it) {
> - SAFE_FILE_PRINTFAT(root->mnt_dir.dir_fd,
> - cgsc, "+%s", ctrl->ctrl_name);
> - } else {
> - tst_file_printfat(root->mnt_dir.dir_fd,
> - cgsc, "+%s", ctrl->ctrl_name);
> + if (!cgroup_pseudo) {
> + if (cgroup_ctrl_on_v2(ctrl)) {
>
This makes things more complicated, what about:
if (cgroup_ctrl_on_v2(ctrl) && !cgroup_pseudo) {
to reduce one nested.
> + if (root->we_mounted_it) {
> + SAFE_FILE_PRINTFAT(root->mnt_dir.dir_fd,
> + cgsc, "+%s",
> ctrl->ctrl_name);
> + } else {
> + tst_file_printfat(root->mnt_dir.dir_fd,
> + cgsc, "+%s",
> ctrl->ctrl_name);
> + }
> }
> }
>
> @@ -864,15 +876,17 @@ mkdirs:
> else
> root->ltp_dir.ctrl_field |= root->mnt_dir.ctrl_field;
>
> - if (cgroup_ctrl_on_v2(ctrl)) {
> - SAFE_FILE_PRINTFAT(root->ltp_dir.dir_fd,
> - cgsc, "+%s", ctrl->ctrl_name);
> - } else {
> - SAFE_FILE_PRINTFAT(root->ltp_dir.dir_fd,
> - "cgroup.clone_children", "%d", 1);
> + if (!cgroup_pseudo) {
> + if (cgroup_ctrl_on_v2(ctrl)) {
>
if (cgroup_ctrl_on_v2(ctrl) && !cgroup_pseudo) {
> + SAFE_FILE_PRINTFAT(root->ltp_dir.dir_fd,
> + cgsc, "+%s", ctrl->ctrl_name);
> + } else {
> + SAFE_FILE_PRINTFAT(root->ltp_dir.dir_fd,
> + "cgroup.clone_children", "%d", 1);
>
> - if (ctrl->ctrl_indx == CTRL_CPUSET)
> - cgroup_copy_cpuset(root);
> + if (ctrl->ctrl_indx == CTRL_CPUSET)
> + cgroup_copy_cpuset(root);
> + }
> }
>
> cgroup_dir_mk(&root->ltp_dir, cgroup_ltp_drain_dir,
> &root->drain_dir);
> @@ -1050,8 +1064,10 @@ static void cgroup_group_add_dir(const struct
> tst_cg_group *const parent,
> if (!parent || dir->dir_root->ver == TST_CG_V1)
> continue;
>
> - SAFE_CG_PRINTF(parent, "cgroup.subtree_control",
> - "+%s", ctrl->ctrl_name);
> + if (strcmp(ctrl->ctrl_name, "cgroup_pseudo")) {
>
Remove "cgroup_" prefix.
> + SAFE_CG_PRINTF(parent, "cgroup.subtree_control",
> + "+%s", ctrl->ctrl_name);
> + }
> }
>
> for (i = 0; cg->dirs[i]; i++)
> --
> 2.35.3
>
>
> --
> Mailing list info: https://lists.linux.it/listinfo/ltp
>
> Other two issues we have to resolve:
1. We'd better guarantee the "pseudo" controller is safely ignored by
Cgroup_V1 in case of '.needs_cgroup_ver = TST_CG_V1'.
Because cgroup_mount_v1(ctrl) will try to mount a 'pseudo' controller
once a system without any available (V1 and V2) ctrls, which ultimately
caused the test to break.
2. Assume the test running on a CgroupV1 supported system, which will
try to mount CgroupV2 because pseudo is not detected exist in V1
controllers, so it by default thinks we need to mount CgroupV2 and
test there.
But the fact is that CgroupV1-only system is not fully supported CgroupV2,
even if we mount the V2 successfully the function is limited and this
ultimately
uses a lack of cgroup.kill file and failed again.
--
Regards,
Li Wang
More information about the ltp
mailing list