Noticed a few minor details while I was reading this code:
Ingo Molnar (3):
sched/nohz: Update idle load-balancing (ILB) comments
sched/nohz: Use consistent variable names in find_new_ilb() and kick_ilb()
sched/nohz: Remove weird error handling from find_new_ilb()
kernel/sched/fair.c | 45 +++++++++++++++++++++++----------------------
1 file changed, 23 insertions(+), 22 deletions(-)
--
2.39.2
Use 'ilb_cpu' consistently in both functions.
Signed-off-by: Ingo Molnar <[email protected]>
Cc: [email protected]
---
kernel/sched/fair.c | 10 +++++-----
1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
index 8435179779e3..d4e90d15bd77 100644
--- a/kernel/sched/fair.c
+++ b/kernel/sched/fair.c
@@ -11558,18 +11558,18 @@ static inline int on_null_domain(struct rq *rq)
*/
static inline int find_new_ilb(void)
{
- int ilb;
const struct cpumask *hk_mask;
+ int ilb_cpu;
hk_mask = housekeeping_cpumask(HK_TYPE_MISC);
- for_each_cpu_and(ilb, nohz.idle_cpus_mask, hk_mask) {
+ for_each_cpu_and(ilb_cpu, nohz.idle_cpus_mask, hk_mask) {
- if (ilb == smp_processor_id())
+ if (ilb_cpu == smp_processor_id())
continue;
- if (idle_cpu(ilb))
- return ilb;
+ if (idle_cpu(ilb_cpu))
+ return ilb_cpu;
}
return nr_cpu_ids;
--
2.39.2
- Fix incorrect/misleading comments,
- clarify some others,
- fix typos & grammar,
- and use more consistent style throughout.
Signed-off-by: Ingo Molnar <[email protected]>
Cc: [email protected]
---
kernel/sched/fair.c | 30 ++++++++++++++++--------------
1 file changed, 16 insertions(+), 14 deletions(-)
diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
index 04fbcbda97d5..8435179779e3 100644
--- a/kernel/sched/fair.c
+++ b/kernel/sched/fair.c
@@ -11547,14 +11547,15 @@ static inline int on_null_domain(struct rq *rq)
#ifdef CONFIG_NO_HZ_COMMON
/*
- * idle load balancing details
- * - When one of the busy CPUs notice that there may be an idle rebalancing
+ * NOHZ idle load balancing (ILB) details:
+ *
+ * - When one of the busy CPUs notices that there may be an idle rebalancing
* needed, they will kick the idle load balancer, which then does idle
* load balancing for all the idle CPUs.
- * - HK_TYPE_MISC CPUs are used for this task, because HK_TYPE_SCHED not set
+ *
+ * - HK_TYPE_MISC CPUs are used for this task, because HK_TYPE_SCHED is not set
* anywhere yet.
*/
-
static inline int find_new_ilb(void)
{
int ilb;
@@ -11575,8 +11576,10 @@ static inline int find_new_ilb(void)
}
/*
- * Kick a CPU to do the nohz balancing, if it is time for it. We pick any
- * idle CPU in the HK_TYPE_MISC housekeeping set (if there is one).
+ * Kick a CPU to do the NOHZ balancing, if it is time for it, via a cross-CPU
+ * SMP function call (IPI).
+ *
+ * We pick the first idle CPU in the HK_TYPE_MISC housekeeping set (if there is one).
*/
static void kick_ilb(unsigned int flags)
{
@@ -11604,7 +11607,7 @@ static void kick_ilb(unsigned int flags)
/*
* This way we generate an IPI on the target CPU which
- * is idle. And the softirq performing nohz idle load balance
+ * is idle, and the softirq performing NOHZ idle load balancing
* will be run before returning from the IPI.
*/
smp_call_function_single_async(ilb_cpu, &cpu_rq(ilb_cpu)->nohz_csd);
@@ -11633,7 +11636,7 @@ static void nohz_balancer_kick(struct rq *rq)
/*
* None are in tickless mode and hence no need for NOHZ idle load
- * balancing.
+ * balancing:
*/
if (likely(!atomic_read(&nohz.nr_cpus)))
return;
@@ -11655,9 +11658,8 @@ static void nohz_balancer_kick(struct rq *rq)
sd = rcu_dereference(rq->sd);
if (sd) {
/*
- * If there's a CFS task and the current CPU has reduced
- * capacity; kick the ILB to see if there's a better CPU to run
- * on.
+ * If there's a runnable CFS task and the current CPU has reduced
+ * capacity, kick the ILB to see if there's a better CPU to run on:
*/
if (rq->cfs.h_nr_running >= 1 && check_cpu_capacity(rq, sd)) {
flags = NOHZ_STATS_KICK | NOHZ_BALANCE_KICK;
@@ -11709,11 +11711,11 @@ static void nohz_balancer_kick(struct rq *rq)
if (sds) {
/*
* If there is an imbalance between LLC domains (IOW we could
- * increase the overall cache use), we need some less-loaded LLC
- * domain to pull some load. Likewise, we may need to spread
+ * increase the overall cache utilization), we need a less-loaded LLC
+ * domain to pull some load from. Likewise, we may need to spread
* load within the current LLC domain (e.g. packed SMT cores but
* other CPUs are idle). We can't really know from here how busy
- * the others are - so just get a nohz balance going if it looks
+ * the others are - so just get a NOHZ balance going if it looks
* like this LLC domain has tasks we could move.
*/
nr_busy = atomic_read(&sds->nr_busy_cpus);
--
2.39.2
On Fri, Oct 06, 2023 at 12:25:15PM +0200, Ingo Molnar wrote:
> Noticed a few minor details while I was reading this code:
>
> Ingo Molnar (3):
> sched/nohz: Update idle load-balancing (ILB) comments
> sched/nohz: Use consistent variable names in find_new_ilb() and kick_ilb()
> sched/nohz: Remove weird error handling from find_new_ilb()
Reviewed-by: Joel Fernandes (Google) <[email protected]>
thanks,
- Joel
>
> kernel/sched/fair.c | 45 +++++++++++++++++++++++----------------------
> 1 file changed, 23 insertions(+), 22 deletions(-)
>
> --
> 2.39.2
>
The following commit has been merged into the sched/core branch of tip:
Commit-ID: b6dd6984832a2868f78879fce30d6965ae899d02
Gitweb: https://git.kernel.org/tip/b6dd6984832a2868f78879fce30d6965ae899d02
Author: Ingo Molnar <[email protected]>
AuthorDate: Fri, 06 Oct 2023 12:25:17 +02:00
Committer: Ingo Molnar <[email protected]>
CommitterDate: Mon, 09 Oct 2023 12:21:23 +02:00
sched/nohz: Use consistent variable names in find_new_ilb() and kick_ilb()
Use 'ilb_cpu' consistently in both functions.
Signed-off-by: Ingo Molnar <[email protected]>
Reviewed-by: Joel Fernandes (Google) <[email protected]>
Link: https://lore.kernel.org/r/[email protected]
---
kernel/sched/fair.c | 10 +++++-----
1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
index 2b63a14..f82b301 100644
--- a/kernel/sched/fair.c
+++ b/kernel/sched/fair.c
@@ -11514,18 +11514,18 @@ static inline int on_null_domain(struct rq *rq)
*/
static inline int find_new_ilb(void)
{
- int ilb;
const struct cpumask *hk_mask;
+ int ilb_cpu;
hk_mask = housekeeping_cpumask(HK_TYPE_MISC);
- for_each_cpu_and(ilb, nohz.idle_cpus_mask, hk_mask) {
+ for_each_cpu_and(ilb_cpu, nohz.idle_cpus_mask, hk_mask) {
- if (ilb == smp_processor_id())
+ if (ilb_cpu == smp_processor_id())
continue;
- if (idle_cpu(ilb))
- return ilb;
+ if (idle_cpu(ilb_cpu))
+ return ilb_cpu;
}
return nr_cpu_ids;
The following commit has been merged into the sched/core branch of tip:
Commit-ID: 7ef7145a2b26b172ac6885c4cf3272a38bc0979a
Gitweb: https://git.kernel.org/tip/7ef7145a2b26b172ac6885c4cf3272a38bc0979a
Author: Ingo Molnar <[email protected]>
AuthorDate: Fri, 06 Oct 2023 12:25:16 +02:00
Committer: Ingo Molnar <[email protected]>
CommitterDate: Mon, 09 Oct 2023 12:21:23 +02:00
sched/nohz: Update idle load-balancing (ILB) comments
- Fix incorrect/misleading comments,
- clarify some others,
- fix typos & grammar,
- and use more consistent style throughout.
Signed-off-by: Ingo Molnar <[email protected]>
Reviewed-by: Joel Fernandes (Google) <[email protected]>
Link: https://lore.kernel.org/r/[email protected]
---
kernel/sched/fair.c | 30 ++++++++++++++++--------------
1 file changed, 16 insertions(+), 14 deletions(-)
diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
index 52c498f..2b63a14 100644
--- a/kernel/sched/fair.c
+++ b/kernel/sched/fair.c
@@ -11503,14 +11503,15 @@ static inline int on_null_domain(struct rq *rq)
#ifdef CONFIG_NO_HZ_COMMON
/*
- * idle load balancing details
- * - When one of the busy CPUs notice that there may be an idle rebalancing
+ * NOHZ idle load balancing (ILB) details:
+ *
+ * - When one of the busy CPUs notices that there may be an idle rebalancing
* needed, they will kick the idle load balancer, which then does idle
* load balancing for all the idle CPUs.
- * - HK_TYPE_MISC CPUs are used for this task, because HK_TYPE_SCHED not set
+ *
+ * - HK_TYPE_MISC CPUs are used for this task, because HK_TYPE_SCHED is not set
* anywhere yet.
*/
-
static inline int find_new_ilb(void)
{
int ilb;
@@ -11531,8 +11532,10 @@ static inline int find_new_ilb(void)
}
/*
- * Kick a CPU to do the nohz balancing, if it is time for it. We pick any
- * idle CPU in the HK_TYPE_MISC housekeeping set (if there is one).
+ * Kick a CPU to do the NOHZ balancing, if it is time for it, via a cross-CPU
+ * SMP function call (IPI).
+ *
+ * We pick the first idle CPU in the HK_TYPE_MISC housekeeping set (if there is one).
*/
static void kick_ilb(unsigned int flags)
{
@@ -11560,7 +11563,7 @@ static void kick_ilb(unsigned int flags)
/*
* This way we generate an IPI on the target CPU which
- * is idle. And the softirq performing nohz idle load balance
+ * is idle, and the softirq performing NOHZ idle load balancing
* will be run before returning from the IPI.
*/
smp_call_function_single_async(ilb_cpu, &cpu_rq(ilb_cpu)->nohz_csd);
@@ -11589,7 +11592,7 @@ static void nohz_balancer_kick(struct rq *rq)
/*
* None are in tickless mode and hence no need for NOHZ idle load
- * balancing.
+ * balancing:
*/
if (likely(!atomic_read(&nohz.nr_cpus)))
return;
@@ -11611,9 +11614,8 @@ static void nohz_balancer_kick(struct rq *rq)
sd = rcu_dereference(rq->sd);
if (sd) {
/*
- * If there's a CFS task and the current CPU has reduced
- * capacity; kick the ILB to see if there's a better CPU to run
- * on.
+ * If there's a runnable CFS task and the current CPU has reduced
+ * capacity, kick the ILB to see if there's a better CPU to run on:
*/
if (rq->cfs.h_nr_running >= 1 && check_cpu_capacity(rq, sd)) {
flags = NOHZ_STATS_KICK | NOHZ_BALANCE_KICK;
@@ -11665,11 +11667,11 @@ static void nohz_balancer_kick(struct rq *rq)
if (sds) {
/*
* If there is an imbalance between LLC domains (IOW we could
- * increase the overall cache use), we need some less-loaded LLC
- * domain to pull some load. Likewise, we may need to spread
+ * increase the overall cache utilization), we need a less-loaded LLC
+ * domain to pull some load from. Likewise, we may need to spread
* load within the current LLC domain (e.g. packed SMT cores but
* other CPUs are idle). We can't really know from here how busy
- * the others are - so just get a nohz balance going if it looks
+ * the others are - so just get a NOHZ balance going if it looks
* like this LLC domain has tasks we could move.
*/
nr_busy = atomic_read(&sds->nr_busy_cpus);
On 06/10/23 12:25, Ingo Molnar wrote:
> - Fix incorrect/misleading comments,
>
> - clarify some others,
>
> - fix typos & grammar,
>
> - and use more consistent style throughout.
>
> Signed-off-by: Ingo Molnar <[email protected]>
> Cc: [email protected]
Reviewed-by: Valentin Schneider <[email protected]>
On 06/10/23 12:25, Ingo Molnar wrote:
> Use 'ilb_cpu' consistently in both functions.
>
> Signed-off-by: Ingo Molnar <[email protected]>
> Cc: [email protected]
Reviewed-by: Valentin Schneider <[email protected]>