Re: [LTP] [PATCH v4] kill01: New case cgroup kill

Wei Gao via ltp
2023-03-07T08:28:13Z

On Tue, Mar 07, 2023 at 03:13:56PM +0800, Li Wang wrote:
> Hi Wei,
>
> The new test generally looks good to me.
>
> I'm thinking maybe we make use of guarded buffers which could
> remove the setup/cleanup function then. And doing memset(buf,...)
> before reading into buf[] is quite necessary, to avoid someone
> running this test with parameter '-i N' that the buf[] is polluted
> by the first time.
>
> https://github.com/linux-test-project/ltp/blob/master/doc/c-test-api.txt#L1799
>
> <https://github.com/linux-test-project/ltp/blob/master/doc/c-test-api.txt#L1799>
> e.g.

I have tried use guarded buffers but compile can not pass.
Following error will trigger since buf_len is caculate instead of FIX constant. So
I suppose we can not use guarded buffers?

kill01.c:131:34: error: initializer element is not constant
.bufs = (struct tst_buffers []) {

For memset before reading buf issue, i will update in next patch.

>
> --- a/testcases/kernel/controllers/kill/kill01.c
> +++ b/testcases/kernel/controllers/kill/kill01.c
> @@ -26,7 +26,7 @@
>
> #define MAX_PID_NUM 100
> #define pid_num MIN(MAX_PID_NUM, (tst_ncpus_available() + 1))
> -#define buf_len (20 * pid_num)
> +#define buf_len (10 * MAX_PID_NUM)
>
> static char *buf;
> static struct tst_cg_group *cg_child_test_simple;
> @@ -60,7 +60,6 @@ static void child_fn(void)
>
> while (getppid() == ppid)
> usleep(1000);
> -
> }
>
> static int cg_run_nowait(const struct tst_cg_group *const cg,
> @@ -82,6 +81,8 @@ static int cg_wait_for_proc_count(const struct
> tst_cg_group *cg, int count)
> int attempts;
> char *ptr;
>
> + memset(buf, 0, buf_len);
> +
> for (attempts = 100; attempts >= 0; attempts--) {
> int nr = 0;
>
> @@ -121,23 +122,14 @@ static void run(void)
> cg_child_test_simple = tst_cg_group_rm(cg_child_test_simple);
> }
>
> -static void setup(void)
> -{
> - buf = SAFE_MALLOC(buf_len);
> - memset(buf, 0, buf_len);
> -}
> -
> -static void cleanup(void)
> -{
> - free(buf);
> -}
> -
> static struct tst_test test = {
> .test_all = run,
> - .setup = setup,
> - .cleanup = cleanup,
> .forks_child = 1,
> .max_runtime = 15,
> .needs_cgroup_ctrls = (const char *const []){ "memory", NULL },
> .needs_cgroup_ver = TST_CG_V2,
> + .bufs = (struct tst_buffers []) {
> + {&buf, .size = buf_len},
> + {}
> + }
> };
>
> Anyway this version you could add my reviewed-by:
>
> Reviewed-by: Li Wang <liwang@redhat.com>
>
>
> --
> Regards,
> Li Wang