[PATCH RESEND] sched/nohz: Add HRTICK_BW for using cfs bandwidth with nohz_full

* [PATCH RESEND] sched/nohz: Add HRTICK_BW for using cfs bandwidth with nohz_full
@ 2023-05-18 13:20 Phil Auld
  2023-05-18 13:47 ` Peter Zijlstra
                   ` (3 more replies)
  0 siblings, 4 replies; 10+ messages in thread
From: Phil Auld @ 2023-05-18 13:20 UTC (permalink / raw)
  To: linux-kernel
  Cc: Juri Lelli, Ingo Molnar, Daniel Bristot de Oliveira,
	Peter Zijlstra, Vincent Guittot, Dietmar Eggemann,
	Valentin Schneider, Ben Segall, Steven Rostedt, Mel Gorman,
	Phil Auld

CFS bandwidth limits and NOHZ full don't play well together.  Tasks
can easily run well past their quotas before a remote tick does
accounting.  This leads to long, multi-period stalls before such
tasks can run again.  Use the hrtick mechanism to set a sched
tick to fire at remaining_runtime in the future if we are on
a nohz full cpu, if the task has quota and if we are likely to
disable the tick (nr_running == 1).  This allows for bandwidth
accounting before tasks go too far over quota.

A number of container workloads use a dynamic number of real
nohz tasks but also have other work that is limited which ends
up running on the "spare" nohz cpus.  This is an artifact of
having to specify nohz_full cpus at boot. Adding this hrtick
resolves the issue of long stalls on these tasks.

Add the sched_feat HRTICK_BW off by default to allow users to
enable this only when needed.

Signed-off-by: Phil Auld <pauld@redhat.com>
Suggested-by: Juri Lelli <jlelli@redhat.com>
Cc: Ingo Molnar <mingo@redhat.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Vincent Guittot <vincent.guittot@linaro.org>
Cc: Juri Lelli <juri.lelli@redhat.com>
Cc: Dietmar Eggemann <dietmar.eggemann@arm.com>
Cc: Valentin Schneider <vschneid@redhat.com>
Cc: Ben Segall <bsegall@google.com>
---

Resend with LKML address instead of rh list...

 kernel/sched/core.c     |  2 +-
 kernel/sched/fair.c     | 20 ++++++++++++++++++++
 kernel/sched/features.h |  1 +
 kernel/sched/sched.h    | 12 ++++++++++++
 4 files changed, 34 insertions(+), 1 deletion(-)

diff --git a/kernel/sched/core.c b/kernel/sched/core.c
index a68d1276bab0..76425c377245 100644
--- a/kernel/sched/core.c
+++ b/kernel/sched/core.c
@@ -6562,7 +6562,7 @@ static void __sched notrace __schedule(unsigned int sched_mode)
 
 	schedule_debug(prev, !!sched_mode);
 
-	if (sched_feat(HRTICK) || sched_feat(HRTICK_DL))
+	if (sched_feat(HRTICK) || sched_feat(HRTICK_DL) || sched_feat(HRTICK_BW))
 		hrtick_clear(rq);
 
 	local_irq_disable();
diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
index 373ff5f55884..0dd1f6a874bc 100644
--- a/kernel/sched/fair.c
+++ b/kernel/sched/fair.c
@@ -5309,6 +5309,22 @@ static int assign_cfs_rq_runtime(struct cfs_rq *cfs_rq)
 	return ret;
 }
 
+#if defined(CONFIG_SCHED_HRTICK) && defined(CONFIG_NO_HZ_FULL)
+static void start_hrtick_cfs_bw(struct rq *rq, struct cfs_rq *cfs_rq)
+{
+	if (!tick_nohz_full_cpu(rq->cpu) || !cfs_bandwidth_used() || !cfs_rq->runtime_enabled)
+		return;
+
+	/* runtime_remaining should never be negative here but just in case */
+	if (rq->nr_running == 1 && cfs_rq->runtime_remaining > 0)
+		hrtick_start(rq, cfs_rq->runtime_remaining);
+}
+#else
+static void start_hrtick_cfs_bw(struct rq *rq, struct cfs_rq *cfs_rq)
+{
+}
+#endif
+
 static void __account_cfs_rq_runtime(struct cfs_rq *cfs_rq, u64 delta_exec)
 {
 	/* dock delta_exec before expiring quota (as it could span periods) */
@@ -5481,6 +5497,7 @@ static bool throttle_cfs_rq(struct cfs_rq *cfs_rq)
 	 */
 	cfs_rq->throttled = 1;
 	cfs_rq->throttled_clock = rq_clock(rq);
+
 	return true;
 }
 
@@ -8096,6 +8113,9 @@ done: __maybe_unused;
 	if (hrtick_enabled_fair(rq))
 		hrtick_start_fair(rq, p);
 
+	if (hrtick_enabled_bw(rq))
+		start_hrtick_cfs_bw(rq, task_cfs_rq(p));
+
 	update_misfit_status(p, rq);
 
 	return p;
diff --git a/kernel/sched/features.h b/kernel/sched/features.h
index ee7f23c76bd3..c9d4fa0c1430 100644
--- a/kernel/sched/features.h
+++ b/kernel/sched/features.h
@@ -39,6 +39,7 @@ SCHED_FEAT(WAKEUP_PREEMPTION, true)
 
 SCHED_FEAT(HRTICK, false)
 SCHED_FEAT(HRTICK_DL, false)
+SCHED_FEAT(HRTICK_BW, false)
 SCHED_FEAT(DOUBLE_TICK, false)
 
 /*
diff --git a/kernel/sched/sched.h b/kernel/sched/sched.h
index ec7b3e0a2b20..db3a3b4b5746 100644
--- a/kernel/sched/sched.h
+++ b/kernel/sched/sched.h
@@ -2507,6 +2507,13 @@ static inline int hrtick_enabled_dl(struct rq *rq)
 	return hrtick_enabled(rq);
 }
 
+static inline int hrtick_enabled_bw(struct rq *rq)
+{
+	if (!sched_feat(HRTICK_BW))
+		return 0;
+	return hrtick_enabled(rq);
+}
+
 void hrtick_start(struct rq *rq, u64 delay);
 
 #else
@@ -2521,6 +2528,11 @@ static inline int hrtick_enabled_dl(struct rq *rq)
 	return 0;
 }
 
+static inline int hrtick_enabled_bw(struct rq *rq)
+{
+	return 0;
+}
+
 static inline int hrtick_enabled(struct rq *rq)
 {
 	return 0;
-- 
2.31.1


^ permalink raw reply related	[flat|nested] 10+ messages in thread