Skip to content

Commit 1abbecd

Browse files
kudureranganathPeter Zijlstra
authored andcommitted
sched/fair: Convert cfs bandwidth throttling to use guards
Routine conversion of rcu_read_lock(), spin_lock*, and rq_lock usage within the cfs bandwidth controller to use class guards. Only notable changes are: - Checking for "cfs_rq->runtime_remaining <= 0" instead of the inverse to spot a throttle and break early. This also saves the need for extra indentation in the unthrottle case. - Reordering of list_del_rcu() against throttled_clock indicator update in unthrottle_cfs_rq(). Both are done with "cfs_b->lock" held after the "cfs_rq->throttled" is cleared which make the reordering safe against concurrent list modifications. No functional changes intended. Signed-off-by: K Prateek Nayak <kprateek.nayak@amd.com> Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org> Reviewed-by: Ben Segall <bsegall@google.com> Tested-by: Aaron Lu <ziqianlu@bytedance.com> Link: https://patch.msgid.link/20260602050005.11160-2-kprateek.nayak@amd.com
1 parent b8fea7a commit 1abbecd

1 file changed

Lines changed: 90 additions & 103 deletions

File tree

kernel/sched/fair.c

Lines changed: 90 additions & 103 deletions
Original file line numberDiff line numberDiff line change
@@ -5035,13 +5035,13 @@ static void __maybe_unused clear_tg_offline_cfs_rqs(struct rq *rq)
50355035
*/
50365036
rq_clock_start_loop_update(rq);
50375037

5038-
rcu_read_lock();
5038+
guard(rcu)();
5039+
50395040
list_for_each_entry_rcu(tg, &task_groups, list) {
50405041
struct cfs_rq *cfs_rq = tg_cfs_rq(tg, cpu_of(rq));
50415042

50425043
clear_tg_load_avg(cfs_rq);
50435044
}
5044-
rcu_read_unlock();
50455045

50465046
rq_clock_stop_loop_update(rq);
50475047
}
@@ -6540,13 +6540,10 @@ static int __assign_cfs_rq_runtime(struct cfs_bandwidth *cfs_b,
65406540
static int assign_cfs_rq_runtime(struct cfs_rq *cfs_rq)
65416541
{
65426542
struct cfs_bandwidth *cfs_b = tg_cfs_bandwidth(cfs_rq->tg);
6543-
int ret;
65446543

6545-
raw_spin_lock(&cfs_b->lock);
6546-
ret = __assign_cfs_rq_runtime(cfs_b, cfs_rq, sched_cfs_bandwidth_slice());
6547-
raw_spin_unlock(&cfs_b->lock);
6544+
guard(raw_spinlock)(&cfs_b->lock);
65486545

6549-
return ret;
6546+
return __assign_cfs_rq_runtime(cfs_b, cfs_rq, sched_cfs_bandwidth_slice());
65506547
}
65516548

65526549
static void __account_cfs_rq_runtime(struct cfs_rq *cfs_rq, u64 delta_exec)
@@ -6835,33 +6832,32 @@ static bool throttle_cfs_rq(struct cfs_rq *cfs_rq)
68356832
{
68366833
struct rq *rq = rq_of(cfs_rq);
68376834
struct cfs_bandwidth *cfs_b = tg_cfs_bandwidth(cfs_rq->tg);
6838-
int dequeue = 1;
68396835

6840-
raw_spin_lock(&cfs_b->lock);
6841-
/* This will start the period timer if necessary */
6842-
if (__assign_cfs_rq_runtime(cfs_b, cfs_rq, 1)) {
6836+
scoped_guard(raw_spinlock, &cfs_b->lock) {
68436837
/*
6844-
* We have raced with bandwidth becoming available, and if we
6845-
* actually throttled the timer might not unthrottle us for an
6846-
* entire period. We additionally needed to make sure that any
6847-
* subsequent check_cfs_rq_runtime calls agree not to throttle
6848-
* us, as we may commit to do cfs put_prev+pick_next, so we ask
6849-
* for 1ns of runtime rather than just check cfs_b.
6838+
* Check if We have raced with bandwidth becoming available. If
6839+
* we actually throttled the timer might not unthrottle us for
6840+
* an entire period. We additionally needed to make sure that
6841+
* any subsequent check_cfs_rq_runtime calls agree not to
6842+
* throttle us, as we may commit to do cfs put_prev+pick_next,
6843+
* so we ask for 1ns of runtime rather than just check cfs_b.
6844+
*
6845+
* This will start the period timer if necessary.
6846+
*/
6847+
if (__assign_cfs_rq_runtime(cfs_b, cfs_rq, 1))
6848+
return false;
6849+
6850+
/*
6851+
* No bandwidth available; Add ourselves on the list to be
6852+
* unthrottled later.
68506853
*/
6851-
dequeue = 0;
6852-
} else {
68536854
list_add_tail_rcu(&cfs_rq->throttled_list,
68546855
&cfs_b->throttled_cfs_rq);
68556856
}
6856-
raw_spin_unlock(&cfs_b->lock);
6857-
6858-
if (!dequeue)
6859-
return false; /* Throttle no longer required. */
68606857

68616858
/* freeze hierarchy runnable averages while throttled */
6862-
rcu_read_lock();
6863-
walk_tg_tree_from(cfs_rq->tg, tg_throttle_down, tg_nop, (void *)rq);
6864-
rcu_read_unlock();
6859+
scoped_guard(rcu)
6860+
walk_tg_tree_from(cfs_rq->tg, tg_throttle_down, tg_nop, (void *)rq);
68656861

68666862
/*
68676863
* Note: distribution will already see us throttled via the
@@ -6894,13 +6890,15 @@ void unthrottle_cfs_rq(struct cfs_rq *cfs_rq)
68946890

68956891
update_rq_clock(rq);
68966892

6897-
raw_spin_lock(&cfs_b->lock);
6898-
if (cfs_rq->throttled_clock) {
6893+
scoped_guard(raw_spinlock, &cfs_b->lock) {
6894+
list_del_rcu(&cfs_rq->throttled_list);
6895+
6896+
if (!cfs_rq->throttled_clock)
6897+
break;
6898+
68996899
cfs_b->throttled_time += rq_clock(rq) - cfs_rq->throttled_clock;
69006900
cfs_rq->throttled_clock = 0;
69016901
}
6902-
list_del_rcu(&cfs_rq->throttled_list);
6903-
raw_spin_unlock(&cfs_b->lock);
69046902

69056903
/* update hierarchical throttle state */
69066904
walk_tg_tree_from(cfs_rq->tg, tg_nop, tg_unthrottle_up, (void *)rq);
@@ -6929,9 +6927,8 @@ static void __cfsb_csd_unthrottle(void *arg)
69296927
{
69306928
struct cfs_rq *cursor, *tmp;
69316929
struct rq *rq = arg;
6932-
struct rq_flags rf;
69336930

6934-
rq_lock(rq, &rf);
6931+
guard(rq_lock)(rq);
69356932

69366933
/*
69376934
* Iterating over the list can trigger several call to
@@ -6948,7 +6945,7 @@ static void __cfsb_csd_unthrottle(void *arg)
69486945
* race with group being freed in the window between removing it
69496946
* from the list and advancing to the next entry in the list.
69506947
*/
6951-
rcu_read_lock();
6948+
guard(rcu)();
69526949

69536950
list_for_each_entry_safe(cursor, tmp, &rq->cfsb_csd_list,
69546951
throttled_csd_list) {
@@ -6958,10 +6955,7 @@ static void __cfsb_csd_unthrottle(void *arg)
69586955
unthrottle_cfs_rq(cursor);
69596956
}
69606957

6961-
rcu_read_unlock();
6962-
69636958
rq_clock_stop_loop_update(rq);
6964-
rq_unlock(rq, &rf);
69656959
}
69666960

69676961
static inline void __unthrottle_cfs_rq_async(struct cfs_rq *cfs_rq)
@@ -7001,11 +6995,11 @@ static bool distribute_cfs_runtime(struct cfs_bandwidth *cfs_b)
70016995
u64 runtime, remaining = 1;
70026996
bool throttled = false;
70036997
struct cfs_rq *cfs_rq, *tmp;
7004-
struct rq_flags rf;
70056998
struct rq *rq;
70066999
LIST_HEAD(local_unthrottle);
70077000

7008-
rcu_read_lock();
7001+
guard(rcu)();
7002+
70097003
list_for_each_entry_rcu(cfs_rq, &cfs_b->throttled_cfs_rq,
70107004
throttled_list) {
70117005
rq = rq_of(cfs_rq);
@@ -7015,65 +7009,63 @@ static bool distribute_cfs_runtime(struct cfs_bandwidth *cfs_b)
70157009
break;
70167010
}
70177011

7018-
rq_lock_irqsave(rq, &rf);
7012+
guard(rq_lock_irqsave)(rq);
7013+
70197014
if (!cfs_rq_throttled(cfs_rq))
7020-
goto next;
7015+
continue;
70217016

70227017
/* Already queued for async unthrottle */
70237018
if (!list_empty(&cfs_rq->throttled_csd_list))
7024-
goto next;
7019+
continue;
70257020

70267021
/* By the above checks, this should never be true */
70277022
WARN_ON_ONCE(cfs_rq->runtime_remaining > 0);
70287023

7029-
raw_spin_lock(&cfs_b->lock);
7030-
runtime = -cfs_rq->runtime_remaining + 1;
7031-
if (runtime > cfs_b->runtime)
7032-
runtime = cfs_b->runtime;
7033-
cfs_b->runtime -= runtime;
7034-
remaining = cfs_b->runtime;
7035-
raw_spin_unlock(&cfs_b->lock);
7024+
scoped_guard(raw_spinlock, &cfs_b->lock) {
7025+
runtime = -cfs_rq->runtime_remaining + 1;
7026+
if (runtime > cfs_b->runtime)
7027+
runtime = cfs_b->runtime;
7028+
cfs_b->runtime -= runtime;
7029+
remaining = cfs_b->runtime;
7030+
}
70367031

70377032
cfs_rq->runtime_remaining += runtime;
70387033

7039-
/* we check whether we're throttled above */
7040-
if (cfs_rq->runtime_remaining > 0) {
7041-
if (cpu_of(rq) != this_cpu) {
7042-
unthrottle_cfs_rq_async(cfs_rq);
7043-
} else {
7044-
/*
7045-
* We currently only expect to be unthrottling
7046-
* a single cfs_rq locally.
7047-
*/
7048-
WARN_ON_ONCE(!list_empty(&local_unthrottle));
7049-
list_add_tail(&cfs_rq->throttled_csd_list,
7050-
&local_unthrottle);
7051-
}
7052-
} else {
7034+
/*
7035+
* Ran out of bandwidth during distribution!
7036+
* Indicate throttled entities and break early.
7037+
*/
7038+
if (cfs_rq->runtime_remaining <= 0) {
70537039
throttled = true;
7040+
break;
70547041
}
70557042

7056-
next:
7057-
rq_unlock_irqrestore(rq, &rf);
7043+
/* we check whether we're throttled above */
7044+
if (cpu_of(rq) != this_cpu) {
7045+
unthrottle_cfs_rq_async(cfs_rq);
7046+
continue;
7047+
}
7048+
7049+
/*
7050+
* We currently only expect to be unthrottling
7051+
* a single cfs_rq locally.
7052+
*/
7053+
WARN_ON_ONCE(!list_empty(&local_unthrottle));
7054+
list_add_tail(&cfs_rq->throttled_csd_list, &local_unthrottle);
70587055
}
70597056

70607057
list_for_each_entry_safe(cfs_rq, tmp, &local_unthrottle,
70617058
throttled_csd_list) {
70627059
struct rq *rq = rq_of(cfs_rq);
70637060

7064-
rq_lock_irqsave(rq, &rf);
7061+
guard(rq_lock_irqsave)(rq);
70657062

70667063
list_del_init(&cfs_rq->throttled_csd_list);
7067-
70687064
if (cfs_rq_throttled(cfs_rq))
70697065
unthrottle_cfs_rq(cfs_rq);
7070-
7071-
rq_unlock_irqrestore(rq, &rf);
70727066
}
70737067
WARN_ON_ONCE(!list_empty(&local_unthrottle));
70747068

7075-
rcu_read_unlock();
7076-
70777069
return throttled;
70787070
}
70797071

@@ -7196,7 +7188,8 @@ static void __return_cfs_rq_runtime(struct cfs_rq *cfs_rq)
71967188
if (slack_runtime <= 0)
71977189
return;
71987190

7199-
raw_spin_lock(&cfs_b->lock);
7191+
guard(raw_spinlock)(&cfs_b->lock);
7192+
72007193
if (cfs_b->quota != RUNTIME_INF) {
72017194
cfs_b->runtime += slack_runtime;
72027195

@@ -7205,7 +7198,6 @@ static void __return_cfs_rq_runtime(struct cfs_rq *cfs_rq)
72057198
!list_empty(&cfs_b->throttled_cfs_rq))
72067199
start_cfs_slack_bandwidth(cfs_b);
72077200
}
7208-
raw_spin_unlock(&cfs_b->lock);
72097201

72107202
/* even if it's not valid for return we don't want to try again */
72117203
cfs_rq->runtime_remaining -= slack_runtime;
@@ -7228,25 +7220,21 @@ static __always_inline void return_cfs_rq_runtime(struct cfs_rq *cfs_rq)
72287220
*/
72297221
static void do_sched_cfs_slack_timer(struct cfs_bandwidth *cfs_b)
72307222
{
7231-
u64 runtime = 0, slice = sched_cfs_bandwidth_slice();
7232-
unsigned long flags;
7233-
72347223
/* confirm we're still not at a refresh boundary */
7235-
raw_spin_lock_irqsave(&cfs_b->lock, flags);
7236-
cfs_b->slack_started = false;
7224+
scoped_guard(raw_spinlock_irqsave, &cfs_b->lock) {
7225+
u64 runtime = 0, slice = sched_cfs_bandwidth_slice();
72377226

7238-
if (runtime_refresh_within(cfs_b, min_bandwidth_expiration)) {
7239-
raw_spin_unlock_irqrestore(&cfs_b->lock, flags);
7240-
return;
7241-
}
7227+
cfs_b->slack_started = false;
72427228

7243-
if (cfs_b->quota != RUNTIME_INF && cfs_b->runtime > slice)
7244-
runtime = cfs_b->runtime;
7229+
if (runtime_refresh_within(cfs_b, min_bandwidth_expiration))
7230+
return;
72457231

7246-
raw_spin_unlock_irqrestore(&cfs_b->lock, flags);
7232+
if (cfs_b->quota != RUNTIME_INF && cfs_b->runtime > slice)
7233+
runtime = cfs_b->runtime;
72477234

7248-
if (!runtime)
7249-
return;
7235+
if (!runtime)
7236+
return;
7237+
}
72507238

72517239
distribute_cfs_runtime(cfs_b);
72527240
}
@@ -7335,18 +7323,18 @@ static enum hrtimer_restart sched_cfs_period_timer(struct hrtimer *timer)
73357323
{
73367324
struct cfs_bandwidth *cfs_b =
73377325
container_of(timer, struct cfs_bandwidth, period_timer);
7338-
unsigned long flags;
73397326
int overrun;
73407327
int idle = 0;
73417328
int count = 0;
73427329

7343-
raw_spin_lock_irqsave(&cfs_b->lock, flags);
7330+
CLASS(raw_spinlock_irqsave, cfsb_guard)(&cfs_b->lock);
7331+
73447332
for (;;) {
73457333
overrun = hrtimer_forward_now(timer, cfs_b->period);
73467334
if (!overrun)
73477335
break;
73487336

7349-
idle = do_sched_cfs_period_timer(cfs_b, overrun, flags);
7337+
idle = do_sched_cfs_period_timer(cfs_b, overrun, cfsb_guard.flags);
73507338

73517339
if (++count > 3) {
73527340
u64 new, old = ktime_to_ns(cfs_b->period);
@@ -7379,11 +7367,13 @@ static enum hrtimer_restart sched_cfs_period_timer(struct hrtimer *timer)
73797367
count = 0;
73807368
}
73817369
}
7382-
if (idle)
7370+
7371+
if (idle) {
73837372
cfs_b->period_active = 0;
7384-
raw_spin_unlock_irqrestore(&cfs_b->lock, flags);
7373+
return HRTIMER_NORESTART;
7374+
}
73857375

7386-
return idle ? HRTIMER_NORESTART : HRTIMER_RESTART;
7376+
return HRTIMER_RESTART;
73877377
}
73887378

73897379
void init_cfs_bandwidth(struct cfs_bandwidth *cfs_b, struct cfs_bandwidth *parent)
@@ -7450,14 +7440,12 @@ static void destroy_cfs_bandwidth(struct cfs_bandwidth *cfs_b)
74507440
*/
74517441
for_each_possible_cpu(i) {
74527442
struct rq *rq = cpu_rq(i);
7453-
unsigned long flags;
74547443

74557444
if (list_empty(&rq->cfsb_csd_list))
74567445
continue;
74577446

7458-
local_irq_save(flags);
7459-
__cfsb_csd_unthrottle(rq);
7460-
local_irq_restore(flags);
7447+
scoped_guard(irqsave)
7448+
__cfsb_csd_unthrottle(rq);
74617449
}
74627450
}
74637451

@@ -7475,16 +7463,15 @@ static void __maybe_unused update_runtime_enabled(struct rq *rq)
74757463

74767464
lockdep_assert_rq_held(rq);
74777465

7478-
rcu_read_lock();
7466+
guard(rcu)();
7467+
74797468
list_for_each_entry_rcu(tg, &task_groups, list) {
74807469
struct cfs_bandwidth *cfs_b = &tg->cfs_bandwidth;
74817470
struct cfs_rq *cfs_rq = tg_cfs_rq(tg, cpu_of(rq));
74827471

7483-
raw_spin_lock(&cfs_b->lock);
7484-
cfs_rq->runtime_enabled = cfs_b->quota != RUNTIME_INF;
7485-
raw_spin_unlock(&cfs_b->lock);
7472+
scoped_guard(raw_spinlock, &cfs_b->lock)
7473+
cfs_rq->runtime_enabled = cfs_b->quota != RUNTIME_INF;
74867474
}
7487-
rcu_read_unlock();
74887475
}
74897476

74907477
/* cpu offline callback */
@@ -7505,7 +7492,8 @@ static void __maybe_unused unthrottle_offline_cfs_rqs(struct rq *rq)
75057492
*/
75067493
rq_clock_start_loop_update(rq);
75077494

7508-
rcu_read_lock();
7495+
guard(rcu)();
7496+
75097497
list_for_each_entry_rcu(tg, &task_groups, list) {
75107498
struct cfs_rq *cfs_rq = tg_cfs_rq(tg, cpu_of(rq));
75117499

@@ -7528,7 +7516,6 @@ static void __maybe_unused unthrottle_offline_cfs_rqs(struct rq *rq)
75287516
cfs_rq->runtime_remaining = 1;
75297517
unthrottle_cfs_rq(cfs_rq);
75307518
}
7531-
rcu_read_unlock();
75327519

75337520
rq_clock_stop_loop_update(rq);
75347521
}

0 commit comments

Comments
 (0)