[PATCHSET sched_ext/for-7.3] sched_ext: NMI-safe exit handling

[PATCHSET sched_ext/for-7.3] sched_ext: NMI-safe exit handling

[Tejun Heo]

Hello,

The exit paths weren't NMI-safe: exit claiming walked the sub-scheduler
hierarchy under scx_sched_lock and the bstr exit kfuncs formatted their
messages into a shared buffer under a raw spinlock. Kfuncs in the "any"
category are callable from tracing progs that can attach to functions
running in NMI, and many of them raise scx_error() on invalid inputs, so
an unlucky bad argument from such a prog could deadlock the machine. The
hardlockup handler had the same problem and deferred its abort to an
irq_work, which can't even run on the CPU that detected the lockup.

This series makes exit handling NMI-safe end to end:

 0001 makes exit claiming lock-free: ->aborting is asserted with a
      synchronous lockless sweep and the locked SCX_EXIT_PARENT
      propagation is deferred to an irq_work.

 0002 reverses the bstr exit sequence to claim-first so the message is
      formatted directly into the winner-owned exit_info buffer. With
      these two, scx_bpf_error() and scx_bpf_exit() are safe from any
      context including NMI.

 0003-0005 apply the newly-possible direct error reporting: NMI kicks
      abort the scheduler instead of being silently dropped, the
      hardlockup handler aborts directly from NMI making self-detected
      lockups recoverable, and scx_link_sched() reports failures inline.

 0001-sched_ext-Make-exit-claiming-lock-free.patch
 0002-sched_ext-Format-bstr-exit-messages-after-claiming-t.patch
 0003-sched_ext-Report-NMI-kicks-with-scx_error.patch
 0004-sched_ext-Abort-directly-from-the-hardlockup-handler.patch
 0005-sched_ext-Report-scx_link_sched-failures-inline.patch

Based on sched_ext/for-7.3 (3a21e34eb258).

The patches are also available in the following git branch:

 git://git.kernel.org/pub/scm/linux/kernel/git/tj/sched_ext.git scx-exit-nmi

diffstat follows. Thanks.

 kernel/sched/ext/ext.c      | 259 ++++++++++++++++++++++++--------------------
 kernel/sched/ext/internal.h |  10 +-
 kernel/sched/ext/sub.c      |  19 ++--
 3 files changed, 156 insertions(+), 132 deletions(-)

--
tejun

[PATCH 1/5] sched_ext: Make exit claiming lock-free

[Tejun Heo]

scx_claim_exit() claims descendants' exits by walking the subtree under
scx_sched_lock, making exit claiming, and thus scx_error(), unusable from
NMI and from under scx_sched_lock. However, kfuncs raising errors can run
from NMI-attached BPF progs, the hardlockup handler runs in NMI, and
scx_link_sched() wants to report failures under the lock.

The walk does two things with different urgencies: ->aborting must be
asserted synchronously to break IRQs-off dispatch-path live-locks, while the
descendants' exit_kind claims can happen later. Split them: sweep ->aborting
locklessly under RCU to unwedge the system and defer the locked
SCX_EXIT_PARENT walk to a new irq_work, both of which are NMI-safe.

The sweep stores each node's ->aborting and then reads its children list
while scx_link_sched() inserts and then checks the parent's ->aborting, the
two sides paired by full barriers - one side always sees the other. A link
that sees ->aborting undoes its insert and fails. As the undo's
list_del_rcu() leaves ->sibling non-empty, list_empty() can no longer
identify a never-linked sched during teardown - add sch->linked instead.

trace_sched_ext_exit can now fire from NMI. The exit backtrace is skipped
for NMI exits as stack_trace_save()'s NMI-safety is arch-dependent and
undocumented.

Signed-off-by: Tejun Heo <tj@kernel.org>
---
 kernel/sched/ext/ext.c      | 107 +++++++++++++++++++++++-------------
 kernel/sched/ext/internal.h |   2 +
 kernel/sched/ext/sub.c      |  12 ++--
 3 files changed, 77 insertions(+), 44 deletions(-)

diff --git a/kernel/sched/ext/ext.c b/kernel/sched/ext/ext.c
index aca8d2380509..30ce4c9428cf 100644
--- a/kernel/sched/ext/ext.c
+++ b/kernel/sched/ext/ext.c
@@ -5027,6 +5027,7 @@ static void scx_sched_free_rcu_work(struct work_struct *work)
 	struct scx_dispatch_q *dsq;
 	int cpu, node;
 
+	irq_work_sync(&sch->propagate_exit_irq_work);
 	irq_work_sync(&sch->disable_irq_work);
 	kthread_destroy_worker(sch->helper);
 	timer_shutdown_sync(&sch->bypass_lb_timer);
@@ -5993,18 +5994,6 @@ s32 scx_link_sched(struct scx_sched *sch)
 		struct scx_sched *parent = scx_parent(sch);
 
 		if (parent) {
-			/*
-			 * scx_claim_exit() propagates exit_kind transition to
-			 * its sub-scheds while holding scx_sched_lock - either
-			 * we can see the parent's non-NONE exit_kind or the
-			 * parent can shoot us down.
-			 */
-			if (atomic_read(&parent->exit_kind) != SCX_EXIT_NONE) {
-				err_msg = "parent disabled";
-				ret = -ENOENT;
-				break;
-			}
-
 			/*
 			 * Bypass state is spread across per-cpu flags and a
 			 * depth count, so inheriting it is tricky and has no
@@ -6024,6 +6013,23 @@ s32 scx_link_sched(struct scx_sched *sch)
 			}
 
 			list_add_tail_rcu(&sch->sibling, &parent->children);
+
+			/*
+			 * Pairs with the mb after the ->aborting assertion in
+			 * scx_claim_exit(). Either we see ->aborting and back
+			 * out, or the exit path sees us and exits us.
+			 */
+			smp_mb();
+			if (unlikely(READ_ONCE(parent->aborting))) {
+				rhashtable_remove_fast(&scx_sched_hash, &sch->hash_node,
+						       scx_sched_hash_params);
+				list_del_rcu(&sch->sibling);
+				err_msg = "parent disabled";
+				ret = -ENOENT;
+				break;
+			}
+
+			sch->linked = true;
 		}
 #endif	/* CONFIG_EXT_SUB_SCHED */
 
@@ -6047,10 +6053,11 @@ void scx_unlink_sched(struct scx_sched *sch)
 {
 	scoped_guard(raw_spinlock_irq, &scx_sched_lock) {
 #ifdef CONFIG_EXT_SUB_SCHED
-		if (scx_parent(sch)) {
+		if (sch->linked) {
 			rhashtable_remove_fast(&scx_sched_hash, &sch->hash_node,
 					       scx_sched_hash_params);
 			list_del_rcu(&sch->sibling);
+			sch->linked = false;
 		}
 #endif	/* CONFIG_EXT_SUB_SCHED */
 		list_del_rcu(&sch->all);
@@ -6260,12 +6267,36 @@ static void scx_root_disable(struct scx_sched *sch)
 	scx_bypass(sch, false);
 }
 
+/**
+ * scx_propagate_exit_irq_workfn - Claim SCX_EXIT_PARENT on the exiting subtree
+ * @irq_work: &scx_sched.propagate_exit_irq_work
+ *
+ * Queued by scx_claim_exit() after a non-PARENT claim. Claims SCX_EXIT_PARENT
+ * on each descendant, giving every one its own disable work - most of disabling
+ * is serialized but ops.exit() can take arbitrarily long and running them in
+ * separate helper kthreads parallelizes it. No recursion as only non-PARENT
+ * claims propagate.
+ */
+static void scx_propagate_exit_irq_workfn(struct irq_work *irq_work)
+{
+	struct scx_sched *sch = container_of(irq_work, struct scx_sched,
+					     propagate_exit_irq_work);
+	struct scx_sched *pos;
+
+	scoped_guard (raw_spinlock_irqsave, &scx_sched_lock) {
+		scx_for_each_descendant_pre(pos, sch)
+			scx_disable(pos, SCX_EXIT_PARENT);
+	}
+}
+
 /*
  * Claim the exit on @sch. The caller must ensure that the helper kthread work
  * is kicked before the current task can be preempted. Once exit_kind is
  * claimed, scx_error() can no longer trigger, so if the current task gets
  * preempted and the BPF scheduler fails to schedule it back, the helper work
  * will never be kicked and the whole system can wedge.
+ *
+ * Lock-free and safe to call from any context including NMI.
  */
 static bool scx_claim_exit(struct scx_sched *sch, enum scx_exit_kind kind)
 {
@@ -6279,35 +6310,28 @@ static bool scx_claim_exit(struct scx_sched *sch, enum scx_exit_kind kind)
 	if (!atomic_try_cmpxchg(&sch->exit_kind, &none, kind))
 		return false;
 
-	/*
-	 * Some CPUs may be trapped in the dispatch paths. Set the aborting
-	 * flag to break potential live-lock scenarios, ensuring we can
-	 * successfully reach scx_bypass().
-	 */
-	WRITE_ONCE(sch->aborting, true);
-
 	trace_sched_ext_exit(sch, kind);
 
-	/*
-	 * Propagate exits to descendants immediately. Each has a dedicated
-	 * helper kthread and can run in parallel. While most of disabling is
-	 * serialized, running them in separate threads allows parallelizing
-	 * ops.exit(), which can take arbitrarily long prolonging bypass mode.
-	 *
-	 * To guarantee forward progress, this propagation must be in-line so
-	 * that ->aborting is synchronously asserted for all sub-scheds. The
-	 * propagation is also the interlocking point against sub-sched
-	 * attachment. See scx_link_sched().
-	 *
-	 * This doesn't cause recursions as propagation only takes place for
-	 * non-propagation exits.
-	 */
-	if (kind != SCX_EXIT_PARENT) {
-		scoped_guard (raw_spinlock_irqsave, &scx_sched_lock) {
-			struct scx_sched *pos;
+	if (kind == SCX_EXIT_PARENT) {
+		/* an ancestor is already sweeping the subtree */
+		WRITE_ONCE(sch->aborting, true);
+	} else {
+		struct scx_sched *pos;
+
+		/*
+		 * CPUs may be live-locked in the dispatch paths of @sch or its
+		 * descendants, which ->aborting breaks. Sweep the subtree
+		 * locklessly so that this works from NMI. smp_store_mb() orders
+		 * each node's ->aborting store before its children are walked -
+		 * either we see a racing scx_link_sched() on ->children or it
+		 * sees ->aborting.
+		 */
+		scoped_guard (rcu) {
 			scx_for_each_descendant_pre(pos, sch)
-				scx_disable(pos, SCX_EXIT_PARENT);
+				smp_store_mb(pos->aborting, true);
 		}
+
+		irq_work_queue(&sch->propagate_exit_irq_work);
 	}
 
 	return true;
@@ -6738,7 +6762,11 @@ bool scx_vexit(struct scx_sched *sch,
 
 	ei->exit_code = exit_code;
 #ifdef CONFIG_STACKTRACE
-	if (kind >= SCX_EXIT_ERROR)
+	/*
+	 * stack_trace_save()'s NMI-safety is arch-dependent and undocumented.
+	 * Skip the backtrace when exiting from NMI.
+	 */
+	if (kind >= SCX_EXIT_ERROR && !in_nmi())
 		ei->bt_len = stack_trace_save(ei->bt, SCX_EXIT_BT_LEN, 1);
 #endif
 	vscnprintf(ei->msg, SCX_EXIT_MSG_LEN, fmt, args);
@@ -6908,6 +6936,7 @@ struct scx_sched *scx_alloc_and_add_sched(struct scx_enable_cmd *cmd,
 	sch->slice_dfl = SCX_SLICE_DFL;
 	atomic_set(&sch->exit_kind, SCX_EXIT_NONE);
 	sch->disable_irq_work = IRQ_WORK_INIT_HARD(scx_disable_irq_workfn);
+	sch->propagate_exit_irq_work = IRQ_WORK_INIT_HARD(scx_propagate_exit_irq_workfn);
 	kthread_init_work(&sch->disable_work, scx_disable_workfn);
 	timer_setup(&sch->bypass_lb_timer, scx_bypass_lb_timerfn, 0);
 
diff --git a/kernel/sched/ext/internal.h b/kernel/sched/ext/internal.h
index 886f1d132e6b..4b07f82bef40 100644
--- a/kernel/sched/ext/internal.h
+++ b/kernel/sched/ext/internal.h
@@ -1562,6 +1562,7 @@ struct scx_sched {
 	char			*cgrp_path;
 	struct kset		*sub_kset;
 
+	bool			linked;		/* on ->children, see scx_link_sched() */
 	bool			sub_attached;
 #endif	/* CONFIG_EXT_SUB_SCHED */
 
@@ -1580,6 +1581,7 @@ struct scx_sched {
 	struct kthread_worker	*helper;
 	struct irq_work		disable_irq_work;
 	struct kthread_work	disable_work;
+	struct irq_work		propagate_exit_irq_work; /* see scx_claim_exit() */
 	struct timer_list	bypass_lb_timer;
 	cpumask_var_t		bypass_lb_donee_cpumask;
 	cpumask_var_t		bypass_lb_resched_cpumask;
diff --git a/kernel/sched/ext/sub.c b/kernel/sched/ext/sub.c
index 6da6c91e4287..76ff58de54e7 100644
--- a/kernel/sched/ext/sub.c
+++ b/kernel/sched/ext/sub.c
@@ -1064,12 +1064,14 @@ void scx_sub_disable(struct scx_sched *sch)
 	scx_cgroup_lock();
 
 	/*
-	 * An enable that failed before scx_link_sched() never owned a cgroup or
-	 * task and won't be waited on by an ancestor's drain_descendants().
-	 * Nothing to reparent and walking the tasks can misbehave as the task
-	 * ownership invariant (either owned by self or parent) does not hold.
+	 * An enable that failed before scx_link_sched() succeeded never owned a
+	 * cgroup or task and won't be waited on by an ancestor's
+	 * drain_descendants(). Nothing to reparent and walking the tasks can
+	 * misbehave as the task ownership invariant (either owned by self or
+	 * parent) does not hold. ->sibling can't discriminate this - an undone
+	 * link leaves it non-empty.
 	 */
-	if (list_empty(&sch->sibling))
+	if (!sch->linked)
 		goto dump;
 
 	set_cgroup_sched(sch_cgroup(sch), parent);
-- 
2.55.0

[PATCH 2/5] sched_ext: Format bstr exit messages after claiming the exit

[Tejun Heo]

The bstr exit kfuncs format the message into a shared static buffer under a
raw spinlock before initiating the exit. The lock can't be taken from NMI
and needlessly serializes all bstr exits system-wide.

Now that exit claiming is lock-free, reverse the order: claim the exit first
and format directly into the exit_info message buffer which the claim winner
owns exclusively. The new scx_exit_bstr() implements the sequence, replacing
scx_bstr_format(), and the shared buffer and lock are deleted; the formatter
itself is what bpf_trace_printk() already runs from NMI. scx_prog_sched()
callers were relying on the lock for RCU protection, which is now provided
explicitly.

A malformed format no longer changes or fails the requested operation:
scx_bpf_exit_bstr() keeps its graceful exit kind and scx_bpf_sub_kill_bstr()
still kills the child, with a fallback message carrying the formatting
errno, while the sched that supplied the bad format is aborted for its bug.

Before this and the previous patch, an "any" category kfunc called from NMI
context could trigger scx_error() and deadlock - e.g. a tracing prog
attached to a function running in NMI calling scx_bpf_dsq_peek() on a
non-existent DSQ would try to grab scx_sched_lock, which may be held by the
interrupted CPU. This and the previous patch fix the deadlock: scx_error()
and the bstr exit kfuncs, and thus scx_bpf_error() and scx_bpf_exit(), are
now safe to call from any context including NMI.

Signed-off-by: Tejun Heo <tj@kernel.org>
---
 kernel/sched/ext/ext.c      | 90 ++++++++++++++++++++++++-------------
 kernel/sched/ext/internal.h |  7 ++-
 kernel/sched/ext/sub.c      |  7 +--
 3 files changed, 64 insertions(+), 40 deletions(-)

diff --git a/kernel/sched/ext/ext.c b/kernel/sched/ext/ext.c
index 30ce4c9428cf..29ca5f5ec30b 100644
--- a/kernel/sched/ext/ext.c
+++ b/kernel/sched/ext/ext.c
@@ -196,9 +196,6 @@ static const struct rhashtable_params dsq_hash_params = {
 
 static LLIST_HEAD(dsqs_to_free);
 
-DEFINE_RAW_SPINLOCK(scx_exit_bstr_buf_lock);
-struct scx_bstr_buf scx_exit_bstr_buf;
-
 /* ops debug dump */
 static DEFINE_RAW_SPINLOCK(scx_dump_lock);
 
@@ -6749,17 +6746,12 @@ static void scx_disable_irq_workfn(struct irq_work *irq_work)
 	kthread_queue_work(sch->helper, &sch->disable_work);
 }
 
-bool scx_vexit(struct scx_sched *sch,
-	       enum scx_exit_kind kind, s64 exit_code, s32 exit_cpu,
-	       const char *fmt, va_list args)
+/* finish exit_info and kick the disable work, ei->msg must already be set */
+static void scx_finish_exit(struct scx_sched *sch, enum scx_exit_kind kind,
+			    s64 exit_code, s32 exit_cpu)
 {
 	struct scx_exit_info *ei = sch->exit_info;
 
-	guard(preempt)();
-
-	if (!scx_claim_exit(sch, kind))
-		return false;
-
 	ei->exit_code = exit_code;
 #ifdef CONFIG_STACKTRACE
 	/*
@@ -6769,8 +6761,6 @@ bool scx_vexit(struct scx_sched *sch,
 	if (kind >= SCX_EXIT_ERROR && !in_nmi())
 		ei->bt_len = stack_trace_save(ei->bt, SCX_EXIT_BT_LEN, 1);
 #endif
-	vscnprintf(ei->msg, SCX_EXIT_MSG_LEN, fmt, args);
-
 	/*
 	 * Set ei->kind and ->reason for scx_dump_state(). They'll be set again
 	 * in scx_disable_workfn().
@@ -6780,6 +6770,22 @@ bool scx_vexit(struct scx_sched *sch,
 	ei->exit_cpu = exit_cpu;
 
 	irq_work_queue(&sch->disable_irq_work);
+}
+
+bool scx_vexit(struct scx_sched *sch,
+	       enum scx_exit_kind kind, s64 exit_code, s32 exit_cpu,
+	       const char *fmt, va_list args)
+{
+	struct scx_exit_info *ei = sch->exit_info;
+
+	guard(preempt)();
+
+	if (!scx_claim_exit(sch, kind))
+		return false;
+
+	vscnprintf(ei->msg, SCX_EXIT_MSG_LEN, fmt, args);
+
+	scx_finish_exit(sch, kind, exit_code, exit_cpu);
 	return true;
 }
 
@@ -9593,12 +9599,38 @@ static s32 __bstr_format(struct scx_sched *sch, u64 *data_buf, char *line_buf,
 	return ret;
 }
 
-__printf(3, 0)
-s32 scx_bstr_format(struct scx_sched *sch, struct scx_bstr_buf *buf,
-		    char *fmt, unsigned long long *data, u32 data__sz)
+/*
+ * Exit @sch with the reason formatted from a BPF-supplied bstr format. The exit
+ * is claimed first and the reason is formatted directly into the winner-owned
+ * exit_info buffer, which allows use from any context including NMI.
+ *
+ * @fmt_blame is the sched blamed for formatting failures through the
+ * scx_error() calls in __bstr_format() and differs from @sch when a parent
+ * supplies the kill reason for a child. A formatting failure doesn't revert the
+ * claim - @sch still exits with the claimed kind and a fallback message.
+ */
+__printf(5, 0)
+bool scx_exit_bstr(struct scx_sched *sch, enum scx_exit_kind kind,
+		   s64 exit_code, struct scx_sched *fmt_blame, char *fmt,
+		   unsigned long long *data, u32 data__sz)
 {
-	return __bstr_format(sch, buf->data, buf->line, sizeof(buf->line),
-			     fmt, data, data__sz);
+	struct scx_exit_info *ei = sch->exit_info;
+	u64 data_buf[MAX_BPRINTF_VARARGS];
+	s32 ret;
+
+	guard(preempt)();
+
+	if (!scx_claim_exit(sch, kind))
+		return false;
+
+	ret = __bstr_format(fmt_blame, data_buf, ei->msg, SCX_EXIT_MSG_LEN,
+			    fmt, data, data__sz);
+	if (ret < 0)
+		scnprintf(ei->msg, SCX_EXIT_MSG_LEN,
+			  "exit message formatting failed (%d)", ret);
+
+	scx_finish_exit(sch, kind, exit_code, raw_smp_processor_id());
+	return true;
 }
 
 __bpf_kfunc_start_defs();
@@ -9620,14 +9652,13 @@ __bpf_kfunc void scx_bpf_exit_bstr(s64 exit_code, char *fmt,
 				   const struct bpf_prog_aux *aux)
 {
 	struct scx_sched *sch;
-	unsigned long flags;
 
-	raw_spin_lock_irqsave(&scx_exit_bstr_buf_lock, flags);
+	guard(rcu)();
+
 	sch = scx_prog_sched(aux);
-	if (likely(sch) &&
-	    scx_bstr_format(sch, &scx_exit_bstr_buf, fmt, data, data__sz) >= 0)
-		scx_exit(sch, SCX_EXIT_UNREG_BPF, exit_code, "%s", scx_exit_bstr_buf.line);
-	raw_spin_unlock_irqrestore(&scx_exit_bstr_buf_lock, flags);
+	if (likely(sch))
+		scx_exit_bstr(sch, SCX_EXIT_UNREG_BPF, exit_code, sch, fmt,
+			      data, data__sz);
 }
 
 /**
@@ -9645,14 +9676,13 @@ __bpf_kfunc void scx_bpf_error_bstr(char *fmt, unsigned long long *data,
 				    u32 data__sz, const struct bpf_prog_aux *aux)
 {
 	struct scx_sched *sch;
-	unsigned long flags;
 
-	raw_spin_lock_irqsave(&scx_exit_bstr_buf_lock, flags);
+	guard(rcu)();
+
 	sch = scx_prog_sched(aux);
-	if (likely(sch) &&
-	    scx_bstr_format(sch, &scx_exit_bstr_buf, fmt, data, data__sz) >= 0)
-		scx_exit(sch, SCX_EXIT_ERROR_BPF, 0, "%s", scx_exit_bstr_buf.line);
-	raw_spin_unlock_irqrestore(&scx_exit_bstr_buf_lock, flags);
+	if (likely(sch))
+		scx_exit_bstr(sch, SCX_EXIT_ERROR_BPF, 0, sch, fmt, data,
+			      data__sz);
 }
 
 /**
diff --git a/kernel/sched/ext/internal.h b/kernel/sched/ext/internal.h
index 4b07f82bef40..a0659f74b065 100644
--- a/kernel/sched/ext/internal.h
+++ b/kernel/sched/ext/internal.h
@@ -2007,15 +2007,14 @@ struct scx_sched *scx_alloc_and_add_sched(struct scx_enable_cmd *cmd,
 int scx_validate_ops(struct scx_sched *sch, const struct sched_ext_ops *ops);
 int scx_sched_sysfs_add(struct scx_sched *sch);
 bool scx_is_descendant(struct scx_sched *sch, struct scx_sched *ancestor);
-__printf(3, 0) s32 scx_bstr_format(struct scx_sched *sch, struct scx_bstr_buf *buf,
-				   char *fmt, unsigned long long *data, u32 data__sz);
+__printf(5, 0) bool scx_exit_bstr(struct scx_sched *sch, enum scx_exit_kind kind,
+				  s64 exit_code, struct scx_sched *fmt_blame,
+				  char *fmt, unsigned long long *data, u32 data__sz);
 
 extern raw_spinlock_t scx_sched_lock;
 extern struct mutex scx_enable_mutex;
 extern struct percpu_rw_semaphore scx_fork_rwsem;
 extern bool scx_cgroup_enabled;
-extern raw_spinlock_t scx_exit_bstr_buf_lock;
-extern struct scx_bstr_buf scx_exit_bstr_buf;
 #ifdef CONFIG_EXT_SUB_SCHED
 extern const struct rhashtable_params scx_sched_hash_params;
 extern struct rhashtable scx_sched_hash;
diff --git a/kernel/sched/ext/sub.c b/kernel/sched/ext/sub.c
index 76ff58de54e7..6b3f3cddf62c 100644
--- a/kernel/sched/ext/sub.c
+++ b/kernel/sched/ext/sub.c
@@ -2150,7 +2150,6 @@ __bpf_kfunc s32 scx_bpf_sub_kill_bstr(u64 cgroup_id, char *fmt,
 				      const struct bpf_prog_aux *aux)
 {
 	struct scx_sched *parent, *child;
-	s32 ret;
 
 	guard(rcu)();
 
@@ -2173,11 +2172,7 @@ __bpf_kfunc s32 scx_bpf_sub_kill_bstr(u64 cgroup_id, char *fmt,
 		return -EINVAL;
 	}
 
-	guard(raw_spinlock_irqsave)(&scx_exit_bstr_buf_lock);
-	ret = scx_bstr_format(parent, &scx_exit_bstr_buf, fmt, data, data__sz);
-	if (ret < 0)
-		return ret;
-	scx_exit(child, SCX_EXIT_PARENT_KILL, 0, "%s", scx_exit_bstr_buf.line);
+	scx_exit_bstr(child, SCX_EXIT_PARENT_KILL, 0, parent, fmt, data, data__sz);
 	return 0;
 }
 
-- 
2.55.0

[PATCH 3/5] sched_ext: Report NMI kicks with scx_error()

[Tejun Heo]

The per-cpu kick lists are protected by IRQ masking which doesn't stop NMIs,
so scx_bpf_kick_cpu() from NMI silently drops the kick after a one-time
warning. A dropped kick can leave a CPU idle when the scheduler believes it
was woken, which is a correctness problem for the scheduler even if the
kernel is fine. Now that scx_error() works from NMI, abort the scheduler
instead so that the bug is surfaced deterministically. The warned_nmi_kick
tracking is no longer needed.

Signed-off-by: Tejun Heo <tj@kernel.org>
---
 kernel/sched/ext/ext.c      | 6 +-----
 kernel/sched/ext/internal.h | 1 -
 2 files changed, 1 insertion(+), 6 deletions(-)

diff --git a/kernel/sched/ext/ext.c b/kernel/sched/ext/ext.c
index 29ca5f5ec30b..4338dc6a3f3a 100644
--- a/kernel/sched/ext/ext.c
+++ b/kernel/sched/ext/ext.c
@@ -9208,11 +9208,7 @@ void scx_kick_cpu(struct scx_sched *sch, s32 cpu, u64 flags)
 	 * not mask NMIs, so kicking from NMI could corrupt it and is unsupported.
 	 */
 	if (unlikely(in_nmi())) {
-		if (!sch->warned_nmi_kick) {
-			sch->warned_nmi_kick = true;
-			pr_warn("sched_ext: %s: scx_bpf_kick_cpu() from NMI ignored\n",
-				sch->ops.name);
-		}
+		scx_error(sch, "scx_bpf_kick_cpu() called from NMI");
 		return;
 	}
 
diff --git a/kernel/sched/ext/internal.h b/kernel/sched/ext/internal.h
index a0659f74b065..b141f0104b44 100644
--- a/kernel/sched/ext/internal.h
+++ b/kernel/sched/ext/internal.h
@@ -1546,7 +1546,6 @@ struct scx_sched {
 	 */
 	bool			warned_zero_slice:1;
 	bool			warned_unassoc_progs:1;
-	bool			warned_nmi_kick:1;
 
 	struct list_head	all;
 
-- 
2.55.0

[PATCH 4/5] sched_ext: Abort directly from the hardlockup handler

[Tejun Heo]

scx_hardlockup() defers the abort to an irq_work because exit claiming used
to take scx_sched_lock and couldn't run from NMI. The deferral is now
unnecessary - claiming is NMI-safe and asserting ->aborting is exactly what
breaks the live-locks that hard-lock CPUs. Call handle_lockup() directly and
drop the irq_work. This also makes the self-detected case recoverable: the
perf watchdog fires on the hard-locked CPU itself, where a queued irq_work
never runs with IRQs off.

Also fix the return value: %true used to be returned whenever sched_ext was
loaded, suppressing the kernel's hardlockup report even when the abort was
refused. Return %true only when this call initiated the abort.

Fixes: bd2d76455b65 ("sched_ext: Defer scx_hardlockup() out of NMI")
Signed-off-by: Tejun Heo <tj@kernel.org>
---
 kernel/sched/ext/ext.c | 35 +++++++++--------------------------
 1 file changed, 9 insertions(+), 26 deletions(-)

diff --git a/kernel/sched/ext/ext.c b/kernel/sched/ext/ext.c
index 4338dc6a3f3a..9f5729c2c5e2 100644
--- a/kernel/sched/ext/ext.c
+++ b/kernel/sched/ext/ext.c
@@ -5382,25 +5382,6 @@ void scx_softlockup(u32 dur_s)
 			cpu, dur_s);
 }
 
-/*
- * scx_hardlockup() runs from NMI and eventually calls scx_claim_exit(),
- * which takes scx_sched_lock. scx_sched_lock isn't NMI-safe and grabbing
- * it from NMI context can lead to deadlocks. Defer via irq_work; the
- * disable path runs off irq_work anyway.
- */
-static atomic_t scx_hardlockup_cpu = ATOMIC_INIT(-1);
-
-static void scx_hardlockup_irq_workfn(struct irq_work *work)
-{
-	int cpu = atomic_xchg(&scx_hardlockup_cpu, -1);
-
-	if (cpu >= 0 && handle_lockup(cpu, "hard lockup - CPU %d", cpu))
-		printk_deferred(KERN_ERR "sched_ext: Hard lockup - CPU %d, disabling BPF scheduler\n",
-				cpu);
-}
-
-static DEFINE_IRQ_WORK(scx_hardlockup_irq_work, scx_hardlockup_irq_workfn);
-
 /**
  * scx_hardlockup - sched_ext hardlockup handler
  * @cpu: the target CPU
@@ -5410,19 +5391,21 @@ static DEFINE_IRQ_WORK(scx_hardlockup_irq_work, scx_hardlockup_irq_workfn);
  * Try kicking out the current scheduler in an attempt to recover the system to
  * a good state before taking more drastic actions.
  *
- * Queues an irq_work; the handle_lockup() call happens in IRQ context (see
- * scx_hardlockup_irq_workfn).
+ * Called from NMI. Aborting the scheduler sets ->aborting throughout the
+ * hierarchy before returning, which is what breaks the dispatch-path live-locks
+ * that can hard-lock CPUs.
  *
- * Returns %true if sched_ext is enabled and the work was queued, %false
- * otherwise.
+ * Returns %true if sched_ext is enabled and abort was initiated, which may
+ * resolve the lockup. %false if sched_ext is not enabled or abort was already
+ * initiated by someone else.
  */
 bool scx_hardlockup(int cpu)
 {
-	if (!rcu_access_pointer(scx_root))
+	if (!handle_lockup(cpu, "hard lockup - CPU %d", cpu))
 		return false;
 
-	atomic_cmpxchg(&scx_hardlockup_cpu, -1, cpu);
-	irq_work_queue(&scx_hardlockup_irq_work);
+	printk_deferred(KERN_ERR "sched_ext: Hard lockup - CPU %d, disabling BPF scheduler\n",
+			cpu);
 	return true;
 }
 
-- 
2.55.0

[PATCH 5/5] sched_ext: Report scx_link_sched() failures inline

[Tejun Heo]

scx_link_sched() carries each failure out of the locked section through
err_msg and ret because scx_error() used to take scx_sched_lock and couldn't
be called under it. That restriction is gone, so report each failure at the
site it's detected and return directly. The scx_error() here claims the exit
on the sched being linked, which has no descendants yet, and the locked
propagation walk is deferred, so nothing reacquires scx_sched_lock inline.

Signed-off-by: Tejun Heo <tj@kernel.org>
---
 kernel/sched/ext/ext.c | 29 +++++++++--------------------
 1 file changed, 9 insertions(+), 20 deletions(-)

diff --git a/kernel/sched/ext/ext.c b/kernel/sched/ext/ext.c
index 9f5729c2c5e2..6ed9818491f3 100644
--- a/kernel/sched/ext/ext.c
+++ b/kernel/sched/ext/ext.c
@@ -5965,31 +5965,30 @@ static void refresh_watchdog(void)
 
 s32 scx_link_sched(struct scx_sched *sch)
 {
-	const char *err_msg = "";
-	s32 ret = 0;
-
 	scoped_guard(raw_spinlock_irqsave, &scx_bypass_lock)	/* for the parent bypass check */
 	scoped_guard(raw_spinlock, &scx_sched_lock) {
 #ifdef CONFIG_EXT_SUB_SCHED
 		struct scx_sched *parent = scx_parent(sch);
 
 		if (parent) {
+			s32 ret;
+
 			/*
 			 * Bypass state is spread across per-cpu flags and a
 			 * depth count, so inheriting it is tricky and has no
 			 * valid use case. Refuse it.
 			 */
 			if (READ_ONCE(parent->bypass_depth)) {
-				err_msg = "parent bypassing";
-				ret = -EBUSY;
-				break;
+				scx_error(sch, "parent bypassing (%d)", -EBUSY);
+				return -EBUSY;
 			}
 
 			ret = rhashtable_lookup_insert_fast(&scx_sched_hash,
 					&sch->hash_node, scx_sched_hash_params);
 			if (ret) {
-				err_msg = "failed to insert into scx_sched_hash";
-				break;
+				scx_error(sch, "failed to insert into scx_sched_hash (%d)",
+					  ret);
+				return ret;
 			}
 
 			list_add_tail_rcu(&sch->sibling, &parent->children);
@@ -6004,9 +6003,8 @@ s32 scx_link_sched(struct scx_sched *sch)
 				rhashtable_remove_fast(&scx_sched_hash, &sch->hash_node,
 						       scx_sched_hash_params);
 				list_del_rcu(&sch->sibling);
-				err_msg = "parent disabled";
-				ret = -ENOENT;
-				break;
+				scx_error(sch, "parent disabled (%d)", -ENOENT);
+				return -ENOENT;
 			}
 
 			sch->linked = true;
@@ -6016,15 +6014,6 @@ s32 scx_link_sched(struct scx_sched *sch)
 		list_add_tail_rcu(&sch->all, &scx_sched_all);
 	}
 
-	/*
-	 * scx_error() takes scx_sched_lock via scx_claim_exit(), so it must run after
-	 * the guard above is released.
-	 */
-	if (ret) {
-		scx_error(sch, "%s (%d)", err_msg, ret);
-		return ret;
-	}
-
 	refresh_watchdog();
 	return 0;
 }
-- 
2.55.0