[SRU,Xenial] sched/cpuacct: Simplify the cpuacct code

Message ID 1528340317-5251-2-git-send-email-gavin.guo@canonical.com
State New
Headers show
Series
  • [SRU,Xenial] sched/cpuacct: Simplify the cpuacct code
Related show

Commit Message

Gavin Guo June 7, 2018, 2:58 a.m.
From: Zhao Lei <zhaolei@cn.fujitsu.com>

BugLink: https://bugs.launchpad.net/bugs/1775326

 - Use for() instead of while() loop in some functions
   to make the code simpler.

 - Use this_cpu_ptr() instead of per_cpu_ptr() to make the code
   cleaner and a bit faster.

Suggested-by: Peter Zijlstra <peterz@infradead.org>
Signed-off-by: Zhao Lei <zhaolei@cn.fujitsu.com>
Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Tejun Heo <htejun@gmail.com>
Cc: Thomas Gleixner <tglx@linutronix.de>
Link: http://lkml.kernel.org/r/d8a7ef9592f55224630cb26dea239f05b6398a4e.1458187654.git.zhaolei@cn.fujitsu.com
Signed-off-by: Ingo Molnar <mingo@kernel.org>
(cherry picked from commit 73e6aafd9ea81498d31361f01db84a0118da2d1c)
Signed-off-by: Gavin Guo <gavin.guo@canonical.com>
---
 kernel/sched/cpuacct.c | 28 +++++-----------------------
 kernel/sched/cpuacct.h |  4 ++--
 2 files changed, 7 insertions(+), 25 deletions(-)

Comments

Kleber Souza June 7, 2018, 7:34 p.m. | #1
On 06/06/18 19:58, Gavin Guo wrote:
> From: Zhao Lei <zhaolei@cn.fujitsu.com>
> 
> BugLink: https://bugs.launchpad.net/bugs/1775326
> 
>  - Use for() instead of while() loop in some functions
>    to make the code simpler.
> 
>  - Use this_cpu_ptr() instead of per_cpu_ptr() to make the code
>    cleaner and a bit faster.
> 
> Suggested-by: Peter Zijlstra <peterz@infradead.org>
> Signed-off-by: Zhao Lei <zhaolei@cn.fujitsu.com>
> Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
> Cc: Linus Torvalds <torvalds@linux-foundation.org>
> Cc: Tejun Heo <htejun@gmail.com>
> Cc: Thomas Gleixner <tglx@linutronix.de>
> Link: http://lkml.kernel.org/r/d8a7ef9592f55224630cb26dea239f05b6398a4e.1458187654.git.zhaolei@cn.fujitsu.com
> Signed-off-by: Ingo Molnar <mingo@kernel.org>
> (cherry picked from commit 73e6aafd9ea81498d31361f01db84a0118da2d1c)
> Signed-off-by: Gavin Guo <gavin.guo@canonical.com>

Acked-by: Kleber Sacilotto de Souza <kleber.souza@canonical.com>

> ---
>  kernel/sched/cpuacct.c | 28 +++++-----------------------
>  kernel/sched/cpuacct.h |  4 ++--
>  2 files changed, 7 insertions(+), 25 deletions(-)
> 
> diff --git a/kernel/sched/cpuacct.c b/kernel/sched/cpuacct.c
> index dd7cbb55bbf2..e14cfd0e8f20 100644
> --- a/kernel/sched/cpuacct.c
> +++ b/kernel/sched/cpuacct.c
> @@ -235,23 +235,10 @@ static struct cftype files[] = {
>  void cpuacct_charge(struct task_struct *tsk, u64 cputime)
>  {
>  	struct cpuacct *ca;
> -	int cpu;
> -
> -	cpu = task_cpu(tsk);
>  
>  	rcu_read_lock();
> -
> -	ca = task_ca(tsk);
> -
> -	while (true) {
> -		u64 *cpuusage = per_cpu_ptr(ca->cpuusage, cpu);
> -		*cpuusage += cputime;
> -
> -		ca = parent_ca(ca);
> -		if (!ca)
> -			break;
> -	}
> -
> +	for (ca = task_ca(tsk); ca; ca = parent_ca(ca))
> +		*this_cpu_ptr(ca->cpuusage) += cputime;
>  	rcu_read_unlock();
>  }
>  
> @@ -260,18 +247,13 @@ void cpuacct_charge(struct task_struct *tsk, u64 cputime)
>   *
>   * Note: it's the caller that updates the account of the root cgroup.
>   */
> -void cpuacct_account_field(struct task_struct *p, int index, u64 val)
> +void cpuacct_account_field(struct task_struct *tsk, int index, u64 val)
>  {
> -	struct kernel_cpustat *kcpustat;
>  	struct cpuacct *ca;
>  
>  	rcu_read_lock();
> -	ca = task_ca(p);
> -	while (ca != &root_cpuacct) {
> -		kcpustat = this_cpu_ptr(ca->cpustat);
> -		kcpustat->cpustat[index] += val;
> -		ca = parent_ca(ca);
> -	}
> +	for (ca = task_ca(tsk); ca != &root_cpuacct; ca = parent_ca(ca))
> +		this_cpu_ptr(ca->cpustat)->cpustat[index] += val;
>  	rcu_read_unlock();
>  }
>  
> diff --git a/kernel/sched/cpuacct.h b/kernel/sched/cpuacct.h
> index ed605624a5e7..ba72807c73d4 100644
> --- a/kernel/sched/cpuacct.h
> +++ b/kernel/sched/cpuacct.h
> @@ -1,7 +1,7 @@
>  #ifdef CONFIG_CGROUP_CPUACCT
>  
>  extern void cpuacct_charge(struct task_struct *tsk, u64 cputime);
> -extern void cpuacct_account_field(struct task_struct *p, int index, u64 val);
> +extern void cpuacct_account_field(struct task_struct *tsk, int index, u64 val);
>  
>  #else
>  
> @@ -10,7 +10,7 @@ static inline void cpuacct_charge(struct task_struct *tsk, u64 cputime)
>  }
>  
>  static inline void
> -cpuacct_account_field(struct task_struct *p, int index, u64 val)
> +cpuacct_account_field(struct task_struct *tsk, int index, u64 val)
>  {
>  }
>  
>
Stefan Bader June 7, 2018, 7:52 p.m. | #2
On 06.06.2018 19:58, Gavin Guo wrote:
> From: Zhao Lei <zhaolei@cn.fujitsu.com>
> 
> BugLink: https://bugs.launchpad.net/bugs/1775326
> 
>  - Use for() instead of while() loop in some functions
>    to make the code simpler.
> 
>  - Use this_cpu_ptr() instead of per_cpu_ptr() to make the code
>    cleaner and a bit faster.
> 
> Suggested-by: Peter Zijlstra <peterz@infradead.org>
> Signed-off-by: Zhao Lei <zhaolei@cn.fujitsu.com>
> Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
> Cc: Linus Torvalds <torvalds@linux-foundation.org>
> Cc: Tejun Heo <htejun@gmail.com>
> Cc: Thomas Gleixner <tglx@linutronix.de>
> Link: http://lkml.kernel.org/r/d8a7ef9592f55224630cb26dea239f05b6398a4e.1458187654.git.zhaolei@cn.fujitsu.com
> Signed-off-by: Ingo Molnar <mingo@kernel.org>
> (cherry picked from commit 73e6aafd9ea81498d31361f01db84a0118da2d1c)
> Signed-off-by: Gavin Guo <gavin.guo@canonical.com>
Acked-by: Stefan Bader <stefan.bader@canonical.com>
> ---
>  kernel/sched/cpuacct.c | 28 +++++-----------------------
>  kernel/sched/cpuacct.h |  4 ++--
>  2 files changed, 7 insertions(+), 25 deletions(-)
> 
> diff --git a/kernel/sched/cpuacct.c b/kernel/sched/cpuacct.c
> index dd7cbb55bbf2..e14cfd0e8f20 100644
> --- a/kernel/sched/cpuacct.c
> +++ b/kernel/sched/cpuacct.c
> @@ -235,23 +235,10 @@ static struct cftype files[] = {
>  void cpuacct_charge(struct task_struct *tsk, u64 cputime)
>  {
>  	struct cpuacct *ca;
> -	int cpu;
> -
> -	cpu = task_cpu(tsk);
>  
>  	rcu_read_lock();
> -
> -	ca = task_ca(tsk);
> -
> -	while (true) {
> -		u64 *cpuusage = per_cpu_ptr(ca->cpuusage, cpu);
> -		*cpuusage += cputime;
> -
> -		ca = parent_ca(ca);
> -		if (!ca)
> -			break;
> -	}
> -
> +	for (ca = task_ca(tsk); ca; ca = parent_ca(ca))
> +		*this_cpu_ptr(ca->cpuusage) += cputime;
>  	rcu_read_unlock();
>  }
>  
> @@ -260,18 +247,13 @@ void cpuacct_charge(struct task_struct *tsk, u64 cputime)
>   *
>   * Note: it's the caller that updates the account of the root cgroup.
>   */
> -void cpuacct_account_field(struct task_struct *p, int index, u64 val)
> +void cpuacct_account_field(struct task_struct *tsk, int index, u64 val)
>  {
> -	struct kernel_cpustat *kcpustat;
>  	struct cpuacct *ca;
>  
>  	rcu_read_lock();
> -	ca = task_ca(p);
> -	while (ca != &root_cpuacct) {
> -		kcpustat = this_cpu_ptr(ca->cpustat);
> -		kcpustat->cpustat[index] += val;
> -		ca = parent_ca(ca);
> -	}
> +	for (ca = task_ca(tsk); ca != &root_cpuacct; ca = parent_ca(ca))
> +		this_cpu_ptr(ca->cpustat)->cpustat[index] += val;
>  	rcu_read_unlock();
>  }
>  
> diff --git a/kernel/sched/cpuacct.h b/kernel/sched/cpuacct.h
> index ed605624a5e7..ba72807c73d4 100644
> --- a/kernel/sched/cpuacct.h
> +++ b/kernel/sched/cpuacct.h
> @@ -1,7 +1,7 @@
>  #ifdef CONFIG_CGROUP_CPUACCT
>  
>  extern void cpuacct_charge(struct task_struct *tsk, u64 cputime);
> -extern void cpuacct_account_field(struct task_struct *p, int index, u64 val);
> +extern void cpuacct_account_field(struct task_struct *tsk, int index, u64 val);
>  
>  #else
>  
> @@ -10,7 +10,7 @@ static inline void cpuacct_charge(struct task_struct *tsk, u64 cputime)
>  }
>  
>  static inline void
> -cpuacct_account_field(struct task_struct *p, int index, u64 val)
> +cpuacct_account_field(struct task_struct *tsk, int index, u64 val)
>  {
>  }
>  
>

Patch

diff --git a/kernel/sched/cpuacct.c b/kernel/sched/cpuacct.c
index dd7cbb55bbf2..e14cfd0e8f20 100644
--- a/kernel/sched/cpuacct.c
+++ b/kernel/sched/cpuacct.c
@@ -235,23 +235,10 @@  static struct cftype files[] = {
 void cpuacct_charge(struct task_struct *tsk, u64 cputime)
 {
 	struct cpuacct *ca;
-	int cpu;
-
-	cpu = task_cpu(tsk);
 
 	rcu_read_lock();
-
-	ca = task_ca(tsk);
-
-	while (true) {
-		u64 *cpuusage = per_cpu_ptr(ca->cpuusage, cpu);
-		*cpuusage += cputime;
-
-		ca = parent_ca(ca);
-		if (!ca)
-			break;
-	}
-
+	for (ca = task_ca(tsk); ca; ca = parent_ca(ca))
+		*this_cpu_ptr(ca->cpuusage) += cputime;
 	rcu_read_unlock();
 }
 
@@ -260,18 +247,13 @@  void cpuacct_charge(struct task_struct *tsk, u64 cputime)
  *
  * Note: it's the caller that updates the account of the root cgroup.
  */
-void cpuacct_account_field(struct task_struct *p, int index, u64 val)
+void cpuacct_account_field(struct task_struct *tsk, int index, u64 val)
 {
-	struct kernel_cpustat *kcpustat;
 	struct cpuacct *ca;
 
 	rcu_read_lock();
-	ca = task_ca(p);
-	while (ca != &root_cpuacct) {
-		kcpustat = this_cpu_ptr(ca->cpustat);
-		kcpustat->cpustat[index] += val;
-		ca = parent_ca(ca);
-	}
+	for (ca = task_ca(tsk); ca != &root_cpuacct; ca = parent_ca(ca))
+		this_cpu_ptr(ca->cpustat)->cpustat[index] += val;
 	rcu_read_unlock();
 }
 
diff --git a/kernel/sched/cpuacct.h b/kernel/sched/cpuacct.h
index ed605624a5e7..ba72807c73d4 100644
--- a/kernel/sched/cpuacct.h
+++ b/kernel/sched/cpuacct.h
@@ -1,7 +1,7 @@ 
 #ifdef CONFIG_CGROUP_CPUACCT
 
 extern void cpuacct_charge(struct task_struct *tsk, u64 cputime);
-extern void cpuacct_account_field(struct task_struct *p, int index, u64 val);
+extern void cpuacct_account_field(struct task_struct *tsk, int index, u64 val);
 
 #else
 
@@ -10,7 +10,7 @@  static inline void cpuacct_charge(struct task_struct *tsk, u64 cputime)
 }
 
 static inline void
-cpuacct_account_field(struct task_struct *p, int index, u64 val)
+cpuacct_account_field(struct task_struct *tsk, int index, u64 val)
 {
 }