[PATCH] sched/rt: Rebuild domains only after successful RT sysctl writes

[PATCH] sched/rt: Rebuild domains only after successful RT sysctl writes

[Joseph Salisbury]

sched_rt_handler() unconditionally calls rebuild_sched_domains() on exit.

That means scheduler domains are rebuilt even for read-only sysctl access,
and also after writes that fail validation and roll back to the old RT
bandwidth values.

Track whether the write path completed validation and global RT/DL updates,
and rebuild scheduler domains only in that case.

Fixes: 440989c10f4e ("sched/deadline: Fix accounting after global limits change")
Cc: stable@vger.kernel.org
Signed-off-by: Joseph Salisbury <joseph.salisbury@oracle.com>
---
 kernel/sched/rt.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/kernel/sched/rt.c b/kernel/sched/rt.c
index 906f6c656c2e..ada9249b22cd 100644
--- a/kernel/sched/rt.c
+++ b/kernel/sched/rt.c
@@ -2871,6 +2871,7 @@ static int sched_rt_handler(const struct ctl_table *table, int write, void *buff
 {
 	int old_period, old_runtime;
 	static DEFINE_MUTEX(mutex);
+	bool rebuild = false;
 	int ret;
 
 	mutex_lock(&mutex);
@@ -2895,6 +2896,7 @@ static int sched_rt_handler(const struct ctl_table *table, int write, void *buff
 
 		sched_rt_do_global();
 		sched_dl_do_global();
+		rebuild = true;
 	}
 	if (0) {
 undo:
@@ -2908,7 +2910,8 @@ static int sched_rt_handler(const struct ctl_table *table, int write, void *buff
 	 * After changing maximum available bandwidth for DEADLINE, we need to
 	 * recompute per root domain and per cpus variables accordingly.
 	 */
-	rebuild_sched_domains();
+	if (rebuild)
+		rebuild_sched_domains();
 
 	return ret;
 }
-- 
2.47.3

Re: [PATCH] sched/rt: Rebuild domains only after successful RT sysctl writes

[Chengfeng Lin]

[-- Attachment #1: Type: text/plain, Size: 2801 bytes --]

Hi Joseph,

I tested your patch on v7.2. It reduced repeated RT sysctl read latency
from about 8.57 us to 0.264 us, a 96.9% reduction. I found the patch while
investigating higher read latency between v6.12.95 and v7.2.

I adjusted only the patch context because v7.2 no longer has the
sched_rt_do_global() call. Your patch's logic is unchanged.

The machine was a bare-metal i7-12700KF with all 20 logical CPUs online.
Both builds used v7.2, GCC 15.2.0 and the same config except for
CONFIG_LOCALVERSION. Runtime settings were full preemption, performance
governor/EPP and Turbo disabled. CONFIG_RT_GROUP_SCHED was disabled.
The RT period and runtime were 1000000 and 950000 us on both kernels.

I ran original A -> patched -> original B, with a fresh boot at each
point. One reader was pinned to CPU 2. Each sample made 4096 pread() calls
at offset zero on an already-open FD and checked every returned value.
Each case had 3 warm-up and 9 measured samples per boot. The timed loop
included the reads and value checks, but not open/close or tracing.

Mean results in ns/read were:

  parameter                  original A     patched    original B
  sched_rt_period_us           8547.294      264.575      8576.337
  sched_rt_runtime_us          8574.240      263.690      8566.828
  sched_rr_timeslice_ms          253.400      250.268       251.304

Against the original A/B midpoint, the two RT read times fell by 96.91%
and 96.92%. Their maximum within-boot CV was 0.89%, and the original
kernel's A/B drift was at most 0.34%. Dropping the first measured sample
at each point gave the same conclusion. The RR control changed by 0.83%.

Separate untimed probes confirmed that patched RT reads skipped
rebuild_sched_domains() and deadline bandwidth re-accounting. The original
kernel reused the existing domains: build_sched_domains() was not
called. Successful writes, including same-value writes, still performed
the global update and rebuild with the patch.

Valid changes, invalid text, out-of-range values and invalid period/runtime
combinations had matching return values and readback on both kernels.
Rejected writes left the old values intact. They also skipped the rebuild
with the patch. No kernel warnings were observed.

These are read-loop microbenchmark results, not application timings.
I have not tested concurrent writes, CPU hotplug or DL admission failure
with active SCHED_DEADLINE tasks. The attached reader is the one used for
the test; it does not change any sysctl values.

The data, configs and v7.2 patch are available here:
https://github.com/lcf0399/linux-regression-evidence/tree/b937265884a2f3c28403634c2bc7e309f83f7153/sched-rt-sysctl-read-rebuild

Are you planning to update or resend this patch? I can test an updated
version if that would help.

Thanks,
Chengfeng

[-- Attachment #2: rt_sysctl_read.c --]
[-- Type: text/plain, Size: 4753 bytes --]

// SPDX-License-Identifier: GPL-2.0-only
#define _GNU_SOURCE
#include <errno.h>
#include <fcntl.h>
#include <inttypes.h>
#include <limits.h>
#include <sched.h>
#include <signal.h>
#include <stdint.h>
#include <stdio.h>
#include <stdlib.h>
#include <string.h>
#include <sys/prctl.h>
#include <time.h>
#include <unistd.h>

/* Stable probe boundaries: exactly one pread() between each pair. */
__attribute__((noinline, noclone, used))
void tr_read_begin(unsigned seq, unsigned kind, int fd)
{
	asm volatile("" : : "r"(seq), "r"(kind), "r"(fd) : "memory");
}

__attribute__((noinline, noclone, used))
void tr_read_end(unsigned seq, unsigned kind, ssize_t result)
{
	asm volatile("" : : "r"(seq), "r"(kind), "r"(result) : "memory");
}

static void die(const char *why)
{
	fprintf(stderr, "rt-sysctl-read: %s (errno=%d)\n", why, errno);
	exit(1);
}

static void alarm_exit(int signo)
{
	(void)signo;
	static const char message[] = "rt-sysctl-read: watchdog timeout\n";
	ssize_t written = write(STDERR_FILENO, message, sizeof(message) - 1);
	(void)written;
	_exit(124);
}

static long number(const char *text, long min, long max)
{
	char *end;
	errno = 0;
	long value = strtol(text, &end, 10);
	if (errno || !*text || *end || value < min || value > max)
		die("invalid integer argument");
	return value;
}

static uint64_t clock_ns(clockid_t clock)
{
	struct timespec ts;
	if (clock_gettime(clock, &ts))
		die("clock_gettime");
	return (uint64_t)ts.tv_sec * 1000000000ULL + ts.tv_nsec;
}

int main(int argc, char **argv)
{
	if (argc != 6 && argc != 7) {
		fprintf(stderr, "usage: %s semantic|probe|timing|fixture period|runtime|rr ops cpu expected [fixture-file]\n", argv[0]);
		return 2;
	}
	int fixture = !strcmp(argv[1], "fixture");
	int probe = !strcmp(argv[1], "probe");
	int timing = !strcmp(argv[1], "timing");
	if ((!fixture && !probe && !timing && strcmp(argv[1], "semantic")) ||
	    (fixture != (argc == 7)))
		die("unknown mode or fixture path outside fixture mode");
	if (timing && (!getenv("KS_ALLOW_SYSCTL_TIMING") ||
		       strcmp(getenv("KS_ALLOW_SYSCTL_TIMING"), "1"))) {
		fprintf(stderr, "timing requires a separately authorized experiment\n");
		return 2;
	}
	const char *names[] = {"period", "runtime", "rr"};
	const char *paths[] = {"/proc/sys/kernel/sched_rt_period_us",
		"/proc/sys/kernel/sched_rt_runtime_us",
		"/proc/sys/kernel/sched_rr_timeslice_ms"};
	unsigned kind;
	for (kind = 0; kind < 3; ++kind)
		if (!strcmp(argv[2], names[kind]))
			break;
	if (kind == 3)
		die("unknown case");
	unsigned ops = number(argv[3], 1, 4096);
	int cpu = number(argv[4], 0, CPU_SETSIZE - 1);
	long expected = number(argv[5], -1, INT_MAX);
	if ((probe || !strcmp(argv[1], "semantic")) && ops > 64)
		die("qualification is limited to 64 reads per invocation");
	cpu_set_t allowed, chosen;
	if (sched_getaffinity(0, sizeof(allowed), &allowed) || !CPU_ISSET(cpu, &allowed))
		die("CPU not in initial affinity");
	CPU_ZERO(&chosen);
	CPU_SET(cpu, &chosen);
	if (sched_setaffinity(0, sizeof(chosen), &chosen))
		die("sched_setaffinity");
	struct sigaction sa = {.sa_handler = alarm_exit};
	if (sigaction(SIGALRM, &sa, NULL) || prctl(PR_SET_PDEATHSIG, SIGKILL))
		die("watchdog setup");
	alarm(10);
	char wanted[64], actual[64];
	int bytes = snprintf(wanted, sizeof(wanted), "%ld\n", expected);
	int fd = open(fixture ? argv[6] : paths[kind], O_RDONLY | O_CLOEXEC | O_NOFOLLOW);
	if (fd < 0)
		die("open");
	uint64_t start = 0, wall = 0, cpu_start = 0, cpu_time = 0;
	if (timing) {
		cpu_start = clock_ns(CLOCK_THREAD_CPUTIME_ID);
		start = clock_ns(CLOCK_MONOTONIC_RAW);
	}
	for (unsigned seq = 0; seq < ops; ++seq) {
		if (probe)
			tr_read_begin(seq, kind + 1, fd);
		ssize_t got = pread(fd, actual, sizeof(actual) - 1, 0);
		if (probe)
			tr_read_end(seq, kind + 1, got);
		/* No retries, partial-read loop, lseek(), writes or implicit EOF read. */
		if (got != bytes || memcmp(actual, wanted, bytes))
			die("unexpected read result");
	}
	if (timing) {
		wall = clock_ns(CLOCK_MONOTONIC_RAW) - start;
		cpu_time = clock_ns(CLOCK_THREAD_CPUTIME_ID) - cpu_start;
	}
	if (sched_getcpu() != cpu || sched_getaffinity(0, sizeof(allowed), &allowed) ||
	    CPU_COUNT(&allowed) != 1 || !CPU_ISSET(cpu, &allowed) || close(fd))
		die("final affinity or close");
	alarm(0);
	printf("{\"schema\":\"rt-sysctl-read-v1\",\"mode\":\"%s\",\"case\":\"%s\","
	       "\"case_id\":%u,\"pid\":%d,\"cpu\":%d,\"reads\":%u,\"expected\":%ld,"
	       "\"bytes_per_read\":%d,\"total_bytes\":%u,\"semantic_pass\":true,"
	       "\"performance_evidence\":%s,\"wall_ns\":%" PRIu64 ",\"thread_cpu_ns\":%" PRIu64 "}\n",
	       argv[1], names[kind], kind + 1, getpid(), cpu, ops, expected, bytes,
	       ops * bytes, timing ? "true" : "false", wall, cpu_time);
	return 0;
}