[PATCH v2 0/5] cpuacct, cgroup: Kill cgroup_subsys.active

[PATCH v2 0/5] cpuacct, cgroup: Kill cgroup_subsys.active

[Li Zefan]

v2:
- rebase against tip-tree
- add acked-by

I said this patchset needn't to be rebased, which is wrong. The last patch
needs rebasing.

===================================

cpuacct is the only user of cgroup_subsys.active flag.

The flag is needed because cpuacct_charge() and cpuacct_account_field() can
be called when cpuacct hasn't been initialized during system bootup.

This patch initializes cpuacct earlier, and the result is we don't have
to check the flag in scheduler hot path.

Note, this patchset is based on "[PATCH v2 0/7] sched: Split cpuacct"

	http://lkml.org/lkml/2013/3/28/1

0001-cpuacct-allocate-per_cpu-cpuusage-for-root-cpuacct-s.patch
0002-cpuacct-Initialize-root-cpuacct-earlier.patch
0003-cpuacct-Initialize-cpuacct-subsystem-earlier.patch
0004-cpuacct-No-need-to-check-subsys-active-state.patch
0005-cgroup-Remove-subsys.active-flag.patch

--
 include/linux/cgroup.h |  1 -
 kernel/cgroup.c        |  3 ---
 kernel/sched/core.c    |  2 --
 kernel/sched/cpuacct.c | 30 +++++++++++-------------------
 kernel/sched/cpuacct.h |  5 -----
 5 files changed, 11 insertions(+), 30 deletions(-)

[PATCH v2 1/5] cpuacct: allocate per_cpu cpuusage for root cpuacct statically

[Li Zefan]

This is a preparation, so later we can initialize cpuacct earlier.

Signed-off-by: Li Zefan <lizefan@huawei.com>
---
 kernel/sched/cpuacct.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/kernel/sched/cpuacct.c b/kernel/sched/cpuacct.c
index 9305fd2..a691c4d 100644
--- a/kernel/sched/cpuacct.c
+++ b/kernel/sched/cpuacct.c
@@ -58,6 +58,7 @@ static inline struct cpuacct *parent_ca(struct cpuacct *ca)
 	return cgroup_ca(ca->css.cgroup->parent);
 }
 
+static DEFINE_PER_CPU(u64, root_cpuacct_cpuusage);
 static struct cpuacct root_cpuacct;
 
 /* create a new cpu accounting group */
@@ -290,8 +291,7 @@ void cpuacct_account_field(struct task_struct *p, int index, u64 val)
 void __init cpuacct_init(void)
 {
 	root_cpuacct.cpustat = &kernel_cpustat;
-	root_cpuacct.cpuusage = alloc_percpu(u64);
-	BUG_ON(!root_cpuacct.cpuusage); /* Too early, not expected to fail */
+	root_cpuacct.cpuusage = &root_cpuacct_cpuusage;
 }
 
 struct cgroup_subsys cpuacct_subsys = {
-- 
1.8.0.2

[PATCH v2 2/5] cpuacct: Initialize root cpuacct earlier

[Li Zefan]

Now we don't need cpuacct_init(), and instead we just initialize
root_cpuacct when it's defined.

Signed-off-by: Li Zefan <lizefan-hv44wF8Li93QT0dZR+AlfA@public.gmane.org>
---
 kernel/sched/core.c    |  2 --
 kernel/sched/cpuacct.c | 11 ++++-------
 kernel/sched/cpuacct.h |  5 -----
 3 files changed, 4 insertions(+), 14 deletions(-)

diff --git a/kernel/sched/core.c b/kernel/sched/core.c
index c7016c1..5338e6d 100644
--- a/kernel/sched/core.c
+++ b/kernel/sched/core.c
@@ -6936,8 +6936,6 @@ void __init sched_init(void)
 
 #endif /* CONFIG_CGROUP_SCHED */
 
-	cpuacct_init();
-
 	for_each_possible_cpu(i) {
 		struct rq *rq;
 
diff --git a/kernel/sched/cpuacct.c b/kernel/sched/cpuacct.c
index a691c4d..0425581 100644
--- a/kernel/sched/cpuacct.c
+++ b/kernel/sched/cpuacct.c
@@ -59,7 +59,10 @@ static inline struct cpuacct *parent_ca(struct cpuacct *ca)
 }
 
 static DEFINE_PER_CPU(u64, root_cpuacct_cpuusage);
-static struct cpuacct root_cpuacct;
+static struct cpuacct root_cpuacct = {
+	.cpustat	= &kernel_cpustat,
+	.cpuusage	= &root_cpuacct_cpuusage,
+};
 
 /* create a new cpu accounting group */
 static struct cgroup_subsys_state *cpuacct_css_alloc(struct cgroup *cgrp)
@@ -288,12 +291,6 @@ void cpuacct_account_field(struct task_struct *p, int index, u64 val)
 	rcu_read_unlock();
 }
 
-void __init cpuacct_init(void)
-{
-	root_cpuacct.cpustat = &kernel_cpustat;
-	root_cpuacct.cpuusage = &root_cpuacct_cpuusage;
-}
-
 struct cgroup_subsys cpuacct_subsys = {
 	.name = "cpuacct",
 	.css_alloc = cpuacct_css_alloc,
diff --git a/kernel/sched/cpuacct.h b/kernel/sched/cpuacct.h
index 51cd76e..ed60562 100644
--- a/kernel/sched/cpuacct.h
+++ b/kernel/sched/cpuacct.h
@@ -1,15 +1,10 @@
 #ifdef CONFIG_CGROUP_CPUACCT
 
-extern void cpuacct_init(void);
 extern void cpuacct_charge(struct task_struct *tsk, u64 cputime);
 extern void cpuacct_account_field(struct task_struct *p, int index, u64 val);
 
 #else
 
-static inline void cpuacct_init(void)
-{
-}
-
 static inline void cpuacct_charge(struct task_struct *tsk, u64 cputime)
 {
 }
-- 
1.8.0.2

[PATCH v2 3/5] cpuacct: Initialize cpuacct subsystem earlier

[Li Zefan]

Initialize cpuacct before the scheduler is functioning, so when
cpuacct_charge() and cpuacct_account_field() are called, task_ca()
won't return NULL.

Signed-off-by: Li Zefan <lizefan-hv44wF8Li93QT0dZR+AlfA@public.gmane.org>
---
 kernel/sched/cpuacct.c | 11 ++++++-----
 1 file changed, 6 insertions(+), 5 deletions(-)

diff --git a/kernel/sched/cpuacct.c b/kernel/sched/cpuacct.c
index 0425581..75e46d2 100644
--- a/kernel/sched/cpuacct.c
+++ b/kernel/sched/cpuacct.c
@@ -292,9 +292,10 @@ void cpuacct_account_field(struct task_struct *p, int index, u64 val)
 }
 
 struct cgroup_subsys cpuacct_subsys = {
-	.name = "cpuacct",
-	.css_alloc = cpuacct_css_alloc,
-	.css_free = cpuacct_css_free,
-	.subsys_id = cpuacct_subsys_id,
-	.base_cftypes = files,
+	.name		= "cpuacct",
+	.css_alloc	= cpuacct_css_alloc,
+	.css_free	= cpuacct_css_free,
+	.subsys_id	= cpuacct_subsys_id,
+	.base_cftypes	= files,
+	.early_init	= 1,
 };
-- 
1.8.0.2

Re: [PATCH v2 0/5] cpuacct, cgroup: Kill cgroup_subsys.active

[Peter Zijlstra]

On Fri, 2013-03-29 at 14:43 +0800, Li Zefan wrote:
> cpuacct is the only user of cgroup_subsys.active flag.
> 
> The flag is needed because cpuacct_charge() and
> cpuacct_account_field() can
> be called when cpuacct hasn't been initialized during system bootup.
> 
> This patch initializes cpuacct earlier, and the result is we don't
> have
> to check the flag in scheduler hot path.
> 
> Note, this patchset is based on "[PATCH v2 0/7] sched: Split cpuacct"
> 
>         http://lkml.org/lkml/2013/3/28/1
> 
> 0001-cpuacct-allocate-per_cpu-cpuusage-for-root-cpuacct-s.patch
> 0002-cpuacct-Initialize-root-cpuacct-earlier.patch
> 0003-cpuacct-Initialize-cpuacct-subsystem-earlier.patch
> 0004-cpuacct-No-need-to-check-subsys-active-state.patch
> 0005-cgroup-Remove-subsys.active-flag.patch


Seems sane enough

Acked-by: Peter Zijlstra <a.p.zijlstra-/NLkJaSkS4VmR6Xm/wNWPw@public.gmane.org>

Re: [PATCH v2 0/5] cpuacct, cgroup: Kill cgroup_subsys.active

[Li Zefan]

Hi Ingo,

Any chance for this patchset and the other one to be queued for 3.10?

Both of them has been acked by Peter.

On 2013/3/29 18:36, Peter Zijlstra wrote:
> On Fri, 2013-03-29 at 14:43 +0800, Li Zefan wrote:
>> cpuacct is the only user of cgroup_subsys.active flag.
>>
>> The flag is needed because cpuacct_charge() and
>> cpuacct_account_field() can
>> be called when cpuacct hasn't been initialized during system bootup.
>>
>> This patch initializes cpuacct earlier, and the result is we don't
>> have
>> to check the flag in scheduler hot path.
>>
>> Note, this patchset is based on "[PATCH v2 0/7] sched: Split cpuacct"
>>
>>         http://lkml.org/lkml/2013/3/28/1
>>
>> 0001-cpuacct-allocate-per_cpu-cpuusage-for-root-cpuacct-s.patch
>> 0002-cpuacct-Initialize-root-cpuacct-earlier.patch
>> 0003-cpuacct-Initialize-cpuacct-subsystem-earlier.patch
>> 0004-cpuacct-No-need-to-check-subsys-active-state.patch
>> 0005-cgroup-Remove-subsys.active-flag.patch
> 
> 
> Seems sane enough
> 
> Acked-by: Peter Zijlstra <a.p.zijlstra-/NLkJaSkS4VmR6Xm/wNWPw@public.gmane.org>
> 

[PATCH v2 4/5] cpuacct: No need to check subsys active state

[Li Zefan]

Now we're guaranteed when cpuacct_charge() and cpuacct_account_field()
are called, cpuacct has already been properly initialized, so we no
longer need those checks.

Signed-off-by: Li Zefan <lizefan@huawei.com>
---
 kernel/sched/cpuacct.c | 6 ------
 1 file changed, 6 deletions(-)

diff --git a/kernel/sched/cpuacct.c b/kernel/sched/cpuacct.c
index 75e46d2..ef57ab6 100644
--- a/kernel/sched/cpuacct.c
+++ b/kernel/sched/cpuacct.c
@@ -247,9 +247,6 @@ void cpuacct_charge(struct task_struct *tsk, u64 cputime)
 	struct cpuacct *ca;
 	int cpu;
 
-	if (unlikely(!cpuacct_subsys.active))
-		return;
-
 	cpu = task_cpu(tsk);
 
 	rcu_read_lock();
@@ -278,9 +275,6 @@ void cpuacct_account_field(struct task_struct *p, int index, u64 val)
 	struct kernel_cpustat *kcpustat;
 	struct cpuacct *ca;
 
-	if (unlikely(!cpuacct_subsys.active))
-		return;
-
 	rcu_read_lock();
 	ca = task_ca(p);
 	while (ca != &root_cpuacct) {
-- 
1.8.0.2

[PATCH v2 5/5] cgroup: Kill subsys.active flag

[Li Zefan]

The only user was cpuacct.

Acked-by: Tejun Heo <tj@kernel.org>
Signed-off-by: Li Zefan <lizefan@huawei.com>
---
 include/linux/cgroup.h | 1 -
 kernel/cgroup.c        | 3 ---
 2 files changed, 4 deletions(-)

diff --git a/include/linux/cgroup.h b/include/linux/cgroup.h
index 900af59..95c02c0 100644
--- a/include/linux/cgroup.h
+++ b/include/linux/cgroup.h
@@ -458,7 +458,6 @@ struct cgroup_subsys {
 	void (*bind)(struct cgroup *root);
 
 	int subsys_id;
-	int active;
 	int disabled;
 	int early_init;
 	/*
diff --git a/kernel/cgroup.c b/kernel/cgroup.c
index a32f943..5c46281 100644
--- a/kernel/cgroup.c
+++ b/kernel/cgroup.c
@@ -4468,7 +4468,6 @@ static void __init cgroup_init_subsys(struct cgroup_subsys *ss)
 	 * need to invoke fork callbacks here. */
 	BUG_ON(!list_empty(&init_task.tasks));
 
-	ss->active = 1;
 	BUG_ON(online_css(ss, dummytop));
 
 	mutex_unlock(&cgroup_mutex);
@@ -4573,7 +4572,6 @@ int __init_or_module cgroup_load_subsys(struct cgroup_subsys *ss)
 	}
 	write_unlock(&css_set_lock);
 
-	ss->active = 1;
 	ret = online_css(ss, dummytop);
 	if (ret)
 		goto err_unload;
@@ -4614,7 +4612,6 @@ void cgroup_unload_subsys(struct cgroup_subsys *ss)
 	mutex_lock(&cgroup_mutex);
 
 	offline_css(ss, dummytop);
-	ss->active = 0;
 
 	if (ss->use_id)
 		idr_destroy(&ss->idr);
-- 
1.8.0.2