[v1] starvation.c: New case for sched starvation

Signed-off-by: Wei Gao <wegao@suse.com>

Add scheduller thread starvation test case base following link:
https://lwn.net/ml/linux-kernel/9fd2c37a05713c206dcbd5866f67ce779f315e9e.camel@gmx.de/
---
 runtest/sched                                 |  1 +
 .../kernel/sched/cfs-scheduler/.gitignore     |  1 +
 .../kernel/sched/cfs-scheduler/starvation.c   | 97 +++++++++++++++++++
 3 files changed, 99 insertions(+)
 create mode 100644 testcases/kernel/sched/cfs-scheduler/starvation.c

Hi Wei,

could you please fix warnings? Both should be trivial.

starvation.c: In function ‘child’:
starvation.c:70:1: warning: no return statement in function returning non-void [-Wreturn-type]
   70 | }
      | ^
In file included from ../../../../include/tst_test.h:110,
                 from starvation.c:22:
starvation.c: In function ‘do_test’:
../../../../include/tst_safe_macros.h:493:51: warning: passing argument 4 of ‘safe_signal’ from incompatible pointer type [-Wincompatible-pointer-types]
  493 |         safe_signal(__FILE__, __LINE__, (signum), (handler))
      |                                                   ^~~~~~~~~
      |                                                   |
      |                                                   void (*)(void)
starvation.c:81:9: note: in expansion of macro ‘SAFE_SIGNAL’
   81 |         SAFE_SIGNAL(SIGUSR1, handler);
      |         ^~~~~~~~~~~
../../../../include/tst_safe_macros.h:490:34: note: expected ‘sighandler_t’ but argument is of type ‘void (*)(void)’
  490 |         int signum, sighandler_t handler);
      |                     ~~~~~~~~~~~~~^~~~~~~

make check-starvation
make -C "/home/pevik/install/src/ltp.git/tools/sparse" all
...
CHECK testcases/kernel/sched/cfs-scheduler/starvation.c
starvation.c:80:9: warning: incorrect type in argument 4 (different argument counts)
starvation.c:80:9:    expected void ( *[usertype] handler )( ... )
starvation.c:80:9:    got void ( * )( ... )


> Signed-off-by: Wei Gao <wegao@suse.com>

> Add scheduller thread starvation test case base following link:
> https://lwn.net/ml/linux-kernel/9fd2c37a05713c206dcbd5866f67ce779f315e9e.camel@gmx.de/
nit: we should use lore, not lwn:

https://lore.kernel.org/lkml/9fd2c37a05713c206dcbd5866f67ce779f315e9e.camel@gmx.de/

...
> +++ b/testcases/kernel/sched/cfs-scheduler/starvation.c
> @@ -0,0 +1,97 @@
> +// SPDX-License-Identifier: GPL-2.0-or-later
> +/* Copyright 2023 Mike Galbraith <efault-AT-gmx.de> */
You can add your credit for porting to LTP :).

> +/*\
> + *
> + * [Description]
> + *
> + * Thread starvation test.
> + * This case copy from following link:
> + * https://lwn.net/ml/linux-kernel/9fd2c37a05713c206dcbd5866f67ce779f315e9e.camel@gmx.de/
https://lore.kernel.org/lkml/9fd2c37a05713c206dcbd5866f67ce779f315e9e.camel@gmx.de/
> + *
Please avoid extra blank line here.
> + */
> +
> +#define _GNU_SOURCE
> +#include <stdio.h>
> +#include <signal.h>
> +#include <unistd.h>
> +#include <sys/types.h>
> +#include <sys/wait.h>
> +#include <stdlib.h>
> +#include <sched.h>
> +
> +#include "tst_test.h"
> +
> +static unsigned long loop = 10000000;
It'd be nice to have option to run less iterations.
> +
> +static int wait_for_pid(pid_t pid)
> +{
> +	int status, ret;
> +
> +again:
> +	ret = waitpid(pid, &status, 0);
> +	if (ret == -1) {
> +		if (errno == EINTR)
> +			goto again;
> +
> +		return -1;
> +	}
> +
> +	if (WIFSIGNALED(status))
> +		return 0;
> +
> +	return -1;
> +}
I'm not sure by this part. I'd be good if somebody else reviewed it.
We cannot use tst_reap_children(), because we want to be signaled.

> +
> +static void setup(void)
> +{
> +	cpu_set_t mask;
> +
> +	CPU_ZERO(&mask);
> +
> +	CPU_SET(0, &mask);
> +
> +	TST_EXP_POSITIVE(sched_setaffinity(0, sizeof(mask), &mask));
> +}
> +
> +static void handler(void)
> +{
> +	if (loop > 0)
> +		--loop;
> +}
> +
> +static int child(void)
> +{
> +	pid_t ppid = getppid();
> +
> +	TST_CHECKPOINT_WAIT(0);
> +
> +	while (1)
> +		kill(ppid, SIGUSR1);
SAFE_KILL(ppid, SIGUSR1) ?
> +}
> +
> +static void do_test(void)
> +{
> +	pid_t child_pid;
> +
> +	child_pid = SAFE_FORK();
> +
> +	if (!child_pid)
> +		child();
> +
> +	SAFE_SIGNAL(SIGUSR1, handler);
> +	TST_CHECKPOINT_WAKE(0);
> +
> +	while (loop)
> +		sleep(1);
> +
> +	kill(child_pid, SIGTERM);
> +	TST_EXP_PASS(wait_for_pid(child_pid));
> +}
> +
> +static struct tst_test test = {
> +	.test_all = do_test,
> +	.setup = setup,
> +	.forks_child = 1,
> +	.needs_checkpoints = 1,
> +	.max_runtime = 120,
Are you sure 2 min is enough? Maybe we need to use tst_remaining_runtime() to
check if we're not running out of time.
Also, if we set getopt to choose number of options, we'd need to adjust it by
tst_set_max_runtime().

Kind regards,
Petr

> +};

On Fri, May 05, 2023 at 07:31:37PM +0200, Petr Vorel wrote:
> Hi Wei,
> 
> > +static struct tst_test test = {
> > +	.test_all = do_test,
> > +	.setup = setup,
> > +	.forks_child = 1,
> > +	.needs_checkpoints = 1,
> > +	.max_runtime = 120,
> Are you sure 2 min is enough? Maybe we need to use tst_remaining_runtime() to
> check if we're not running out of time.
Seems difficult to check with tst_remaining_runtime since i can use code such as:
while(tst_remaining_runtime()) 
	wait_for_pid();
> Also, if we set getopt to choose number of options, we'd need to adjust it by
> tst_set_max_runtime().
> 
> Kind regards,
> Petr
> 
> > +};

> On Fri, May 05, 2023 at 07:31:37PM +0200, Petr Vorel wrote:
> > Hi Wei,

> > > +static struct tst_test test = {
> > > +	.test_all = do_test,
> > > +	.setup = setup,
> > > +	.forks_child = 1,
> > > +	.needs_checkpoints = 1,
> > > +	.max_runtime = 120,
> > Are you sure 2 min is enough? Maybe we need to use tst_remaining_runtime() to
> > check if we're not running out of time.

> Seems difficult to check with tst_remaining_runtime since i can use code such as:
> while(tst_remaining_runtime()) 
> 	wait_for_pid();

If you don't know how to use tst_remaining_runtime() how about getting
inspiration from lib/newlib_tests/test_runtime01.c ?

static void run(void)
{
	int runtime;

	tst_res(TINFO, "...");

	do {
		runtime = tst_remaining_runtime();
		tst_res(TINFO, "Remaining runtime %d", runtime);
		sleep(1);
	} while (runtime);

	tst_res(TPASS, "Test passed");
}

static struct tst_test test = {
	.test_all = run,
	.max_runtime = 120,
};

=> Also, it's a question, how long should the test be running.
Is it worth to be running for 2 mins? It's kind of stress test, right?
But even that wouldn't be 30 sec enough?

Kind regards,
Petr

> > Also, if we set getopt to choose number of options, we'd need to adjust it by
> > tst_set_max_runtime().

> > Kind regards,
> > Petr

> > > +};

On Tue, May 09, 2023 at 07:52:56AM +0200, Petr Vorel wrote:
> > On Fri, May 05, 2023 at 07:31:37PM +0200, Petr Vorel wrote:
> > > Hi Wei,
> 
> > > > +static struct tst_test test = {
> > > > +	.test_all = do_test,
> > > > +	.setup = setup,
> > > > +	.forks_child = 1,
> > > > +	.needs_checkpoints = 1,
> > > > +	.max_runtime = 120,
> > > Are you sure 2 min is enough? Maybe we need to use tst_remaining_runtime() to
> > > check if we're not running out of time.
> 
> > Seems difficult to check with tst_remaining_runtime since i can use code such as:
> > while(tst_remaining_runtime()) 
> > 	wait_for_pid();
> 
> If you don't know how to use tst_remaining_runtime() how about getting
> inspiration from lib/newlib_tests/test_runtime01.c ?
What i mean is i can not use while or do loop to check tst_remaining_runtime, since 
the last function wait_for_pid() already hold(block) the current process and you do not need 
and can not use tst_remaining_runtime(). Sorry if any misunderstood.

> 
> static void run(void)
> {
> 	int runtime;
> 
> 	tst_res(TINFO, "...");
> 
> 	do {
> 		runtime = tst_remaining_runtime();
> 		tst_res(TINFO, "Remaining runtime %d", runtime);
> 		sleep(1);
> 	} while (runtime);
> 
> 	tst_res(TPASS, "Test passed");
> }
> 
> static struct tst_test test = {
> 	.test_all = run,
> 	.max_runtime = 120,
> };
> 
> => Also, it's a question, how long should the test be running.
> Is it worth to be running for 2 mins? It's kind of stress test, right?
> But even that wouldn't be 30 sec enough?
Normally the default value which i set will spend ~30s i test in virtual machine.
I suppose 2mins max_runtime should be safe enough. 
Yes it can be taken as stress test, you need a big loop number to let test run.

In latest v2 patch the number of loop and max_runtime both configurable, so if your machine 
is too slow maybe you need adjust parameter.
> 
> Kind regards,
> Petr
> 
> > > Also, if we set getopt to choose number of options, we'd need to adjust it by
> > > tst_set_max_runtime().
> 
> > > Kind regards,
> > > Petr
> 
> > > > +};