[v9,2/2] tst_cgroup.c: Add a cgroup pseudo controller

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 | 47 ++++++++++++++++++++++++++++++++---------------
 1 file changed, 32 insertions(+), 15 deletions(-)

Hi Wei,

On Sat, Apr 22, 2023 at 9:54 PM 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 | 47 ++++++++++++++++++++++++++++++++---------------
>  1 file changed, 32 insertions(+), 15 deletions(-)
>
> diff --git a/lib/tst_cgroup.c b/lib/tst_cgroup.c
> index 77575431d..ed3e0758f 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
>

I vote for considering use of 'CTRL_BASE' (suggested by Cryil),
because here we indeed use the basic functionality of CgroupV
 but not a pseudo controller.

Otherwise, the rest looks good to me.

=======================

@Cryil, @Richard

Apart from that, there is another problem with the test logic
of this library, that it potentially mixed Cgroup V1 and V2
together to be mounted at the same time. The scenario
happens once someone just requests CTRL_BASE
(or controllers not mounted) on a V1-mounted system.

Upstream always objected to enabling Cgroup like that,
which brings many unexpected issue during the test.

So I would strongly suggest avoiding LTP mount V1&V2
even if there is no overlap in controllers.

Which should be achieved in a separate patch:
(document should be updated as well)

--- a/lib/tst_cgroup.c
+++ b/lib/tst_cgroup.c
@@ -828,13 +828,14 @@ void tst_cg_require(const char *const ctrl_name,
        if (ctrl->ctrl_root)
                goto mkdirs;

-       if (!cgroup_v2_mounted() && options->needs_ver != TST_CG_V1)
+       if (!cgroup_v2_mounted() && options->needs_ver != TST_CG_V1
+                       && !cgroup_v1_mounted())
                cgroup_mount_v2();

        if (ctrl->ctrl_root)
                goto mkdirs;

-       if (options->needs_ver != TST_CG_V2)
+       if (options->needs_ver != TST_CG_V2 && !cgroup_v2_mounted())
                cgroup_mount_v1(ctrl);

        if (pseudo)

Hi!
> 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.

Can we please call it "base" controller? Or something better fitting?

> Signed-off-by: Wei Gao <wegao@suse.com>
> ---
>  lib/tst_cgroup.c | 47 ++++++++++++++++++++++++++++++++---------------
>  1 file changed, 32 insertions(+), 15 deletions(-)
> 
> diff --git a/lib/tst_cgroup.c b/lib/tst_cgroup.c
> index 77575431d..ed3e0758f 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 pseudo_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 }
> @@ -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(pseudo, CTRL_PSEUDO),
>  	{ }
>  };
>  
> @@ -798,6 +804,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 pseudo = !strcmp(ctrl->ctrl_name, "pseudo");
> +
> +	if (pseudo && options->needs_ver != TST_CG_V2)
> +		tst_brk(TCONF, "pseudo control only support needs_ver TST_CG_V2!");
>  
>  	if (!ctrl) {
>  		tst_brk(TBROK, "'%s' controller is unknown to LTP", ctrl_name);
> @@ -827,6 +837,9 @@ void tst_cg_require(const char *const ctrl_name,
>  	if (options->needs_ver != TST_CG_V2)
>  		cgroup_mount_v1(ctrl);
>  
> +	if (pseudo)
> +		ctrl->ctrl_root = roots;
> +
>  	if (!ctrl->ctrl_root) {
>  		tst_brk(TCONF,
>  			"'%s' controller required, but not available",
> @@ -849,13 +862,13 @@ mkdirs:
>  			ctrl->ctrl_name);
>  	}
>  
> -	if (cgroup_ctrl_on_v2(ctrl)) {
> +	if (cgroup_ctrl_on_v2(ctrl) && !pseudo) {
>  		if (root->we_mounted_it) {
>  			SAFE_FILE_PRINTFAT(root->mnt_dir.dir_fd,
> -					   cgsc, "+%s", ctrl->ctrl_name);
> +					cgsc, "+%s", ctrl->ctrl_name);
>  		} else {
>  			tst_file_printfat(root->mnt_dir.dir_fd,
> -					  cgsc, "+%s", ctrl->ctrl_name);
> +					cgsc, "+%s", ctrl->ctrl_name);
>  		}
>  	}
>  
> @@ -864,15 +877,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 (!pseudo) {
> +		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);
> @@ -1050,8 +1065,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, "pseudo")) {
> +			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