* [PATCH v2 sched_ext/for-7.2 0/2] sched_ext: Add cookie API for early qseq capture
@ 2026-05-06 16:04 Cheng-Yang Chou
2026-05-06 16:04 ` [PATCH v2 1/2] " Cheng-Yang Chou
` (2 more replies)
0 siblings, 3 replies; 7+ messages in thread
From: Cheng-Yang Chou @ 2026-05-06 16:04 UTC (permalink / raw)
To: sched-ext, Tejun Heo, David Vernet, Andrea Righi, Changwoo Min
Cc: Kuba Piecuch, Ching-Chun Huang, Chia-Ping Tsai, yphbchou0911
scx_bpf_dsq_insert() captures the task's sequence number at insert time,
so any pre-dispatch validity checks a BPF scheduler performs before the
insert fall outside the race detection window.
Patch 1 introduces two new kfuncs:
- scx_bpf_task_get_cookie() captures the sequence number early, before
any pre-dispatch checks
- scx_bpf_dsq_insert_with_cookie() inserts using that early-captured
value, extending the window. A stale cookie causes finish_dispatch()
to silently discard the dispatch. This is intended for schedulers that
do not implement properly synchronized dequeue.
Patch 2 adds a selftest exercising the two new kfuncs.
This series is discussed in [1].
[1]: https://lore.kernel.org/r/20260319083518.94673-1-arighi@nvidia.com/
Changes in v2:
- Reword kdoc for both kfuncs to avoid exposing qseq and ops_state as
implementation details (Kuba Piecuch)
- Add __ksym __weak declarations for new kfuncs in common.bpf.h
- Add dispatch_cookie selftest (Kuba Piecuch)
- Link to v1:
https://lore.kernel.org/r/20260506075925.371138-1-yphbchou0911@gmail.com/
Thanks,
Cheng-Yang
---
Cheng-Yang Chou (2):
sched_ext: Add cookie API for early qseq capture
selftests/sched_ext: Add dispatch_cookie test
kernel/sched/ext.c | 69 ++++++++++++++++-
tools/sched_ext/include/scx/common.bpf.h | 2 +
tools/testing/selftests/sched_ext/Makefile | 1 +
.../selftests/sched_ext/dispatch_cookie.bpf.c | 38 ++++++++++
.../selftests/sched_ext/dispatch_cookie.c | 74 +++++++++++++++++++
5 files changed, 180 insertions(+), 4 deletions(-)
create mode 100644 tools/testing/selftests/sched_ext/dispatch_cookie.bpf.c
create mode 100644 tools/testing/selftests/sched_ext/dispatch_cookie.c
--
2.48.1
^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH v2 1/2] sched_ext: Add cookie API for early qseq capture
2026-05-06 16:04 [PATCH v2 sched_ext/for-7.2 0/2] sched_ext: Add cookie API for early qseq capture Cheng-Yang Chou
@ 2026-05-06 16:04 ` Cheng-Yang Chou
2026-05-06 20:34 ` sashiko-bot
2026-05-06 16:04 ` [PATCH v2 2/2] selftests/sched_ext: Add dispatch_cookie test Cheng-Yang Chou
2026-05-08 18:51 ` [PATCH v2 sched_ext/for-7.2 0/2] sched_ext: Add cookie API for early qseq capture Tejun Heo
2 siblings, 1 reply; 7+ messages in thread
From: Cheng-Yang Chou @ 2026-05-06 16:04 UTC (permalink / raw)
To: sched-ext, Tejun Heo, David Vernet, Andrea Righi, Changwoo Min
Cc: Kuba Piecuch, Ching-Chun Huang, Chia-Ping Tsai, yphbchou0911
scx_bpf_dsq_insert() captures qseq at insert time. Any BPF-side
pre-dispatch checks (e.g. CPU affinity validation) performed before
the insert are outside the qseq protection window: a dequeue/re-enqueue
race occurring between the check and the insert goes undetected by
finish_dispatch(), which sees a matching qseq and proceeds with stale
assumptions.
Introduce two new kfuncs to extend the qseq protection window:
- scx_bpf_task_get_cookie(p)
Reads @p's current qseq from ops_state and returns it as an opaque
u64 cookie. The BPF scheduler should call this before performing
pre-dispatch validity checks. The cookie may be stored in BPF maps
to support cross-CPU dispatch patterns.
- scx_bpf_dsq_insert_with_cookie(p, dsq_id, enq_flags, cookie)
Like scx_bpf_dsq_insert() with slice=0, but uses the cookie's qseq
instead of re-reading ops_state at insert time. If @p was dequeued
and re-enqueued between get_cookie() and here, qseq will have
changed and finish_dispatch() will silently discard the stale
dispatch. Use scx_bpf_task_set_slice() to set a non-default slice.
To support explicit qseq passing, refactor scx_dsq_insert_commit() to
take qseq as a parameter; all existing callers capture ops_state at
their call site, preserving the original behavior.
This mechanism is intended for schedulers that do not implement
properly synchronized dequeue. A scheduler whose ops.dequeue()
synchronizes atomically with the dispatch path does not need cookies.
Suggested-by: Tejun Heo <tj@kernel.org>
Suggested-by: Kuba Piecuch <jpiecuch@google.com>
Suggested-by: Andrea Righi <arighi@nvidia.com>
Reported-by: Andrea Righi <arighi@nvidia.com>
Link: https://lore.kernel.org/r/20260203230639.1259869-1-arighi@nvidia.com/
Signed-off-by: Cheng-Yang Chou <yphbchou0911@gmail.com>
---
kernel/sched/ext.c | 69 ++++++++++++++++++++++--
tools/sched_ext/include/scx/common.bpf.h | 2 +
2 files changed, 67 insertions(+), 4 deletions(-)
diff --git a/kernel/sched/ext.c b/kernel/sched/ext.c
index c80f0efd42c0..ff70550e1c12 100644
--- a/kernel/sched/ext.c
+++ b/kernel/sched/ext.c
@@ -8303,7 +8303,7 @@ static bool scx_dsq_insert_preamble(struct scx_sched *sch, struct task_struct *p
}
static void scx_dsq_insert_commit(struct scx_sched *sch, struct task_struct *p,
- u64 dsq_id, u64 enq_flags)
+ u64 dsq_id, u64 enq_flags, unsigned long qseq)
{
struct scx_dsp_ctx *dspc = &this_cpu_ptr(sch->pcpu)->dsp_ctx;
struct task_struct *ddsp_task;
@@ -8321,7 +8321,7 @@ static void scx_dsq_insert_commit(struct scx_sched *sch, struct task_struct *p,
dspc->buf[dspc->cursor++] = (struct scx_dsp_buf_ent){
.task = p,
- .qseq = atomic_long_read(&p->scx.ops_state) & SCX_OPSS_QSEQ_MASK,
+ .qseq = qseq,
.dsq_id = dsq_id,
.enq_flags = enq_flags,
};
@@ -8388,7 +8388,8 @@ __bpf_kfunc bool scx_bpf_dsq_insert___v2(struct task_struct *p, u64 dsq_id,
else
p->scx.slice = p->scx.slice ?: 1;
- scx_dsq_insert_commit(sch, p, dsq_id, enq_flags);
+ scx_dsq_insert_commit(sch, p, dsq_id, enq_flags,
+ atomic_long_read(&p->scx.ops_state) & SCX_OPSS_QSEQ_MASK);
return true;
}
@@ -8416,7 +8417,8 @@ static bool scx_dsq_insert_vtime(struct scx_sched *sch, struct task_struct *p,
p->scx.dsq_vtime = vtime;
- scx_dsq_insert_commit(sch, p, dsq_id, enq_flags | SCX_ENQ_DSQ_PRIQ);
+ scx_dsq_insert_commit(sch, p, dsq_id, enq_flags | SCX_ENQ_DSQ_PRIQ,
+ atomic_long_read(&p->scx.ops_state) & SCX_OPSS_QSEQ_MASK);
return true;
}
@@ -8505,13 +8507,71 @@ __bpf_kfunc void scx_bpf_dsq_insert_vtime(struct task_struct *p, u64 dsq_id,
scx_dsq_insert_vtime(sch, p, dsq_id, slice, vtime, enq_flags);
}
+/**
+ * scx_bpf_task_get_cookie - Get an opaque dispatch cookie for a task
+ * @p: task_struct to read cookie from
+ *
+ * Returns an opaque u64 dispatch cookie. Pass the cookie to
+ * scx_bpf_dsq_insert_with_cookie() to extend the time window during
+ * which sched_ext will detect racing dequeues/enqueues of @p. The
+ * extended window begins with this call and ends at the same point as
+ * for dispatches without cookies, i.e. when sched_ext attempts to
+ * finish the dispatch.
+ *
+ * This API is intended for schedulers that do not implement properly
+ * synchronized dequeue.
+ */
+__bpf_kfunc u64 scx_bpf_task_get_cookie(struct task_struct *p)
+{
+ return atomic_long_read(&p->scx.ops_state) & SCX_OPSS_QSEQ_MASK;
+}
+
+/**
+ * scx_bpf_dsq_insert_with_cookie - Insert a task using an early-captured cookie
+ * @p: task_struct to insert
+ * @dsq_id: DSQ to insert into
+ * @enq_flags: SCX_ENQ_*
+ * @cookie: cookie from scx_bpf_task_get_cookie()
+ * @aux: implicit BPF argument
+ *
+ * Like scx_bpf_dsq_insert() with slice=0, but uses an early-captured
+ * @cookie to extend the dispatch race detection window. A stale cookie
+ * causes sched_ext to silently discard the dispatch. Use
+ * scx_bpf_task_set_slice() to set a non-default slice.
+ *
+ * Returns %true on success, %false on failure.
+ */
+__bpf_kfunc bool scx_bpf_dsq_insert_with_cookie(struct task_struct *p,
+ u64 dsq_id, u64 enq_flags,
+ u64 cookie,
+ const struct bpf_prog_aux *aux)
+{
+ struct scx_sched *sch;
+
+ guard(rcu)();
+ sch = scx_prog_sched(aux);
+ if (unlikely(!sch))
+ return false;
+
+ if (!scx_dsq_insert_preamble(sch, p, dsq_id, &enq_flags))
+ return false;
+
+ p->scx.slice = p->scx.slice ?: 1;
+
+ scx_dsq_insert_commit(sch, p, dsq_id, enq_flags, (unsigned long)cookie);
+
+ return true;
+}
+
__bpf_kfunc_end_defs();
BTF_KFUNCS_START(scx_kfunc_ids_enqueue_dispatch)
BTF_ID_FLAGS(func, scx_bpf_dsq_insert, KF_IMPLICIT_ARGS | KF_RCU)
BTF_ID_FLAGS(func, scx_bpf_dsq_insert___v2, KF_IMPLICIT_ARGS | KF_RCU)
+BTF_ID_FLAGS(func, scx_bpf_dsq_insert_with_cookie, KF_IMPLICIT_ARGS | KF_RCU)
BTF_ID_FLAGS(func, __scx_bpf_dsq_insert_vtime, KF_IMPLICIT_ARGS | KF_RCU)
BTF_ID_FLAGS(func, scx_bpf_dsq_insert_vtime, KF_RCU)
+BTF_ID_FLAGS(func, scx_bpf_task_get_cookie, KF_RCU)
BTF_KFUNCS_END(scx_kfunc_ids_enqueue_dispatch)
static const struct btf_kfunc_id_set scx_kfunc_set_enqueue_dispatch = {
@@ -10181,6 +10241,7 @@ BTF_ID_FLAGS(func, scx_bpf_put_cpumask, KF_RELEASE)
BTF_ID_FLAGS(func, scx_bpf_task_running, KF_RCU)
BTF_ID_FLAGS(func, scx_bpf_task_cpu, KF_RCU)
BTF_ID_FLAGS(func, scx_bpf_task_cid, KF_RCU)
+BTF_ID_FLAGS(func, scx_bpf_task_get_cookie, KF_RCU)
BTF_ID_FLAGS(func, scx_bpf_cpu_rq, KF_IMPLICIT_ARGS)
BTF_ID_FLAGS(func, scx_bpf_locked_rq, KF_IMPLICIT_ARGS | KF_RET_NULL)
BTF_ID_FLAGS(func, scx_bpf_cpu_curr, KF_IMPLICIT_ARGS | KF_RET_NULL | KF_RCU_PROTECTED)
diff --git a/tools/sched_ext/include/scx/common.bpf.h b/tools/sched_ext/include/scx/common.bpf.h
index 5f715d69cde6..ae04f54d3dd1 100644
--- a/tools/sched_ext/include/scx/common.bpf.h
+++ b/tools/sched_ext/include/scx/common.bpf.h
@@ -63,6 +63,8 @@ s32 scx_bpf_select_cpu_dfl(struct task_struct *p, s32 prev_cpu, u64 wake_flags,
s32 __scx_bpf_select_cpu_and(struct task_struct *p, const struct cpumask *cpus_allowed,
struct scx_bpf_select_cpu_and_args *args) __ksym __weak;
bool __scx_bpf_dsq_insert_vtime(struct task_struct *p, struct scx_bpf_dsq_insert_vtime_args *args) __ksym __weak;
+u64 scx_bpf_task_get_cookie(struct task_struct *p) __ksym __weak;
+bool scx_bpf_dsq_insert_with_cookie(struct task_struct *p, u64 dsq_id, u64 enq_flags, u64 cookie) __ksym __weak;
u32 scx_bpf_dispatch_nr_slots(void) __ksym;
void scx_bpf_dispatch_cancel(void) __ksym;
void scx_bpf_kick_cpu(s32 cpu, u64 flags) __ksym;
--
2.48.1
^ permalink raw reply related [flat|nested] 7+ messages in thread
* [PATCH v2 2/2] selftests/sched_ext: Add dispatch_cookie test
2026-05-06 16:04 [PATCH v2 sched_ext/for-7.2 0/2] sched_ext: Add cookie API for early qseq capture Cheng-Yang Chou
2026-05-06 16:04 ` [PATCH v2 1/2] " Cheng-Yang Chou
@ 2026-05-06 16:04 ` Cheng-Yang Chou
2026-05-06 20:52 ` sashiko-bot
2026-05-08 18:51 ` [PATCH v2 sched_ext/for-7.2 0/2] sched_ext: Add cookie API for early qseq capture Tejun Heo
2 siblings, 1 reply; 7+ messages in thread
From: Cheng-Yang Chou @ 2026-05-06 16:04 UTC (permalink / raw)
To: sched-ext, Tejun Heo, David Vernet, Andrea Righi, Changwoo Min
Cc: Kuba Piecuch, Ching-Chun Huang, Chia-Ping Tsai, yphbchou0911
Test scx_bpf_task_get_cookie() and scx_bpf_dsq_insert_with_cookie().
The scheduler captures a cookie via scx_bpf_task_get_cookie() before
performing pre-dispatch validity checks, then dispatches tasks through
scx_bpf_dsq_insert_with_cookie(). The userspace side verifies all
children complete successfully and that nr_cookie_dispatched reflects
actual cookie-based dispatch activity.
Suggested-by: Kuba Piecuch <jpiecuch@google.com>
Signed-off-by: Cheng-Yang Chou <yphbchou0911@gmail.com>
---
tools/testing/selftests/sched_ext/Makefile | 1 +
.../selftests/sched_ext/dispatch_cookie.bpf.c | 38 ++++++++++
.../selftests/sched_ext/dispatch_cookie.c | 74 +++++++++++++++++++
3 files changed, 113 insertions(+)
create mode 100644 tools/testing/selftests/sched_ext/dispatch_cookie.bpf.c
create mode 100644 tools/testing/selftests/sched_ext/dispatch_cookie.c
diff --git a/tools/testing/selftests/sched_ext/Makefile b/tools/testing/selftests/sched_ext/Makefile
index 5d2dffca0e91..ae3dc0913378 100644
--- a/tools/testing/selftests/sched_ext/Makefile
+++ b/tools/testing/selftests/sched_ext/Makefile
@@ -164,6 +164,7 @@ all_test_bpfprogs := $(foreach prog,$(wildcard *.bpf.c),$(INCLUDE_DIR)/$(patsubs
auto-test-targets := \
create_dsq \
dequeue \
+ dispatch_cookie \
enq_last_no_enq_fails \
ddsp_bogus_dsq_fail \
ddsp_vtimelocal_fail \
diff --git a/tools/testing/selftests/sched_ext/dispatch_cookie.bpf.c b/tools/testing/selftests/sched_ext/dispatch_cookie.bpf.c
new file mode 100644
index 000000000000..cb198a5f50e8
--- /dev/null
+++ b/tools/testing/selftests/sched_ext/dispatch_cookie.bpf.c
@@ -0,0 +1,38 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * Test scx_bpf_task_get_cookie() and scx_bpf_dsq_insert_with_cookie().
+ *
+ * Copyright (C) 2026 Ching-Chun (Jim) Huang <jserv@ccns.ncku.edu.tw>
+ * Copyright (C) 2026 Cheng-Yang Chou <yphbchou0911@gmail.com>
+ */
+#include <scx/common.bpf.h>
+
+char _license[] SEC("license") = "GPL";
+
+UEI_DEFINE(uei);
+
+long nr_cookie_dispatched;
+
+void BPF_STRUCT_OPS(dispatch_cookie_enqueue, struct task_struct *p,
+ u64 enq_flags)
+{
+ u64 cookie = scx_bpf_task_get_cookie(p);
+
+ if (scx_bpf_dsq_insert_with_cookie(p, SCX_DSQ_GLOBAL, 0, cookie))
+ __sync_fetch_and_add(&nr_cookie_dispatched, 1);
+ else
+ scx_bpf_dsq_insert(p, SCX_DSQ_GLOBAL, SCX_SLICE_DFL, enq_flags);
+}
+
+void BPF_STRUCT_OPS(dispatch_cookie_exit, struct scx_exit_info *ei)
+{
+ UEI_RECORD(uei, ei);
+}
+
+SEC(".struct_ops.link")
+struct sched_ext_ops dispatch_cookie_ops = {
+ .enqueue = (void *) dispatch_cookie_enqueue,
+ .exit = (void *) dispatch_cookie_exit,
+ .name = "dispatch_cookie",
+ .timeout_ms = 5000U,
+};
diff --git a/tools/testing/selftests/sched_ext/dispatch_cookie.c b/tools/testing/selftests/sched_ext/dispatch_cookie.c
new file mode 100644
index 000000000000..f799b5828f9d
--- /dev/null
+++ b/tools/testing/selftests/sched_ext/dispatch_cookie.c
@@ -0,0 +1,74 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * Test scx_bpf_task_get_cookie() and scx_bpf_dsq_insert_with_cookie().
+ *
+ * Copyright (C) 2026 Ching-Chun (Jim) Huang <jserv@ccns.ncku.edu.tw>
+ * Copyright (C) 2026 Cheng-Yang Chou <yphbchou0911@gmail.com>
+ */
+#include <bpf/bpf.h>
+#include <scx/common.h>
+#include <sys/wait.h>
+#include <unistd.h>
+#include "dispatch_cookie.bpf.skel.h"
+#include "scx_test.h"
+
+#define NUM_CHILDREN 128
+
+static enum scx_test_status setup(void **ctx)
+{
+ struct dispatch_cookie *skel;
+
+ skel = dispatch_cookie__open();
+ SCX_FAIL_IF(!skel, "Failed to open skel");
+ SCX_ENUM_INIT(skel);
+ SCX_FAIL_IF(dispatch_cookie__load(skel), "Failed to load skel");
+
+ *ctx = skel;
+
+ return SCX_TEST_PASS;
+}
+
+static enum scx_test_status run(void *ctx)
+{
+ struct dispatch_cookie *skel = ctx;
+ struct bpf_link *link;
+ pid_t pids[NUM_CHILDREN];
+ int i, status;
+
+ link = bpf_map__attach_struct_ops(skel->maps.dispatch_cookie_ops);
+ SCX_FAIL_IF(!link, "Failed to attach scheduler");
+
+ for (i = 0; i < NUM_CHILDREN; i++) {
+ pids[i] = fork();
+ if (pids[i] == 0)
+ exit(0);
+ }
+
+ for (i = 0; i < NUM_CHILDREN; i++) {
+ SCX_EQ(waitpid(pids[i], &status, 0), pids[i]);
+ SCX_EQ(status, 0);
+ }
+
+ bpf_link__destroy(link);
+
+ SCX_GT(skel->bss->nr_cookie_dispatched, 0);
+
+ return SCX_TEST_PASS;
+}
+
+static void cleanup(void *ctx)
+{
+ struct dispatch_cookie *skel = ctx;
+
+ dispatch_cookie__destroy(skel);
+}
+
+struct scx_test dispatch_cookie = {
+ .name = "dispatch_cookie",
+ .description = "Verify scx_bpf_task_get_cookie() and "
+ "scx_bpf_dsq_insert_with_cookie() dispatch tasks correctly",
+ .setup = setup,
+ .run = run,
+ .cleanup = cleanup,
+};
+REGISTER_SCX_TEST(&dispatch_cookie)
--
2.48.1
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH v2 1/2] sched_ext: Add cookie API for early qseq capture
2026-05-06 16:04 ` [PATCH v2 1/2] " Cheng-Yang Chou
@ 2026-05-06 20:34 ` sashiko-bot
0 siblings, 0 replies; 7+ messages in thread
From: sashiko-bot @ 2026-05-06 20:34 UTC (permalink / raw)
To: Cheng-Yang Chou; +Cc: sched-ext
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] scx_bpf_task_get_cookie() reads the sequence counter (ops_state) without acquire semantics, failing to prevent the BPF program's pre-dispatch checks from being reordered before the cookie capture.
--
commit 4466cb81e5097999461b1978bdcc6623cb37dcf1
Author: Cheng-Yang Chou <yphbchou0911@gmail.com>
sched_ext: Add cookie API for early qseq capture
This commit introduces a cookie API for early qseq capture, extending the
protection window for BPF-side pre-dispatch checks against concurrent
dequeue/re-enqueue races.
Link: https://lore.kernel.org/r/20260203230639.1259869-1-arighi@nvidia.com/
> diff --git a/kernel/sched/ext.c b/kernel/sched/ext.c
> index 5600618dcef8e..f28baf7ce79cc 100644
> --- a/kernel/sched/ext.c
> +++ b/kernel/sched/ext.c
[ ... ]
> @@ -8505,13 +8507,71 @@ __bpf_kfunc void scx_bpf_dsq_insert_vtime(struct task_struct *p, u64 dsq_id,
> scx_dsq_insert_vtime(sch, p, dsq_id, slice, vtime, enq_flags);
> }
>
> +/**
> + * scx_bpf_task_get_cookie - Get an opaque dispatch cookie for a task
> + * @p: task_struct to read cookie from
> + *
> + * Returns an opaque u64 dispatch cookie. Pass the cookie to
> + * scx_bpf_dsq_insert_with_cookie() to extend the time window during
> + * which sched_ext will detect racing dequeues/enqueues of @p. The
> + * extended window begins with this call and ends at the same point as
> + * for dispatches without cookies, i.e. when sched_ext attempts to
> + * finish the dispatch.
> + *
> + * This API is intended for schedulers that do not implement properly
> + * synchronized dequeue.
> + */
> +__bpf_kfunc u64 scx_bpf_task_get_cookie(struct task_struct *p)
> +{
> + return atomic_long_read(&p->scx.ops_state) & SCX_OPSS_QSEQ_MASK;
> +}
Does this sequence counter read require acquire semantics?
Because this kfunc acts as the read-begin phase of a sequence lock for the
BPF program, using atomic_long_read() might allow the CPU to speculatively
reorder the BPF program's subsequent memory loads before the ops_state read
on weakly-ordered architectures.
If a concurrent task state update occurs between the reordered data read and
the ops_state read, could the CPU fetch stale task data but capture the new
ops_state as its cookie?
Could this missing ordering cause a regression where the dispatch is falsely
validated later, since the cookie would match the task's current ops_state?
Would it make sense to use atomic_long_read_acquire() here or include an
smp_rmb() to enforce the necessary load-load ordering?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260506160412.522199-1-yphbchou0911@gmail.com?part=1
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v2 2/2] selftests/sched_ext: Add dispatch_cookie test
2026-05-06 16:04 ` [PATCH v2 2/2] selftests/sched_ext: Add dispatch_cookie test Cheng-Yang Chou
@ 2026-05-06 20:52 ` sashiko-bot
2026-05-07 10:43 ` Cheng-Yang Chou
0 siblings, 1 reply; 7+ messages in thread
From: sashiko-bot @ 2026-05-06 20:52 UTC (permalink / raw)
To: Cheng-Yang Chou; +Cc: sched-ext
Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [High] Failing an assertion in the run() function leaks the bpf_link and leaves the BPF scheduler permanently attached.
- [Medium] The BPF program incorrectly drops the kernel-provided enq_flags when calling scx_bpf_dsq_insert_with_cookie().
- [Medium] The return value of fork() is not checked for errors, leading to incorrect waitpid() behavior.
--
commit de7f9ee9893bdc6cb049e25475e095b028e1ed26
Author: Cheng-Yang Chou <yphbchou0911@gmail.com>
selftests/sched_ext: Add dispatch_cookie test
This commit adds a new sched_ext selftest to verify the behavior of
scx_bpf_task_get_cookie() and scx_bpf_dsq_insert_with_cookie(). It captures
a cookie before dispatch and verifies that userspace accurately reflects
the cookie-based dispatch activity.
> diff --git a/tools/testing/selftests/sched_ext/dispatch_cookie.bpf.c b/tools/testing/selftests/sched_ext/dispatch_cookie.bpf.c
> new file mode 100644
> index 0000000000000..cb198a5f50e89
> --- /dev/null
> +++ b/tools/testing/selftests/sched_ext/dispatch_cookie.bpf.c
[ ... ]
> +void BPF_STRUCT_OPS(dispatch_cookie_enqueue, struct task_struct *p,
> + u64 enq_flags)
> +{
> + u64 cookie = scx_bpf_task_get_cookie(p);
> +
> + if (scx_bpf_dsq_insert_with_cookie(p, SCX_DSQ_GLOBAL, 0, cookie))
Does this code unintentionally drop the enq_flags provided by the kernel?
The fallback branch correctly passes enq_flags to scx_bpf_dsq_insert(),
but hardcoding 0 here might alter task placement context by dropping
directives like SCX_ENQ_WAKEUP.
> + __sync_fetch_and_add(&nr_cookie_dispatched, 1);
> + else
> + scx_bpf_dsq_insert(p, SCX_DSQ_GLOBAL, SCX_SLICE_DFL, enq_flags);
> +}
[ ... ]
> diff --git a/tools/testing/selftests/sched_ext/dispatch_cookie.c b/tools/testing/selftests/sched_ext/dispatch_cookie.c
> new file mode 100644
> index 0000000000000..f799b5828f9d6
> --- /dev/null
> +++ b/tools/testing/selftests/sched_ext/dispatch_cookie.c
[ ... ]
> +static enum scx_test_status run(void *ctx)
> +{
> + struct dispatch_cookie *skel = ctx;
> + struct bpf_link *link;
> + pid_t pids[NUM_CHILDREN];
> + int i, status;
> +
> + link = bpf_map__attach_struct_ops(skel->maps.dispatch_cookie_ops);
> + SCX_FAIL_IF(!link, "Failed to attach scheduler");
> +
> + for (i = 0; i < NUM_CHILDREN; i++) {
> + pids[i] = fork();
Should the return value of fork() be checked here?
If fork() fails and returns -1, waitpid() in the subsequent loop will be
called with -1, causing it to wait for any child process rather than the
specific process. This could lead to out-of-order reaping and confusing
test failures.
> + if (pids[i] == 0)
> + exit(0);
> + }
> +
> + for (i = 0; i < NUM_CHILDREN; i++) {
> + SCX_EQ(waitpid(pids[i], &status, 0), pids[i]);
> + SCX_EQ(status, 0);
Can this cause a resource leak that affects other tests?
SCX_EQ() forces an immediate return upon failure. If a check fails here,
the function will return before reaching bpf_link__destroy(link).
Leaving the BPF scheduler attached could cause subsequent tests in the
runner to fail with -EBUSY, since sched_ext only allows one active
scheduler globally.
> + }
> +
> + bpf_link__destroy(link);
> +
> + SCX_GT(skel->bss->nr_cookie_dispatched, 0);
> +
> + return SCX_TEST_PASS;
> +}
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260506160412.522199-1-yphbchou0911@gmail.com?part=2
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v2 2/2] selftests/sched_ext: Add dispatch_cookie test
2026-05-06 20:52 ` sashiko-bot
@ 2026-05-07 10:43 ` Cheng-Yang Chou
0 siblings, 0 replies; 7+ messages in thread
From: Cheng-Yang Chou @ 2026-05-07 10:43 UTC (permalink / raw)
To: sashiko; +Cc: sched-ext
On Wed, May 06, 2026 at 08:52:34PM +0000, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
> - [High] Failing an assertion in the run() function leaks the bpf_link and leaves the BPF scheduler permanently attached.
> - [Medium] The BPF program incorrectly drops the kernel-provided enq_flags when calling scx_bpf_dsq_insert_with_cookie().
> - [Medium] The return value of fork() is not checked for errors, leading to incorrect waitpid() behavior.
> --
>
> commit de7f9ee9893bdc6cb049e25475e095b028e1ed26
> Author: Cheng-Yang Chou <yphbchou0911@gmail.com>
> selftests/sched_ext: Add dispatch_cookie test
>
> This commit adds a new sched_ext selftest to verify the behavior of
> scx_bpf_task_get_cookie() and scx_bpf_dsq_insert_with_cookie(). It captures
> a cookie before dispatch and verifies that userspace accurately reflects
> the cookie-based dispatch activity.
>
> > diff --git a/tools/testing/selftests/sched_ext/dispatch_cookie.bpf.c b/tools/testing/selftests/sched_ext/dispatch_cookie.bpf.c
> > new file mode 100644
> > index 0000000000000..cb198a5f50e89
> > --- /dev/null
> > +++ b/tools/testing/selftests/sched_ext/dispatch_cookie.bpf.c
> [ ... ]
> > +void BPF_STRUCT_OPS(dispatch_cookie_enqueue, struct task_struct *p,
> > + u64 enq_flags)
> > +{
> > + u64 cookie = scx_bpf_task_get_cookie(p);
> > +
> > + if (scx_bpf_dsq_insert_with_cookie(p, SCX_DSQ_GLOBAL, 0, cookie))
>
> Does this code unintentionally drop the enq_flags provided by the kernel?
>
> The fallback branch correctly passes enq_flags to scx_bpf_dsq_insert(),
> but hardcoding 0 here might alter task placement context by dropping
> directives like SCX_ENQ_WAKEUP.
>
> > + __sync_fetch_and_add(&nr_cookie_dispatched, 1);
> > + else
> > + scx_bpf_dsq_insert(p, SCX_DSQ_GLOBAL, SCX_SLICE_DFL, enq_flags);
> > +}
> [ ... ]
> > diff --git a/tools/testing/selftests/sched_ext/dispatch_cookie.c b/tools/testing/selftests/sched_ext/dispatch_cookie.c
> > new file mode 100644
> > index 0000000000000..f799b5828f9d6
> > --- /dev/null
> > +++ b/tools/testing/selftests/sched_ext/dispatch_cookie.c
> [ ... ]
> > +static enum scx_test_status run(void *ctx)
> > +{
> > + struct dispatch_cookie *skel = ctx;
> > + struct bpf_link *link;
> > + pid_t pids[NUM_CHILDREN];
> > + int i, status;
> > +
> > + link = bpf_map__attach_struct_ops(skel->maps.dispatch_cookie_ops);
> > + SCX_FAIL_IF(!link, "Failed to attach scheduler");
> > +
> > + for (i = 0; i < NUM_CHILDREN; i++) {
> > + pids[i] = fork();
>
> Should the return value of fork() be checked here?
>
> If fork() fails and returns -1, waitpid() in the subsequent loop will be
> called with -1, causing it to wait for any child process rather than the
> specific process. This could lead to out-of-order reaping and confusing
> test failures.
>
> > + if (pids[i] == 0)
> > + exit(0);
> > + }
> > +
> > + for (i = 0; i < NUM_CHILDREN; i++) {
> > + SCX_EQ(waitpid(pids[i], &status, 0), pids[i]);
> > + SCX_EQ(status, 0);
>
> Can this cause a resource leak that affects other tests?
>
> SCX_EQ() forces an immediate return upon failure. If a check fails here,
> the function will return before reaching bpf_link__destroy(link).
>
> Leaving the BPF scheduler attached could cause subsequent tests in the
> runner to fail with -EBUSY, since sched_ext only allows one active
> scheduler globally.
This is a valid issue, which can be fixed by moving bpf_link__destory()
before the assertion loop.
Will address this in v3, but would like to wait for more reviews before
sending it. Thanks!
>
> > + }
> > +
> > + bpf_link__destroy(link);
> > +
> > + SCX_GT(skel->bss->nr_cookie_dispatched, 0);
> > +
> > + return SCX_TEST_PASS;
> > +}
>
> --
> Sashiko AI review · https://sashiko.dev/#/patchset/20260506160412.522199-1-yphbchou0911@gmail.com?part=2
--
Cheers,
Cheng-Yang
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v2 sched_ext/for-7.2 0/2] sched_ext: Add cookie API for early qseq capture
2026-05-06 16:04 [PATCH v2 sched_ext/for-7.2 0/2] sched_ext: Add cookie API for early qseq capture Cheng-Yang Chou
2026-05-06 16:04 ` [PATCH v2 1/2] " Cheng-Yang Chou
2026-05-06 16:04 ` [PATCH v2 2/2] selftests/sched_ext: Add dispatch_cookie test Cheng-Yang Chou
@ 2026-05-08 18:51 ` Tejun Heo
2 siblings, 0 replies; 7+ messages in thread
From: Tejun Heo @ 2026-05-08 18:51 UTC (permalink / raw)
To: Cheng-Yang Chou
Cc: sched-ext, David Vernet, Andrea Righi, Changwoo Min, Kuba Piecuch,
Ching-Chun Huang, Chia-Ping Tsai
Hello, Cheng-Yang.
The cookie/qseq vocabulary leaks the implementation. The API is
really a dispatch transaction where commit fails if the task is no
longer ours to dispatch (dequeued, or claimed by someone else).
Naming along those lines would describe what the API does, e.g.:
scx_bpf_dsq_insert_begin(p) -> u64 cookie
scx_bpf_dsq_insert_commit(p, dsq_id, enq_flags, cookie)
The internal scx_dsq_insert_commit() helper would need a rename.
The kdoc can then describe the transaction directly without
referring to qseq or ops_state.
For the selftest: the BPF program calls insert from ops.enqueue(),
which goes through mark_direct_dispatch() and never stages an entry
in the dispatch buffer. The cookie is silently ignored. The test
would still pass if patch 1/2 stored 0 in place of the cookie. To
actually exercise the race-detection window, the insert needs to
happen from ops.dispatch() with something invalidating the task in
between (e.g. an affinity flip).
Thanks.
--
tejun
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2026-05-08 18:51 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-05-06 16:04 [PATCH v2 sched_ext/for-7.2 0/2] sched_ext: Add cookie API for early qseq capture Cheng-Yang Chou
2026-05-06 16:04 ` [PATCH v2 1/2] " Cheng-Yang Chou
2026-05-06 20:34 ` sashiko-bot
2026-05-06 16:04 ` [PATCH v2 2/2] selftests/sched_ext: Add dispatch_cookie test Cheng-Yang Chou
2026-05-06 20:52 ` sashiko-bot
2026-05-07 10:43 ` Cheng-Yang Chou
2026-05-08 18:51 ` [PATCH v2 sched_ext/for-7.2 0/2] sched_ext: Add cookie API for early qseq capture Tejun Heo
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox