mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: Andrea Righi <arighi@nvidia.com>
To: thamrinjonathan@gmail.com
Cc: Tejun Heo <tj@kernel.org>, David Vernet <void@manifault.com>,
	Changwoo Min <changwoo@igalia.com>, Shuah Khan <shuah@kernel.org>,
	Joel Fernandes <joelagnelf@nvidia.com>,
	Emil Tsalapatis <emil@etsalapatis.com>,
	"Peter Zijlstra (Intel)" <peterz@infradead.org>,
	sched-ext@lists.linux.dev, linux-kselftest@vger.kernel.org,
	linux-kernel@vger.kernel.org,
	Ihor Solodrai <ihor.solodrai@linux.dev>,
	Cheng-Yang Chou <yphbchou0911@gmail.com>
Subject: Re: [PATCH sched_ext for-7.4-fixes] selftests/sched_ext: Measure rt_stall runtime over sleep window only
Date: Sat, 14 Nov 2026 10:33:26 +0100	[thread overview]
Message-ID: <avgq5uoA1hXkZg5P@gpd4> (raw)
In-Reply-To: <20261114-scx-rt-stall-v1-1-4188a9378067@gmail.com>

Hi Jonathan,

On Sat, Nov 14, 2026 at 02:34:26PM +0700, Jonathan Bastari Thamrin via B4 Relay wrote:
> From: Jonathan Bastari Thamrin <thamrinjonathan@gmail.com>
> 
> In sched_stress_test(), the parent can be slow to start as it has
> to wait for the DL server before reaching sleep() while the RT task
> still runs. The runtime of each task is read as a total since fork
> instead of during sleep(). This adds extra time to RT and makes the
> ratio inaccurate which causes flaky failures on CI.
> 
> With the parent pinned to CORE_ID, the RT task had already run for
> ~0.95s before sleep(), so it was measured at 5.69s in a 5s window and
> the EXT share dropped to ~4.2% against the 4% threshold. CI has seen
> 3.0-3.9%.

I don't think this fixes the flakiness completely, in two of the three
CI failures from the linked issue RT ran less than 5s (4.74s), so
there's nothing to subtract there. And in all of them the EXT/FAIR task
got only ~0.18s instead of ~0.25s.

My guess is that the test is flaky because the test task isn't the only
one consuming the dl_server budget: the 50ms/1s reservation is per CPU
and shared by all the EXT (or FAIR) tasks runnable there, including
per-CPU kworkers, ksoftirqd, etc. On a noisy CI VM they can easily take
a good chunk of the 250ms available in a 5s window, especially right
after the scheduler is detached/attached, which would also explain why
the failures mostly happen in iterations 2 and 3. On top of that,
utime+stime are in USER_HZ units (10ms), which isn't negligible with
~0.25s of runtime and a 1% margin.

How about something like this:

1) Read the CPU time of the children with clock_getcpuclockid() +
   clock_gettime() and use deltas around the measurement window. This
   keeps the part of your patch that is correct, but without the USER_HZ
   rounding.

2) Pin the parent to a CPU other than CORE_ID, if there's one. The
   parent is also an EXT (or FAIR), so if it runs on CORE_ID it consumes
   part of the same server budget, adding noise to the runtime measured
   for the child.

3) if 1) and 2) aren't enough, slightly lower the threshold (e.g., from
   4% to 3%), to account for the server budget being shared with other
   tasks on CORE_ID, while still being strict enough to catch a
   misbehaving server.

Thanks,
-Andrea

> 
> Measure CPU time right before calling sleep() and use deltas to get
> an accurate measurement window. With the parent pinned to CORE_ID, RT
> measures 4.75s and EXT share stays between 4.81% and 5.57% over 20
> runs.
> 
> Fixes: be621a76341c ("selftests/sched_ext: Add test for sched_ext dl_server")
> Link: https://github.com/kernel-patches/vmtest/issues/458
> Cc: Ihor Solodrai <ihor.solodrai@linux.dev>
> Cc: Cheng-Yang Chou <yphbchou0911@gmail.com>
> Signed-off-by: Jonathan Bastari Thamrin <thamrinjonathan@gmail.com>
> ---
>  tools/testing/selftests/sched_ext/rt_stall.c | 21 ++++++++++++++++++++-
>  1 file changed, 20 insertions(+), 1 deletion(-)
> 
> diff --git a/tools/testing/selftests/sched_ext/rt_stall.c b/tools/testing/selftests/sched_ext/rt_stall.c
> index a1552b38a6e8..0c6403d25298 100644
> --- a/tools/testing/selftests/sched_ext/rt_stall.c
> +++ b/tools/testing/selftests/sched_ext/rt_stall.c
> @@ -149,7 +149,7 @@ static bool sched_stress_test(bool is_ext)
>  	const float expected_min_ratio = 0.04; /* 4% */
>  	const char *class_str = is_ext ? "EXT" : "FAIR";
>  
> -	float ext_runtime, rt_runtime, actual_ratio;
> +	float ext_runtime_start, rt_runtime_start, ext_runtime, rt_runtime, actual_ratio;
>  	int ext_pid, rt_pid;
>  	int ext_ready[2], rt_ready[2];
>  	bool ret = false;
> @@ -201,6 +201,23 @@ static bool sched_stress_test(bool is_ext)
>  	if (!wait_ready(ext_ready[0]) || !wait_ready(rt_ready[0]))
>  		goto out_kill;
>  
> +	/*
> +	 * The parent can be delayed before reaching sleep(). Record each task's
> +	 * CPU time right before calling sleep() so only time used during the
> +	 * measurement window is counted.
> +	 */
> +	ext_runtime_start = get_process_runtime(ext_pid);
> +	if (ext_runtime_start == -1) {
> +		fprintf(stderr, "Failed to read %s task runtime\n", class_str);
> +		goto out_kill;
> +	}
> +
> +	rt_runtime_start = get_process_runtime(rt_pid);
> +	if (rt_runtime_start == -1) {
> +		fprintf(stderr, "Failed to read RT task runtime\n");
> +		goto out_kill;
> +	}
> +
>  	/* Let the processes run for the specified time */
>  	sleep(RUN_TIME);
>  
> @@ -218,6 +235,8 @@ static bool sched_stress_test(bool is_ext)
>  		goto out_kill;
>  	}
>  
> +	ext_runtime = ext_runtime - ext_runtime_start;
> +	rt_runtime = rt_runtime - rt_runtime_start;
>  	/* Verify that the scx task got enough runtime */
>  	actual_ratio = ext_runtime / (ext_runtime + rt_runtime);
>  	fprintf(stderr, "%s task ran %.3fs, RT task ran %.3fs (%.2f%% of runtime)\n",
> 
> ---
> base-commit: 0021f17128de5dc0b19f1a523f448df2440d8088
> change-id: 20261114-scx-rt-stall-ca1767a8ab4f
> 
> Best regards,
> -- 
> Jonathan Bastari Thamrin <thamrinjonathan@gmail.com>
> 
> 

      reply	other threads:[~2026-11-14  9:33 UTC|newest]

Thread overview: 2+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-11-14  7:34 Jonathan Bastari Thamrin via B4 Relay
2026-11-14  9:33 ` Andrea Righi [this message]

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=avgq5uoA1hXkZg5P@gpd4 \
    --to=arighi@nvidia.com \
    --cc=changwoo@igalia.com \
    --cc=emil@etsalapatis.com \
    --cc=ihor.solodrai@linux.dev \
    --cc=joelagnelf@nvidia.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-kselftest@vger.kernel.org \
    --cc=peterz@infradead.org \
    --cc=sched-ext@lists.linux.dev \
    --cc=shuah@kernel.org \
    --cc=thamrinjonathan@gmail.com \
    --cc=tj@kernel.org \
    --cc=void@manifault.com \
    --cc=yphbchou0911@gmail.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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®