lkml.org 
[lkml]   [2013]   [Feb]   [15]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    Patch in this message
    /
    Date
    From
    SubjectRe: [GIT PULL] cputime: Full dynticks task/cputime accounting v7
    Kevin Hilman wrote:
    > Frederic Weisbecker <fweisbec@gmail.com> 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 <khilman@linaro.org>
    > 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 majordomo@vger.kernel.org
    > 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 <mats.liljegren@enea.com>
    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

    \
     
     \ /
      Last update: 2013-02-15 15:21    [W:4.643 / U:0.280 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site