Re: [LTP] [PATCH v9 2/2] tst_cgroup.c: Add a cgroup pseudo controller

Li Wang
2023-04-23T06:46:58Z

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)

--
Regards,
Li Wang