sched: cpuacct: Track cpuusage per cpu frequency

New file: cpuacct.cpufreq when CONFIG_CPU_FREQ_STATS is enabled.

cpuacct.cpufreq accounts for cpu time per-cpu frequency, time is exported in nano-seconds

From the code, it appears that the file reports a breakdown of how

We do not know the cpufreq table size at compile time.
So a new config option CONFIG_CPUACCT_CPUFREQ_TABLE_MAX is intruduced, to determine the cpufreq table per-cpu in the cpuacct struct.

Signed-off-by Mike Chan <[email protected]>

---
 init/Kconfig   |    5 +++
 kernel/sched.c |   99 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++

 2 files changed, 104 insertions(+), 0 deletions(-)

init/Kconfig +0 -0

index eb77e8c..e1e86df 100644
@@ -534,6 +534,11 @@ config CGROUP_CPUACCT

         Provides a simple Resource Controller for monitoring the total CPU consumed by the tasks in a cgroup.

init/Kconfig +5 -0

+config CPUACCT_CPUFREQ_TABLE_MAX
+       int "Max CPUFREQ table size"
+       depends on CGROUP_CPUACCT && CPU_FREQ_TABLE
+       default 32
+

 config RESOURCE_COUNTERS
       bool "Resource counters"
       help

kernel/sched.c +99 -1

index 528a105..a0b56b5 100644
@@ -71,6 +71,7 @@
  #include <linux/debugfs.h>
  #include <linux/ctype.h>
  #include <linux/ftrace.h>
+#include <linux/cpufreq.h>
 
  #include <asm/tlb.h>
  #include <asm/irq_regs.h>

@@ -8817,6 +8818,11 @@ struct cgroup_subsys cpu_cgroup_subsys = {
  * ([email protected]).
  */
 
+#ifdef CONFIG_CPU_FREQ_STAT
+/* The alloc_percpu macro uses typeof so we must define a type here. */
+typedef struct { u64 usage[CONFIG_CPUACCT_CPUFREQ_TABLE_MAX]; } cpufreq_usage_t;
+#endif
+
  /* track cpu usage of a group of tasks and its child groups */
  struct cpuacct {
        struct cgroup_subsys_state css;

@@ -8824,6 +8830,10 @@ struct cpuacct {
        u64 __percpu *cpuusage;
        struct percpu_counter cpustat[CPUACCT_STAT_NSTATS];
        struct cpuacct *parent;
+#ifdef CONFIG_CPU_FREQ_STAT
+       /* cpufreq_usage is a pointer to an array of u64-types on every cpu */
+       cpufreq_usage_t *cpufreq_usage;
+#endif
  };
 
  struct cgroup_subsys cpuacct_subsys;

@@ -8856,6 +8866,10 @@ static struct cgroup_subsys_state *cpuacct_create(
        if (!ca->cpuusage)
                goto out_free_ca;
 
+#ifdef CONFIG_CPU_FREQ_STAT
+       ca->cpufreq_usage = alloc_percpu(cpufreq_usage_t);
+#endif
+
        for (i = 0; i < CPUACCT_STAT_NSTATS; i++)
                if (percpu_counter_init(&ca->cpustat[i], 0))
                        goto out_free_counters;

@@ -8888,6 +8902,74 @@ cpuacct_destroy(struct cgroup_subsys *ss, struct cgroup *cgrp)
        kfree(ca);
  }
 
+#ifdef CONFIG_CPU_FREQ_STAT
+static int cpufreq_index;
+static int cpuacct_cpufreq_notify(struct notifier_block *nb, unsigned long val,
+                                       void *data)
+{
+       int ret;
+       struct cpufreq_policy *policy;
+       struct cpufreq_freqs *freq = data;
+       struct cpufreq_frequency_table *table;
+
+       if (val != CPUFREQ_POSTCHANGE)
+               return 0;
+
+       /* Update cpufreq_index with current speed */
+       table = cpufreq_frequency_get_table(freq->cpu);
+       policy = cpufreq_cpu_get(freq->cpu);
+       ret = cpufreq_frequency_table_target(policy, table,
+                       cpufreq_quick_get(freq->cpu),
+                       CPUFREQ_RELATION_L, &cpufreq_index);
+       cpufreq_cpu_put(policy);
+       return ret;
+}
+
+static struct notifier_block cpufreq_notifier = {
+       .notifier_call = cpuacct_cpufreq_notify,
+};
+
+static __init int cpuacct_init(void)
+{
+       return cpufreq_register_notifier(&cpufreq_notifier,
+                                       CPUFREQ_TRANSITION_NOTIFIER);
+}
+
+module_init(cpuacct_init);
+
+static int cpuacct_cpufreq_show(struct cgroup *cgrp, struct cftype *cft,
+               struct cgroup_map_cb *cb)
+{
+       int i;
+       unsigned int cpu;
+       char buf[32];
+       struct cpuacct *ca = cgroup_ca(cgrp);
+       struct cpufreq_frequency_table *table =
+               cpufreq_frequency_get_table(smp_processor_id());
+
+       for (i = 0; table[i].frequency != CPUFREQ_TABLE_END; i++) {
+               u64 total = 0;
+
+               if (table[i].frequency == CPUFREQ_ENTRY_INVALID)
+                       continue;
+
+               for_each_present_cpu(cpu) {
+                       cpufreq_usage_t *cpufreq_usage;
+
+                       cpufreq_usage = per_cpu_ptr(ca->cpufreq_usage, cpu);
+                       table = cpufreq_frequency_get_table(cpu);
+
+                       total += cpufreq_usage->usage[i];
+               }
+
+               snprintf(buf, sizeof(buf), "%d", table[i].frequency);
+               cb->fill(cb, buf, total);
+       }
+
+       return 0;
+}
+#endif
+
  static u64 cpuacct_cpuusage_read(struct cpuacct *ca, int cpu)
  {
        u64 *cpuusage = per_cpu_ptr(ca->cpuusage, cpu);

@@ -9003,6 +9085,12 @@ static struct cftype files[] = {
                .name = "stat",
                .read_map = cpuacct_stats_show,
        },
+#ifdef CONFIG_CPU_FREQ_STAT
+       {
+               .name = "cpufreq",
+               .read_map = cpuacct_cpufreq_show,
+       },
+#endif
  };
 
  static int cpuacct_populate(struct cgroup_subsys *ss, struct cgroup *cgrp)

@@ -9031,6 +9119,17 @@ static void cpuacct_charge(struct task_struct *tsk, u64 cputime)
 
        for (; ca; ca = ca->parent) {
                u64 *cpuusage = per_cpu_ptr(ca->cpuusage, cpu);
+#ifdef CONFIG_CPU_FREQ_STAT
+               cpufreq_usage_t *cpufreq_usage =
+                       per_cpu_ptr(ca->cpufreq_usage, cpu);
+
+               if (cpufreq_index > CONFIG_CPUACCT_CPUFREQ_TABLE_MAX)
+                       printk_once(KERN_WARNING "cpuacct_charge: "
+                                       "cpufreq_index: %d exceeds max table "
+                                       "size\n", cpufreq_index);
+               else
+                       cpufreq_usage->usage[cpufreq_index] += cputime;
+#endif
                *cpuusage += cputime;
        }
 
--

1.7.0.1

Quoted 4 lines · Mike Chan

New file: cpuacct.cpufreq when CONFIG_CPU_FREQ_STATS is enabled.

cpuacct.cpufreq accounts for cpu time per-cpu frequency, time is exported in nano-seconds

Can you clarify the wording of this (and describe it in the relevant Documentation/... file)? It's not clear.

From the code, it appears that the file reports a breakdown of how much CPU time the cgroup has been consuming at each different CPU frequency level. If so, then you probably want to reword the description to avoid "per-cpu", since that makes it sounds as though it's reporting something, well, "per CPU".

Also, what's the motivation here? If it's for power monitoring purposes, might it be simpler to just report a single number, that's the integral of the CPU usage by frequency index (i.e. calculated from the same information that this patch is already gathering in cpuacct_charge()) rather than dumping a whole table on userspace?

Paul

Quoted 183 lines

We do not know the cpufreq table size at compile time.
So a new config option CONFIG_CPUACCT_CPUFREQ_TABLE_MAX is intruduced, to determine the cpufreq table per-cpu in the cpuacct struct.

Signed-off-by Mike Chan <[email protected]>

---
 init/Kconfig   |    5 +++
 kernel/sched.c |   99 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++

 2 files changed, 104 insertions(+), 0 deletions(-)

init/Kconfig +0 -0

index eb77e8c..e1e86df 100644
@@ -534,6 +534,11 @@ config CGROUP_CPUACCT

         Provides a simple Resource Controller for monitoring the total CPU consumed by the tasks in a cgroup.

init/Kconfig +5 -0

+config CPUACCT_CPUFREQ_TABLE_MAX
+       int "Max CPUFREQ table size"
+       depends on CGROUP_CPUACCT && CPU_FREQ_TABLE
+       default 32
+

 config RESOURCE_COUNTERS
       bool "Resource counters"
       help

kernel/sched.c +99 -1

index 528a105..a0b56b5 100644
@@ -71,6 +71,7 @@
  #include <linux/debugfs.h>
  #include <linux/ctype.h>
  #include <linux/ftrace.h>
+#include <linux/cpufreq.h>
 
  #include <asm/tlb.h>
  #include <asm/irq_regs.h>

@@ -8817,6 +8818,11 @@ struct cgroup_subsys cpu_cgroup_subsys = {
  * ([email protected]).
  */
 
+#ifdef CONFIG_CPU_FREQ_STAT
+/* The alloc_percpu macro uses typeof so we must define a type here. */
+typedef struct { u64 usage[CONFIG_CPUACCT_CPUFREQ_TABLE_MAX]; } cpufreq_usage_t;
+#endif
+
  /* track cpu usage of a group of tasks and its child groups */
  struct cpuacct {
        struct cgroup_subsys_state css;

@@ -8824,6 +8830,10 @@ struct cpuacct {
        u64 __percpu *cpuusage;
        struct percpu_counter cpustat[CPUACCT_STAT_NSTATS];
        struct cpuacct *parent;
+#ifdef CONFIG_CPU_FREQ_STAT
+       /* cpufreq_usage is a pointer to an array of u64-types on every cpu */
+       cpufreq_usage_t *cpufreq_usage;
+#endif
  };
 
  struct cgroup_subsys cpuacct_subsys;

@@ -8856,6 +8866,10 @@ static struct cgroup_subsys_state *cpuacct_create(
        if (!ca->cpuusage)
                goto out_free_ca;
 
+#ifdef CONFIG_CPU_FREQ_STAT
+       ca->cpufreq_usage = alloc_percpu(cpufreq_usage_t);
+#endif
+
        for (i = 0; i < CPUACCT_STAT_NSTATS; i++)
                if (percpu_counter_init(&ca->cpustat[i], 0))
                        goto out_free_counters;

@@ -8888,6 +8902,74 @@ cpuacct_destroy(struct cgroup_subsys *ss, struct cgroup *cgrp)
        kfree(ca);
  }
 
+#ifdef CONFIG_CPU_FREQ_STAT
+static int cpufreq_index;
+static int cpuacct_cpufreq_notify(struct notifier_block *nb, unsigned long val,
+                                       void *data)
+{
+       int ret;
+       struct cpufreq_policy *policy;
+       struct cpufreq_freqs *freq = data;
+       struct cpufreq_frequency_table *table;
+
+       if (val != CPUFREQ_POSTCHANGE)
+               return 0;
+
+       /* Update cpufreq_index with current speed */
+       table = cpufreq_frequency_get_table(freq->cpu);
+       policy = cpufreq_cpu_get(freq->cpu);
+       ret = cpufreq_frequency_table_target(policy, table,
+                       cpufreq_quick_get(freq->cpu),
+                       CPUFREQ_RELATION_L, &cpufreq_index);
+       cpufreq_cpu_put(policy);
+       return ret;
+}
+
+static struct notifier_block cpufreq_notifier = {
+       .notifier_call = cpuacct_cpufreq_notify,
+};
+
+static __init int cpuacct_init(void)
+{
+       return cpufreq_register_notifier(&cpufreq_notifier,
+                                       CPUFREQ_TRANSITION_NOTIFIER);
+}
+
+module_init(cpuacct_init);
+
+static int cpuacct_cpufreq_show(struct cgroup *cgrp, struct cftype *cft,
+               struct cgroup_map_cb *cb)
+{
+       int i;
+       unsigned int cpu;
+       char buf[32];
+       struct cpuacct *ca = cgroup_ca(cgrp);
+       struct cpufreq_frequency_table *table =
+               cpufreq_frequency_get_table(smp_processor_id());
+
+       for (i = 0; table[i].frequency != CPUFREQ_TABLE_END; i++) {
+               u64 total = 0;
+
+               if (table[i].frequency == CPUFREQ_ENTRY_INVALID)
+                       continue;
+
+               for_each_present_cpu(cpu) {
+                       cpufreq_usage_t *cpufreq_usage;
+
+                       cpufreq_usage = per_cpu_ptr(ca->cpufreq_usage, cpu);
+                       table = cpufreq_frequency_get_table(cpu);
+
+                       total += cpufreq_usage->usage[i];
+               }
+
+               snprintf(buf, sizeof(buf), "%d", table[i].frequency);
+               cb->fill(cb, buf, total);
+       }
+
+       return 0;
+}
+#endif
+
  static u64 cpuacct_cpuusage_read(struct cpuacct *ca, int cpu)
  {
        u64 *cpuusage = per_cpu_ptr(ca->cpuusage, cpu);

@@ -9003,6 +9085,12 @@ static struct cftype files[] = {
                .name = "stat",
                .read_map = cpuacct_stats_show,
        },
+#ifdef CONFIG_CPU_FREQ_STAT
+       {
+               .name = "cpufreq",
+               .read_map = cpuacct_cpufreq_show,
+       },
+#endif
  };
 
  static int cpuacct_populate(struct cgroup_subsys *ss, struct cgroup *cgrp)

@@ -9031,6 +9119,17 @@ static void cpuacct_charge(struct task_struct *tsk, u64 cputime)
 
        for (; ca; ca = ca->parent) {
                u64 *cpuusage = per_cpu_ptr(ca->cpuusage, cpu);
+#ifdef CONFIG_CPU_FREQ_STAT
+               cpufreq_usage_t *cpufreq_usage =
+                       per_cpu_ptr(ca->cpufreq_usage, cpu);
+
+               if (cpufreq_index > CONFIG_CPUACCT_CPUFREQ_TABLE_MAX)
+                       printk_once(KERN_WARNING "cpuacct_charge: "
+                                       "cpufreq_index: %d exceeds max table "
+                                       "size\n", cpufreq_index);
+               else
+                       cpufreq_usage->usage[cpufreq_index] += cputime;
+#endif
                *cpuusage += cputime;
        }
 
--

1.7.0.1

Quoted 4 lines · Mike Chan

New file: cpuacct.cpufreq when CONFIG_CPU_FREQ_STATS is enabled.

cpuacct.cpufreq accounts for cpu time per-cpu frequency, time is exported in nano-seconds

Can you clarify the wording of this (and describe it in the relevant Documentation/... file)? It's not clear.

From the code, it appears that the file reports a breakdown of how much CPU time the cgroup has been consuming at each different CPU frequency level. If so, then you probably want to reword the description to avoid "per-cpu", since that makes it sounds as though it's reporting something, well, "per CPU".

Also, what's the motivation here? If it's for power monitoring purposes, might it be simpler to just report a single number, that's the integral of the CPU usage by frequency index (i.e. calculated from the same information that this patch is already gathering in cpuacct_charge()) rather than dumping a whole table on userspace?

Quoted 15 lines · * [email protected] <[email protected]> [2010-04-05 12:52:57]:

Quoted 4 lines · Mike Chan

New file: cpuacct.cpufreq when CONFIG_CPU_FREQ_STATS is enabled.

cpuacct.cpufreq accounts for cpu time per-cpu frequency, time is exported in nano-seconds

Can you clarify the wording of this (and describe it in the relevant Documentation/... file)? It's not clear.

From the code, it appears that the file reports a breakdown of how much CPU time the cgroup has been consuming at each different CPU frequency level. If so, then you probably want to reword the description to avoid "per-cpu", since that makes it sounds as though it's reporting something, well, "per CPU".

Quoted 5 lines

Also, what's the motivation here? If it's for power monitoring purposes, might it be simpler to just report a single number, that's the integral of the CPU usage by frequency index (i.e. calculated from the same information that this patch is already gathering in cpuacct_charge()) rather than dumping a whole table on userspace?

As utilization increases, won't the integral quickly overflow? BTW, Mike have you looked at the scaled accounting infrastructure we have in taskstats?

--
       Three Cheers,
       Balbir

New file: cpuacct.cpufreq when CONFIG_CPU_FREQ_STATS is enabled.

cpuacct.cpufreq accounts for cpu time per-cpu frequency, time is exported in nano-seconds

We do not know the cpufreq table size at compile time.
So a new config option CONFIG_CPUACCT_CPUFREQ_TABLE_MAX is intruduced, to determine the cpufreq table per-cpu in the cpuacct struct.

Signed-off-by Mike Chan <[email protected]>

---
 init/Kconfig   |    5 +++
 kernel/sched.c |   99 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 104 insertions(+), 0 deletions(-)

init/Kconfig +5 -0

index eb77e8c..e1e86df 100644
@@ -534,6 +534,11 @@ config CGROUP_CPUACCT
 	  Provides a simple Resource Controller for monitoring the
 	  total CPU consumed by the tasks in a cgroup.
 
+config CPUACCT_CPUFREQ_TABLE_MAX
+	int "Max CPUFREQ table size"
+	depends on CGROUP_CPUACCT && CPU_FREQ_TABLE
+	default 32
+
 config RESOURCE_COUNTERS
 	bool "Resource counters"
 	help

kernel/sched.c +22 -0

index 528a105..a0b56b5 100644
@@ -71,6 +71,7 @@
 #include <linux/debugfs.h>
 #include <linux/ctype.h>
 #include <linux/ftrace.h>
+#include <linux/cpufreq.h>
 
 #include <asm/tlb.h>
 #include <asm/irq_regs.h>

@@ -8817,6 +8818,11 @@ struct cgroup_subsys cpu_cgroup_subsys = {
  * ([email protected]).
  */
 
+#ifdef CONFIG_CPU_FREQ_STAT
+/* The alloc_percpu macro uses typeof so we must define a type here. */
+typedef struct { u64 usage[CONFIG_CPUACCT_CPUFREQ_TABLE_MAX]; } cpufreq_usage_t;
+#endif
+
 /* track cpu usage of a group of tasks and its child groups */
 struct cpuacct {
 	struct cgroup_subsys_state css;

@@ -8824,6 +8830,10 @@ struct cpuacct {
 	u64 __percpu *cpuusage;
 	struct percpu_counter cpustat[CPUACCT_STAT_NSTATS];
 	struct cpuacct *parent;
+#ifdef CONFIG_CPU_FREQ_STAT
+	/* cpufreq_usage is a pointer to an array of u64-types on every cpu */
+	cpufreq_usage_t *cpufreq_usage;
+#endif
 };
 
 struct cgroup_subsys cpuacct_subsys;

@@ -8856,6 +8866,10 @@ static struct cgroup_subsys_state *cpuacct_create(
 	if (!ca->cpuusage)
 		goto out_free_ca;
 
+#ifdef CONFIG_CPU_FREQ_STAT
+	ca->cpufreq_usage = alloc_percpu(cpufreq_usage_t);
+#endif
+
 	for (i = 0; i < CPUACCT_STAT_NSTATS; i++)
 		if (percpu_counter_init(&ca->cpustat[i], 0))
 			goto out_free_counters;

@@ -8888,6 +8902,74 @@ cpuacct_destroy(struct cgroup_subsys *ss, struct cgroup *cgrp)
 	kfree(ca);
 }
 
+#ifdef CONFIG_CPU_FREQ_STAT
+static int cpufreq_index;
+static int cpuacct_cpufreq_notify(struct notifier_block *nb, unsigned long val,
+					void *data)
+{
+	int ret;
+	struct cpufreq_policy *policy;
+	struct cpufreq_freqs *freq = data;

kernel/sched.c +23 -0

+	cpufreq_cpu_put(policy);
+	return ret;
+}
+
+static struct notifier_block cpufreq_notifier = {
+	.notifier_call = cpuacct_cpufreq_notify,
+};
+
+static __init int cpuacct_init(void)
+{
+	return cpufreq_register_notifier(&cpufreq_notifier,
+					CPUFREQ_TRANSITION_NOTIFIER);
+}
+
+module_init(cpuacct_init);
+
+static int cpuacct_cpufreq_show(struct cgroup *cgrp, struct cftype *cft,
+		struct cgroup_map_cb *cb)
+{
+	int i;
+	unsigned int cpu;
+	char buf[32];
+	struct cpuacct *ca = cgroup_ca(cgrp);

kernel/sched.c +22 -0

+			cpufreq_usage_t *cpufreq_usage;
+
+			cpufreq_usage = per_cpu_ptr(ca->cpufreq_usage, cpu);
+			table = cpufreq_frequency_get_table(cpu);
+
+			total += cpufreq_usage->usage[i];
+		}
+
+		snprintf(buf, sizeof(buf), "%d", table[i].frequency);
+		cb->fill(cb, buf, total);
+	}
+
+	return 0;
+}
+#endif
+
 static u64 cpuacct_cpuusage_read(struct cpuacct *ca, int cpu)
 {
 	u64 *cpuusage = per_cpu_ptr(ca->cpuusage, cpu);

@@ -9003,6 +9085,12 @@ static struct cftype files[] = {
 		.name = "stat",
 		.read_map = cpuacct_stats_show,
 	},
+#ifdef CONFIG_CPU_FREQ_STAT
+	{
+		.name = "cpufreq",
+		.read_map = cpuacct_cpufreq_show,
+	},
+#endif
 };
 
 static int cpuacct_populate(struct cgroup_subsys *ss, struct cgroup *cgrp)

@@ -9031,6 +9119,17 @@ static void cpuacct_charge(struct task_struct *tsk, u64 cputime)
 

kernel/sched.c +3 -0

+		else
+			cpufreq_usage->usage[cpufreq_index] += cputime;
+#endif
 		*cpuusage += cputime;
 	}
 

Signature · 1 line

1.7.0.1

Quoted 94 lines · Mike Chan

New file: cpuacct.cpufreq when CONFIG_CPU_FREQ_STATS is enabled.

cpuacct.cpufreq accounts for cpu time per-cpu frequency, time is exported in nano-seconds

We do not know the cpufreq table size at compile time.
So a new config option CONFIG_CPUACCT_CPUFREQ_TABLE_MAX is intruduced, to determine the cpufreq table per-cpu in the cpuacct struct.

Signed-off-by Mike Chan <[email protected]>

---
 init/Kconfig   |    5 +++
 kernel/sched.c |   99 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++

 2 files changed, 104 insertions(+), 0 deletions(-)

init/Kconfig +0 -0

index eb77e8c..e1e86df 100644
@@ -534,6 +534,11 @@ config CGROUP_CPUACCT

        Provides a simple Resource Controller for monitoring the total CPU consumed by the tasks in a cgroup.

init/Kconfig +5 -0

+config CPUACCT_CPUFREQ_TABLE_MAX
+     int "Max CPUFREQ table size"
+     depends on CGROUP_CPUACCT && CPU_FREQ_TABLE
+     default 32
+

 config RESOURCE_COUNTERS
      bool "Resource counters"
      help

kernel/sched.c +27 -0

index 528a105..a0b56b5 100644
@@ -71,6 +71,7 @@
  #include <linux/debugfs.h>
  #include <linux/ctype.h>
  #include <linux/ftrace.h>
+#include <linux/cpufreq.h>
 
  #include <asm/tlb.h>
  #include <asm/irq_regs.h>

@@ -8817,6 +8818,11 @@ struct cgroup_subsys cpu_cgroup_subsys = {
   * ([email protected]).
   */
 
+#ifdef CONFIG_CPU_FREQ_STAT
+/* The alloc_percpu macro uses typeof so we must define a type here. */
+typedef struct { u64 usage[CONFIG_CPUACCT_CPUFREQ_TABLE_MAX]; } cpufreq_usage_t;
+#endif
+
  /* track cpu usage of a group of tasks and its child groups */
  struct cpuacct {
       struct cgroup_subsys_state css;

@@ -8824,6 +8830,10 @@ struct cpuacct {
       u64 __percpu *cpuusage;
       struct percpu_counter cpustat[CPUACCT_STAT_NSTATS];
       struct cpuacct *parent;
+#ifdef CONFIG_CPU_FREQ_STAT
+     /* cpufreq_usage is a pointer to an array of u64-types on every cpu */
+     cpufreq_usage_t *cpufreq_usage;
+#endif
  };
 
  struct cgroup_subsys cpuacct_subsys;

@@ -8856,6 +8866,10 @@ static struct cgroup_subsys_state *cpuacct_create(
       if (!ca->cpuusage)
               goto out_free_ca;
 
+#ifdef CONFIG_CPU_FREQ_STAT
+     ca->cpufreq_usage = alloc_percpu(cpufreq_usage_t);
+#endif
+
       for (i = 0; i < CPUACCT_STAT_NSTATS; i++)
               if (percpu_counter_init(&ca->cpustat[i], 0))
                       goto out_free_counters;

@@ -8888,6 +8902,74 @@ cpuacct_destroy(struct cgroup_subsys *ss, struct cgroup *cgrp)
       kfree(ca);
  }
 
+#ifdef CONFIG_CPU_FREQ_STAT
+static int cpufreq_index;
+static int cpuacct_cpufreq_notify(struct notifier_block *nb, unsigned long val,
+                                     void *data)
+{
+     int ret;
+     struct cpufreq_policy *policy;
+     struct cpufreq_freqs *freq = data;
+     struct cpufreq_frequency_table *table;
+
+     if (val != CPUFREQ_POSTCHANGE)
+             return 0;
+

kernel/sched.c cpuacct_destroy(struct cgroup_subsys *ss, struct cgroup *cgrp) · line 8922

Quoted 5 lines

kernel/sched.c +5 -0

+     /* Update cpufreq_index with current speed */
+     table = cpufreq_frequency_get_table(freq->cpu);
+     policy = cpufreq_cpu_get(freq->cpu);
+     ret = cpufreq_frequency_table_target(policy, table,
+                     cpufreq_quick_get(freq->cpu),

Since you've already done a cpufreq_cpu_get, can't you directly dereference policy->cur here?

kernel/sched.c cpuacct_destroy(struct cgroup_subsys *ss, struct cgroup *cgrp) · line 8923

Quoted 2 lines

kernel/sched.c +1 -0

+                     CPUFREQ_RELATION_L, &cpufreq_index);

The code is unclear

Quoted 28 lines

kernel/sched.c +28 -0

+     cpufreq_cpu_put(policy);
+     return ret;
+}
+
+static struct notifier_block cpufreq_notifier = {
+     .notifier_call = cpuacct_cpufreq_notify,
+};
+
+static __init int cpuacct_init(void)
+{
+     return cpufreq_register_notifier(&cpufreq_notifier,
+                                     CPUFREQ_TRANSITION_NOTIFIER);
+}
+
+module_init(cpuacct_init);
+
+static int cpuacct_cpufreq_show(struct cgroup *cgrp, struct cftype *cft,
+             struct cgroup_map_cb *cb)
+{
+     int i;
+     unsigned int cpu;
+     char buf[32];
+     struct cpuacct *ca = cgroup_ca(cgrp);
+     struct cpufreq_frequency_table *table =
+             cpufreq_frequency_get_table(smp_processor_id());
+
+     for (i = 0; table[i].frequency != CPUFREQ_TABLE_END; i++) {
+             u64 total = 0;

kernel/sched.c cpuacct_destroy(struct cgroup_subsys *ss, struct cgroup *cgrp) · line 8956

Quoted 5 lines

kernel/sched.c +5 -0

+
+             if (table[i].frequency == CPUFREQ_ENTRY_INVALID)
+                     continue;
+
+             for_each_present_cpu(cpu) {

Why do we care about all present cpus? A comment would be nice, is this an ABI thing? Do we care about data of offline cpus, etc.

Quoted 35 lines

kernel/sched.c +22 -0

+                     cpufreq_usage_t *cpufreq_usage;
+
+                     cpufreq_usage = per_cpu_ptr(ca->cpufreq_usage, cpu);
+                     table = cpufreq_frequency_get_table(cpu);
+
+                     total += cpufreq_usage->usage[i];
+             }
+
+             snprintf(buf, sizeof(buf), "%d", table[i].frequency);
+             cb->fill(cb, buf, total);
+     }
+
+     return 0;
+}
+#endif
+
  static u64 cpuacct_cpuusage_read(struct cpuacct *ca, int cpu)
  {
       u64 *cpuusage = per_cpu_ptr(ca->cpuusage, cpu);

@@ -9003,6 +9085,12 @@ static struct cftype files[] = {
               .name = "stat",
               .read_map = cpuacct_stats_show,
       },
+#ifdef CONFIG_CPU_FREQ_STAT
+     {
+             .name = "cpufreq",
+             .read_map = cpuacct_cpufreq_show,
+     },
+#endif
  };
 
  static int cpuacct_populate(struct cgroup_subsys *ss, struct cgroup *cgrp)

@@ -9031,6 +9119,17 @@ static void cpuacct_charge(struct task_struct *tsk, u64 cputime)
 

kernel/sched.c static void cpuacct_charge(struct task_struct *tsk, u64 cputime) · line 9129

Quoted 10 lines

kernel/sched.c +8 -0

       for (; ca; ca = ca->parent) {
               u64 *cpuusage = per_cpu_ptr(ca->cpuusage, cpu);
+#ifdef CONFIG_CPU_FREQ_STAT
+             cpufreq_usage_t *cpufreq_usage =
+                     per_cpu_ptr(ca->cpufreq_usage, cpu);
+
+             if (cpufreq_index > CONFIG_CPUACCT_CPUFREQ_TABLE_MAX)
+                     printk_once(KERN_WARNING "cpuacct_charge: "
+                                     "cpufreq_index: %d exceeds max table "
+                                     "size\n", cpufreq_index);

WARN_ON_ONCE() would be more readable, IMHO

Quoted 10 lines

kernel/sched.c +3 -1

+             else
+                     cpufreq_usage->usage[cpufreq_index] += cputime;
+#endif
               *cpuusage += cputime;
       }
 
--

1.7.0.1

--
       Three Cheers,
       Balbir

New file: cpuacct.cpufreq when CONFIG_CPU_FREQ_STATS is enabled.

cpuacct.cpufreq accounts for cpu time per-cpu frequency, time is exported in nano-seconds

We do not know the cpufreq table size at compile time.
So a new config option CONFIG_CPUACCT_CPUFREQ_TABLE_MAX is intruduced, to determine the cpufreq table per-cpu in the cpuacct struct.

Signed-off-by Mike Chan <[email protected]>

---
 init/Kconfig   |    5 +++
 kernel/sched.c |   99 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 104 insertions(+), 0 deletions(-)

init/Kconfig +0 -0

index eb77e8c..e1e86df 100644

init/Kconfig +0 -0

 config RESOURCE_COUNTERS
 	bool "Resource counters"
 	help

kernel/sched.c +1 -0

index 528a105..a0b56b5 100644
@@ -71,6 +71,7 @@
 #include <linux/debugfs.h>
 #include <linux/ctype.h>
 #include <linux/ftrace.h>
+#include <linux/cpufreq.h>
 
 #include <asm/tlb.h>
 #include <asm/irq_regs.h>

Quoted 23 lines · Mike Chan

New file: cpuacct.cpufreq when CONFIG_CPU_FREQ_STATS is enabled.

cpuacct.cpufreq accounts for cpu time per-cpu frequency, time is exported in nano-seconds

We do not know the cpufreq table size at compile time.
So a new config option CONFIG_CPUACCT_CPUFREQ_TABLE_MAX is intruduced, to determine the cpufreq table per-cpu in the cpuacct struct.

Signed-off-by Mike Chan <[email protected]>

---
 init/Kconfig   |    5 +++
 kernel/sched.c |   99 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++

 2 files changed, 104 insertions(+), 0 deletions(-)

init/Kconfig +0 -0

index eb77e8c..e1e86df 100644
@@ -534,6 +534,11 @@ config CGROUP_CPUACCT

        Provides a simple Resource Controller for monitoring the total CPU consumed by the tasks in a cgroup.

init/Kconfig config CGROUP_CPUACCT · line 541

Quoted 5 lines

init/Kconfig +5 -0

+config CPUACCT_CPUFREQ_TABLE_MAX
+     int "Max CPUFREQ table size"
+     depends on CGROUP_CPUACCT && CPU_FREQ_TABLE
+     default 32
+

I'd say make it just a regular define unless you can think of a reason why a non-developer would want to touch this value.

Quoted 17 lines

 config RESOURCE_COUNTERS
      bool "Resource counters"
      help

kernel/sched.c +1 -0

index 528a105..a0b56b5 100644
@@ -71,6 +71,7 @@
  #include <linux/debugfs.h>
  #include <linux/ctype.h>
  #include <linux/ftrace.h>
+#include <linux/cpufreq.h>
 
  #include <asm/tlb.h>
  #include <asm/irq_regs.h>

kernel/sched.c struct cgroup_subsys cpu_cgroup_subsys = { · line 8824

Quoted 7 lines

kernel/sched.c +4 -0

@@ -8817,6 +8818,11 @@ struct cgroup_subsys cpu_cgroup_subsys = {
   * ([email protected]).
   */
 
+#ifdef CONFIG_CPU_FREQ_STAT
+/* The alloc_percpu macro uses typeof so we must define a type here. */
+typedef struct { u64 usage[CONFIG_CPUACCT_CPUFREQ_TABLE_MAX]; } cpufreq_usage_t;
+#endif

you should be able to send it a regular struct . I found this usage,

lport->dev_stats = alloc_percpu(struct fcoe_dev_stats);

as an example.

Daniel

Quoted 24 lines · Daniel Walker

Quoted 23 lines · Mike Chan

New file: cpuacct.cpufreq when CONFIG_CPU_FREQ_STATS is enabled.

cpuacct.cpufreq accounts for cpu time per-cpu frequency, time is exported in nano-seconds

We do not know the cpufreq table size at compile time.
So a new config option CONFIG_CPUACCT_CPUFREQ_TABLE_MAX is intruduced, to determine the cpufreq table per-cpu in the cpuacct struct.

Signed-off-by Mike Chan <[email protected]>

---
 init/Kconfig   |    5 +++
 kernel/sched.c |   99 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 104 insertions(+), 0 deletions(-)

init/Kconfig +0 -0

index eb77e8c..e1e86df 100644
@@ -534,6 +534,11 @@ config CGROUP_CPUACCT
        Provides a simple Resource Controller for monitoring the
        total CPU consumed by the tasks in a cgroup.
 

Quoted 8 lines

init/Kconfig config CGROUP_CPUACCT · line 541

Quoted 5 lines

init/Kconfig +5 -0

+config CPUACCT_CPUFREQ_TABLE_MAX
+     int "Max CPUFREQ table size"
+     depends on CGROUP_CPUACCT && CPU_FREQ_TABLE
+     default 32
+

I'd say make it just a regular define unless you can think of a reason why a non-developer would want to touch this value.

I originally thought about doing this, but my concern here is for future (or existing) cpu's that have more than 32 speed stepping. This will have to be updated as new cpu's are supported in mainline (if they exceed the max). Which might be acceptable or not.

Ideally it would be nice to be able to pull the table size from the board-file, or determine the size at run-time instead of compile time.