| Message ID | 20230430074811.29555-2-wegao@suse.com |
|---|---|
| State | Accepted |
| Headers | show |
| Series | cgroup_core03.c: New case test cgroup kill feature | expand |
Hi Wei,
Seems we have to skip the cgroup.subtree_control cleanup
process inside tst_cg_cleanup(), otherwise, test complains that
there is error operation in line#1020 if the unified cgroup-V2
was mounted by LTP.
(no need to resend another patch, someone who merge this can help fix.)
--- a/lib/tst_cgroup.c
+++ b/lib/tst_cgroup.c
@@ -1012,7 +1012,8 @@ void tst_cg_cleanup(void)
}
for_each_ctrl(ctrl) {
- if (!cgroup_ctrl_on_v2(ctrl) ||
!ctrl->ctrl_root->we_mounted_it)
+ if (!cgroup_ctrl_on_v2(ctrl) ||
!ctrl->ctrl_root->we_mounted_it
+ || !strcmp(ctrl->ctrl_name, "base"))
continue;
SAFE_FILE_PRINTFAT(ctrl->ctrl_root->mnt_dir.dir_fd,
With above fix:
Reviewed-by: Li Wang <liwang@redhat.com>
On Sun, Apr 30, 2023 at 09:44:08PM +0800, Li Wang wrote: > Hi Wei, > > Seems we have to skip the cgroup.subtree_control cleanup > process inside tst_cg_cleanup(), otherwise, test complains that > there is error operation in line#1020 if the unified cgroup-V2 > was mounted by LTP. > > (no need to resend another patch, someone who merge this can help fix.) Thanks a lot :) > > --- a/lib/tst_cgroup.c > +++ b/lib/tst_cgroup.c > @@ -1012,7 +1012,8 @@ void tst_cg_cleanup(void) > } > > for_each_ctrl(ctrl) { > - if (!cgroup_ctrl_on_v2(ctrl) || > !ctrl->ctrl_root->we_mounted_it) > + if (!cgroup_ctrl_on_v2(ctrl) || > !ctrl->ctrl_root->we_mounted_it > + || !strcmp(ctrl->ctrl_name, "base")) > continue; > > SAFE_FILE_PRINTFAT(ctrl->ctrl_root->mnt_dir.dir_fd, > > With above fix: > Reviewed-by: Li Wang <liwang@redhat.com> > > > -- > Regards, > Li Wang
Hi Wei, Li, > Hi Wei, > Seems we have to skip the cgroup.subtree_control cleanup > process inside tst_cg_cleanup(), otherwise, test complains that > there is error operation in line#1020 if the unified cgroup-V2 > was mounted by LTP. > (no need to resend another patch, someone who merge this can help fix.) > --- a/lib/tst_cgroup.c > +++ b/lib/tst_cgroup.c > @@ -1012,7 +1012,8 @@ void tst_cg_cleanup(void) > } > for_each_ctrl(ctrl) { > - if (!cgroup_ctrl_on_v2(ctrl) || > !ctrl->ctrl_root->we_mounted_it) > + if (!cgroup_ctrl_on_v2(ctrl) || > !ctrl->ctrl_root->we_mounted_it > + || !strcmp(ctrl->ctrl_name, "base")) > continue; Agree. Reviewed-by: Petr Vorel <pvorel@suse.cz> Kind regards, Petr > SAFE_FILE_PRINTFAT(ctrl->ctrl_root->mnt_dir.dir_fd, > With above fix:
Hi Li, Wei, > Hi Wei, > Seems we have to skip the cgroup.subtree_control cleanup > process inside tst_cg_cleanup(), otherwise, test complains that > there is error operation in line#1020 if the unified cgroup-V2 > was mounted by LTP. > (no need to resend another patch, someone who merge this can help fix.) > --- a/lib/tst_cgroup.c > +++ b/lib/tst_cgroup.c > @@ -1012,7 +1012,8 @@ void tst_cg_cleanup(void) > } > for_each_ctrl(ctrl) { > - if (!cgroup_ctrl_on_v2(ctrl) || > !ctrl->ctrl_root->we_mounted_it) > + if (!cgroup_ctrl_on_v2(ctrl) || > !ctrl->ctrl_root->we_mounted_it > + || !strcmp(ctrl->ctrl_name, "base")) > continue; > SAFE_FILE_PRINTFAT(ctrl->ctrl_root->mnt_dir.dir_fd, Merged with this change. Thanks to both! Kind regards, Petr
diff --git a/lib/tst_cgroup.c b/lib/tst_cgroup.c index 50699bc63..249f25aa4 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_BASE }; -#define CTRLS_MAX CTRL_RDMA +#define CTRLS_MAX CTRL_BASE /* At most we can have one cgroup V1 tree for each controller and one * (empty) v2 tree. @@ -258,6 +259,10 @@ static const struct cgroup_file rdma_ctrl_files[] = { { } }; +static const struct cgroup_file base_ctrl_files[] = { + { } +}; + #define CTRL_NAME_MAX 31 #define CGROUP_CTRL_MEMBER(x, y)[y] = { .ctrl_name = #x, .files = \ x ## _ctrl_files, .ctrl_indx = y, NULL, 0 } @@ -281,6 +286,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(base, CTRL_BASE), { } }; @@ -797,6 +803,10 @@ 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 base = !strcmp(ctrl->ctrl_name, "base"); + + if (base && options->needs_ver != TST_CG_V2) + tst_brk(TCONF, "Base control only support needs_ver TST_CG_V2!"); if (!ctrl) { tst_brk(TBROK, "'%s' controller is unknown to LTP", ctrl_name); @@ -826,6 +836,9 @@ void tst_cg_require(const char *const ctrl_name, if (options->needs_ver != TST_CG_V2) cgroup_mount_v1(ctrl); + if (base) + ctrl->ctrl_root = roots; + if (!ctrl->ctrl_root) { tst_brk(TCONF, "'%s' controller required, but not available", @@ -848,7 +861,7 @@ mkdirs: ctrl->ctrl_name); } - if (cgroup_ctrl_on_v2(ctrl)) { + if (cgroup_ctrl_on_v2(ctrl) && !base) { if (root->we_mounted_it) { SAFE_FILE_PRINTFAT(root->mnt_dir.dir_fd, cgsc, "+%s", ctrl->ctrl_name); @@ -863,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 (!base) { + 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 (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); @@ -1049,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, "base")) { + SAFE_CG_PRINTF(parent, "cgroup.subtree_control", + "+%s", ctrl->ctrl_name); + } } for (i = 0; cg->dirs[i]; i++)
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" base controller. Signed-off-by: Wei Gao <wegao@suse.com> --- lib/tst_cgroup.c | 43 ++++++++++++++++++++++++++++++------------- 1 file changed, 30 insertions(+), 13 deletions(-)