All of lore.kernel.org
 help / color / mirror / Atom feed
From: Tejun Heo <tj@kernel.org>
To: David Vernet <void@manifault.com>,
	Andrea Righi <arighi@nvidia.com>,
	Changwoo Min <changwoo@igalia.com>
Cc: sched-ext@lists.linux.dev, Emil Tsalapatis <emil@etsalapatis.com>,
	linux-kernel@vger.kernel.org, Tejun Heo <tj@kernel.org>
Subject: [PATCH sched_ext/for-7.3-fixes 4/4] sched_ext: scx_qmap: Place only on cids whose caps are in effect
Date: Sat,  5 Sep 2026 06:09:58 -1000	[thread overview]
Message-ID: <20260905160958.1565156-5-tj@kernel.org> (raw)
In-Reply-To: <20260905160958.1565156-1-tj@kernel.org>

qmap decides placements from self_cids, which redistribute() derives from
the caps view at ops.sub_caps_updated() time. That view runs ahead of the
cpus: a granted cid can be in self_cids before its cpu has reported the caps
in effect through ops.sub_ecaps_updated(). ops.update_idle() only comes once
BASE is in effect, so the idle-gated placements reach such a cid only
through an idle bit left over from an earlier hold. The highpri scan has no
gate at all:

  parent                    cpu Y, qmap               cpu X
  grants ENQ on X to qmap
                            sub_caps_updated() adds X
                            to self_cids
                            highpri scan moves a task
                            to X with PREEMPT
                                                      caps not in effect,
                                                      move denied, task
                                                      bounced with REENQ_CAP
                                                      reject drain, enqueue
                            the scan moves it to X
                            again
                                                      denied again
                                                      dispatch syncs ecaps,
                                                      sub_ecaps_updated(X)

Every highpri move to X in that window is denied and bounced. The two
callbacks are meant to split the roles: ops.sub_caps_updated() tracks what
the node holds and drives what it delegates to its children, while
ops.sub_ecaps_updated() says whether a task can run on a cpu now. qmap used
the first for both. Track the caps in effect from ops.sub_ecaps_updated() as
avail_cids and place only on self_cids & avail_cids, so that self_cids stays
the delegation split and avail_cids gates the placement.

The stranded tests keep self_cids, as they ask whether the split gives the
task anywhere at all. A highpri task whose self_cids lack caps in effect
waits for them instead of being moved and bounced.

Signed-off-by: Tejun Heo <tj@kernel.org>
---
 tools/sched_ext/scx_qmap.bpf.c | 83 ++++++++++++++++++++++++++--------
 tools/sched_ext/scx_qmap.h     |  3 ++
 2 files changed, 68 insertions(+), 18 deletions(-)

diff --git a/tools/sched_ext/scx_qmap.bpf.c b/tools/sched_ext/scx_qmap.bpf.c
index e4e51303bd29..062bb22ee65c 100644
--- a/tools/sched_ext/scx_qmap.bpf.c
+++ b/tools/sched_ext/scx_qmap.bpf.c
@@ -24,6 +24,9 @@
  *            time-share that stays self-local.
  *   self   - The excl cpus the node kept for itself, plus all of held_shared.
  *   owner  - Who holds a cid - a child slot, CID_SELF, or CID_NONE.
+ *   avail  - Cpus whose caps are in effect, per ops.sub_ecaps_updated().
+ *   usable - self AND avail. Placement decisions use this: self is the
+ *            delegation split and can run ahead of what the cpus honor.
  *
  * The scheduler splits its held-excl cpus among self and the children in
  * proportion to each node's cpu.weight, handing each the floor of its share as
@@ -208,8 +211,8 @@ static int qmap_spin_lock(struct bpf_res_spin_lock *lock)
 }
 
 /*
- * Try prev_cid, then scan cpus_allowed AND idle_cids AND self_cids round-robin
- * from prev_cid + 1. Atomic claim retries on race; bounded by
+ * Try prev_cid, then scan cpus_allowed AND idle_cids AND usable_cids
+ * round-robin from prev_cid + 1. Atomic claim retries on race; bounded by
  * IDLE_PICK_RETRIES to keep the verifier's insn budget in check.
  */
 #define IDLE_PICK_RETRIES	16
@@ -221,7 +224,7 @@ static s32 pick_direct_dispatch_cid(struct task_struct *p, s32 prev_cid,
 	s32 cid;
 	u32 i;
 
-	if (cmask_test(prev_cid, &qa.self_cids.mask) &&
+	if (cmask_test(prev_cid, &qa.usable_cids.mask) &&
 	    cmask_test_and_clear(prev_cid, &qa.idle_cids.mask))
 		return prev_cid;
 
@@ -229,7 +232,7 @@ static s32 pick_direct_dispatch_cid(struct task_struct *p, s32 prev_cid,
 	bpf_for(i, 0, IDLE_PICK_RETRIES) {
 		cid = cmask_next_and2_set_wrap(&taskc->cpus_allowed,
 					       &qa.idle_cids.mask,
-					       &qa.self_cids.mask, cid + 1);
+					       &qa.usable_cids.mask, cid + 1);
 		barrier_var(cid);
 		if (cid >= nr_cids)
 			return -1;
@@ -542,7 +545,7 @@ void BPF_STRUCT_OPS(qmap_enqueue, struct task_struct *p, u64 enq_flags)
 		scx_bpf_dsq_insert(p, SHARED_DSQ, 0, enq_flags);
 		cid = cmask_next_and2_set_wrap(&taskc->cpus_allowed,
 					       &qa.idle_cids.mask,
-					       &qa.self_cids.mask, 0);
+					       &qa.usable_cids.mask, 0);
 		if (cid < scx_bpf_nr_cids())
 			scx_bpf_kick_cid(cid, SCX_KICK_IDLE);
 		return;
@@ -646,18 +649,23 @@ static bool scan_shared_dsq(bool from_timer)
 		if (!(taskc = lookup_task_ctx(p)))
 			return false;
 
-		/* only run highpri tasks on cids this node holds, not delegated ones */
+		/* only run highpri tasks on cids this node can use right now */
 		if (cmask_test(this_cid, &taskc->cpus_allowed) &&
-		    cmask_test(this_cid, &qa.self_cids.mask))
+		    cmask_test(this_cid, &qa.usable_cids.mask))
 			cid = this_cid;
 		else
 			cid = cmask_next_and_set_wrap(&taskc->cpus_allowed,
-						      &qa.self_cids.mask,
+						      &qa.usable_cids.mask,
 						      this_cid + 1);
 		if (cid >= nr_cids) {
-			/* stranded after the cull - rescue it from here */
-			s32 c = cmask_next_set_wrap(&taskc->cpus_allowed, 0);
+			s32 c;
+
+			/* self cids lack caps in effect yet, leave it queued */
+			if (cmask_intersects(&taskc->cpus_allowed, &qa.self_cids.mask))
+				continue;
 
+			/* stranded after the cull - rescue it from here */
+			c = cmask_next_set_wrap(&taskc->cpus_allowed, 0);
 			if (c >= 0 && c < nr_cids) {
 				__sync_fetch_and_add(&qa.nr_rescue_dsp, 1);
 				scx_bpf_dsq_move(BPF_FOR_EACH_ITER, p, SCX_DSQ_LOCAL_ON | c,
@@ -1115,7 +1123,7 @@ void BPF_STRUCT_OPS(qmap_update_idle, s32 cid, bool idle)
 	/*
 	 * The kernel delivers update_idle() for every cid this node holds
 	 * SCX_CAP_BASE on. Track every cid's idle state regardless of
-	 * delegation: the direct-dispatch pick masks idle_cids with self_cids
+	 * delegation: the direct-dispatch pick masks idle_cids with usable_cids
 	 * at selection, so a cid already idle when it returns to self needs no
 	 * reseed here.
 	 */
@@ -1539,6 +1547,19 @@ static __noinline void account_alloc(void)
 	}
 }
 
+/*
+ * usable_cids = self_cids & avail_cids. The inputs have separate writers,
+ * apply_partition() and qmap_sub_ecaps_updated(), so the result is rebuilt in
+ * full under the partition guard, in scratch first so that readers never see
+ * self_cids alone.
+ */
+static void refresh_usable(void)
+{
+	cmask_copy(&qa.usable_scratch.mask, &qa.self_cids.mask);
+	cmask_and(&qa.usable_scratch.mask, &qa.avail_cids.mask);
+	cmask_copy(&qa.usable_cids.mask, &qa.usable_scratch.mask);
+}
+
 /*
  * apply_partition - execute the plan compute_partition() built
  *
@@ -1561,6 +1582,7 @@ __noinline void apply_partition(void)
 	/* no excl cpu: run own tasks on the held shares, evict children */
 	if (!qa.part.nr_excl) {
 		cmask_copy(&qa.self_cids.mask, &qa.held_shared.mask);
+		refresh_usable();
 		bpf_for(i, 0, MAX_SUB_SCHEDS)
 			if (qa.sub_sched_ctxs[i].cgroup_id)
 				scx_bpf_sub_kill(qa.sub_sched_ctxs[i].cgroup_id,
@@ -1598,6 +1620,7 @@ __noinline void apply_partition(void)
 		else if (o == CID_SELF)
 			cmask_set(cid, &qa.self_cids.mask);
 	}
+	refresh_usable();
 
 	/*
 	 * Apply each child's exclusive cids as a delta against its previous
@@ -1839,8 +1862,11 @@ s32 BPF_STRUCT_OPS_SLEEPABLE(qmap_init)
 	cmask_init(&qa.rr_cids.mask, 0, nr_cids);
 	cmask_init(&qa.prev_rr_cids.mask, 0, nr_cids);
 	cmask_init(&qa.self_cids.mask, 0, nr_cids);
+	cmask_init(&qa.avail_cids.mask, 0, nr_cids);
+	cmask_init(&qa.usable_cids.mask, 0, nr_cids);
 	cmask_init(&qa.to_revoke_cids.mask, 0, nr_cids);
 	cmask_init(&qa.to_grant_cids.mask, 0, nr_cids);
+	cmask_init(&qa.usable_scratch.mask, 0, nr_cids);
 	cmask_init(&qa.held_excl.mask, 0, nr_cids);
 	cmask_init(&qa.held_shared.mask, 0, nr_cids);
 
@@ -1854,14 +1880,16 @@ s32 BPF_STRUCT_OPS_SLEEPABLE(qmap_init)
 	}
 
 	/*
-	 * The root starts holding every cid. qmap_sub_ecaps_updated() maintains
-	 * per-cid shared state as effective caps settle, and redistribute()
-	 * rebuilds owner and self from held caps. A non-root node starts with
-	 * nothing.
+	 * The root starts holding every cid and gets no ecaps notifications, so
+	 * its avail set is fixed here. qmap_sub_ecaps_updated() maintains the
+	 * per-cid state as effective caps settle, and redistribute() rebuilds
+	 * owner and self from held caps. A non-root node starts with nothing.
 	 */
 	bpf_for(i, 0, nr_cids) {
 		if (!sub_cgroup_id) {
 			cmask_set(i, &qa.self_cids.mask);
+			cmask_set(i, &qa.avail_cids.mask);
+			cmask_set(i, &qa.usable_cids.mask);
 			qa.part.cid_owner[i] = CID_SELF;
 		} else {
 			qa.part.cid_owner[i] = CID_NONE;
@@ -2002,12 +2030,31 @@ void BPF_STRUCT_OPS(qmap_sub_ecaps_updated, s32 cid, u64 before, u64 after)
 {
 	/*
 	 * Effective caps updated. Track which cids hold shared caps so a self
-	 * task placed there enqueues IMMED.
+	 * task placed there enqueues IMMED, and which cids have ENQ_IMMED in
+	 * effect at all (avail, see the header comment).
 	 */
-	if (after & SCX_CAP_ENQ_IMMED)
+	if (after & SCX_CAP_ENQ_IMMED) {
 		qa.cid_shared[cid] = (after & SCX_CAP_ENQ) ? 0 : 1;
-	else
+		cmask_set(cid, &qa.avail_cids.mask);
+	} else {
 		qa.cid_shared[cid] = 0;
+		cmask_clear(cid, &qa.avail_cids.mask);
+	}
+
+	/*
+	 * When another runner holds the partition guard, set part_pending:
+	 * redistribute() drains it before releasing and rr_advance() checks it
+	 * after, so the deferred refresh lands by the next rr tick. A
+	 * repartition that lost the guard to us runs here.
+	 */
+	if (part_try_start()) {
+		refresh_usable();
+		part_end();
+		if (__sync_fetch_and_or(&part_pending, 0))
+			redistribute();
+	} else {
+		__sync_fetch_and_or(&part_pending, 1);
+	}
 }
 
 SCX_OPS_CID_DEFINE(qmap_ops,
diff --git a/tools/sched_ext/scx_qmap.h b/tools/sched_ext/scx_qmap.h
index c78d61806b39..e95fffcf7b23 100644
--- a/tools/sched_ext/scx_qmap.h
+++ b/tools/sched_ext/scx_qmap.h
@@ -165,12 +165,15 @@ struct qmap_arena {
 
 	/* bpf-internal cmasks (embedded, see struct qmap_cmask) */
 	struct qmap_cmask self_cids;	/* cids this node runs its own tasks on */
+	struct qmap_cmask avail_cids;	/* cids with caps in effect on the cpu */
+	struct qmap_cmask usable_cids;	/* self_cids & avail_cids, placeable right now */
 	struct qmap_cmask idle_cids;	/* idle state of all cids regardless of delegation */
 	struct qmap_cmask rr_cids;	/* the shared pool, as a mask for grant/revoke */
 
 	/* scratch cmasks */
 	struct qmap_cmask to_revoke_cids; /* delta cids to revoke */
 	struct qmap_cmask to_grant_cids; /* delta cids to grant */
+	struct qmap_cmask usable_scratch; /* refresh_usable() build area */
 	struct qmap_cmask prev_rr_cids; /* previous shared pool, to clear stale grants */
 	struct qmap_cmask held_excl;	/* cids held excl (ENQ): delegatable */
 	struct qmap_cmask held_shared;	/* cids held shared (ENQ_IMMED only): self-local */
-- 
2.55.0


  parent reply	other threads:[~2026-09-05 16:10 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-09-05 16:09 [PATCHSET sched_ext/for-7.3-fixes] sched_ext: Fix keep-last for sub-scheduler tasks and two scx_qmap placement loops Tejun Heo
2026-09-05 16:09 ` [PATCH sched_ext/for-7.3-fixes 1/4] sched_ext: Rename sch to root_sch in dispatch_one() Tejun Heo
2026-09-05 16:09 ` [PATCH sched_ext/for-7.3-fixes 2/4] sched_ext: Use @prev's scheduler for the keep decisions " Tejun Heo
2026-09-05 16:09 ` [PATCH sched_ext/for-7.3-fixes 3/4] sched_ext: scx_qmap: Do not add IMMED to rescue inserts Tejun Heo
2026-09-05 16:09 ` Tejun Heo [this message]
2026-09-05 16:22   ` [PATCH sched_ext/for-7.3-fixes 4/4] sched_ext: scx_qmap: Place only on cids whose caps are in effect sashiko-bot
2026-09-05 16:40     ` Tejun Heo
2026-09-05 19:24       ` Andrea Righi
2026-09-05 22:57         ` Tejun Heo
2026-09-05 19:25 ` [PATCHSET sched_ext/for-7.3-fixes] sched_ext: Fix keep-last for sub-scheduler tasks and two scx_qmap placement loops Andrea Righi
2026-09-05 22:25 ` Tejun Heo

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=20260905160958.1565156-5-tj@kernel.org \
    --to=tj@kernel.org \
    --cc=arighi@nvidia.com \
    --cc=changwoo@igalia.com \
    --cc=emil@etsalapatis.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=sched-ext@lists.linux.dev \
    --cc=void@manifault.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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.