Skip to contentSkip to conversation

cpuacct, cgroup: Remove cgroup_subsys.active

PATCH Superseded Li Zefan 2 participants 7 messages Mar 28, 2013

Subsystems (2)
Scheduler cgroup

v1 of 2 Superseded Latest is v2 →

lore
Share & tools
lore

Li Zefan

author
#0 lore raw

cover cpuacct, cgroup: Remove cgroup_subsys.active

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 0/7] sched: Split cpuacct" (just because there're code conflicts)

	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(-)
author
#1 lore raw

PATCH 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
author
#2 lore raw

PATCH 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 9f5a804..e12649c 100644
@@ -6860,8 +6860,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
author
#3 lore raw

PATCH 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
author
#4 lore raw

PATCH 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
author
#5 lore raw

PATCH 5/5 cgroup: Remove subsys.active flag

In reply to Li Zefan

The only user was cpuacct.

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 7d73905..a236a5b 100644
@@ -457,7 +457,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 4855892..64dd57d 100644
@@ -4415,7 +4415,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);

@@ -4523,7 +4522,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;

@@ -4565,7 +4563,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_remove_all(&ss->idr);
Signature · 1 line
1.8.0.2