Re: [PATCH v2 03/14] Move /proc/stat logic inside sched.c

[Paul Turner <pjt@google.com>]

On 11/01/2011 02:19 PM, Glauber Costa wrote:
> This patch moves all of the /proc/stat display code inside
> sched.c. The goal is to later on, have a different version
> of it per-cgroup. In containers environment, this is useful
> to give each container a different and independent view of
> the statistics displayed in this file.
>
> Signed-off-by: Glauber Costa<glommer@parallels.com>
> ---
>   fs/proc/stat.c        |  139 +-----------------------------------------------
>   include/linux/sched.h |    1 +
>   kernel/sched.c        |  142 +++++++++++++++++++++++++++++++++++++++++++++++++
>   3 files changed, 144 insertions(+), 138 deletions(-)
>

This feels a little contrived.  sched.c isn't the right place for much of this 
code.  Why do you want to move it all instead of exporting the functionality 
(e.g. remove static?).

> diff --git a/kernel/sched.c b/kernel/sched.c
> index e78e1aa..3f42916 100644
> --- a/kernel/sched.c
> +++ b/kernel/sched.c
> @@ -71,6 +71,7 @@
>   #include<linux/ctype.h>
>   #include<linux/ftrace.h>
>   #include<linux/slab.h>
> +#include<linux/tick.h>
>
>   #include<asm/tlb.h>
>   #include<asm/irq_regs.h>
> @@ -9494,6 +9495,147 @@ struct cgroup_subsys cpu_cgroup_subsys = {
>
>   #endif	/* CONFIG_CGROUP_SCHED */
>
> +#ifndef fi
> +#define arch_irq_stat_cpu(cpu) 0
> +#endif
> +#ifndef arch_irq_stat
> +#define arch_irq_stat() 0
> +#endif
> +#ifndef arch_idle_time
> +#define arch_idle_time(cpu) 0
> +#endif
> +
> +static u64 get_idle_time(int cpu)
> +{
> +	u64 idle, idle_time = get_cpu_idle_time_us(cpu, NULL);
> +
> +	if (idle_time == -1ULL) {
> +		/* !NO_HZ so we can rely on cpustat.idle */
> +		idle = kstat_cpu(cpu).cpustat[IDLE];
> +		idle += arch_idle_time(cpu);
> +	} else
> +		idle = usecs_to_cputime(idle_time);
> +
> +	return idle;
> +}
> +
> +static u64 get_iowait_time(int cpu)
> +{
> +	u64 iowait, iowait_time = get_cpu_iowait_time_us(cpu, NULL);
> +
> +	if (iowait_time == -1ULL)
> +		/* !NO_HZ so we can rely on cpustat.iowait */
> +		iowait = kstat_cpu(cpu).cpustat[IOWAIT];
> +	else
> +		iowait = usecs_to_cputime(iowait_time);
> +
> +	return iowait;
> +}
> +
> +int cpu_cgroup_proc_stat(struct seq_file *p)
> +{
> +	int i, j;
> +	unsigned long jif;
> +	u64 user, nice, system, idle, iowait, irq, softirq, steal;
> +	u64 guest, guest_nice;
> +	u64 sum = 0;
> +	u64 sum_softirq = 0;
> +	unsigned int per_softirq_sums[NR_SOFTIRQS] = {0};
> +	struct timespec boottime;
> +
> +	user = nice = system = idle = iowait =
> +		irq = softirq = steal = 0;
> +	guest = guest_nice = 0;
> +	getboottime(&boottime);
> +	jif = boottime.tv_sec;
> +
> +	for_each_possible_cpu(i) {
> +		user += kstat_cpu(i).cpustat[USER];
> +		nice += kstat_cpu(i).cpustat[NICE];
> +		system += kstat_cpu(i).cpustat[SYSTEM];
> +		idle += get_idle_time(i);
> +		iowait += get_iowait_time(i);
> +		irq += kstat_cpu(i).cpustat[IRQ];
> +		softirq += kstat_cpu(i).cpustat[SOFTIRQ];
> +		steal += kstat_cpu(i).cpustat[STEAL];
> +		guest += kstat_cpu(i).cpustat[GUEST];
> +		guest_nice += kstat_cpu(i).cpustat[GUEST_NICE];
> +
> +		for (j = 0; j<  NR_SOFTIRQS; j++) {
> +			unsigned int softirq_stat = kstat_softirqs_cpu(j, i);
> +
> +			per_softirq_sums[j] += softirq_stat;
> +			sum_softirq += softirq_stat;
> +		}
> +	}
> +	sum += arch_irq_stat();
> +
> +	seq_printf(p, "cpu  %llu %llu %llu %llu %llu %llu %llu %llu %llu "
> +		"%llu\n",
> +		(unsigned long long)cputime64_to_clock_t(user),
> +		(unsigned long long)cputime64_to_clock_t(nice),
> +		(unsigned long long)cputime64_to_clock_t(system),
> +		(unsigned long long)cputime64_to_clock_t(idle),
> +		(unsigned long long)cputime64_to_clock_t(iowait),
> +		(unsigned long long)cputime64_to_clock_t(irq),
> +		(unsigned long long)cputime64_to_clock_t(softirq),
> +		(unsigned long long)cputime64_to_clock_t(steal),
> +		(unsigned long long)cputime64_to_clock_t(guest),
> +		(unsigned long long)cputime64_to_clock_t(guest_nice));
> +	for_each_online_cpu(i) {
> +		/* Copy values here to work around gcc-2.95.3, gcc-2.96 */

GCC 3.2 is the listed current minimum requirement.  If this code is being 
revisited this could be cleaned up.

> +		user = kstat_cpu(i).cpustat[USER];
> +		nice = kstat_cpu(i).cpustat[NICE];
> +		system = kstat_cpu(i).cpustat[SYSTEM];
> +		idle = get_idle_time(i);
> +		iowait = get_iowait_time(i);
> +		irq = kstat_cpu(i).cpustat[IRQ];
> +		softirq = kstat_cpu(i).cpustat[SOFTIRQ];
> +		steal = kstat_cpu(i).cpustat[STEAL];
> +		guest = kstat_cpu(i).cpustat[GUEST];
> +		guest_nice = kstat_cpu(i).cpustat[GUEST_NICE];
> +		seq_printf(p,
> +			"cpu%d %llu %llu %llu %llu %llu %llu %llu %llu %llu "
> +			"%llu\n",
> +			i,
> +			(unsigned long long)cputime64_to_clock_t(user),
> +			(unsigned long long)cputime64_to_clock_t(nice),
> +			(unsigned long long)cputime64_to_clock_t(system),
> +			(unsigned long long)cputime64_to_clock_t(idle),
> +			(unsigned long long)cputime64_to_clock_t(iowait),
> +			(unsigned long long)cputime64_to_clock_t(irq),
> +			(unsigned long long)cputime64_to_clock_t(softirq),
> +			(unsigned long long)cputime64_to_clock_t(steal),
> +			(unsigned long long)cputime64_to_clock_t(guest),
> +			(unsigned long long)cputime64_to_clock_t(guest_nice));
> +	}
> +	seq_printf(p, "intr %llu", (unsigned long long)sum);
> +
> +	/* sum again ? it could be updated? */
> +	for_each_irq_nr(j)
> +		seq_printf(p, " %u", kstat_irqs(j));
> +
> +	seq_printf(p,
> +		"\nctxt %llu\n"
> +		"btime %lu\n"
> +		"processes %lu\n"
> +		"procs_running %lu\n"
> +		"procs_blocked %lu\n",
> +		nr_context_switches(),
> +		(unsigned long)jif,
> +		total_forks,
> +		nr_running(),
> +		nr_iowait());
> +
> +	seq_printf(p, "softirq %llu", (unsigned long long)sum_softirq);
> +
> +	for (i = 0; i<  NR_SOFTIRQS; i++)
> +		seq_printf(p, " %u", per_softirq_sums[i]);
> +	seq_putc(p, '\n');
> +
> +	return 0;
> +}
> +
>   #ifdef CONFIG_CGROUP_CPUACCT
>
>   /*