Message-Id: <20220106004656.126790-16-daniel.m.jordan@oracle.com>
Date:   Wed,  5 Jan 2022 19:46:55 -0500
From:   Daniel Jordan <daniel.m.jordan@...cle.com>
To:     Alexander Duyck <alexanderduyck@...com>,
        Alex Williamson <alex.williamson@...hat.com>,
        Andrew Morton <akpm@...ux-foundation.org>,
        Ben Segall <bsegall@...gle.com>,
        Cornelia Huck <cohuck@...hat.com>,
        Dan Williams <dan.j.williams@...el.com>,
        Dave Hansen <dave.hansen@...ux.intel.com>,
        Dietmar Eggemann <dietmar.eggemann@....com>,
        Herbert Xu <herbert@...dor.apana.org.au>,
        Ingo Molnar <mingo@...hat.com>,
        Jason Gunthorpe <jgg@...dia.com>,
        Johannes Weiner <hannes@...xchg.org>,
        Josh Triplett <josh@...htriplett.org>,
        Michal Hocko <mhocko@...e.com>, Nico Pache <npache@...hat.com>,
        Pasha Tatashin <pasha.tatashin@...een.com>,
        Peter Zijlstra <peterz@...radead.org>,
        Steffen Klassert <steffen.klassert@...unet.com>,
        Steve Sistare <steven.sistare@...cle.com>,
        Tejun Heo <tj@...nel.org>,
        Tim Chen <tim.c.chen@...ux.intel.com>,
        Vincent Guittot <vincent.guittot@...aro.org>
Cc:     linux-mm@...ck.org, kvm@...r.kernel.org,
        linux-kernel@...r.kernel.org, linux-crypto@...r.kernel.org,
        Daniel Jordan <daniel.m.jordan@...cle.com>
Subject: [RFC 15/16] sched/fair: Account kthread runtime debt for CFS bandwidth

As before, helpers in multithreaded jobs don't honor the main thread's
CFS bandwidth limits, which could lead to the group exceeding its quota.

Fix it by having helpers remote charge their CPU time to the main
thread's task group.  A helper calls a pair of new interfaces
cpu_cgroup_remote_begin() and cpu_cgroup_remote_charge() (see function
header comments) to achieve this.

This is just supposed to start a discussion, so it's pretty simple.
Once a kthread has finished a remote charging period with
cpu_cgroup_remote_charge(), its runtime is subtracted from the target
task group's runtime (cfs_bandwidth::runtime) and any remainder is saved
as debt (cfs_bandwidth::debt) to pay off in later periods.

Remote charging tasks aren't throttled when the group reaches its quota,
and a task group doesn't run at all until its debt is completely paid,
but these shortcomings can be addressed if the approach ends up being
taken.

Signed-off-by: Daniel Jordan <daniel.m.jordan@...cle.com>
---
 include/linux/sched.h        |  2 +
 include/linux/sched/cgroup.h | 16 ++++++
 kernel/padata.c              | 26 +++++++---
 kernel/sched/core.c          | 39 +++++++++++++++
 kernel/sched/fair.c          | 94 +++++++++++++++++++++++++++++++++++-
 kernel/sched/sched.h         |  5 ++
 6 files changed, 174 insertions(+), 8 deletions(-)

diff --git a/include/linux/sched.h b/include/linux/sched.h
index ec8d07d88641..cc04367d4458 100644
--- a/include/linux/sched.h
+++ b/include/linux/sched.h
@@ -487,6 +487,8 @@ struct sched_entity {
 	struct cfs_rq			*my_q;
 	/* cached value of my_q->h_nr_running */
 	unsigned long			runnable_weight;
+	/* sum_exec_runtime at the start of the remote charging period */
+	u64				remote_runtime_begin;
 #endif
 
 #ifdef CONFIG_SMP
diff --git a/include/linux/sched/cgroup.h b/include/linux/sched/cgroup.h
index f89d92e9e015..cb3b7941149f 100644
--- a/include/linux/sched/cgroup.h
+++ b/include/linux/sched/cgroup.h
@@ -5,6 +5,22 @@
 #include <linux/cgroup-defs.h>
 #include <linux/cpumask.h>
 
+#ifdef CONFIG_FAIR_GROUP_SCHED
+
+void cpu_cgroup_remote_begin(struct task_struct *p,
+			     struct cgroup_subsys_state *css);
+void cpu_cgroup_remote_charge(struct task_struct *p,
+			      struct cgroup_subsys_state *css);
+
+#else /* CONFIG_FAIR_GROUP_SCHED */
+
+static inline void cpu_cgroup_remote_begin(struct task_struct *p,
+					   struct cgroup_subsys_state *css) {}
+static inline void cpu_cgroup_remote_charge(struct task_struct *p,
+					    struct cgroup_subsys_state *css) {}
+
+#endif /* CONFIG_FAIR_GROUP_SCHED */
+
 #ifdef CONFIG_CFS_BANDWIDTH
 
 int max_cfs_bandwidth_cpus(struct cgroup_subsys_state *css);
diff --git a/kernel/padata.c b/kernel/padata.c
index 52f670a5d6d9..d595f11c2fdd 100644
--- a/kernel/padata.c
+++ b/kernel/padata.c
@@ -43,6 +43,7 @@
 enum padata_work_flags {
 	PADATA_WORK_FINISHED	= 1,
 	PADATA_WORK_UNDO	= 2,
+	PADATA_WORK_MAIN_THR	= 4,
 };
 
 struct padata_work {
@@ -75,6 +76,7 @@ struct padata_mt_job_state {
 #ifdef CONFIG_LOCKDEP
 	struct lockdep_map	lockdep_map;
 #endif
+	struct cgroup_subsys_state *cpu_css;
 };
 
 static void padata_free_pd(struct parallel_data *pd);
@@ -495,6 +497,10 @@ static int padata_mt_helper(void *__pw)
 	struct padata_work *pw = __pw;
 	struct padata_mt_job_state *ps = pw->pw_data;
 	struct padata_mt_job *job = ps->job;
+	bool is_main = pw->pw_flags & PADATA_WORK_MAIN_THR;
+
+	if (!is_main)
+		cpu_cgroup_remote_begin(current, ps->cpu_css);
 
 	spin_lock(&ps->lock);
 
@@ -518,6 +524,10 @@ static int padata_mt_helper(void *__pw)
 		ret = job->thread_fn(position, end, job->fn_arg);
 
 		lock_map_release(&ps->lockdep_map);
+
+		if (!is_main)
+			cpu_cgroup_remote_charge(current, ps->cpu_css);
+
 		spin_lock(&ps->lock);
 
 		if (ret) {
@@ -612,7 +622,6 @@ int padata_do_multithreaded_job(struct padata_mt_job *job,
 {
 	/* In case threads finish at different times. */
 	static const unsigned long load_balance_factor = 4;
-	struct cgroup_subsys_state *cpu_css;
 	struct padata_work my_work, *pw;
 	struct padata_mt_job_state ps;
 	LIST_HEAD(unfinished_works);
@@ -628,18 +637,20 @@ int padata_do_multithreaded_job(struct padata_mt_job *job,
 	req = min(req, current->nr_cpus_allowed);
 
 #ifdef CONFIG_CGROUP_SCHED
+	ps.cpu_css = task_get_css(current, cpu_cgrp_id);
+
 	/*
 	 * Cap threads at the max number of CPUs current's CFS bandwidth
 	 * settings allow.  Keep it simple, don't try to keep this value up to
 	 * date.  The ifdef guards cpu_cgrp_id.
 	 */
-	rcu_read_lock();
-	cpu_css = task_css(current, cpu_cgrp_id);
-	req = min(req, max_cfs_bandwidth_cpus(cpu_css));
-	rcu_read_unlock();
+	req = min(req, max_cfs_bandwidth_cpus(ps.cpu_css));
 #endif
 
 	if (req == 1) {
+#ifdef CONFIG_CGROUP_SCHED
+		css_put(ps.cpu_css);
+#endif
 		/* Single thread, no coordination needed, cut to the chase. */
 		return job->thread_fn(job->start, job->start + job->size,
 				      job->fn_arg);
@@ -687,12 +698,15 @@ int padata_do_multithreaded_job(struct padata_mt_job *job,
 
 	/* Use the current task, which saves starting a kthread. */
 	my_work.pw_data = &ps;
-	my_work.pw_flags = 0;
+	my_work.pw_flags = PADATA_WORK_MAIN_THR;
 	INIT_LIST_HEAD(&my_work.pw_list);
 	padata_mt_helper(&my_work);
 
 	/* Wait for all the helpers to finish. */
 	padata_wait_for_helpers(&ps, &unfinished_works, &finished_works);
+#ifdef CONFIG_CGROUP_SCHED
+	css_put(ps.cpu_css);
+#endif
 
 	if (ps.error && job->undo_fn)
 		padata_undo(&ps, &finished_works, &my_work);
diff --git a/kernel/sched/core.c b/kernel/sched/core.c
index 848c9fec8006..a5e24b6bd7e0 100644
--- a/kernel/sched/core.c
+++ b/kernel/sched/core.c
@@ -9913,6 +9913,7 @@ static int tg_set_cfs_bandwidth(struct task_group *tg, u64 period, u64 quota,
 	cfs_b->period = ns_to_ktime(period);
 	cfs_b->quota = quota;
 	cfs_b->burst = burst;
+	cfs_b->debt = 0;
 
 	__refill_cfs_bandwidth_runtime(cfs_b);
 
@@ -10181,6 +10182,44 @@ static int cpu_cfs_stat_show(struct seq_file *sf, void *v)
 	return 0;
 }
 #endif /* CONFIG_CFS_BANDWIDTH */
+
+/**
+ * cpu_cgroup_remote_begin - begin charging p's CPU usage to a remote css
+ * @p: the kernel thread whose CPU usage should be accounted
+ * @css: the css to which the CPU usage should be accounted
+ *
+ * Begin charging a kernel thread's CPU usage to a remote (non-root) task group
+ * to account CPU time that the kernel thread spends working on behalf of the
+ * group.  Pair with at least one subsequent call to cpu_cgroup_remote_charge()
+ * to complete the charge.
+ *
+ * Supports CFS bandwidth and cgroup2 CPU accounting stats but not weight-based
+ * control for now.
+ */
+void cpu_cgroup_remote_begin(struct task_struct *p,
+			     struct cgroup_subsys_state *css)
+{
+	if (p->sched_class == &fair_sched_class)
+		cpu_cgroup_remote_begin_fair(p, css_tg(css));
+}
+
+/**
+ * cpu_cgroup_remote_charge - account p's CPU usage to a remote css
+ * @p: the kernel thread whose CPU usage should be accounted
+ * @css: the css to which the CPU usage should be accounted
+ *
+ * Account a kernel thread's CPU usage to a remote (non-root) task group.  Pair
+ * with a previous call to cpu_cgroup_remote_begin() with the same @p and @css.
+ * This may be invoked multiple times after the initial
+ * cpu_cgroup_remote_begin() to account additional CPU usage.
+ */
+void cpu_cgroup_remote_charge(struct task_struct *p,
+			      struct cgroup_subsys_state *css)
+{
+	if (p->sched_class == &fair_sched_class)
+		cpu_cgroup_remote_charge_fair(p, css_tg(css));
+}
+
 #endif /* CONFIG_FAIR_GROUP_SCHED */
 
 #ifdef CONFIG_RT_GROUP_SCHED
diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
index 44c452072a1b..3c2d7f245c68 100644
--- a/kernel/sched/fair.c
+++ b/kernel/sched/fair.c
@@ -4655,10 +4655,19 @@ static inline u64 sched_cfs_bandwidth_slice(void)
  */
 void __refill_cfs_bandwidth_runtime(struct cfs_bandwidth *cfs_b)
 {
-	if (unlikely(cfs_b->quota == RUNTIME_INF))
+	u64 quota = cfs_b->quota;
+	u64 payment;
+
+	if (unlikely(quota == RUNTIME_INF))
 		return;
 
-	cfs_b->runtime += cfs_b->quota;
+	if (cfs_b->debt) {
+		payment = min(quota, cfs_b->debt);
+		cfs_b->debt -= payment;
+		quota -= payment;
+	}
+
+	cfs_b->runtime += quota;
 	cfs_b->runtime = min(cfs_b->runtime, cfs_b->quota + cfs_b->burst);
 }
 
@@ -5406,6 +5415,32 @@ static void __maybe_unused unthrottle_offline_cfs_rqs(struct rq *rq)
 	rcu_read_unlock();
 }
 
+static void incur_cfs_debt(struct rq *rq, struct sched_entity *se,
+			   struct task_group *tg, u64 debt)
+{
+	if (!cfs_bandwidth_used())
+		return;
+
+	while (tg != &root_task_group) {
+		struct cfs_rq *cfs_rq = tg->cfs_rq[cpu_of(rq)];
+
+		if (cfs_rq->runtime_enabled) {
+			struct cfs_bandwidth *cfs_b = &tg->cfs_bandwidth;
+			u64 payment;
+
+			raw_spin_lock(&cfs_b->lock);
+
+			payment = min(cfs_b->runtime, debt);
+			cfs_b->runtime -= payment;
+			cfs_b->debt += debt - payment;
+
+			raw_spin_unlock(&cfs_b->lock);
+		}
+
+		tg = tg->parent;
+	}
+}
+
 #else /* CONFIG_CFS_BANDWIDTH */
 
 static inline bool cfs_bandwidth_used(void)
@@ -5448,6 +5483,8 @@ static inline struct cfs_bandwidth *tg_cfs_bandwidth(struct task_group *tg)
 static inline void destroy_cfs_bandwidth(struct cfs_bandwidth *cfs_b) {}
 static inline void update_runtime_enabled(struct rq *rq) {}
 static inline void unthrottle_offline_cfs_rqs(struct rq *rq) {}
+static inline void incur_cfs_debt(struct rq *rq, struct sched_entity *se,
+				  struct task_group *tg, u64 debt) {}
 
 #endif /* CONFIG_CFS_BANDWIDTH */
 
@@ -11452,6 +11489,59 @@ int sched_group_set_shares(struct task_group *tg, unsigned long shares)
 	mutex_unlock(&shares_mutex);
 	return 0;
 }
+
+#define INCUR_DEBT		1
+
+static void cpu_cgroup_remote(struct task_struct *p, struct task_group *tg,
+			      int flags)
+{
+	struct sched_entity *se = &p->se;
+	struct cfs_rq *cfs_rq;
+	struct rq_flags rf;
+	struct rq *rq;
+
+	/*
+	 * User tasks might change task groups between calls to this function,
+	 * which isn't handled for now, so disallow them.
+	 */
+	if (!(p->flags & PF_KTHREAD))
+		return;
+
+	/* kthreads already run in the root, so no need for remote charging. */
+	if (tg == &root_task_group)
+		return;
+
+	rq = task_rq_lock(p, &rf);
+	update_rq_clock(rq);
+
+	cfs_rq = cfs_rq_of(se);
+	update_curr(cfs_rq);
+
+	if (flags & INCUR_DEBT) {
+		u64 debt = se->sum_exec_runtime - se->remote_runtime_begin;
+
+		if (unlikely((s64)debt <= 0))
+			goto out;
+
+		incur_cfs_debt(rq, se, tg, debt);
+	}
+
+out:
+	se->remote_runtime_begin = se->sum_exec_runtime;
+
+	task_rq_unlock(rq, p, &rf);
+}
+
+void cpu_cgroup_remote_begin_fair(struct task_struct *p, struct task_group *tg)
+{
+	cpu_cgroup_remote(p, tg, 0);
+}
+
+void cpu_cgroup_remote_charge_fair(struct task_struct *p, struct task_group *tg)
+{
+	cpu_cgroup_remote(p, tg, INCUR_DEBT);
+}
+
 #else /* CONFIG_FAIR_GROUP_SCHED */
 
 void free_fair_sched_group(struct task_group *tg) { }
diff --git a/kernel/sched/sched.h b/kernel/sched/sched.h
index ddefb0419d7a..75dd6f89e295 100644
--- a/kernel/sched/sched.h
+++ b/kernel/sched/sched.h
@@ -367,6 +367,7 @@ struct cfs_bandwidth {
 	u64			quota;
 	u64			runtime;
 	u64			burst;
+	u64			debt;
 	s64			hierarchical_quota;
 
 	u8			idle;
@@ -472,6 +473,10 @@ extern void free_fair_sched_group(struct task_group *tg);
 extern int alloc_fair_sched_group(struct task_group *tg, struct task_group *parent);
 extern void online_fair_sched_group(struct task_group *tg);
 extern void unregister_fair_sched_group(struct task_group *tg);
+extern void cpu_cgroup_remote_begin_fair(struct task_struct *p,
+					 struct task_group *tg);
+extern void cpu_cgroup_remote_charge_fair(struct task_struct *p,
+					  struct task_group *tg);
 extern void init_tg_cfs_entry(struct task_group *tg, struct cfs_rq *cfs_rq,
 			struct sched_entity *se, int cpu,
 			struct sched_entity *parent);
-- 
2.34.1

Powered by blists - more mailing lists