2024-02-16 18:31:46

by Joel Fernandes

[permalink] [raw]
Subject: [PATCH 00/10] Fair scheduling deadline server fixes

Hello,
The deadline server [1] allows RT tasks to run on a system safely, while not
wasting CPU that RT tasks may not get on an idle system due to RT throttling.

Here are patches that are mostly fixes that we found while testing out the
deadline server [1] for ChromeOS.

The main fix is to core scheduling, but we found several other issues.

These patches are based on Daniel's preview branch for v6:
https://git.kernel.org/pub/scm/linux/kernel/git/bristot/linux.git/?h=dl_server_v6

Daniel mentioned he is working on fixing the fair server interface issues [2].
These patches apply cleanly on his preview version.

[1] https://lore.kernel.org/all/[email protected]/
[2] https://lore.kernel.org/all/[email protected]/

Joel Fernandes (Google) (8):
sched/core: Add clearing of ->dl_server in put_prev_task_balance()
sched/core: Fix priority checking for DL server picks
sched/core: Fix picking of tasks for core scheduling with DL server
sched/debug: Use unsigned long for cpu variable to prevent cast errors
selftests/sched: Add a test to verify that DL server works with core
scheduling
selftests/sched: Migrate cs_prctl_test to kselfttest
admin-guide/hw-vuln: Correct prctl() argument description
sched: Fix build error in "sched/rt: Remove default bandwidth control"

Suleiman Souhlal (1):
sched: server: Don't start hrtick for DL server tasks

Youssef Esmat (1):
sched/core: Clear prev->dl_server in CFS pick fast path

.../admin-guide/hw-vuln/core-scheduling.rst | 4 +-
include/linux/sched.h | 3 +-
kernel/sched/core.c | 46 +++-
kernel/sched/deadline.c | 34 ++-
kernel/sched/debug.c | 4 +-
kernel/sched/fair.c | 22 +-
kernel/sched/rt.c | 2 +
kernel/sched/sched.h | 3 +-
tools/testing/selftests/sched/Makefile | 17 +-
tools/testing/selftests/sched/common.c | 24 ++
tools/testing/selftests/sched/common.h | 8 +
.../selftests/sched/cs_dlserver_test.c | 254 ++++++++++++++++++
tools/testing/selftests/sched/cs_prctl_test.c | 74 ++---
13 files changed, 424 insertions(+), 71 deletions(-)
create mode 100644 tools/testing/selftests/sched/common.c
create mode 100644 tools/testing/selftests/sched/common.h
create mode 100644 tools/testing/selftests/sched/cs_dlserver_test.c

--
2.34.1



2024-02-16 18:32:01

by Joel Fernandes

[permalink] [raw]
Subject: [PATCH 01/10] sched/core: Add clearing of ->dl_server in put_prev_task_balance()

Paths using put_prev_task_balance() need to do a pick shortly after. Make sure
they also clear the ->dl_server on prev as a part of that.

Signed-off-by: Joel Fernandes (Google) <[email protected]>
---
kernel/sched/core.c | 16 ++++++++--------
1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/kernel/sched/core.c b/kernel/sched/core.c
index 973fd610d089..7f3a2596c1ed 100644
--- a/kernel/sched/core.c
+++ b/kernel/sched/core.c
@@ -5998,6 +5998,14 @@ static void put_prev_task_balance(struct rq *rq, struct task_struct *prev,
#endif

put_prev_task(rq, prev);
+
+ /*
+ * We've updated @prev and no longer need the server link, clear it.
+ * Must be done before ->pick_next_task() because that can (re)set
+ * ->dl_server.
+ */
+ if (prev->dl_server)
+ prev->dl_server = NULL;
}

/*
@@ -6041,14 +6049,6 @@ __pick_next_task(struct rq *rq, struct task_struct *prev, struct rq_flags *rf)
restart:
put_prev_task_balance(rq, prev, rf);

- /*
- * We've updated @prev and no longer need the server link, clear it.
- * Must be done before ->pick_next_task() because that can (re)set
- * ->dl_server.
- */
- if (prev->dl_server)
- prev->dl_server = NULL;
-
for_each_class(class) {
p = class->pick_next_task(rq);
if (p)
--
2.34.1


2024-02-16 18:32:09

by Joel Fernandes

[permalink] [raw]
Subject: [PATCH 02/10] sched/core: Clear prev->dl_server in CFS pick fast path

From: Youssef Esmat <[email protected]>

In case the previous pick was a DL server pick, ->dl_server might be
set. Clear it in the fast path as well.

Signed-off-by: Youssef Esmat <[email protected]>
Signed-off-by: Joel Fernandes (Google) <[email protected]>
---
kernel/sched/core.c | 7 +++++++
1 file changed, 7 insertions(+)

diff --git a/kernel/sched/core.c b/kernel/sched/core.c
index 7f3a2596c1ed..63f41453b79e 100644
--- a/kernel/sched/core.c
+++ b/kernel/sched/core.c
@@ -6036,6 +6036,13 @@ __pick_next_task(struct rq *rq, struct task_struct *prev, struct rq_flags *rf)
p = pick_next_task_idle(rq);
}

+ /*
+ * This is a normal CFS pick, but the previous could be a DL pick.
+ * Clear it as previous is no longer picked.
+ */
+ if (prev->dl_server)
+ prev->dl_server = NULL;
+
/*
* This is the fast path; it cannot be a DL server pick;
* therefore even if @p == @prev, ->dl_server must be NULL.
--
2.34.1


2024-02-16 18:32:31

by Joel Fernandes

[permalink] [raw]
Subject: [PATCH 03/10] sched/core: Fix priority checking for DL server picks

In core scheduling, a DL server pick (which is CFS task) should be given higher
priority than tasks in other classes.

Not doing so causes CFS starvation. A kselftest is added later to demonstrate
this. A CFS task that is competing with RT tasks can be completely starved
without this and the DL server's boosting completely ignored.

Fix these problems.

Reviewed-by: Vineeth Pillai <[email protected]>
Reported-by: Suleiman Souhlal <[email protected]>
Signed-off-by: Joel Fernandes (Google) <[email protected]>
---
kernel/sched/core.c | 23 +++++++++++++++++++++--
1 file changed, 21 insertions(+), 2 deletions(-)

diff --git a/kernel/sched/core.c b/kernel/sched/core.c
index 63f41453b79e..0a05caf9d3d7 100644
--- a/kernel/sched/core.c
+++ b/kernel/sched/core.c
@@ -162,6 +162,9 @@ static inline int __task_prio(const struct task_struct *p)
if (p->sched_class == &stop_sched_class) /* trumps deadline */
return -2;

+ if (p->dl_server)
+ return -1; /* deadline */
+
if (rt_prio(p->prio)) /* includes deadline */
return p->prio; /* [-1, 99] */

@@ -191,8 +194,24 @@ static inline bool prio_less(const struct task_struct *a,
if (-pb < -pa)
return false;

- if (pa == -1) /* dl_prio() doesn't work because of stop_class above */
- return !dl_time_before(a->dl.deadline, b->dl.deadline);
+ if (pa == -1) { /* dl_prio() doesn't work because of stop_class above */
+ const struct sched_dl_entity *a_dl, *b_dl;
+
+ a_dl = &a->dl;
+ /*
+ * Since,'a' and 'b' can be CFS tasks served by DL server,
+ * __task_prio() can return -1 (for DL) even for those. In that
+ * case, get to the dl_server's DL entity.
+ */
+ if (a->dl_server)
+ a_dl = a->dl_server;
+
+ b_dl = &b->dl;
+ if (b->dl_server)
+ b_dl = b->dl_server;
+
+ return !dl_time_before(a_dl->deadline, b_dl->deadline);
+ }

if (pa == MAX_RT_PRIO + MAX_NICE) /* fair */
return cfs_prio_less(a, b, in_fi);
--
2.34.1


2024-02-16 18:33:40

by Joel Fernandes

[permalink] [raw]
Subject: [PATCH 07/10] selftests/sched: Add a test to verify that DL server works with core scheduling

This test verifies that DL server infrastructure gives CFS tasks a fixed
bandwidth even when RT tasks are being "core scheduled" on a core.
Verify that they are getting the expected bandwidth (and thus not being
starved).

Also verified that not having core scheduling fixes makes the test fail
as the CFS task gets no bandwidth.

Sample output:

# Runtime of PID 97 is 4.440000 seconds
# Runtime of PID 98 is 4.560000 seconds
# Runtime of PID 99 is 4.550000 seconds
ok 1 PASS

Notes about test that generated the sample output:

The test runs for 12 seconds. We check the runtimes at 9 seconds. We
expect the CFS task (PID 7) to get ~50% of the 9 seconds. The DL server
is configured for 50% bandwidth.

The RT tasks (PID 98, 99) each get 50% as well, because they run
concurrently on 2 hyperthreads of a core.

Signed-off-by: Joel Fernandes (Google) <[email protected]>
---
tools/testing/selftests/sched/Makefile | 13 +-
tools/testing/selftests/sched/common.c | 24 ++
tools/testing/selftests/sched/common.h | 8 +
.../selftests/sched/cs_dlserver_test.c | 254 ++++++++++++++++++
4 files changed, 290 insertions(+), 9 deletions(-)
create mode 100644 tools/testing/selftests/sched/common.c
create mode 100644 tools/testing/selftests/sched/common.h
create mode 100644 tools/testing/selftests/sched/cs_dlserver_test.c

diff --git a/tools/testing/selftests/sched/Makefile b/tools/testing/selftests/sched/Makefile
index 099ee9213557..f491d741cb45 100644
--- a/tools/testing/selftests/sched/Makefile
+++ b/tools/testing/selftests/sched/Makefile
@@ -1,14 +1,9 @@
# SPDX-License-Identifier: GPL-2.0+
+TEST_GEN_PROGS := cs_dlserver_test

-ifneq ($(shell $(CC) --version 2>&1 | head -n 1 | grep clang),)
-CLANG_FLAGS += -no-integrated-as
-endif
+cs_dlserver_test: cs_dlserver_test.c common.c

-CFLAGS += -O2 -Wall -g -I./ $(KHDR_INCLUDES) -Wl,-rpath=./ \
- $(CLANG_FLAGS)
-LDLIBS += -lpthread
-
-TEST_GEN_FILES := cs_prctl_test
-TEST_PROGS := cs_prctl_test
+CFLAGS += $(KHDR_INCLUDES)
+CFLAGS += -Wall

include ../lib.mk
diff --git a/tools/testing/selftests/sched/common.c b/tools/testing/selftests/sched/common.c
new file mode 100644
index 000000000000..5cf0022acc8d
--- /dev/null
+++ b/tools/testing/selftests/sched/common.c
@@ -0,0 +1,24 @@
+// SPDX-License-Identifier: GPL-2.0
+
+#include "common.h"
+
+bool hyperthreading_enabled(void)
+{
+ FILE *file = fopen("/sys/devices/system/cpu/smt/active", "r");
+ char smt_active[2];
+
+ if (file == NULL) {
+ ksft_print_msg("Could not determine if hyperthreading is enabled\n");
+ return false;
+ }
+
+ if (fgets(smt_active, sizeof(smt_active), file) == NULL) {
+ perror("Failed to read smt_active");
+ return false;
+ }
+ fclose(file);
+
+ if (smt_active[0] != '1')
+ return false;
+ return true;
+}
diff --git a/tools/testing/selftests/sched/common.h b/tools/testing/selftests/sched/common.h
new file mode 100644
index 000000000000..7bcedbd0ed99
--- /dev/null
+++ b/tools/testing/selftests/sched/common.h
@@ -0,0 +1,8 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+
+#include <stdio.h>
+#include <stdbool.h>
+#include <dirent.h>
+#include "../kselftest.h"
+
+bool hyperthreading_enabled(void);
diff --git a/tools/testing/selftests/sched/cs_dlserver_test.c b/tools/testing/selftests/sched/cs_dlserver_test.c
new file mode 100644
index 000000000000..9f2a74a25686
--- /dev/null
+++ b/tools/testing/selftests/sched/cs_dlserver_test.c
@@ -0,0 +1,254 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Use the DL server infrastructure to give CFS tasks a fixed bandwidth
+ * even when RT tasks are being "core scheduled" on a core. Verify that
+ * they are getting the expected bandwidth (and thus not being starved).
+ *
+ * Copyright (c) 2024 Google.
+ * Author: Joel Fernandes <[email protected]>
+ *
+ * This library is free software; you can redistribute it and/or modify it
+ * under the terms of version 2.1 of the GNU Lesser General Public License as
+ * published by the Free Software Foundation.
+ *
+ * This library is distributed in the hope that it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE. See the GNU Lesser General Public License
+ * for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public License
+ * along with this library; if not, see <http://www.gnu.org/licenses>.
+ */
+
+#define _GNU_SOURCE
+
+#include <stdlib.h>
+#include <unistd.h>
+#include <sched.h>
+#include <time.h>
+#include <sys/wait.h>
+#include <sys/types.h>
+#include <sys/prctl.h>
+#include <fcntl.h>
+#include <string.h>
+
+#include "common.h"
+
+enum pid_type {PIDTYPE_PID = 0, PIDTYPE_TGID, PIDTYPE_PGID};
+
+#define RUN_TIME 12 // Running time of the test in seconds
+#define CORE_ID 0 // Assuming we're pinning processes to the first core
+#define DL_SERVER_DEBUGFS "/sys/kernel/debug/sched/fair_server"
+
+void write_server_debugfs(char *file, char *type, unsigned long value)
+{
+ char path[1024], buf[1024];
+ int fd, n;
+
+ snprintf(path, sizeof(path), "%s/%s/%s", DL_SERVER_DEBUGFS, file, type);
+ fd = open(path, O_WRONLY);
+ if (fd == -1) {
+ perror("Failed to open file for writing");
+ return;
+ }
+ n = snprintf(buf, sizeof(buf), "%lu\n", value);
+ n = write(fd, buf, n);
+ if (n == -1)
+ perror("Failed to write file");
+
+ close(fd);
+}
+
+void write_dl_server_params(void)
+{
+ DIR *dir;
+ struct dirent *entry;
+
+ if (access(DL_SERVER_DEBUGFS, F_OK) == -1) {
+ perror("DL server debugfs not found, cannot set DL parameters.");
+ exit(EXIT_FAILURE);
+ }
+
+ dir = opendir(DL_SERVER_DEBUGFS);
+ if (dir == NULL) {
+ perror("Failed to open directory");
+ exit(EXIT_FAILURE);
+ }
+
+ while ((entry = readdir(dir)) != NULL) {
+ if (strcmp(entry->d_name, ".") == 0 || strcmp(entry->d_name, "..") == 0)
+ continue;
+
+ write_server_debugfs(entry->d_name, "period", 100000000);
+ write_server_debugfs(entry->d_name, "runtime", 50000000);
+ }
+ closedir(dir);
+}
+
+void process_func(void)
+{
+ unsigned long long count = 0;
+ time_t end;
+
+ // Busy loop for RUN_TIME seconds
+ end = time(NULL) + RUN_TIME;
+ while (time(NULL) < end) {
+ count++; // Just a dummy operation
+ }
+}
+
+void set_affinity(int cpu_id)
+{
+ cpu_set_t cpuset;
+
+ CPU_ZERO(&cpuset);
+ CPU_SET(cpu_id, &cpuset);
+ CPU_SET(cpu_id + 1, &cpuset);
+
+ if (sched_setaffinity(0, sizeof(cpu_set_t), &cpuset) != 0) {
+ perror("sched_setaffinity");
+ exit(EXIT_FAILURE);
+ }
+}
+
+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);
+ }
+}
+
+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; // Indicate failure
+ }
+
+ // 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; // Indicate failure
+ }
+
+ // 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;
+}
+
+int main(void)
+{
+ float runtime1, runtime2, runtime3;
+ int pid1, pid2, pid3;
+
+ if (!hyperthreading_enabled())
+ ksft_test_result_skip("This test requires hyperthreading to be enabled\n");
+
+ write_dl_server_params();
+
+ ksft_print_header();
+ ksft_set_plan(1);
+
+ // Create and set up a CFS task
+ pid1 = fork();
+ if (pid1 == 0) {
+ set_affinity(CORE_ID);
+ process_func();
+ exit(0);
+ } else if (pid1 < 0) {
+ perror("fork for p1");
+ ksft_exit_fail();
+ }
+
+ // Create a new unique cookie for the CFS task
+ if (prctl(PR_SCHED_CORE, PR_SCHED_CORE_CREATE, pid1, PIDTYPE_TGID, 0) < 0) {
+ perror("prctl for pid1");
+ ksft_exit_fail();
+ }
+
+ // Create a new unique cookie for the current process. Future
+ // forks will inherit this cookie.
+ if (prctl(PR_SCHED_CORE, PR_SCHED_CORE_CREATE, 0, PIDTYPE_TGID, 0) < 0) {
+ perror("prctl for current process");
+ ksft_exit_fail();
+ }
+
+ // Create an RT task which inherits the parent's cookie
+ pid2 = fork();
+ if (pid2 == 0) {
+ set_affinity(CORE_ID);
+ set_sched(SCHED_FIFO, 50);
+ process_func();
+ exit(0);
+ } else if (pid2 < 0) {
+ perror("fork for p2");
+ ksft_exit_fail();
+ }
+
+ // Create another RT task which inherits the parent's cookie
+ pid3 = fork();
+ if (pid3 == 0) {
+ set_affinity(CORE_ID);
+ set_sched(SCHED_FIFO, 50);
+ process_func();
+ exit(0);
+ } else if (pid3 < 0) {
+ perror("fork for p3");
+ ksft_exit_fail();
+ }
+
+ sleep(RUN_TIME * 3 / 4);
+ runtime1 = get_process_runtime(pid1);
+ if (runtime1 != -1)
+ ksft_print_msg("Runtime of PID %d is %f seconds\n", pid1, runtime1);
+ else
+ ksft_exit_fail_msg("Error getting runtime for PID %d\n", pid1);
+
+ runtime2 = get_process_runtime(pid2);
+ if (runtime2 != -1)
+ ksft_print_msg("Runtime of PID %d is %f seconds\n", pid2, runtime2);
+ else
+ ksft_exit_fail_msg("Error getting runtime for PID %d\n", pid2);
+
+ runtime3 = get_process_runtime(pid3);
+ if (runtime3 != -1)
+ ksft_print_msg("Runtime of PID %d is %f seconds\n", pid3, runtime3);
+ else
+ ksft_exit_fail_msg("Error getting runtime for PID %d\n", pid3);
+
+ // Make sure runtime1 is within 30% of runtime2
+ if (runtime1 < 0.7 * runtime2 || runtime1 > 1.3 * runtime2)
+ ksft_exit_fail_msg("Runtime of PID %d is not within 30%% of runtime of PID %d\n",
+ pid1, pid2);
+
+ // Make sure runtime1 is within 30% of runtime3
+ if (runtime1 < 0.7 * runtime3 || runtime1 > 1.3 * runtime3)
+ ksft_exit_fail_msg("Runtime of PID %d is not within 30%% of runtime of PID %d\n",
+ pid1, pid3);
+
+ waitpid(pid1, NULL, 0);
+ waitpid(pid2, NULL, 0);
+ waitpid(pid3, NULL, 0);
+
+ ksft_test_result_pass("PASS\n");
+ return 0;
+}
--
2.34.1


2024-02-16 18:33:56

by Joel Fernandes

[permalink] [raw]
Subject: [PATCH 08/10] selftests/sched: Migrate cs_prctl_test to kselfttest

This test begs to be a kselftest, is in the kselftest hierarchy and does
not even use a single kselftest API. Convert it.

It simplifies some of the code and the output also looks much nicer now:

Totals: pass:17 fail:0 xfail:0 xpass:0 skip:0 error:0

Signed-off-by: Joel Fernandes (Google) <[email protected]>
---
tools/testing/selftests/sched/Makefile | 6 +-
tools/testing/selftests/sched/cs_prctl_test.c | 74 ++++++++++---------
2 files changed, 43 insertions(+), 37 deletions(-)

diff --git a/tools/testing/selftests/sched/Makefile b/tools/testing/selftests/sched/Makefile
index f491d741cb45..90c53bc1337e 100644
--- a/tools/testing/selftests/sched/Makefile
+++ b/tools/testing/selftests/sched/Makefile
@@ -1,9 +1,11 @@
# SPDX-License-Identifier: GPL-2.0+
TEST_GEN_PROGS := cs_dlserver_test
-
-cs_dlserver_test: cs_dlserver_test.c common.c
+TEST_GEN_PROGS += cs_prctl_test

CFLAGS += $(KHDR_INCLUDES)
CFLAGS += -Wall

include ../lib.mk
+
+$(OUTPUT)/cs_dlserver_test: cs_dlserver_test.c common.c
+$(OUTPUT)/cs_prctl_test: cs_prctl_test.c common.c
diff --git a/tools/testing/selftests/sched/cs_prctl_test.c b/tools/testing/selftests/sched/cs_prctl_test.c
index 7ba057154343..bb7aee703cdf 100644
--- a/tools/testing/selftests/sched/cs_prctl_test.c
+++ b/tools/testing/selftests/sched/cs_prctl_test.c
@@ -28,10 +28,11 @@
#include <unistd.h>
#include <time.h>
#include <errno.h>
-#include <stdio.h>
#include <stdlib.h>
#include <string.h>

+#include "common.h"
+
#if __GLIBC_PREREQ(2, 30) == 0
#include <sys/syscall.h>
static pid_t gettid(void)
@@ -80,7 +81,7 @@ static int _prctl(int option, unsigned long arg2, unsigned long arg3, unsigned l
int res;

res = prctl(option, arg2, arg3, arg4, arg5);
- printf("%d = prctl(%d, %ld, %ld, %ld, %lx)\n", res, option, (long)arg2, (long)arg3,
+ ksft_print_msg("%d = prctl(%d, %ld, %ld, %ld, %lx)\n", res, option, (long)arg2, (long)arg3,
(long)arg4, arg5);
return res;
}
@@ -91,21 +92,20 @@ static int _prctl(int option, unsigned long arg2, unsigned long arg3, unsigned l
static void __handle_error(char *fn, int ln, char *msg)
{
int pidx;
- printf("(%s:%d) - ", fn, ln);
+ ksft_print_msg("(%s:%d) - ", fn, ln);
perror(msg);
if (need_cleanup) {
for (pidx = 0; pidx < num_processes; ++pidx)
kill(procs[pidx].cpid, 15);
need_cleanup = 0;
}
- exit(EXIT_FAILURE);
+ ksft_exit_fail();
}

static void handle_usage(int rc, char *msg)
{
- puts(USAGE);
- puts(msg);
- putchar('\n');
+ ksft_print_msg("%s\n", USAGE);
+ ksft_print_msg("%s\n\n", msg);
exit(rc);
}

@@ -117,7 +117,7 @@ static unsigned long get_cs_cookie(int pid)
ret = prctl(PR_SCHED_CORE, PR_SCHED_CORE_GET, pid, PIDTYPE_PID,
(unsigned long)&cookie);
if (ret) {
- printf("Not a core sched system\n");
+ ksft_print_msg("Not a core sched system\n");
return -1UL;
}

@@ -160,7 +160,7 @@ static int child_func_process(void *arg)

ret = write(ca->pfd[1], &ca->thr_tids, sizeof(int) * ca->num_threads);
if (ret == -1)
- printf("write failed on pfd[%d] - error (%s)\n",
+ ksft_print_msg("write failed on pfd[%d] - error (%s)\n",
ca->pfd[1], strerror(errno));

close(ca->pfd[1]);
@@ -192,7 +192,7 @@ void create_processes(int num_processes, int num_threads, struct child_args proc
for (i = 0; i < num_processes; ++i) {
ret = read(proc[i].pfd[0], &proc[i].thr_tids, sizeof(int) * proc[i].num_threads);
if (ret == -1)
- printf("read failed on proc[%d].pfd[0] error (%s)\n",
+ ksft_print_msg("read failed on proc[%d].pfd[0] error (%s)\n",
i, strerror(errno));
close(proc[i].pfd[0]);
}
@@ -202,30 +202,29 @@ void disp_processes(int num_processes, struct child_args proc[])
{
int i, j;

- printf("tid=%d, / tgid=%d / pgid=%d: %lx\n", gettid(), getpid(), getpgid(0),
+ ksft_print_msg("tid=%d, / tgid=%d / pgid=%d: %lx\n", gettid(), getpid(), getpgid(0),
get_cs_cookie(getpid()));

for (i = 0; i < num_processes; ++i) {
- printf(" tid=%d, / tgid=%d / pgid=%d: %lx\n", proc[i].cpid, proc[i].cpid,
+ ksft_print_msg(" tid=%d, / tgid=%d / pgid=%d: %lx\n", proc[i].cpid, proc[i].cpid,
getpgid(proc[i].cpid), get_cs_cookie(proc[i].cpid));
for (j = 0; j < proc[i].num_threads; ++j) {
- printf(" tid=%d, / tgid=%d / pgid=%d: %lx\n", proc[i].thr_tids[j],
+ ksft_print_msg(" tid=%d, / tgid=%d / pgid=%d: %lx\n", proc[i].thr_tids[j],
proc[i].cpid, getpgid(0), get_cs_cookie(proc[i].thr_tids[j]));
}
}
puts("\n");
}

-static int errors;
-
#define validate(v) _validate(__LINE__, v, #v)
void _validate(int line, int val, char *msg)
{
if (!val) {
- ++errors;
- printf("(%d) FAILED: %s\n", line, msg);
+ ksft_print_msg("(%d) FAILED: %s\n", line, msg);
+ ksft_inc_fail_cnt();
} else {
- printf("(%d) PASSED: %s\n", line, msg);
+ ksft_print_msg("(%d) PASSED: %s\n", line, msg);
+ ksft_inc_pass_cnt();
}
}

@@ -254,13 +253,17 @@ int main(int argc, char *argv[])
keypress = 1;
break;
case 'h':
- printf(USAGE);
+ ksft_print_msg(USAGE);
exit(EXIT_SUCCESS);
default:
handle_usage(20, "unknown option");
}
}

+ if (!hyperthreading_enabled()) {
+ ksft_exit_skip("This test requires hyperthreading to be enabled\n");
+ }
+
if (num_processes < 1 || num_processes > MAX_PROCESSES)
handle_usage(1, "Bad processes value");

@@ -272,17 +275,22 @@ int main(int argc, char *argv[])

srand(time(NULL));

- /* put into separate process group */
+ /* Put into separate process group */
if (setpgid(0, 0) != 0)
handle_error("process group");

- printf("\n## Create a thread/process/process group hiearchy\n");
+ ksft_print_header();
+
+ /* Increase the count if adding more validate() statements. */
+ ksft_set_plan(17);
+
+ ksft_print_msg("\n## Create a thread/process/process group hiearchy\n");
create_processes(num_processes, num_threads, procs);
need_cleanup = 1;
disp_processes(num_processes, procs);
validate(get_cs_cookie(0) == 0);

- printf("\n## Set a cookie on entire process group\n");
+ ksft_print_msg("\n## Set a cookie on entire process group\n");
if (_prctl(PR_SCHED_CORE, PR_SCHED_CORE_CREATE, 0, PIDTYPE_PGID, 0) < 0)
handle_error("core_sched create failed -- PGID");
disp_processes(num_processes, procs);
@@ -296,7 +304,7 @@ int main(int argc, char *argv[])
validate(get_cs_cookie(0) == get_cs_cookie(pid));
validate(get_cs_cookie(0) == get_cs_cookie(procs[pidx].thr_tids[0]));

- printf("\n## Set a new cookie on entire process/TGID [%d]\n", pid);
+ ksft_print_msg("\n## Set a new cookie on entire process/TGID [%d]\n", pid);
if (_prctl(PR_SCHED_CORE, PR_SCHED_CORE_CREATE, pid, PIDTYPE_TGID, 0) < 0)
handle_error("core_sched create failed -- TGID");
disp_processes(num_processes, procs);
@@ -305,7 +313,7 @@ int main(int argc, char *argv[])
validate(get_cs_cookie(pid) != 0);
validate(get_cs_cookie(pid) == get_cs_cookie(procs[pidx].thr_tids[0]));

- printf("\n## Copy the cookie of current/PGID[%d], to pid [%d] as PIDTYPE_PID\n",
+ ksft_print_msg("\n## Copy the cookie of current/PGID[%d], to pid [%d] as PIDTYPE_PID\n",
getpid(), pid);
if (_prctl(PR_SCHED_CORE, PR_SCHED_CORE_SHARE_TO, pid, PIDTYPE_PID, 0) < 0)
handle_error("core_sched share to itself failed -- PID");
@@ -315,7 +323,7 @@ int main(int argc, char *argv[])
validate(get_cs_cookie(pid) != 0);
validate(get_cs_cookie(pid) != get_cs_cookie(procs[pidx].thr_tids[0]));

- printf("\n## Copy cookie from a thread [%d] to current/PGID [%d] as PIDTYPE_PID\n",
+ ksft_print_msg("\n## Copy cookie from a thread [%d] to current/PGID [%d] as PIDTYPE_PID\n",
procs[pidx].thr_tids[0], getpid());
if (_prctl(PR_SCHED_CORE, PR_SCHED_CORE_SHARE_FROM, procs[pidx].thr_tids[0],
PIDTYPE_PID, 0) < 0)
@@ -325,7 +333,7 @@ int main(int argc, char *argv[])
validate(get_cs_cookie(0) == get_cs_cookie(procs[pidx].thr_tids[0]));
validate(get_cs_cookie(pid) != get_cs_cookie(procs[pidx].thr_tids[0]));

- printf("\n## Copy cookie from current [%d] to current as pidtype PGID\n", getpid());
+ ksft_print_msg("\n## Copy cookie from current [%d] to current as pidtype PGID\n", getpid());
if (_prctl(PR_SCHED_CORE, PR_SCHED_CORE_SHARE_TO, 0, PIDTYPE_PGID, 0) < 0)
handle_error("core_sched share to self failed -- PGID");
disp_processes(num_processes, procs);
@@ -340,20 +348,16 @@ int main(int argc, char *argv[])
validate(_prctl(PR_SCHED_CORE, PR_SCHED_CORE_SHARE_TO, 0, PIDTYPE_PGID, 1) < 0
&& errno == EINVAL);

- if (errors) {
- printf("TESTS FAILED. errors: %d\n", errors);
- res = 10;
- } else {
- printf("SUCCESS !!!\n");
- }
-
- if (keypress)
+ if (keypress) {
+ ksft_print_msg("Waiting for keypress to exit\n");
getchar();
- else
+ } else {
sleep(delay);
+ }

for (pidx = 0; pidx < num_processes; ++pidx)
kill(procs[pidx].cpid, 15);

+ ksft_finished();
return res;
}
--
2.34.1


2024-02-16 18:34:12

by Joel Fernandes

[permalink] [raw]
Subject: [PATCH 10/10] sched: Fix build error in "sched/rt: Remove default bandwidth control"

This fixes a build error introduced by "sched/rt: Remove default
bandwidth control". The issue happens because a function is unused
when !CONFIG_SMP. It could be squashed into the original patch.

Signed-off-by: Joel Fernandes (Google) <[email protected]>
---
kernel/sched/rt.c | 2 ++
1 file changed, 2 insertions(+)

diff --git a/kernel/sched/rt.c b/kernel/sched/rt.c
index 37bee56a70f7..d3065fe35c61 100644
--- a/kernel/sched/rt.c
+++ b/kernel/sched/rt.c
@@ -964,8 +964,10 @@ struct rt_rq *sched_rt_period_rt_rq(struct rt_bandwidth *rt_b, int cpu)
return &cpu_rq(cpu)->rt;
}

+#ifdef CONFIG_SMP
static void __enable_runtime(struct rq *rq) { }
static void __disable_runtime(struct rq *rq) { }
+#endif

#endif /* CONFIG_RT_GROUP_SCHED */

--
2.34.1


2024-02-16 18:42:18

by Joel Fernandes

[permalink] [raw]
Subject: [PATCH 09/10] admin-guide/hw-vuln: Correct prctl() argument description

Correct the description for arg4 which appears to be outdated.

Signed-off-by: Joel Fernandes (Google) <[email protected]>
---
Documentation/admin-guide/hw-vuln/core-scheduling.rst | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/Documentation/admin-guide/hw-vuln/core-scheduling.rst b/Documentation/admin-guide/hw-vuln/core-scheduling.rst
index cf1eeefdfc32..338d639a8127 100644
--- a/Documentation/admin-guide/hw-vuln/core-scheduling.rst
+++ b/Documentation/admin-guide/hw-vuln/core-scheduling.rst
@@ -62,8 +62,8 @@ arg3:

arg4:
``pid_type`` for which the operation applies. It is one of
- ``PR_SCHED_CORE_SCOPE_``-prefixed macro constants. For example, if arg4
- is ``PR_SCHED_CORE_SCOPE_THREAD_GROUP``, then the operation of this command
+ ``PIDTIME_``-prefixed macro constants. For example, if arg4
+ is ``PIDTYPE_TGID``, then the operation of this command
will be performed for all tasks in the task group of ``pid``.

arg5:
--
2.34.1


2024-02-16 18:43:23

by Joel Fernandes

[permalink] [raw]
Subject: [PATCH 05/10] sched/debug: Use unsigned long for cpu variable to prevent cast errors

This avoids compiler errors seen with clang:

In file included from kernel/sched/build_utility.c:72:
kernel/sched/debug.c:532:47: error: cast to 'void *' from smaller integer
type 'unsigned int' [-Werror,-Wint-to-void-pointer-cast]
debugfs_create_file("runtime", 0644, d_cpu, (void *) cpu, &fair_server_runtime_fops);

Signed-off-by: Joel Fernandes (Google) <[email protected]>
---
kernel/sched/debug.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/kernel/sched/debug.c b/kernel/sched/debug.c
index c6c0120ff105..2590041696bc 100644
--- a/kernel/sched/debug.c
+++ b/kernel/sched/debug.c
@@ -516,7 +516,7 @@ static struct dentry *debugfs_sched;
static void debugfs_fair_server_init(void)
{
struct dentry *d_fair;
- unsigned int cpu;
+ unsigned long cpu;

d_fair = debugfs_create_dir("fair_server", debugfs_sched);
if (!d_fair)
@@ -526,7 +526,7 @@ static void debugfs_fair_server_init(void)
struct dentry *d_cpu;
char buf[32];

- snprintf(buf, sizeof(buf), "cpu%d", cpu);
+ snprintf(buf, sizeof(buf), "cpu%lu", cpu);
d_cpu = debugfs_create_dir(buf, d_fair);

debugfs_create_file("runtime", 0644, d_cpu, (void *) cpu, &fair_server_runtime_fops);
--
2.34.1


2024-02-16 18:43:28

by Joel Fernandes

[permalink] [raw]
Subject: [PATCH 04/10] sched/core: Fix picking of tasks for core scheduling with DL server

* Use simple CFS pick_task for DL pick_task

DL server's pick_task calls CFS's pick_next_task_fair(), this is wrong
because core scheduling's pick_task only calls CFS's pick_task() for
evaluation / checking of the CFS task (comparing across CPUs), not for
actually affirmatively picking the next task. This causes RB tree corruption
issues in CFS that were found by syzbot.

* Make pick_task_fair clear DL server

A DL task pick might set ->dl_server, but it is possible the task will
never run (say the other HT has a stop task). If the CFS task is picked
in the future directly (say without DL server), ->dl_server will be
set. So clear it in pick_task_fair().

This fixes the KASAN issue reported by syzbot in set_next_entity().

(DL refactoring suggestions by Vineeth Pillai).

Reviewed-by: Vineeth Pillai <[email protected]>
Reported-by: Suleiman Souhlal <[email protected]>
Signed-off-by: Joel Fernandes (Google) <[email protected]>
---
include/linux/sched.h | 3 ++-
kernel/sched/deadline.c | 27 ++++++++++++++++++++++-----
kernel/sched/fair.c | 22 ++++++++++++++++++++--
kernel/sched/sched.h | 3 ++-
4 files changed, 46 insertions(+), 9 deletions(-)

diff --git a/include/linux/sched.h b/include/linux/sched.h
index a1f918674383..e5ad1f232b35 100644
--- a/include/linux/sched.h
+++ b/include/linux/sched.h
@@ -672,7 +672,8 @@ struct sched_dl_entity {
*/
struct rq *rq;
dl_server_has_tasks_f server_has_tasks;
- dl_server_pick_f server_pick;
+ dl_server_pick_f server_pick_next;
+ dl_server_pick_f server_pick_task;

#ifdef CONFIG_RT_MUTEXES
/*
diff --git a/kernel/sched/deadline.c b/kernel/sched/deadline.c
index f5aaa3adac94..8fafe3f8b59c 100644
--- a/kernel/sched/deadline.c
+++ b/kernel/sched/deadline.c
@@ -1567,11 +1567,13 @@ void dl_server_stop(struct sched_dl_entity *dl_se)

void dl_server_init(struct sched_dl_entity *dl_se, struct rq *rq,
dl_server_has_tasks_f has_tasks,
- dl_server_pick_f pick)
+ dl_server_pick_f pick_next,
+ dl_server_pick_f pick_task)
{
dl_se->rq = rq;
dl_se->server_has_tasks = has_tasks;
- dl_se->server_pick = pick;
+ dl_se->server_pick_next = pick_next;
+ dl_se->server_pick_task = pick_task;
}

int dl_server_apply_params(struct sched_dl_entity *dl_se, u64 runtime, u64 period, bool init)
@@ -2271,7 +2273,12 @@ static struct sched_dl_entity *pick_next_dl_entity(struct dl_rq *dl_rq)
return __node_2_dle(left);
}

-static struct task_struct *pick_task_dl(struct rq *rq)
+/*
+ * __pick_next_task_dl - Helper to pick the next -deadline task to run.
+ * @rq: The runqueue to pick the next task from.
+ * @peek: If true, just peek at the next task. Only relevant for dlserver.
+ */
+static struct task_struct *__pick_next_task_dl(struct rq *rq, bool peek)
{
struct sched_dl_entity *dl_se;
struct dl_rq *dl_rq = &rq->dl;
@@ -2285,7 +2292,10 @@ static struct task_struct *pick_task_dl(struct rq *rq)
WARN_ON_ONCE(!dl_se);

if (dl_server(dl_se)) {
- p = dl_se->server_pick(dl_se);
+ if (IS_ENABLED(CONFIG_SMP) && peek)
+ p = dl_se->server_pick_task(dl_se);
+ else
+ p = dl_se->server_pick_next(dl_se);
if (!p) {
WARN_ON_ONCE(1);
dl_se->dl_yielded = 1;
@@ -2300,11 +2310,18 @@ static struct task_struct *pick_task_dl(struct rq *rq)
return p;
}

+#ifdef CONFIG_SMP
+static struct task_struct *pick_task_dl(struct rq *rq)
+{
+ return __pick_next_task_dl(rq, true);
+}
+#endif
+
static struct task_struct *pick_next_task_dl(struct rq *rq)
{
struct task_struct *p;

- p = pick_task_dl(rq);
+ p = __pick_next_task_dl(rq, false);
if (!p)
return p;

diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
index b48287629610..9cc528a14001 100644
--- a/kernel/sched/fair.c
+++ b/kernel/sched/fair.c
@@ -8392,6 +8392,14 @@ static struct task_struct *pick_task_fair(struct rq *rq)
cfs_rq = group_cfs_rq(se);
} while (cfs_rq);

+ /*
+ * This can be called from directly from CFS's ->pick_task() or indirectly
+ * from DL's ->pick_task when fair server is enabled. In the indirect case,
+ * DL will set ->dl_server just after this function is called, so its Ok to
+ * clear. In the direct case, we are picking directly so we must clear it.
+ */
+ task_of(se)->dl_server = NULL;
+
return task_of(se);
}
#endif
@@ -8551,7 +8559,16 @@ static bool fair_server_has_tasks(struct sched_dl_entity *dl_se)
return !!dl_se->rq->cfs.nr_running;
}

-static struct task_struct *fair_server_pick(struct sched_dl_entity *dl_se)
+static struct task_struct *fair_server_pick_task(struct sched_dl_entity *dl_se)
+{
+#ifdef CONFIG_SMP
+ return pick_task_fair(dl_se->rq);
+#else
+ return NULL;
+#endif
+}
+
+static struct task_struct *fair_server_pick_next(struct sched_dl_entity *dl_se)
{
return pick_next_task_fair(dl_se->rq, NULL, NULL);
}
@@ -8561,7 +8578,8 @@ void fair_server_init(struct rq *rq)
struct sched_dl_entity *dl_se = &rq->fair_server;

init_dl_entity(dl_se);
- dl_server_init(dl_se, rq, fair_server_has_tasks, fair_server_pick);
+ dl_server_init(dl_se, rq, fair_server_has_tasks, fair_server_pick_next,
+ fair_server_pick_task);
}

/*
diff --git a/kernel/sched/sched.h b/kernel/sched/sched.h
index 4d2c216049cb..bfb15037489c 100644
--- a/kernel/sched/sched.h
+++ b/kernel/sched/sched.h
@@ -338,7 +338,8 @@ extern void dl_server_start(struct sched_dl_entity *dl_se);
extern void dl_server_stop(struct sched_dl_entity *dl_se);
extern void dl_server_init(struct sched_dl_entity *dl_se, struct rq *rq,
dl_server_has_tasks_f has_tasks,
- dl_server_pick_f pick);
+ dl_server_pick_f pick_next,
+ dl_server_pick_f pick_task);

extern void fair_server_init(struct rq *);
extern int dl_server_apply_params(struct sched_dl_entity *dl_se,
--
2.34.1


2024-02-16 18:43:46

by Joel Fernandes

[permalink] [raw]
Subject: [PATCH 06/10] sched: server: Don't start hrtick for DL server tasks

From: Suleiman Souhlal <[email protected]>

Otherwise, we might start it even for tasks in a sched class that should
have it off.

Signed-off-by: Suleiman Souhlal <[email protected]>
---
kernel/sched/deadline.c | 7 ++++---
1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/kernel/sched/deadline.c b/kernel/sched/deadline.c
index 8fafe3f8b59c..5adfc15803c3 100644
--- a/kernel/sched/deadline.c
+++ b/kernel/sched/deadline.c
@@ -2325,11 +2325,12 @@ static struct task_struct *pick_next_task_dl(struct rq *rq)
if (!p)
return p;

- if (!p->dl_server)
+ if (!p->dl_server) {
set_next_task_dl(rq, p, true);

- if (hrtick_enabled(rq))
- start_hrtick_dl(rq, &p->dl);
+ if (hrtick_enabled(rq))
+ start_hrtick_dl(rq, &p->dl);
+ }

return p;
}
--
2.34.1


2024-02-16 19:18:44

by Chris Hyser

[permalink] [raw]
Subject: Re: [PATCH 08/10] selftests/sched: Migrate cs_prctl_test to kselfttest

On 2/16/24 13:31, Joel Fernandes (Google) wrote:

> This test begs to be a kselftest, is in the kselftest hierarchy and does
> not even use a single kselftest API. Convert it.
>
> It simplifies some of the code and the output also looks much nicer now:
>
> Totals: pass:17 fail:0 xfail:0 xpass:0 skip:0 error:0
>
> Signed-off-by: Joel Fernandes (Google) <[email protected]>

Reviewed-by: Chris Hyser <[email protected]>


> ---
> tools/testing/selftests/sched/Makefile | 6 +-
> tools/testing/selftests/sched/cs_prctl_test.c | 74 ++++++++++---------
> 2 files changed, 43 insertions(+), 37 deletions(-)
>
> diff --git a/tools/testing/selftests/sched/Makefile b/tools/testing/selftests/sched/Makefile
> index f491d741cb45..90c53bc1337e 100644
> --- a/tools/testing/selftests/sched/Makefile
> +++ b/tools/testing/selftests/sched/Makefile
> @@ -1,9 +1,11 @@
> # SPDX-License-Identifier: GPL-2.0+
> TEST_GEN_PROGS := cs_dlserver_test
> -
> -cs_dlserver_test: cs_dlserver_test.c common.c
> +TEST_GEN_PROGS += cs_prctl_test
>
> CFLAGS += $(KHDR_INCLUDES)
> CFLAGS += -Wall
>
> include ../lib.mk
> +
> +$(OUTPUT)/cs_dlserver_test: cs_dlserver_test.c common.c
> +$(OUTPUT)/cs_prctl_test: cs_prctl_test.c common.c
> diff --git a/tools/testing/selftests/sched/cs_prctl_test.c b/tools/testing/selftests/sched/cs_prctl_test.c
> index 7ba057154343..bb7aee703cdf 100644
> --- a/tools/testing/selftests/sched/cs_prctl_test.c
> +++ b/tools/testing/selftests/sched/cs_prctl_test.c
> @@ -28,10 +28,11 @@
> #include <unistd.h>
> #include <time.h>
> #include <errno.h>
> -#include <stdio.h>
> #include <stdlib.h>
> #include <string.h>
>
> +#include "common.h"
> +
> #if __GLIBC_PREREQ(2, 30) == 0
> #include <sys/syscall.h>
> static pid_t gettid(void)
> @@ -80,7 +81,7 @@ static int _prctl(int option, unsigned long arg2, unsigned long arg3, unsigned l
> int res;
>
> res = prctl(option, arg2, arg3, arg4, arg5);
> - printf("%d = prctl(%d, %ld, %ld, %ld, %lx)\n", res, option, (long)arg2, (long)arg3,
> + ksft_print_msg("%d = prctl(%d, %ld, %ld, %ld, %lx)\n", res, option, (long)arg2, (long)arg3,
> (long)arg4, arg5);
> return res;
> }
> @@ -91,21 +92,20 @@ static int _prctl(int option, unsigned long arg2, unsigned long arg3, unsigned l
> static void __handle_error(char *fn, int ln, char *msg)
> {
> int pidx;
> - printf("(%s:%d) - ", fn, ln);
> + ksft_print_msg("(%s:%d) - ", fn, ln);
> perror(msg);
> if (need_cleanup) {
> for (pidx = 0; pidx < num_processes; ++pidx)
> kill(procs[pidx].cpid, 15);
> need_cleanup = 0;
> }
> - exit(EXIT_FAILURE);
> + ksft_exit_fail();
> }
>
> static void handle_usage(int rc, char *msg)
> {
> - puts(USAGE);
> - puts(msg);
> - putchar('\n');
> + ksft_print_msg("%s\n", USAGE);
> + ksft_print_msg("%s\n\n", msg);
> exit(rc);
> }
>
> @@ -117,7 +117,7 @@ static unsigned long get_cs_cookie(int pid)
> ret = prctl(PR_SCHED_CORE, PR_SCHED_CORE_GET, pid, PIDTYPE_PID,
> (unsigned long)&cookie);
> if (ret) {
> - printf("Not a core sched system\n");
> + ksft_print_msg("Not a core sched system\n");
> return -1UL;
> }
>
> @@ -160,7 +160,7 @@ static int child_func_process(void *arg)
>
> ret = write(ca->pfd[1], &ca->thr_tids, sizeof(int) * ca->num_threads);
> if (ret == -1)
> - printf("write failed on pfd[%d] - error (%s)\n",
> + ksft_print_msg("write failed on pfd[%d] - error (%s)\n",
> ca->pfd[1], strerror(errno));
>
> close(ca->pfd[1]);
> @@ -192,7 +192,7 @@ void create_processes(int num_processes, int num_threads, struct child_args proc
> for (i = 0; i < num_processes; ++i) {
> ret = read(proc[i].pfd[0], &proc[i].thr_tids, sizeof(int) * proc[i].num_threads);
> if (ret == -1)
> - printf("read failed on proc[%d].pfd[0] error (%s)\n",
> + ksft_print_msg("read failed on proc[%d].pfd[0] error (%s)\n",
> i, strerror(errno));
> close(proc[i].pfd[0]);
> }
> @@ -202,30 +202,29 @@ void disp_processes(int num_processes, struct child_args proc[])
> {
> int i, j;
>
> - printf("tid=%d, / tgid=%d / pgid=%d: %lx\n", gettid(), getpid(), getpgid(0),
> + ksft_print_msg("tid=%d, / tgid=%d / pgid=%d: %lx\n", gettid(), getpid(), getpgid(0),
> get_cs_cookie(getpid()));
>
> for (i = 0; i < num_processes; ++i) {
> - printf(" tid=%d, / tgid=%d / pgid=%d: %lx\n", proc[i].cpid, proc[i].cpid,
> + ksft_print_msg(" tid=%d, / tgid=%d / pgid=%d: %lx\n", proc[i].cpid, proc[i].cpid,
> getpgid(proc[i].cpid), get_cs_cookie(proc[i].cpid));
> for (j = 0; j < proc[i].num_threads; ++j) {
> - printf(" tid=%d, / tgid=%d / pgid=%d: %lx\n", proc[i].thr_tids[j],
> + ksft_print_msg(" tid=%d, / tgid=%d / pgid=%d: %lx\n", proc[i].thr_tids[j],
> proc[i].cpid, getpgid(0), get_cs_cookie(proc[i].thr_tids[j]));
> }
> }
> puts("\n");
> }
>
> -static int errors;
> -
> #define validate(v) _validate(__LINE__, v, #v)
> void _validate(int line, int val, char *msg)
> {
> if (!val) {
> - ++errors;
> - printf("(%d) FAILED: %s\n", line, msg);
> + ksft_print_msg("(%d) FAILED: %s\n", line, msg);
> + ksft_inc_fail_cnt();
> } else {
> - printf("(%d) PASSED: %s\n", line, msg);
> + ksft_print_msg("(%d) PASSED: %s\n", line, msg);
> + ksft_inc_pass_cnt();
> }
> }
>
> @@ -254,13 +253,17 @@ int main(int argc, char *argv[])
> keypress = 1;
> break;
> case 'h':
> - printf(USAGE);
> + ksft_print_msg(USAGE);
> exit(EXIT_SUCCESS);
> default:
> handle_usage(20, "unknown option");
> }
> }
>
> + if (!hyperthreading_enabled()) {
> + ksft_exit_skip("This test requires hyperthreading to be enabled\n");
> + }
> +
> if (num_processes < 1 || num_processes > MAX_PROCESSES)
> handle_usage(1, "Bad processes value");
>
> @@ -272,17 +275,22 @@ int main(int argc, char *argv[])
>
> srand(time(NULL));
>
> - /* put into separate process group */
> + /* Put into separate process group */
> if (setpgid(0, 0) != 0)
> handle_error("process group");
>
> - printf("\n## Create a thread/process/process group hiearchy\n");
> + ksft_print_header();
> +
> + /* Increase the count if adding more validate() statements. */
> + ksft_set_plan(17);
> +
> + ksft_print_msg("\n## Create a thread/process/process group hiearchy\n");
> create_processes(num_processes, num_threads, procs);
> need_cleanup = 1;
> disp_processes(num_processes, procs);
> validate(get_cs_cookie(0) == 0);
>
> - printf("\n## Set a cookie on entire process group\n");
> + ksft_print_msg("\n## Set a cookie on entire process group\n");
> if (_prctl(PR_SCHED_CORE, PR_SCHED_CORE_CREATE, 0, PIDTYPE_PGID, 0) < 0)
> handle_error("core_sched create failed -- PGID");
> disp_processes(num_processes, procs);
> @@ -296,7 +304,7 @@ int main(int argc, char *argv[])
> validate(get_cs_cookie(0) == get_cs_cookie(pid));
> validate(get_cs_cookie(0) == get_cs_cookie(procs[pidx].thr_tids[0]));
>
> - printf("\n## Set a new cookie on entire process/TGID [%d]\n", pid);
> + ksft_print_msg("\n## Set a new cookie on entire process/TGID [%d]\n", pid);
> if (_prctl(PR_SCHED_CORE, PR_SCHED_CORE_CREATE, pid, PIDTYPE_TGID, 0) < 0)
> handle_error("core_sched create failed -- TGID");
> disp_processes(num_processes, procs);
> @@ -305,7 +313,7 @@ int main(int argc, char *argv[])
> validate(get_cs_cookie(pid) != 0);
> validate(get_cs_cookie(pid) == get_cs_cookie(procs[pidx].thr_tids[0]));
>
> - printf("\n## Copy the cookie of current/PGID[%d], to pid [%d] as PIDTYPE_PID\n",
> + ksft_print_msg("\n## Copy the cookie of current/PGID[%d], to pid [%d] as PIDTYPE_PID\n",
> getpid(), pid);
> if (_prctl(PR_SCHED_CORE, PR_SCHED_CORE_SHARE_TO, pid, PIDTYPE_PID, 0) < 0)
> handle_error("core_sched share to itself failed -- PID");
> @@ -315,7 +323,7 @@ int main(int argc, char *argv[])
> validate(get_cs_cookie(pid) != 0);
> validate(get_cs_cookie(pid) != get_cs_cookie(procs[pidx].thr_tids[0]));
>
> - printf("\n## Copy cookie from a thread [%d] to current/PGID [%d] as PIDTYPE_PID\n",
> + ksft_print_msg("\n## Copy cookie from a thread [%d] to current/PGID [%d] as PIDTYPE_PID\n",
> procs[pidx].thr_tids[0], getpid());
> if (_prctl(PR_SCHED_CORE, PR_SCHED_CORE_SHARE_FROM, procs[pidx].thr_tids[0],
> PIDTYPE_PID, 0) < 0)
> @@ -325,7 +333,7 @@ int main(int argc, char *argv[])
> validate(get_cs_cookie(0) == get_cs_cookie(procs[pidx].thr_tids[0]));
> validate(get_cs_cookie(pid) != get_cs_cookie(procs[pidx].thr_tids[0]));
>
> - printf("\n## Copy cookie from current [%d] to current as pidtype PGID\n", getpid());
> + ksft_print_msg("\n## Copy cookie from current [%d] to current as pidtype PGID\n", getpid());
> if (_prctl(PR_SCHED_CORE, PR_SCHED_CORE_SHARE_TO, 0, PIDTYPE_PGID, 0) < 0)
> handle_error("core_sched share to self failed -- PGID");
> disp_processes(num_processes, procs);
> @@ -340,20 +348,16 @@ int main(int argc, char *argv[])
> validate(_prctl(PR_SCHED_CORE, PR_SCHED_CORE_SHARE_TO, 0, PIDTYPE_PGID, 1) < 0
> && errno == EINVAL);
>
> - if (errors) {
> - printf("TESTS FAILED. errors: %d\n", errors);
> - res = 10;
> - } else {
> - printf("SUCCESS !!!\n");
> - }
> -
> - if (keypress)
> + if (keypress) {
> + ksft_print_msg("Waiting for keypress to exit\n");
> getchar();
> - else
> + } else {
> sleep(delay);
> + }
>
> for (pidx = 0; pidx < num_processes; ++pidx)
> kill(procs[pidx].cpid, 15);
>
> + ksft_finished();
> return res;
> }

2024-02-21 00:03:28

by Shuah Khan

[permalink] [raw]
Subject: Re: [PATCH 08/10] selftests/sched: Migrate cs_prctl_test to kselfttest

On 2/16/24 12:18, Chris Hyser wrote:
> On 2/16/24 13:31, Joel Fernandes (Google) wrote:
>
>> This test begs to be a kselftest, is in the kselftest hierarchy and does
>> not even use a single kselftest API. Convert it.
>>
>> It simplifies some of the code and the output also looks much nicer now:
>>
>>   Totals: pass:17 fail:0 xfail:0 xpass:0 skip:0 error:0
>>
>> Signed-off-by: Joel Fernandes (Google) <[email protected]>
>
> Reviewed-by: Chris Hyser <[email protected]>
>

Assuming this is going through sched tree,

Acked-by: Shuah Khan <[email protected]>

thanks,
-- Shuah

2024-02-21 00:22:57

by Joel Fernandes

[permalink] [raw]
Subject: Re: [PATCH 08/10] selftests/sched: Migrate cs_prctl_test to kselfttest



On 2/20/2024 7:03 PM, Shuah Khan wrote:
> On 2/16/24 12:18, Chris Hyser wrote:
>> On 2/16/24 13:31, Joel Fernandes (Google) wrote:
>>
>>> This test begs to be a kselftest, is in the kselftest hierarchy and does
>>> not even use a single kselftest API. Convert it.
>>>
>>> It simplifies some of the code and the output also looks much nicer now:
>>>
>>>   Totals: pass:17 fail:0 xfail:0 xpass:0 skip:0 error:0
>>>
>>> Signed-off-by: Joel Fernandes (Google) <[email protected]>
>>
>> Reviewed-by: Chris Hyser <[email protected]>
>>
>
> Assuming this is going through sched tree,
>
> Acked-by: Shuah Khan <[email protected]>

Thanks, I think Peter is out so it may take a while going through the sched
tree. This individual patch is independent as such and could go through the
kselftest tree. However, I am totally Ok with either option.

Also, thanks to you and Chris for the tags and I will add them for future
submissions.

- Joel

2024-02-21 00:23:07

by Joel Fernandes

[permalink] [raw]
Subject: Re: [PATCH 08/10] selftests/sched: Migrate cs_prctl_test to kselfttest



On 2/20/2024 7:03 PM, Shuah Khan wrote:
> On 2/16/24 12:18, Chris Hyser wrote:
>> On 2/16/24 13:31, Joel Fernandes (Google) wrote:
>>
>>> This test begs to be a kselftest, is in the kselftest hierarchy and does
>>> not even use a single kselftest API. Convert it.
>>>
>>> It simplifies some of the code and the output also looks much nicer now:
>>>
>>>   Totals: pass:17 fail:0 xfail:0 xpass:0 skip:0 error:0
>>>
>>> Signed-off-by: Joel Fernandes (Google) <[email protected]>
>>
>> Reviewed-by: Chris Hyser <[email protected]>
>>
>
> Assuming this is going through sched tree,
>
> Acked-by: Shuah Khan <[email protected]>

Thanks, I think Peter is out so it may take a while going through the sched
tree. This individual patch is independent as such and could go through the
kselftest tree. However, I am totally Ok with either option.

Also, thanks to you and Chris for the tags and I will add them for future postings.

- Joel

Subject: Re: [PATCH 06/10] sched: server: Don't start hrtick for DL server tasks

On 2/16/24 19:31, Joel Fernandes (Google) wrote:
> From: Suleiman Souhlal <[email protected]>
>
> Otherwise, we might start it even for tasks in a sched class that should
> have it off.

If the task is fair, but it is running inside a DL reservation, we want the
hrtick for the dl reservation.

This is one of the reasons why we are moving to the DL server, having microseconds
granularity for the boost duration (runtime).

-- Daniel

>
> Signed-off-by: Suleiman Souhlal <[email protected]>
> ---
> kernel/sched/deadline.c | 7 ++++---
> 1 file changed, 4 insertions(+), 3 deletions(-)
>
> diff --git a/kernel/sched/deadline.c b/kernel/sched/deadline.c
> index 8fafe3f8b59c..5adfc15803c3 100644
> --- a/kernel/sched/deadline.c
> +++ b/kernel/sched/deadline.c
> @@ -2325,11 +2325,12 @@ static struct task_struct *pick_next_task_dl(struct rq *rq)
> if (!p)
> return p;
>
> - if (!p->dl_server)
> + if (!p->dl_server) {
> set_next_task_dl(rq, p, true);
>
> - if (hrtick_enabled(rq))
> - start_hrtick_dl(rq, &p->dl);
> + if (hrtick_enabled(rq))
> + start_hrtick_dl(rq, &p->dl);
> + }
>
> return p;
> }