[PATCH 0/2] Fix brown paper bag bugs on previous submission

[PATCH 0/2] Fix brown paper bag bugs on previous submission

[Glauber Costa]

Hi Peter,

Now that I rebased to your queue tree and put my other patches ontop
of it, I noticed two quite stupid bug I've made in the last series you
merged.

In the end, one of them masked the other: one of them causes us to double
account to the root cgroup, the other, sends one of the accounting to the 
wrong place.

Would you please add on top of what you have?

Glauber Costa (2):
  Fix user/system tick double accounting
  fix parameter passing in task_group_account_field

 kernel/sched/core.c |   16 ++++------------
 1 files changed, 4 insertions(+), 12 deletions(-)

-- 
1.7.6.4

[PATCH 1/2] Fix user/system tick double accounting

[Glauber Costa]

Now that we're  pointing cpuacct's root cgroup to cpustat and accounting
through task_group_account_field(), we should not access cpustat directly.
Since it is done anyway inside the acessor function, we end up accounting
it twice, which is wrong.

Signed-off-by: Glauber Costa <glommer@parallels.com>
---
 kernel/sched/core.c |   12 ++----------
 1 files changed, 2 insertions(+), 10 deletions(-)

diff --git a/kernel/sched/core.c b/kernel/sched/core.c
index 5181a39..b908df3 100644
--- a/kernel/sched/core.c
+++ b/kernel/sched/core.c
@@ -2625,8 +2625,6 @@ static inline void task_group_account_field(struct task_struct *p,
 void account_user_time(struct task_struct *p, cputime_t cputime,
 		       cputime_t cputime_scaled)
 {
-	u64 *cpustat = kcpustat_this_cpu->cpustat;
-	u64 tmp;
 	int index;
 
 	/* Add user time to process. */
@@ -2634,13 +2632,11 @@ void account_user_time(struct task_struct *p, cputime_t cputime,
 	p->utimescaled = cputime_add(p->utimescaled, cputime_scaled);
 	account_group_user_time(p, cputime);
 
-	/* Add user time to cpustat. */
-	tmp = cputime_to_cputime64(cputime);
-
 	index = (TASK_NICE(p) > 0) ? CPUTIME_NICE : CPUTIME_USER;
-	cpustat[index] += tmp;
 
+	/* Add user time to cpustat. */
 	task_group_account_field(p, index, cputime);
+
 	/* Account for user time used */
 	acct_update_integrals(p);
 }
@@ -2686,16 +2682,12 @@ static inline
 void __account_system_time(struct task_struct *p, cputime_t cputime,
 			cputime_t cputime_scaled, int index)
 {
-	u64 tmp = cputime_to_cputime64(cputime);
-	u64 *cpustat = kcpustat_this_cpu->cpustat;
-
 	/* Add system time to process. */
 	p->stime = cputime_add(p->stime, cputime);
 	p->stimescaled = cputime_add(p->stimescaled, cputime_scaled);
 	account_group_system_time(p, cputime);
 
 	/* Add system time to cpustat. */
-	cpustat[index] += tmp;
 	task_group_account_field(p, index, cputime);
 
 	/* Account for system time used */
-- 
1.7.6.4

[PATCH 2/2] fix parameter passing in task_group_account_field

[Glauber Costa]

The order of parameters is inverted. The index parameter
should come first.

Signed-off-by: Glauber Costa <glommer@parallels.com>
---
 kernel/sched/core.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/kernel/sched/core.c b/kernel/sched/core.c
index b908df3..9fd0038 100644
--- a/kernel/sched/core.c
+++ b/kernel/sched/core.c
@@ -2585,8 +2585,8 @@ struct cgroup_subsys cpuacct_subsys;
 struct cpuacct root_cpuacct;
 #endif
 
-static inline void task_group_account_field(struct task_struct *p,
-					     u64 tmp, int index)
+static inline void task_group_account_field(struct task_struct *p, int index,
+					    u64 tmp)
 {
 #ifdef CONFIG_CGROUP_CPUACCT
 	struct kernel_cpustat *kcpustat;
-- 
1.7.6.4

[tip:sched/core] sched/accounting: Fix user/system tick double accounting

[tip-bot for Glauber Costa]

Commit-ID:  1c77f38ad623d8c3bc062f0ff9b8c5a2dfb2f1a2
Gitweb:     http://git.kernel.org/tip/1c77f38ad623d8c3bc062f0ff9b8c5a2dfb2f1a2
Author:     Glauber Costa <glommer@parallels.com>
AuthorDate: Fri, 2 Dec 2011 19:58:38 -0200
Committer:  Ingo Molnar <mingo@elte.hu>
CommitDate: Tue, 6 Dec 2011 20:51:23 +0100

sched/accounting: Fix user/system tick double accounting

Now that we're  pointing cpuacct's root cgroup to cpustat and accounting
through task_group_account_field(), we should not access cpustat directly.
Since it is done anyway inside the acessor function, we end up accounting
it twice, which is wrong.

Signed-off-by: Glauber Costa <glommer@parallels.com>
Signed-off-by: Peter Zijlstra <a.p.zijlstra@chello.nl>
Link: http://lkml.kernel.org/r/1322863119-14225-2-git-send-email-glommer@parallels.com
Signed-off-by: Ingo Molnar <mingo@elte.hu>
---
 kernel/sched/core.c |   12 ++----------
 1 files changed, 2 insertions(+), 10 deletions(-)

diff --git a/kernel/sched/core.c b/kernel/sched/core.c
index 3e078f2..6e86010 100644
--- a/kernel/sched/core.c
+++ b/kernel/sched/core.c
@@ -2601,8 +2601,6 @@ static inline void task_group_account_field(struct task_struct *p,
 void account_user_time(struct task_struct *p, cputime_t cputime,
 		       cputime_t cputime_scaled)
 {
-	u64 *cpustat = kcpustat_this_cpu->cpustat;
-	u64 tmp;
 	int index;
 
 	/* Add user time to process. */
@@ -2610,13 +2608,11 @@ void account_user_time(struct task_struct *p, cputime_t cputime,
 	p->utimescaled = cputime_add(p->utimescaled, cputime_scaled);
 	account_group_user_time(p, cputime);
 
-	/* Add user time to cpustat. */
-	tmp = cputime_to_cputime64(cputime);
-
 	index = (TASK_NICE(p) > 0) ? CPUTIME_NICE : CPUTIME_USER;
-	cpustat[index] += tmp;
 
+	/* Add user time to cpustat. */
 	task_group_account_field(p, index, cputime);
+
 	/* Account for user time used */
 	acct_update_integrals(p);
 }
@@ -2662,16 +2658,12 @@ static inline
 void __account_system_time(struct task_struct *p, cputime_t cputime,
 			cputime_t cputime_scaled, int index)
 {
-	u64 tmp = cputime_to_cputime64(cputime);
-	u64 *cpustat = kcpustat_this_cpu->cpustat;
-
 	/* Add system time to process. */
 	p->stime = cputime_add(p->stime, cputime);
 	p->stimescaled = cputime_add(p->stimescaled, cputime_scaled);
 	account_group_system_time(p, cputime);
 
 	/* Add system time to cpustat. */
-	cpustat[index] += tmp;
 	task_group_account_field(p, index, cputime);
 
 	/* Account for system time used */

[tip:sched/core] sched/accounting: Fix parameter passing in task_group_account_field

[tip-bot for Glauber Costa]

Commit-ID:  be726ffd1ef291c04c4d6632ac277afa1c281712
Gitweb:     http://git.kernel.org/tip/be726ffd1ef291c04c4d6632ac277afa1c281712
Author:     Glauber Costa <glommer@parallels.com>
AuthorDate: Fri, 2 Dec 2011 19:58:39 -0200
Committer:  Ingo Molnar <mingo@elte.hu>
CommitDate: Tue, 6 Dec 2011 20:51:24 +0100

sched/accounting: Fix parameter passing in task_group_account_field

The order of parameters is inverted. The index parameter
should come first.

Signed-off-by: Glauber Costa <glommer@parallels.com>
Signed-off-by: Peter Zijlstra <a.p.zijlstra@chello.nl>
Link: http://lkml.kernel.org/r/1322863119-14225-3-git-send-email-glommer@parallels.com
Signed-off-by: Ingo Molnar <mingo@elte.hu>
---
 kernel/sched/core.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/kernel/sched/core.c b/kernel/sched/core.c
index 6e86010..9ac22d2 100644
--- a/kernel/sched/core.c
+++ b/kernel/sched/core.c
@@ -2561,8 +2561,8 @@ struct cgroup_subsys cpuacct_subsys;
 struct cpuacct root_cpuacct;
 #endif
 
-static inline void task_group_account_field(struct task_struct *p,
-					     u64 tmp, int index)
+static inline void task_group_account_field(struct task_struct *p, int index,
+					    u64 tmp)
 {
 #ifdef CONFIG_CGROUP_CPUACCT
 	struct kernel_cpustat *kcpustat;