mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: Chengfeng Lin <lin2530632123@gmail.com>
To: Joseph Salisbury <joseph.salisbury@oracle.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>,
	K Prateek Nayak <kprateek.nayak@amd.com>,
	linux-kernel@vger.kernel.org
Subject: Re: [PATCH] sched/rt: Rebuild domains only after successful RT sysctl writes
Date: Wed, 23 Sep 2026 17:20:36 +0000	[thread overview]
Message-ID: <179018297078.3.9126136955304841169@gmail.com> (raw)
In-Reply-To: <20260313183716.990792-1-joseph.salisbury@oracle.com>

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

      reply	other threads:[~2026-09-23 17:20 UTC|newest]

Thread overview: 2+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-03-13 18:37 Joseph Salisbury
2026-09-23 17:20 ` Chengfeng Lin [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=179018297078.3.9126136955304841169@gmail.com \
    --to=lin2530632123@gmail.com \
    --cc=bsegall@google.com \
    --cc=dietmar.eggemann@arm.com \
    --cc=joseph.salisbury@oracle.com \
    --cc=juri.lelli@redhat.com \
    --cc=kprateek.nayak@amd.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mgorman@suse.de \
    --cc=mingo@redhat.com \
    --cc=peterz@infradead.org \
    --cc=rostedt@goodmis.org \
    --cc=vincent.guittot@linaro.org \
    --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 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®