Messages in this thread | | | Date | Sat, 12 Nov 2011 08:27:21 -0200 | From | Glauber Costa <> | Subject | Re: [PATCH v2 03/14] Move /proc/stat logic inside sched.c |
| |
On 11/11/2011 11:35 PM, Paul Turner wrote: > 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?).
Because later on in the series I start using task_group, which is a scheduler internal data structure (Since the very goal here is achieving per cgroup data.
It seemed to me better to do this way - as much as I agree with you that a lot here may not belong in sched.c - than to use external functions. Since all we have outside sched.c is a task_struct, we'd have to derive a task_group from it every time. Also, as you can see in the followup patches, which task_group to use depends on the caller and possibly some runtime variables. So the computation is not as trivially fast as just getting a field in a structure.
>> 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 >> >> /* >
| |