Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1308545 > unrolled thread

[PATCH 2/4] sched: Consolidate nohz CPU load update code

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  [PATCH 2/4] sched: Consolidate nohz CPU load update code Frederic Weisbecker <fweisbec@gmail.com> - 2016-01-13 17:10 +0100
    Re: [PATCH 2/4] sched: Consolidate nohz CPU load update code Byungchul Park <byungchul.park@lge.com> - 2016-01-14 03:40 +0100
    Re: [PATCH 2/4] sched: Consolidate nohz CPU load update code Byungchul Park <byungchul.park@lge.com> - 2016-01-14 06:20 +0100

#1308545 — [PATCH 2/4] sched: Consolidate nohz CPU load update code

Lets factorize a bit of code there. We'll even have a third user soon.
While at it, standardize the idle update function name against the
others.

Cc: Byungchul Park <byungchul.park@lge.com>
Cc: Mike Galbraith <efault@gmx.de>
Cc: Chris Metcalf <cmetcalf@ezchip.com>
Cc: Christoph Lameter <cl@linux.com>
Cc: Luiz Capitulino <lcapitulino@redhat.com>
Cc: Paul E . McKenney <paulmck@linux.vnet.ibm.com>
Cc: Rik van Riel <riel@redhat.com>
Cc: Thomas Gleixner <tglx@linutronix.de>
Signed-off-by: Frederic Weisbecker <fweisbec@gmail.com>
---
 kernel/sched/fair.c | 48 +++++++++++++++++++++++++-----------------------
 1 file changed, 25 insertions(+), 23 deletions(-)

diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
index b849ea8..161cee2 100644
--- a/kernel/sched/fair.c
+++ b/kernel/sched/fair.c
@@ -4450,6 +4450,25 @@ static unsigned long weighted_cpuload(const int cpu)
 }
 
 #ifdef CONFIG_NO_HZ_COMMON
+static void __update_cpu_load_nohz(struct rq *this_rq,
+				   unsigned long curr_jiffies,
+				   unsigned long load,
+				   int active)
+{
+	unsigned long pending_updates;
+
+	pending_updates = curr_jiffies - this_rq->last_load_update_tick;
+	if (pending_updates) {
+		this_rq->last_load_update_tick = curr_jiffies;
+		/*
+		 * In the regular NOHZ case, we were idle, this means load 0.
+		 * In the NOHZ_FULL case, we were non-idle, we should consider
+		 * its weighted load.
+		 */
+		__update_cpu_load(this_rq, load, pending_updates, active);
+	}
+}
+
 /*
  * There is no sane way to deal with nohz on smp when using jiffies because the
  * cpu doing the jiffies update might drift wrt the cpu doing the jiffy reading
@@ -4467,22 +4486,15 @@ static unsigned long weighted_cpuload(const int cpu)
  * Called from nohz_idle_balance() to update the load ratings before doing the
  * idle balance.
  */
-static void update_idle_cpu_load(struct rq *this_rq)
+static void update_cpu_load_idle(struct rq *this_rq)
 {
-	unsigned long curr_jiffies = READ_ONCE(jiffies);
-	unsigned long load = weighted_cpuload(cpu_of(this_rq));
-	unsigned long pending_updates;
-
 	/*
 	 * bail if there's load or we're actually up-to-date.
 	 */
-	if (load || curr_jiffies == this_rq->last_load_update_tick)
+	if (weighted_cpuload(cpu_of(this_rq)))
 		return;
 
-	pending_updates = curr_jiffies - this_rq->last_load_update_tick;
-	this_rq->last_load_update_tick = curr_jiffies;
-
-	__update_cpu_load(this_rq, load, pending_updates, 0);
+	__update_cpu_load_nohz(this_rq, READ_ONCE(jiffies), 0, 0);
 }
 
 /*
@@ -4493,22 +4505,12 @@ void update_cpu_load_nohz(int active)
 	struct rq *this_rq = this_rq();
 	unsigned long curr_jiffies = READ_ONCE(jiffies);
 	unsigned long load = active ? weighted_cpuload(cpu_of(this_rq)) : 0;
-	unsigned long pending_updates;
 
 	if (curr_jiffies == this_rq->last_load_update_tick)
 		return;
 
 	raw_spin_lock(&this_rq->lock);
-	pending_updates = curr_jiffies - this_rq->last_load_update_tick;
-	if (pending_updates) {
-		this_rq->last_load_update_tick = curr_jiffies;
-		/*
-		 * In the regular NOHZ case, we were idle, this means load 0.
-		 * In the NOHZ_FULL case, we were non-idle, we should consider
-		 * its weighted load.
-		 */
-		__update_cpu_load(this_rq, load, pending_updates, active);
-	}
+	__update_cpu_load_nohz(this_rq, curr_jiffies, load, active);
 	raw_spin_unlock(&this_rq->lock);
 }
 #endif /* CONFIG_NO_HZ */
@@ -4529,7 +4531,7 @@ void update_cpu_load_active(struct rq *this_rq)
 	if (tick_nohz_tick_stopped())
 		return;
 	/*
-	 * See the mess around update_idle_cpu_load() / update_cpu_load_nohz().
+	 * See the mess around update_cpu_load_idle() / update_cpu_load_nohz().
 	 */
 	load = weighted_cpuload(cpu_of(this_rq));
 	this_rq->last_load_update_tick = jiffies;
@@ -7824,7 +7826,7 @@ static void nohz_idle_balance(struct rq *this_rq, enum cpu_idle_type idle)
 		if (time_after_eq(jiffies, rq->next_balance)) {
 			raw_spin_lock_irq(&rq->lock);
 			update_rq_clock(rq);
-			update_idle_cpu_load(rq);
+			update_cpu_load_idle(rq);
 			raw_spin_unlock_irq(&rq->lock);
 			rebalance_domains(rq, CPU_IDLE);
 		}
-- 
2.6.4

[toc] | [next] | [standalone]

#1308966

On Wed, Jan 13, 2016 at 05:01:29PM +0100, Frederic Weisbecker wrote:
> Lets factorize a bit of code there. We'll even have a third user soon.
> While at it, standardize the idle update function name against the
> others.

This factorization was one of what I thought it needs to be done! :-)

> 

[toc] | [prev] | [next] | [standalone]

#1309010

On Wed, Jan 13, 2016 at 05:01:29PM +0100, Frederic Weisbecker wrote:
>  #ifdef CONFIG_NO_HZ_COMMON
> +static void __update_cpu_load_nohz(struct rq *this_rq,
> +				   unsigned long curr_jiffies,

Do we need to pass current jiffies as a function parameter?

> +				   unsigned long load,
> +				   int active)
> +{
> +	unsigned long pending_updates;
> +
> +	pending_updates = curr_jiffies - this_rq->last_load_update_tick;
> +	if (pending_updates) {
> +		this_rq->last_load_update_tick = curr_jiffies;
> +		/*
> +		 * In the regular NOHZ case, we were idle, this means load 0.
> +		 * In the NOHZ_FULL case, we were non-idle, we should consider
> +		 * its weighted load.
> +		 */
> +		__update_cpu_load(this_rq, load, pending_updates, active);
> +	}
> +}
> +
>  /*
>   * There is no sane way to deal with nohz on smp when using jiffies because the
>   * cpu doing the jiffies update might drift wrt the cpu doing the jiffy reading
> @@ -4467,22 +4486,15 @@ static unsigned long weighted_cpuload(const int cpu)
>   * Called from nohz_idle_balance() to update the load ratings before doing the
>   * idle balance.
>   */
> -static void update_idle_cpu_load(struct rq *this_rq)
> +static void update_cpu_load_idle(struct rq *this_rq)
>  {
> -	unsigned long curr_jiffies = READ_ONCE(jiffies);
> -	unsigned long load = weighted_cpuload(cpu_of(this_rq));
> -	unsigned long pending_updates;
> -
>  	/*
>  	 * bail if there's load or we're actually up-to-date.
>  	 */
> -	if (load || curr_jiffies == this_rq->last_load_update_tick)
> +	if (weighted_cpuload(cpu_of(this_rq)))
>  		return;
>  
> -	pending_updates = curr_jiffies - this_rq->last_load_update_tick;
> -	this_rq->last_load_update_tick = curr_jiffies;
> -
> -	__update_cpu_load(this_rq, load, pending_updates, 0);
> +	__update_cpu_load_nohz(this_rq, READ_ONCE(jiffies), 0, 0);

This question is not directly related to this patch but I am just
curious about... Should we use READ_ONCE on jiffies which is already
volatile type?

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web