[LTP] [PATCH v1] clone3: Add clone3's clone_args cgroup

[LTP] [PATCH v1] clone3: Add clone3's clone_args cgroup

[Wei Gao via ltp]

Signed-off-by: Wei Gao <wegao@suse.com>
---
 include/lapi/sched.h | 3 +++
 include/tst_clone.h  | 1 +
 lib/tst_clone.c      | 1 +
 3 files changed, 5 insertions(+)

diff --git a/include/lapi/sched.h b/include/lapi/sched.h
index 1065665d1..3ad16d136 100644
--- a/include/lapi/sched.h
+++ b/include/lapi/sched.h
@@ -54,6 +54,9 @@ struct clone_args {
 	uint64_t __attribute__((aligned(8))) stack;
 	uint64_t __attribute__((aligned(8))) stack_size;
 	uint64_t __attribute__((aligned(8))) tls;
+	uint64_t __attribute__((aligned(8))) set_tid;
+	uint64_t __attribute__((aligned(8))) set_tid_size;
+	uint64_t __attribute__((aligned(8))) cgroup;
 };
 
 static inline int clone3(struct clone_args *args, size_t size)
diff --git a/include/tst_clone.h b/include/tst_clone.h
index 9ffdc68d1..7b278dfa7 100644
--- a/include/tst_clone.h
+++ b/include/tst_clone.h
@@ -11,6 +11,7 @@
 struct tst_clone_args {
 	uint64_t flags;
 	uint64_t exit_signal;
+	uint64_t cgroup;
 };
 
 /* clone3 with fallbacks to clone when possible. Be aware that it
diff --git a/lib/tst_clone.c b/lib/tst_clone.c
index ecc84408c..2aa00beb1 100644
--- a/lib/tst_clone.c
+++ b/lib/tst_clone.c
@@ -15,6 +15,7 @@ pid_t tst_clone(const struct tst_clone_args *tst_args)
 	struct clone_args args = {
 		.flags = tst_args->flags,
 		.exit_signal = tst_args->exit_signal,
+		.cgroup = tst_args->cgroup,
 	};
 	int flags;
 	pid_t pid = -1;
-- 
2.35.3


-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

Re: [LTP] [PATCH v1] clone3: Add clone3's clone_args cgroup

[Petr Vorel]

Hi Wei,

Do you plan to use this somewhere? Otherwise I don't see the point to just
add it.

Kind regards,
Petr

> Signed-off-by: Wei Gao <wegao@suse.com>
> ---
>  include/lapi/sched.h | 3 +++
>  include/tst_clone.h  | 1 +
>  lib/tst_clone.c      | 1 +
>  3 files changed, 5 insertions(+)

> diff --git a/include/lapi/sched.h b/include/lapi/sched.h
> index 1065665d1..3ad16d136 100644
> --- a/include/lapi/sched.h
> +++ b/include/lapi/sched.h
> @@ -54,6 +54,9 @@ struct clone_args {
>  	uint64_t __attribute__((aligned(8))) stack;
>  	uint64_t __attribute__((aligned(8))) stack_size;
>  	uint64_t __attribute__((aligned(8))) tls;
> +	uint64_t __attribute__((aligned(8))) set_tid;
> +	uint64_t __attribute__((aligned(8))) set_tid_size;
> +	uint64_t __attribute__((aligned(8))) cgroup;
>  };

>  static inline int clone3(struct clone_args *args, size_t size)
> diff --git a/include/tst_clone.h b/include/tst_clone.h
> index 9ffdc68d1..7b278dfa7 100644
> --- a/include/tst_clone.h
> +++ b/include/tst_clone.h
> @@ -11,6 +11,7 @@
>  struct tst_clone_args {
>  	uint64_t flags;
>  	uint64_t exit_signal;
> +	uint64_t cgroup;
>  };

>  /* clone3 with fallbacks to clone when possible. Be aware that it
> diff --git a/lib/tst_clone.c b/lib/tst_clone.c
> index ecc84408c..2aa00beb1 100644
> --- a/lib/tst_clone.c
> +++ b/lib/tst_clone.c
> @@ -15,6 +15,7 @@ pid_t tst_clone(const struct tst_clone_args *tst_args)
>  	struct clone_args args = {
>  		.flags = tst_args->flags,
>  		.exit_signal = tst_args->exit_signal,
> +		.cgroup = tst_args->cgroup,
>  	};
>  	int flags;
>  	pid_t pid = -1;

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

Re: [LTP] [PATCH v1] clone3: Add clone3's clone_args cgroup

[Wei Gao via ltp]

On Thu, Mar 23, 2023 at 10:26:55AM +0100, Petr Vorel wrote:
> Hi Wei,
> 
> Do you plan to use this somewhere? Otherwise I don't see the point to just
> add it.
Sorry i need add another commit above this and start using new added args, will
create test case later, maybe you can put ChangRequest firstly, thanks.
> 
> Kind regards,
> Petr
> 
> > Signed-off-by: Wei Gao <wegao@suse.com>
> > ---
> >  include/lapi/sched.h | 3 +++
> >  include/tst_clone.h  | 1 +
> >  lib/tst_clone.c      | 1 +
> >  3 files changed, 5 insertions(+)
> 
> > diff --git a/include/lapi/sched.h b/include/lapi/sched.h
> > index 1065665d1..3ad16d136 100644
> > --- a/include/lapi/sched.h
> > +++ b/include/lapi/sched.h
> > @@ -54,6 +54,9 @@ struct clone_args {
> >  	uint64_t __attribute__((aligned(8))) stack;
> >  	uint64_t __attribute__((aligned(8))) stack_size;
> >  	uint64_t __attribute__((aligned(8))) tls;
> > +	uint64_t __attribute__((aligned(8))) set_tid;
> > +	uint64_t __attribute__((aligned(8))) set_tid_size;
> > +	uint64_t __attribute__((aligned(8))) cgroup;
> >  };
> 
> >  static inline int clone3(struct clone_args *args, size_t size)
> > diff --git a/include/tst_clone.h b/include/tst_clone.h
> > index 9ffdc68d1..7b278dfa7 100644
> > --- a/include/tst_clone.h
> > +++ b/include/tst_clone.h
> > @@ -11,6 +11,7 @@
> >  struct tst_clone_args {
> >  	uint64_t flags;
> >  	uint64_t exit_signal;
> > +	uint64_t cgroup;
> >  };
> 
> >  /* clone3 with fallbacks to clone when possible. Be aware that it
> > diff --git a/lib/tst_clone.c b/lib/tst_clone.c
> > index ecc84408c..2aa00beb1 100644
> > --- a/lib/tst_clone.c
> > +++ b/lib/tst_clone.c
> > @@ -15,6 +15,7 @@ pid_t tst_clone(const struct tst_clone_args *tst_args)
> >  	struct clone_args args = {
> >  		.flags = tst_args->flags,
> >  		.exit_signal = tst_args->exit_signal,
> > +		.cgroup = tst_args->cgroup,
> >  	};
> >  	int flags;
> >  	pid_t pid = -1;

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

Re: [LTP] [PATCH v1] clone3: Add clone3's clone_args cgroup

[Petr Vorel]

Hi Wei,
> On Thu, Mar 23, 2023 at 10:26:55AM +0100, Petr Vorel wrote:
> > Hi Wei,

> > Do you plan to use this somewhere? Otherwise I don't see the point to just
> > add it.
> Sorry i need add another commit above this and start using new added args, will
> create test case later, maybe you can put ChangRequest firstly, thanks.

I'd prefer to merge them once they are actually needed. But Richie might have a
different opinion.

Kind regards,
Petr

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

Re: [LTP] [PATCH v1] clone3: Add clone3's clone_args cgroup

[Wei Gao via ltp]

On Fri, Mar 24, 2023 at 07:32:54AM +0100, Petr Vorel wrote:
> Hi Wei,
> > On Thu, Mar 23, 2023 at 10:26:55AM +0100, Petr Vorel wrote:
> > > Hi Wei,
> 
> > > Do you plan to use this somewhere? Otherwise I don't see the point to just
> > > add it.
> > Sorry i need add another commit above this and start using new added args, will
> > create test case later, maybe you can put ChangRequest firstly, thanks.
> 
> I'd prefer to merge them once they are actually needed. But Richie might have a
> different opinion.
> 
@Petr So i need continue add case or not? 
@Richie what's your plan?
> Kind regards,
> Petr

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

Re: [LTP] [PATCH v1] clone3: Add clone3's clone_args cgroup

[Petr Vorel]

> On Fri, Mar 24, 2023 at 07:32:54AM +0100, Petr Vorel wrote:
> > Hi Wei,
> > > On Thu, Mar 23, 2023 at 10:26:55AM +0100, Petr Vorel wrote:
> > > > Hi Wei,

> > > > Do you plan to use this somewhere? Otherwise I don't see the point to just
> > > > add it.
> > > Sorry i need add another commit above this and start using new added args, will
> > > create test case later, maybe you can put ChangRequest firstly, thanks.

> > I'd prefer to merge them once they are actually needed. But Richie might have a
> > different opinion.

> @Petr So i need continue add case or not? 
That's up to you. I'm just saying I would not merge code which is not use.

Kind regards,
Petr

> @Richie what's your plan?
> > Kind regards,
> > Petr

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

[LTP] [PATCH v2] clone3: Add clone3's clone_args cgroup

[Wei Gao via ltp]

Signed-off-by: Wei Gao <wegao@suse.com>
---
 include/lapi/sched.h                        |  4 +
 include/tst_clone.h                         |  1 +
 lib/tst_cgroup.c                            |  1 +
 lib/tst_clone.c                             |  1 +
 runtest/syscalls                            |  1 +
 testcases/kernel/syscalls/clone3/.gitignore |  1 +
 testcases/kernel/syscalls/clone3/clone303.c | 98 +++++++++++++++++++++
 7 files changed, 107 insertions(+)
 create mode 100644 testcases/kernel/syscalls/clone3/clone303.c

diff --git a/include/lapi/sched.h b/include/lapi/sched.h
index 1065665d1..d5dd6f76b 100644
--- a/include/lapi/sched.h
+++ b/include/lapi/sched.h
@@ -13,6 +13,7 @@
 #include <inttypes.h>
 #include "config.h"
 #include "lapi/syscalls.h"
+#include "lapi/sched.h"
 
 struct sched_attr {
 	uint32_t size;
@@ -54,6 +55,9 @@ struct clone_args {
 	uint64_t __attribute__((aligned(8))) stack;
 	uint64_t __attribute__((aligned(8))) stack_size;
 	uint64_t __attribute__((aligned(8))) tls;
+	uint64_t __attribute__((aligned(8))) set_tid;
+	uint64_t __attribute__((aligned(8))) set_tid_size;
+	uint64_t __attribute__((aligned(8))) cgroup;
 };
 
 static inline int clone3(struct clone_args *args, size_t size)
diff --git a/include/tst_clone.h b/include/tst_clone.h
index 9ffdc68d1..7b278dfa7 100644
--- a/include/tst_clone.h
+++ b/include/tst_clone.h
@@ -11,6 +11,7 @@
 struct tst_clone_args {
 	uint64_t flags;
 	uint64_t exit_signal;
+	uint64_t cgroup;
 };
 
 /* clone3 with fallbacks to clone when possible. Be aware that it
diff --git a/lib/tst_cgroup.c b/lib/tst_cgroup.c
index 50699bc63..77575431d 100644
--- a/lib/tst_cgroup.c
+++ b/lib/tst_cgroup.c
@@ -166,6 +166,7 @@ static const struct cgroup_file cgroup_ctrl_files[] = {
 	{ "cgroup.controllers", NULL, 0 },
 	{ "cgroup.subtree_control", NULL, 0 },
 	{ "cgroup.clone_children", "cgroup.clone_children", 0 },
+	{ "cgroup.kill", NULL, 0 },
 	{ }
 };
 
diff --git a/lib/tst_clone.c b/lib/tst_clone.c
index ecc84408c..2aa00beb1 100644
--- a/lib/tst_clone.c
+++ b/lib/tst_clone.c
@@ -15,6 +15,7 @@ pid_t tst_clone(const struct tst_clone_args *tst_args)
 	struct clone_args args = {
 		.flags = tst_args->flags,
 		.exit_signal = tst_args->exit_signal,
+		.cgroup = tst_args->cgroup,
 	};
 	int flags;
 	pid_t pid = -1;
diff --git a/runtest/syscalls b/runtest/syscalls
index 9c23a4248..0b6adfd7f 100644
--- a/runtest/syscalls
+++ b/runtest/syscalls
@@ -117,6 +117,7 @@ clone09 clone09
 
 clone301 clone301
 clone302 clone302
+clone303 clone303
 
 close01 close01
 close02 close02
diff --git a/testcases/kernel/syscalls/clone3/.gitignore b/testcases/kernel/syscalls/clone3/.gitignore
index 604cb903e..10369954b 100644
--- a/testcases/kernel/syscalls/clone3/.gitignore
+++ b/testcases/kernel/syscalls/clone3/.gitignore
@@ -1,2 +1,3 @@
 clone301
 clone302
+clone303
diff --git a/testcases/kernel/syscalls/clone3/clone303.c b/testcases/kernel/syscalls/clone3/clone303.c
new file mode 100644
index 000000000..3310b21dc
--- /dev/null
+++ b/testcases/kernel/syscalls/clone3/clone303.c
@@ -0,0 +1,98 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * Copyright (c) 2023 SUSE LLC <wegao@suse.com>
+ */
+
+/*\
+ * [Description]
+ *
+ * This test case check clone3 CLONE_INTO_CGROUP flag
+ *
+ */
+
+#define _GNU_SOURCE
+#include <stdio.h>
+#include <stdlib.h>
+#include <sys/wait.h>
+
+#include "tst_test.h"
+#include "lapi/sched.h"
+#include "lapi/pidfd.h"
+
+#define BUF_LEN 20
+#define CLONE_INTO_CGROUP 0x200000000ULL
+
+static struct tst_cg_group *cg_child_test_simple;
+static struct clone_args *args;
+
+static pid_t clone_into_cgroup(int cgroup_fd)
+{
+	pid_t pid;
+
+	args->flags = CLONE_INTO_CGROUP;
+	args->exit_signal = SIGCHLD;
+	args->cgroup = cgroup_fd;
+
+	pid = clone3(args, sizeof(*args));
+
+	if (pid < 0)
+		tst_res(TFAIL | TTERRNO, "clone3() failed !");
+
+	return pid;
+}
+
+static void run(void)
+{
+	char path[100];
+	int fd;
+	pid_t pid;
+
+	cg_child_test_simple = tst_cg_group_mk(tst_cg, "cg_test_simple");
+	sprintf(path, "/sys/fs/cgroup/ltp/%s/%s", tst_cg_group_name(tst_cg), tst_cg_group_name(cg_child_test_simple));
+	tst_res(TINFO, "cgroup path is %s", path);
+	fd = SAFE_OPEN(path, O_DIRECTORY | O_CLOEXEC | O_NOFOLLOW | O_PATH);
+
+	pid = clone_into_cgroup(fd);
+
+	if (!pid) {
+		TST_CHECKPOINT_WAIT(0);
+	} else {
+
+		char buf[BUF_LEN];
+
+		SAFE_CG_READ(cg_child_test_simple, "cgroup.procs", buf, BUF_LEN);
+
+		int x = atoi(buf);
+
+		if (x == pid)
+			tst_res(TPASS, "clone3 case pass!");
+		else
+			tst_brk(TFAIL | TTERRNO, "clone3() failed !");
+
+		TST_CHECKPOINT_WAKE(0);
+
+		SAFE_WAITPID(pid, NULL, 0);
+
+		cg_child_test_simple = tst_cg_group_rm(cg_child_test_simple);
+	}
+}
+
+static void setup(void)
+{
+	clone3_supported_by_kernel();
+}
+
+static struct tst_test test = {
+	.test_all = run,
+	.setup = setup,
+	.forks_child = 1,
+	.max_runtime = 20,
+	.needs_cgroup_ctrls = (const char *const []){ "memory", NULL },
+	.needs_cgroup_ver = TST_CG_V2,
+	.needs_checkpoints = 1,
+	.min_kver = "5.7",
+	.bufs = (struct tst_buffers []) {
+		{&args, .size = sizeof(*args)},
+		{},
+	}
+};
-- 
2.35.3


-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

Re: [LTP] [PATCH v2] clone3: Add clone3's clone_args cgroup

[Cyril Hrubis]

Hi!
> --- /dev/null
> +++ b/testcases/kernel/syscalls/clone3/clone303.c
> @@ -0,0 +1,98 @@
> +// SPDX-License-Identifier: GPL-2.0-or-later
> +/*
> + * Copyright (c) 2023 SUSE LLC <wegao@suse.com>
> + */
> +
> +/*\
> + * [Description]
> + *
> + * This test case check clone3 CLONE_INTO_CGROUP flag
> + *
> + */
> +
> +#define _GNU_SOURCE
> +#include <stdio.h>
> +#include <stdlib.h>
> +#include <sys/wait.h>
> +
> +#include "tst_test.h"
> +#include "lapi/sched.h"
> +#include "lapi/pidfd.h"
> +
> +#define BUF_LEN 20
> +#define CLONE_INTO_CGROUP 0x200000000ULL

This should be added as a fallback definition into lapi/sched.h

> +static struct tst_cg_group *cg_child_test_simple;
> +static struct clone_args *args;
> +
> +static pid_t clone_into_cgroup(int cgroup_fd)
> +{
> +	pid_t pid;
> +
> +	args->flags = CLONE_INTO_CGROUP;
> +	args->exit_signal = SIGCHLD;
> +	args->cgroup = cgroup_fd;
> +
> +	pid = clone3(args, sizeof(*args));
> +
> +	if (pid < 0)
> +		tst_res(TFAIL | TTERRNO, "clone3() failed !");
> +
> +	return pid;
> +}
> +
> +static void run(void)
> +{
> +	char path[100];
> +	int fd;
> +	pid_t pid;
> +
> +	cg_child_test_simple = tst_cg_group_mk(tst_cg, "cg_test_simple");
> +	sprintf(path, "/sys/fs/cgroup/ltp/%s/%s", tst_cg_group_name(tst_cg), tst_cg_group_name(cg_child_test_simple));

There is no guarantee that the cgroup is mounted in this hardcoded path
and that the top level directory for LTP tests is called ltp.

Moreover we already do have a file descriptor pointing to the directory
for cgroup v2 opened in the cgroup library. All we need is a function in
the library that would find the take the tst_cg_group and find the right
cgroup_dir in the dirs[] array.

> +	tst_res(TINFO, "cgroup path is %s", path);
> +	fd = SAFE_OPEN(path, O_DIRECTORY | O_CLOEXEC | O_NOFOLLOW | O_PATH);
> +
> +	pid = clone_into_cgroup(fd);
> +
> +	if (!pid) {
> +		TST_CHECKPOINT_WAIT(0);

Just add reutrn; here and drop the else below.

> +	} else {
> +
> +		char buf[BUF_LEN];
> +
> +		SAFE_CG_READ(cg_child_test_simple, "cgroup.procs", buf, BUF_LEN);
                                                                          ^
									  sizeof(buf);
> +
> +		int x = atoi(buf);
> +
> +		if (x == pid)
> +			tst_res(TPASS, "clone3 case pass!");
> +		else
> +			tst_brk(TFAIL | TTERRNO, "clone3() failed !");
> +
> +		TST_CHECKPOINT_WAKE(0);
> +
> +		SAFE_WAITPID(pid, NULL, 0);
> +
> +		cg_child_test_simple = tst_cg_group_rm(cg_child_test_simple);
> +	}
> +}
> +
> +static void setup(void)
> +{
> +	clone3_supported_by_kernel();
> +}
> +
> +static struct tst_test test = {
> +	.test_all = run,
> +	.setup = setup,
> +	.forks_child = 1,
> +	.max_runtime = 20,
> +	.needs_cgroup_ctrls = (const char *const []){ "memory", NULL },
> +	.needs_cgroup_ver = TST_CG_V2,
> +	.needs_checkpoints = 1,
> +	.min_kver = "5.7",
> +	.bufs = (struct tst_buffers []) {
> +		{&args, .size = sizeof(*args)},
> +		{},
> +	}
> +};
> -- 
> 2.35.3
> 
> 
> -- 
> Mailing list info: https://lists.linux.it/listinfo/ltp

-- 
Cyril Hrubis
chrubis@suse.cz

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

[LTP] [PATCH v3] clone3: Add clone3's clone_args cgroup

[Wei Gao via ltp]

Signed-off-by: Wei Gao <wegao@suse.com>
---
 include/lapi/sched.h                        |  8 ++
 include/tst_cgroup.h                        |  3 +
 include/tst_clone.h                         |  1 +
 lib/tst_cgroup.c                            |  6 ++
 lib/tst_clone.c                             |  1 +
 runtest/syscalls                            |  1 +
 testcases/kernel/syscalls/clone3/.gitignore |  1 +
 testcases/kernel/syscalls/clone3/clone303.c | 94 +++++++++++++++++++++
 8 files changed, 115 insertions(+)
 create mode 100644 testcases/kernel/syscalls/clone3/clone303.c

diff --git a/include/lapi/sched.h b/include/lapi/sched.h
index 1065665d1..ac766efc5 100644
--- a/include/lapi/sched.h
+++ b/include/lapi/sched.h
@@ -13,6 +13,7 @@
 #include <inttypes.h>
 #include "config.h"
 #include "lapi/syscalls.h"
+#include "lapi/sched.h"
 
 struct sched_attr {
 	uint32_t size;
@@ -54,6 +55,9 @@ struct clone_args {
 	uint64_t __attribute__((aligned(8))) stack;
 	uint64_t __attribute__((aligned(8))) stack_size;
 	uint64_t __attribute__((aligned(8))) tls;
+	uint64_t __attribute__((aligned(8))) set_tid;
+	uint64_t __attribute__((aligned(8))) set_tid_size;
+	uint64_t __attribute__((aligned(8))) cgroup;
 };
 
 static inline int clone3(struct clone_args *args, size_t size)
@@ -133,4 +137,8 @@ static inline int getcpu(unsigned *cpu, unsigned *node)
 # define CLONE_NEWTIME		0x00000080
 #endif
 
+#ifndef CLONE_INTO_CGROUP
+# define CLONE_INTO_CGROUP 0x200000000ULL
+#endif
+
 #endif /* LAPI_SCHED_H__ */
diff --git a/include/tst_cgroup.h b/include/tst_cgroup.h
index 2826ddad1..d8e621962 100644
--- a/include/tst_cgroup.h
+++ b/include/tst_cgroup.h
@@ -157,6 +157,9 @@ const char *
 tst_cg_group_name(const struct tst_cg_group *const cg)
 		      __attribute__ ((nonnull, warn_unused_result));
 
+int tst_cg_group_dir_fd(const struct tst_cg_group *const cg)
+		      __attribute__ ((nonnull, warn_unused_result));
+
 /* Remove a descendant CGroup */
 struct tst_cg_group *
 tst_cg_group_rm(struct tst_cg_group *const cg)
diff --git a/include/tst_clone.h b/include/tst_clone.h
index 9ffdc68d1..7b278dfa7 100644
--- a/include/tst_clone.h
+++ b/include/tst_clone.h
@@ -11,6 +11,7 @@
 struct tst_clone_args {
 	uint64_t flags;
 	uint64_t exit_signal;
+	uint64_t cgroup;
 };
 
 /* clone3 with fallbacks to clone when possible. Be aware that it
diff --git a/lib/tst_cgroup.c b/lib/tst_cgroup.c
index 50699bc63..c63d04c67 100644
--- a/lib/tst_cgroup.c
+++ b/lib/tst_cgroup.c
@@ -166,6 +166,7 @@ static const struct cgroup_file cgroup_ctrl_files[] = {
 	{ "cgroup.controllers", NULL, 0 },
 	{ "cgroup.subtree_control", NULL, 0 },
 	{ "cgroup.clone_children", "cgroup.clone_children", 0 },
+	{ "cgroup.kill", NULL, 0 },
 	{ }
 };
 
@@ -1093,6 +1094,11 @@ const char *tst_cg_group_name(const struct tst_cg_group *const cg)
 	return cg->group_name;
 }
 
+int tst_cg_group_dir_fd(const struct tst_cg_group *const cg)
+{
+	return cg->dirs[0]->dir_fd;
+}
+
 struct tst_cg_group *tst_cg_group_rm(struct tst_cg_group *const cg)
 {
 	struct cgroup_dir **dir;
diff --git a/lib/tst_clone.c b/lib/tst_clone.c
index ecc84408c..2aa00beb1 100644
--- a/lib/tst_clone.c
+++ b/lib/tst_clone.c
@@ -15,6 +15,7 @@ pid_t tst_clone(const struct tst_clone_args *tst_args)
 	struct clone_args args = {
 		.flags = tst_args->flags,
 		.exit_signal = tst_args->exit_signal,
+		.cgroup = tst_args->cgroup,
 	};
 	int flags;
 	pid_t pid = -1;
diff --git a/runtest/syscalls b/runtest/syscalls
index 9c23a4248..0b6adfd7f 100644
--- a/runtest/syscalls
+++ b/runtest/syscalls
@@ -117,6 +117,7 @@ clone09 clone09
 
 clone301 clone301
 clone302 clone302
+clone303 clone303
 
 close01 close01
 close02 close02
diff --git a/testcases/kernel/syscalls/clone3/.gitignore b/testcases/kernel/syscalls/clone3/.gitignore
index 604cb903e..10369954b 100644
--- a/testcases/kernel/syscalls/clone3/.gitignore
+++ b/testcases/kernel/syscalls/clone3/.gitignore
@@ -1,2 +1,3 @@
 clone301
 clone302
+clone303
diff --git a/testcases/kernel/syscalls/clone3/clone303.c b/testcases/kernel/syscalls/clone3/clone303.c
new file mode 100644
index 000000000..e35cc8def
--- /dev/null
+++ b/testcases/kernel/syscalls/clone3/clone303.c
@@ -0,0 +1,94 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * Copyright (c) 2023 SUSE LLC <wegao@suse.com>
+ */
+
+/*\
+ * [Description]
+ *
+ * This test case check clone3 CLONE_INTO_CGROUP flag
+ *
+ */
+
+#define _GNU_SOURCE
+#include <stdio.h>
+#include <stdlib.h>
+#include <sys/wait.h>
+
+#include "tst_test.h"
+#include "lapi/sched.h"
+#include "lapi/pidfd.h"
+
+#define BUF_LEN 20
+
+static struct tst_cg_group *cg_child_test_simple;
+static struct clone_args *args;
+
+static pid_t clone_into_cgroup(int cgroup_fd)
+{
+	pid_t pid;
+
+	args->flags = CLONE_INTO_CGROUP;
+	args->exit_signal = SIGCHLD;
+	args->cgroup = cgroup_fd;
+
+	pid = clone3(args, sizeof(*args));
+
+	if (pid < 0)
+		tst_res(TFAIL | TTERRNO, "clone3() failed !");
+
+	return pid;
+}
+
+static void run(void)
+{
+	int fd;
+	pid_t pid;
+
+	cg_child_test_simple = tst_cg_group_mk(tst_cg, "cg_test_simple");
+	fd = tst_cg_group_dir_fd(cg_child_test_simple);
+
+	pid = clone_into_cgroup(fd);
+
+	if (!pid) {
+		TST_CHECKPOINT_WAIT(0);
+		return;
+	}
+
+	char buf[BUF_LEN];
+
+	SAFE_CG_READ(cg_child_test_simple, "cgroup.procs", buf, BUF_LEN);
+
+	int x = atoi(buf);
+
+	if (x == pid)
+		tst_res(TPASS, "clone3 case pass!");
+	else
+		tst_brk(TFAIL | TTERRNO, "clone3() failed !");
+
+	TST_CHECKPOINT_WAKE(0);
+
+	SAFE_WAITPID(pid, NULL, 0);
+
+	cg_child_test_simple = tst_cg_group_rm(cg_child_test_simple);
+}
+
+static void setup(void)
+{
+	clone3_supported_by_kernel();
+}
+
+static struct tst_test test = {
+	.test_all = run,
+	.setup = setup,
+	.forks_child = 1,
+	.max_runtime = 20,
+	.needs_cgroup_ctrls = (const char *const []){ "memory", NULL },
+	.needs_cgroup_ver = TST_CG_V2,
+	.needs_checkpoints = 1,
+	.min_kver = "5.7",
+	.bufs = (struct tst_buffers []) {
+		{&args, .size = sizeof(*args)},
+		{},
+	}
+};
-- 
2.35.3


-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

Re: [LTP] [PATCH v3] clone3: Add clone3's clone_args cgroup

[Cyril Hrubis]

Hi!
> diff --git a/include/tst_cgroup.h b/include/tst_cgroup.h
> index 2826ddad1..d8e621962 100644
> --- a/include/tst_cgroup.h
> +++ b/include/tst_cgroup.h
> @@ -157,6 +157,9 @@ const char *
>  tst_cg_group_name(const struct tst_cg_group *const cg)
>  		      __attribute__ ((nonnull, warn_unused_result));
>  
> +int tst_cg_group_dir_fd(const struct tst_cg_group *const cg)
> +		      __attribute__ ((nonnull, warn_unused_result));
> +
>  /* Remove a descendant CGroup */
>  struct tst_cg_group *
>  tst_cg_group_rm(struct tst_cg_group *const cg)
> diff --git a/include/tst_clone.h b/include/tst_clone.h
> index 9ffdc68d1..7b278dfa7 100644
> --- a/include/tst_clone.h
> +++ b/include/tst_clone.h
> @@ -11,6 +11,7 @@
>  struct tst_clone_args {
>  	uint64_t flags;
>  	uint64_t exit_signal;
> +	uint64_t cgroup;
>  };
>  
>  /* clone3 with fallbacks to clone when possible. Be aware that it
> diff --git a/lib/tst_cgroup.c b/lib/tst_cgroup.c
> index 50699bc63..c63d04c67 100644
> --- a/lib/tst_cgroup.c
> +++ b/lib/tst_cgroup.c
> @@ -166,6 +166,7 @@ static const struct cgroup_file cgroup_ctrl_files[] = {
>  	{ "cgroup.controllers", NULL, 0 },
>  	{ "cgroup.subtree_control", NULL, 0 },
>  	{ "cgroup.clone_children", "cgroup.clone_children", 0 },
> +	{ "cgroup.kill", NULL, 0 },
>  	{ }
>  };

This seems to be unrelated change.

> @@ -1093,6 +1094,11 @@ const char *tst_cg_group_name(const struct tst_cg_group *const cg)
>  	return cg->group_name;
>  }
>  
> +int tst_cg_group_dir_fd(const struct tst_cg_group *const cg)
> +{
> +	return cg->dirs[0]->dir_fd;

Does this actually work in the case of mixed cgroup v1 and v2 setup?
Even if so I would just make sure that this call returns a fd pointing
to a v2 directory. Also the function should include "unified" in the
name so that it's clear that it returns fd to the v2 "unified"
hierarchy. I guess that tst_cg_group_unified_dir_fd() would be fitting
name.

To implement that we have to loop over dirs[] array and check which one
of them is v2, which should be reachable via the root pointer in the dir.

> +}
> +
>  struct tst_cg_group *tst_cg_group_rm(struct tst_cg_group *const cg)
>  {
>  	struct cgroup_dir **dir;
> diff --git a/lib/tst_clone.c b/lib/tst_clone.c
> index ecc84408c..2aa00beb1 100644
> --- a/lib/tst_clone.c
> +++ b/lib/tst_clone.c
> @@ -15,6 +15,7 @@ pid_t tst_clone(const struct tst_clone_args *tst_args)
>  	struct clone_args args = {
>  		.flags = tst_args->flags,
>  		.exit_signal = tst_args->exit_signal,
> +		.cgroup = tst_args->cgroup,
>  	};
>  	int flags;
>  	pid_t pid = -1;
> diff --git a/runtest/syscalls b/runtest/syscalls
> index 9c23a4248..0b6adfd7f 100644
> --- a/runtest/syscalls
> +++ b/runtest/syscalls
> @@ -117,6 +117,7 @@ clone09 clone09
>  
>  clone301 clone301
>  clone302 clone302
> +clone303 clone303
>  
>  close01 close01
>  close02 close02
> diff --git a/testcases/kernel/syscalls/clone3/.gitignore b/testcases/kernel/syscalls/clone3/.gitignore
> index 604cb903e..10369954b 100644
> --- a/testcases/kernel/syscalls/clone3/.gitignore
> +++ b/testcases/kernel/syscalls/clone3/.gitignore
> @@ -1,2 +1,3 @@
>  clone301
>  clone302
> +clone303
> diff --git a/testcases/kernel/syscalls/clone3/clone303.c b/testcases/kernel/syscalls/clone3/clone303.c
> new file mode 100644
> index 000000000..e35cc8def
> --- /dev/null
> +++ b/testcases/kernel/syscalls/clone3/clone303.c
> @@ -0,0 +1,94 @@
> +// SPDX-License-Identifier: GPL-2.0-or-later
> +/*
> + * Copyright (c) 2023 SUSE LLC <wegao@suse.com>
> + */
> +
> +/*\
> + * [Description]
> + *
> + * This test case check clone3 CLONE_INTO_CGROUP flag
> + *
> + */
> +
> +#define _GNU_SOURCE
> +#include <stdio.h>
> +#include <stdlib.h>
> +#include <sys/wait.h>
> +
> +#include "tst_test.h"
> +#include "lapi/sched.h"
> +#include "lapi/pidfd.h"
> +
> +#define BUF_LEN 20
> +
> +static struct tst_cg_group *cg_child_test_simple;
> +static struct clone_args *args;
> +
> +static pid_t clone_into_cgroup(int cgroup_fd)
> +{
> +	pid_t pid;
> +
> +	args->flags = CLONE_INTO_CGROUP;
> +	args->exit_signal = SIGCHLD;
> +	args->cgroup = cgroup_fd;
> +
> +	pid = clone3(args, sizeof(*args));
> +
> +	if (pid < 0)
> +		tst_res(TFAIL | TTERRNO, "clone3() failed !");
> +
> +	return pid;
> +}
> +
> +static void run(void)
> +{
> +	int fd;
> +	pid_t pid;
> +
> +	cg_child_test_simple = tst_cg_group_mk(tst_cg, "cg_test_simple");
> +	fd = tst_cg_group_dir_fd(cg_child_test_simple);

I suppoose that this can be done once in the test setup.

> +	pid = clone_into_cgroup(fd);
> +
> +	if (!pid) {
> +		TST_CHECKPOINT_WAIT(0);
> +		return;
> +	}
> +
> +	char buf[BUF_LEN];
> +
> +	SAFE_CG_READ(cg_child_test_simple, "cgroup.procs", buf, BUF_LEN);
> +
> +	int x = atoi(buf);
> +
> +	if (x == pid)
> +		tst_res(TPASS, "clone3 case pass!");
> +	else
> +		tst_brk(TFAIL | TTERRNO, "clone3() failed !");
> +
> +	TST_CHECKPOINT_WAKE(0);
> +
> +	SAFE_WAITPID(pid, NULL, 0);
> +
> +	cg_child_test_simple = tst_cg_group_rm(cg_child_test_simple);

And this could be done in the test cleanup.

> +}
> +
> +static void setup(void)
> +{
> +	clone3_supported_by_kernel();
> +}
> +
> +static struct tst_test test = {
> +	.test_all = run,
> +	.setup = setup,
> +	.forks_child = 1,
> +	.max_runtime = 20,
> +	.needs_cgroup_ctrls = (const char *const []){ "memory", NULL },
> +	.needs_cgroup_ver = TST_CG_V2,
> +	.needs_checkpoints = 1,
> +	.min_kver = "5.7",
> +	.bufs = (struct tst_buffers []) {
> +		{&args, .size = sizeof(*args)},
> +		{},
> +	}
> +};
> -- 
> 2.35.3
> 
> 
> -- 
> Mailing list info: https://lists.linux.it/listinfo/ltp

-- 
Cyril Hrubis
chrubis@suse.cz

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

[LTP] [PATCH v4] clone3: Add clone3's clone_args cgroup

[Wei Gao via ltp]

Signed-off-by: Wei Gao <wegao@suse.com>
---
 include/lapi/sched.h                        |   8 ++
 include/tst_cgroup.h                        |   3 +
 include/tst_clone.h                         |   1 +
 lib/tst_cgroup.c                            |   9 ++
 lib/tst_clone.c                             |   1 +
 runtest/syscalls                            |   1 +
 testcases/kernel/syscalls/clone3/.gitignore |   1 +
 testcases/kernel/syscalls/clone3/clone303.c | 103 ++++++++++++++++++++
 8 files changed, 127 insertions(+)
 create mode 100644 testcases/kernel/syscalls/clone3/clone303.c

diff --git a/include/lapi/sched.h b/include/lapi/sched.h
index 1065665d1..ac766efc5 100644
--- a/include/lapi/sched.h
+++ b/include/lapi/sched.h
@@ -13,6 +13,7 @@
 #include <inttypes.h>
 #include "config.h"
 #include "lapi/syscalls.h"
+#include "lapi/sched.h"
 
 struct sched_attr {
 	uint32_t size;
@@ -54,6 +55,9 @@ struct clone_args {
 	uint64_t __attribute__((aligned(8))) stack;
 	uint64_t __attribute__((aligned(8))) stack_size;
 	uint64_t __attribute__((aligned(8))) tls;
+	uint64_t __attribute__((aligned(8))) set_tid;
+	uint64_t __attribute__((aligned(8))) set_tid_size;
+	uint64_t __attribute__((aligned(8))) cgroup;
 };
 
 static inline int clone3(struct clone_args *args, size_t size)
@@ -133,4 +137,8 @@ static inline int getcpu(unsigned *cpu, unsigned *node)
 # define CLONE_NEWTIME		0x00000080
 #endif
 
+#ifndef CLONE_INTO_CGROUP
+# define CLONE_INTO_CGROUP 0x200000000ULL
+#endif
+
 #endif /* LAPI_SCHED_H__ */
diff --git a/include/tst_cgroup.h b/include/tst_cgroup.h
index 2826ddad1..2ac909192 100644
--- a/include/tst_cgroup.h
+++ b/include/tst_cgroup.h
@@ -157,6 +157,9 @@ const char *
 tst_cg_group_name(const struct tst_cg_group *const cg)
 		      __attribute__ ((nonnull, warn_unused_result));
 
+int tst_cg_group_unified_dir_fd(const struct tst_cg_group *const cg)
+		      __attribute__ ((nonnull, warn_unused_result));
+
 /* Remove a descendant CGroup */
 struct tst_cg_group *
 tst_cg_group_rm(struct tst_cg_group *const cg)
diff --git a/include/tst_clone.h b/include/tst_clone.h
index 9ffdc68d1..7b278dfa7 100644
--- a/include/tst_clone.h
+++ b/include/tst_clone.h
@@ -11,6 +11,7 @@
 struct tst_clone_args {
 	uint64_t flags;
 	uint64_t exit_signal;
+	uint64_t cgroup;
 };
 
 /* clone3 with fallbacks to clone when possible. Be aware that it
diff --git a/lib/tst_cgroup.c b/lib/tst_cgroup.c
index 274c73fea..43055e8cf 100644
--- a/lib/tst_cgroup.c
+++ b/lib/tst_cgroup.c
@@ -1112,6 +1112,15 @@ const char *tst_cg_group_name(const struct tst_cg_group *const cg)
 	return cg->group_name;
 }
 
+int tst_cg_group_unified_dir_fd(const struct tst_cg_group *const cg)
+{
+	for (int i = 0; cg->dirs[i]; i++) {
+		if (cg->dirs[i]->dir_root->ver == TST_CG_V2)
+			return cg->dirs[i]->dir_fd;
+	}
+	return -1;
+}
+
 struct tst_cg_group *tst_cg_group_rm(struct tst_cg_group *const cg)
 {
 	struct cgroup_dir **dir;
diff --git a/lib/tst_clone.c b/lib/tst_clone.c
index ecc84408c..2aa00beb1 100644
--- a/lib/tst_clone.c
+++ b/lib/tst_clone.c
@@ -15,6 +15,7 @@ pid_t tst_clone(const struct tst_clone_args *tst_args)
 	struct clone_args args = {
 		.flags = tst_args->flags,
 		.exit_signal = tst_args->exit_signal,
+		.cgroup = tst_args->cgroup,
 	};
 	int flags;
 	pid_t pid = -1;
diff --git a/runtest/syscalls b/runtest/syscalls
index 9c23a4248..0b6adfd7f 100644
--- a/runtest/syscalls
+++ b/runtest/syscalls
@@ -117,6 +117,7 @@ clone09 clone09
 
 clone301 clone301
 clone302 clone302
+clone303 clone303
 
 close01 close01
 close02 close02
diff --git a/testcases/kernel/syscalls/clone3/.gitignore b/testcases/kernel/syscalls/clone3/.gitignore
index 604cb903e..10369954b 100644
--- a/testcases/kernel/syscalls/clone3/.gitignore
+++ b/testcases/kernel/syscalls/clone3/.gitignore
@@ -1,2 +1,3 @@
 clone301
 clone302
+clone303
diff --git a/testcases/kernel/syscalls/clone3/clone303.c b/testcases/kernel/syscalls/clone3/clone303.c
new file mode 100644
index 000000000..2eadaadba
--- /dev/null
+++ b/testcases/kernel/syscalls/clone3/clone303.c
@@ -0,0 +1,103 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * Copyright (c) 2023 SUSE LLC <wegao@suse.com>
+ */
+
+/*\
+ * [Description]
+ *
+ * This test case check clone3 CLONE_INTO_CGROUP flag
+ *
+ */
+
+#define _GNU_SOURCE
+#include <stdio.h>
+#include <stdlib.h>
+#include <sys/wait.h>
+
+#include "tst_test.h"
+#include "lapi/sched.h"
+#include "lapi/pidfd.h"
+
+#define BUF_LEN 20
+
+static struct tst_cg_group *cg_child_test_simple;
+static int fd;
+static struct clone_args *args;
+
+static pid_t clone_into_cgroup(int cgroup_fd)
+{
+	pid_t pid;
+
+	args->flags = CLONE_INTO_CGROUP;
+	args->exit_signal = SIGCHLD;
+	args->cgroup = cgroup_fd;
+
+	pid = clone3(args, sizeof(*args));
+
+	if (pid < 0)
+		tst_res(TFAIL | TTERRNO, "clone3() failed !");
+
+	return pid;
+}
+
+static void run(void)
+{
+	pid_t pid;
+
+	pid = clone_into_cgroup(fd);
+
+	if (!pid) {
+		TST_CHECKPOINT_WAIT(0);
+		return;
+	}
+
+	char buf[BUF_LEN];
+
+	SAFE_CG_READ(cg_child_test_simple, "cgroup.procs", buf, BUF_LEN);
+
+	int x = atoi(buf);
+
+	if (x == pid)
+		tst_res(TPASS, "clone3 case pass!");
+	else
+		tst_brk(TFAIL | TTERRNO, "clone3() failed !");
+
+	TST_CHECKPOINT_WAKE(0);
+
+	SAFE_WAITPID(pid, NULL, 0);
+
+}
+
+static void setup(void)
+{
+	clone3_supported_by_kernel();
+
+	cg_child_test_simple = tst_cg_group_mk(tst_cg, "cg_test_simple");
+
+	fd = tst_cg_group_unified_dir_fd(cg_child_test_simple);
+
+	if (fd < 0)
+		tst_res(TFAIL | TTERRNO, "get dir fd failed !");
+}
+
+static void cleanup(void)
+{
+	cg_child_test_simple = tst_cg_group_rm(cg_child_test_simple);
+}
+
+static struct tst_test test = {
+	.test_all = run,
+	.setup = setup,
+	.cleanup = cleanup,
+	.forks_child = 1,
+	.max_runtime = 20,
+	.needs_cgroup_ctrls = (const char *const []){ "base", NULL },
+	.needs_cgroup_ver = TST_CG_V2,
+	.needs_checkpoints = 1,
+	.min_kver = "5.7",
+	.bufs = (struct tst_buffers []) {
+		{&args, .size = sizeof(*args)},
+		{},
+	}
+};
-- 
2.35.3


-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

Re: [LTP] [PATCH v4] clone3: Add clone3's clone_args cgroup

[Petr Vorel]

Hi Wei,

...
> +++ b/include/tst_cgroup.h
> @@ -157,6 +157,9 @@ const char *
>  tst_cg_group_name(const struct tst_cg_group *const cg)
>  		      __attribute__ ((nonnull, warn_unused_result));

Maybe to add a comment to describe the function? (other functions have a
description).

> +int tst_cg_group_unified_dir_fd(const struct tst_cg_group *const cg)
> +		      __attribute__ ((nonnull, warn_unused_result));
> +
>  /* Remove a descendant CGroup */
>  struct tst_cg_group *
>  tst_cg_group_rm(struct tst_cg_group *const cg)
> diff --git a/include/tst_clone.h b/include/tst_clone.h
> index 9ffdc68d1..7b278dfa7 100644
> --- a/include/tst_clone.h
> +++ b/include/tst_clone.h
> @@ -11,6 +11,7 @@
>  struct tst_clone_args {
>  	uint64_t flags;
>  	uint64_t exit_signal;
> +	uint64_t cgroup;
>  };

...
> +++ b/testcases/kernel/syscalls/clone3/clone303.c
...
> +
> +static void run(void)
> +{
> +	pid_t pid;
> +
> +	pid = clone_into_cgroup(fd);
> +
> +	if (!pid) {
> +		TST_CHECKPOINT_WAIT(0);
> +		return;
> +	}
> +
> +	char buf[BUF_LEN];
> +
> +	SAFE_CG_READ(cg_child_test_simple, "cgroup.procs", buf, BUF_LEN);
> +
> +	int x = atoi(buf);
> +
> +	if (x == pid)
x is using only here. Why not just:
	if (atoi(buf) == pid)

> +		tst_res(TPASS, "clone3 case pass!");
> +	else
> +		tst_brk(TFAIL | TTERRNO, "clone3() failed !");
> +
> +	TST_CHECKPOINT_WAKE(0);
> +
> +	SAFE_WAITPID(pid, NULL, 0);
> +
> +}
> +
> +static void setup(void)
> +{
> +	clone3_supported_by_kernel();
> +
> +	cg_child_test_simple = tst_cg_group_mk(tst_cg, "cg_test_simple");
> +
> +	fd = tst_cg_group_unified_dir_fd(cg_child_test_simple);
> +
> +	if (fd < 0)
> +		tst_res(TFAIL | TTERRNO, "get dir fd failed !");
Wouldn't be better to use here
		tst_brk(TBROK | TTERRNO, "get dir fd failed!");

Or even just
		tst_brk(TBROK, "get dir fd failed!");

Because tst_cg_group_unified_dir_fd() does not do any real operation which would
set errno.

I mean does make sense to continue testing if fd is invalid?
> +}

...

Kind regards,
Petr

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

[LTP] [PATCH v5] clone3: Add clone3's clone_args cgroup

[Wei Gao via ltp]

Signed-off-by: Wei Gao <wegao@suse.com>
---
 include/lapi/sched.h                        |   8 ++
 include/tst_cgroup.h                        |   4 +
 include/tst_clone.h                         |   1 +
 lib/tst_cgroup.c                            |   9 ++
 lib/tst_clone.c                             |   1 +
 runtest/syscalls                            |   1 +
 testcases/kernel/syscalls/clone3/.gitignore |   1 +
 testcases/kernel/syscalls/clone3/clone303.c | 101 ++++++++++++++++++++
 8 files changed, 126 insertions(+)
 create mode 100644 testcases/kernel/syscalls/clone3/clone303.c

diff --git a/include/lapi/sched.h b/include/lapi/sched.h
index 1065665d1..ac766efc5 100644
--- a/include/lapi/sched.h
+++ b/include/lapi/sched.h
@@ -13,6 +13,7 @@
 #include <inttypes.h>
 #include "config.h"
 #include "lapi/syscalls.h"
+#include "lapi/sched.h"
 
 struct sched_attr {
 	uint32_t size;
@@ -54,6 +55,9 @@ struct clone_args {
 	uint64_t __attribute__((aligned(8))) stack;
 	uint64_t __attribute__((aligned(8))) stack_size;
 	uint64_t __attribute__((aligned(8))) tls;
+	uint64_t __attribute__((aligned(8))) set_tid;
+	uint64_t __attribute__((aligned(8))) set_tid_size;
+	uint64_t __attribute__((aligned(8))) cgroup;
 };
 
 static inline int clone3(struct clone_args *args, size_t size)
@@ -133,4 +137,8 @@ static inline int getcpu(unsigned *cpu, unsigned *node)
 # define CLONE_NEWTIME		0x00000080
 #endif
 
+#ifndef CLONE_INTO_CGROUP
+# define CLONE_INTO_CGROUP 0x200000000ULL
+#endif
+
 #endif /* LAPI_SCHED_H__ */
diff --git a/include/tst_cgroup.h b/include/tst_cgroup.h
index 2826ddad1..be14d07c6 100644
--- a/include/tst_cgroup.h
+++ b/include/tst_cgroup.h
@@ -157,6 +157,10 @@ const char *
 tst_cg_group_name(const struct tst_cg_group *const cg)
 		      __attribute__ ((nonnull, warn_unused_result));
 
+/* This call returns a fd pointing to a v2 directory */
+int tst_cg_group_unified_dir_fd(const struct tst_cg_group *const cg)
+		      __attribute__ ((nonnull, warn_unused_result));
+
 /* Remove a descendant CGroup */
 struct tst_cg_group *
 tst_cg_group_rm(struct tst_cg_group *const cg)
diff --git a/include/tst_clone.h b/include/tst_clone.h
index 9ffdc68d1..7b278dfa7 100644
--- a/include/tst_clone.h
+++ b/include/tst_clone.h
@@ -11,6 +11,7 @@
 struct tst_clone_args {
 	uint64_t flags;
 	uint64_t exit_signal;
+	uint64_t cgroup;
 };
 
 /* clone3 with fallbacks to clone when possible. Be aware that it
diff --git a/lib/tst_cgroup.c b/lib/tst_cgroup.c
index 274c73fea..43055e8cf 100644
--- a/lib/tst_cgroup.c
+++ b/lib/tst_cgroup.c
@@ -1112,6 +1112,15 @@ const char *tst_cg_group_name(const struct tst_cg_group *const cg)
 	return cg->group_name;
 }
 
+int tst_cg_group_unified_dir_fd(const struct tst_cg_group *const cg)
+{
+	for (int i = 0; cg->dirs[i]; i++) {
+		if (cg->dirs[i]->dir_root->ver == TST_CG_V2)
+			return cg->dirs[i]->dir_fd;
+	}
+	return -1;
+}
+
 struct tst_cg_group *tst_cg_group_rm(struct tst_cg_group *const cg)
 {
 	struct cgroup_dir **dir;
diff --git a/lib/tst_clone.c b/lib/tst_clone.c
index ecc84408c..2aa00beb1 100644
--- a/lib/tst_clone.c
+++ b/lib/tst_clone.c
@@ -15,6 +15,7 @@ pid_t tst_clone(const struct tst_clone_args *tst_args)
 	struct clone_args args = {
 		.flags = tst_args->flags,
 		.exit_signal = tst_args->exit_signal,
+		.cgroup = tst_args->cgroup,
 	};
 	int flags;
 	pid_t pid = -1;
diff --git a/runtest/syscalls b/runtest/syscalls
index 9c23a4248..0b6adfd7f 100644
--- a/runtest/syscalls
+++ b/runtest/syscalls
@@ -117,6 +117,7 @@ clone09 clone09
 
 clone301 clone301
 clone302 clone302
+clone303 clone303
 
 close01 close01
 close02 close02
diff --git a/testcases/kernel/syscalls/clone3/.gitignore b/testcases/kernel/syscalls/clone3/.gitignore
index 604cb903e..10369954b 100644
--- a/testcases/kernel/syscalls/clone3/.gitignore
+++ b/testcases/kernel/syscalls/clone3/.gitignore
@@ -1,2 +1,3 @@
 clone301
 clone302
+clone303
diff --git a/testcases/kernel/syscalls/clone3/clone303.c b/testcases/kernel/syscalls/clone3/clone303.c
new file mode 100644
index 000000000..5bf02edef
--- /dev/null
+++ b/testcases/kernel/syscalls/clone3/clone303.c
@@ -0,0 +1,101 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * Copyright (c) 2023 SUSE LLC <wegao@suse.com>
+ */
+
+/*\
+ * [Description]
+ *
+ * This test case check clone3 CLONE_INTO_CGROUP flag
+ *
+ */
+
+#define _GNU_SOURCE
+#include <stdio.h>
+#include <stdlib.h>
+#include <sys/wait.h>
+
+#include "tst_test.h"
+#include "lapi/sched.h"
+#include "lapi/pidfd.h"
+
+#define BUF_LEN 20
+
+static struct tst_cg_group *cg_child_test_simple;
+static int fd;
+static struct clone_args *args;
+
+static pid_t clone_into_cgroup(int cgroup_fd)
+{
+	pid_t pid;
+
+	args->flags = CLONE_INTO_CGROUP;
+	args->exit_signal = SIGCHLD;
+	args->cgroup = cgroup_fd;
+
+	pid = clone3(args, sizeof(*args));
+
+	if (pid < 0)
+		tst_res(TFAIL | TTERRNO, "clone3() failed !");
+
+	return pid;
+}
+
+static void run(void)
+{
+	pid_t pid;
+
+	pid = clone_into_cgroup(fd);
+
+	if (!pid) {
+		TST_CHECKPOINT_WAIT(0);
+		return;
+	}
+
+	char buf[BUF_LEN];
+
+	SAFE_CG_READ(cg_child_test_simple, "cgroup.procs", buf, BUF_LEN);
+
+	if (atoi(buf) == pid)
+		tst_res(TPASS, "clone3 case pass!");
+	else
+		tst_brk(TFAIL | TTERRNO, "clone3() failed !");
+
+	TST_CHECKPOINT_WAKE(0);
+
+	SAFE_WAITPID(pid, NULL, 0);
+
+}
+
+static void setup(void)
+{
+	clone3_supported_by_kernel();
+
+	cg_child_test_simple = tst_cg_group_mk(tst_cg, "cg_test_simple");
+
+	fd = tst_cg_group_unified_dir_fd(cg_child_test_simple);
+
+	if (fd < 0)
+		tst_brk(TBROK, "get dir fd failed!");
+}
+
+static void cleanup(void)
+{
+	cg_child_test_simple = tst_cg_group_rm(cg_child_test_simple);
+}
+
+static struct tst_test test = {
+	.test_all = run,
+	.setup = setup,
+	.cleanup = cleanup,
+	.forks_child = 1,
+	.max_runtime = 20,
+	.needs_cgroup_ctrls = (const char *const []){ "base", NULL },
+	.needs_cgroup_ver = TST_CG_V2,
+	.needs_checkpoints = 1,
+	.min_kver = "5.7",
+	.bufs = (struct tst_buffers []) {
+		{&args, .size = sizeof(*args)},
+		{},
+	}
+};
-- 
2.35.3


-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

Re: [LTP] [PATCH v5] clone3: Add clone3's clone_args cgroup

[Richard Palethorpe]

Hello,

Wei Gao via ltp <ltp@lists.linux.it> writes:

> Signed-off-by: Wei Gao <wegao@suse.com>
> ---
>  include/lapi/sched.h                        |   8 ++
>  include/tst_cgroup.h                        |   4 +
>  include/tst_clone.h                         |   1 +
>  lib/tst_cgroup.c                            |   9 ++
>  lib/tst_clone.c                             |   1 +
>  runtest/syscalls                            |   1 +
>  testcases/kernel/syscalls/clone3/.gitignore |   1 +
>  testcases/kernel/syscalls/clone3/clone303.c | 101 ++++++++++++++++++++
>  8 files changed, 126 insertions(+)
>  create mode 100644 testcases/kernel/syscalls/clone3/clone303.c
>
> diff --git a/include/lapi/sched.h b/include/lapi/sched.h
> index 1065665d1..ac766efc5 100644
> --- a/include/lapi/sched.h
> +++ b/include/lapi/sched.h
> @@ -13,6 +13,7 @@
>  #include <inttypes.h>
>  #include "config.h"
>  #include "lapi/syscalls.h"
> +#include "lapi/sched.h"
>  
>  struct sched_attr {
>  	uint32_t size;
> @@ -54,6 +55,9 @@ struct clone_args {
>  	uint64_t __attribute__((aligned(8))) stack;
>  	uint64_t __attribute__((aligned(8))) stack_size;
>  	uint64_t __attribute__((aligned(8))) tls;
> +	uint64_t __attribute__((aligned(8))) set_tid;
> +	uint64_t __attribute__((aligned(8))) set_tid_size;
> +	uint64_t __attribute__((aligned(8))) cgroup;
>  };
>  
>  static inline int clone3(struct clone_args *args, size_t size)
> @@ -133,4 +137,8 @@ static inline int getcpu(unsigned *cpu, unsigned *node)
>  # define CLONE_NEWTIME		0x00000080
>  #endif
>  
> +#ifndef CLONE_INTO_CGROUP
> +# define CLONE_INTO_CGROUP 0x200000000ULL
> +#endif
> +
>  #endif /* LAPI_SCHED_H__ */
> diff --git a/include/tst_cgroup.h b/include/tst_cgroup.h
> index 2826ddad1..be14d07c6 100644
> --- a/include/tst_cgroup.h
> +++ b/include/tst_cgroup.h
> @@ -157,6 +157,10 @@ const char *
>  tst_cg_group_name(const struct tst_cg_group *const cg)
>  		      __attribute__ ((nonnull, warn_unused_result));
>  
> +/* This call returns a fd pointing to a v2 directory */
> +int tst_cg_group_unified_dir_fd(const struct tst_cg_group *const cg)
> +		      __attribute__ ((nonnull, warn_unused_result));
> +
>  /* Remove a descendant CGroup */
>  struct tst_cg_group *
>  tst_cg_group_rm(struct tst_cg_group *const cg)
> diff --git a/include/tst_clone.h b/include/tst_clone.h
> index 9ffdc68d1..7b278dfa7 100644
> --- a/include/tst_clone.h
> +++ b/include/tst_clone.h
> @@ -11,6 +11,7 @@
>  struct tst_clone_args {
>  	uint64_t flags;
>  	uint64_t exit_signal;
> +	uint64_t cgroup;

This is not used in the test being added so I will not merge it because
I don't want to do any more work than necessary (I would still merge the
rest of the test, but there is another issue below). The reason is
because it may cause some test which does use tst_clone_args to fail
because it increases the struct size. If some other test does not
initialise the members correctly we may start sending uninitialised data
to the kernel.

In general I don't want to add anything which isn't immediately
necessary without having to think about any potential problems it could
cause.

>  };
>  
>  /* clone3 with fallbacks to clone when possible. Be aware that it
> diff --git a/lib/tst_cgroup.c b/lib/tst_cgroup.c
> index 274c73fea..43055e8cf 100644
> --- a/lib/tst_cgroup.c
> +++ b/lib/tst_cgroup.c
> @@ -1112,6 +1112,15 @@ const char *tst_cg_group_name(const struct tst_cg_group *const cg)
>  	return cg->group_name;
>  }
>  
> +int tst_cg_group_unified_dir_fd(const struct tst_cg_group *const cg)
> +{
> +	for (int i = 0; cg->dirs[i]; i++) {
> +		if (cg->dirs[i]->dir_root->ver == TST_CG_V2)
> +			return cg->dirs[i]->dir_fd;

The loop is unecessary; cg->dirs_by_ctrl[0] is always the V2 directory
if it exists.

Otherwise the test LGTM. I'll set to changes requested in patchwork.

-- 
Thank you,
Richard.

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

[LTP] [PATCH v6] clone3: Add clone3's clone_args cgroup

[Wei Gao via ltp]

Signed-off-by: Wei Gao <wegao@suse.com>
---
 include/lapi/sched.h                        |   8 ++
 include/tst_cgroup.h                        |   4 +
 include/tst_clone.h                         |   1 +
 lib/tst_cgroup.c                            |   8 ++
 lib/tst_clone.c                             |   1 +
 runtest/syscalls                            |   1 +
 testcases/kernel/syscalls/clone3/.gitignore |   1 +
 testcases/kernel/syscalls/clone3/clone303.c | 101 ++++++++++++++++++++
 8 files changed, 125 insertions(+)
 create mode 100644 testcases/kernel/syscalls/clone3/clone303.c

diff --git a/include/lapi/sched.h b/include/lapi/sched.h
index 1065665d1..ac766efc5 100644
--- a/include/lapi/sched.h
+++ b/include/lapi/sched.h
@@ -13,6 +13,7 @@
 #include <inttypes.h>
 #include "config.h"
 #include "lapi/syscalls.h"
+#include "lapi/sched.h"
 
 struct sched_attr {
 	uint32_t size;
@@ -54,6 +55,9 @@ struct clone_args {
 	uint64_t __attribute__((aligned(8))) stack;
 	uint64_t __attribute__((aligned(8))) stack_size;
 	uint64_t __attribute__((aligned(8))) tls;
+	uint64_t __attribute__((aligned(8))) set_tid;
+	uint64_t __attribute__((aligned(8))) set_tid_size;
+	uint64_t __attribute__((aligned(8))) cgroup;
 };
 
 static inline int clone3(struct clone_args *args, size_t size)
@@ -133,4 +137,8 @@ static inline int getcpu(unsigned *cpu, unsigned *node)
 # define CLONE_NEWTIME		0x00000080
 #endif
 
+#ifndef CLONE_INTO_CGROUP
+# define CLONE_INTO_CGROUP 0x200000000ULL
+#endif
+
 #endif /* LAPI_SCHED_H__ */
diff --git a/include/tst_cgroup.h b/include/tst_cgroup.h
index 2826ddad1..be14d07c6 100644
--- a/include/tst_cgroup.h
+++ b/include/tst_cgroup.h
@@ -157,6 +157,10 @@ const char *
 tst_cg_group_name(const struct tst_cg_group *const cg)
 		      __attribute__ ((nonnull, warn_unused_result));
 
+/* This call returns a fd pointing to a v2 directory */
+int tst_cg_group_unified_dir_fd(const struct tst_cg_group *const cg)
+		      __attribute__ ((nonnull, warn_unused_result));
+
 /* Remove a descendant CGroup */
 struct tst_cg_group *
 tst_cg_group_rm(struct tst_cg_group *const cg)
diff --git a/include/tst_clone.h b/include/tst_clone.h
index 9ffdc68d1..7b278dfa7 100644
--- a/include/tst_clone.h
+++ b/include/tst_clone.h
@@ -11,6 +11,7 @@
 struct tst_clone_args {
 	uint64_t flags;
 	uint64_t exit_signal;
+	uint64_t cgroup;
 };
 
 /* clone3 with fallbacks to clone when possible. Be aware that it
diff --git a/lib/tst_cgroup.c b/lib/tst_cgroup.c
index 274c73fea..fdeac6059 100644
--- a/lib/tst_cgroup.c
+++ b/lib/tst_cgroup.c
@@ -1112,6 +1112,14 @@ const char *tst_cg_group_name(const struct tst_cg_group *const cg)
 	return cg->group_name;
 }
 
+int tst_cg_group_unified_dir_fd(const struct tst_cg_group *const cg)
+{
+	if(cg->dirs_by_ctrl[0])
+		return cg->dirs_by_ctrl[0]->dir_fd;
+
+	return -1;
+}
+
 struct tst_cg_group *tst_cg_group_rm(struct tst_cg_group *const cg)
 {
 	struct cgroup_dir **dir;
diff --git a/lib/tst_clone.c b/lib/tst_clone.c
index ecc84408c..2aa00beb1 100644
--- a/lib/tst_clone.c
+++ b/lib/tst_clone.c
@@ -15,6 +15,7 @@ pid_t tst_clone(const struct tst_clone_args *tst_args)
 	struct clone_args args = {
 		.flags = tst_args->flags,
 		.exit_signal = tst_args->exit_signal,
+		.cgroup = tst_args->cgroup,
 	};
 	int flags;
 	pid_t pid = -1;
diff --git a/runtest/syscalls b/runtest/syscalls
index 9c23a4248..0b6adfd7f 100644
--- a/runtest/syscalls
+++ b/runtest/syscalls
@@ -117,6 +117,7 @@ clone09 clone09
 
 clone301 clone301
 clone302 clone302
+clone303 clone303
 
 close01 close01
 close02 close02
diff --git a/testcases/kernel/syscalls/clone3/.gitignore b/testcases/kernel/syscalls/clone3/.gitignore
index 604cb903e..10369954b 100644
--- a/testcases/kernel/syscalls/clone3/.gitignore
+++ b/testcases/kernel/syscalls/clone3/.gitignore
@@ -1,2 +1,3 @@
 clone301
 clone302
+clone303
diff --git a/testcases/kernel/syscalls/clone3/clone303.c b/testcases/kernel/syscalls/clone3/clone303.c
new file mode 100644
index 000000000..5bf02edef
--- /dev/null
+++ b/testcases/kernel/syscalls/clone3/clone303.c
@@ -0,0 +1,101 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * Copyright (c) 2023 SUSE LLC <wegao@suse.com>
+ */
+
+/*\
+ * [Description]
+ *
+ * This test case check clone3 CLONE_INTO_CGROUP flag
+ *
+ */
+
+#define _GNU_SOURCE
+#include <stdio.h>
+#include <stdlib.h>
+#include <sys/wait.h>
+
+#include "tst_test.h"
+#include "lapi/sched.h"
+#include "lapi/pidfd.h"
+
+#define BUF_LEN 20
+
+static struct tst_cg_group *cg_child_test_simple;
+static int fd;
+static struct clone_args *args;
+
+static pid_t clone_into_cgroup(int cgroup_fd)
+{
+	pid_t pid;
+
+	args->flags = CLONE_INTO_CGROUP;
+	args->exit_signal = SIGCHLD;
+	args->cgroup = cgroup_fd;
+
+	pid = clone3(args, sizeof(*args));
+
+	if (pid < 0)
+		tst_res(TFAIL | TTERRNO, "clone3() failed !");
+
+	return pid;
+}
+
+static void run(void)
+{
+	pid_t pid;
+
+	pid = clone_into_cgroup(fd);
+
+	if (!pid) {
+		TST_CHECKPOINT_WAIT(0);
+		return;
+	}
+
+	char buf[BUF_LEN];
+
+	SAFE_CG_READ(cg_child_test_simple, "cgroup.procs", buf, BUF_LEN);
+
+	if (atoi(buf) == pid)
+		tst_res(TPASS, "clone3 case pass!");
+	else
+		tst_brk(TFAIL | TTERRNO, "clone3() failed !");
+
+	TST_CHECKPOINT_WAKE(0);
+
+	SAFE_WAITPID(pid, NULL, 0);
+
+}
+
+static void setup(void)
+{
+	clone3_supported_by_kernel();
+
+	cg_child_test_simple = tst_cg_group_mk(tst_cg, "cg_test_simple");
+
+	fd = tst_cg_group_unified_dir_fd(cg_child_test_simple);
+
+	if (fd < 0)
+		tst_brk(TBROK, "get dir fd failed!");
+}
+
+static void cleanup(void)
+{
+	cg_child_test_simple = tst_cg_group_rm(cg_child_test_simple);
+}
+
+static struct tst_test test = {
+	.test_all = run,
+	.setup = setup,
+	.cleanup = cleanup,
+	.forks_child = 1,
+	.max_runtime = 20,
+	.needs_cgroup_ctrls = (const char *const []){ "base", NULL },
+	.needs_cgroup_ver = TST_CG_V2,
+	.needs_checkpoints = 1,
+	.min_kver = "5.7",
+	.bufs = (struct tst_buffers []) {
+		{&args, .size = sizeof(*args)},
+		{},
+	}
+};
-- 
2.35.3


-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

Re: [LTP] [PATCH v5] clone3: Add clone3's clone_args cgroup

[Wei Gao via ltp]

Hi Richard

Thanks for your review. I flag my comments in former email start with [GW].

Thanks.
Regards
Gao Wei


-----Original Message-----
From: Richard Palethorpe <rpalethorpe@suse.de> 
Sent: Friday, August 25, 2023 6:36 PM
To: Wei Gao <wegao@suse.com>
Cc: ltp@lists.linux.it
Subject: Re: [LTP] [PATCH v5] clone3: Add clone3's clone_args cgroup

Hello,

Wei Gao via ltp <ltp@lists.linux.it> writes:

> Signed-off-by: Wei Gao <wegao@suse.com>
> ---
>  include/lapi/sched.h                        |   8 ++
>  include/tst_cgroup.h                        |   4 +
>  include/tst_clone.h                         |   1 +
>  lib/tst_cgroup.c                            |   9 ++
>  lib/tst_clone.c                             |   1 +
>  runtest/syscalls                            |   1 +
>  testcases/kernel/syscalls/clone3/.gitignore |   1 +
>  testcases/kernel/syscalls/clone3/clone303.c | 101 
> ++++++++++++++++++++
>  8 files changed, 126 insertions(+)
>  create mode 100644 testcases/kernel/syscalls/clone3/clone303.c
>
> diff --git a/include/lapi/sched.h b/include/lapi/sched.h index 
> 1065665d1..ac766efc5 100644
> --- a/include/lapi/sched.h
> +++ b/include/lapi/sched.h
> @@ -13,6 +13,7 @@
>  #include <inttypes.h>
>  #include "config.h"
>  #include "lapi/syscalls.h"
> +#include "lapi/sched.h"
>  
>  struct sched_attr {
>  	uint32_t size;
> @@ -54,6 +55,9 @@ struct clone_args {
>  	uint64_t __attribute__((aligned(8))) stack;
>  	uint64_t __attribute__((aligned(8))) stack_size;
>  	uint64_t __attribute__((aligned(8))) tls;
> +	uint64_t __attribute__((aligned(8))) set_tid;
> +	uint64_t __attribute__((aligned(8))) set_tid_size;
> +	uint64_t __attribute__((aligned(8))) cgroup;
>  };
>  
>  static inline int clone3(struct clone_args *args, size_t size) @@ 
> -133,4 +137,8 @@ static inline int getcpu(unsigned *cpu, unsigned *node)
>  # define CLONE_NEWTIME		0x00000080
>  #endif
>  
> +#ifndef CLONE_INTO_CGROUP
> +# define CLONE_INTO_CGROUP 0x200000000ULL #endif
> +
>  #endif /* LAPI_SCHED_H__ */
> diff --git a/include/tst_cgroup.h b/include/tst_cgroup.h index 
> 2826ddad1..be14d07c6 100644
> --- a/include/tst_cgroup.h
> +++ b/include/tst_cgroup.h
> @@ -157,6 +157,10 @@ const char *
>  tst_cg_group_name(const struct tst_cg_group *const cg)
>  		      __attribute__ ((nonnull, warn_unused_result));
>  
> +/* This call returns a fd pointing to a v2 directory */ int 
> +tst_cg_group_unified_dir_fd(const struct tst_cg_group *const cg)
> +		      __attribute__ ((nonnull, warn_unused_result));
> +
>  /* Remove a descendant CGroup */
>  struct tst_cg_group *
>  tst_cg_group_rm(struct tst_cg_group *const cg) diff --git 
> a/include/tst_clone.h b/include/tst_clone.h index 9ffdc68d1..7b278dfa7 
> 100644
> --- a/include/tst_clone.h
> +++ b/include/tst_clone.h
> @@ -11,6 +11,7 @@
>  struct tst_clone_args {
>  	uint64_t flags;
>  	uint64_t exit_signal;
> +	uint64_t cgroup;

This is not used in the test being added so I will not merge it because I don't want to do any more work than necessary (I would still merge the rest of the test, but there is another issue below). The reason is because it may cause some test which does use tst_clone_args to fail because it increases the struct size. If some other test does not initialise the members correctly we may start sending uninitialised data to the kernel.

In general I don't want to add anything which isn't immediately necessary without having to think about any potential problems it could cause.

[GW]:  The point of this case is test cgroup parameter, if you remove this then following error will happen:
tst_clone.c:18:21: error: 'const struct tst_clone_args' has no member named 'cgroup'
   .cgroup = tst_args->cgroup,


>  };
>  
>  /* clone3 with fallbacks to clone when possible. Be aware that it 
> diff --git a/lib/tst_cgroup.c b/lib/tst_cgroup.c index 
> 274c73fea..43055e8cf 100644
> --- a/lib/tst_cgroup.c
> +++ b/lib/tst_cgroup.c
> @@ -1112,6 +1112,15 @@ const char *tst_cg_group_name(const struct tst_cg_group *const cg)
>  	return cg->group_name;
>  }
>  
> +int tst_cg_group_unified_dir_fd(const struct tst_cg_group *const cg) 
> +{
> +	for (int i = 0; cg->dirs[i]; i++) {
> +		if (cg->dirs[i]->dir_root->ver == TST_CG_V2)
> +			return cg->dirs[i]->dir_fd;

The loop is unecessary; cg->dirs_by_ctrl[0] is always the V2 directory if it exists.

[GW]: I have updated and sent new Patch

Otherwise the test LGTM. I'll set to changes requested in patchwork.

--
Thank you,
Richard.

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

Re: [LTP] [PATCH v5] clone3: Add clone3's clone_args cgroup

[Richard Palethorpe]

Hello,

Wei Gao <wegao@suse.com> writes:

> Hi Richard
>
> Thanks for your review. I flag my comments in former email start with [GW].
>
> Thanks.
> Regards
> Gao Wei
>
>
> -----Original Message-----
> From: Richard Palethorpe <rpalethorpe@suse.de> 
> Sent: Friday, August 25, 2023 6:36 PM
> To: Wei Gao <wegao@suse.com>
> Cc: ltp@lists.linux.it
> Subject: Re: [LTP] [PATCH v5] clone3: Add clone3's clone_args cgroup
>
> Hello,
>
> Wei Gao via ltp <ltp@lists.linux.it> writes:
>
>> Signed-off-by: Wei Gao <wegao@suse.com>
>> ---
>>  include/lapi/sched.h                        |   8 ++
>>  include/tst_cgroup.h                        |   4 +
>>  include/tst_clone.h                         |   1 +
>>  lib/tst_cgroup.c                            |   9 ++
>>  lib/tst_clone.c                             |   1 +
>>  runtest/syscalls                            |   1 +
>>  testcases/kernel/syscalls/clone3/.gitignore |   1 +
>>  testcases/kernel/syscalls/clone3/clone303.c | 101 
>> ++++++++++++++++++++
>>  8 files changed, 126 insertions(+)
>>  create mode 100644 testcases/kernel/syscalls/clone3/clone303.c
>>
>> diff --git a/include/lapi/sched.h b/include/lapi/sched.h index 
>> 1065665d1..ac766efc5 100644
>> --- a/include/lapi/sched.h
>> +++ b/include/lapi/sched.h
>> @@ -13,6 +13,7 @@
>>  #include <inttypes.h>
>>  #include "config.h"
>>  #include "lapi/syscalls.h"
>> +#include "lapi/sched.h"
>>  
>>  struct sched_attr {
>>  	uint32_t size;
>> @@ -54,6 +55,9 @@ struct clone_args {
>>  	uint64_t __attribute__((aligned(8))) stack;
>>  	uint64_t __attribute__((aligned(8))) stack_size;
>>  	uint64_t __attribute__((aligned(8))) tls;
>> +	uint64_t __attribute__((aligned(8))) set_tid;
>> +	uint64_t __attribute__((aligned(8))) set_tid_size;
>> +	uint64_t __attribute__((aligned(8))) cgroup;
>>  };
>>  
>>  static inline int clone3(struct clone_args *args, size_t size) @@ 
>> -133,4 +137,8 @@ static inline int getcpu(unsigned *cpu, unsigned *node)
>>  # define CLONE_NEWTIME		0x00000080
>>  #endif
>>  
>> +#ifndef CLONE_INTO_CGROUP
>> +# define CLONE_INTO_CGROUP 0x200000000ULL #endif
>> +
>>  #endif /* LAPI_SCHED_H__ */
>> diff --git a/include/tst_cgroup.h b/include/tst_cgroup.h index 
>> 2826ddad1..be14d07c6 100644
>> --- a/include/tst_cgroup.h
>> +++ b/include/tst_cgroup.h
>> @@ -157,6 +157,10 @@ const char *
>>  tst_cg_group_name(const struct tst_cg_group *const cg)
>>  		      __attribute__ ((nonnull, warn_unused_result));
>>  
>> +/* This call returns a fd pointing to a v2 directory */ int 
>> +tst_cg_group_unified_dir_fd(const struct tst_cg_group *const cg)
>> +		      __attribute__ ((nonnull, warn_unused_result));
>> +
>>  /* Remove a descendant CGroup */
>>  struct tst_cg_group *
>>  tst_cg_group_rm(struct tst_cg_group *const cg) diff --git 
>> a/include/tst_clone.h b/include/tst_clone.h index 9ffdc68d1..7b278dfa7 
>> 100644
>> --- a/include/tst_clone.h
>> +++ b/include/tst_clone.h
>> @@ -11,6 +11,7 @@
>>  struct tst_clone_args {
>>  	uint64_t flags;
>>  	uint64_t exit_signal;
>> +	uint64_t cgroup;
>
> This is not used in the test being added so I will not merge it because I don't want to do any more work than necessary (I would still merge the rest of the test, but there is another issue below). The reason is because it may cause some test which does use tst_clone_args to fail because it increases the struct size. If some other test does not initialise the members correctly we may start sending uninitialised data to the kernel.
>
> In general I don't want to add anything which isn't immediately necessary without having to think about any potential problems it could cause.
>
> [GW]:  The point of this case is test cgroup parameter, if you remove this then following error will happen:
> tst_clone.c:18:21: error: 'const struct tst_clone_args' has no member named 'cgroup'
>    .cgroup = tst_args->cgroup,

But you use clone3 directly, I don't understand where tst_clone is being
used in the test?

>
>
>>  };
>>  
>>  /* clone3 with fallbacks to clone when possible. Be aware that it 
>> diff --git a/lib/tst_cgroup.c b/lib/tst_cgroup.c index 
>> 274c73fea..43055e8cf 100644
>> --- a/lib/tst_cgroup.c
>> +++ b/lib/tst_cgroup.c
>> @@ -1112,6 +1112,15 @@ const char *tst_cg_group_name(const struct tst_cg_group *const cg)
>>  	return cg->group_name;
>>  }
>>  
>> +int tst_cg_group_unified_dir_fd(const struct tst_cg_group *const cg) 
>> +{
>> +	for (int i = 0; cg->dirs[i]; i++) {
>> +		if (cg->dirs[i]->dir_root->ver == TST_CG_V2)
>> +			return cg->dirs[i]->dir_fd;
>
> The loop is unecessary; cg->dirs_by_ctrl[0] is always the V2 directory if it exists.
>
> [GW]: I have updated and sent new Patch
>
> Otherwise the test LGTM. I'll set to changes requested in patchwork.


-- 
Thank you,
Richard.

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

[LTP] [PATCH v7] clone3: Add clone3's clone_args cgroup

[Wei Gao via ltp]

Signed-off-by: Wei Gao <wegao@suse.com>
---
 include/lapi/sched.h                        |  8 ++
 include/tst_cgroup.h                        |  4 +
 include/tst_clone.h                         |  1 +
 lib/tst_cgroup.c                            |  8 ++
 lib/tst_clone.c                             |  1 +
 runtest/syscalls                            |  1 +
 testcases/kernel/syscalls/clone3/.gitignore |  1 +
 testcases/kernel/syscalls/clone3/clone303.c | 95 +++++++++++++++++++++
 8 files changed, 119 insertions(+)
 create mode 100644 testcases/kernel/syscalls/clone3/clone303.c

diff --git a/include/lapi/sched.h b/include/lapi/sched.h
index 1065665d1..ac766efc5 100644
--- a/include/lapi/sched.h
+++ b/include/lapi/sched.h
@@ -13,6 +13,7 @@
 #include <inttypes.h>
 #include "config.h"
 #include "lapi/syscalls.h"
+#include "lapi/sched.h"
 
 struct sched_attr {
 	uint32_t size;
@@ -54,6 +55,9 @@ struct clone_args {
 	uint64_t __attribute__((aligned(8))) stack;
 	uint64_t __attribute__((aligned(8))) stack_size;
 	uint64_t __attribute__((aligned(8))) tls;
+	uint64_t __attribute__((aligned(8))) set_tid;
+	uint64_t __attribute__((aligned(8))) set_tid_size;
+	uint64_t __attribute__((aligned(8))) cgroup;
 };
 
 static inline int clone3(struct clone_args *args, size_t size)
@@ -133,4 +137,8 @@ static inline int getcpu(unsigned *cpu, unsigned *node)
 # define CLONE_NEWTIME		0x00000080
 #endif
 
+#ifndef CLONE_INTO_CGROUP
+# define CLONE_INTO_CGROUP 0x200000000ULL
+#endif
+
 #endif /* LAPI_SCHED_H__ */
diff --git a/include/tst_cgroup.h b/include/tst_cgroup.h
index 2826ddad1..be14d07c6 100644
--- a/include/tst_cgroup.h
+++ b/include/tst_cgroup.h
@@ -157,6 +157,10 @@ const char *
 tst_cg_group_name(const struct tst_cg_group *const cg)
 		      __attribute__ ((nonnull, warn_unused_result));
 
+/* This call returns a fd pointing to a v2 directory */
+int tst_cg_group_unified_dir_fd(const struct tst_cg_group *const cg)
+		      __attribute__ ((nonnull, warn_unused_result));
+
 /* Remove a descendant CGroup */
 struct tst_cg_group *
 tst_cg_group_rm(struct tst_cg_group *const cg)
diff --git a/include/tst_clone.h b/include/tst_clone.h
index 9ffdc68d1..7b278dfa7 100644
--- a/include/tst_clone.h
+++ b/include/tst_clone.h
@@ -11,6 +11,7 @@
 struct tst_clone_args {
 	uint64_t flags;
 	uint64_t exit_signal;
+	uint64_t cgroup;
 };
 
 /* clone3 with fallbacks to clone when possible. Be aware that it
diff --git a/lib/tst_cgroup.c b/lib/tst_cgroup.c
index 274c73fea..fdeac6059 100644
--- a/lib/tst_cgroup.c
+++ b/lib/tst_cgroup.c
@@ -1112,6 +1112,14 @@ const char *tst_cg_group_name(const struct tst_cg_group *const cg)
 	return cg->group_name;
 }
 
+int tst_cg_group_unified_dir_fd(const struct tst_cg_group *const cg)
+{
+	if(cg->dirs_by_ctrl[0])
+		return cg->dirs_by_ctrl[0]->dir_fd;
+
+	return -1;
+}
+
 struct tst_cg_group *tst_cg_group_rm(struct tst_cg_group *const cg)
 {
 	struct cgroup_dir **dir;
diff --git a/lib/tst_clone.c b/lib/tst_clone.c
index ecc84408c..2aa00beb1 100644
--- a/lib/tst_clone.c
+++ b/lib/tst_clone.c
@@ -15,6 +15,7 @@ pid_t tst_clone(const struct tst_clone_args *tst_args)
 	struct clone_args args = {
 		.flags = tst_args->flags,
 		.exit_signal = tst_args->exit_signal,
+		.cgroup = tst_args->cgroup,
 	};
 	int flags;
 	pid_t pid = -1;
diff --git a/runtest/syscalls b/runtest/syscalls
index 9c23a4248..0b6adfd7f 100644
--- a/runtest/syscalls
+++ b/runtest/syscalls
@@ -117,6 +117,7 @@ clone09 clone09
 
 clone301 clone301
 clone302 clone302
+clone303 clone303
 
 close01 close01
 close02 close02
diff --git a/testcases/kernel/syscalls/clone3/.gitignore b/testcases/kernel/syscalls/clone3/.gitignore
index 604cb903e..10369954b 100644
--- a/testcases/kernel/syscalls/clone3/.gitignore
+++ b/testcases/kernel/syscalls/clone3/.gitignore
@@ -1,2 +1,3 @@
 clone301
 clone302
+clone303
diff --git a/testcases/kernel/syscalls/clone3/clone303.c b/testcases/kernel/syscalls/clone3/clone303.c
new file mode 100644
index 000000000..767d7c1a9
--- /dev/null
+++ b/testcases/kernel/syscalls/clone3/clone303.c
@@ -0,0 +1,95 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * Copyright (c) 2023 SUSE LLC <wegao@suse.com>
+ */
+
+/*\
+ * [Description]
+ *
+ * This test case check clone3 CLONE_INTO_CGROUP flag
+ *
+ */
+
+#define _GNU_SOURCE
+#include <stdio.h>
+#include <stdlib.h>
+#include <sys/wait.h>
+
+#include "tst_test.h"
+#include "lapi/sched.h"
+#include "lapi/pidfd.h"
+
+#define BUF_LEN 20
+
+static struct tst_cg_group *cg_child_test_simple;
+static int fd;
+static struct tst_clone_args *args;
+
+static pid_t clone_into_cgroup(int cgroup_fd)
+{
+
+	args->flags = CLONE_INTO_CGROUP;
+	args->exit_signal = SIGCHLD;
+	args->cgroup = cgroup_fd;
+
+	return tst_clone(args);
+}
+
+static void run(void)
+{
+	pid_t pid;
+
+	pid = clone_into_cgroup(fd);
+
+	if (!pid) {
+		TST_CHECKPOINT_WAIT(0);
+		return;
+	}
+
+	char buf[BUF_LEN];
+
+	SAFE_CG_READ(cg_child_test_simple, "cgroup.procs", buf, BUF_LEN);
+
+	if (atoi(buf) == pid)
+		tst_res(TPASS, "clone3 case pass!");
+	else
+		tst_brk(TFAIL | TTERRNO, "clone3() failed !");
+
+	TST_CHECKPOINT_WAKE(0);
+
+	SAFE_WAITPID(pid, NULL, 0);
+
+}
+
+static void setup(void)
+{
+	clone3_supported_by_kernel();
+
+	cg_child_test_simple = tst_cg_group_mk(tst_cg, "cg_test_simple");
+
+	fd = tst_cg_group_unified_dir_fd(cg_child_test_simple);
+
+	if (fd < 0)
+		tst_brk(TBROK, "get dir fd failed!");
+}
+
+static void cleanup(void)
+{
+	cg_child_test_simple = tst_cg_group_rm(cg_child_test_simple);
+}
+
+static struct tst_test test = {
+	.test_all = run,
+	.setup = setup,
+	.cleanup = cleanup,
+	.forks_child = 1,
+	.max_runtime = 20,
+	.needs_cgroup_ctrls = (const char *const []){ "base", NULL },
+	.needs_cgroup_ver = TST_CG_V2,
+	.needs_checkpoints = 1,
+	.min_kver = "5.7",
+	.bufs = (struct tst_buffers []) {
+		{&args, .size = sizeof(*args)},
+		{},
+	}
+};
-- 
2.35.3


-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

Re: [LTP] [PATCH v7] clone3: Add clone3's clone_args cgroup

[Richard Palethorpe]

Hello,

Wei Gao via ltp <ltp@lists.linux.it> writes:

> +
> +static struct tst_test test = {
> +	.test_all = run,
> +	.setup = setup,
> +	.cleanup = cleanup,
> +	.forks_child = 1,
> +	.max_runtime = 20,

I removed this and merged!

-- 
Thank you,
Richard.

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

Re: [LTP] [PATCH v7] clone3: Add clone3's clone_args cgroup

[Petr Vorel]

Hi Wei, Richie,

> Hello,

> Wei Gao via ltp <ltp@lists.linux.it> writes:

> > +
> > +static struct tst_test test = {
> > +	.test_all = run,
> > +	.setup = setup,
> > +	.cleanup = cleanup,
> > +	.forks_child = 1,
> > +	.max_runtime = 20,

> I removed this and merged!

This change, merged as 45f6916ba ("clone3: Add clone3's clone_args cgroup")
caused failing clone302:

tst_buffers.c:56: TINFO: Test is using guarded buffers
tst_test.c:1559: TINFO: Timeout per run is 0h 00m 30s
clone302.c:101: TPASS: invalid args: clone3() failed as expected: EFAULT (14)
clone302.c:101: TPASS: zero size: clone3() failed as expected: EINVAL (22)
clone302.c:91: TFAIL: short size: clone3() passed unexpectedly
clone302.c:101: TPASS: extra size: clone3() failed as expected: EFAULT (14)
clone302.c:101: TPASS: sighand-no-VM: clone3() failed as expected: EINVAL (22)
clone302.c:101: TPASS: thread-no-sighand: clone3() failed as expected: EINVAL (22)
clone302.c:101: TPASS: fs-newns: clone3() failed as expected: EINVAL (22)
clone302.c:101: TPASS: invalid pidfd: clone3() failed as expected: EFAULT (14)
clone302.c:101: TPASS: invalid signal: clone3() failed as expected: EINVAL (22)
clone302.c:101: TPASS: zero-stack-size: clone3() failed as expected: EINVAL (22)
clone302.c:101: TPASS: invalid-stack: clone3() failed as expected: EINVAL (22)

Summary:
passed   10
failed   1
broken   0
skipped  0
warnings 0
### TEST clone302 COMPLETE >>> 1.

Could you please have look and fix it?

Kind regards,
Petr

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

Re: [LTP] [PATCH v7] clone3: Add clone3's clone_args cgroup

[Petr Vorel]

> Hi Wei, Richie,

> > Hello,

> > Wei Gao via ltp <ltp@lists.linux.it> writes:

> > > +
> > > +static struct tst_test test = {
> > > +	.test_all = run,
> > > +	.setup = setup,
> > > +	.cleanup = cleanup,
> > > +	.forks_child = 1,
> > > +	.max_runtime = 20,

> > I removed this and merged!

> This change, merged as 45f6916ba ("clone3: Add clone3's clone_args cgroup")
> caused failing clone302:

> tst_buffers.c:56: TINFO: Test is using guarded buffers
> tst_test.c:1559: TINFO: Timeout per run is 0h 00m 30s
> clone302.c:101: TPASS: invalid args: clone3() failed as expected: EFAULT (14)
> clone302.c:101: TPASS: zero size: clone3() failed as expected: EINVAL (22)
> clone302.c:91: TFAIL: short size: clone3() passed unexpectedly
> clone302.c:101: TPASS: extra size: clone3() failed as expected: EFAULT (14)
> clone302.c:101: TPASS: sighand-no-VM: clone3() failed as expected: EINVAL (22)
> clone302.c:101: TPASS: thread-no-sighand: clone3() failed as expected: EINVAL (22)
> clone302.c:101: TPASS: fs-newns: clone3() failed as expected: EINVAL (22)
> clone302.c:101: TPASS: invalid pidfd: clone3() failed as expected: EFAULT (14)
> clone302.c:101: TPASS: invalid signal: clone3() failed as expected: EINVAL (22)
> clone302.c:101: TPASS: zero-stack-size: clone3() failed as expected: EINVAL (22)
> clone302.c:101: TPASS: invalid-stack: clone3() failed as expected: EINVAL (22)

FYI include/lapi/sched.h change caused the regression.

Kind regards,
Petr

> Summary:
> passed   10
> failed   1
> broken   0
> skipped  0
> warnings 0
> ### TEST clone302 COMPLETE >>> 1.

> Could you please have look and fix it?

> Kind regards,
> Petr

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

Re: [LTP] [PATCH v7] clone3: Add clone3's clone_args cgroup

[Wei Gao via ltp]

On Fri, Sep 01, 2023 at 12:19:54PM +0200, Petr Vorel wrote:
> Hi Wei, Richie,
> 
> > Hello,
> 
> > Wei Gao via ltp <ltp@lists.linux.it> writes:
> 
> > > +
> > > +static struct tst_test test = {
> > > +	.test_all = run,
> > > +	.setup = setup,
> > > +	.cleanup = cleanup,
> > > +	.forks_child = 1,
> > > +	.max_runtime = 20,
> 
> > I removed this and merged!
> 
> This change, merged as 45f6916ba ("clone3: Add clone3's clone_args cgroup")
> caused failing clone302:
> 
> tst_buffers.c:56: TINFO: Test is using guarded buffers
> tst_test.c:1559: TINFO: Timeout per run is 0h 00m 30s
> clone302.c:101: TPASS: invalid args: clone3() failed as expected: EFAULT (14)
> clone302.c:101: TPASS: zero size: clone3() failed as expected: EINVAL (22)
> clone302.c:91: TFAIL: short size: clone3() passed unexpectedly
> clone302.c:101: TPASS: extra size: clone3() failed as expected: EFAULT (14)
> clone302.c:101: TPASS: sighand-no-VM: clone3() failed as expected: EINVAL (22)
> clone302.c:101: TPASS: thread-no-sighand: clone3() failed as expected: EINVAL (22)
> clone302.c:101: TPASS: fs-newns: clone3() failed as expected: EINVAL (22)
> clone302.c:101: TPASS: invalid pidfd: clone3() failed as expected: EFAULT (14)
> clone302.c:101: TPASS: invalid signal: clone3() failed as expected: EINVAL (22)
> clone302.c:101: TPASS: zero-stack-size: clone3() failed as expected: EINVAL (22)
> clone302.c:101: TPASS: invalid-stack: clone3() failed as expected: EINVAL (22)
> 
> Summary:
> passed   10
> failed   1
> broken   0
> skipped  0
> warnings 0
> ### TEST clone302 COMPLETE >>> 1.
> 
> Could you please have look and fix it?
No problem, i will check this.
> 
> Kind regards,
> Petr

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

Re: [LTP] [PATCH v7] clone3: Add clone3's clone_args cgroup

[Petr Vorel]

> Signed-off-by: Wei Gao <wegao@suse.com>
> ---
>  include/lapi/sched.h                        |  8 ++
>  include/tst_cgroup.h                        |  4 +
>  include/tst_clone.h                         |  1 +
>  lib/tst_cgroup.c                            |  8 ++
>  lib/tst_clone.c                             |  1 +
>  runtest/syscalls                            |  1 +
>  testcases/kernel/syscalls/clone3/.gitignore |  1 +
>  testcases/kernel/syscalls/clone3/clone303.c | 95 +++++++++++++++++++++
Note lib changes should have been separated from adding new test clone303.c
(commit would be readable).

>  8 files changed, 119 insertions(+)
>  create mode 100644 testcases/kernel/syscalls/clone3/clone303.c

> diff --git a/include/lapi/sched.h b/include/lapi/sched.h
> index 1065665d1..ac766efc5 100644
> --- a/include/lapi/sched.h
> +++ b/include/lapi/sched.h
> @@ -13,6 +13,7 @@
>  #include <inttypes.h>
>  #include "config.h"
>  #include "lapi/syscalls.h"
> +#include "lapi/sched.h"

>  struct sched_attr {
>  	uint32_t size;
> @@ -54,6 +55,9 @@ struct clone_args {
>  	uint64_t __attribute__((aligned(8))) stack;
>  	uint64_t __attribute__((aligned(8))) stack_size;
>  	uint64_t __attribute__((aligned(8))) tls;
> +	uint64_t __attribute__((aligned(8))) set_tid;
> +	uint64_t __attribute__((aligned(8))) set_tid_size;
> +	uint64_t __attribute__((aligned(8))) cgroup;

Adding new members introduced warnings like this:

tst_ns_exec.c: In function ‘main’:
tst_ns_exec.c:70:16: warning: missing initializer for field ‘cgroup’ of ‘struct tst_clone_args’ [-Wmissing-field-initializers]
   70 |         struct tst_clone_args args = { 0, SIGCHLD };
      |                ^~~~~~~~~~~~~~
In file included from ../../include/tst_test.h:113,
                 from tst_ns_exec.c:20:
../../include/tst_clone.h:14:18: note: ‘cgroup’ declared here
   14 |         uint64_t cgroup;
      |                  ^~~~~~

I'm going to fix it.

Kind regards,
Petr

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp