All of lore.kernel.org
 help / color / mirror / Atom feed
From: Andrea Righi <arighi@nvidia.com>
To: Emil Tsalapatis <linux-lists@etsalapatis.com>
Cc: Ingo Molnar <mingo@redhat.com>,
	Peter Zijlstra <peterz@infradead.org>,
	Juri Lelli <juri.lelli@redhat.com>,
	Vincent Guittot <vincent.guittot@linaro.org>,
	Dietmar Eggemann <dietmar.eggemann@arm.com>,
	Steven Rostedt <rostedt@goodmis.org>,
	Ben Segall <bsegall@google.com>, Mel Gorman <mgorman@suse.de>,
	Valentin Schneider <vschneid@redhat.com>,
	Joel Fernandes <joelagnelf@nvidia.com>, Tejun Heo <tj@kernel.org>,
	David Vernet <void@manifault.com>,
	Changwoo Min <changwoo@igalia.com>, Shuah Khan <shuah@kernel.org>,
	sched-ext@lists.linux.dev, bpf@vger.kernel.org,
	linux-kernel@vger.kernel.org
Subject: Re: [PATCH 13/14] selftests/sched_ext: Add test for sched_ext dl_server
Date: Mon, 20 Oct 2025 15:22:39 +0200	[thread overview]
Message-ID: <aPY3n5vIlzfTZMru@gpd4> (raw)
In-Reply-To: <CABFh=a578RNXxjtze1TxAcPBkx9_M58qBc=6E4o-uFJx0DB4Jg@mail.gmail.com>

Hi Emil,

On Sun, Oct 19, 2025 at 03:04:22PM -0400, Emil Tsalapatis wrote:
> On Fri, Oct 17, 2025 at 5:38 AM Andrea Righi <arighi@nvidia.com> wrote:
> >
> > Add a selftest to validate the correct behavior of the deadline server
> > for the ext_sched_class.
> >
> > [ Joel: Replaced occurences of CFS in the test with EXT. ]
> >
> > Co-developed-by: Joel Fernandes <joelagnelf@nvidia.com>
> > Signed-off-by: Joel Fernandes <joelagnelf@nvidia.com>
> > Signed-off-by: Andrea Righi <arighi@nvidia.com>
> > ---
> 
> Nits listed below, but otherwise:
> Reviewed-by: Emil Tsalapatis <emil@etsalapatis.com>
> 
> Code review aside, on my VM the test alternates between 4.81% and 5.20% for me
> so it's working as expected.

Yeah, that sounds right, a bit of fluctuation like that is expected.

> 
> >  tools/testing/selftests/sched_ext/Makefile    |   1 +
> >  .../selftests/sched_ext/rt_stall.bpf.c        |  23 ++
> >  tools/testing/selftests/sched_ext/rt_stall.c  | 214 ++++++++++++++++++
> >  3 files changed, 238 insertions(+)
> >  create mode 100644 tools/testing/selftests/sched_ext/rt_stall.bpf.c
> >  create mode 100644 tools/testing/selftests/sched_ext/rt_stall.c
> >
> > diff --git a/tools/testing/selftests/sched_ext/Makefile b/tools/testing/selftests/sched_ext/Makefile
> > index 5fe45f9c5f8fd..c9255d1499b6e 100644
> > --- a/tools/testing/selftests/sched_ext/Makefile
> > +++ b/tools/testing/selftests/sched_ext/Makefile
> > @@ -183,6 +183,7 @@ auto-test-targets :=                        \
> >         select_cpu_dispatch_bad_dsq     \
> >         select_cpu_dispatch_dbl_dsp     \
> >         select_cpu_vtime                \
> > +       rt_stall                        \
> >         test_example                    \
> >
> >  testcase-targets := $(addsuffix .o,$(addprefix $(SCXOBJ_DIR)/,$(auto-test-targets)))
> > diff --git a/tools/testing/selftests/sched_ext/rt_stall.bpf.c b/tools/testing/selftests/sched_ext/rt_stall.bpf.c
> > new file mode 100644
> > index 0000000000000..80086779dd1eb
> > --- /dev/null
> > +++ b/tools/testing/selftests/sched_ext/rt_stall.bpf.c
> > @@ -0,0 +1,23 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +/*
> > + * A scheduler that verified if RT tasks can stall SCHED_EXT tasks.
> > + *
> > + * Copyright (c) 2025 NVIDIA Corporation.
> > + */
> > +
> > +#include <scx/common.bpf.h>
> > +
> > +char _license[] SEC("license") = "GPL";
> > +
> > +UEI_DEFINE(uei);
> > +
> > +void BPF_STRUCT_OPS(rt_stall_exit, struct scx_exit_info *ei)
> > +{
> > +       UEI_RECORD(uei, ei);
> > +}
> > +
> > +SEC(".struct_ops.link")
> > +struct sched_ext_ops rt_stall_ops = {
> > +       .exit                   = (void *)rt_stall_exit,
> > +       .name                   = "rt_stall",
> > +};
> > diff --git a/tools/testing/selftests/sched_ext/rt_stall.c b/tools/testing/selftests/sched_ext/rt_stall.c
> > new file mode 100644
> > index 0000000000000..e9a0def9ee323
> > --- /dev/null
> > +++ b/tools/testing/selftests/sched_ext/rt_stall.c
> > @@ -0,0 +1,214 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +/*
> > + * Copyright (c) 2025 NVIDIA Corporation.
> > + */
> > +#define _GNU_SOURCE
> > +#include <stdio.h>
> > +#include <stdlib.h>
> > +#include <unistd.h>
> > +#include <sched.h>
> > +#include <sys/prctl.h>
> > +#include <sys/types.h>
> > +#include <sys/wait.h>
> > +#include <time.h>
> > +#include <linux/sched.h>
> > +#include <signal.h>
> > +#include <bpf/bpf.h>
> > +#include <scx/common.h>
> > +#include <sys/wait.h>
> > +#include <unistd.h>
> > +#include "rt_stall.bpf.skel.h"
> > +#include "scx_test.h"
> > +#include "../kselftest.h"
> > +
> > +#define CORE_ID                0       /* CPU to pin tasks to */
> > +#define RUN_TIME        5      /* How long to run the test in seconds */
> > +
> > +/* Simple busy-wait function for test tasks */
> > +static void process_func(void)
> > +{
> > +       while (1) {
> > +               /* Busy wait */
> > +               for (volatile unsigned long i = 0; i < 10000000UL; i++)
> > +                       ;
> > +       }
> > +}
> > +
> > +/* Set CPU affinity to a specific core */
> > +static void set_affinity(int cpu)
> > +{
> > +       cpu_set_t mask;
> > +
> > +       CPU_ZERO(&mask);
> > +       CPU_SET(cpu, &mask);
> > +       if (sched_setaffinity(0, sizeof(mask), &mask) != 0) {
> > +               perror("sched_setaffinity");
> > +               exit(EXIT_FAILURE);
> > +       }
> > +}
> > +
> > +/* Set task scheduling policy and priority */
> > +static void set_sched(int policy, int priority)
> > +{
> > +       struct sched_param param;
> > +
> > +       param.sched_priority = priority;
> > +       if (sched_setscheduler(0, policy, &param) != 0) {
> > +               perror("sched_setscheduler");
> > +               exit(EXIT_FAILURE);
> > +       }
> > +}
> > +
> > +/* Get process runtime from /proc/<pid>/stat */
> > +static float get_process_runtime(int pid)
> > +{
> > +       char path[256];
> > +       FILE *file;
> > +       long utime, stime;
> > +       int fields;
> > +
> > +       snprintf(path, sizeof(path), "/proc/%d/stat", pid);
> > +       file = fopen(path, "r");
> > +       if (file == NULL) {
> > +               perror("Failed to open stat file");
> > +               return -1;
> > +       }
> > +
> > +       /* Skip the first 13 fields and read the 14th and 15th */
> > +       fields = fscanf(file,
> > +                       "%*d %*s %*c %*d %*d %*d %*d %*d %*u %*u %*u %*u %*u %lu %lu",
> > +                       &utime, &stime);
> > +       fclose(file);
> > +
> > +       if (fields != 2) {
> > +               fprintf(stderr, "Failed to read stat file\n");
> > +               return -1;
> > +       }
> > +
> > +       /* Calculate the total time spent in the process */
> > +       long total_time = utime + stime;
> > +       long ticks_per_second = sysconf(_SC_CLK_TCK);
> > +       float runtime_seconds = total_time * 1.0 / ticks_per_second;
> > +
> > +       return runtime_seconds;
> > +}
> > +
> > +static enum scx_test_status setup(void **ctx)
> > +{
> > +       struct rt_stall *skel;
> > +
> > +       skel = rt_stall__open();
> > +       SCX_FAIL_IF(!skel, "Failed to open");
> > +       SCX_ENUM_INIT(skel);
> > +       SCX_FAIL_IF(rt_stall__load(skel), "Failed to load skel");
> > +
> > +       *ctx = skel;
> > +
> > +       return SCX_TEST_PASS;
> > +}
> > +
> > +static bool sched_stress_test(void)
> > +{
> > +       float cfs_runtime, rt_runtime, actual_ratio;
> > +       int cfs_pid, rt_pid;
> 
> I think it should be cfs_pid -> ext_pid, cfs_runtime -> ext_runtime
> 
> > +       float expected_min_ratio = 0.04; /* 4% */
> 
> Maybe add a comment that explains the 4% value? As in, we're expecting
> it to be around 5% so 0.04 accounts for values close enough but
> below < 5%.

Makes sense, I’ll add this comment (or something along those lines).

> 
> > +
> > +       ksft_print_header();
> > +       ksft_set_plan(1);
> > +
> > +       /* Create and set up a EXT task */
> > +       cfs_pid = fork();
> > +       if (cfs_pid == 0) {
> > +               set_affinity(CORE_ID);
> > +               process_func();
> > +               exit(0);
> > +       } else if (cfs_pid < 0) {
> > +               perror("fork for EXT task");
> > +               ksft_exit_fail();
> > +       }
> > +
> > +       /* Create an RT task */
> > +       rt_pid = fork();
> > +       if (rt_pid == 0) {
> > +               set_affinity(CORE_ID);
> > +               set_sched(SCHED_FIFO, 50);
> > +               process_func();
> > +               exit(0);
> > +       } else if (rt_pid < 0) {
> > +               perror("fork for RT task");
> > +               ksft_exit_fail();
> > +       }
> > +
> > +       /* Let the processes run for the specified time */
> > +       sleep(RUN_TIME);
> > +
> > +       /* Get runtime for the EXT task */
> > +       cfs_runtime = get_process_runtime(cfs_pid);
> > +       if (cfs_runtime != -1)
> > +               ksft_print_msg("Runtime of EXT task (PID %d) is %f seconds\n",
> > +                              cfs_pid, cfs_runtime);
> > +       else
> > +               ksft_exit_fail_msg("Error getting runtime for EXT task (PID %d)\n", cfs_pid);
> > +
> > +       /* Get runtime for the RT task */
> > +       rt_runtime = get_process_runtime(rt_pid);
> > +       if (rt_runtime != -1)
> > +               ksft_print_msg("Runtime of RT task (PID %d) is %f seconds\n", rt_pid, rt_runtime);
> > +       else
> > +               ksft_exit_fail_msg("Error getting runtime for RT task (PID %d)\n", rt_pid);
> > +
> 
> Minor, but why not
> 
> if (rt_runtime == -1)
>         ksft_exit_fail_msg("Error getting runtime for RT task (PID
> %d)\n", rt_pid);
> ksft_print_msg("Runtime of RT task (PID %d) is %f seconds\n", rt_pid,
> rt_runtime);
> 
> since ksft_exit_fail_msg never returns?

Ack.

> 
> > +       /* Kill the processes */
> > +       kill(cfs_pid, SIGKILL);
> > +       kill(rt_pid, SIGKILL);
> > +       waitpid(cfs_pid, NULL, 0);
> > +       waitpid(rt_pid, NULL, 0);
> > +
> > +       /* Verify that the scx task got enough runtime */
> > +       actual_ratio = cfs_runtime / (cfs_runtime + rt_runtime);
> > +       ksft_print_msg("EXT task got %.2f%% of total runtime\n", actual_ratio * 100);
> > +
> > +       if (actual_ratio >= expected_min_ratio) {
> > +               ksft_test_result_pass("PASS: EXT task got more than %.2f%% of runtime\n",
> > +                                     expected_min_ratio * 100);
> > +               return true;
> > +       }
> > +       ksft_test_result_fail("FAIL: EXT task got less than %.2f%% of runtime\n",
> > +                             expected_min_ratio * 100);
> > +       return false;
> > +}
> > +
> > +static enum scx_test_status run(void *ctx)
> > +{
> > +       struct rt_stall *skel = ctx;
> > +       struct bpf_link *link;
> > +       bool res;
> > +
> > +       link = bpf_map__attach_struct_ops(skel->maps.rt_stall_ops);
> > +       SCX_FAIL_IF(!link, "Failed to attach scheduler");
> > +
> > +       res = sched_stress_test();
> > +
> > +       SCX_EQ(skel->data->uei.kind, EXIT_KIND(SCX_EXIT_NONE));
> > +       bpf_link__destroy(link);
> > +
> > +       if (!res)
> > +               ksft_exit_fail();
> > +
> > +       return SCX_TEST_PASS;
> > +}
> > +
> > +static void cleanup(void *ctx)
> > +{
> > +       struct rt_stall *skel = ctx;
> > +
> > +       rt_stall__destroy(skel);
> > +}
> > +
> > +struct scx_test rt_stall = {
> > +       .name = "rt_stall",
> > +       .description = "Verify that RT tasks cannot stall SCHED_EXT tasks",
> > +       .setup = setup,
> > +       .run = run,
> > +       .cleanup = cleanup,
> > +};
> > +REGISTER_SCX_TEST(&rt_stall)
> > --
> > 2.51.0
> >
> >

Thanks,
-Andrea

  reply	other threads:[~2025-10-20 13:22 UTC|newest]

Thread overview: 45+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-10-17  9:25 [PATCHSET v9 sched_ext/for-6.19] Add a deadline server for sched_ext tasks Andrea Righi
2025-10-17  9:25 ` [PATCH 01/14] sched/debug: Fix updating of ppos on server write ops Andrea Righi
2025-10-20  8:36   ` Juri Lelli
2025-10-17  9:25 ` [PATCH 02/14] sched/debug: Stop and start server based on if it was active Andrea Righi
2025-10-20  9:12   ` Juri Lelli
2025-10-20  9:27     ` Juri Lelli
2025-10-17  9:25 ` [PATCH 03/14] sched/deadline: Clear the defer params Andrea Righi
2025-10-17  9:25 ` [PATCH 04/14] sched/deadline: Return EBUSY if dl_bw_cpus is zero Andrea Righi
2025-10-20  9:49   ` Juri Lelli
2025-10-20 13:38     ` Andrea Righi
2025-10-20 14:03       ` Andrea Righi
2025-10-20 14:12         ` Juri Lelli
2025-10-17  9:25 ` [PATCH 05/14] sched: Add a server arg to dl_server_update_idle_time() Andrea Righi
2025-10-20  9:54   ` Juri Lelli
2025-10-20 12:49   ` Peter Zijlstra
2025-10-17  9:25 ` [PATCH 06/14] sched_ext: Add a DL server for sched_ext tasks Andrea Righi
2025-10-17 15:40   ` Tejun Heo
2025-10-17 19:00     ` Andrea Righi
2025-10-17 15:47   ` Tejun Heo
2025-10-17 18:58     ` Andrea Righi
2025-10-17 19:04       ` Tejun Heo
2025-10-17 19:06         ` Andrea Righi
2025-10-20 11:58   ` Juri Lelli
2025-10-20 13:50     ` Andrea Righi
2025-10-20 14:09       ` Juri Lelli
2025-10-17  9:25 ` [PATCH 07/14] sched/debug: Add support to change sched_ext server params Andrea Righi
2025-10-20 12:45   ` Juri Lelli
2025-10-21  6:23     ` Andrea Righi
2025-10-17  9:25 ` [PATCH 08/14] sched/deadline: Add support to remove DL server's bandwidth contribution Andrea Righi
2025-10-20 13:46   ` Juri Lelli
2025-10-17  9:25 ` [PATCH 09/14] sched/deadline: Account ext server bandwidth Andrea Righi
2025-10-17  9:25 ` [PATCH 10/14] sched/deadline: Allow to initialize DL server when needed Andrea Righi
2025-10-17  9:25 ` [PATCH 11/14] sched/deadline: Fix DL server crash in inactive_timer callback Andrea Righi
2025-10-17  9:25 ` [PATCH 12/14] sched_ext: Selectively enable ext and fair DL servers Andrea Righi
2025-10-17  9:26 ` [PATCH 13/14] selftests/sched_ext: Add test for sched_ext dl_server Andrea Righi
2025-10-19 19:04   ` Emil Tsalapatis
2025-10-20 13:22     ` Andrea Righi [this message]
2025-10-20 13:44       ` Andrea Righi
2025-10-20 13:26   ` Christian Loehle
2025-10-20 13:55     ` Andrea Righi
2025-10-20 14:00       ` Andrea Righi
2025-10-20 14:21       ` Christian Loehle
2025-10-23 15:01         ` Christian Loehle
2025-10-23 15:11           ` Andrea Righi
2025-10-17  9:26 ` [PATCH 14/14] selftests/sched_ext: Add test for DL server total_bw consistency Andrea Righi

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=aPY3n5vIlzfTZMru@gpd4 \
    --to=arighi@nvidia.com \
    --cc=bpf@vger.kernel.org \
    --cc=bsegall@google.com \
    --cc=changwoo@igalia.com \
    --cc=dietmar.eggemann@arm.com \
    --cc=joelagnelf@nvidia.com \
    --cc=juri.lelli@redhat.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-lists@etsalapatis.com \
    --cc=mgorman@suse.de \
    --cc=mingo@redhat.com \
    --cc=peterz@infradead.org \
    --cc=rostedt@goodmis.org \
    --cc=sched-ext@lists.linux.dev \
    --cc=shuah@kernel.org \
    --cc=tj@kernel.org \
    --cc=vincent.guittot@linaro.org \
    --cc=void@manifault.com \
    --cc=vschneid@redhat.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.