Skip to content

Commit 1ee14e6

Browse files
Ben SegallIngo Molnar
authored andcommitted
sched: Fix race on toggling cfs_bandwidth_used
When we transition cfs_bandwidth_used to false, any currently throttled groups will incorrectly return false from cfs_rq_throttled. While tg_set_cfs_bandwidth will unthrottle them eventually, currently running code (including at least dequeue_task_fair and distribute_cfs_runtime) will cause errors. Fix this by turning off cfs_bandwidth_used only after unthrottling all cfs_rqs. Tested: toggle bandwidth back and forth on a loaded cgroup. Caused crashes in minutes without the patch, hasn't crashed with it. Signed-off-by: Ben Segall <[email protected]> Signed-off-by: Peter Zijlstra <[email protected]> Cc: [email protected] Link: http://lkml.kernel.org/r/20131016181611.22647.80365.stgit@sword-of-the-dawn.mtv.corp.google.com Signed-off-by: Ingo Molnar <[email protected]>
1 parent ac9ff79 commit 1ee14e6

File tree

3 files changed

+19
-9
lines changed

3 files changed

+19
-9
lines changed

kernel/sched/core.c

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7436,7 +7436,12 @@ static int tg_set_cfs_bandwidth(struct task_group *tg, u64 period, u64 quota)
74367436

74377437
runtime_enabled = quota != RUNTIME_INF;
74387438
runtime_was_enabled = cfs_b->quota != RUNTIME_INF;
7439-
account_cfs_bandwidth_used(runtime_enabled, runtime_was_enabled);
7439+
/*
7440+
* If we need to toggle cfs_bandwidth_used, off->on must occur
7441+
* before making related changes, and on->off must occur afterwards
7442+
*/
7443+
if (runtime_enabled && !runtime_was_enabled)
7444+
cfs_bandwidth_usage_inc();
74407445
raw_spin_lock_irq(&cfs_b->lock);
74417446
cfs_b->period = ns_to_ktime(period);
74427447
cfs_b->quota = quota;
@@ -7462,6 +7467,8 @@ static int tg_set_cfs_bandwidth(struct task_group *tg, u64 period, u64 quota)
74627467
unthrottle_cfs_rq(cfs_rq);
74637468
raw_spin_unlock_irq(&rq->lock);
74647469
}
7470+
if (runtime_was_enabled && !runtime_enabled)
7471+
cfs_bandwidth_usage_dec();
74657472
out_unlock:
74667473
mutex_unlock(&cfs_constraints_mutex);
74677474

kernel/sched/fair.c

Lines changed: 9 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -2845,21 +2845,23 @@ static inline bool cfs_bandwidth_used(void)
28452845
return static_key_false(&__cfs_bandwidth_used);
28462846
}
28472847

2848-
void account_cfs_bandwidth_used(int enabled, int was_enabled)
2848+
void cfs_bandwidth_usage_inc(void)
28492849
{
2850-
/* only need to count groups transitioning between enabled/!enabled */
2851-
if (enabled && !was_enabled)
2852-
static_key_slow_inc(&__cfs_bandwidth_used);
2853-
else if (!enabled && was_enabled)
2854-
static_key_slow_dec(&__cfs_bandwidth_used);
2850+
static_key_slow_inc(&__cfs_bandwidth_used);
2851+
}
2852+
2853+
void cfs_bandwidth_usage_dec(void)
2854+
{
2855+
static_key_slow_dec(&__cfs_bandwidth_used);
28552856
}
28562857
#else /* HAVE_JUMP_LABEL */
28572858
static bool cfs_bandwidth_used(void)
28582859
{
28592860
return true;
28602861
}
28612862

2862-
void account_cfs_bandwidth_used(int enabled, int was_enabled) {}
2863+
void cfs_bandwidth_usage_inc(void) {}
2864+
void cfs_bandwidth_usage_dec(void) {}
28632865
#endif /* HAVE_JUMP_LABEL */
28642866

28652867
/*

kernel/sched/sched.h

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1352,7 +1352,8 @@ extern void print_rt_stats(struct seq_file *m, int cpu);
13521352
extern void init_cfs_rq(struct cfs_rq *cfs_rq);
13531353
extern void init_rt_rq(struct rt_rq *rt_rq, struct rq *rq);
13541354

1355-
extern void account_cfs_bandwidth_used(int enabled, int was_enabled);
1355+
extern void cfs_bandwidth_usage_inc(void);
1356+
extern void cfs_bandwidth_usage_dec(void);
13561357

13571358
#ifdef CONFIG_NO_HZ_COMMON
13581359
enum rq_nohz_flag_bits {

0 commit comments

Comments
 (0)