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

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

[Wei Gao via ltp]

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

diff --git a/runtest/sched b/runtest/sched
index 592898723..172fe4174 100644
--- a/runtest/sched
+++ b/runtest/sched
@@ -9,6 +9,7 @@ trace_sched01		trace_sched -c 1
 cfs_bandwidth01 cfs_bandwidth01 -i 5
 hackbench01 hackbench 50 process 1000
 hackbench02 hackbench 20 thread 1000
+starvation starvation
 
 sched_cli_serv run_sched_cliserv.sh
 # Run this stress test for 2 minutes
diff --git a/testcases/kernel/sched/cfs-scheduler/.gitignore b/testcases/kernel/sched/cfs-scheduler/.gitignore
index c5dacd6ef..e86178f80 100644
--- a/testcases/kernel/sched/cfs-scheduler/.gitignore
+++ b/testcases/kernel/sched/cfs-scheduler/.gitignore
@@ -1,2 +1,3 @@
 /hackbench
 cfs_bandwidth01
+/starvation
diff --git a/testcases/kernel/sched/cfs-scheduler/starvation.c b/testcases/kernel/sched/cfs-scheduler/starvation.c
new file mode 100644
index 000000000..74bffd741
--- /dev/null
+++ 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> */
+/*\
+ *
+ * [Description]
+ *
+ * Thread starvation test.
+ * This case copy from following link:
+ * https://lwn.net/ml/linux-kernel/9fd2c37a05713c206dcbd5866f67ce779f315e9e.camel@gmx.de/
+ *
+ */
+
+#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;
+
+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;
+}
+
+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);
+}
+
+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,
+};
-- 
2.35.3


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

Re: [LTP] [PATCH v1] starvation.c: New case for sched starvation

[Petr Vorel]

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

> +};

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

[LTP] [PATCH v2] starvation.c: New case for sched starvation

[Wei Gao via ltp]

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   | 112 ++++++++++++++++++
 3 files changed, 114 insertions(+)
 create mode 100644 testcases/kernel/sched/cfs-scheduler/starvation.c

diff --git a/runtest/sched b/runtest/sched
index 592898723..172fe4174 100644
--- a/runtest/sched
+++ b/runtest/sched
@@ -9,6 +9,7 @@ trace_sched01		trace_sched -c 1
 cfs_bandwidth01 cfs_bandwidth01 -i 5
 hackbench01 hackbench 50 process 1000
 hackbench02 hackbench 20 thread 1000
+starvation starvation
 
 sched_cli_serv run_sched_cliserv.sh
 # Run this stress test for 2 minutes
diff --git a/testcases/kernel/sched/cfs-scheduler/.gitignore b/testcases/kernel/sched/cfs-scheduler/.gitignore
index c5dacd6ef..e86178f80 100644
--- a/testcases/kernel/sched/cfs-scheduler/.gitignore
+++ b/testcases/kernel/sched/cfs-scheduler/.gitignore
@@ -1,2 +1,3 @@
 /hackbench
 cfs_bandwidth01
+/starvation
diff --git a/testcases/kernel/sched/cfs-scheduler/starvation.c b/testcases/kernel/sched/cfs-scheduler/starvation.c
new file mode 100644
index 000000000..5a3655a3f
--- /dev/null
+++ b/testcases/kernel/sched/cfs-scheduler/starvation.c
@@ -0,0 +1,112 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/* Copyright 2023 Mike Galbraith <efault-AT-gmx.de> */
+/* Copyright 2023 Wei Gao <wegao@suse.com> */
+/*\
+ *
+ * [Description]
+ *
+ * Thread starvation test.
+ * This case copy from following link:
+ * https://lore.kernel.org/lkml/9fd2c37a05713c206dcbd5866f67ce779f315e9e.camel@gmx.de/
+ */
+
+#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 char *str_loop;
+static long loop = 10000000;
+static char *str_timeout;
+static int timeout = 240;
+
+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;
+}
+
+static void setup(void)
+{
+	cpu_set_t mask;
+
+	CPU_ZERO(&mask);
+
+	CPU_SET(0, &mask);
+
+	TST_EXP_POSITIVE(sched_setaffinity(0, sizeof(mask), &mask));
+
+	if (tst_parse_long(str_loop, &loop, 1, LONG_MAX))
+		tst_brk(TBROK, "Invalid number of loop number '%s'", str_loop);
+
+	if (tst_parse_int(str_timeout, &timeout, 1, INT_MAX))
+		tst_brk(TBROK, "Invalid number of timeout '%s'", str_timeout);
+
+	tst_set_max_runtime(timeout);
+}
+
+static void handler(int sig LTP_ATTRIBUTE_UNUSED)
+{
+	if (loop > 0)
+		--loop;
+}
+
+static void child(void)
+{
+	pid_t ppid = getppid();
+
+	TST_CHECKPOINT_WAIT(0);
+
+	while (1)
+		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);
+
+	SAFE_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,
+	.options = (struct tst_option[]) {
+		{"l:", &str_loop, "Number of loops (default 10000000)"},
+		{"t:", &str_timeout, "Max timeout (default 240s)"},
+		{}
+	},
+	.needs_checkpoints = 1,
+};
-- 
2.35.3


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

Re: [LTP] [PATCH v1] starvation.c: New case for sched starvation

[Wei Gao via ltp]

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
> 
> > +};

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

Re: [LTP] [PATCH v1] starvation.c: New case for sched starvation

[Petr Vorel]

> 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

> > > +};

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

Re: [LTP] [PATCH v1] starvation.c: New case for sched starvation

[Wei Gao via ltp]

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
> 
> > > > +};

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

Re: [LTP] [PATCH v2] starvation.c: New case for sched starvation

[Cyril Hrubis]

Hi!
> +static long loop = 10000000;

I've reduced the default number of loops to 2000000 since the test
runtime on small ARM board was quite close to the timeout. Given that
when the scheduller was broken the original reproducer haven't finished
after running for 40 times longer than on working kernel, 2000000 should
be enough to trip the timeout even on fairly fast machine.

Also added note to the test description that the test is expected to
timeout when scheduller is broken.

And pushed, thanks.

-- 
Cyril Hrubis
chrubis@suse.cz

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