cpuacct, cgroup: Kill cgroup_subsys.active

PATCH In review Li Zefan 3 participants 13 messages Mar 29, 2013 – Apr 10, 2013

Subsystems (2)

Scheduler cgroup

lore

Share & tools

lore

Li Zefan

author

2013-03-29 06:43 UTC #0 lore raw

v2 cover cpuacct, cgroup: Kill cgroup_subsys.active

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(-)

Li Zefan

author

2013-03-29 06:43 UTC #1 lore raw

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

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

Signed-off-by Li Zefan <[email protected]>

---
 kernel/sched/cpuacct.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

kernel/sched/cpuacct.c +2 -2

index 9305fd2..a691c4d 100644
@@ -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 = {

Signature · 1 line

1.8.0.2

tip-bot for Li Zefan

merge bot

2013-04-10 13:45 UTC #2 lore raw

[tip:sched/core] sched/cpuacct: Allocate per_cpu cpuusage for root cpuacct statically

sched/cpuacct: Allocate per_cpu cpuusage for root cpuacct statically

Commit-ID:  7943e15a3e91db78a7a3fbc84e45cf9d1c7c7d23
Gitweb:     http://git.kernel.org/tip/7943e15a3e91db78a7a3fbc84e45cf9d1c7c7d23
Author:     Li Zefan <[email protected]>
AuthorDate: Fri, 29 Mar 2013 14:43:46 +0800
Committer:  Ingo Molnar <[email protected]>
CommitDate: Wed, 10 Apr 2013 13:54:20 +0200

sched/cpuacct: Allocate per_cpu cpuusage for root cpuacct statically

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

Signed-off-by Li Zefan <[email protected]>

Cc Tejun Heo <[email protected]>

Acked-by Peter Zijlstra <[email protected]>

Link http://lkml.kernel.org/r/[email protected] open here

Signed-off-by Ingo Molnar <[email protected]>

---
 kernel/sched/cpuacct.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

kernel/sched/cpuacct.c +2 -2

index 9305fd2..a691c4d 100644
@@ -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 = {

Li Zefan

author

2013-03-29 06:44 UTC #3 lore raw

PATCH v2 2/5 cpuacct: Initialize root cpuacct earlier

In reply to 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 <[email protected]>

---
 kernel/sched/core.c    |  2 --
 kernel/sched/cpuacct.c | 11 ++++-------
 kernel/sched/cpuacct.h |  5 -----
 3 files changed, 4 insertions(+), 14 deletions(-)

kernel/sched/core.c +0 -2

index c7016c1..5338e6d 100644
@@ -6936,8 +6936,6 @@ void __init sched_init(void)
 
 #endif /* CONFIG_CGROUP_SCHED */
 
-	cpuacct_init();
-
 	for_each_possible_cpu(i) {
 		struct rq *rq;

kernel/sched/cpuacct.c +4 -7

index a691c4d..0425581 100644
@@ -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,

kernel/sched/cpuacct.h +0 -5

index 51cd76e..ed60562 100644
@@ -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)
 {
 }

Signature · 1 line

1.8.0.2

tip-bot for Li Zefan

merge bot

2013-04-10 13:46 UTC #4 lore raw

[tip:sched/core] sched/cpuacct: Initialize root cpuacct earlier

sched/cpuacct: Initialize root cpuacct earlier

Commit-ID:  14c6d3c8a47ced185b6375c4940b5b393f1a294e
Gitweb:     http://git.kernel.org/tip/14c6d3c8a47ced185b6375c4940b5b393f1a294e
Author:     Li Zefan <[email protected]>
AuthorDate: Fri, 29 Mar 2013 14:44:04 +0800
Committer:  Ingo Molnar <[email protected]>
CommitDate: Wed, 10 Apr 2013 13:54:20 +0200

sched/cpuacct: Initialize root cpuacct earlier

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

Signed-off-by Li Zefan <[email protected]>

Cc Tejun Heo <[email protected]>

Acked-by Peter Zijlstra <[email protected]>

Link http://lkml.kernel.org/r/[email protected] open here

Signed-off-by Ingo Molnar <[email protected]>

---
 kernel/sched/core.c    |  2 --
 kernel/sched/cpuacct.c | 11 ++++-------
 kernel/sched/cpuacct.h |  5 -----
 3 files changed, 4 insertions(+), 14 deletions(-)

kernel/sched/core.c +0 -2

index 92930a8..ee8c1bd 100644
@@ -6936,8 +6936,6 @@ void __init sched_init(void)
 
 #endif /* CONFIG_CGROUP_SCHED */
 
-	cpuacct_init();
-
 	for_each_possible_cpu(i) {
 		struct rq *rq;

kernel/sched/cpuacct.c +4 -7

index a691c4d..0425581 100644
@@ -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,

kernel/sched/cpuacct.h +0 -5

index 51cd76e..ed60562 100644
@@ -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)
 {
 }

Li Zefan

author

2013-03-29 06:44 UTC #5 lore raw

PATCH v2 3/5 cpuacct: Initialize cpuacct subsystem earlier

In reply to 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 <[email protected]>

---
 kernel/sched/cpuacct.c | 11 ++++++-----
 1 file changed, 6 insertions(+), 5 deletions(-)

kernel/sched/cpuacct.c +6 -5

index 0425581..75e46d2 100644
@@ -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,
 };

Signature · 1 line

1.8.0.2

tip-bot for Li Zefan

merge bot

2013-04-10 13:47 UTC #6 lore raw

[tip:sched/core] sched/cpuacct: Initialize cpuacct subsystem earlier

sched/cpuacct: Initialize cpuacct subsystem earlier

Commit-ID:  621e2de02403a6f776852c564b79c38bf3cc9032
Gitweb:     http://git.kernel.org/tip/621e2de02403a6f776852c564b79c38bf3cc9032
Author:     Li Zefan <[email protected]>
AuthorDate: Fri, 29 Mar 2013 14:44:15 +0800
Committer:  Ingo Molnar <[email protected]>
CommitDate: Wed, 10 Apr 2013 13:54:21 +0200

sched/cpuacct: Initialize cpuacct subsystem earlier

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 <[email protected]>

Cc Tejun Heo <[email protected]>

Acked-by Peter Zijlstra <[email protected]>

Link http://lkml.kernel.org/r/[email protected] open here

Signed-off-by Ingo Molnar <[email protected]>

---
 kernel/sched/cpuacct.c | 11 ++++++-----
 1 file changed, 6 insertions(+), 5 deletions(-)

kernel/sched/cpuacct.c +6 -5

index 0425581..75e46d2 100644
@@ -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,
 };

Li Zefan

author

2013-03-29 06:44 UTC #7 lore raw

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

In reply to 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 <[email protected]>

---
 kernel/sched/cpuacct.c | 6 ------
 1 file changed, 6 deletions(-)

kernel/sched/cpuacct.c +0 -6

index 75e46d2..ef57ab6 100644
@@ -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) {

Signature · 1 line

1.8.0.2

tip-bot for Li Zefan

merge bot

2013-04-10 13:48 UTC #8 lore raw

[tip:sched/core] sched/cpuacct: No need to check subsys active state

sched/cpuacct: No need to check subsys active state

Commit-ID:  a2b0ae25fc8bfeeb4022b8e847ab811b3c8368d1
Gitweb:     http://git.kernel.org/tip/a2b0ae25fc8bfeeb4022b8e847ab811b3c8368d1
Author:     Li Zefan <[email protected]>
AuthorDate: Fri, 29 Mar 2013 14:44:28 +0800
Committer:  Ingo Molnar <[email protected]>
CommitDate: Wed, 10 Apr 2013 13:54:22 +0200

sched/cpuacct: No need to check subsys active state

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 <[email protected]>

Cc Tejun Heo <[email protected]>

Acked-by Peter Zijlstra <[email protected]>

Link http://lkml.kernel.org/r/[email protected] open here

Signed-off-by Ingo Molnar <[email protected]>

---
 kernel/sched/cpuacct.c | 6 ------
 1 file changed, 6 deletions(-)

kernel/sched/cpuacct.c +0 -6

index 75e46d2..ef57ab6 100644
@@ -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) {

Li Zefan

author

2013-03-29 06:44 UTC #9 lore raw

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

In reply to Li Zefan

The only user was cpuacct.

Acked-by Tejun Heo <[email protected]>

Signed-off-by Li Zefan <[email protected]>

---
 include/linux/cgroup.h | 1 -
 kernel/cgroup.c        | 3 ---
 2 files changed, 4 deletions(-)

include/linux/cgroup.h +0 -1

index 900af59..95c02c0 100644
@@ -458,7 +458,6 @@ struct cgroup_subsys {
 	void (*bind)(struct cgroup *root);
 
 	int subsys_id;
-	int active;
 	int disabled;
 	int early_init;
 	/*

kernel/cgroup.c +0 -3

index a32f943..5c46281 100644
@@ -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);

Signature · 1 line

1.8.0.2

tip-bot for Li Zefan

merge bot

2013-04-10 13:50 UTC #10 lore raw

[tip:sched/core] cgroup: Kill subsys.active flag

cgroup: Kill subsys.active flag

Commit-ID:  479f614110b889d5783acdaec865ede3cdb96b97
Gitweb:     http://git.kernel.org/tip/479f614110b889d5783acdaec865ede3cdb96b97
Author:     Li Zefan <[email protected]>
AuthorDate: Fri, 29 Mar 2013 14:44:42 +0800
Committer:  Ingo Molnar <[email protected]>
CommitDate: Wed, 10 Apr 2013 13:54:22 +0200

cgroup: Kill subsys.active flag

The only user was cpuacct.

Acked-by Tejun Heo <[email protected]>

Signed-off-by Li Zefan <[email protected]>

Acked-by Peter Zijlstra <[email protected]>

Link http://lkml.kernel.org/r/[email protected] open here

Signed-off-by Ingo Molnar <[email protected]>

---
 include/linux/cgroup.h | 1 -
 kernel/cgroup.c        | 3 ---
 2 files changed, 4 deletions(-)

include/linux/cgroup.h +0 -1

index 900af59..95c02c0 100644
@@ -458,7 +458,6 @@ struct cgroup_subsys {
 	void (*bind)(struct cgroup *root);
 
 	int subsys_id;
-	int active;
 	int disabled;
 	int early_init;
 	/*

kernel/cgroup.c +0 -3

index a32f943..5c46281 100644
@@ -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);

Peter Zijlstra

maintainer Acked-by on the v2 series

2013-03-29 10:36 UTC #11 lore raw

In reply to Li Zefan

Quoted 12 lines · Li Zefan

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"

Quoted 7 lines

        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/[email protected]>

Li Zefan

author

2013-04-09 01:29 UTC #12 lore raw

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.

Quoted 26 lines · Peter Zijlstra

Quoted 12 lines · Li Zefan
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"
Quoted 7 lines
        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/[email protected]>