cpu,cpuacct: make cpu serve cpuacct files and deprecate cpuacct

Hello,

This is an attempt at allowing quick deprecation and removal of cpuacct. The patchset makes cpu serve the same cpuacct.* files and updates cgroup core such that it ignores cpuacct if requested to be co-mounted with cpu whether CONFIG_CGROUP_CPUACCT is enabled or not.

This de-couples cpuacct deprecation on kernel side from userland transition as long as userland is co-mounting cpu and cpuacct. In this series, cpuacct implementation is simply copied into cpu, the goal being to provide base for proper optimization (most likely periodic collection of stats across the hierarchy).

I didn't try to maintain everything the same. /proc/cgroups and /proc/PID/cgroup may mis-represent or miss cpuacct entry. Faking it completely is doable too but I'd like to keep it as simple as possible.

* If cpuacct is not used, nothing changes of course.

* If cpuacct is requested to be co-mounted with cpu, cpuacct is not used. It doesn't matter whether CONFIG_CGROUP_CPUACCT is enabled or not. cpu will provide cpusage.* statistics.

* If cpuacct is requested to be mounted separately and cpuacct is enabled, cgroup whines before mounting it. If cpuacct is disabled, it fails.

I think the resulting behavior isn't too crazy and it should allow us to remove anything cpuacct related fairly soon.

Longer term, I'd prefer to be able to drop the compatibility hack from cgroup core too although I don't think that's too urgent. I think userland can either create symlinks for compatibility - e.g. /sys/fs/cgroup/cpuacct and /sys/fs/cgroup/cpu,cpucct both pointing to /sys/fs/cgroup/cpu - whether cpuacct exists or not, or just ignore anything cpuacct related.

This series is based on for-3.7-hierarchy 8c7f6edbda0 ("cgroup: mark subsystems with broken hierarchy...") and also available in the following git branch.

git://git.kernel.org/pub/scm/linux/kernel/git/tj/cgroup.git review-deprecate-cpuacct

 include/linux/cgroup.h   |    1 
 init/Kconfig             |   11 ++
 kernel/cgroup.c          |   57 ++++++++++++-
 kernel/sched/core.c      |  196 ++++++++++++++++++++++++++++++++++++++++++++++-
 kernel/sched/fair.c      |    1 
 kernel/sched/rt.c        |    1 
 kernel/sched/sched.h     |    7 +
 kernel/sched/stop_task.c |    1 
 8 files changed, 269 insertions(+), 6 deletions(-)

Thanks.

--
tejun

When cgroup files are created, cgroup core automatically prepends the name of the subsystem as prefix. This patch adds CFTYPE_NO_PREFIX which disables the automatic prefix.

This will be used to deprecate cpuacct which will make cpu create and serve the cpuacct files.

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

index 68e8df7..7d6a298 100644
@@ -283,6 +283,7 @@ struct cgroup_map_cb {
 /* cftype->flags */
 #define CFTYPE_ONLY_ON_ROOT	(1U << 0)	/* only create on root cg */
 #define CFTYPE_NOT_ON_ROOT	(1U << 1)	/* don't create onp root cg */
+#define CFTYPE_NO_PREFIX	(1U << 2)	/* skip subsys prefix */
 
 #define MAX_CFTYPE_NAME		64
 

index b7d9606..08edb52 100644
@@ -2687,7 +2687,8 @@ static int cgroup_add_file(struct cgroup *cgrp, struct cgroup_subsys *subsys,
 	if ((cft->flags & CFTYPE_ONLY_ON_ROOT) && cgrp->parent)
 		return 0;
 
-	if (subsys && !test_bit(ROOT_NOPREFIX, &cgrp->root->flags)) {
+	if (subsys && !(cft->flags & CFTYPE_NO_PREFIX) &&
+	    !test_bit(ROOT_NOPREFIX, &cgrp->root->flags)) {
 		strcpy(name, subsys->name);
 		strcat(name, ".");
 	}

1.7.7.3

cpuacct being on a separate hierarchy is one of the main cgroup related complaints from scheduler side and the consensus seems to be

* Allowing cpuacct to be a separate controller was a mistake. In general multiple controllers on the same type of resource should be avoided, especially accounting-only ones.

* Statistics provided by cpuacct are useful and should instead be served by cpu.

This patch makes cpu maintain and serve all cpuacct.* files and make cgroup core ignore cpuacct if it's co-mounted with cpu. This is a step in deprecating cpuacct. The next patch will allow disabling or dropping cpuacct without affecting userland too much.

Note that this creates some discrepancies in /proc/cgroups and /proc/PID/cgroup. The co-mounted cpuacct won't be reflected correctly there. cpuacct will eventually be removed completely probably except for the statistics filenames and I'd like to keep the amount of compatbility hackery to minimum as much as possible.

The cpu statistics implementation isn't optimized in any way. It's mostly verbatim copy from cpuacct. The goal is allowing quick disabling and removal of CONFIG_CGROUP_CPUACCT and creating a base on top of which cpu can implement proper optimization.

---
 kernel/cgroup.c          |   13 +++
 kernel/sched/core.c      |  194 +++++++++++++++++++++++++++++++++++++++++++++-
 kernel/sched/fair.c      |    1 +
 kernel/sched/rt.c        |    1 +
 kernel/sched/sched.h     |    7 ++
 kernel/sched/stop_task.c |    1 +
 6 files changed, 215 insertions(+), 2 deletions(-)

index 08edb52..01c11e3 100644
@@ -1265,6 +1265,19 @@ static int parse_cgroupfs_options(char *data, struct cgroup_sb_opts *opts)
 	/* Consistency checks */
 
 	/*
+	 * cpuacct is deprecated and cpu will serve the same stat files.
+	 * If co-mount with cpu is requested, ignore cpuacct.  Note that
+	 * this creates some discrepancies in /proc/cgroups and
+	 * /proc/PID/cgroup.
+	 *
+	 * https://lkml.org/lkml/2012/9/13/542
+	 */
+#if IS_ENABLED(CONFIG_CGROUP_SCHED) && IS_ENABLED(CONFIG_CGROUP_CPUACCT)
+	if ((opts->subsys_bits & (1 << cpu_cgroup_subsys_id)) &&
+	    (opts->subsys_bits & (1 << cpuacct_subsys_id)))
+		opts->subsys_bits &= ~(1 << cpuacct_subsys_id);
+#endif
+	/*
 	 * Option noprefix was introduced just for backward compatibility
 	 * with the old cpuset, so we allow noprefix only if mounting just
 	 * the cpuset subsystem.

index c219bf8..8935c3a 100644
@@ -706,6 +706,7 @@ static void update_curr(struct cfs_rq *cfs_rq)
 		struct task_struct *curtask = task_of(curr);
 
 		trace_sched_stat_runtime(curtask, delta_exec, curr->vruntime);
+		task_group_charge(curtask, delta_exec);
 		cpuacct_charge(curtask, delta_exec);
 		account_group_exec_runtime(curtask, delta_exec);
 	}

index 944cb68..53f7d3d 100644
@@ -934,6 +934,7 @@ static void update_curr_rt(struct rq *rq)
 	account_group_exec_runtime(curr, delta_exec);
 
 	curr->se.exec_start = rq->clock_task;
+	task_group_charge(curr, delta_exec);
 	cpuacct_charge(curr, delta_exec);
 
 	sched_rt_avg_update(rq, delta_exec);

index f6714d0..bfa115e 100644
@@ -104,6 +104,10 @@ struct cfs_bandwidth {
 struct task_group {
 	struct cgroup_subsys_state css;
 
+	/* statistics */
+	u64 __percpu *cpuusage;
+	struct kernel_cpustat __percpu *cpustat;
+
 #ifdef CONFIG_FAIR_GROUP_SCHED
 	/* schedulable entities of this group on each cpu */
 	struct sched_entity **se;

@@ -575,6 +579,8 @@ static inline void set_task_rq(struct task_struct *p, unsigned int cpu)
 #endif
 }
 
+extern void task_group_charge(struct task_struct *tsk, u64 cputime);
+
 #else /* CONFIG_CGROUP_SCHED */
 
 static inline void set_task_rq(struct task_struct *p, unsigned int cpu) { }

@@ -582,6 +588,7 @@ static inline struct task_group *task_group(struct task_struct *p)
 {
 	return NULL;
 }
+static inline void task_group_charge(struct task_struct *tsk, u64 cputime) { }
 
 #endif /* CONFIG_CGROUP_SCHED */
 

index da5eb5b..e323790 100644
@@ -68,6 +68,7 @@ static void put_prev_task_stop(struct rq *rq, struct task_struct *prev)
 	account_group_exec_runtime(curr, delta_exec);
 
 	curr->se.exec_start = rq->clock_task;
+	task_group_charge(curr, delta_exec);
 	cpuacct_charge(curr, delta_exec);
 }
 

1.7.7.3

Hell, Glauber.

Yeah, the whole point of this series is enabling that and other optimizations.

Yeah yeah, sure, now you can build that on top of this patchset without worrying about cpuacct, which can also, hopefully, be removed fairly soon.

Thanks.

tejun

Hell, Glauber.

Yeah, the whole point of this series is enabling that and other optimizations.

Yeah yeah, sure, now you can build that on top of this patchset without worrying about cpuacct, which can also, hopefully, be removed fairly soon.

Thanks.

tejun

Now that cpu serves the same files as cpuacct and using cpuacct separately from cpu is deprecated, we can deprecate cpuacct. To avoid disturbing userland which has been co-mounting cpu and cpuacct, implement some hackery in cgroup core so that cpuacct co-mounting still works even if cpuacct is disabled.

The goal of this patch is to accelerate disabling and removal of cpuacct by decoupling kernel-side deprecation from userland changes. Userland is recommended to do the following.

* If /proc/cgroups lists cpuacct, always co-mount it with cpu under e.g. /sys/fs/cgroup/cpu.

* Optionally create symlinks for compatibility -
e.g. /sys/fs/cgroup/cpuacct and /sys/fs/cgroup/cpu,cpucct both pointing to /sys/fs/cgroup/cpu - whether cpuacct exists or not.

This compatibility hack will eventually go away.

---
 init/Kconfig        |   11 ++++++++++-
 kernel/cgroup.c     |   41 +++++++++++++++++++++++++++++++++++++++--
 kernel/sched/core.c |    2 ++
 3 files changed, 51 insertions(+), 3 deletions(-)

index af6c7f8..a13cf8f 100644
@@ -675,11 +675,20 @@ config PROC_PID_CPUSET
 	default y
 
 config CGROUP_CPUACCT
-	bool "Simple CPU accounting cgroup subsystem"
+	bool "DEPRECATED: Simple CPU accounting cgroup subsystem"
+	default n
 	help
 	  Provides a simple Resource Controller for monitoring the
 	  total CPU consumed by the tasks in a cgroup.
 
+	  This cgroup subsystem is deprecated.  The CPU cgroup
+	  subsystem serves the same accounting files and "cpuacct"
+	  mount option is ignored if specified with "cpu".  As long as
+	  userland co-mounts cpu and cpuacct, disabling this
+	  controller should be mostly unnoticeable - one notable
+	  difference is that /proc/PID/cgroup won't list cpuacct
+	  anymore.
+
 config RESOURCE_COUNTERS
 	bool "Resource counters"
 	help

index 9648671..10176a3 100644
@@ -8569,6 +8569,8 @@ struct cgroup_subsys cpu_cgroup_subsys = {
 
 #ifdef CONFIG_CGROUP_CPUACCT
 
+#warning CONFIG_CGROUP_CPUACCT is deprecated, read the Kconfig help message
+
 /*
  * CPU accounting code for task groups.
  *

1.7.7.3