lkml.org 
[lkml]   [2022]   [Mar]   [28]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
Patch in this message
/
From
Subject[PATCH] sched/fair: fix broken bandwidth control with nohz_full
Date
With nohz_full enabled on cpu, the scheduler_tick() will be stopped
when only one CFS task left on rq.

scheduler_tick()
task_tick_fair()
entity_tick()
update_curr()
account_cfs_rq_runtime(cfs_rq, delta_exec) --> stopped

So that running task can't account its runtime periodically, but
the cfs_bandwidth hrtimer still __refill_cfs_bandwidth_runtime()
periodically. Later in one period, the task would account very
big delta_exec, which cause the cfs_rq to be throttled for a
long time.

There are two solutions for the problem, the first is that we
can check in sched_can_stop_tick() if current task's cfs_rq
have runtime_enabled, in which case we don't stop tick. But
it will make nohz_full almost useless in cloud environment
that every container has the cpu bandwidth control setting.

The other is what this patch implemented, cfs_bandwidth hrtimer
would sync unaccounted runtime from all running cfs_rqs with
tick stopped, just before __refill_cfs_bandwidth_runtime().
Also do the same thing in tg_set_cfs_bandwidth().

A testcase to reproduce:
```
cd /sys/fs/cgroup
echo "+cpu" > cgroup.subtree_control

mkdir test
echo "105000 100000" > test/cpu.max

echo $$ > test/cgroup.procs
taskset -c 1 bash -c "while true; do let i++; done"
```
Ctrl-C and cat test/cpu.stat to see if nr_throttled > 0.

The above testcase uses period 100ms and quota 105ms, would
only see nr_throttled > 0 on nohz_full system. The problem
is gone in test with this patch.

Signed-off-by: Chengming Zhou <zhouchengming@bytedance.com>
---
kernel/sched/core.c | 4 ++++
kernel/sched/fair.c | 30 ++++++++++++++++++++++++++++++
kernel/sched/sched.h | 3 +++
3 files changed, 37 insertions(+)

diff --git a/kernel/sched/core.c b/kernel/sched/core.c
index d575b4914925..17b5e3d27401 100644
--- a/kernel/sched/core.c
+++ b/kernel/sched/core.c
@@ -10443,6 +10443,10 @@ static int tg_set_cfs_bandwidth(struct task_group *tg, u64 period, u64 quota,
*/
if (runtime_enabled && !runtime_was_enabled)
cfs_bandwidth_usage_inc();
+
+ if (runtime_was_enabled)
+ sync_cfs_bandwidth_runtime(cfs_b);
+
raw_spin_lock_irq(&cfs_b->lock);
cfs_b->period = ns_to_ktime(period);
cfs_b->quota = quota;
diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
index ee0664c9d291..ebda70a0e3a8 100644
--- a/kernel/sched/fair.c
+++ b/kernel/sched/fair.c
@@ -5339,6 +5339,34 @@ static enum hrtimer_restart sched_cfs_slack_timer(struct hrtimer *timer)
return HRTIMER_NORESTART;
}

+#ifdef CONFIG_NO_HZ_FULL
+void sync_cfs_bandwidth_runtime(struct cfs_bandwidth *cfs_b)
+{
+ unsigned int cpu;
+ struct rq *rq;
+ struct rq_flags rf;
+ struct cfs_rq *cfs_rq;
+ struct task_group *tg;
+
+ tg = container_of(cfs_b, struct task_group, cfs_bandwidth);
+
+ for_each_online_cpu(cpu) {
+ if (!tick_nohz_tick_stopped_cpu(cpu))
+ continue;
+
+ rq = cpu_rq(cpu);
+ cfs_rq = tg->cfs_rq[cpu];
+
+ rq_lock_irqsave(rq, &rf);
+ if (cfs_rq->curr) {
+ update_rq_clock(rq);
+ update_curr(cfs_rq);
+ }
+ rq_unlock_irqrestore(rq, &rf);
+ }
+}
+#endif
+
extern const u64 max_cfs_quota_period;

static enum hrtimer_restart sched_cfs_period_timer(struct hrtimer *timer)
@@ -5350,6 +5378,8 @@ static enum hrtimer_restart sched_cfs_period_timer(struct hrtimer *timer)
int idle = 0;
int count = 0;

+ sync_cfs_bandwidth_runtime(cfs_b);
+
raw_spin_lock_irqsave(&cfs_b->lock, flags);
for (;;) {
overrun = hrtimer_forward_now(timer, cfs_b->period);
diff --git a/kernel/sched/sched.h b/kernel/sched/sched.h
index 58263f90c559..57f9da9c50c1 100644
--- a/kernel/sched/sched.h
+++ b/kernel/sched/sched.h
@@ -2351,9 +2351,12 @@ static inline void sched_update_tick_dependency(struct rq *rq)
else
tick_nohz_dep_set_cpu(cpu, TICK_DEP_BIT_SCHED);
}
+
+extern void sync_cfs_bandwidth_runtime(struct cfs_bandwidth *cfs_b);
#else
static inline int sched_tick_offload_init(void) { return 0; }
static inline void sched_update_tick_dependency(struct rq *rq) { }
+static inline void sync_cfs_bandwidth_runtime(struct cfs_bandwidth *cfs_b) {}
#endif

static inline void add_nr_running(struct rq *rq, unsigned count)
--
2.35.1
\
 
 \ /
  Last update: 2022-03-28 13:09    [W:0.069 / U:2.392 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site