History

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.

K Prateek Nayak committed 5 months ago

Commit 1abbecd1 parent b8fea7a1 file · +90 −103Browse files

Showing the changes to kernel (1 of 1 file). Show all

Unified · Split

kernel/sched/fair.c+90−103
@@ -5035,13 +5035,13 @@ static void __maybe_unused clear_tg_offline_cfs_rqs(struct rq *rq) · 5034 unmodified lines
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, · 1492 unmodified lines
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) · 282 unmodified lines
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) · 26 unmodified lines
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) · 22 unmodified lines
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) · 10 unmodified lines
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) · 3 unmodified lines
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) · 33 unmodified lines
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) · 3 unmodified lines
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) · 116 unmodified lines
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) · 2 unmodified lines
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) · 16 unmodified lines
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) · 82 unmodified lines
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) · 26 unmodified lines
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) · 60 unmodified lines
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) · 11 unmodified lines
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) · 14 unmodified lines
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) · 16 unmodified lines
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 }