Re: [RFC][-mm] [1/2] Simple stats for cpu resource controller

Previous thread: [RFC][-mm] [0/2] Basic stats for cgroups by Balaji Rao on Wednesday, March 26, 2008 - 2:18 pm. (1 message)

Next thread: [RFC][-mm] [2/2] Simple stats for memory resource controller by Balaji Rao on Wednesday, March 26, 2008 - 2:18 pm. (4 messages)
To: <linux-kernel@...>
Cc: <menage@...>, <balbir@...>, <containers@...>, <dhaval@...>
Date: Wednesday, March 26, 2008 - 2:18 pm

This patch implements trivial statistics for the cpu controller.

Signed-off-by: Balaji Rao <balajirrao@gmail.com>
CC: Balbir Singh <balbir@linux.vnet.ibm.com>
CC: Dhaval Giani <dhaval@linux.vnet.ibm.com>

diff --git a/kernel/sched.c b/kernel/sched.c
index 9fbfa05..eac9333 100644
--- a/kernel/sched.c
+++ b/kernel/sched.c
@@ -164,10 +164,38 @@ struct cfs_rq;

static LIST_HEAD(task_groups);

+#ifdef CONFIG_CGROUP_SCHED
+enum cpu_cgroup_stat_index {
+ CPU_CGROUP_STAT_UTIME, /* Usertime of the task group */
+ CPU_CGROUP_STAT_STIME, /* Kerneltime of the task group */
+
+ CPU_CGROUP_STAT_NSTATS,
+};
+
+struct cpu_cgroup_stat_cpu {
+ s64 count[CPU_CGROUP_STAT_NSTATS];
+} ____cacheline_aligned_in_smp;
+
+struct cpu_cgroup_stat {
+ struct cpu_cgroup_stat_cpu cpustat[NR_CPUS];
+};
+
+/* Called under irq disable. */
+static void __cpu_cgroup_stat_add_safe(struct cpu_cgroup_stat *stat,
+ enum cpu_cgroup_stat_index idx, int val)
+{
+ int cpu = smp_processor_id();
+
+ BUG_ON(!irqs_disabled());
+ stat->cpustat[cpu].count[idx] += val;
+}
+#endif
+
/* task group related information */
struct task_group {
#ifdef CONFIG_CGROUP_SCHED
struct cgroup_subsys_state css;
+ struct cpu_cgroup_stat stat;
#endif

#ifdef CONFIG_FAIR_GROUP_SCHED
@@ -3670,6 +3698,16 @@ void account_user_time(struct task_struct *p, cputime_t cputime)
cpustat->nice = cputime64_add(cpustat->nice, tmp);
else
cpustat->user = cputime64_add(cpustat->user, tmp);
+
+ /* Charge the task's group */
+#ifdef CONFIG_CGROUP_SCHED
+ {
+ struct task_group *tg;
+ tg = task_group(p);
+ __cpu_cgroup_stat_add_safe(&tg->stat, CPU_CGROUP_STAT_UTIME,
+ cputime_to_msecs(cputime));
+ }
+#endif
}

/*
@@ -3733,6 +3771,15 @@ void account_system_time(struct task_struct *p, int hardirq_offset,
cpustat->idle = cputime64_add(cpustat->idle, tmp);
/* Account for system time used */
acct_update_integrals(p);
+
+#ifdef CONFIG_CGROUP_SCHED
+ {
+ stru...

To: Balaji Rao <balajirrao@...>
Cc: <linux-kernel@...>, <menage@...>, <balbir@...>, <containers@...>, <dhaval@...>
Date: Wednesday, March 26, 2008 - 3:58 pm

--

To: Peter Zijlstra <a.p.zijlstra@...>
Cc: <linux-kernel@...>, <menage@...>, <balbir@...>, <containers@...>, <dhaval@...>
Date: Friday, March 28, 2008 - 6:02 am

Thanks for the review.
--
regards,
balaji rao
--

To: Balaji Rao <balajirrao@...>
Cc: <linux-kernel@...>, <menage@...>, <balbir@...>, <containers@...>, <dhaval@...>
Date: Friday, March 28, 2008 - 6:17 am

It can be called from any context that has hardirqs disabled. And the __
prefix suggests as much, no need to tag _safe to the end as well, we

Not sure what you want to use this for, but yeah, that makes most sense.

That is, I do know _what_ you want to use it for, just not sure which
requirements you put on it. The pure tick based thing might be good
enough for most purposes, the runtime proportion thing is just more

--

To: Balaji Rao <balajirrao@...>
Cc: <linux-kernel@...>, <balbir@...>, <containers@...>, <dhaval@...>
Date: Wednesday, March 26, 2008 - 3:00 pm

On a 32-bit architecture I think this could race with a non-atomic
update that crosses a 32-bit boundary and get a corrupted result.

Paul
--

Previous thread: [RFC][-mm] [0/2] Basic stats for cgroups by Balaji Rao on Wednesday, March 26, 2008 - 2:18 pm. (1 message)

Next thread: [RFC][-mm] [2/2] Simple stats for memory resource controller by Balaji Rao on Wednesday, March 26, 2008 - 2:18 pm. (4 messages)