Skip to content

Commit 4d3ca89

Browse files
committed
sched_ext: Refactor consume_remote_task()
The tricky p->scx.holding_cpu handling was split across consume_remote_task() body and move_task_to_local_dsq(). Refactor such that: - All the tricky part is now in the new unlink_dsq_and_lock_src_rq() with consolidated documentation. - move_task_to_local_dsq() now implements straightforward task migration making it easier to use in other places. - dispatch_to_local_dsq() is another user move_task_to_local_dsq(). The usage is updated accordingly. This makes the local and remote cases more symmetric. No functional changes intended. v2: s/task_rq/src_rq/ for consistency. Signed-off-by: Tejun Heo <[email protected]> Acked-by: David Vernet <[email protected]>
1 parent fdaedba commit 4d3ca89

File tree

1 file changed

+76
-69
lines changed

1 file changed

+76
-69
lines changed

kernel/sched/ext.c

Lines changed: 76 additions & 69 deletions
Original file line numberDiff line numberDiff line change
@@ -2178,49 +2178,13 @@ static bool yield_to_task_scx(struct rq *rq, struct task_struct *to)
21782178
* @src_rq: rq to move the task from, locked on entry, released on return
21792179
* @dst_rq: rq to move the task into, locked on return
21802180
*
2181-
* Move @p which is currently on @src_rq to @dst_rq's local DSQ. The caller
2182-
* must:
2183-
*
2184-
* 1. Start with exclusive access to @p either through its DSQ lock or
2185-
* %SCX_OPSS_DISPATCHING flag.
2186-
*
2187-
* 2. Set @p->scx.holding_cpu to raw_smp_processor_id().
2188-
*
2189-
* 3. Remember task_rq(@p) as @src_rq. Release the exclusive access so that we
2190-
* don't deadlock with dequeue.
2191-
*
2192-
* 4. Lock @src_rq from #3.
2193-
*
2194-
* 5. Call this function.
2195-
*
2196-
* Returns %true if @p was successfully moved. %false after racing dequeue and
2197-
* losing. On return, @src_rq is unlocked and @dst_rq is locked.
2181+
* Move @p which is currently on @src_rq to @dst_rq's local DSQ.
21982182
*/
2199-
static bool move_task_to_local_dsq(struct task_struct *p, u64 enq_flags,
2183+
static void move_task_to_local_dsq(struct task_struct *p, u64 enq_flags,
22002184
struct rq *src_rq, struct rq *dst_rq)
22012185
{
22022186
lockdep_assert_rq_held(src_rq);
22032187

2204-
/*
2205-
* If dequeue got to @p while we were trying to lock @src_rq, it'd have
2206-
* cleared @p->scx.holding_cpu to -1. While other cpus may have updated
2207-
* it to different values afterwards, as this operation can't be
2208-
* preempted or recurse, @p->scx.holding_cpu can never become
2209-
* raw_smp_processor_id() again before we're done. Thus, we can tell
2210-
* whether we lost to dequeue by testing whether @p->scx.holding_cpu is
2211-
* still raw_smp_processor_id().
2212-
*
2213-
* @p->rq couldn't have changed if we're still the holding cpu.
2214-
*
2215-
* See dispatch_dequeue() for the counterpart.
2216-
*/
2217-
if (unlikely(p->scx.holding_cpu != raw_smp_processor_id()) ||
2218-
WARN_ON_ONCE(src_rq != task_rq(p))) {
2219-
raw_spin_rq_unlock(src_rq);
2220-
raw_spin_rq_lock(dst_rq);
2221-
return false;
2222-
}
2223-
22242188
/* the following marks @p MIGRATING which excludes dequeue */
22252189
deactivate_task(src_rq, p, 0);
22262190
set_task_cpu(p, cpu_of(dst_rq));
@@ -2239,8 +2203,6 @@ static bool move_task_to_local_dsq(struct task_struct *p, u64 enq_flags,
22392203
dst_rq->scx.extra_enq_flags = enq_flags;
22402204
activate_task(dst_rq, p, 0);
22412205
dst_rq->scx.extra_enq_flags = 0;
2242-
2243-
return true;
22442206
}
22452207

22462208
#endif /* CONFIG_SMP */
@@ -2305,28 +2267,69 @@ static bool task_can_run_on_remote_rq(struct task_struct *p, struct rq *rq,
23052267
return true;
23062268
}
23072269

2308-
static bool consume_remote_task(struct rq *rq, struct scx_dispatch_q *dsq,
2309-
struct task_struct *p, struct rq *task_rq)
2270+
/**
2271+
* unlink_dsq_and_lock_src_rq() - Unlink task from its DSQ and lock its task_rq
2272+
* @p: target task
2273+
* @dsq: locked DSQ @p is currently on
2274+
* @src_rq: rq @p is currently on, stable with @dsq locked
2275+
*
2276+
* Called with @dsq locked but no rq's locked. We want to move @p to a different
2277+
* DSQ, including any local DSQ, but are not locking @src_rq. Locking @src_rq is
2278+
* required when transferring into a local DSQ. Even when transferring into a
2279+
* non-local DSQ, it's better to use the same mechanism to protect against
2280+
* dequeues and maintain the invariant that @p->scx.dsq can only change while
2281+
* @src_rq is locked, which e.g. scx_dump_task() depends on.
2282+
*
2283+
* We want to grab @src_rq but that can deadlock if we try while locking @dsq,
2284+
* so we want to unlink @p from @dsq, drop its lock and then lock @src_rq. As
2285+
* this may race with dequeue, which can't drop the rq lock or fail, do a little
2286+
* dancing from our side.
2287+
*
2288+
* @p->scx.holding_cpu is set to this CPU before @dsq is unlocked. If @p gets
2289+
* dequeued after we unlock @dsq but before locking @src_rq, the holding_cpu
2290+
* would be cleared to -1. While other cpus may have updated it to different
2291+
* values afterwards, as this operation can't be preempted or recurse, the
2292+
* holding_cpu can never become this CPU again before we're done. Thus, we can
2293+
* tell whether we lost to dequeue by testing whether the holding_cpu still
2294+
* points to this CPU. See dispatch_dequeue() for the counterpart.
2295+
*
2296+
* On return, @dsq is unlocked and @src_rq is locked. Returns %true if @p is
2297+
* still valid. %false if lost to dequeue.
2298+
*/
2299+
static bool unlink_dsq_and_lock_src_rq(struct task_struct *p,
2300+
struct scx_dispatch_q *dsq,
2301+
struct rq *src_rq)
23102302
{
2311-
lockdep_assert_held(&dsq->lock); /* released on return */
2303+
s32 cpu = raw_smp_processor_id();
2304+
2305+
lockdep_assert_held(&dsq->lock);
23122306

2313-
/*
2314-
* @dsq is locked and @p is on a remote rq. @p is currently protected by
2315-
* @dsq->lock. We want to pull @p to @rq but may deadlock if we grab
2316-
* @task_rq while holding @dsq and @rq locks. As dequeue can't drop the
2317-
* rq lock or fail, do a little dancing from our side. See
2318-
* move_task_to_local_dsq().
2319-
*/
23202307
WARN_ON_ONCE(p->scx.holding_cpu >= 0);
23212308
task_unlink_from_dsq(p, dsq);
23222309
dsq_mod_nr(dsq, -1);
2323-
p->scx.holding_cpu = raw_smp_processor_id();
2310+
p->scx.holding_cpu = cpu;
2311+
23242312
raw_spin_unlock(&dsq->lock);
2313+
raw_spin_rq_lock(src_rq);
23252314

2326-
raw_spin_rq_unlock(rq);
2327-
raw_spin_rq_lock(task_rq);
2315+
/* task_rq couldn't have changed if we're still the holding cpu */
2316+
return likely(p->scx.holding_cpu == cpu) &&
2317+
!WARN_ON_ONCE(src_rq != task_rq(p));
2318+
}
23282319

2329-
return move_task_to_local_dsq(p, 0, task_rq, rq);
2320+
static bool consume_remote_task(struct rq *this_rq, struct scx_dispatch_q *dsq,
2321+
struct task_struct *p, struct rq *src_rq)
2322+
{
2323+
raw_spin_rq_unlock(this_rq);
2324+
2325+
if (unlink_dsq_and_lock_src_rq(p, dsq, src_rq)) {
2326+
move_task_to_local_dsq(p, 0, src_rq, this_rq);
2327+
return true;
2328+
} else {
2329+
raw_spin_rq_unlock(src_rq);
2330+
raw_spin_rq_lock(this_rq);
2331+
return false;
2332+
}
23302333
}
23312334
#else /* CONFIG_SMP */
23322335
static inline bool task_can_run_on_remote_rq(struct task_struct *p, struct rq *rq, bool trigger_error) { return false; }
@@ -2430,7 +2433,8 @@ dispatch_to_local_dsq(struct rq *rq, u64 dsq_id, struct task_struct *p,
24302433
* As DISPATCHING guarantees that @p is wholly ours, we can
24312434
* pretend that we're moving from a DSQ and use the same
24322435
* mechanism - mark the task under transfer with holding_cpu,
2433-
* release DISPATCHING and then follow the same protocol.
2436+
* release DISPATCHING and then follow the same protocol. See
2437+
* unlink_dsq_and_lock_src_rq().
24342438
*/
24352439
p->scx.holding_cpu = raw_smp_processor_id();
24362440

@@ -2443,28 +2447,31 @@ dispatch_to_local_dsq(struct rq *rq, u64 dsq_id, struct task_struct *p,
24432447
raw_spin_rq_lock(src_rq);
24442448
}
24452449

2446-
if (src_rq == dst_rq) {
2450+
/* task_rq couldn't have changed if we're still the holding cpu */
2451+
dsp = p->scx.holding_cpu == raw_smp_processor_id() &&
2452+
!WARN_ON_ONCE(src_rq != task_rq(p));
2453+
2454+
if (likely(dsp)) {
24472455
/*
2448-
* As @p is staying on the same rq, there's no need to
2456+
* If @p is staying on the same rq, there's no need to
24492457
* go through the full deactivate/activate cycle.
24502458
* Optimize by abbreviating the operations in
24512459
* move_task_to_local_dsq().
24522460
*/
2453-
dsp = p->scx.holding_cpu == raw_smp_processor_id();
2454-
if (likely(dsp)) {
2461+
if (src_rq == dst_rq) {
24552462
p->scx.holding_cpu = -1;
2456-
dispatch_enqueue(&dst_rq->scx.local_dsq, p,
2457-
enq_flags);
2463+
dispatch_enqueue(&dst_rq->scx.local_dsq,
2464+
p, enq_flags);
2465+
} else {
2466+
move_task_to_local_dsq(p, enq_flags,
2467+
src_rq, dst_rq);
24582468
}
2459-
} else {
2460-
dsp = move_task_to_local_dsq(p, enq_flags,
2461-
src_rq, dst_rq);
2462-
}
24632469

2464-
/* if the destination CPU is idle, wake it up */
2465-
if (dsp && sched_class_above(p->sched_class,
2466-
dst_rq->curr->sched_class))
2467-
resched_curr(dst_rq);
2470+
/* if the destination CPU is idle, wake it up */
2471+
if (sched_class_above(p->sched_class,
2472+
dst_rq->curr->sched_class))
2473+
resched_curr(dst_rq);
2474+
}
24682475

24692476
/* switch back to @rq lock */
24702477
if (rq != dst_rq) {

0 commit comments

Comments
 (0)