[LTP] Re [PATCH] syscalls/sched_setscheduler04: new test for sched_setscheduler()

[zhaogongyi]

Hi!

> 
> Hello,
> 
> Zhao Gongyi via ltp <ltp@lists.linux.it> writes:
> 
> > Verify that the scheduling policy and parameters are in fact
> > per-thread attributes on Linux:
> > 1. Specifying pid as 0 will operate on the attributes of the calling
> > thread 2. The value returned from a call to gettid(2) can be passed in the
> argument
> >    pid.
> > 3. Passing the value returned from a call to getpid(2) will operate on the
> >    attributes of the main thread of the  thread  group
> >
> > Signed-off-by: Zhao Gongyi <zhaogongyi@huawei.com>
> > ---
> >  runtest/syscalls                              |  1 +
> >  .../syscalls/sched_setscheduler/.gitignore    |  1 +
> >  .../syscalls/sched_setscheduler/Makefile      |  2 +
> >  .../sched_setscheduler/sched_setscheduler04.c | 91
> > +++++++++++++++++++
> >  4 files changed, 95 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_setscheduler/.gitignore
> > @@ -1,3 +1,4 @@
> >  /sched_setscheduler01
> >  /sched_setscheduler02
> >  /sched_setscheduler03
> > +/sched_setscheduler04
> > diff --git a/testcases/kernel/syscalls/sched_setscheduler/Makefile
> > b/testcases/kernel/syscalls/sched_setscheduler/Makefile
> > index 044619fb8..e3d54e33e 100644
> > --- a/testcases/kernel/syscalls/sched_setscheduler/Makefile
> > +++ b/testcases/kernel/syscalls/sched_setscheduler/Makefile
> > @@ -3,6 +3,8 @@
> >
> >  top_srcdir		?= ../../../..
> >
> > +sched_setscheduler04: CFLAGS += -pthread
> > +
> >  include $(top_srcdir)/include/mk/testcases.mk
> >
> >  include $(top_srcdir)/include/mk/generic_leaf_target.mk
> > 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..6f985be88
> > --- /dev/null
> > +++
> b/testcases/kernel/syscalls/sched_setscheduler/sched_setscheduler0
> > +++ 4.c
> > @@ -0,0 +1,91 @@
> > +// SPDX-License-Identifier: GPL-2.0-or-later
> > +/*
> > + * Copyright(c) 2022 Huawei Technologies Co., Ltd
> > + * Author: Zhao Gongyi <zhaogongyi@huawei.com>  */
> > +
> > +/*\
> > + * [Description]
> > + *
> > + * Verify that the scheduling policy and parameters are in fact
> > +per-thread
> > + * attributes on Linux:
> > + * 1. Specifying pid as 0 will operate on the attributes of the
> > +calling thread
> > + * 2. The value returned from a call to gettid(2) can be passed in the
> argument
> > + *    pid.
> > + * 3. Passing the value returned from a call to getpid(2) will operate on
> the
> > + *    attributes of the main thread of the  thread  group
> > + */
> > +#include "tst_test.h"
> > +#include "lapi/syscalls.h"
> > +#include "tst_safe_pthread.h"
> > +#include <pthread.h>
> > +
> > +static struct sched_param param;
> > +static volatile int sched_prio;
> > +
> > +#define SCHED_POLICY SCHED_FIFO
> > +
> > +static void set_param(int tid)
> > +{
> > +	param.sched_priority = sched_prio;
> > +
> > +	if (sched_setscheduler(tid, SCHED_POLICY, &param)) {
> > +		tst_brk(TBROK | TERRNO,
> > +			"sched_setscheduler(%d, %d, ...) failed",
> > +			tid, SCHED_POLICY);
> > +	}
> > +}
> > +
> > +static void check_prio(int tid)
> > +{
> > +	if (sched_getparam(tid, &param) != 0)
> > +		tst_brk(TBROK | TERRNO, "sched_getparam() failed");
> > +
> > +	if (param.sched_priority != sched_prio)
> > +		tst_res(TFAIL, "Checking of thread priority failed");
> > +	else
> > +		tst_res(TPASS, "Checking of thread priority passed");
> 
> How are we going to debug a test failure?
> 
> This can be replaced with the TST_EXP_* macros which will print more
> info if used correctly.
> 
> At the least we should print what thread or pid we are operating on. What
> the priority and policies were originally, what we changed them to and
> what they are at the end.

Agree, I will fix it in the next version.

> 
> > +}
> > +
> > +static void *thread_func(LTP_ATTRIBUTE_UNUSED void *arg) {
> > +	pid_t threadid = tst_syscall(__NR_gettid);
> > +
> > +	sched_prio++;
> > +	set_param(0);
> > +	check_prio(threadid);
> > +
> > +	sched_prio++;
> > +	set_param(threadid);
> > +	check_prio(threadid);
> > +
> > +	return NULL;
> > +}
> > +
> > +static void test_sched_setscheduler01(void) {
> > +	sched_prio++;
> > +	set_param(getpid());
> > +	check_prio(tst_syscall(__NR_gettid));
> > +}
> > +
> > +static void test_sched_setscheduler02(void) {
> > +	pthread_t tid;
> > +
> > +	SAFE_PTHREAD_CREATE(&tid, NULL, thread_func, NULL);
> > +	SAFE_PTHREAD_JOIN(tid, NULL);
> > +}
> > +
> > +
> > +static void run(void)
> > +{
> > +	sched_prio = sched_get_priority_min(SCHED_POLICY);
> 
> This can go in the setup function.

If move it to setup function, we run the test with option "-I 200", it will fail.

> 
> > +
> > +	test_sched_setscheduler01();
> > +	test_sched_setscheduler02();
> 
> This should be replaced with .tcnt = 2 and .test. or else just merge them
> into run.

Agree, I will fix it int the next version.

> 
> > +}
> > +
> > +static struct tst_test test = {
> > +	.test_all = run,
> 
> We probably need to ensure CAP_SYS_NICE. Full details are in 'man 7
> sched'
> 
> i.e. .caps = (struct tst_cap[]) {
>   TST_CAP(TST_CAP_REQ, CAP_SYS_NICE),
>    null
> }
> 

In this testcase, we just increase the priority, should we add the checking of capability?


Regards,
Gongyi