Re: [LTP] [PATCH v1] sched_setscheduler: test SCHED_RESET_ON_FORK

[Richard Palethorpe <rpalethorpe@suse.de>]

Hello,

This patch doesn't apply, the PGP signature appears to be confusing 'git
am'.

Also you need to add the
"Signed-off-by: Federico Bonfiglio <federico.bonfiglio@protonmail.ch>"
trailer to the commit message.

I suppose it should be possible to send the patch with "git send-email"
from any e-mail address using whatever SMTP server. Git should CC in
your proton mail address if you use it in the Signed-off-by trailer.

From just looking at it, the actual patch looks good with just one
comment below.

Federico Bonfiglio via ltp <ltp@lists.linux.it> writes:

> [[PGP Signed Part:Undecided]]
> ---
>  runtest/syscalls                              |  1 +
>  .../syscalls/sched_setscheduler/.gitignore    |  1 +
>  .../sched_setscheduler/sched_setscheduler04.c | 71 +++++++++++++++++++
>  3 files changed, 73 insertions(+)
>  create mode 100644 testcases/kernel/syscalls/sched_setscheduler/sched_setscheduler04.c
>
> diff --git a/runtest/syscalls b/runtest/syscalls
> index 3dc6fa397..ff516af3d 100644
> --- a/runtest/syscalls
> +++ b/runtest/syscalls
> @@ -1204,6 +1204,7 @@ sched_getscheduler02 sched_getscheduler02
>  sched_setscheduler01 sched_setscheduler01
>  sched_setscheduler02 sched_setscheduler02
>  sched_setscheduler03 sched_setscheduler03
> +sched_setscheduler04 sched_setscheduler04
>
>  sched_yield01 sched_yield01
>
> diff --git a/testcases/kernel/syscalls/sched_setscheduler/.gitignore b/testcases/kernel/syscalls/sched_setscheduler/.gitignore
> index aa8ad9695..1b8860d2c 100644
> --- a/testcases/kernel/syscalls/sched_setscheduler/.gitignore
> +++ b/testcases/kernel/syscalls/sched_setschedule
> r/.gitignore
> @@ -1,3 +1,4 @@
>  /sched_setscheduler01
>  /sched_setscheduler02
>  /sched_setscheduler03
> +/sched_setscheduler04
> diff --git a/testcases/kernel/syscalls/sched_setscheduler/sched_setscheduler04.c b/testcases/kernel/syscalls/sched_setscheduler/sched_setscheduler04.c
> new file mode 100644
> index 000000000..f2643f001
> --- /dev/null
> +++ b/testcases/kernel/syscalls/sched_setscheduler/sched_setscheduler04.c
> @@ -0,0 +1,71 @@
> +// SPDX-License-Identifier: GPL-2.0-or-later
> +/*
> + * Copyright (c) 2022 Federico Bonfiglio <federico.bonfiglio@protonmail.ch>
> + */
> +
> +/*
> + * [Description]
> + *
> + * Testcases that test if sched_setscheduler with flag
> + * SCHED_RESET_ON_FORK restores children policy to
> + * SCHED_NORMAL.
> + *
> + */
> +
> +#define _GNU_SOURCE
> +#include <stdio.h>
> +#include <stdlib.h>
> +#include <sched.h>
> +#include <linux/sched.h>
> +
> +#include "tst_test.h"
> +#include "tst_sched.h"
> +
> +struct test_case_t {
> +	int policy;
> +	char *desc;
> +};
> +
> +static struct test_cas
> e_t cases[] = {
> +	{
> +		.policy = SCHED_FIFO,
> +		.desc = "SCHED_FIFO"
> +	},
> +	{
> +		.policy = SCHED_RR,
> +		.desc = "SCHED_RR"
> +	}
> +};
> +
> +static void test_reset_on_fork(unsigned int i)
> +{
> +	struct sched_variant *tv = &sched_variants[tst_variant];
> +	struct test_case_t *tc = &cases[i];
> +
> +	tst_res(TINFO, "Testing %s variant", tv->desc);
> +	tst_res(TINFO, "Testing %s policy", tc->desc);
> +
> +	struct sched_param param = { .sched_priority = 1 };
> +
> +	tv->sched_setscheduler(getpid(), tc->policy | SCHED_RESET_ON_FORK, &param);
> +
> +	pid_t pid = SAFE_FORK();
> +
> +	if (!pid == 0) {
> +		if (sched_getscheduler(pid) == SCHED_NORMAL)
> +			tst_res(TPASS, "Policy reset to SCHED_NORMAL");
> +		else
> +			tst_res(TFAIL, "Policy NOT reset to SCHED_NORMAL");
> +	}
> +
> +	tst_reap_children();
> +}
> +
> +static struct tst_test test = {
> +	.forks_child = 1,
> +	.needs_root = 1,

Does it actually need root or is just CAP_SYS_NICE enough?

We can just set

	.caps = (struct tst_cap []) {
		TST_CAP(TST_CAP_REQ, CAP_SYS_NICE)
        }

If so.

-- 
Thank you,
Richard.

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