* [PATCH 0/2] Fix brown paper bag bugs on previous submission @ 2011-12-02 21:58 Glauber Costa 2011-12-02 21:58 ` [PATCH 1/2] Fix user/system tick double accounting Glauber Costa 2011-12-02 21:58 ` [PATCH 2/2] fix parameter passing in task_group_account_field Glauber Costa 0 siblings, 2 replies; 5+ messages in thread From: Glauber Costa @ 2011-12-02 21:58 UTC (permalink / raw) To: linux-kernel; +Cc: a.p.zijlstra 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 ^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH 1/2] Fix user/system tick double accounting 2011-12-02 21:58 [PATCH 0/2] Fix brown paper bag bugs on previous submission Glauber Costa @ 2011-12-02 21:58 ` Glauber Costa 2011-12-06 20:21 ` [tip:sched/core] sched/accounting: " tip-bot for Glauber Costa 2011-12-02 21:58 ` [PATCH 2/2] fix parameter passing in task_group_account_field Glauber Costa 1 sibling, 1 reply; 5+ messages in thread From: Glauber Costa @ 2011-12-02 21:58 UTC (permalink / raw) To: linux-kernel; +Cc: a.p.zijlstra, 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 ^ permalink raw reply [flat|nested] 5+ messages in thread
* [tip:sched/core] sched/accounting: Fix user/system tick double accounting 2011-12-02 21:58 ` [PATCH 1/2] Fix user/system tick double accounting Glauber Costa @ 2011-12-06 20:21 ` tip-bot for Glauber Costa 0 siblings, 0 replies; 5+ messages in thread From: tip-bot for Glauber Costa @ 2011-12-06 20:21 UTC (permalink / raw) To: linux-tip-commits Cc: linux-kernel, hpa, mingo, a.p.zijlstra, glommer, tglx, mingo 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 */ ^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH 2/2] fix parameter passing in task_group_account_field 2011-12-02 21:58 [PATCH 0/2] Fix brown paper bag bugs on previous submission Glauber Costa 2011-12-02 21:58 ` [PATCH 1/2] Fix user/system tick double accounting Glauber Costa @ 2011-12-02 21:58 ` Glauber Costa 2011-12-06 20:22 ` [tip:sched/core] sched/accounting: Fix " tip-bot for Glauber Costa 1 sibling, 1 reply; 5+ messages in thread From: Glauber Costa @ 2011-12-02 21:58 UTC (permalink / raw) To: linux-kernel; +Cc: a.p.zijlstra, 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 ^ permalink raw reply [flat|nested] 5+ messages in thread
* [tip:sched/core] sched/accounting: Fix parameter passing in task_group_account_field 2011-12-02 21:58 ` [PATCH 2/2] fix parameter passing in task_group_account_field Glauber Costa @ 2011-12-06 20:22 ` tip-bot for Glauber Costa 0 siblings, 0 replies; 5+ messages in thread From: tip-bot for Glauber Costa @ 2011-12-06 20:22 UTC (permalink / raw) To: linux-tip-commits Cc: linux-kernel, hpa, mingo, a.p.zijlstra, glommer, tglx, mingo 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; ^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2011-12-06 20:23 UTC | newest] Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2011-12-02 21:58 [PATCH 0/2] Fix brown paper bag bugs on previous submission Glauber Costa 2011-12-02 21:58 ` [PATCH 1/2] Fix user/system tick double accounting Glauber Costa 2011-12-06 20:21 ` [tip:sched/core] sched/accounting: " tip-bot for Glauber Costa 2011-12-02 21:58 ` [PATCH 2/2] fix parameter passing in task_group_account_field Glauber Costa 2011-12-06 20:22 ` [tip:sched/core] sched/accounting: Fix " tip-bot for Glauber Costa
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox
Powered by JetHome