diff mbox series

cfs-scheduler/starvation.c: Skip test on realtime kernels

Message ID 20250120085017.63442-1-acarmina@redhat.com
State Changes Requested
Headers show
Series cfs-scheduler/starvation.c: Skip test on realtime kernels | expand

Commit Message

Alessandro Carminati Jan. 20, 2025, 8:50 a.m. UTC
This commit introduces a check in the starvation test case to detect and
skip execution on realtime kernels. The test is designed for use with the
Completely Fair Scheduler and produces meaningless results when run on
realtime kernels.

By skipping the test on realtime kernels, we avoid confusion caused by
misleading results.

Changes include:
- Added a detection mechanism for realtime kernels.
- Logic to skip the test execution if the kernel is identified as
  realtime.

Signed-off-by: Alessandro Carminati <acarmina@redhat.com>
---
 include/tst_kernel.h                              |  9 +++++++++
 lib/tst_kernel.c                                  | 10 ++++++++++
 testcases/kernel/sched/cfs-scheduler/starvation.c |  3 +++
 3 files changed, 22 insertions(+)

Comments

Petr Vorel Jan. 20, 2025, 9:46 a.m. UTC | #1
Hi Alessandro,

> This commit introduces a check in the starvation test case to detect and
> skip execution on realtime kernels. The test is designed for use with the
> Completely Fair Scheduler and produces meaningless results when run on
> realtime kernels.

> By skipping the test on realtime kernels, we avoid confusion caused by
> misleading results.

> Changes include:
> - Added a detection mechanism for realtime kernels.
> - Logic to skip the test execution if the kernel is identified as
>   realtime.

LGTM. Could you please test if starvation.c worked previously?
Or was it always broken?

Reviewed-by: Petr Vorel <pvorel@suse.cz>

> +int tst_check_preempt_rt(void)
> +{
> +	struct utsname uval;
> +
> +	uname(&uval);
> +	if (strstr(uval.version, "PREEMPT_RT"))
> +		return -1;
> +	return 0;
> +}
> diff --git a/testcases/kernel/sched/cfs-scheduler/starvation.c b/testcases/kernel/sched/cfs-scheduler/starvation.c
> index 901556a7b..c620c9c3e 100644
> --- a/testcases/kernel/sched/cfs-scheduler/starvation.c
> +++ b/testcases/kernel/sched/cfs-scheduler/starvation.c
> @@ -82,6 +82,9 @@ static void setup(void)

>  	CPU_ZERO(&mask);

> +	if (tst_check_preempt_rt())
> +		tst_brk(TCONF, "This test is not designed for the RT kernel");
nit: I would move it above CPU_ZERO().

NOTE: we should also move tst_has_slow_kconfig() check to be at the top of
setup(). But that's unrelated. Also at least on Tumbleweed and SLES which get
detected as slow due CONFIG_LATENCYTOP test was worked before.

Kind regards,
Petr
Alessandro Carminati Jan. 20, 2025, 10:21 a.m. UTC | #2
Hello Petr,

On Mon, Jan 20, 2025 at 10:46 AM Petr Vorel <pvorel@suse.cz> wrote:
>
> Hi Alessandro,
>
> > This commit introduces a check in the starvation test case to detect and
> > skip execution on realtime kernels. The test is designed for use with the
> > Completely Fair Scheduler and produces meaningless results when run on
> > realtime kernels.
>
> > By skipping the test on realtime kernels, we avoid confusion caused by
> > misleading results.
>
> > Changes include:
> > - Added a detection mechanism for realtime kernels.
> > - Logic to skip the test execution if the kernel is identified as
> >   realtime.
>
> LGTM. Could you please test if starvation.c worked previously?
> Or was it always broken?

Before submitting the patch, I tested the case in various environments.
One thing that puzzled me for a while was the test's high sensitivity to
latencies.
While it works correctly on bare-metal systems under the intended
conditions with the CFS scheduler, running it on a real-time kernel
or in a virtualized/emulated environment is likely to cause the
test to fail.

This patch addresses the real-time kernel scenario, which is relatively
easier to handle.
In a separate RFC patch I plan to send, I would like to ask the
community for guidance on how to handle the virtualization/emulation
environment issue.

>
> Reviewed-by: Petr Vorel <pvorel@suse.cz>
>
> > +int tst_check_preempt_rt(void)
> > +{
> > +     struct utsname uval;
> > +
> > +     uname(&uval);
> > +     if (strstr(uval.version, "PREEMPT_RT"))
> > +             return -1;
> > +     return 0;
> > +}
> > diff --git a/testcases/kernel/sched/cfs-scheduler/starvation.c b/testcases/kernel/sched/cfs-scheduler/starvation.c
> > index 901556a7b..c620c9c3e 100644
> > --- a/testcases/kernel/sched/cfs-scheduler/starvation.c
> > +++ b/testcases/kernel/sched/cfs-scheduler/starvation.c
> > @@ -82,6 +82,9 @@ static void setup(void)
>
> >       CPU_ZERO(&mask);
>
> > +     if (tst_check_preempt_rt())
> > +             tst_brk(TCONF, "This test is not designed for the RT kernel");
> nit: I would move it above CPU_ZERO().
Ok, I'll fix it in the v2

>
> NOTE: we should also move tst_has_slow_kconfig() check to be at the top of
> setup(). But that's unrelated. Also at least on Tumbleweed and SLES which get
> detected as slow due CONFIG_LATENCYTOP test was worked before.
Want me to address this issue?

>
> Kind regards,
> Petr
>

Cheers
Alessandro
Petr Vorel Jan. 21, 2025, 8:56 a.m. UTC | #3
Hi Alessandro,

> Hello Petr,
...
> > LGTM. Could you please test if starvation.c worked previously?
> > Or was it always broken?

> Before submitting the patch, I tested the case in various environments.
> One thing that puzzled me for a while was the test's high sensitivity to
> latencies.
> While it works correctly on bare-metal systems under the intended
> conditions with the CFS scheduler, running it on a real-time kernel
> or in a virtualized/emulated environment is likely to cause the
> test to fail.

> This patch addresses the real-time kernel scenario, which is relatively
> easier to handle.
> In a separate RFC patch I plan to send, I would like to ask the
> community for guidance on how to handle the virtualization/emulation
> environment issue.

Thanks, reply send.

FYI there is a git freeze before upcoming release, only fixes are merged.
https://lore.kernel.org/ltp/5202b2ba-a13e-4250-97c5-937dde849975@suse.com/T/#t

But fixes like this should go in (I'll leave the decision to Cyril).

> > Reviewed-by: Petr Vorel <pvorel@suse.cz>

> > > +int tst_check_preempt_rt(void)
> > > +{
> > > +     struct utsname uval;
> > > +
> > > +     uname(&uval);
> > > +     if (strstr(uval.version, "PREEMPT_RT"))
> > > +             return -1;
> > > +     return 0;
> > > +}
> > > diff --git a/testcases/kernel/sched/cfs-scheduler/starvation.c b/testcases/kernel/sched/cfs-scheduler/starvation.c
> > > index 901556a7b..c620c9c3e 100644
> > > --- a/testcases/kernel/sched/cfs-scheduler/starvation.c
> > > +++ b/testcases/kernel/sched/cfs-scheduler/starvation.c
> > > @@ -82,6 +82,9 @@ static void setup(void)

> > >       CPU_ZERO(&mask);

> > > +     if (tst_check_preempt_rt())
> > > +             tst_brk(TCONF, "This test is not designed for the RT kernel");
> > nit: I would move it above CPU_ZERO().
> Ok, I'll fix it in the v2

Thanks!

> > NOTE: we should also move tst_has_slow_kconfig() check to be at the top of
> > setup(). But that's unrelated. Also at least on Tumbleweed and SLES which get
> > detected as slow due CONFIG_LATENCYTOP test was worked before.

> Want me to address this issue?

Already done:
https://patchwork.ozlabs.org/project/ltp/patch/20250120143420.815363-1-pvorel@suse.cz/

Kind regards,
Petr

> > Kind regards,
> > Petr


> Cheers
> Alessandro
diff mbox series

Patch

diff --git a/include/tst_kernel.h b/include/tst_kernel.h
index 5f49952b7..63ecb19a4 100644
--- a/include/tst_kernel.h
+++ b/include/tst_kernel.h
@@ -58,4 +58,13 @@  int tst_check_builtin_driver(const char *driver);
  */
 int tst_check_driver(const char *driver);
 
+/**
+ * tst_check_preempt_rt() - Check if the running kernel is RT.
+ *
+ * Check support for the kernel module (both built-in and loadable).
+ *
+ * Return: -1 if the kernel is RT, 0 otherwise.
+ */
+int tst_check_preempt_rt(void);
+
 #endif	/* TST_KERNEL_H__ */
diff --git a/lib/tst_kernel.c b/lib/tst_kernel.c
index 8dabfeba2..7084289c3 100644
--- a/lib/tst_kernel.c
+++ b/lib/tst_kernel.c
@@ -214,3 +214,13 @@  int tst_check_driver(const char *driver)
 
 	return -1;
 }
+
+int tst_check_preempt_rt(void)
+{
+	struct utsname uval;
+
+	uname(&uval);
+	if (strstr(uval.version, "PREEMPT_RT"))
+		return -1;
+	return 0;
+}
diff --git a/testcases/kernel/sched/cfs-scheduler/starvation.c b/testcases/kernel/sched/cfs-scheduler/starvation.c
index 901556a7b..c620c9c3e 100644
--- a/testcases/kernel/sched/cfs-scheduler/starvation.c
+++ b/testcases/kernel/sched/cfs-scheduler/starvation.c
@@ -82,6 +82,9 @@  static void setup(void)
 
 	CPU_ZERO(&mask);
 
+	if (tst_check_preempt_rt())
+		tst_brk(TCONF, "This test is not designed for the RT kernel");
+
 	/* Restrict test to a single cpu */
 	if (sched_getaffinity(0, sizeof(mask), &mask) < 0)
 		tst_brk(TBROK | TERRNO, "sched_getaffinity() failed");