[PATCH 1/2] sched/core: Introduce sched_class::can_stop_tick()

Hao Jia posted 2 patches 2 years, 3 months ago
[PATCH 1/2] sched/core: Introduce sched_class::can_stop_tick()
Posted by Hao Jia 2 years, 3 months ago
Extract a can_stop_tick() callback function for each
sched_class from sched_can_stop_tick(). It will
clean up some checks about cfs_bandwidth in sched/core.c.
Put these checks into their own sched_class,
and make some functions static.

This also makes it easier for us to deal with
"nohz_full vs rt_bandwidth" case later.

No functional changes.

Signed-off-by: Hao Jia <jiahao.os@bytedance.com>
---
 kernel/sched/core.c     | 67 ++++++-----------------------------------
 kernel/sched/deadline.c | 16 ++++++++++
 kernel/sched/fair.c     | 56 +++++++++++++++++++++++++++++++---
 kernel/sched/rt.c       | 34 +++++++++++++++++++++
 kernel/sched/sched.h    |  5 +--
 5 files changed, 114 insertions(+), 64 deletions(-)

diff --git a/kernel/sched/core.c b/kernel/sched/core.c
index efe3848978a0..1107ce6e4f6c 100644
--- a/kernel/sched/core.c
+++ b/kernel/sched/core.c
@@ -1191,68 +1191,21 @@ static void nohz_csd_func(void *info)
 #endif /* CONFIG_NO_HZ_COMMON */
 
 #ifdef CONFIG_NO_HZ_FULL
-static inline bool __need_bw_check(struct rq *rq, struct task_struct *p)
-{
-	if (rq->nr_running != 1)
-		return false;
-
-	if (p->sched_class != &fair_sched_class)
-		return false;
-
-	if (!task_on_rq_queued(p))
-		return false;
-
-	return true;
-}
-
 bool sched_can_stop_tick(struct rq *rq)
 {
-	int fifo_nr_running;
-
-	/* Deadline tasks, even if single, need the tick */
-	if (rq->dl.dl_nr_running)
-		return false;
-
-	/*
-	 * If there are more than one RR tasks, we need the tick to affect the
-	 * actual RR behaviour.
-	 */
-	if (rq->rt.rr_nr_running) {
-		if (rq->rt.rr_nr_running == 1)
-			return true;
-		else
-			return false;
-	}
-
-	/*
-	 * If there's no RR tasks, but FIFO tasks, we can skip the tick, no
-	 * forced preemption between FIFO tasks.
-	 */
-	fifo_nr_running = rq->rt.rt_nr_running - rq->rt.rr_nr_running;
-	if (fifo_nr_running)
-		return true;
-
-	/*
-	 * If there are no DL,RR/FIFO tasks, there must only be CFS tasks left;
-	 * if there's more than one we need the tick for involuntary
-	 * preemption.
-	 */
-	if (rq->nr_running > 1)
-		return false;
+	const struct sched_class *class;
+	int stop_next = 0;
+	bool ret = true;
 
-	/*
-	 * If there is one task and it has CFS runtime bandwidth constraints
-	 * and it's on the cpu now we don't want to stop the tick.
-	 * This check prevents clearing the bit if a newly enqueued task here is
-	 * dequeued by migrating while the constrained task continues to run.
-	 * E.g. going from 2->1 without going through pick_next_task().
-	 */
-	if (sched_feat(HZ_BW) && __need_bw_check(rq, rq->curr)) {
-		if (cfs_task_bw_constrained(rq->curr))
-			return false;
+	for_each_class(class) {
+		if (class->can_stop_tick) {
+			ret = class->can_stop_tick(rq, &stop_next);
+			if (stop_next)
+				break;
+		}
 	}
 
-	return true;
+	return ret;
 }
 #endif /* CONFIG_NO_HZ_FULL */
 #endif /* CONFIG_SMP */
diff --git a/kernel/sched/deadline.c b/kernel/sched/deadline.c
index 58b542bf2893..0b461cb40408 100644
--- a/kernel/sched/deadline.c
+++ b/kernel/sched/deadline.c
@@ -2715,6 +2715,19 @@ static int task_is_throttled_dl(struct task_struct *p, int cpu)
 }
 #endif
 
+#ifdef CONFIG_NO_HZ_FULL
+static bool can_stop_tick_dl(struct rq *rq, int *stop_next)
+{
+	/* Deadline tasks, even if single, need the tick */
+	if (rq->dl.dl_nr_running) {
+		*stop_next = 1;
+		return false;
+	}
+
+	return true;
+}
+#endif
+
 DEFINE_SCHED_CLASS(dl) = {
 
 	.enqueue_task		= enqueue_task_dl,
@@ -2750,6 +2763,9 @@ DEFINE_SCHED_CLASS(dl) = {
 #ifdef CONFIG_SCHED_CORE
 	.task_is_throttled	= task_is_throttled_dl,
 #endif
+#ifdef CONFIG_NO_HZ_FULL
+	.can_stop_tick		= can_stop_tick_dl,
+#endif
 };
 
 /* Used for dl_bw check and update, used under sched_rt_handler()::mutex */
diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
index 128a78f3f264..7fa4892267f1 100644
--- a/kernel/sched/fair.c
+++ b/kernel/sched/fair.c
@@ -6267,7 +6267,8 @@ static void __maybe_unused unthrottle_offline_cfs_rqs(struct rq *rq)
 	rq_clock_stop_loop_update(rq);
 }
 
-bool cfs_task_bw_constrained(struct task_struct *p)
+#ifdef CONFIG_NO_HZ_FULL
+static inline bool cfs_task_bw_constrained(struct task_struct *p)
 {
 	struct cfs_rq *cfs_rq = task_cfs_rq(p);
 
@@ -6281,7 +6282,6 @@ bool cfs_task_bw_constrained(struct task_struct *p)
 	return false;
 }
 
-#ifdef CONFIG_NO_HZ_FULL
 /* called from pick_next_task_fair() */
 static void sched_fair_update_stop_tick(struct rq *rq, struct task_struct *p)
 {
@@ -6305,6 +6305,44 @@ static void sched_fair_update_stop_tick(struct rq *rq, struct task_struct *p)
 	if (cfs_task_bw_constrained(p))
 		tick_nohz_dep_set_cpu(cpu, TICK_DEP_BIT_SCHED);
 }
+
+static inline bool __need_bw_check(struct rq *rq, struct task_struct *p)
+{
+	if (rq->nr_running != 1)
+		return false;
+
+	if (p->sched_class != &fair_sched_class)
+		return false;
+
+	if (!task_on_rq_queued(p))
+		return false;
+
+	return true;
+}
+
+static bool can_stop_tick_fair(struct rq *rq, int *stop_next)
+{
+	if (rq->nr_running > 1) {
+		*stop_next = 1;
+		return false;
+	}
+
+	/*
+	 * If there is one task and it has CFS runtime bandwidth constraints
+	 * and it's on the cpu now we don't want to stop the tick.
+	 * This check prevents clearing the bit if a newly enqueued task here is
+	 * dequeued by migrating while the constrained task continues to run.
+	 * E.g. going from 2->1 without going through pick_next_task().
+	 */
+	if (sched_feat(HZ_BW) && __need_bw_check(rq, rq->curr)) {
+		if (cfs_task_bw_constrained(rq->curr)) {
+			*stop_next = 1;
+			return false;
+		}
+	}
+
+	return true;
+}
 #endif
 
 #else /* CONFIG_CFS_BANDWIDTH */
@@ -6348,10 +6386,15 @@ 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) {}
-#ifdef CONFIG_CGROUP_SCHED
-bool cfs_task_bw_constrained(struct task_struct *p)
+#ifdef CONFIG_NO_HZ_FULL
+static bool can_stop_tick_fair(struct rq *rq, int *stop_next)
 {
-	return false;
+	if (rq->nr_running > 1) {
+		*stop_next = 1;
+		return false;
+	}
+
+	return true;
 }
 #endif
 #endif /* CONFIG_CFS_BANDWIDTH */
@@ -12864,6 +12907,9 @@ DEFINE_SCHED_CLASS(fair) = {
 #ifdef CONFIG_SCHED_CORE
 	.task_is_throttled	= task_is_throttled_fair,
 #endif
+#ifdef CONFIG_NO_HZ_FULL
+	.can_stop_tick		= can_stop_tick_fair,
+#endif
 
 #ifdef CONFIG_UCLAMP_TASK
 	.uclamp_enabled		= 1,
diff --git a/kernel/sched/rt.c b/kernel/sched/rt.c
index 0597ba0f85ff..0b9e9467ef61 100644
--- a/kernel/sched/rt.c
+++ b/kernel/sched/rt.c
@@ -1740,6 +1740,37 @@ static void check_preempt_curr_rt(struct rq *rq, struct task_struct *p, int flag
 #endif
 }
 
+#ifdef CONFIG_NO_HZ_FULL
+static bool can_stop_tick_rt(struct rq *rq, int *stop_next)
+{
+	int fifo_nr_running;
+
+	/*
+	 * If there are more than one RR tasks, we need the tick to affect the
+	 * actual RR behaviour.
+	 */
+	if (rq->rt.rr_nr_running) {
+		*stop_next = 1;
+		if (rq->rt.rr_nr_running == 1)
+			return true;
+		else
+			return false;
+	}
+
+	/*
+	 * If there's no RR tasks, but FIFO tasks, we can skip the tick, no
+	 * forced preemption between FIFO tasks.
+	 */
+	fifo_nr_running = rq->rt.rt_nr_running - rq->rt.rr_nr_running;
+	if (fifo_nr_running) {
+		*stop_next = 1;
+		return true;
+	}
+
+	return true;
+}
+#endif
+
 static inline void set_next_task_rt(struct rq *rq, struct task_struct *p, bool first)
 {
 	struct sched_rt_entity *rt_se = &p->rt;
@@ -2732,6 +2763,9 @@ DEFINE_SCHED_CLASS(rt) = {
 #ifdef CONFIG_SCHED_CORE
 	.task_is_throttled	= task_is_throttled_rt,
 #endif
+#ifdef CONFIG_NO_HZ_FULL
+	.can_stop_tick		= can_stop_tick_rt,
+#endif
 
 #ifdef CONFIG_UCLAMP_TASK
 	.uclamp_enabled		= 1,
diff --git a/kernel/sched/sched.h b/kernel/sched/sched.h
index 4d4b6f178e99..f464e7fff0ef 100644
--- a/kernel/sched/sched.h
+++ b/kernel/sched/sched.h
@@ -459,7 +459,6 @@ extern void init_cfs_bandwidth(struct cfs_bandwidth *cfs_b, struct cfs_bandwidth
 extern void __refill_cfs_bandwidth_runtime(struct cfs_bandwidth *cfs_b);
 extern void start_cfs_bandwidth(struct cfs_bandwidth *cfs_b);
 extern void unthrottle_cfs_rq(struct cfs_rq *cfs_rq);
-extern bool cfs_task_bw_constrained(struct task_struct *p);
 
 extern void init_tg_rt_entry(struct task_group *tg, struct rt_rq *rt_rq,
 		struct sched_rt_entity *rt_se, int cpu,
@@ -495,7 +494,6 @@ static inline void set_task_rq_fair(struct sched_entity *se,
 #else /* CONFIG_CGROUP_SCHED */
 
 struct cfs_bandwidth { };
-static inline bool cfs_task_bw_constrained(struct task_struct *p) { return false; }
 
 #endif	/* CONFIG_CGROUP_SCHED */
 
@@ -2289,6 +2287,9 @@ struct sched_class {
 #ifdef CONFIG_SCHED_CORE
 	int (*task_is_throttled)(struct task_struct *p, int cpu);
 #endif
+#ifdef CONFIG_NO_HZ_FULL
+	bool (*can_stop_tick)(struct rq *rq, int *stop_next);
+#endif
 };
 
 static inline void put_prev_task(struct rq *rq, struct task_struct *prev)
-- 
2.39.2
Re: [PATCH 1/2] sched/core: Introduce sched_class::can_stop_tick()
Posted by kernel test robot 2 years, 3 months ago
Hi Hao,

kernel test robot noticed the following build warnings:

[auto build test WARNING on tip/sched/core]
[also build test WARNING on next-20230824]
[cannot apply to linus/master v6.5-rc7]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Hao-Jia/sched-core-Introduce-sched_class-can_stop_tick/20230821-175200
base:   tip/sched/core
patch link:    https://lore.kernel.org/r/20230821094927.51079-2-jiahao.os%40bytedance.com
patch subject: [PATCH 1/2] sched/core: Introduce sched_class::can_stop_tick()
config: arm64-randconfig-r122-20230824 (https://download.01.org/0day-ci/archive/20230824/202308241526.be4l9ROa-lkp@intel.com/config)
compiler: aarch64-linux-gcc (GCC) 12.3.0
reproduce: (https://download.01.org/0day-ci/archive/20230824/202308241526.be4l9ROa-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202308241526.be4l9ROa-lkp@intel.com/

sparse warnings: (new ones prefixed by >>)
   kernel/sched/fair.c:702:6: sparse: sparse: symbol 'update_entity_lag' was not declared. Should it be static?
   kernel/sched/fair.c:1140:34: sparse: sparse: incorrect type in argument 1 (different address spaces) @@     expected struct sched_entity const *se @@     got struct sched_entity [noderef] __rcu * @@
   kernel/sched/fair.c:1140:34: sparse:     expected struct sched_entity const *se
   kernel/sched/fair.c:1140:34: sparse:     got struct sched_entity [noderef] __rcu *
   kernel/sched/fair.c:12087:9: sparse: sparse: incorrect type in assignment (different address spaces) @@     expected struct sched_domain *[assigned] sd @@     got struct sched_domain [noderef] __rcu *parent @@
   kernel/sched/fair.c:12087:9: sparse:     expected struct sched_domain *[assigned] sd
   kernel/sched/fair.c:12087:9: sparse:     got struct sched_domain [noderef] __rcu *parent
   kernel/sched/fair.c:5674:22: sparse: sparse: incompatible types in comparison expression (different address spaces):
   kernel/sched/fair.c:5674:22: sparse:    struct task_struct [noderef] __rcu *
   kernel/sched/fair.c:5674:22: sparse:    struct task_struct *
>> kernel/sched/fair.c:6336:56: sparse: sparse: incorrect type in argument 2 (different address spaces) @@     expected struct task_struct *p @@     got struct task_struct [noderef] __rcu *curr @@
   kernel/sched/fair.c:6336:56: sparse:     expected struct task_struct *p
   kernel/sched/fair.c:6336:56: sparse:     got struct task_struct [noderef] __rcu *curr
   kernel/sched/fair.c:6337:47: sparse: sparse: incorrect type in argument 1 (different address spaces) @@     expected struct task_struct *p @@     got struct task_struct [noderef] __rcu *curr @@
   kernel/sched/fair.c:6337:47: sparse:     expected struct task_struct *p
   kernel/sched/fair.c:6337:47: sparse:     got struct task_struct [noderef] __rcu *curr
   kernel/sched/fair.c:6438:38: sparse: sparse: incorrect type in initializer (different address spaces) @@     expected struct task_struct *curr @@     got struct task_struct [noderef] __rcu *curr @@
   kernel/sched/fair.c:6438:38: sparse:     expected struct task_struct *curr
   kernel/sched/fair.c:6438:38: sparse:     got struct task_struct [noderef] __rcu *curr
   kernel/sched/fair.c:7738:20: sparse: sparse: incorrect type in assignment (different address spaces) @@     expected struct sched_domain *[assigned] sd @@     got struct sched_domain [noderef] __rcu *parent @@
   kernel/sched/fair.c:7738:20: sparse:     expected struct sched_domain *[assigned] sd
   kernel/sched/fair.c:7738:20: sparse:     got struct sched_domain [noderef] __rcu *parent
   kernel/sched/fair.c:7940:9: sparse: sparse: incorrect type in assignment (different address spaces) @@     expected struct sched_domain *[assigned] tmp @@     got struct sched_domain [noderef] __rcu *parent @@
   kernel/sched/fair.c:7940:9: sparse:     expected struct sched_domain *[assigned] tmp
   kernel/sched/fair.c:7940:9: sparse:     got struct sched_domain [noderef] __rcu *parent
   kernel/sched/fair.c:8039:38: sparse: sparse: incorrect type in initializer (different address spaces) @@     expected struct task_struct *curr @@     got struct task_struct [noderef] __rcu *curr @@
   kernel/sched/fair.c:8039:38: sparse:     expected struct task_struct *curr
   kernel/sched/fair.c:8039:38: sparse:     got struct task_struct [noderef] __rcu *curr
   kernel/sched/fair.c:8319:38: sparse: sparse: incorrect type in initializer (different address spaces) @@     expected struct task_struct *curr @@     got struct task_struct [noderef] __rcu *curr @@
   kernel/sched/fair.c:8319:38: sparse:     expected struct task_struct *curr
   kernel/sched/fair.c:8319:38: sparse:     got struct task_struct [noderef] __rcu *curr
   kernel/sched/fair.c:9312:40: sparse: sparse: incorrect type in initializer (different address spaces) @@     expected struct sched_domain *child @@     got struct sched_domain [noderef] __rcu *child @@
   kernel/sched/fair.c:9312:40: sparse:     expected struct sched_domain *child
   kernel/sched/fair.c:9312:40: sparse:     got struct sched_domain [noderef] __rcu *child
   kernel/sched/fair.c:9939:22: sparse: sparse: incompatible types in comparison expression (different address spaces):
   kernel/sched/fair.c:9939:22: sparse:    struct task_struct [noderef] __rcu *
   kernel/sched/fair.c:9939:22: sparse:    struct task_struct *
   kernel/sched/fair.c:11359:9: sparse: sparse: incorrect type in assignment (different address spaces) @@     expected struct sched_domain *[assigned] sd @@     got struct sched_domain [noderef] __rcu *parent @@
   kernel/sched/fair.c:11359:9: sparse:     expected struct sched_domain *[assigned] sd
   kernel/sched/fair.c:11359:9: sparse:     got struct sched_domain [noderef] __rcu *parent
   kernel/sched/fair.c:11018:44: sparse: sparse: incorrect type in initializer (different address spaces) @@     expected struct sched_domain *sd_parent @@     got struct sched_domain [noderef] __rcu *parent @@
   kernel/sched/fair.c:11018:44: sparse:     expected struct sched_domain *sd_parent
   kernel/sched/fair.c:11018:44: sparse:     got struct sched_domain [noderef] __rcu *parent
   kernel/sched/fair.c:11455:9: sparse: sparse: incorrect type in assignment (different address spaces) @@     expected struct sched_domain *[assigned] sd @@     got struct sched_domain [noderef] __rcu *parent @@
   kernel/sched/fair.c:11455:9: sparse:     expected struct sched_domain *[assigned] sd
   kernel/sched/fair.c:11455:9: sparse:     got struct sched_domain [noderef] __rcu *parent
   kernel/sched/fair.c: note: in included file:
   kernel/sched/sched.h:2130:25: sparse: sparse: incompatible types in comparison expression (different address spaces):
   kernel/sched/sched.h:2130:25: sparse:    struct task_struct [noderef] __rcu *
   kernel/sched/sched.h:2130:25: sparse:    struct task_struct *
   kernel/sched/sched.h:2296:9: sparse: sparse: incompatible types in comparison expression (different address spaces):
   kernel/sched/sched.h:2296:9: sparse:    struct task_struct [noderef] __rcu *
   kernel/sched/sched.h:2296:9: sparse:    struct task_struct *
   kernel/sched/sched.h:2130:25: sparse: sparse: incompatible types in comparison expression (different address spaces):
   kernel/sched/sched.h:2130:25: sparse:    struct task_struct [noderef] __rcu *
   kernel/sched/sched.h:2130:25: sparse:    struct task_struct *
   kernel/sched/sched.h:2130:25: sparse: sparse: incompatible types in comparison expression (different address spaces):
   kernel/sched/sched.h:2130:25: sparse:    struct task_struct [noderef] __rcu *
   kernel/sched/sched.h:2130:25: sparse:    struct task_struct *

vim +6336 kernel/sched/fair.c

  6321	
  6322	static bool can_stop_tick_fair(struct rq *rq, int *stop_next)
  6323	{
  6324		if (rq->nr_running > 1) {
  6325			*stop_next = 1;
  6326			return false;
  6327		}
  6328	
  6329		/*
  6330		 * If there is one task and it has CFS runtime bandwidth constraints
  6331		 * and it's on the cpu now we don't want to stop the tick.
  6332		 * This check prevents clearing the bit if a newly enqueued task here is
  6333		 * dequeued by migrating while the constrained task continues to run.
  6334		 * E.g. going from 2->1 without going through pick_next_task().
  6335		 */
> 6336		if (sched_feat(HZ_BW) && __need_bw_check(rq, rq->curr)) {
  6337			if (cfs_task_bw_constrained(rq->curr)) {
  6338				*stop_next = 1;
  6339				return false;
  6340			}
  6341		}
  6342	
  6343		return true;
  6344	}
  6345	#endif
  6346	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki