Kevin Hilman wrote: > Frederic Weisbecker <[email protected]> writes: > > > Ingo, > > > > Please pull the new full dynticks cputime accounting code that > > can be found at: > > > > git://git.kernel.org/pub/scm/linux/kernel/git/frederic/linux-dynticks.git > > tags/full-dynticks-cputime-for-mingo > > > > My last concern is the dependency on CONFIG_64BIT. We rely on cputime_t > > being u64 for reasonable nanosec granularity implementation. And therefore > > we need a single instruction fetch to read kernel cpustat for atomicity > > requirement against concurrent incrementation, which only 64 bit archs > > can provide. > > Actually, moderately recent 32-bit ARMs can do atomic 64-bit load/stores > too. > > Also, is it just kernel_cpustat increments that need protection? or do > the various reads of the task_struct's cputime fields also need > protection (hmm, thinking twice, maybe those are already sufficiently > protected by the vtime_seqlock?) > > > It's probably no big deal to solve this issue. What we need is simply some > > atomic accessors. > > What about using the atomic64_* accessors? Those would just use the > native loads/stores on arches that have them, otherwise > CONFIG_GENERIC_ATOMIC64 provides some fallbacks. > > To give it a try, below is a quick patch to convert kernel_cpustat to > atomic64. I only got as far as compile testing and basic boot testing > on a 32-bit ARM platform, but let me know if this is the right > direction. > > > There is just no emergency though as this new option depends on the context > > tracking subsystem that only x86-64 (and soon ppc64) implements yet. And > > this set is complex enough already. I think we can deal with that later. > > I've started working on the ARM version of the context_tracker, so > "later" is coming quickly and I will do what I can to help this along. > > Kevin > > > > >From 9881fc4b86a63244018a4bdb5a383f3b862a0b8b Mon Sep 17 00:00:00 2001 > From: Kevin Hilman <[email protected]> > Date: Thu, 14 Feb 2013 17:46:08 -0800 > Subject: [PATCH] kernel_cpustat: convert to atomic 64-bit accessors > > WIP: only compile tested and basic boot tested on 32-bit ARM platform > --- > fs/proc/stat.c | 36 ++++++++++++++++++------------------ > fs/proc/uptime.c | 2 +- > include/linux/kernel_stat.h | 2 +- > kernel/sched/cputime.c | 22 +++++++++++----------- > 4 files changed, 31 insertions(+), 31 deletions(-) > > diff --git a/fs/proc/stat.c b/fs/proc/stat.c > index e296572..2a4c723 100644 > --- a/fs/proc/stat.c > +++ b/fs/proc/stat.c > @@ -52,7 +52,7 @@ static u64 get_idle_time(int cpu) > > if (idle_time == -1ULL) > /* !NO_HZ or cpu offline so we can rely on cpustat.idle */ > - idle = kcpustat_cpu(cpu).cpustat[CPUTIME_IDLE]; > + idle = atomic64_read(&kcpustat_cpu(cpu).cpustat[CPUTIME_IDLE]); > else > idle = usecs_to_cputime64(idle_time); > > @@ -68,7 +68,7 @@ static u64 get_iowait_time(int cpu) > > if (iowait_time == -1ULL) > /* !NO_HZ or cpu offline so we can rely on cpustat.iowait */ > - iowait = kcpustat_cpu(cpu).cpustat[CPUTIME_IOWAIT]; > + iowait = > atomic64_read(&kcpustat_cpu(cpu).cpustat[CPUTIME_IOWAIT]); > else > iowait = usecs_to_cputime64(iowait_time); > > @@ -95,16 +95,16 @@ static int show_stat(struct seq_file *p, void *v) > jif = boottime.tv_sec; > > for_each_possible_cpu(i) { > - user += kcpustat_cpu(i).cpustat[CPUTIME_USER]; > - nice += kcpustat_cpu(i).cpustat[CPUTIME_NICE]; > - system += kcpustat_cpu(i).cpustat[CPUTIME_SYSTEM]; > + user += atomic64_read(&kcpustat_cpu(i).cpustat[CPUTIME_USER]); > + nice += atomic64_read(&kcpustat_cpu(i).cpustat[CPUTIME_NICE]); > + system += > atomic64_read(&kcpustat_cpu(i).cpustat[CPUTIME_SYSTEM]); > idle += get_idle_time(i); > iowait += get_iowait_time(i); > - irq += kcpustat_cpu(i).cpustat[CPUTIME_IRQ]; > - softirq += kcpustat_cpu(i).cpustat[CPUTIME_SOFTIRQ]; > - steal += kcpustat_cpu(i).cpustat[CPUTIME_STEAL]; > - guest += kcpustat_cpu(i).cpustat[CPUTIME_GUEST]; > - guest_nice += kcpustat_cpu(i).cpustat[CPUTIME_GUEST_NICE]; > + irq += atomic64_read(&kcpustat_cpu(i).cpustat[CPUTIME_IRQ]); > + softirq += > atomic64_read(&kcpustat_cpu(i).cpustat[CPUTIME_SOFTIRQ]); > + steal += atomic64_read(&kcpustat_cpu(i).cpustat[CPUTIME_STEAL]); > + guest += atomic64_read(&kcpustat_cpu(i).cpustat[CPUTIME_GUEST]); > + guest_nice += > atomic64_read(&kcpustat_cpu(i).cpustat[CPUTIME_GUEST_NICE]); > sum += kstat_cpu_irqs_sum(i); > sum += arch_irq_stat_cpu(i); > > @@ -132,16 +132,16 @@ static int show_stat(struct seq_file *p, void *v) > > for_each_online_cpu(i) { > /* Copy values here to work around gcc-2.95.3, gcc-2.96 */ > - user = kcpustat_cpu(i).cpustat[CPUTIME_USER]; > - nice = kcpustat_cpu(i).cpustat[CPUTIME_NICE]; > - system = kcpustat_cpu(i).cpustat[CPUTIME_SYSTEM]; > + user = atomic64_read(&kcpustat_cpu(i).cpustat[CPUTIME_USER]); > + nice = atomic64_read(&kcpustat_cpu(i).cpustat[CPUTIME_NICE]); > + system = > atomic64_read(&kcpustat_cpu(i).cpustat[CPUTIME_SYSTEM]); > idle = get_idle_time(i); > iowait = get_iowait_time(i); > - irq = kcpustat_cpu(i).cpustat[CPUTIME_IRQ]; > - softirq = kcpustat_cpu(i).cpustat[CPUTIME_SOFTIRQ]; > - steal = kcpustat_cpu(i).cpustat[CPUTIME_STEAL]; > - guest = kcpustat_cpu(i).cpustat[CPUTIME_GUEST]; > - guest_nice = kcpustat_cpu(i).cpustat[CPUTIME_GUEST_NICE]; > + irq = atomic64_read(&kcpustat_cpu(i).cpustat[CPUTIME_IRQ]); > + softirq = > atomic64_read(&kcpustat_cpu(i).cpustat[CPUTIME_SOFTIRQ]); > + steal = atomic64_read(&kcpustat_cpu(i).cpustat[CPUTIME_STEAL]); > + guest = atomic64_read(&kcpustat_cpu(i).cpustat[CPUTIME_GUEST]); > + guest_nice = > atomic64_read(&kcpustat_cpu(i).cpustat[CPUTIME_GUEST_NICE]); > seq_printf(p, "cpu%d", i); > seq_put_decimal_ull(p, ' ', cputime64_to_clock_t(user)); > seq_put_decimal_ull(p, ' ', cputime64_to_clock_t(nice)); > diff --git a/fs/proc/uptime.c b/fs/proc/uptime.c > index 9610ac7..10c0f6e 100644 > --- a/fs/proc/uptime.c > +++ b/fs/proc/uptime.c > @@ -18,7 +18,7 @@ static int uptime_proc_show(struct seq_file *m, void *v) > > idletime = 0; > for_each_possible_cpu(i) > - idletime += (__force u64) kcpustat_cpu(i).cpustat[CPUTIME_IDLE]; > + idletime += > atomic64_read(&kcpustat_cpu(i).cpustat[CPUTIME_IDLE]); > > do_posix_clock_monotonic_gettime(&uptime); > monotonic_to_bootbased(&uptime); > diff --git a/include/linux/kernel_stat.h b/include/linux/kernel_stat.h > index ed5f6ed..45b9f71 100644 > --- a/include/linux/kernel_stat.h > +++ b/include/linux/kernel_stat.h > @@ -32,7 +32,7 @@ enum cpu_usage_stat { > }; > > struct kernel_cpustat { > - u64 cpustat[NR_STATS]; > + atomic64_t cpustat[NR_STATS]; > }; > > struct kernel_stat { > diff --git a/kernel/sched/cputime.c b/kernel/sched/cputime.c > index ccff275..c1ac2b2 100644 > --- a/kernel/sched/cputime.c > +++ b/kernel/sched/cputime.c > @@ -125,7 +125,7 @@ static inline void task_group_account_field(struct > task_struct *p, int index, > * is the only cgroup, then nothing else should be necessary. > * > */ > - __get_cpu_var(kernel_cpustat).cpustat[index] += tmp; > + atomic64_add(tmp, &__get_cpu_var(kernel_cpustat).cpustat[index]); > > #ifdef CONFIG_CGROUP_CPUACCT > if (unlikely(!cpuacct_subsys.active)) > @@ -176,7 +176,7 @@ void account_user_time(struct task_struct *p, cputime_t > cputime, > static void account_guest_time(struct task_struct *p, cputime_t cputime, > cputime_t cputime_scaled) > { > - u64 *cpustat = kcpustat_this_cpu->cpustat; > + atomic64_t *cpustat = kcpustat_this_cpu->cpustat; > > /* Add guest time to process. */ > p->utime += cputime; > @@ -186,11 +186,11 @@ static void account_guest_time(struct task_struct *p, > cputime_t cputime, > > /* Add guest time to cpustat. */ > if (TASK_NICE(p) > 0) { > - cpustat[CPUTIME_NICE] += (__force u64) cputime; > - cpustat[CPUTIME_GUEST_NICE] += (__force u64) cputime; > + atomic64_add((__force u64) cputime, &cpustat[CPUTIME_NICE]); > + atomic64_add((__force u64) cputime, > &cpustat[CPUTIME_GUEST_NICE]); > } else { > - cpustat[CPUTIME_USER] += (__force u64) cputime; > - cpustat[CPUTIME_GUEST] += (__force u64) cputime; > + atomic64_add((__force u64) cputime, &cpustat[CPUTIME_USER]); > + atomic64_add((__force u64) cputime, &cpustat[CPUTIME_GUEST]); > } > } > > @@ -250,9 +250,9 @@ void account_system_time(struct task_struct *p, int > hardirq_offset, > */ > void account_steal_time(cputime_t cputime) > { > - u64 *cpustat = kcpustat_this_cpu->cpustat; > + atomic64_t *cpustat = kcpustat_this_cpu->cpustat; > > - cpustat[CPUTIME_STEAL] += (__force u64) cputime; > + atomic64_add((__force u64) cputime, &cpustat[CPUTIME_STEAL]); > } > > /* > @@ -261,13 +261,13 @@ void account_steal_time(cputime_t cputime) > */ > void account_idle_time(cputime_t cputime) > { > - u64 *cpustat = kcpustat_this_cpu->cpustat; > + atomic64_t *cpustat = kcpustat_this_cpu->cpustat; > struct rq *rq = this_rq(); > > if (atomic_read(&rq->nr_iowait) > 0) > - cpustat[CPUTIME_IOWAIT] += (__force u64) cputime; > + atomic64_add((__force u64) cputime, &cpustat[CPUTIME_IOWAIT]); > else > - cpustat[CPUTIME_IDLE] += (__force u64) cputime; > + atomic64_add((__force u64) cputime, &cpustat[CPUTIME_IDLE]); > } > > static __always_inline bool steal_account_process_tick(void) > -- > 1.8.1.2 > > -- > To unsubscribe from this list: send the line "unsubscribe linux-kernel" in > the body of a message to [email protected] > More majordomo info at http://vger.kernel.org/majordomo-info.html > Please read the FAQ at http://www.tux.org/lkml/
I had to do some changes to get it to compile. You can check my patch below and see whether I got it all wrong... /Mats >From e3da56bdb36a3efb5a9869ded20f346039f381f2 Mon Sep 17 00:00:00 2001 From: Mats Liljegren <[email protected]> Date: Fri, 15 Feb 2013 13:46:29 +0100 Subject: [PATCH] Added atomic operators to make it compile --- kernel/sched/core.c | 10 +++++----- kernel/sched/cputime.c | 2 +- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/kernel/sched/core.c b/kernel/sched/core.c index 2fad439..5415e85 100644 --- a/kernel/sched/core.c +++ b/kernel/sched/core.c @@ -8168,8 +8168,8 @@ static int cpuacct_stats_show(struct cgroup *cgrp, struct cftype *cft, for_each_online_cpu(cpu) { struct kernel_cpustat *kcpustat = per_cpu_ptr(ca->cpustat, cpu); - val += kcpustat->cpustat[CPUTIME_USER]; - val += kcpustat->cpustat[CPUTIME_NICE]; + val += atomic64_read(&kcpustat->cpustat[CPUTIME_USER]); + val += atomic64_read(&kcpustat->cpustat[CPUTIME_NICE]); } val = cputime64_to_clock_t(val); cb->fill(cb, cpuacct_stat_desc[CPUACCT_STAT_USER], val); @@ -8177,9 +8177,9 @@ static int cpuacct_stats_show(struct cgroup *cgrp, struct cftype *cft, val = 0; for_each_online_cpu(cpu) { struct kernel_cpustat *kcpustat = per_cpu_ptr(ca->cpustat, cpu); - val += kcpustat->cpustat[CPUTIME_SYSTEM]; - val += kcpustat->cpustat[CPUTIME_IRQ]; - val += kcpustat->cpustat[CPUTIME_SOFTIRQ]; + val += atomic64_read(&kcpustat->cpustat[CPUTIME_SYSTEM]); + val += atomic64_read(&kcpustat->cpustat[CPUTIME_IRQ]); + val += atomic64_read(&kcpustat->cpustat[CPUTIME_SOFTIRQ]); } val = cputime64_to_clock_t(val); diff --git a/kernel/sched/cputime.c b/kernel/sched/cputime.c index 50de5bf..2bb998b 100644 --- a/kernel/sched/cputime.c +++ b/kernel/sched/cputime.c @@ -135,7 +135,7 @@ static inline void task_group_account_field(struct task_struct *p, int index, ca = task_ca(p); while (ca && (ca != &root_cpuacct)) { kcpustat = this_cpu_ptr(ca->cpustat); - kcpustat->cpustat[index] += tmp; + atomic64_add(tmp, &kcpustat->cpustat[index]); ca = parent_ca(ca); } rcu_read_unlock(); -- 1.7.5.4 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
