[<prev] [next>] [thread-next>] [day] [month] [year] [list]
Message-ID: <ZyqSm4B4NuzuHEbp@slm.duckdns.org>
Date: Tue, 5 Nov 2024 11:48:11 -1000
From: Tejun Heo <tj@...nel.org>
To: David Vernet <void@...ifault.com>
Cc: linux-kernel@...r.kernel.org, kernel-team@...a.com, sched-ext@...a.com,
Andrea Righi <arighi@...dia.com>,
Changwoo Min <multics69@...il.com>
Subject: [PATCH sched_ext/for-6.13 1/2] sched_ext: Avoid live-locking bypass
mode switching
A poorly behaving BPF scheduler can live-lock the system by e.g. incessantly
banging on the same DSQ on a large NUMA system to the point where switching
to the bypass mode can take a long time. Turning on the bypass mode requires
dequeueing and re-enqueueing currently runnable tasks, if the DSQs that they
are on are live-locked, this can take tens of seconds cascading into other
failures. This was observed on 2 x Intel Sapphire Rapids machines with 224
logical CPUs.
Inject artifical delays while the bypass mode is switching to guarantee
timely completion.
While at it, move __scx_ops_bypass_lock into scx_ops_bypass() and rename it
to bypass_lock.
Signed-off-by: Tejun Heo <tj@...nel.org>
Reported-by: Valentin Andrei <vandrei@...a.com>
Reported-by: Patrick Lu <patlu@...a.com>
---
kernel/sched/ext.c | 55 ++++++++++++++++++++++++++++++++++++++++++++++++++---
1 file changed, 52 insertions(+), 3 deletions(-)
--- a/kernel/sched/ext.c
+++ b/kernel/sched/ext.c
@@ -867,8 +867,8 @@ static DEFINE_MUTEX(scx_ops_enable_mutex
DEFINE_STATIC_KEY_FALSE(__scx_ops_enabled);
DEFINE_STATIC_PERCPU_RWSEM(scx_fork_rwsem);
static atomic_t scx_ops_enable_state_var = ATOMIC_INIT(SCX_OPS_DISABLED);
+static atomic_t scx_ops_breather_depth = ATOMIC_INIT(0);
static int scx_ops_bypass_depth;
-static DEFINE_RAW_SPINLOCK(__scx_ops_bypass_lock);
static bool scx_ops_init_task_enabled;
static bool scx_switching_all;
DEFINE_STATIC_KEY_FALSE(__scx_switched_all);
@@ -2474,11 +2474,48 @@ static struct rq *move_task_between_dsqs
return dst_rq;
}
+/*
+ * A poorly behaving BPF scheduler can live-lock the system by e.g. incessantly
+ * banging on the same DSQ on a large NUMA system to the point where switching
+ * to the bypass mode can take a long time. Inject artifical delays while the
+ * bypass mode is switching to guarantee timely completion.
+ */
+static void scx_ops_breather(struct rq *rq)
+{
+ u64 until;
+
+ lockdep_assert_rq_held(rq);
+
+ if (likely(!atomic_read(&scx_ops_breather_depth)))
+ return;
+
+ raw_spin_rq_unlock(rq);
+
+ until = ktime_get_ns() + NSEC_PER_MSEC;
+
+ do {
+ int cnt = 1024;
+ while (atomic_read(&scx_ops_breather_depth) && --cnt)
+ cpu_relax();
+ } while (atomic_read(&scx_ops_breather_depth) &&
+ time_before64(ktime_get_ns(), until));
+
+ raw_spin_rq_lock(rq);
+}
+
static bool consume_dispatch_q(struct rq *rq, struct scx_dispatch_q *dsq)
{
struct task_struct *p;
retry:
/*
+ * This retry loop can repeatedly race against scx_ops_bypass()
+ * dequeueing tasks from @dsq trying to put the system into the bypass
+ * mode. On some multi-socket machines (e.g. 2x Intel 8480c), this can
+ * live-lock the machine into soft lockups. Give a breather.
+ */
+ scx_ops_breather(rq);
+
+ /*
* The caller can't expect to successfully consume a task if the task's
* addition to @dsq isn't guaranteed to be visible somehow. Test
* @dsq->list without locking and skip if it seems empty.
@@ -4550,10 +4587,11 @@ bool task_should_scx(struct task_struct
*/
static void scx_ops_bypass(bool bypass)
{
+ static DEFINE_RAW_SPINLOCK(bypass_lock);
int cpu;
unsigned long flags;
- raw_spin_lock_irqsave(&__scx_ops_bypass_lock, flags);
+ raw_spin_lock_irqsave(&bypass_lock, flags);
if (bypass) {
scx_ops_bypass_depth++;
WARN_ON_ONCE(scx_ops_bypass_depth <= 0);
@@ -4566,6 +4604,8 @@ static void scx_ops_bypass(bool bypass)
goto unlock;
}
+ atomic_inc(&scx_ops_breather_depth);
+
/*
* No task property is changing. We just need to make sure all currently
* queued tasks are re-queued according to the new scx_rq_bypassing()
@@ -4621,8 +4661,10 @@ static void scx_ops_bypass(bool bypass)
/* resched to restore ticks and idle state */
resched_cpu(cpu);
}
+
+ atomic_dec(&scx_ops_breather_depth);
unlock:
- raw_spin_unlock_irqrestore(&__scx_ops_bypass_lock, flags);
+ raw_spin_unlock_irqrestore(&bypass_lock, flags);
}
static void free_exit_info(struct scx_exit_info *ei)
@@ -6275,6 +6317,13 @@ static bool scx_dispatch_from_dsq(struct
raw_spin_rq_lock(src_rq);
}
+ /*
+ * If the BPF scheduler keeps calling this function repeatedly, it can
+ * cause similar live-lock conditions as consume_dispatch_q(). Insert a
+ * breather if necessary.
+ */
+ scx_ops_breather(src_rq);
+
locked_rq = src_rq;
raw_spin_lock(&src_dsq->lock);
Powered by blists - more mailing lists