All of lore.kernel.org
 help / color / mirror / Atom feed
* [RFC/PATCH 0/5] context_tracking: prerequisites for ARM support
@ 2013-02-20 19:41 ` Kevin Hilman
  0 siblings, 0 replies; 42+ messages in thread
From: Kevin Hilman @ 2013-02-20 19:41 UTC (permalink / raw)
  To: Frederic Weisbecker
  Cc: Mats Liljegren, linux-kernel, linux-arm-kernel, linaro-dev

Frederic,

Here is a first pass at some changes needed in order to add ARM
support.

I have another series coming that instruments the syscalls, exceptions
etc. on ARM also, but this one is just to get some prerequisites out
for broader discussion.

This series applies on top of your 3.8-rc6-nohz4 branch, and is also available
here: 

git://git.kernel.org/pub/scm/linux/kernel/git/khilman/linux.git wip/arm-nohz/prereq

I'm aware of the >80 column warnings from checkpatch on the atomic64
conversion patch, but wanted to get some feedback on this approach
before fixing all those up.  Maybe some simpler wrappers in
kernel_stat.h would make this a bit cleaner?

Kevin


Kevin Hilman (5):
  context tracking: conditionalize guest support based on CONFIG_KVM
  kernel_cpustat: convert to atomic 64-bit accessors
  virt CPU accounting: Kconfig: drop 64-bit requirment
  cputime: use do_div() for nsec resolution conversion helpers
  ARM: Kconfig: allow virt CPU accounting

 arch/arm/Kconfig                    |  1 +
 fs/proc/stat.c                      | 36 +++++++++++++-------------
 fs/proc/uptime.c                    |  2 +-
 include/asm-generic/cputime_nsecs.h | 51 +++++++++++++++++++++++++++----------
 include/linux/kernel_stat.h         |  2 +-
 init/Kconfig                        |  2 +-
 kernel/context_tracking.c           |  4 +++
 kernel/sched/core.c                 | 10 ++++----
 kernel/sched/cputime.c              | 24 ++++++++---------
 9 files changed, 80 insertions(+), 52 deletions(-)

-- 
1.8.1.2


^ permalink raw reply	[flat|nested] 42+ messages in thread

* [RFC/PATCH 0/5] context_tracking: prerequisites for ARM support
@ 2013-02-20 19:41 ` Kevin Hilman
  0 siblings, 0 replies; 42+ messages in thread
From: Kevin Hilman @ 2013-02-20 19:41 UTC (permalink / raw)
  To: linux-arm-kernel

Frederic,

Here is a first pass at some changes needed in order to add ARM
support.

I have another series coming that instruments the syscalls, exceptions
etc. on ARM also, but this one is just to get some prerequisites out
for broader discussion.

This series applies on top of your 3.8-rc6-nohz4 branch, and is also available
here: 

git://git.kernel.org/pub/scm/linux/kernel/git/khilman/linux.git wip/arm-nohz/prereq

I'm aware of the >80 column warnings from checkpatch on the atomic64
conversion patch, but wanted to get some feedback on this approach
before fixing all those up.  Maybe some simpler wrappers in
kernel_stat.h would make this a bit cleaner?

Kevin


Kevin Hilman (5):
  context tracking: conditionalize guest support based on CONFIG_KVM
  kernel_cpustat: convert to atomic 64-bit accessors
  virt CPU accounting: Kconfig: drop 64-bit requirment
  cputime: use do_div() for nsec resolution conversion helpers
  ARM: Kconfig: allow virt CPU accounting

 arch/arm/Kconfig                    |  1 +
 fs/proc/stat.c                      | 36 +++++++++++++-------------
 fs/proc/uptime.c                    |  2 +-
 include/asm-generic/cputime_nsecs.h | 51 +++++++++++++++++++++++++++----------
 include/linux/kernel_stat.h         |  2 +-
 init/Kconfig                        |  2 +-
 kernel/context_tracking.c           |  4 +++
 kernel/sched/core.c                 | 10 ++++----
 kernel/sched/cputime.c              | 24 ++++++++---------
 9 files changed, 80 insertions(+), 52 deletions(-)

-- 
1.8.1.2

^ permalink raw reply	[flat|nested] 42+ messages in thread

* [RFC/PATCH 1/5] context tracking: conditionalize guest support based on CONFIG_KVM
  2013-02-20 19:41 ` Kevin Hilman
@ 2013-02-20 19:41   ` Kevin Hilman
  -1 siblings, 0 replies; 42+ messages in thread
From: Kevin Hilman @ 2013-02-20 19:41 UTC (permalink / raw)
  To: Frederic Weisbecker
  Cc: Mats Liljegren, linux-kernel, linux-arm-kernel, linaro-dev

So that it can build on !KVM systems too.

Signed-off-by: Kevin Hilman <khilman@linaro.org>
---
 kernel/context_tracking.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/kernel/context_tracking.c b/kernel/context_tracking.c
index 74f68f4..6fe96b1 100644
--- a/kernel/context_tracking.c
+++ b/kernel/context_tracking.c
@@ -1,5 +1,7 @@
 #include <linux/context_tracking.h>
+#ifdef CONFIG_KVM
 #include <linux/kvm_host.h>
+#endif
 #include <linux/rcupdate.h>
 #include <linux/sched.h>
 #include <linux/hardirq.h>
@@ -62,6 +64,7 @@ void user_exit(void)
 	local_irq_restore(flags);
 }
 
+#ifdef CONFIG_KVM
 void guest_enter(void)
 {
 	if (vtime_accounting_enabled())
@@ -79,6 +82,7 @@ void guest_exit(void)
 		__guest_exit();
 }
 EXPORT_SYMBOL_GPL(guest_exit);
+#endif
 
 void context_tracking_task_switch(struct task_struct *prev,
 			     struct task_struct *next)
-- 
1.8.1.2


^ permalink raw reply related	[flat|nested] 42+ messages in thread

* [RFC/PATCH 1/5] context tracking: conditionalize guest support based on CONFIG_KVM
@ 2013-02-20 19:41   ` Kevin Hilman
  0 siblings, 0 replies; 42+ messages in thread
From: Kevin Hilman @ 2013-02-20 19:41 UTC (permalink / raw)
  To: linux-arm-kernel

So that it can build on !KVM systems too.

Signed-off-by: Kevin Hilman <khilman@linaro.org>
---
 kernel/context_tracking.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/kernel/context_tracking.c b/kernel/context_tracking.c
index 74f68f4..6fe96b1 100644
--- a/kernel/context_tracking.c
+++ b/kernel/context_tracking.c
@@ -1,5 +1,7 @@
 #include <linux/context_tracking.h>
+#ifdef CONFIG_KVM
 #include <linux/kvm_host.h>
+#endif
 #include <linux/rcupdate.h>
 #include <linux/sched.h>
 #include <linux/hardirq.h>
@@ -62,6 +64,7 @@ void user_exit(void)
 	local_irq_restore(flags);
 }
 
+#ifdef CONFIG_KVM
 void guest_enter(void)
 {
 	if (vtime_accounting_enabled())
@@ -79,6 +82,7 @@ void guest_exit(void)
 		__guest_exit();
 }
 EXPORT_SYMBOL_GPL(guest_exit);
+#endif
 
 void context_tracking_task_switch(struct task_struct *prev,
 			     struct task_struct *next)
-- 
1.8.1.2

^ permalink raw reply related	[flat|nested] 42+ messages in thread

* [RFC/PATCH 2/5] kernel_cpustat: convert to atomic 64-bit accessors
  2013-02-20 19:41 ` Kevin Hilman
@ 2013-02-20 19:41   ` Kevin Hilman
  -1 siblings, 0 replies; 42+ messages in thread
From: Kevin Hilman @ 2013-02-20 19:41 UTC (permalink / raw)
  To: Frederic Weisbecker
  Cc: Mats Liljegren, linux-kernel, linux-arm-kernel, linaro-dev

Use the atomic64_* accessors for all the kernel_cpustat fields to
ensure atomic access on non-64 bit platforms.

Thanks to Mats Liljegren for CGROUP_CPUACCT related fixes.

Cc: Mats Liljegren <mats.liljegren@enea.com>
Signed-off-by: Kevin Hilman <khilman@linaro.org>
---
 fs/proc/stat.c              | 40 ++++++++++++++++++++--------------------
 fs/proc/uptime.c            |  2 +-
 include/linux/kernel_stat.h |  2 +-
 kernel/sched/core.c         | 10 +++++-----
 kernel/sched/cputime.c      | 38 +++++++++++++++++++-------------------
 5 files changed, 46 insertions(+), 46 deletions(-)

diff --git a/fs/proc/stat.c b/fs/proc/stat.c
index e296572..93f7f30 100644
--- a/fs/proc/stat.c
+++ b/fs/proc/stat.c
@@ -25,7 +25,7 @@ static cputime64_t get_idle_time(int cpu)
 {
 	cputime64_t idle;
 
-	idle = kcpustat_cpu(cpu).cpustat[CPUTIME_IDLE];
+	idle = atomic64_read(&kcpustat_cpu(cpu).cpustat[CPUTIME_IDLE]);
 	if (cpu_online(cpu) && !nr_iowait_cpu(cpu))
 		idle += arch_idle_time(cpu);
 	return idle;
@@ -35,7 +35,7 @@ static cputime64_t get_iowait_time(int cpu)
 {
 	cputime64_t iowait;
 
-	iowait = kcpustat_cpu(cpu).cpustat[CPUTIME_IOWAIT];
+	iowait = atomic64_read(&kcpustat_cpu(cpu).cpustat[CPUTIME_IOWAIT]);
 	if (cpu_online(cpu) && nr_iowait_cpu(cpu))
 		iowait += arch_idle_time(cpu);
 	return iowait;
@@ -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/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 ccff275..4c639ee 100644
--- a/kernel/sched/cputime.c
+++ b/kernel/sched/cputime.c
@@ -78,14 +78,14 @@ EXPORT_SYMBOL_GPL(irqtime_account_irq);
 
 static int irqtime_account_hi_update(void)
 {
-	u64 *cpustat = kcpustat_this_cpu->cpustat;
+	atomic64_t *cpustat = kcpustat_this_cpu->cpustat;
 	unsigned long flags;
 	u64 latest_ns;
 	int ret = 0;
 
 	local_irq_save(flags);
 	latest_ns = this_cpu_read(cpu_hardirq_time);
-	if (nsecs_to_cputime64(latest_ns) > cpustat[CPUTIME_IRQ])
+	if (nsecs_to_cputime64(latest_ns) > atomic64_read(&cpustat[CPUTIME_IRQ]));
 		ret = 1;
 	local_irq_restore(flags);
 	return ret;
@@ -93,14 +93,14 @@ static int irqtime_account_hi_update(void)
 
 static int irqtime_account_si_update(void)
 {
-	u64 *cpustat = kcpustat_this_cpu->cpustat;
+	atomic64_t *cpustat = kcpustat_this_cpu->cpustat;
 	unsigned long flags;
 	u64 latest_ns;
 	int ret = 0;
 
 	local_irq_save(flags);
 	latest_ns = this_cpu_read(cpu_softirq_time);
-	if (nsecs_to_cputime64(latest_ns) > cpustat[CPUTIME_SOFTIRQ])
+	if (nsecs_to_cputime64(latest_ns) > atomic64_read(&cpustat[CPUTIME_SOFTIRQ]))
 		ret = 1;
 	local_irq_restore(flags);
 	return ret;
@@ -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))
@@ -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();
@@ -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)
@@ -345,15 +345,15 @@ static void irqtime_account_process_tick(struct task_struct *p, int user_tick,
 						struct rq *rq)
 {
 	cputime_t one_jiffy_scaled = cputime_to_scaled(cputime_one_jiffy);
-	u64 *cpustat = kcpustat_this_cpu->cpustat;
+	atomic64_t *cpustat = kcpustat_this_cpu->cpustat;
 
 	if (steal_account_process_tick())
 		return;
 
 	if (irqtime_account_hi_update()) {
-		cpustat[CPUTIME_IRQ] += (__force u64) cputime_one_jiffy;
+		atomic64_add((__force u64) cputime_one_jiffy, &cpustat[CPUTIME_IRQ]);
 	} else if (irqtime_account_si_update()) {
-		cpustat[CPUTIME_SOFTIRQ] += (__force u64) cputime_one_jiffy;
+		atomic64_add((__force u64) cputime_one_jiffy, &cpustat[CPUTIME_SOFTIRQ]);
 	} else if (this_cpu_ksoftirqd() == p) {
 		/*
 		 * ksoftirqd time do not get accounted in cpu_softirq_time.
-- 
1.8.1.2


^ permalink raw reply related	[flat|nested] 42+ messages in thread

* [RFC/PATCH 2/5] kernel_cpustat: convert to atomic 64-bit accessors
@ 2013-02-20 19:41   ` Kevin Hilman
  0 siblings, 0 replies; 42+ messages in thread
From: Kevin Hilman @ 2013-02-20 19:41 UTC (permalink / raw)
  To: linux-arm-kernel

Use the atomic64_* accessors for all the kernel_cpustat fields to
ensure atomic access on non-64 bit platforms.

Thanks to Mats Liljegren for CGROUP_CPUACCT related fixes.

Cc: Mats Liljegren <mats.liljegren@enea.com>
Signed-off-by: Kevin Hilman <khilman@linaro.org>
---
 fs/proc/stat.c              | 40 ++++++++++++++++++++--------------------
 fs/proc/uptime.c            |  2 +-
 include/linux/kernel_stat.h |  2 +-
 kernel/sched/core.c         | 10 +++++-----
 kernel/sched/cputime.c      | 38 +++++++++++++++++++-------------------
 5 files changed, 46 insertions(+), 46 deletions(-)

diff --git a/fs/proc/stat.c b/fs/proc/stat.c
index e296572..93f7f30 100644
--- a/fs/proc/stat.c
+++ b/fs/proc/stat.c
@@ -25,7 +25,7 @@ static cputime64_t get_idle_time(int cpu)
 {
 	cputime64_t idle;
 
-	idle = kcpustat_cpu(cpu).cpustat[CPUTIME_IDLE];
+	idle = atomic64_read(&kcpustat_cpu(cpu).cpustat[CPUTIME_IDLE]);
 	if (cpu_online(cpu) && !nr_iowait_cpu(cpu))
 		idle += arch_idle_time(cpu);
 	return idle;
@@ -35,7 +35,7 @@ static cputime64_t get_iowait_time(int cpu)
 {
 	cputime64_t iowait;
 
-	iowait = kcpustat_cpu(cpu).cpustat[CPUTIME_IOWAIT];
+	iowait = atomic64_read(&kcpustat_cpu(cpu).cpustat[CPUTIME_IOWAIT]);
 	if (cpu_online(cpu) && nr_iowait_cpu(cpu))
 		iowait += arch_idle_time(cpu);
 	return iowait;
@@ -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/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 ccff275..4c639ee 100644
--- a/kernel/sched/cputime.c
+++ b/kernel/sched/cputime.c
@@ -78,14 +78,14 @@ EXPORT_SYMBOL_GPL(irqtime_account_irq);
 
 static int irqtime_account_hi_update(void)
 {
-	u64 *cpustat = kcpustat_this_cpu->cpustat;
+	atomic64_t *cpustat = kcpustat_this_cpu->cpustat;
 	unsigned long flags;
 	u64 latest_ns;
 	int ret = 0;
 
 	local_irq_save(flags);
 	latest_ns = this_cpu_read(cpu_hardirq_time);
-	if (nsecs_to_cputime64(latest_ns) > cpustat[CPUTIME_IRQ])
+	if (nsecs_to_cputime64(latest_ns) > atomic64_read(&cpustat[CPUTIME_IRQ]));
 		ret = 1;
 	local_irq_restore(flags);
 	return ret;
@@ -93,14 +93,14 @@ static int irqtime_account_hi_update(void)
 
 static int irqtime_account_si_update(void)
 {
-	u64 *cpustat = kcpustat_this_cpu->cpustat;
+	atomic64_t *cpustat = kcpustat_this_cpu->cpustat;
 	unsigned long flags;
 	u64 latest_ns;
 	int ret = 0;
 
 	local_irq_save(flags);
 	latest_ns = this_cpu_read(cpu_softirq_time);
-	if (nsecs_to_cputime64(latest_ns) > cpustat[CPUTIME_SOFTIRQ])
+	if (nsecs_to_cputime64(latest_ns) > atomic64_read(&cpustat[CPUTIME_SOFTIRQ]))
 		ret = 1;
 	local_irq_restore(flags);
 	return ret;
@@ -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))
@@ -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();
@@ -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)
@@ -345,15 +345,15 @@ static void irqtime_account_process_tick(struct task_struct *p, int user_tick,
 						struct rq *rq)
 {
 	cputime_t one_jiffy_scaled = cputime_to_scaled(cputime_one_jiffy);
-	u64 *cpustat = kcpustat_this_cpu->cpustat;
+	atomic64_t *cpustat = kcpustat_this_cpu->cpustat;
 
 	if (steal_account_process_tick())
 		return;
 
 	if (irqtime_account_hi_update()) {
-		cpustat[CPUTIME_IRQ] += (__force u64) cputime_one_jiffy;
+		atomic64_add((__force u64) cputime_one_jiffy, &cpustat[CPUTIME_IRQ]);
 	} else if (irqtime_account_si_update()) {
-		cpustat[CPUTIME_SOFTIRQ] += (__force u64) cputime_one_jiffy;
+		atomic64_add((__force u64) cputime_one_jiffy, &cpustat[CPUTIME_SOFTIRQ]);
 	} else if (this_cpu_ksoftirqd() == p) {
 		/*
 		 * ksoftirqd time do not get accounted in cpu_softirq_time.
-- 
1.8.1.2

^ permalink raw reply related	[flat|nested] 42+ messages in thread

* [RFC/PATCH 3/5] virt CPU accounting: Kconfig: drop 64-bit requirment
  2013-02-20 19:41 ` Kevin Hilman
@ 2013-02-20 19:41   ` Kevin Hilman
  -1 siblings, 0 replies; 42+ messages in thread
From: Kevin Hilman @ 2013-02-20 19:41 UTC (permalink / raw)
  To: Frederic Weisbecker
  Cc: Mats Liljegren, linux-kernel, linux-arm-kernel, linaro-dev

The 64-bit requirement can be removed after the conversion of
kernel_cpustat accessors to the portable atomic64 accessors.

Signed-off-by: Kevin Hilman <khilman@linaro.org>
---
 init/Kconfig | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/init/Kconfig b/init/Kconfig
index 3abb019..b13af06 100644
--- a/init/Kconfig
+++ b/init/Kconfig
@@ -356,7 +356,7 @@ config VIRT_CPU_ACCOUNTING_NATIVE
 
 config VIRT_CPU_ACCOUNTING_GEN
 	bool "Full dynticks CPU time accounting"
-	depends on HAVE_CONTEXT_TRACKING && 64BIT
+	depends on HAVE_CONTEXT_TRACKING
 	select VIRT_CPU_ACCOUNTING
 	select CONTEXT_TRACKING
 	help
-- 
1.8.1.2


^ permalink raw reply related	[flat|nested] 42+ messages in thread

* [RFC/PATCH 3/5] virt CPU accounting: Kconfig: drop 64-bit requirment
@ 2013-02-20 19:41   ` Kevin Hilman
  0 siblings, 0 replies; 42+ messages in thread
From: Kevin Hilman @ 2013-02-20 19:41 UTC (permalink / raw)
  To: linux-arm-kernel

The 64-bit requirement can be removed after the conversion of
kernel_cpustat accessors to the portable atomic64 accessors.

Signed-off-by: Kevin Hilman <khilman@linaro.org>
---
 init/Kconfig | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/init/Kconfig b/init/Kconfig
index 3abb019..b13af06 100644
--- a/init/Kconfig
+++ b/init/Kconfig
@@ -356,7 +356,7 @@ config VIRT_CPU_ACCOUNTING_NATIVE
 
 config VIRT_CPU_ACCOUNTING_GEN
 	bool "Full dynticks CPU time accounting"
-	depends on HAVE_CONTEXT_TRACKING && 64BIT
+	depends on HAVE_CONTEXT_TRACKING
 	select VIRT_CPU_ACCOUNTING
 	select CONTEXT_TRACKING
 	help
-- 
1.8.1.2

^ permalink raw reply related	[flat|nested] 42+ messages in thread

* [RFC/PATCH 4/5] cputime: use do_div() for nsec resolution conversion helpers
  2013-02-20 19:41 ` Kevin Hilman
@ 2013-02-20 19:41   ` Kevin Hilman
  -1 siblings, 0 replies; 42+ messages in thread
From: Kevin Hilman @ 2013-02-20 19:41 UTC (permalink / raw)
  To: Frederic Weisbecker
  Cc: Mats Liljegren, linux-kernel, linux-arm-kernel, linaro-dev

For the nsec resolution conversions to be useful on non 64-bit
architectures, do_div() needs to be used for the 64-bit divisions.

Signed-off-by: Kevin Hilman <khilman@linaro.org>
---
 include/asm-generic/cputime_nsecs.h | 51 +++++++++++++++++++++++++++----------
 1 file changed, 37 insertions(+), 14 deletions(-)

diff --git a/include/asm-generic/cputime_nsecs.h b/include/asm-generic/cputime_nsecs.h
index b6485ca..daa6075 100644
--- a/include/asm-generic/cputime_nsecs.h
+++ b/include/asm-generic/cputime_nsecs.h
@@ -24,13 +24,17 @@ typedef u64 __nocast cputime64_t;
 /*
  * Convert cputime <-> jiffies (HZ)
  */
-#define cputime_to_jiffies(__ct)	\
-	((__force u64)(__ct) / (NSEC_PER_SEC / HZ))
+static inline u64 cputime_to_jiffies(const cputime_t ct)
+{
+	cputime_t __ct = ct;
+
+	do_div(__ct, NSEC_PER_SEC / HZ);
+	return __ct;
+}
 #define cputime_to_scaled(__ct)		(__ct)
 #define jiffies_to_cputime(__jif)	\
 	(__force cputime_t)((__jif) * (NSEC_PER_SEC / HZ))
-#define cputime64_to_jiffies64(__ct)	\
-	((__force u64)(__ct) / (NSEC_PER_SEC / HZ))
+#define cputime64_to_jiffies64(__ct) cputime_to_jiffies(__ct)
 #define jiffies64_to_cputime64(__jif)	\
 	(__force cputime64_t)((__jif) * (NSEC_PER_SEC / HZ))
 
@@ -44,8 +48,13 @@ typedef u64 __nocast cputime64_t;
 /*
  * Convert cputime <-> microseconds
  */
-#define cputime_to_usecs(__ct)		\
-	((__force u64)(__ct) / NSEC_PER_USEC)
+static inline u64 cputime_to_usecs(const cputime_t ct)
+{
+	cputime_t __ct = ct;
+
+	do_div(__ct, NSEC_PER_USEC);
+	return __ct;
+}
 #define usecs_to_cputime(__usecs)	\
 	(__force cputime_t)((__usecs) * NSEC_PER_USEC)
 #define usecs_to_cputime64(__usecs)	\
@@ -54,8 +63,13 @@ typedef u64 __nocast cputime64_t;
 /*
  * Convert cputime <-> seconds
  */
-#define cputime_to_secs(__ct)		\
-	((__force u64)(__ct) / NSEC_PER_SEC)
+static inline u64 cputime_to_secs(const cputime_t ct)
+{
+	cputime_t __ct = ct;
+
+	do_div(__ct, NSEC_PER_SEC);
+	return __ct;
+}
 #define secs_to_cputime(__secs)		\
 	(__force cputime_t)((__secs) * NSEC_PER_SEC)
 
@@ -69,8 +83,10 @@ static inline cputime_t timespec_to_cputime(const struct timespec *val)
 }
 static inline void cputime_to_timespec(const cputime_t ct, struct timespec *val)
 {
-	val->tv_sec  = (__force u64) ct / NSEC_PER_SEC;
-	val->tv_nsec = (__force u64) ct % NSEC_PER_SEC;
+	cputime_t __ct = ct;
+
+	val->tv_nsec = do_div(__ct, NSEC_PER_SEC);
+	val->tv_sec = __ct;
 }
 
 /*
@@ -83,15 +99,22 @@ static inline cputime_t timeval_to_cputime(struct timeval *val)
 }
 static inline void cputime_to_timeval(const cputime_t ct, struct timeval *val)
 {
-	val->tv_sec = (__force u64) ct / NSEC_PER_SEC;
-	val->tv_usec = ((__force u64) ct % NSEC_PER_SEC) / NSEC_PER_USEC;
+	cputime_t __ct = ct;
+
+	val->tv_usec = do_div(__ct, NSEC_PER_SEC) / NSEC_PER_USEC;
+	val->tv_sec = __ct;
 }
 
 /*
  * Convert cputime <-> clock (USER_HZ)
  */
-#define cputime_to_clock_t(__ct)	\
-	((__force u64)(__ct) / (NSEC_PER_SEC / USER_HZ))
+static inline u64 cputime_to_clock_t(const cputime_t ct)
+{
+	cputime_t __ct = ct;
+
+	do_div(__ct, (NSEC_PER_SEC / USER_HZ));
+	return __ct;
+}
 #define clock_t_to_cputime(__x)		\
 	(__force cputime_t)((__x) * (NSEC_PER_SEC / USER_HZ))
 
-- 
1.8.1.2


^ permalink raw reply related	[flat|nested] 42+ messages in thread

* [RFC/PATCH 4/5] cputime: use do_div() for nsec resolution conversion helpers
@ 2013-02-20 19:41   ` Kevin Hilman
  0 siblings, 0 replies; 42+ messages in thread
From: Kevin Hilman @ 2013-02-20 19:41 UTC (permalink / raw)
  To: linux-arm-kernel

For the nsec resolution conversions to be useful on non 64-bit
architectures, do_div() needs to be used for the 64-bit divisions.

Signed-off-by: Kevin Hilman <khilman@linaro.org>
---
 include/asm-generic/cputime_nsecs.h | 51 +++++++++++++++++++++++++++----------
 1 file changed, 37 insertions(+), 14 deletions(-)

diff --git a/include/asm-generic/cputime_nsecs.h b/include/asm-generic/cputime_nsecs.h
index b6485ca..daa6075 100644
--- a/include/asm-generic/cputime_nsecs.h
+++ b/include/asm-generic/cputime_nsecs.h
@@ -24,13 +24,17 @@ typedef u64 __nocast cputime64_t;
 /*
  * Convert cputime <-> jiffies (HZ)
  */
-#define cputime_to_jiffies(__ct)	\
-	((__force u64)(__ct) / (NSEC_PER_SEC / HZ))
+static inline u64 cputime_to_jiffies(const cputime_t ct)
+{
+	cputime_t __ct = ct;
+
+	do_div(__ct, NSEC_PER_SEC / HZ);
+	return __ct;
+}
 #define cputime_to_scaled(__ct)		(__ct)
 #define jiffies_to_cputime(__jif)	\
 	(__force cputime_t)((__jif) * (NSEC_PER_SEC / HZ))
-#define cputime64_to_jiffies64(__ct)	\
-	((__force u64)(__ct) / (NSEC_PER_SEC / HZ))
+#define cputime64_to_jiffies64(__ct) cputime_to_jiffies(__ct)
 #define jiffies64_to_cputime64(__jif)	\
 	(__force cputime64_t)((__jif) * (NSEC_PER_SEC / HZ))
 
@@ -44,8 +48,13 @@ typedef u64 __nocast cputime64_t;
 /*
  * Convert cputime <-> microseconds
  */
-#define cputime_to_usecs(__ct)		\
-	((__force u64)(__ct) / NSEC_PER_USEC)
+static inline u64 cputime_to_usecs(const cputime_t ct)
+{
+	cputime_t __ct = ct;
+
+	do_div(__ct, NSEC_PER_USEC);
+	return __ct;
+}
 #define usecs_to_cputime(__usecs)	\
 	(__force cputime_t)((__usecs) * NSEC_PER_USEC)
 #define usecs_to_cputime64(__usecs)	\
@@ -54,8 +63,13 @@ typedef u64 __nocast cputime64_t;
 /*
  * Convert cputime <-> seconds
  */
-#define cputime_to_secs(__ct)		\
-	((__force u64)(__ct) / NSEC_PER_SEC)
+static inline u64 cputime_to_secs(const cputime_t ct)
+{
+	cputime_t __ct = ct;
+
+	do_div(__ct, NSEC_PER_SEC);
+	return __ct;
+}
 #define secs_to_cputime(__secs)		\
 	(__force cputime_t)((__secs) * NSEC_PER_SEC)
 
@@ -69,8 +83,10 @@ static inline cputime_t timespec_to_cputime(const struct timespec *val)
 }
 static inline void cputime_to_timespec(const cputime_t ct, struct timespec *val)
 {
-	val->tv_sec  = (__force u64) ct / NSEC_PER_SEC;
-	val->tv_nsec = (__force u64) ct % NSEC_PER_SEC;
+	cputime_t __ct = ct;
+
+	val->tv_nsec = do_div(__ct, NSEC_PER_SEC);
+	val->tv_sec = __ct;
 }
 
 /*
@@ -83,15 +99,22 @@ static inline cputime_t timeval_to_cputime(struct timeval *val)
 }
 static inline void cputime_to_timeval(const cputime_t ct, struct timeval *val)
 {
-	val->tv_sec = (__force u64) ct / NSEC_PER_SEC;
-	val->tv_usec = ((__force u64) ct % NSEC_PER_SEC) / NSEC_PER_USEC;
+	cputime_t __ct = ct;
+
+	val->tv_usec = do_div(__ct, NSEC_PER_SEC) / NSEC_PER_USEC;
+	val->tv_sec = __ct;
 }
 
 /*
  * Convert cputime <-> clock (USER_HZ)
  */
-#define cputime_to_clock_t(__ct)	\
-	((__force u64)(__ct) / (NSEC_PER_SEC / USER_HZ))
+static inline u64 cputime_to_clock_t(const cputime_t ct)
+{
+	cputime_t __ct = ct;
+
+	do_div(__ct, (NSEC_PER_SEC / USER_HZ));
+	return __ct;
+}
 #define clock_t_to_cputime(__x)		\
 	(__force cputime_t)((__x) * (NSEC_PER_SEC / USER_HZ))
 
-- 
1.8.1.2

^ permalink raw reply related	[flat|nested] 42+ messages in thread

* [RFC/PATCH 5/5] ARM: Kconfig: allow virt CPU accounting
  2013-02-20 19:41 ` Kevin Hilman
@ 2013-02-20 19:41   ` Kevin Hilman
  -1 siblings, 0 replies; 42+ messages in thread
From: Kevin Hilman @ 2013-02-20 19:41 UTC (permalink / raw)
  To: Frederic Weisbecker
  Cc: Mats Liljegren, linux-kernel, linux-arm-kernel, linaro-dev

With the 64-bit requirement removed from virt CPU accounting,
allow ARM platforms to enable it.

Signed-off-by: Kevin Hilman <khilman@linaro.org>
---
 arch/arm/Kconfig | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
index 9bbe760..732f8d3 100644
--- a/arch/arm/Kconfig
+++ b/arch/arm/Kconfig
@@ -55,6 +55,7 @@ config ARM
 	select HAVE_MOD_ARCH_SPECIFIC if ARM_UNWIND
 	select MODULES_USE_ELF_REL
 	select CLONE_BACKWARDS
+	select HAVE_VIRT_CPU_ACCOUNTING
 	help
 	  The ARM series is a line of low-power-consumption RISC chip designs
 	  licensed by ARM Ltd and targeted at embedded applications and
-- 
1.8.1.2


^ permalink raw reply related	[flat|nested] 42+ messages in thread

* [RFC/PATCH 5/5] ARM: Kconfig: allow virt CPU accounting
@ 2013-02-20 19:41   ` Kevin Hilman
  0 siblings, 0 replies; 42+ messages in thread
From: Kevin Hilman @ 2013-02-20 19:41 UTC (permalink / raw)
  To: linux-arm-kernel

With the 64-bit requirement removed from virt CPU accounting,
allow ARM platforms to enable it.

Signed-off-by: Kevin Hilman <khilman@linaro.org>
---
 arch/arm/Kconfig | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
index 9bbe760..732f8d3 100644
--- a/arch/arm/Kconfig
+++ b/arch/arm/Kconfig
@@ -55,6 +55,7 @@ config ARM
 	select HAVE_MOD_ARCH_SPECIFIC if ARM_UNWIND
 	select MODULES_USE_ELF_REL
 	select CLONE_BACKWARDS
+	select HAVE_VIRT_CPU_ACCOUNTING
 	help
 	  The ARM series is a line of low-power-consumption RISC chip designs
 	  licensed by ARM Ltd and targeted at embedded applications and
-- 
1.8.1.2

^ permalink raw reply related	[flat|nested] 42+ messages in thread

* Re: [RFC/PATCH 4/5] cputime: use do_div() for nsec resolution conversion helpers
  2013-02-20 19:41   ` Kevin Hilman
@ 2013-02-21 16:24     ` Frederic Weisbecker
  -1 siblings, 0 replies; 42+ messages in thread
From: Frederic Weisbecker @ 2013-02-21 16:24 UTC (permalink / raw)
  To: Kevin Hilman; +Cc: Mats Liljegren, linux-kernel, linux-arm-kernel, linaro-dev

2013/2/20 Kevin Hilman <khilman@linaro.org>:
> For the nsec resolution conversions to be useful on non 64-bit
> architectures, do_div() needs to be used for the 64-bit divisions.
>
> Signed-off-by: Kevin Hilman <khilman@linaro.org>
> ---

The patch looks good. I'll run and apply it if everything's fine.

^ permalink raw reply	[flat|nested] 42+ messages in thread

* [RFC/PATCH 4/5] cputime: use do_div() for nsec resolution conversion helpers
@ 2013-02-21 16:24     ` Frederic Weisbecker
  0 siblings, 0 replies; 42+ messages in thread
From: Frederic Weisbecker @ 2013-02-21 16:24 UTC (permalink / raw)
  To: linux-arm-kernel

2013/2/20 Kevin Hilman <khilman@linaro.org>:
> For the nsec resolution conversions to be useful on non 64-bit
> architectures, do_div() needs to be used for the 64-bit divisions.
>
> Signed-off-by: Kevin Hilman <khilman@linaro.org>
> ---

The patch looks good. I'll run and apply it if everything's fine.

^ permalink raw reply	[flat|nested] 42+ messages in thread

* Re: [RFC/PATCH 4/5] cputime: use do_div() for nsec resolution conversion helpers
  2013-02-20 19:41   ` Kevin Hilman
@ 2013-02-21 17:58     ` Namhyung Kim
  -1 siblings, 0 replies; 42+ messages in thread
From: Namhyung Kim @ 2013-02-21 17:58 UTC (permalink / raw)
  To: Kevin Hilman
  Cc: Frederic Weisbecker, Mats Liljegren, linaro-dev, linux-kernel,
	linux-arm-kernel

Hi Kevin,

On Wed, Feb 20, 2013 at 11:41:41AM -0800, Kevin Hilman wrote:
> For the nsec resolution conversions to be useful on non 64-bit
> architectures, do_div() needs to be used for the 64-bit divisions.
> 
> Signed-off-by: Kevin Hilman <khilman@linaro.org>
> ---
>  include/asm-generic/cputime_nsecs.h | 51 +++++++++++++++++++++++++++----------
>  1 file changed, 37 insertions(+), 14 deletions(-)
> 
> diff --git a/include/asm-generic/cputime_nsecs.h b/include/asm-generic/cputime_nsecs.h
> index b6485ca..daa6075 100644
> --- a/include/asm-generic/cputime_nsecs.h
> +++ b/include/asm-generic/cputime_nsecs.h
> @@ -24,13 +24,17 @@ typedef u64 __nocast cputime64_t;
>  /*
>   * Convert cputime <-> jiffies (HZ)
>   */
> -#define cputime_to_jiffies(__ct)	\
> -	((__force u64)(__ct) / (NSEC_PER_SEC / HZ))
> +static inline u64 cputime_to_jiffies(const cputime_t ct)
> +{
> +	cputime_t __ct = ct;

Why it removed the "(__force u64)" conversion part?  Shouldn't it be:

	u64 __ct = (__force u64) ct;

?  (and ditto for below changes)

Thanks,
Namhyung


> +
> +	do_div(__ct, NSEC_PER_SEC / HZ);
> +	return __ct;
> +}
>  #define cputime_to_scaled(__ct)		(__ct)
>  #define jiffies_to_cputime(__jif)	\
>  	(__force cputime_t)((__jif) * (NSEC_PER_SEC / HZ))
> -#define cputime64_to_jiffies64(__ct)	\
> -	((__force u64)(__ct) / (NSEC_PER_SEC / HZ))
> +#define cputime64_to_jiffies64(__ct) cputime_to_jiffies(__ct)
>  #define jiffies64_to_cputime64(__jif)	\
>  	(__force cputime64_t)((__jif) * (NSEC_PER_SEC / HZ))
>  
> @@ -44,8 +48,13 @@ typedef u64 __nocast cputime64_t;
>  /*
>   * Convert cputime <-> microseconds
>   */
> -#define cputime_to_usecs(__ct)		\
> -	((__force u64)(__ct) / NSEC_PER_USEC)
> +static inline u64 cputime_to_usecs(const cputime_t ct)
> +{
> +	cputime_t __ct = ct;
> +
> +	do_div(__ct, NSEC_PER_USEC);
> +	return __ct;
> +}
>  #define usecs_to_cputime(__usecs)	\
>  	(__force cputime_t)((__usecs) * NSEC_PER_USEC)
>  #define usecs_to_cputime64(__usecs)	\
> @@ -54,8 +63,13 @@ typedef u64 __nocast cputime64_t;
>  /*
>   * Convert cputime <-> seconds
>   */
> -#define cputime_to_secs(__ct)		\
> -	((__force u64)(__ct) / NSEC_PER_SEC)
> +static inline u64 cputime_to_secs(const cputime_t ct)
> +{
> +	cputime_t __ct = ct;
> +
> +	do_div(__ct, NSEC_PER_SEC);
> +	return __ct;
> +}
>  #define secs_to_cputime(__secs)		\
>  	(__force cputime_t)((__secs) * NSEC_PER_SEC)
>  
> @@ -69,8 +83,10 @@ static inline cputime_t timespec_to_cputime(const struct timespec *val)
>  }
>  static inline void cputime_to_timespec(const cputime_t ct, struct timespec *val)
>  {
> -	val->tv_sec  = (__force u64) ct / NSEC_PER_SEC;
> -	val->tv_nsec = (__force u64) ct % NSEC_PER_SEC;
> +	cputime_t __ct = ct;
> +
> +	val->tv_nsec = do_div(__ct, NSEC_PER_SEC);
> +	val->tv_sec = __ct;
>  }
>  
>  /*
> @@ -83,15 +99,22 @@ static inline cputime_t timeval_to_cputime(struct timeval *val)
>  }
>  static inline void cputime_to_timeval(const cputime_t ct, struct timeval *val)
>  {
> -	val->tv_sec = (__force u64) ct / NSEC_PER_SEC;
> -	val->tv_usec = ((__force u64) ct % NSEC_PER_SEC) / NSEC_PER_USEC;
> +	cputime_t __ct = ct;
> +
> +	val->tv_usec = do_div(__ct, NSEC_PER_SEC) / NSEC_PER_USEC;
> +	val->tv_sec = __ct;
>  }
>  
>  /*
>   * Convert cputime <-> clock (USER_HZ)
>   */
> -#define cputime_to_clock_t(__ct)	\
> -	((__force u64)(__ct) / (NSEC_PER_SEC / USER_HZ))
> +static inline u64 cputime_to_clock_t(const cputime_t ct)
> +{
> +	cputime_t __ct = ct;
> +
> +	do_div(__ct, (NSEC_PER_SEC / USER_HZ));
> +	return __ct;
> +}
>  #define clock_t_to_cputime(__x)		\
>  	(__force cputime_t)((__x) * (NSEC_PER_SEC / USER_HZ))
>  
> -- 
> 1.8.1.2
> 
> 
> _______________________________________________
> linaro-dev mailing list
> linaro-dev@lists.linaro.org
> http://lists.linaro.org/mailman/listinfo/linaro-dev

^ permalink raw reply	[flat|nested] 42+ messages in thread

* [RFC/PATCH 4/5] cputime: use do_div() for nsec resolution conversion helpers
@ 2013-02-21 17:58     ` Namhyung Kim
  0 siblings, 0 replies; 42+ messages in thread
From: Namhyung Kim @ 2013-02-21 17:58 UTC (permalink / raw)
  To: linux-arm-kernel

Hi Kevin,

On Wed, Feb 20, 2013 at 11:41:41AM -0800, Kevin Hilman wrote:
> For the nsec resolution conversions to be useful on non 64-bit
> architectures, do_div() needs to be used for the 64-bit divisions.
> 
> Signed-off-by: Kevin Hilman <khilman@linaro.org>
> ---
>  include/asm-generic/cputime_nsecs.h | 51 +++++++++++++++++++++++++++----------
>  1 file changed, 37 insertions(+), 14 deletions(-)
> 
> diff --git a/include/asm-generic/cputime_nsecs.h b/include/asm-generic/cputime_nsecs.h
> index b6485ca..daa6075 100644
> --- a/include/asm-generic/cputime_nsecs.h
> +++ b/include/asm-generic/cputime_nsecs.h
> @@ -24,13 +24,17 @@ typedef u64 __nocast cputime64_t;
>  /*
>   * Convert cputime <-> jiffies (HZ)
>   */
> -#define cputime_to_jiffies(__ct)	\
> -	((__force u64)(__ct) / (NSEC_PER_SEC / HZ))
> +static inline u64 cputime_to_jiffies(const cputime_t ct)
> +{
> +	cputime_t __ct = ct;

Why it removed the "(__force u64)" conversion part?  Shouldn't it be:

	u64 __ct = (__force u64) ct;

?  (and ditto for below changes)

Thanks,
Namhyung


> +
> +	do_div(__ct, NSEC_PER_SEC / HZ);
> +	return __ct;
> +}
>  #define cputime_to_scaled(__ct)		(__ct)
>  #define jiffies_to_cputime(__jif)	\
>  	(__force cputime_t)((__jif) * (NSEC_PER_SEC / HZ))
> -#define cputime64_to_jiffies64(__ct)	\
> -	((__force u64)(__ct) / (NSEC_PER_SEC / HZ))
> +#define cputime64_to_jiffies64(__ct) cputime_to_jiffies(__ct)
>  #define jiffies64_to_cputime64(__jif)	\
>  	(__force cputime64_t)((__jif) * (NSEC_PER_SEC / HZ))
>  
> @@ -44,8 +48,13 @@ typedef u64 __nocast cputime64_t;
>  /*
>   * Convert cputime <-> microseconds
>   */
> -#define cputime_to_usecs(__ct)		\
> -	((__force u64)(__ct) / NSEC_PER_USEC)
> +static inline u64 cputime_to_usecs(const cputime_t ct)
> +{
> +	cputime_t __ct = ct;
> +
> +	do_div(__ct, NSEC_PER_USEC);
> +	return __ct;
> +}
>  #define usecs_to_cputime(__usecs)	\
>  	(__force cputime_t)((__usecs) * NSEC_PER_USEC)
>  #define usecs_to_cputime64(__usecs)	\
> @@ -54,8 +63,13 @@ typedef u64 __nocast cputime64_t;
>  /*
>   * Convert cputime <-> seconds
>   */
> -#define cputime_to_secs(__ct)		\
> -	((__force u64)(__ct) / NSEC_PER_SEC)
> +static inline u64 cputime_to_secs(const cputime_t ct)
> +{
> +	cputime_t __ct = ct;
> +
> +	do_div(__ct, NSEC_PER_SEC);
> +	return __ct;
> +}
>  #define secs_to_cputime(__secs)		\
>  	(__force cputime_t)((__secs) * NSEC_PER_SEC)
>  
> @@ -69,8 +83,10 @@ static inline cputime_t timespec_to_cputime(const struct timespec *val)
>  }
>  static inline void cputime_to_timespec(const cputime_t ct, struct timespec *val)
>  {
> -	val->tv_sec  = (__force u64) ct / NSEC_PER_SEC;
> -	val->tv_nsec = (__force u64) ct % NSEC_PER_SEC;
> +	cputime_t __ct = ct;
> +
> +	val->tv_nsec = do_div(__ct, NSEC_PER_SEC);
> +	val->tv_sec = __ct;
>  }
>  
>  /*
> @@ -83,15 +99,22 @@ static inline cputime_t timeval_to_cputime(struct timeval *val)
>  }
>  static inline void cputime_to_timeval(const cputime_t ct, struct timeval *val)
>  {
> -	val->tv_sec = (__force u64) ct / NSEC_PER_SEC;
> -	val->tv_usec = ((__force u64) ct % NSEC_PER_SEC) / NSEC_PER_USEC;
> +	cputime_t __ct = ct;
> +
> +	val->tv_usec = do_div(__ct, NSEC_PER_SEC) / NSEC_PER_USEC;
> +	val->tv_sec = __ct;
>  }
>  
>  /*
>   * Convert cputime <-> clock (USER_HZ)
>   */
> -#define cputime_to_clock_t(__ct)	\
> -	((__force u64)(__ct) / (NSEC_PER_SEC / USER_HZ))
> +static inline u64 cputime_to_clock_t(const cputime_t ct)
> +{
> +	cputime_t __ct = ct;
> +
> +	do_div(__ct, (NSEC_PER_SEC / USER_HZ));
> +	return __ct;
> +}
>  #define clock_t_to_cputime(__x)		\
>  	(__force cputime_t)((__x) * (NSEC_PER_SEC / USER_HZ))
>  
> -- 
> 1.8.1.2
> 
> 
> _______________________________________________
> linaro-dev mailing list
> linaro-dev at lists.linaro.org
> http://lists.linaro.org/mailman/listinfo/linaro-dev

^ permalink raw reply	[flat|nested] 42+ messages in thread

* Re: [RFC/PATCH 4/5] cputime: use do_div() for nsec resolution conversion helpers
  2013-02-21 17:58     ` Namhyung Kim
@ 2013-02-21 19:21       ` Kevin Hilman
  -1 siblings, 0 replies; 42+ messages in thread
From: Kevin Hilman @ 2013-02-21 19:21 UTC (permalink / raw)
  To: Namhyung Kim
  Cc: Frederic Weisbecker, Mats Liljegren, linaro-dev, linux-kernel,
	linux-arm-kernel

Namhyung Kim <namhyung@kernel.org> writes:

> Hi Kevin,
>
> On Wed, Feb 20, 2013 at 11:41:41AM -0800, Kevin Hilman wrote:
>> For the nsec resolution conversions to be useful on non 64-bit
>> architectures, do_div() needs to be used for the 64-bit divisions.
>> 
>> Signed-off-by: Kevin Hilman <khilman@linaro.org>
>> ---
>>  include/asm-generic/cputime_nsecs.h | 51 +++++++++++++++++++++++++++----------
>>  1 file changed, 37 insertions(+), 14 deletions(-)
>> 
>> diff --git a/include/asm-generic/cputime_nsecs.h b/include/asm-generic/cputime_nsecs.h
>> index b6485ca..daa6075 100644
>> --- a/include/asm-generic/cputime_nsecs.h
>> +++ b/include/asm-generic/cputime_nsecs.h
>> @@ -24,13 +24,17 @@ typedef u64 __nocast cputime64_t;
>>  /*
>>   * Convert cputime <-> jiffies (HZ)
>>   */
>> -#define cputime_to_jiffies(__ct)	\
>> -	((__force u64)(__ct) / (NSEC_PER_SEC / HZ))
>> +static inline u64 cputime_to_jiffies(const cputime_t ct)
>> +{
>> +	cputime_t __ct = ct;
>
> Why it removed the "(__force u64)" conversion part?  Shouldn't it be:
>
> 	u64 __ct = (__force u64) ct;
>
> ?

You're right, that was an oversight.  Updated patch below.

Frederic, if you merge this, please take the one below with the __force
attributes added back.

I've updated my branch to include the fixed version.

Kevin


>From a8a0a8b8b12512a7f862ade459cd88d2b48e2bf3 Mon Sep 17 00:00:00 2001
From: Kevin Hilman <khilman@linaro.org>
Date: Thu, 14 Feb 2013 11:27:36 -0800
Subject: [PATCH 4/5] cputime: use do_div() for nsec resolution conversion
 helpers

For the nsec resolution conversions to be useful on non 64-bit
architectures, do_div() needs to be used for the 64-bit divisions.

Special thanks to Namhyung Kim for pointing out omissions of the
__force attribute in an earlier version.

Cc: Namhyung Kim <namhyung@kernel.org>
Signed-off-by: Kevin Hilman <khilman@linaro.org>
---
 include/asm-generic/cputime_nsecs.h | 51 +++++++++++++++++++++++++++----------
 1 file changed, 37 insertions(+), 14 deletions(-)

diff --git a/include/asm-generic/cputime_nsecs.h b/include/asm-generic/cputime_nsecs.h
index b6485ca..d4944c9 100644
--- a/include/asm-generic/cputime_nsecs.h
+++ b/include/asm-generic/cputime_nsecs.h
@@ -24,13 +24,17 @@ typedef u64 __nocast cputime64_t;
 /*
  * Convert cputime <-> jiffies (HZ)
  */
-#define cputime_to_jiffies(__ct)	\
-	((__force u64)(__ct) / (NSEC_PER_SEC / HZ))
+static inline u64 cputime_to_jiffies(const cputime_t ct)
+{
+	u64 __ct = (__force u64) ct;
+
+	do_div(__ct, NSEC_PER_SEC / HZ);
+	return __ct;
+}
 #define cputime_to_scaled(__ct)		(__ct)
 #define jiffies_to_cputime(__jif)	\
 	(__force cputime_t)((__jif) * (NSEC_PER_SEC / HZ))
-#define cputime64_to_jiffies64(__ct)	\
-	((__force u64)(__ct) / (NSEC_PER_SEC / HZ))
+#define cputime64_to_jiffies64(__ct) cputime_to_jiffies(__ct)
 #define jiffies64_to_cputime64(__jif)	\
 	(__force cputime64_t)((__jif) * (NSEC_PER_SEC / HZ))
 
@@ -44,8 +48,13 @@ typedef u64 __nocast cputime64_t;
 /*
  * Convert cputime <-> microseconds
  */
-#define cputime_to_usecs(__ct)		\
-	((__force u64)(__ct) / NSEC_PER_USEC)
+static inline u64 cputime_to_usecs(const cputime_t ct)
+{
+	u64 __ct = (__force u64) ct;
+
+	do_div(__ct, NSEC_PER_USEC);
+	return __ct;
+}
 #define usecs_to_cputime(__usecs)	\
 	(__force cputime_t)((__usecs) * NSEC_PER_USEC)
 #define usecs_to_cputime64(__usecs)	\
@@ -54,8 +63,13 @@ typedef u64 __nocast cputime64_t;
 /*
  * Convert cputime <-> seconds
  */
-#define cputime_to_secs(__ct)		\
-	((__force u64)(__ct) / NSEC_PER_SEC)
+static inline u64 cputime_to_secs(const cputime_t ct)
+{
+	u64 __ct = (__force u64) ct;
+
+	do_div(__ct, NSEC_PER_SEC);
+	return __ct;
+}
 #define secs_to_cputime(__secs)		\
 	(__force cputime_t)((__secs) * NSEC_PER_SEC)
 
@@ -69,8 +83,10 @@ static inline cputime_t timespec_to_cputime(const struct timespec *val)
 }
 static inline void cputime_to_timespec(const cputime_t ct, struct timespec *val)
 {
-	val->tv_sec  = (__force u64) ct / NSEC_PER_SEC;
-	val->tv_nsec = (__force u64) ct % NSEC_PER_SEC;
+	u64 __ct = (__force u64) ct;
+
+	val->tv_nsec = do_div(__ct, NSEC_PER_SEC);
+	val->tv_sec = __ct;
 }
 
 /*
@@ -83,15 +99,22 @@ static inline cputime_t timeval_to_cputime(struct timeval *val)
 }
 static inline void cputime_to_timeval(const cputime_t ct, struct timeval *val)
 {
-	val->tv_sec = (__force u64) ct / NSEC_PER_SEC;
-	val->tv_usec = ((__force u64) ct % NSEC_PER_SEC) / NSEC_PER_USEC;
+	u64 __ct = (__force u64) ct;
+
+	val->tv_usec = do_div(__ct, NSEC_PER_SEC) / NSEC_PER_USEC;
+	val->tv_sec = __ct;
 }
 
 /*
  * Convert cputime <-> clock (USER_HZ)
  */
-#define cputime_to_clock_t(__ct)	\
-	((__force u64)(__ct) / (NSEC_PER_SEC / USER_HZ))
+static inline u64 cputime_to_clock_t(const cputime_t ct)
+{
+	u64 __ct = (__force u64) ct;
+
+	do_div(__ct, (NSEC_PER_SEC / USER_HZ));
+	return __ct;
+}
 #define clock_t_to_cputime(__x)		\
 	(__force cputime_t)((__x) * (NSEC_PER_SEC / USER_HZ))
 
-- 
1.8.1.2


^ permalink raw reply related	[flat|nested] 42+ messages in thread

* [RFC/PATCH 4/5] cputime: use do_div() for nsec resolution conversion helpers
@ 2013-02-21 19:21       ` Kevin Hilman
  0 siblings, 0 replies; 42+ messages in thread
From: Kevin Hilman @ 2013-02-21 19:21 UTC (permalink / raw)
  To: linux-arm-kernel

Namhyung Kim <namhyung@kernel.org> writes:

> Hi Kevin,
>
> On Wed, Feb 20, 2013 at 11:41:41AM -0800, Kevin Hilman wrote:
>> For the nsec resolution conversions to be useful on non 64-bit
>> architectures, do_div() needs to be used for the 64-bit divisions.
>> 
>> Signed-off-by: Kevin Hilman <khilman@linaro.org>
>> ---
>>  include/asm-generic/cputime_nsecs.h | 51 +++++++++++++++++++++++++++----------
>>  1 file changed, 37 insertions(+), 14 deletions(-)
>> 
>> diff --git a/include/asm-generic/cputime_nsecs.h b/include/asm-generic/cputime_nsecs.h
>> index b6485ca..daa6075 100644
>> --- a/include/asm-generic/cputime_nsecs.h
>> +++ b/include/asm-generic/cputime_nsecs.h
>> @@ -24,13 +24,17 @@ typedef u64 __nocast cputime64_t;
>>  /*
>>   * Convert cputime <-> jiffies (HZ)
>>   */
>> -#define cputime_to_jiffies(__ct)	\
>> -	((__force u64)(__ct) / (NSEC_PER_SEC / HZ))
>> +static inline u64 cputime_to_jiffies(const cputime_t ct)
>> +{
>> +	cputime_t __ct = ct;
>
> Why it removed the "(__force u64)" conversion part?  Shouldn't it be:
>
> 	u64 __ct = (__force u64) ct;
>
> ?

You're right, that was an oversight.  Updated patch below.

Frederic, if you merge this, please take the one below with the __force
attributes added back.

I've updated my branch to include the fixed version.

Kevin


>From a8a0a8b8b12512a7f862ade459cd88d2b48e2bf3 Mon Sep 17 00:00:00 2001
From: Kevin Hilman <khilman@linaro.org>
Date: Thu, 14 Feb 2013 11:27:36 -0800
Subject: [PATCH 4/5] cputime: use do_div() for nsec resolution conversion
 helpers

For the nsec resolution conversions to be useful on non 64-bit
architectures, do_div() needs to be used for the 64-bit divisions.

Special thanks to Namhyung Kim for pointing out omissions of the
__force attribute in an earlier version.

Cc: Namhyung Kim <namhyung@kernel.org>
Signed-off-by: Kevin Hilman <khilman@linaro.org>
---
 include/asm-generic/cputime_nsecs.h | 51 +++++++++++++++++++++++++++----------
 1 file changed, 37 insertions(+), 14 deletions(-)

diff --git a/include/asm-generic/cputime_nsecs.h b/include/asm-generic/cputime_nsecs.h
index b6485ca..d4944c9 100644
--- a/include/asm-generic/cputime_nsecs.h
+++ b/include/asm-generic/cputime_nsecs.h
@@ -24,13 +24,17 @@ typedef u64 __nocast cputime64_t;
 /*
  * Convert cputime <-> jiffies (HZ)
  */
-#define cputime_to_jiffies(__ct)	\
-	((__force u64)(__ct) / (NSEC_PER_SEC / HZ))
+static inline u64 cputime_to_jiffies(const cputime_t ct)
+{
+	u64 __ct = (__force u64) ct;
+
+	do_div(__ct, NSEC_PER_SEC / HZ);
+	return __ct;
+}
 #define cputime_to_scaled(__ct)		(__ct)
 #define jiffies_to_cputime(__jif)	\
 	(__force cputime_t)((__jif) * (NSEC_PER_SEC / HZ))
-#define cputime64_to_jiffies64(__ct)	\
-	((__force u64)(__ct) / (NSEC_PER_SEC / HZ))
+#define cputime64_to_jiffies64(__ct) cputime_to_jiffies(__ct)
 #define jiffies64_to_cputime64(__jif)	\
 	(__force cputime64_t)((__jif) * (NSEC_PER_SEC / HZ))
 
@@ -44,8 +48,13 @@ typedef u64 __nocast cputime64_t;
 /*
  * Convert cputime <-> microseconds
  */
-#define cputime_to_usecs(__ct)		\
-	((__force u64)(__ct) / NSEC_PER_USEC)
+static inline u64 cputime_to_usecs(const cputime_t ct)
+{
+	u64 __ct = (__force u64) ct;
+
+	do_div(__ct, NSEC_PER_USEC);
+	return __ct;
+}
 #define usecs_to_cputime(__usecs)	\
 	(__force cputime_t)((__usecs) * NSEC_PER_USEC)
 #define usecs_to_cputime64(__usecs)	\
@@ -54,8 +63,13 @@ typedef u64 __nocast cputime64_t;
 /*
  * Convert cputime <-> seconds
  */
-#define cputime_to_secs(__ct)		\
-	((__force u64)(__ct) / NSEC_PER_SEC)
+static inline u64 cputime_to_secs(const cputime_t ct)
+{
+	u64 __ct = (__force u64) ct;
+
+	do_div(__ct, NSEC_PER_SEC);
+	return __ct;
+}
 #define secs_to_cputime(__secs)		\
 	(__force cputime_t)((__secs) * NSEC_PER_SEC)
 
@@ -69,8 +83,10 @@ static inline cputime_t timespec_to_cputime(const struct timespec *val)
 }
 static inline void cputime_to_timespec(const cputime_t ct, struct timespec *val)
 {
-	val->tv_sec  = (__force u64) ct / NSEC_PER_SEC;
-	val->tv_nsec = (__force u64) ct % NSEC_PER_SEC;
+	u64 __ct = (__force u64) ct;
+
+	val->tv_nsec = do_div(__ct, NSEC_PER_SEC);
+	val->tv_sec = __ct;
 }
 
 /*
@@ -83,15 +99,22 @@ static inline cputime_t timeval_to_cputime(struct timeval *val)
 }
 static inline void cputime_to_timeval(const cputime_t ct, struct timeval *val)
 {
-	val->tv_sec = (__force u64) ct / NSEC_PER_SEC;
-	val->tv_usec = ((__force u64) ct % NSEC_PER_SEC) / NSEC_PER_USEC;
+	u64 __ct = (__force u64) ct;
+
+	val->tv_usec = do_div(__ct, NSEC_PER_SEC) / NSEC_PER_USEC;
+	val->tv_sec = __ct;
 }
 
 /*
  * Convert cputime <-> clock (USER_HZ)
  */
-#define cputime_to_clock_t(__ct)	\
-	((__force u64)(__ct) / (NSEC_PER_SEC / USER_HZ))
+static inline u64 cputime_to_clock_t(const cputime_t ct)
+{
+	u64 __ct = (__force u64) ct;
+
+	do_div(__ct, (NSEC_PER_SEC / USER_HZ));
+	return __ct;
+}
 #define clock_t_to_cputime(__x)		\
 	(__force cputime_t)((__x) * (NSEC_PER_SEC / USER_HZ))
 
-- 
1.8.1.2

^ permalink raw reply related	[flat|nested] 42+ messages in thread

* Re: [RFC/PATCH 2/5] kernel_cpustat: convert to atomic 64-bit accessors
  2013-02-20 19:41   ` Kevin Hilman
@ 2013-02-21 19:38     ` Kevin Hilman
  -1 siblings, 0 replies; 42+ messages in thread
From: Kevin Hilman @ 2013-02-21 19:38 UTC (permalink / raw)
  To: Frederic Weisbecker
  Cc: Mats Liljegren, linux-kernel, linux-arm-kernel, linaro-dev

Kevin Hilman <khilman@linaro.org> writes:

> Use the atomic64_* accessors for all the kernel_cpustat fields to
> ensure atomic access on non-64 bit platforms.
>
> Thanks to Mats Liljegren for CGROUP_CPUACCT related fixes.
>
> Cc: Mats Liljegren <mats.liljegren@enea.com>
> Signed-off-by: Kevin Hilman <khilman@linaro.org>

The kbuild test bot reported some build errors where I missed some
conversions (e.g. drivers/cpufreq and arch/s390/appldata).

Below is an updated patch that adds in those changes.

I've updated my branch with this version.

Kevin

>From fff74c8e41bb68f48639441484dd0ad4fc7137aa Mon Sep 17 00:00:00 2001
From: Kevin Hilman <khilman@linaro.org>
Date: Thu, 14 Feb 2013 17:46:08 -0800
Subject: [PATCH 2/5] kernel_cpustat: convert to atomic 64-bit accessors

Use the atomic64_* accessors for all the kernel_cpustat fields to
ensure atomic access on non-64 bit platforms.

Thanks to Mats Liljegren for CGROUP_CPUACCT related fixes.

Cc: Mats Liljegren <mats.liljegren@enea.com>
Signed-off-by: Kevin Hilman <khilman@linaro.org>
---
 arch/s390/appldata/appldata_os.c   | 41 +++++++++++++++++++++++---------------
 drivers/cpufreq/cpufreq_governor.c | 18 ++++++++---------
 drivers/cpufreq/cpufreq_ondemand.c |  2 +-
 drivers/macintosh/rack-meter.c     |  6 +++---
 fs/proc/stat.c                     | 40 ++++++++++++++++++-------------------
 fs/proc/uptime.c                   |  2 +-
 include/linux/kernel_stat.h        |  2 +-
 kernel/sched/core.c                | 10 +++++-----
 kernel/sched/cputime.c             | 38 +++++++++++++++++------------------
 9 files changed, 84 insertions(+), 75 deletions(-)

diff --git a/arch/s390/appldata/appldata_os.c b/arch/s390/appldata/appldata_os.c
index 87521ba..008b180 100644
--- a/arch/s390/appldata/appldata_os.c
+++ b/arch/s390/appldata/appldata_os.c
@@ -99,6 +99,7 @@ static void appldata_get_os_data(void *data)
 	int i, j, rc;
 	struct appldata_os_data *os_data;
 	unsigned int new_size;
+	u64 val;
 
 	os_data = data;
 	os_data->sync_count_1++;
@@ -112,22 +113,30 @@ static void appldata_get_os_data(void *data)
 
 	j = 0;
 	for_each_online_cpu(i) {
-		os_data->os_cpu[j].per_cpu_user =
-			cputime_to_jiffies(kcpustat_cpu(i).cpustat[CPUTIME_USER]);
-		os_data->os_cpu[j].per_cpu_nice =
-			cputime_to_jiffies(kcpustat_cpu(i).cpustat[CPUTIME_NICE]);
-		os_data->os_cpu[j].per_cpu_system =
-			cputime_to_jiffies(kcpustat_cpu(i).cpustat[CPUTIME_SYSTEM]);
-		os_data->os_cpu[j].per_cpu_idle =
-			cputime_to_jiffies(kcpustat_cpu(i).cpustat[CPUTIME_IDLE]);
-		os_data->os_cpu[j].per_cpu_irq =
-			cputime_to_jiffies(kcpustat_cpu(i).cpustat[CPUTIME_IRQ]);
-		os_data->os_cpu[j].per_cpu_softirq =
-			cputime_to_jiffies(kcpustat_cpu(i).cpustat[CPUTIME_SOFTIRQ]);
-		os_data->os_cpu[j].per_cpu_iowait =
-			cputime_to_jiffies(kcpustat_cpu(i).cpustat[CPUTIME_IOWAIT]);
-		os_data->os_cpu[j].per_cpu_steal =
-			cputime_to_jiffies(kcpustat_cpu(i).cpustat[CPUTIME_STEAL]);
+		val = atomic64_read(&kcpustat_cpu(i).cpustat[CPUTIME_USER]);
+		os_data->os_cpu[j].per_cpu_user = cputime_to_jiffies(val);
+			
+		val = atomic64_read(&kcpustat_cpu(i).cpustat[CPUTIME_NICE]);
+		os_data->os_cpu[j].per_cpu_nice = cputime_to_jiffies(val);
+
+		val = atomic64_read(&kcpustat_cpu(i).cpustat[CPUTIME_SYSTEM]);
+		os_data->os_cpu[j].per_cpu_system = cputime_to_jiffies(val);
+
+		val = atomci64_read(&kcpustat_cpu(i).cpustat[CPUTIME_IDLE]);
+		os_data->os_cpu[j].per_cpu_idle = cputime_to_jiffies(val);
+
+		val = atomci64_read(&kcpustat_cpu(i).cpustat[CPUTIME_IRQ]);
+		os_data->os_cpu[j].per_cpu_irq = cputime_to_jiffies(val);
+
+		val = atomci64_read(&kcpustat_cpu(i).cpustat[CPUTIME_SOFTIRQ]);
+		os_data->os_cpu[j].per_cpu_softirq = cputime_to_jiffies(val);
+
+		val = atomic64_read(&kcpustat_cpu(i).cpustat[CPUTIME_IOWAIT]);
+		os_data->os_cpu[j].per_cpu_iowait = cputime_to_jiffies(val);
+
+		val = atomic64_read(&kcpustat_cpu(i).cpustat[CPUTIME_STEAL]);
+		os_data->os_cpu[j].per_cpu_steal = cputime_to_jiffies(val);
+
 		os_data->os_cpu[j].cpu_id = i;
 		j++;
 	}
diff --git a/drivers/cpufreq/cpufreq_governor.c b/drivers/cpufreq/cpufreq_governor.c
index 6c5f1d3..a239f8c 100644
--- a/drivers/cpufreq/cpufreq_governor.c
+++ b/drivers/cpufreq/cpufreq_governor.c
@@ -36,12 +36,12 @@ static inline u64 get_cpu_idle_time_jiffy(unsigned int cpu, u64 *wall)
 
 	cur_wall_time = jiffies64_to_cputime64(get_jiffies_64());
 
-	busy_time = kcpustat_cpu(cpu).cpustat[CPUTIME_USER];
-	busy_time += kcpustat_cpu(cpu).cpustat[CPUTIME_SYSTEM];
-	busy_time += kcpustat_cpu(cpu).cpustat[CPUTIME_IRQ];
-	busy_time += kcpustat_cpu(cpu).cpustat[CPUTIME_SOFTIRQ];
-	busy_time += kcpustat_cpu(cpu).cpustat[CPUTIME_STEAL];
-	busy_time += kcpustat_cpu(cpu).cpustat[CPUTIME_NICE];
+	busy_time = atomic64_read(&kcpustat_cpu(cpu).cpustat[CPUTIME_USER]);
+	busy_time += atomic64_read(&kcpustat_cpu(cpu).cpustat[CPUTIME_SYSTEM]);
+	busy_time += atomic64_read(&kcpustat_cpu(cpu).cpustat[CPUTIME_IRQ]);
+	busy_time += atomic64_read(&kcpustat_cpu(cpu).cpustat[CPUTIME_SOFTIRQ]);
+	busy_time += atomic64_read(&kcpustat_cpu(cpu).cpustat[CPUTIME_STEAL]);
+	busy_time += atomic64_read(&kcpustat_cpu(cpu).cpustat[CPUTIME_NICE]);
 
 	idle_time = cur_wall_time - busy_time;
 	if (wall)
@@ -103,7 +103,7 @@ void dbs_check_cpu(struct dbs_data *dbs_data, int cpu)
 			u64 cur_nice;
 			unsigned long cur_nice_jiffies;
 
-			cur_nice = kcpustat_cpu(j).cpustat[CPUTIME_NICE] -
+			cur_nice = atomic64_read(&kcpustat_cpu(j).cpustat[CPUTIME_NICE]) -
 					 cdbs->prev_cpu_nice;
 			/*
 			 * Assumption: nice time between sampling periods will
@@ -113,7 +113,7 @@ void dbs_check_cpu(struct dbs_data *dbs_data, int cpu)
 					cputime64_to_jiffies64(cur_nice);
 
 			cdbs->prev_cpu_nice =
-				kcpustat_cpu(j).cpustat[CPUTIME_NICE];
+				atomic64_read(&kcpustat_cpu(j).cpustat[CPUTIME_NICE]);
 			idle_time += jiffies_to_usecs(cur_nice_jiffies);
 		}
 
@@ -216,7 +216,7 @@ int cpufreq_governor_dbs(struct dbs_data *dbs_data,
 					&j_cdbs->prev_cpu_wall);
 			if (ignore_nice)
 				j_cdbs->prev_cpu_nice =
-					kcpustat_cpu(j).cpustat[CPUTIME_NICE];
+					atomic64_read(&kcpustat_cpu(j).cpustat[CPUTIME_NICE]);
 		}
 
 		/*
diff --git a/drivers/cpufreq/cpufreq_ondemand.c b/drivers/cpufreq/cpufreq_ondemand.c
index 7731f7c..d761c9f 100644
--- a/drivers/cpufreq/cpufreq_ondemand.c
+++ b/drivers/cpufreq/cpufreq_ondemand.c
@@ -403,7 +403,7 @@ static ssize_t store_ignore_nice_load(struct kobject *a, struct attribute *b,
 						&dbs_info->cdbs.prev_cpu_wall);
 		if (od_tuners.ignore_nice)
 			dbs_info->cdbs.prev_cpu_nice =
-				kcpustat_cpu(j).cpustat[CPUTIME_NICE];
+				atomic64_read(&kcpustat_cpu(j).cpustat[CPUTIME_NICE]);
 
 	}
 	return count;
diff --git a/drivers/macintosh/rack-meter.c b/drivers/macintosh/rack-meter.c
index cad0e19..597fe20 100644
--- a/drivers/macintosh/rack-meter.c
+++ b/drivers/macintosh/rack-meter.c
@@ -83,11 +83,11 @@ static inline cputime64_t get_cpu_idle_time(unsigned int cpu)
 {
 	u64 retval;
 
-	retval = kcpustat_cpu(cpu).cpustat[CPUTIME_IDLE] +
-		 kcpustat_cpu(cpu).cpustat[CPUTIME_IOWAIT];
+	retval = atomic64_read(&kcpustat_cpu(cpu).cpustat[CPUTIME_IDLE]) +
+ 		 atomic64_read(&kcpustat_cpu(cpu).cpustat[CPUTIME_IOWAIT]);
 
 	if (rackmeter_ignore_nice)
-		retval += kcpustat_cpu(cpu).cpustat[CPUTIME_NICE];
+		retval += atomic64_read(&kcpustat_cpu(cpu).cpustat[CPUTIME_NICE]);
 
 	return retval;
 }
diff --git a/fs/proc/stat.c b/fs/proc/stat.c
index e296572..93f7f30 100644
--- a/fs/proc/stat.c
+++ b/fs/proc/stat.c
@@ -25,7 +25,7 @@ static cputime64_t get_idle_time(int cpu)
 {
 	cputime64_t idle;
 
-	idle = kcpustat_cpu(cpu).cpustat[CPUTIME_IDLE];
+	idle = atomic64_read(&kcpustat_cpu(cpu).cpustat[CPUTIME_IDLE]);
 	if (cpu_online(cpu) && !nr_iowait_cpu(cpu))
 		idle += arch_idle_time(cpu);
 	return idle;
@@ -35,7 +35,7 @@ static cputime64_t get_iowait_time(int cpu)
 {
 	cputime64_t iowait;
 
-	iowait = kcpustat_cpu(cpu).cpustat[CPUTIME_IOWAIT];
+	iowait = atomic64_read(&kcpustat_cpu(cpu).cpustat[CPUTIME_IOWAIT]);
 	if (cpu_online(cpu) && nr_iowait_cpu(cpu))
 		iowait += arch_idle_time(cpu);
 	return iowait;
@@ -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/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 ccff275..4c639ee 100644
--- a/kernel/sched/cputime.c
+++ b/kernel/sched/cputime.c
@@ -78,14 +78,14 @@ EXPORT_SYMBOL_GPL(irqtime_account_irq);
 
 static int irqtime_account_hi_update(void)
 {
-	u64 *cpustat = kcpustat_this_cpu->cpustat;
+	atomic64_t *cpustat = kcpustat_this_cpu->cpustat;
 	unsigned long flags;
 	u64 latest_ns;
 	int ret = 0;
 
 	local_irq_save(flags);
 	latest_ns = this_cpu_read(cpu_hardirq_time);
-	if (nsecs_to_cputime64(latest_ns) > cpustat[CPUTIME_IRQ])
+	if (nsecs_to_cputime64(latest_ns) > atomic64_read(&cpustat[CPUTIME_IRQ]));
 		ret = 1;
 	local_irq_restore(flags);
 	return ret;
@@ -93,14 +93,14 @@ static int irqtime_account_hi_update(void)
 
 static int irqtime_account_si_update(void)
 {
-	u64 *cpustat = kcpustat_this_cpu->cpustat;
+	atomic64_t *cpustat = kcpustat_this_cpu->cpustat;
 	unsigned long flags;
 	u64 latest_ns;
 	int ret = 0;
 
 	local_irq_save(flags);
 	latest_ns = this_cpu_read(cpu_softirq_time);
-	if (nsecs_to_cputime64(latest_ns) > cpustat[CPUTIME_SOFTIRQ])
+	if (nsecs_to_cputime64(latest_ns) > atomic64_read(&cpustat[CPUTIME_SOFTIRQ]))
 		ret = 1;
 	local_irq_restore(flags);
 	return ret;
@@ -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))
@@ -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();
@@ -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)
@@ -345,15 +345,15 @@ static void irqtime_account_process_tick(struct task_struct *p, int user_tick,
 						struct rq *rq)
 {
 	cputime_t one_jiffy_scaled = cputime_to_scaled(cputime_one_jiffy);
-	u64 *cpustat = kcpustat_this_cpu->cpustat;
+	atomic64_t *cpustat = kcpustat_this_cpu->cpustat;
 
 	if (steal_account_process_tick())
 		return;
 
 	if (irqtime_account_hi_update()) {
-		cpustat[CPUTIME_IRQ] += (__force u64) cputime_one_jiffy;
+		atomic64_add((__force u64) cputime_one_jiffy, &cpustat[CPUTIME_IRQ]);
 	} else if (irqtime_account_si_update()) {
-		cpustat[CPUTIME_SOFTIRQ] += (__force u64) cputime_one_jiffy;
+		atomic64_add((__force u64) cputime_one_jiffy, &cpustat[CPUTIME_SOFTIRQ]);
 	} else if (this_cpu_ksoftirqd() == p) {
 		/*
 		 * ksoftirqd time do not get accounted in cpu_softirq_time.
-- 
1.8.1.2


^ permalink raw reply related	[flat|nested] 42+ messages in thread

* [RFC/PATCH 2/5] kernel_cpustat: convert to atomic 64-bit accessors
@ 2013-02-21 19:38     ` Kevin Hilman
  0 siblings, 0 replies; 42+ messages in thread
From: Kevin Hilman @ 2013-02-21 19:38 UTC (permalink / raw)
  To: linux-arm-kernel

Kevin Hilman <khilman@linaro.org> writes:

> Use the atomic64_* accessors for all the kernel_cpustat fields to
> ensure atomic access on non-64 bit platforms.
>
> Thanks to Mats Liljegren for CGROUP_CPUACCT related fixes.
>
> Cc: Mats Liljegren <mats.liljegren@enea.com>
> Signed-off-by: Kevin Hilman <khilman@linaro.org>

The kbuild test bot reported some build errors where I missed some
conversions (e.g. drivers/cpufreq and arch/s390/appldata).

Below is an updated patch that adds in those changes.

I've updated my branch with this version.

Kevin

>From fff74c8e41bb68f48639441484dd0ad4fc7137aa Mon Sep 17 00:00:00 2001
From: Kevin Hilman <khilman@linaro.org>
Date: Thu, 14 Feb 2013 17:46:08 -0800
Subject: [PATCH 2/5] kernel_cpustat: convert to atomic 64-bit accessors

Use the atomic64_* accessors for all the kernel_cpustat fields to
ensure atomic access on non-64 bit platforms.

Thanks to Mats Liljegren for CGROUP_CPUACCT related fixes.

Cc: Mats Liljegren <mats.liljegren@enea.com>
Signed-off-by: Kevin Hilman <khilman@linaro.org>
---
 arch/s390/appldata/appldata_os.c   | 41 +++++++++++++++++++++++---------------
 drivers/cpufreq/cpufreq_governor.c | 18 ++++++++---------
 drivers/cpufreq/cpufreq_ondemand.c |  2 +-
 drivers/macintosh/rack-meter.c     |  6 +++---
 fs/proc/stat.c                     | 40 ++++++++++++++++++-------------------
 fs/proc/uptime.c                   |  2 +-
 include/linux/kernel_stat.h        |  2 +-
 kernel/sched/core.c                | 10 +++++-----
 kernel/sched/cputime.c             | 38 +++++++++++++++++------------------
 9 files changed, 84 insertions(+), 75 deletions(-)

diff --git a/arch/s390/appldata/appldata_os.c b/arch/s390/appldata/appldata_os.c
index 87521ba..008b180 100644
--- a/arch/s390/appldata/appldata_os.c
+++ b/arch/s390/appldata/appldata_os.c
@@ -99,6 +99,7 @@ static void appldata_get_os_data(void *data)
 	int i, j, rc;
 	struct appldata_os_data *os_data;
 	unsigned int new_size;
+	u64 val;
 
 	os_data = data;
 	os_data->sync_count_1++;
@@ -112,22 +113,30 @@ static void appldata_get_os_data(void *data)
 
 	j = 0;
 	for_each_online_cpu(i) {
-		os_data->os_cpu[j].per_cpu_user =
-			cputime_to_jiffies(kcpustat_cpu(i).cpustat[CPUTIME_USER]);
-		os_data->os_cpu[j].per_cpu_nice =
-			cputime_to_jiffies(kcpustat_cpu(i).cpustat[CPUTIME_NICE]);
-		os_data->os_cpu[j].per_cpu_system =
-			cputime_to_jiffies(kcpustat_cpu(i).cpustat[CPUTIME_SYSTEM]);
-		os_data->os_cpu[j].per_cpu_idle =
-			cputime_to_jiffies(kcpustat_cpu(i).cpustat[CPUTIME_IDLE]);
-		os_data->os_cpu[j].per_cpu_irq =
-			cputime_to_jiffies(kcpustat_cpu(i).cpustat[CPUTIME_IRQ]);
-		os_data->os_cpu[j].per_cpu_softirq =
-			cputime_to_jiffies(kcpustat_cpu(i).cpustat[CPUTIME_SOFTIRQ]);
-		os_data->os_cpu[j].per_cpu_iowait =
-			cputime_to_jiffies(kcpustat_cpu(i).cpustat[CPUTIME_IOWAIT]);
-		os_data->os_cpu[j].per_cpu_steal =
-			cputime_to_jiffies(kcpustat_cpu(i).cpustat[CPUTIME_STEAL]);
+		val = atomic64_read(&kcpustat_cpu(i).cpustat[CPUTIME_USER]);
+		os_data->os_cpu[j].per_cpu_user = cputime_to_jiffies(val);
+			
+		val = atomic64_read(&kcpustat_cpu(i).cpustat[CPUTIME_NICE]);
+		os_data->os_cpu[j].per_cpu_nice = cputime_to_jiffies(val);
+
+		val = atomic64_read(&kcpustat_cpu(i).cpustat[CPUTIME_SYSTEM]);
+		os_data->os_cpu[j].per_cpu_system = cputime_to_jiffies(val);
+
+		val = atomci64_read(&kcpustat_cpu(i).cpustat[CPUTIME_IDLE]);
+		os_data->os_cpu[j].per_cpu_idle = cputime_to_jiffies(val);
+
+		val = atomci64_read(&kcpustat_cpu(i).cpustat[CPUTIME_IRQ]);
+		os_data->os_cpu[j].per_cpu_irq = cputime_to_jiffies(val);
+
+		val = atomci64_read(&kcpustat_cpu(i).cpustat[CPUTIME_SOFTIRQ]);
+		os_data->os_cpu[j].per_cpu_softirq = cputime_to_jiffies(val);
+
+		val = atomic64_read(&kcpustat_cpu(i).cpustat[CPUTIME_IOWAIT]);
+		os_data->os_cpu[j].per_cpu_iowait = cputime_to_jiffies(val);
+
+		val = atomic64_read(&kcpustat_cpu(i).cpustat[CPUTIME_STEAL]);
+		os_data->os_cpu[j].per_cpu_steal = cputime_to_jiffies(val);
+
 		os_data->os_cpu[j].cpu_id = i;
 		j++;
 	}
diff --git a/drivers/cpufreq/cpufreq_governor.c b/drivers/cpufreq/cpufreq_governor.c
index 6c5f1d3..a239f8c 100644
--- a/drivers/cpufreq/cpufreq_governor.c
+++ b/drivers/cpufreq/cpufreq_governor.c
@@ -36,12 +36,12 @@ static inline u64 get_cpu_idle_time_jiffy(unsigned int cpu, u64 *wall)
 
 	cur_wall_time = jiffies64_to_cputime64(get_jiffies_64());
 
-	busy_time = kcpustat_cpu(cpu).cpustat[CPUTIME_USER];
-	busy_time += kcpustat_cpu(cpu).cpustat[CPUTIME_SYSTEM];
-	busy_time += kcpustat_cpu(cpu).cpustat[CPUTIME_IRQ];
-	busy_time += kcpustat_cpu(cpu).cpustat[CPUTIME_SOFTIRQ];
-	busy_time += kcpustat_cpu(cpu).cpustat[CPUTIME_STEAL];
-	busy_time += kcpustat_cpu(cpu).cpustat[CPUTIME_NICE];
+	busy_time = atomic64_read(&kcpustat_cpu(cpu).cpustat[CPUTIME_USER]);
+	busy_time += atomic64_read(&kcpustat_cpu(cpu).cpustat[CPUTIME_SYSTEM]);
+	busy_time += atomic64_read(&kcpustat_cpu(cpu).cpustat[CPUTIME_IRQ]);
+	busy_time += atomic64_read(&kcpustat_cpu(cpu).cpustat[CPUTIME_SOFTIRQ]);
+	busy_time += atomic64_read(&kcpustat_cpu(cpu).cpustat[CPUTIME_STEAL]);
+	busy_time += atomic64_read(&kcpustat_cpu(cpu).cpustat[CPUTIME_NICE]);
 
 	idle_time = cur_wall_time - busy_time;
 	if (wall)
@@ -103,7 +103,7 @@ void dbs_check_cpu(struct dbs_data *dbs_data, int cpu)
 			u64 cur_nice;
 			unsigned long cur_nice_jiffies;
 
-			cur_nice = kcpustat_cpu(j).cpustat[CPUTIME_NICE] -
+			cur_nice = atomic64_read(&kcpustat_cpu(j).cpustat[CPUTIME_NICE]) -
 					 cdbs->prev_cpu_nice;
 			/*
 			 * Assumption: nice time between sampling periods will
@@ -113,7 +113,7 @@ void dbs_check_cpu(struct dbs_data *dbs_data, int cpu)
 					cputime64_to_jiffies64(cur_nice);
 
 			cdbs->prev_cpu_nice =
-				kcpustat_cpu(j).cpustat[CPUTIME_NICE];
+				atomic64_read(&kcpustat_cpu(j).cpustat[CPUTIME_NICE]);
 			idle_time += jiffies_to_usecs(cur_nice_jiffies);
 		}
 
@@ -216,7 +216,7 @@ int cpufreq_governor_dbs(struct dbs_data *dbs_data,
 					&j_cdbs->prev_cpu_wall);
 			if (ignore_nice)
 				j_cdbs->prev_cpu_nice =
-					kcpustat_cpu(j).cpustat[CPUTIME_NICE];
+					atomic64_read(&kcpustat_cpu(j).cpustat[CPUTIME_NICE]);
 		}
 
 		/*
diff --git a/drivers/cpufreq/cpufreq_ondemand.c b/drivers/cpufreq/cpufreq_ondemand.c
index 7731f7c..d761c9f 100644
--- a/drivers/cpufreq/cpufreq_ondemand.c
+++ b/drivers/cpufreq/cpufreq_ondemand.c
@@ -403,7 +403,7 @@ static ssize_t store_ignore_nice_load(struct kobject *a, struct attribute *b,
 						&dbs_info->cdbs.prev_cpu_wall);
 		if (od_tuners.ignore_nice)
 			dbs_info->cdbs.prev_cpu_nice =
-				kcpustat_cpu(j).cpustat[CPUTIME_NICE];
+				atomic64_read(&kcpustat_cpu(j).cpustat[CPUTIME_NICE]);
 
 	}
 	return count;
diff --git a/drivers/macintosh/rack-meter.c b/drivers/macintosh/rack-meter.c
index cad0e19..597fe20 100644
--- a/drivers/macintosh/rack-meter.c
+++ b/drivers/macintosh/rack-meter.c
@@ -83,11 +83,11 @@ static inline cputime64_t get_cpu_idle_time(unsigned int cpu)
 {
 	u64 retval;
 
-	retval = kcpustat_cpu(cpu).cpustat[CPUTIME_IDLE] +
-		 kcpustat_cpu(cpu).cpustat[CPUTIME_IOWAIT];
+	retval = atomic64_read(&kcpustat_cpu(cpu).cpustat[CPUTIME_IDLE]) +
+ 		 atomic64_read(&kcpustat_cpu(cpu).cpustat[CPUTIME_IOWAIT]);
 
 	if (rackmeter_ignore_nice)
-		retval += kcpustat_cpu(cpu).cpustat[CPUTIME_NICE];
+		retval += atomic64_read(&kcpustat_cpu(cpu).cpustat[CPUTIME_NICE]);
 
 	return retval;
 }
diff --git a/fs/proc/stat.c b/fs/proc/stat.c
index e296572..93f7f30 100644
--- a/fs/proc/stat.c
+++ b/fs/proc/stat.c
@@ -25,7 +25,7 @@ static cputime64_t get_idle_time(int cpu)
 {
 	cputime64_t idle;
 
-	idle = kcpustat_cpu(cpu).cpustat[CPUTIME_IDLE];
+	idle = atomic64_read(&kcpustat_cpu(cpu).cpustat[CPUTIME_IDLE]);
 	if (cpu_online(cpu) && !nr_iowait_cpu(cpu))
 		idle += arch_idle_time(cpu);
 	return idle;
@@ -35,7 +35,7 @@ static cputime64_t get_iowait_time(int cpu)
 {
 	cputime64_t iowait;
 
-	iowait = kcpustat_cpu(cpu).cpustat[CPUTIME_IOWAIT];
+	iowait = atomic64_read(&kcpustat_cpu(cpu).cpustat[CPUTIME_IOWAIT]);
 	if (cpu_online(cpu) && nr_iowait_cpu(cpu))
 		iowait += arch_idle_time(cpu);
 	return iowait;
@@ -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/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 ccff275..4c639ee 100644
--- a/kernel/sched/cputime.c
+++ b/kernel/sched/cputime.c
@@ -78,14 +78,14 @@ EXPORT_SYMBOL_GPL(irqtime_account_irq);
 
 static int irqtime_account_hi_update(void)
 {
-	u64 *cpustat = kcpustat_this_cpu->cpustat;
+	atomic64_t *cpustat = kcpustat_this_cpu->cpustat;
 	unsigned long flags;
 	u64 latest_ns;
 	int ret = 0;
 
 	local_irq_save(flags);
 	latest_ns = this_cpu_read(cpu_hardirq_time);
-	if (nsecs_to_cputime64(latest_ns) > cpustat[CPUTIME_IRQ])
+	if (nsecs_to_cputime64(latest_ns) > atomic64_read(&cpustat[CPUTIME_IRQ]));
 		ret = 1;
 	local_irq_restore(flags);
 	return ret;
@@ -93,14 +93,14 @@ static int irqtime_account_hi_update(void)
 
 static int irqtime_account_si_update(void)
 {
-	u64 *cpustat = kcpustat_this_cpu->cpustat;
+	atomic64_t *cpustat = kcpustat_this_cpu->cpustat;
 	unsigned long flags;
 	u64 latest_ns;
 	int ret = 0;
 
 	local_irq_save(flags);
 	latest_ns = this_cpu_read(cpu_softirq_time);
-	if (nsecs_to_cputime64(latest_ns) > cpustat[CPUTIME_SOFTIRQ])
+	if (nsecs_to_cputime64(latest_ns) > atomic64_read(&cpustat[CPUTIME_SOFTIRQ]))
 		ret = 1;
 	local_irq_restore(flags);
 	return ret;
@@ -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))
@@ -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();
@@ -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)
@@ -345,15 +345,15 @@ static void irqtime_account_process_tick(struct task_struct *p, int user_tick,
 						struct rq *rq)
 {
 	cputime_t one_jiffy_scaled = cputime_to_scaled(cputime_one_jiffy);
-	u64 *cpustat = kcpustat_this_cpu->cpustat;
+	atomic64_t *cpustat = kcpustat_this_cpu->cpustat;
 
 	if (steal_account_process_tick())
 		return;
 
 	if (irqtime_account_hi_update()) {
-		cpustat[CPUTIME_IRQ] += (__force u64) cputime_one_jiffy;
+		atomic64_add((__force u64) cputime_one_jiffy, &cpustat[CPUTIME_IRQ]);
 	} else if (irqtime_account_si_update()) {
-		cpustat[CPUTIME_SOFTIRQ] += (__force u64) cputime_one_jiffy;
+		atomic64_add((__force u64) cputime_one_jiffy, &cpustat[CPUTIME_SOFTIRQ]);
 	} else if (this_cpu_ksoftirqd() == p) {
 		/*
 		 * ksoftirqd time do not get accounted in cpu_softirq_time.
-- 
1.8.1.2

^ permalink raw reply related	[flat|nested] 42+ messages in thread

* Re: [RFC/PATCH 2/5] kernel_cpustat: convert to atomic 64-bit accessors
  2013-02-21 19:38     ` Kevin Hilman
@ 2013-02-21 21:53       ` Frederic Weisbecker
  -1 siblings, 0 replies; 42+ messages in thread
From: Frederic Weisbecker @ 2013-02-21 21:53 UTC (permalink / raw)
  To: Kevin Hilman; +Cc: Mats Liljegren, linux-kernel, linux-arm-kernel, linaro-dev

2013/2/21 Kevin Hilman <khilman@linaro.org>:
> Subject: [PATCH 2/5] kernel_cpustat: convert to atomic 64-bit accessors
>
> Use the atomic64_* accessors for all the kernel_cpustat fields to
> ensure atomic access on non-64 bit platforms.
>
> Thanks to Mats Liljegren for CGROUP_CPUACCT related fixes.
>
> Cc: Mats Liljegren <mats.liljegren@enea.com>
> Signed-off-by: Kevin Hilman <khilman@linaro.org>

Funny stuff, I thought struct kernel_cpustat was made of cputime_t
field. Actually it's u64. So the issue is independant from the new
full dynticks cputime accounting. It was already broken before.

But yeah that's not the point, we still want to fix this anyway. But
let's just treat this patch as independant.

> ---
>  arch/s390/appldata/appldata_os.c   | 41 +++++++++++++++++++++++---------------
>  drivers/cpufreq/cpufreq_governor.c | 18 ++++++++---------
>  drivers/cpufreq/cpufreq_ondemand.c |  2 +-
>  drivers/macintosh/rack-meter.c     |  6 +++---
>  fs/proc/stat.c                     | 40 ++++++++++++++++++-------------------
>  fs/proc/uptime.c                   |  2 +-
>  include/linux/kernel_stat.h        |  2 +-
>  kernel/sched/core.c                | 10 +++++-----
>  kernel/sched/cputime.c             | 38 +++++++++++++++++------------------
>  9 files changed, 84 insertions(+), 75 deletions(-)
>
> diff --git a/arch/s390/appldata/appldata_os.c b/arch/s390/appldata/appldata_os.c
> index 87521ba..008b180 100644
> --- a/arch/s390/appldata/appldata_os.c
> +++ b/arch/s390/appldata/appldata_os.c
> @@ -99,6 +99,7 @@ static void appldata_get_os_data(void *data)
>         int i, j, rc;
>         struct appldata_os_data *os_data;
>         unsigned int new_size;
> +       u64 val;
>
>         os_data = data;
>         os_data->sync_count_1++;
> @@ -112,22 +113,30 @@ static void appldata_get_os_data(void *data)
>
>         j = 0;
>         for_each_online_cpu(i) {
> -               os_data->os_cpu[j].per_cpu_user =
> -                       cputime_to_jiffies(kcpustat_cpu(i).cpustat[CPUTIME_USER]);
> -               os_data->os_cpu[j].per_cpu_nice =
> -                       cputime_to_jiffies(kcpustat_cpu(i).cpustat[CPUTIME_NICE]);
> -               os_data->os_cpu[j].per_cpu_system =
> -                       cputime_to_jiffies(kcpustat_cpu(i).cpustat[CPUTIME_SYSTEM]);
> -               os_data->os_cpu[j].per_cpu_idle =
> -                       cputime_to_jiffies(kcpustat_cpu(i).cpustat[CPUTIME_IDLE]);
> -               os_data->os_cpu[j].per_cpu_irq =
> -                       cputime_to_jiffies(kcpustat_cpu(i).cpustat[CPUTIME_IRQ]);
> -               os_data->os_cpu[j].per_cpu_softirq =
> -                       cputime_to_jiffies(kcpustat_cpu(i).cpustat[CPUTIME_SOFTIRQ]);
> -               os_data->os_cpu[j].per_cpu_iowait =
> -                       cputime_to_jiffies(kcpustat_cpu(i).cpustat[CPUTIME_IOWAIT]);
> -               os_data->os_cpu[j].per_cpu_steal =
> -                       cputime_to_jiffies(kcpustat_cpu(i).cpustat[CPUTIME_STEAL]);
> +               val = atomic64_read(&kcpustat_cpu(i).cpustat[CPUTIME_USER]);

So I see this repeated pattern everywhere. How about converting that to:
           kcpustat_cpu_get(i, CPUTIME_USER)

and use that accessor in all other places. That's much more readable
and then we can later modify the accessing code in one go.

We should perhaps even use atomic_64 in 32 bits and u64 in 64 bits.

[...]
> @@ -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]);

That too should be kcpustat_this_cpu_set(), or kcpustat_this_cpu_add()
FWIW. But we probably don't need the overhead of atomic_add() that
does a LOCK.
atomic_set(var, atomic_read(var) + delta) would be better. All we need
is that low/high parts of the 64 bits values are stored and read
without messing up altogether.

Thanks.

^ permalink raw reply	[flat|nested] 42+ messages in thread

* [RFC/PATCH 2/5] kernel_cpustat: convert to atomic 64-bit accessors
@ 2013-02-21 21:53       ` Frederic Weisbecker
  0 siblings, 0 replies; 42+ messages in thread
From: Frederic Weisbecker @ 2013-02-21 21:53 UTC (permalink / raw)
  To: linux-arm-kernel

2013/2/21 Kevin Hilman <khilman@linaro.org>:
> Subject: [PATCH 2/5] kernel_cpustat: convert to atomic 64-bit accessors
>
> Use the atomic64_* accessors for all the kernel_cpustat fields to
> ensure atomic access on non-64 bit platforms.
>
> Thanks to Mats Liljegren for CGROUP_CPUACCT related fixes.
>
> Cc: Mats Liljegren <mats.liljegren@enea.com>
> Signed-off-by: Kevin Hilman <khilman@linaro.org>

Funny stuff, I thought struct kernel_cpustat was made of cputime_t
field. Actually it's u64. So the issue is independant from the new
full dynticks cputime accounting. It was already broken before.

But yeah that's not the point, we still want to fix this anyway. But
let's just treat this patch as independant.

> ---
>  arch/s390/appldata/appldata_os.c   | 41 +++++++++++++++++++++++---------------
>  drivers/cpufreq/cpufreq_governor.c | 18 ++++++++---------
>  drivers/cpufreq/cpufreq_ondemand.c |  2 +-
>  drivers/macintosh/rack-meter.c     |  6 +++---
>  fs/proc/stat.c                     | 40 ++++++++++++++++++-------------------
>  fs/proc/uptime.c                   |  2 +-
>  include/linux/kernel_stat.h        |  2 +-
>  kernel/sched/core.c                | 10 +++++-----
>  kernel/sched/cputime.c             | 38 +++++++++++++++++------------------
>  9 files changed, 84 insertions(+), 75 deletions(-)
>
> diff --git a/arch/s390/appldata/appldata_os.c b/arch/s390/appldata/appldata_os.c
> index 87521ba..008b180 100644
> --- a/arch/s390/appldata/appldata_os.c
> +++ b/arch/s390/appldata/appldata_os.c
> @@ -99,6 +99,7 @@ static void appldata_get_os_data(void *data)
>         int i, j, rc;
>         struct appldata_os_data *os_data;
>         unsigned int new_size;
> +       u64 val;
>
>         os_data = data;
>         os_data->sync_count_1++;
> @@ -112,22 +113,30 @@ static void appldata_get_os_data(void *data)
>
>         j = 0;
>         for_each_online_cpu(i) {
> -               os_data->os_cpu[j].per_cpu_user =
> -                       cputime_to_jiffies(kcpustat_cpu(i).cpustat[CPUTIME_USER]);
> -               os_data->os_cpu[j].per_cpu_nice =
> -                       cputime_to_jiffies(kcpustat_cpu(i).cpustat[CPUTIME_NICE]);
> -               os_data->os_cpu[j].per_cpu_system =
> -                       cputime_to_jiffies(kcpustat_cpu(i).cpustat[CPUTIME_SYSTEM]);
> -               os_data->os_cpu[j].per_cpu_idle =
> -                       cputime_to_jiffies(kcpustat_cpu(i).cpustat[CPUTIME_IDLE]);
> -               os_data->os_cpu[j].per_cpu_irq =
> -                       cputime_to_jiffies(kcpustat_cpu(i).cpustat[CPUTIME_IRQ]);
> -               os_data->os_cpu[j].per_cpu_softirq =
> -                       cputime_to_jiffies(kcpustat_cpu(i).cpustat[CPUTIME_SOFTIRQ]);
> -               os_data->os_cpu[j].per_cpu_iowait =
> -                       cputime_to_jiffies(kcpustat_cpu(i).cpustat[CPUTIME_IOWAIT]);
> -               os_data->os_cpu[j].per_cpu_steal =
> -                       cputime_to_jiffies(kcpustat_cpu(i).cpustat[CPUTIME_STEAL]);
> +               val = atomic64_read(&kcpustat_cpu(i).cpustat[CPUTIME_USER]);

So I see this repeated pattern everywhere. How about converting that to:
           kcpustat_cpu_get(i, CPUTIME_USER)

and use that accessor in all other places. That's much more readable
and then we can later modify the accessing code in one go.

We should perhaps even use atomic_64 in 32 bits and u64 in 64 bits.

[...]
> @@ -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]);

That too should be kcpustat_this_cpu_set(), or kcpustat_this_cpu_add()
FWIW. But we probably don't need the overhead of atomic_add() that
does a LOCK.
atomic_set(var, atomic_read(var) + delta) would be better. All we need
is that low/high parts of the 64 bits values are stored and read
without messing up altogether.

Thanks.

^ permalink raw reply	[flat|nested] 42+ messages in thread

* Re: [RFC/PATCH 2/5] kernel_cpustat: convert to atomic 64-bit accessors
  2013-02-21 21:53       ` Frederic Weisbecker
@ 2013-02-21 21:54         ` Frederic Weisbecker
  -1 siblings, 0 replies; 42+ messages in thread
From: Frederic Weisbecker @ 2013-02-21 21:54 UTC (permalink / raw)
  To: Kevin Hilman
  Cc: Mats Liljegren, linux-kernel, linux-arm-kernel, linaro-dev,
	Peter Zijlstra, Steven Rostedt, Ingo Molnar, Thomas Gleixner,
	Andrew Morton

2013/2/21 Frederic Weisbecker <fweisbec@gmail.com>:
> 2013/2/21 Kevin Hilman <khilman@linaro.org>:
>> Subject: [PATCH 2/5] kernel_cpustat: convert to atomic 64-bit accessors
>>
>> Use the atomic64_* accessors for all the kernel_cpustat fields to
>> ensure atomic access on non-64 bit platforms.
>>
>> Thanks to Mats Liljegren for CGROUP_CPUACCT related fixes.
>>
>> Cc: Mats Liljegren <mats.liljegren@enea.com>
>> Signed-off-by: Kevin Hilman <khilman@linaro.org>
>
> Funny stuff, I thought struct kernel_cpustat was made of cputime_t
> field. Actually it's u64. So the issue is independant from the new
> full dynticks cputime accounting. It was already broken before.
>
> But yeah that's not the point, we still want to fix this anyway. But
> let's just treat this patch as independant.
>
>> ---
>>  arch/s390/appldata/appldata_os.c   | 41 +++++++++++++++++++++++---------------
>>  drivers/cpufreq/cpufreq_governor.c | 18 ++++++++---------
>>  drivers/cpufreq/cpufreq_ondemand.c |  2 +-
>>  drivers/macintosh/rack-meter.c     |  6 +++---
>>  fs/proc/stat.c                     | 40 ++++++++++++++++++-------------------
>>  fs/proc/uptime.c                   |  2 +-
>>  include/linux/kernel_stat.h        |  2 +-
>>  kernel/sched/core.c                | 10 +++++-----
>>  kernel/sched/cputime.c             | 38 +++++++++++++++++------------------
>>  9 files changed, 84 insertions(+), 75 deletions(-)
>>
>> diff --git a/arch/s390/appldata/appldata_os.c b/arch/s390/appldata/appldata_os.c
>> index 87521ba..008b180 100644
>> --- a/arch/s390/appldata/appldata_os.c
>> +++ b/arch/s390/appldata/appldata_os.c
>> @@ -99,6 +99,7 @@ static void appldata_get_os_data(void *data)
>>         int i, j, rc;
>>         struct appldata_os_data *os_data;
>>         unsigned int new_size;
>> +       u64 val;
>>
>>         os_data = data;
>>         os_data->sync_count_1++;
>> @@ -112,22 +113,30 @@ static void appldata_get_os_data(void *data)
>>
>>         j = 0;
>>         for_each_online_cpu(i) {
>> -               os_data->os_cpu[j].per_cpu_user =
>> -                       cputime_to_jiffies(kcpustat_cpu(i).cpustat[CPUTIME_USER]);
>> -               os_data->os_cpu[j].per_cpu_nice =
>> -                       cputime_to_jiffies(kcpustat_cpu(i).cpustat[CPUTIME_NICE]);
>> -               os_data->os_cpu[j].per_cpu_system =
>> -                       cputime_to_jiffies(kcpustat_cpu(i).cpustat[CPUTIME_SYSTEM]);
>> -               os_data->os_cpu[j].per_cpu_idle =
>> -                       cputime_to_jiffies(kcpustat_cpu(i).cpustat[CPUTIME_IDLE]);
>> -               os_data->os_cpu[j].per_cpu_irq =
>> -                       cputime_to_jiffies(kcpustat_cpu(i).cpustat[CPUTIME_IRQ]);
>> -               os_data->os_cpu[j].per_cpu_softirq =
>> -                       cputime_to_jiffies(kcpustat_cpu(i).cpustat[CPUTIME_SOFTIRQ]);
>> -               os_data->os_cpu[j].per_cpu_iowait =
>> -                       cputime_to_jiffies(kcpustat_cpu(i).cpustat[CPUTIME_IOWAIT]);
>> -               os_data->os_cpu[j].per_cpu_steal =
>> -                       cputime_to_jiffies(kcpustat_cpu(i).cpustat[CPUTIME_STEAL]);
>> +               val = atomic64_read(&kcpustat_cpu(i).cpustat[CPUTIME_USER]);
>
> So I see this repeated pattern everywhere. How about converting that to:
>            kcpustat_cpu_get(i, CPUTIME_USER)
>
> and use that accessor in all other places. That's much more readable
> and then we can later modify the accessing code in one go.
>
> We should perhaps even use atomic_64 in 32 bits and u64 in 64 bits.
>
> [...]
>> @@ -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]);
>
> That too should be kcpustat_this_cpu_set(), or kcpustat_this_cpu_add()
> FWIW. But we probably don't need the overhead of atomic_add() that
> does a LOCK.
> atomic_set(var, atomic_read(var) + delta) would be better. All we need
> is that low/high parts of the 64 bits values are stored and read
> without messing up altogether.
>
> Thanks.

Adding some more people in Cc.

^ permalink raw reply	[flat|nested] 42+ messages in thread

* [RFC/PATCH 2/5] kernel_cpustat: convert to atomic 64-bit accessors
@ 2013-02-21 21:54         ` Frederic Weisbecker
  0 siblings, 0 replies; 42+ messages in thread
From: Frederic Weisbecker @ 2013-02-21 21:54 UTC (permalink / raw)
  To: linux-arm-kernel

2013/2/21 Frederic Weisbecker <fweisbec@gmail.com>:
> 2013/2/21 Kevin Hilman <khilman@linaro.org>:
>> Subject: [PATCH 2/5] kernel_cpustat: convert to atomic 64-bit accessors
>>
>> Use the atomic64_* accessors for all the kernel_cpustat fields to
>> ensure atomic access on non-64 bit platforms.
>>
>> Thanks to Mats Liljegren for CGROUP_CPUACCT related fixes.
>>
>> Cc: Mats Liljegren <mats.liljegren@enea.com>
>> Signed-off-by: Kevin Hilman <khilman@linaro.org>
>
> Funny stuff, I thought struct kernel_cpustat was made of cputime_t
> field. Actually it's u64. So the issue is independant from the new
> full dynticks cputime accounting. It was already broken before.
>
> But yeah that's not the point, we still want to fix this anyway. But
> let's just treat this patch as independant.
>
>> ---
>>  arch/s390/appldata/appldata_os.c   | 41 +++++++++++++++++++++++---------------
>>  drivers/cpufreq/cpufreq_governor.c | 18 ++++++++---------
>>  drivers/cpufreq/cpufreq_ondemand.c |  2 +-
>>  drivers/macintosh/rack-meter.c     |  6 +++---
>>  fs/proc/stat.c                     | 40 ++++++++++++++++++-------------------
>>  fs/proc/uptime.c                   |  2 +-
>>  include/linux/kernel_stat.h        |  2 +-
>>  kernel/sched/core.c                | 10 +++++-----
>>  kernel/sched/cputime.c             | 38 +++++++++++++++++------------------
>>  9 files changed, 84 insertions(+), 75 deletions(-)
>>
>> diff --git a/arch/s390/appldata/appldata_os.c b/arch/s390/appldata/appldata_os.c
>> index 87521ba..008b180 100644
>> --- a/arch/s390/appldata/appldata_os.c
>> +++ b/arch/s390/appldata/appldata_os.c
>> @@ -99,6 +99,7 @@ static void appldata_get_os_data(void *data)
>>         int i, j, rc;
>>         struct appldata_os_data *os_data;
>>         unsigned int new_size;
>> +       u64 val;
>>
>>         os_data = data;
>>         os_data->sync_count_1++;
>> @@ -112,22 +113,30 @@ static void appldata_get_os_data(void *data)
>>
>>         j = 0;
>>         for_each_online_cpu(i) {
>> -               os_data->os_cpu[j].per_cpu_user =
>> -                       cputime_to_jiffies(kcpustat_cpu(i).cpustat[CPUTIME_USER]);
>> -               os_data->os_cpu[j].per_cpu_nice =
>> -                       cputime_to_jiffies(kcpustat_cpu(i).cpustat[CPUTIME_NICE]);
>> -               os_data->os_cpu[j].per_cpu_system =
>> -                       cputime_to_jiffies(kcpustat_cpu(i).cpustat[CPUTIME_SYSTEM]);
>> -               os_data->os_cpu[j].per_cpu_idle =
>> -                       cputime_to_jiffies(kcpustat_cpu(i).cpustat[CPUTIME_IDLE]);
>> -               os_data->os_cpu[j].per_cpu_irq =
>> -                       cputime_to_jiffies(kcpustat_cpu(i).cpustat[CPUTIME_IRQ]);
>> -               os_data->os_cpu[j].per_cpu_softirq =
>> -                       cputime_to_jiffies(kcpustat_cpu(i).cpustat[CPUTIME_SOFTIRQ]);
>> -               os_data->os_cpu[j].per_cpu_iowait =
>> -                       cputime_to_jiffies(kcpustat_cpu(i).cpustat[CPUTIME_IOWAIT]);
>> -               os_data->os_cpu[j].per_cpu_steal =
>> -                       cputime_to_jiffies(kcpustat_cpu(i).cpustat[CPUTIME_STEAL]);
>> +               val = atomic64_read(&kcpustat_cpu(i).cpustat[CPUTIME_USER]);
>
> So I see this repeated pattern everywhere. How about converting that to:
>            kcpustat_cpu_get(i, CPUTIME_USER)
>
> and use that accessor in all other places. That's much more readable
> and then we can later modify the accessing code in one go.
>
> We should perhaps even use atomic_64 in 32 bits and u64 in 64 bits.
>
> [...]
>> @@ -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]);
>
> That too should be kcpustat_this_cpu_set(), or kcpustat_this_cpu_add()
> FWIW. But we probably don't need the overhead of atomic_add() that
> does a LOCK.
> atomic_set(var, atomic_read(var) + delta) would be better. All we need
> is that low/high parts of the 64 bits values are stored and read
> without messing up altogether.
>
> Thanks.

Adding some more people in Cc.

^ permalink raw reply	[flat|nested] 42+ messages in thread

* Re: [RFC/PATCH 2/5] kernel_cpustat: convert to atomic 64-bit accessors
  2013-02-21 21:53       ` Frederic Weisbecker
@ 2013-02-21 21:58         ` Russell King - ARM Linux
  -1 siblings, 0 replies; 42+ messages in thread
From: Russell King - ARM Linux @ 2013-02-21 21:58 UTC (permalink / raw)
  To: Frederic Weisbecker
  Cc: Kevin Hilman, Mats Liljegren, linaro-dev, linux-kernel, linux-arm-kernel

On Thu, Feb 21, 2013 at 10:53:07PM +0100, Frederic Weisbecker wrote:
> That too should be kcpustat_this_cpu_set(), or kcpustat_this_cpu_add()
> FWIW. But we probably don't need the overhead of atomic_add() that
> does a LOCK.
> atomic_set(var, atomic_read(var) + delta) would be better. All we need

You mean atomic64_set() and atomic64_read().  Looking at the generic
version in lib/atomic64.c, atomic64_add() is cheaper for 32-bit arches
because it doesn't involve taking the lock twice.

^ permalink raw reply	[flat|nested] 42+ messages in thread

* [RFC/PATCH 2/5] kernel_cpustat: convert to atomic 64-bit accessors
@ 2013-02-21 21:58         ` Russell King - ARM Linux
  0 siblings, 0 replies; 42+ messages in thread
From: Russell King - ARM Linux @ 2013-02-21 21:58 UTC (permalink / raw)
  To: linux-arm-kernel

On Thu, Feb 21, 2013 at 10:53:07PM +0100, Frederic Weisbecker wrote:
> That too should be kcpustat_this_cpu_set(), or kcpustat_this_cpu_add()
> FWIW. But we probably don't need the overhead of atomic_add() that
> does a LOCK.
> atomic_set(var, atomic_read(var) + delta) would be better. All we need

You mean atomic64_set() and atomic64_read().  Looking at the generic
version in lib/atomic64.c, atomic64_add() is cheaper for 32-bit arches
because it doesn't involve taking the lock twice.

^ permalink raw reply	[flat|nested] 42+ messages in thread

* Re: [RFC/PATCH 2/5] kernel_cpustat: convert to atomic 64-bit accessors
  2013-02-21 21:58         ` Russell King - ARM Linux
@ 2013-02-21 22:15           ` Frederic Weisbecker
  -1 siblings, 0 replies; 42+ messages in thread
From: Frederic Weisbecker @ 2013-02-21 22:15 UTC (permalink / raw)
  To: Russell King - ARM Linux
  Cc: Kevin Hilman, Mats Liljegren, linaro-dev, linux-kernel, linux-arm-kernel

2013/2/21 Russell King - ARM Linux <linux@arm.linux.org.uk>:
> On Thu, Feb 21, 2013 at 10:53:07PM +0100, Frederic Weisbecker wrote:
>> That too should be kcpustat_this_cpu_set(), or kcpustat_this_cpu_add()
>> FWIW. But we probably don't need the overhead of atomic_add() that
>> does a LOCK.
>> atomic_set(var, atomic_read(var) + delta) would be better. All we need
>
> You mean atomic64_set() and atomic64_read().

Right.

> Looking at the generic
> version in lib/atomic64.c, atomic64_add() is cheaper for 32-bit arches
> because it doesn't involve taking the lock twice.

Yeah but that's the generic version. I guess the archs that have
overriden don't take any locks there.

^ permalink raw reply	[flat|nested] 42+ messages in thread

* [RFC/PATCH 2/5] kernel_cpustat: convert to atomic 64-bit accessors
@ 2013-02-21 22:15           ` Frederic Weisbecker
  0 siblings, 0 replies; 42+ messages in thread
From: Frederic Weisbecker @ 2013-02-21 22:15 UTC (permalink / raw)
  To: linux-arm-kernel

2013/2/21 Russell King - ARM Linux <linux@arm.linux.org.uk>:
> On Thu, Feb 21, 2013 at 10:53:07PM +0100, Frederic Weisbecker wrote:
>> That too should be kcpustat_this_cpu_set(), or kcpustat_this_cpu_add()
>> FWIW. But we probably don't need the overhead of atomic_add() that
>> does a LOCK.
>> atomic_set(var, atomic_read(var) + delta) would be better. All we need
>
> You mean atomic64_set() and atomic64_read().

Right.

> Looking at the generic
> version in lib/atomic64.c, atomic64_add() is cheaper for 32-bit arches
> because it doesn't involve taking the lock twice.

Yeah but that's the generic version. I guess the archs that have
overriden don't take any locks there.

^ permalink raw reply	[flat|nested] 42+ messages in thread

* Re: [RFC/PATCH 2/5] kernel_cpustat: convert to atomic 64-bit accessors
  2013-02-21 21:54         ` Frederic Weisbecker
@ 2013-02-22  5:57           ` Kevin Hilman
  -1 siblings, 0 replies; 42+ messages in thread
From: Kevin Hilman @ 2013-02-22  5:57 UTC (permalink / raw)
  To: Frederic Weisbecker
  Cc: Mats Liljegren, linux-kernel, linux-arm-kernel, linaro-dev,
	Peter Zijlstra, Steven Rostedt, Ingo Molnar, Thomas Gleixner,
	Andrew Morton

Frederic Weisbecker <fweisbec@gmail.com> writes:

> 2013/2/21 Frederic Weisbecker <fweisbec@gmail.com>:
>> 2013/2/21 Kevin Hilman <khilman@linaro.org>:
>>> Subject: [PATCH 2/5] kernel_cpustat: convert to atomic 64-bit accessors
>>>
>>> Use the atomic64_* accessors for all the kernel_cpustat fields to
>>> ensure atomic access on non-64 bit platforms.
>>>
>>> Thanks to Mats Liljegren for CGROUP_CPUACCT related fixes.
>>>
>>> Cc: Mats Liljegren <mats.liljegren@enea.com>
>>> Signed-off-by: Kevin Hilman <khilman@linaro.org>
>>
>> Funny stuff, I thought struct kernel_cpustat was made of cputime_t
>> field. Actually it's u64. So the issue is independant from the new
>> full dynticks cputime accounting. It was already broken before.
>>
>> But yeah that's not the point, we still want to fix this anyway. But
>> let's just treat this patch as independant.

OK, I just sent an updated series based on your proposal.

Thanks for the review,

Kevin

^ permalink raw reply	[flat|nested] 42+ messages in thread

* [RFC/PATCH 2/5] kernel_cpustat: convert to atomic 64-bit accessors
@ 2013-02-22  5:57           ` Kevin Hilman
  0 siblings, 0 replies; 42+ messages in thread
From: Kevin Hilman @ 2013-02-22  5:57 UTC (permalink / raw)
  To: linux-arm-kernel

Frederic Weisbecker <fweisbec@gmail.com> writes:

> 2013/2/21 Frederic Weisbecker <fweisbec@gmail.com>:
>> 2013/2/21 Kevin Hilman <khilman@linaro.org>:
>>> Subject: [PATCH 2/5] kernel_cpustat: convert to atomic 64-bit accessors
>>>
>>> Use the atomic64_* accessors for all the kernel_cpustat fields to
>>> ensure atomic access on non-64 bit platforms.
>>>
>>> Thanks to Mats Liljegren for CGROUP_CPUACCT related fixes.
>>>
>>> Cc: Mats Liljegren <mats.liljegren@enea.com>
>>> Signed-off-by: Kevin Hilman <khilman@linaro.org>
>>
>> Funny stuff, I thought struct kernel_cpustat was made of cputime_t
>> field. Actually it's u64. So the issue is independant from the new
>> full dynticks cputime accounting. It was already broken before.
>>
>> But yeah that's not the point, we still want to fix this anyway. But
>> let's just treat this patch as independant.

OK, I just sent an updated series based on your proposal.

Thanks for the review,

Kevin

^ permalink raw reply	[flat|nested] 42+ messages in thread

* Re: [RFC/PATCH 1/5] context tracking: conditionalize guest support based on CONFIG_KVM
  2013-02-20 19:41   ` Kevin Hilman
@ 2013-02-22 13:27     ` Frederic Weisbecker
  -1 siblings, 0 replies; 42+ messages in thread
From: Frederic Weisbecker @ 2013-02-22 13:27 UTC (permalink / raw)
  To: Kevin Hilman; +Cc: Mats Liljegren, linux-kernel, linux-arm-kernel, linaro-dev

On Wed, Feb 20, 2013 at 11:41:38AM -0800, Kevin Hilman wrote:
> So that it can build on !KVM systems too.
> 
> Signed-off-by: Kevin Hilman <khilman@linaro.org>
> ---
>  kernel/context_tracking.c | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/kernel/context_tracking.c b/kernel/context_tracking.c
> index 74f68f4..6fe96b1 100644
> --- a/kernel/context_tracking.c
> +++ b/kernel/context_tracking.c
> @@ -1,5 +1,7 @@
>  #include <linux/context_tracking.h>
> +#ifdef CONFIG_KVM
>  #include <linux/kvm_host.h>
> +#endif

The header should take care of the off-case, no need to ifdef its inclusion.

>  #include <linux/rcupdate.h>
>  #include <linux/sched.h>
>  #include <linux/hardirq.h>
> @@ -62,6 +64,7 @@ void user_exit(void)
>  	local_irq_restore(flags);
>  }
>  
> +#ifdef CONFIG_KVM
>  void guest_enter(void)
>  {
>  	if (vtime_accounting_enabled())
> @@ -79,6 +82,7 @@ void guest_exit(void)
>  		__guest_exit();
>  }
>  EXPORT_SYMBOL_GPL(guest_exit);
> +#endif

Kvm might be built as a module so we can't actually do this unfortunately.

Thanks.

>  
>  void context_tracking_task_switch(struct task_struct *prev,
>  			     struct task_struct *next)
> -- 
> 1.8.1.2
> 

^ permalink raw reply	[flat|nested] 42+ messages in thread

* [RFC/PATCH 1/5] context tracking: conditionalize guest support based on CONFIG_KVM
@ 2013-02-22 13:27     ` Frederic Weisbecker
  0 siblings, 0 replies; 42+ messages in thread
From: Frederic Weisbecker @ 2013-02-22 13:27 UTC (permalink / raw)
  To: linux-arm-kernel

On Wed, Feb 20, 2013 at 11:41:38AM -0800, Kevin Hilman wrote:
> So that it can build on !KVM systems too.
> 
> Signed-off-by: Kevin Hilman <khilman@linaro.org>
> ---
>  kernel/context_tracking.c | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/kernel/context_tracking.c b/kernel/context_tracking.c
> index 74f68f4..6fe96b1 100644
> --- a/kernel/context_tracking.c
> +++ b/kernel/context_tracking.c
> @@ -1,5 +1,7 @@
>  #include <linux/context_tracking.h>
> +#ifdef CONFIG_KVM
>  #include <linux/kvm_host.h>
> +#endif

The header should take care of the off-case, no need to ifdef its inclusion.

>  #include <linux/rcupdate.h>
>  #include <linux/sched.h>
>  #include <linux/hardirq.h>
> @@ -62,6 +64,7 @@ void user_exit(void)
>  	local_irq_restore(flags);
>  }
>  
> +#ifdef CONFIG_KVM
>  void guest_enter(void)
>  {
>  	if (vtime_accounting_enabled())
> @@ -79,6 +82,7 @@ void guest_exit(void)
>  		__guest_exit();
>  }
>  EXPORT_SYMBOL_GPL(guest_exit);
> +#endif

Kvm might be built as a module so we can't actually do this unfortunately.

Thanks.

>  
>  void context_tracking_task_switch(struct task_struct *prev,
>  			     struct task_struct *next)
> -- 
> 1.8.1.2
> 

^ permalink raw reply	[flat|nested] 42+ messages in thread

* Re: [RFC/PATCH 4/5] cputime: use do_div() for nsec resolution conversion helpers
  2013-02-21 19:21       ` Kevin Hilman
@ 2013-02-26 15:21         ` Frederic Weisbecker
  -1 siblings, 0 replies; 42+ messages in thread
From: Frederic Weisbecker @ 2013-02-26 15:21 UTC (permalink / raw)
  To: Kevin Hilman
  Cc: Namhyung Kim, Mats Liljegren, linaro-dev, linux-kernel, linux-arm-kernel

2013/2/21 Kevin Hilman <khilman@linaro.org>:
> From a8a0a8b8b12512a7f862ade459cd88d2b48e2bf3 Mon Sep 17 00:00:00 2001
> From: Kevin Hilman <khilman@linaro.org>
> Date: Thu, 14 Feb 2013 11:27:36 -0800
> Subject: [PATCH 4/5] cputime: use do_div() for nsec resolution conversion
>  helpers
>
> For the nsec resolution conversions to be useful on non 64-bit
> architectures, do_div() needs to be used for the 64-bit divisions.
>
> Special thanks to Namhyung Kim for pointing out omissions of the
> __force attribute in an earlier version.
>
> Cc: Namhyung Kim <namhyung@kernel.org>
> Signed-off-by: Kevin Hilman <khilman@linaro.org>
> ---
>  include/asm-generic/cputime_nsecs.h | 51 +++++++++++++++++++++++++++----------
>  1 file changed, 37 insertions(+), 14 deletions(-)
>
> diff --git a/include/asm-generic/cputime_nsecs.h b/include/asm-generic/cputime_nsecs.h
> index b6485ca..d4944c9 100644
> --- a/include/asm-generic/cputime_nsecs.h
> +++ b/include/asm-generic/cputime_nsecs.h
> @@ -24,13 +24,17 @@ typedef u64 __nocast cputime64_t;
>  /*
>   * Convert cputime <-> jiffies (HZ)
>   */
> -#define cputime_to_jiffies(__ct)       \
> -       ((__force u64)(__ct) / (NSEC_PER_SEC / HZ))
> +static inline u64 cputime_to_jiffies(const cputime_t ct)
> +{
> +       u64 __ct = (__force u64) ct;
> +
> +       do_div(__ct, NSEC_PER_SEC / HZ);

So, when you don't need the remainder, could you please rather use
div_u64() ? It defaults to use do_div() anyway if the arch doesn't
override it. And none does apparently yet. But at least it paves the
way for further optimizations. And you can directly return the result.

Also, how about:

#define cputime_div(cputime, divisor)  div_u64((__force u64)cputime, divisor)

Thanks.

^ permalink raw reply	[flat|nested] 42+ messages in thread

* [RFC/PATCH 4/5] cputime: use do_div() for nsec resolution conversion helpers
@ 2013-02-26 15:21         ` Frederic Weisbecker
  0 siblings, 0 replies; 42+ messages in thread
From: Frederic Weisbecker @ 2013-02-26 15:21 UTC (permalink / raw)
  To: linux-arm-kernel

2013/2/21 Kevin Hilman <khilman@linaro.org>:
> From a8a0a8b8b12512a7f862ade459cd88d2b48e2bf3 Mon Sep 17 00:00:00 2001
> From: Kevin Hilman <khilman@linaro.org>
> Date: Thu, 14 Feb 2013 11:27:36 -0800
> Subject: [PATCH 4/5] cputime: use do_div() for nsec resolution conversion
>  helpers
>
> For the nsec resolution conversions to be useful on non 64-bit
> architectures, do_div() needs to be used for the 64-bit divisions.
>
> Special thanks to Namhyung Kim for pointing out omissions of the
> __force attribute in an earlier version.
>
> Cc: Namhyung Kim <namhyung@kernel.org>
> Signed-off-by: Kevin Hilman <khilman@linaro.org>
> ---
>  include/asm-generic/cputime_nsecs.h | 51 +++++++++++++++++++++++++++----------
>  1 file changed, 37 insertions(+), 14 deletions(-)
>
> diff --git a/include/asm-generic/cputime_nsecs.h b/include/asm-generic/cputime_nsecs.h
> index b6485ca..d4944c9 100644
> --- a/include/asm-generic/cputime_nsecs.h
> +++ b/include/asm-generic/cputime_nsecs.h
> @@ -24,13 +24,17 @@ typedef u64 __nocast cputime64_t;
>  /*
>   * Convert cputime <-> jiffies (HZ)
>   */
> -#define cputime_to_jiffies(__ct)       \
> -       ((__force u64)(__ct) / (NSEC_PER_SEC / HZ))
> +static inline u64 cputime_to_jiffies(const cputime_t ct)
> +{
> +       u64 __ct = (__force u64) ct;
> +
> +       do_div(__ct, NSEC_PER_SEC / HZ);

So, when you don't need the remainder, could you please rather use
div_u64() ? It defaults to use do_div() anyway if the arch doesn't
override it. And none does apparently yet. But at least it paves the
way for further optimizations. And you can directly return the result.

Also, how about:

#define cputime_div(cputime, divisor)  div_u64((__force u64)cputime, divisor)

Thanks.

^ permalink raw reply	[flat|nested] 42+ messages in thread

* Re: [RFC/PATCH 1/5] context tracking: conditionalize guest support based on CONFIG_KVM
  2013-02-22 13:27     ` Frederic Weisbecker
@ 2013-02-26 19:29       ` Kevin Hilman
  -1 siblings, 0 replies; 42+ messages in thread
From: Kevin Hilman @ 2013-02-26 19:29 UTC (permalink / raw)
  To: Frederic Weisbecker
  Cc: Mats Liljegren, linux-kernel, linux-arm-kernel, linaro-dev

Frederic Weisbecker <fweisbec@gmail.com> writes:

> On Wed, Feb 20, 2013 at 11:41:38AM -0800, Kevin Hilman wrote:
>> So that it can build on !KVM systems too.
>> 
>> Signed-off-by: Kevin Hilman <khilman@linaro.org>
>> ---
>>  kernel/context_tracking.c | 4 ++++
>>  1 file changed, 4 insertions(+)
>> 
>> diff --git a/kernel/context_tracking.c b/kernel/context_tracking.c
>> index 74f68f4..6fe96b1 100644
>> --- a/kernel/context_tracking.c
>> +++ b/kernel/context_tracking.c
>> @@ -1,5 +1,7 @@
>>  #include <linux/context_tracking.h>
>> +#ifdef CONFIG_KVM
>>  #include <linux/kvm_host.h>
>> +#endif
>
> The header should take care of the off-case, no need to ifdef its inclusion.

In their current form, the headers will not build on platforms without
KVM support.  For example, these platforms (like ARM) don't even have
<asm/kvm.h> or <asm/kvm_host.h>.

>>  #include <linux/rcupdate.h>
>>  #include <linux/sched.h>
>>  #include <linux/hardirq.h>
>> @@ -62,6 +64,7 @@ void user_exit(void)
>>  	local_irq_restore(flags);
>>  }
>>  
>> +#ifdef CONFIG_KVM
>>  void guest_enter(void)
>>  {
>>  	if (vtime_accounting_enabled())
>> @@ -79,6 +82,7 @@ void guest_exit(void)
>>  		__guest_exit();
>>  }
>>  EXPORT_SYMBOL_GPL(guest_exit);
>> +#endif
>
> Kvm might be built as a module so we can't actually do this unfortunately.

Ah, right.  How bout using 

#if defined(CONFIG_KVM) || defined(CONFIG_KVM_MODULE)

for both conditionals above?  Updated patch below.

Kevin


>From 4444ce39eea21a5fc63b04a711addab2a5c63243 Mon Sep 17 00:00:00 2001
From: Kevin Hilman <khilman@linaro.org>
Date: Thu, 14 Feb 2013 10:17:37 -0800
Subject: [PATCH] context tracking: conditionalize guest support based on
 CONFIG_KVM

So that it can build on !KVM systems too.

Signed-off-by: Kevin Hilman <khilman@linaro.org>
---
 kernel/context_tracking.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/kernel/context_tracking.c b/kernel/context_tracking.c
index 74f68f4..d9c98dd 100644
--- a/kernel/context_tracking.c
+++ b/kernel/context_tracking.c
@@ -1,5 +1,7 @@
 #include <linux/context_tracking.h>
+#if defined(CONFIG_KVM) || defined(CONFIG_KVM_MODULE)
 #include <linux/kvm_host.h>
+#endif
 #include <linux/rcupdate.h>
 #include <linux/sched.h>
 #include <linux/hardirq.h>
@@ -62,6 +64,7 @@ void user_exit(void)
 	local_irq_restore(flags);
 }
 
+#if defined(CONFIG_KVM) || defined(CONFIG_KVM_MODULE)
 void guest_enter(void)
 {
 	if (vtime_accounting_enabled())
@@ -79,6 +82,7 @@ void guest_exit(void)
 		__guest_exit();
 }
 EXPORT_SYMBOL_GPL(guest_exit);
+#endif
 
 void context_tracking_task_switch(struct task_struct *prev,
 			     struct task_struct *next)
-- 
1.8.1.2


^ permalink raw reply related	[flat|nested] 42+ messages in thread

* [RFC/PATCH 1/5] context tracking: conditionalize guest support based on CONFIG_KVM
@ 2013-02-26 19:29       ` Kevin Hilman
  0 siblings, 0 replies; 42+ messages in thread
From: Kevin Hilman @ 2013-02-26 19:29 UTC (permalink / raw)
  To: linux-arm-kernel

Frederic Weisbecker <fweisbec@gmail.com> writes:

> On Wed, Feb 20, 2013 at 11:41:38AM -0800, Kevin Hilman wrote:
>> So that it can build on !KVM systems too.
>> 
>> Signed-off-by: Kevin Hilman <khilman@linaro.org>
>> ---
>>  kernel/context_tracking.c | 4 ++++
>>  1 file changed, 4 insertions(+)
>> 
>> diff --git a/kernel/context_tracking.c b/kernel/context_tracking.c
>> index 74f68f4..6fe96b1 100644
>> --- a/kernel/context_tracking.c
>> +++ b/kernel/context_tracking.c
>> @@ -1,5 +1,7 @@
>>  #include <linux/context_tracking.h>
>> +#ifdef CONFIG_KVM
>>  #include <linux/kvm_host.h>
>> +#endif
>
> The header should take care of the off-case, no need to ifdef its inclusion.

In their current form, the headers will not build on platforms without
KVM support.  For example, these platforms (like ARM) don't even have
<asm/kvm.h> or <asm/kvm_host.h>.

>>  #include <linux/rcupdate.h>
>>  #include <linux/sched.h>
>>  #include <linux/hardirq.h>
>> @@ -62,6 +64,7 @@ void user_exit(void)
>>  	local_irq_restore(flags);
>>  }
>>  
>> +#ifdef CONFIG_KVM
>>  void guest_enter(void)
>>  {
>>  	if (vtime_accounting_enabled())
>> @@ -79,6 +82,7 @@ void guest_exit(void)
>>  		__guest_exit();
>>  }
>>  EXPORT_SYMBOL_GPL(guest_exit);
>> +#endif
>
> Kvm might be built as a module so we can't actually do this unfortunately.

Ah, right.  How bout using 

#if defined(CONFIG_KVM) || defined(CONFIG_KVM_MODULE)

for both conditionals above?  Updated patch below.

Kevin


>From 4444ce39eea21a5fc63b04a711addab2a5c63243 Mon Sep 17 00:00:00 2001
From: Kevin Hilman <khilman@linaro.org>
Date: Thu, 14 Feb 2013 10:17:37 -0800
Subject: [PATCH] context tracking: conditionalize guest support based on
 CONFIG_KVM

So that it can build on !KVM systems too.

Signed-off-by: Kevin Hilman <khilman@linaro.org>
---
 kernel/context_tracking.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/kernel/context_tracking.c b/kernel/context_tracking.c
index 74f68f4..d9c98dd 100644
--- a/kernel/context_tracking.c
+++ b/kernel/context_tracking.c
@@ -1,5 +1,7 @@
 #include <linux/context_tracking.h>
+#if defined(CONFIG_KVM) || defined(CONFIG_KVM_MODULE)
 #include <linux/kvm_host.h>
+#endif
 #include <linux/rcupdate.h>
 #include <linux/sched.h>
 #include <linux/hardirq.h>
@@ -62,6 +64,7 @@ void user_exit(void)
 	local_irq_restore(flags);
 }
 
+#if defined(CONFIG_KVM) || defined(CONFIG_KVM_MODULE)
 void guest_enter(void)
 {
 	if (vtime_accounting_enabled())
@@ -79,6 +82,7 @@ void guest_exit(void)
 		__guest_exit();
 }
 EXPORT_SYMBOL_GPL(guest_exit);
+#endif
 
 void context_tracking_task_switch(struct task_struct *prev,
 			     struct task_struct *next)
-- 
1.8.1.2

^ permalink raw reply related	[flat|nested] 42+ messages in thread

* Re: [RFC/PATCH 1/5] context tracking: conditionalize guest support based on CONFIG_KVM
  2013-02-26 19:29       ` Kevin Hilman
@ 2013-02-26 22:13         ` Namhyung Kim
  -1 siblings, 0 replies; 42+ messages in thread
From: Namhyung Kim @ 2013-02-26 22:13 UTC (permalink / raw)
  To: Kevin Hilman
  Cc: Frederic Weisbecker, Mats Liljegren, linaro-dev, linux-kernel,
	linux-arm-kernel

2013-02-26 (화), 11:29 -0800, Kevin Hilman:
> Frederic Weisbecker <fweisbec@gmail.com> writes:
> 
> > On Wed, Feb 20, 2013 at 11:41:38AM -0800, Kevin Hilman wrote:
> >> So that it can build on !KVM systems too.
> >> 
> >> Signed-off-by: Kevin Hilman <khilman@linaro.org>
> >> ---
> >>  kernel/context_tracking.c | 4 ++++
> >>  1 file changed, 4 insertions(+)
> >> 
> >> diff --git a/kernel/context_tracking.c b/kernel/context_tracking.c
> >> index 74f68f4..6fe96b1 100644
> >> --- a/kernel/context_tracking.c
> >> +++ b/kernel/context_tracking.c
> >> @@ -1,5 +1,7 @@
> >>  #include <linux/context_tracking.h>
> >> +#ifdef CONFIG_KVM
> >>  #include <linux/kvm_host.h>
> >> +#endif
> >
> > The header should take care of the off-case, no need to ifdef its inclusion.
> 
> In their current form, the headers will not build on platforms without
> KVM support.  For example, these platforms (like ARM) don't even have
> <asm/kvm.h> or <asm/kvm_host.h>.
> 
> >>  #include <linux/rcupdate.h>
> >>  #include <linux/sched.h>
> >>  #include <linux/hardirq.h>
> >> @@ -62,6 +64,7 @@ void user_exit(void)
> >>  	local_irq_restore(flags);
> >>  }
> >>  
> >> +#ifdef CONFIG_KVM
> >>  void guest_enter(void)
> >>  {
> >>  	if (vtime_accounting_enabled())
> >> @@ -79,6 +82,7 @@ void guest_exit(void)
> >>  		__guest_exit();
> >>  }
> >>  EXPORT_SYMBOL_GPL(guest_exit);
> >> +#endif
> >
> > Kvm might be built as a module so we can't actually do this unfortunately.
> 
> Ah, right.  How bout using 
> 
> #if defined(CONFIG_KVM) || defined(CONFIG_KVM_MODULE)
> 
> for both conditionals above?  Updated patch below.

This is what IS_ENABLED(CONFIG_KVM) is for.

Thanks,
Namhyung

> 
> Kevin
> 
> 
> From 4444ce39eea21a5fc63b04a711addab2a5c63243 Mon Sep 17 00:00:00 2001
> From: Kevin Hilman <khilman@linaro.org>
> Date: Thu, 14 Feb 2013 10:17:37 -0800
> Subject: [PATCH] context tracking: conditionalize guest support based on
>  CONFIG_KVM
> 
> So that it can build on !KVM systems too.
> 
> Signed-off-by: Kevin Hilman <khilman@linaro.org>
> ---
>  kernel/context_tracking.c | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/kernel/context_tracking.c b/kernel/context_tracking.c
> index 74f68f4..d9c98dd 100644
> --- a/kernel/context_tracking.c
> +++ b/kernel/context_tracking.c
> @@ -1,5 +1,7 @@
>  #include <linux/context_tracking.h>
> +#if defined(CONFIG_KVM) || defined(CONFIG_KVM_MODULE)
>  #include <linux/kvm_host.h>
> +#endif
>  #include <linux/rcupdate.h>
>  #include <linux/sched.h>
>  #include <linux/hardirq.h>
> @@ -62,6 +64,7 @@ void user_exit(void)
>  	local_irq_restore(flags);
>  }
>  
> +#if defined(CONFIG_KVM) || defined(CONFIG_KVM_MODULE)
>  void guest_enter(void)
>  {
>  	if (vtime_accounting_enabled())
> @@ -79,6 +82,7 @@ void guest_exit(void)
>  		__guest_exit();
>  }
>  EXPORT_SYMBOL_GPL(guest_exit);
> +#endif
>  
>  void context_tracking_task_switch(struct task_struct *prev,
>  			     struct task_struct *next)




^ permalink raw reply	[flat|nested] 42+ messages in thread

* [RFC/PATCH 1/5] context tracking: conditionalize guest support based on CONFIG_KVM
@ 2013-02-26 22:13         ` Namhyung Kim
  0 siblings, 0 replies; 42+ messages in thread
From: Namhyung Kim @ 2013-02-26 22:13 UTC (permalink / raw)
  To: linux-arm-kernel

2013-02-26 (?), 11:29 -0800, Kevin Hilman:
> Frederic Weisbecker <fweisbec@gmail.com> writes:
> 
> > On Wed, Feb 20, 2013 at 11:41:38AM -0800, Kevin Hilman wrote:
> >> So that it can build on !KVM systems too.
> >> 
> >> Signed-off-by: Kevin Hilman <khilman@linaro.org>
> >> ---
> >>  kernel/context_tracking.c | 4 ++++
> >>  1 file changed, 4 insertions(+)
> >> 
> >> diff --git a/kernel/context_tracking.c b/kernel/context_tracking.c
> >> index 74f68f4..6fe96b1 100644
> >> --- a/kernel/context_tracking.c
> >> +++ b/kernel/context_tracking.c
> >> @@ -1,5 +1,7 @@
> >>  #include <linux/context_tracking.h>
> >> +#ifdef CONFIG_KVM
> >>  #include <linux/kvm_host.h>
> >> +#endif
> >
> > The header should take care of the off-case, no need to ifdef its inclusion.
> 
> In their current form, the headers will not build on platforms without
> KVM support.  For example, these platforms (like ARM) don't even have
> <asm/kvm.h> or <asm/kvm_host.h>.
> 
> >>  #include <linux/rcupdate.h>
> >>  #include <linux/sched.h>
> >>  #include <linux/hardirq.h>
> >> @@ -62,6 +64,7 @@ void user_exit(void)
> >>  	local_irq_restore(flags);
> >>  }
> >>  
> >> +#ifdef CONFIG_KVM
> >>  void guest_enter(void)
> >>  {
> >>  	if (vtime_accounting_enabled())
> >> @@ -79,6 +82,7 @@ void guest_exit(void)
> >>  		__guest_exit();
> >>  }
> >>  EXPORT_SYMBOL_GPL(guest_exit);
> >> +#endif
> >
> > Kvm might be built as a module so we can't actually do this unfortunately.
> 
> Ah, right.  How bout using 
> 
> #if defined(CONFIG_KVM) || defined(CONFIG_KVM_MODULE)
> 
> for both conditionals above?  Updated patch below.

This is what IS_ENABLED(CONFIG_KVM) is for.

Thanks,
Namhyung

> 
> Kevin
> 
> 
> From 4444ce39eea21a5fc63b04a711addab2a5c63243 Mon Sep 17 00:00:00 2001
> From: Kevin Hilman <khilman@linaro.org>
> Date: Thu, 14 Feb 2013 10:17:37 -0800
> Subject: [PATCH] context tracking: conditionalize guest support based on
>  CONFIG_KVM
> 
> So that it can build on !KVM systems too.
> 
> Signed-off-by: Kevin Hilman <khilman@linaro.org>
> ---
>  kernel/context_tracking.c | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/kernel/context_tracking.c b/kernel/context_tracking.c
> index 74f68f4..d9c98dd 100644
> --- a/kernel/context_tracking.c
> +++ b/kernel/context_tracking.c
> @@ -1,5 +1,7 @@
>  #include <linux/context_tracking.h>
> +#if defined(CONFIG_KVM) || defined(CONFIG_KVM_MODULE)
>  #include <linux/kvm_host.h>
> +#endif
>  #include <linux/rcupdate.h>
>  #include <linux/sched.h>
>  #include <linux/hardirq.h>
> @@ -62,6 +64,7 @@ void user_exit(void)
>  	local_irq_restore(flags);
>  }
>  
> +#if defined(CONFIG_KVM) || defined(CONFIG_KVM_MODULE)
>  void guest_enter(void)
>  {
>  	if (vtime_accounting_enabled())
> @@ -79,6 +82,7 @@ void guest_exit(void)
>  		__guest_exit();
>  }
>  EXPORT_SYMBOL_GPL(guest_exit);
> +#endif
>  
>  void context_tracking_task_switch(struct task_struct *prev,
>  			     struct task_struct *next)

^ permalink raw reply	[flat|nested] 42+ messages in thread

* Re: [RFC/PATCH 1/5] context tracking: conditionalize guest support based on CONFIG_KVM
  2013-02-26 22:13         ` Namhyung Kim
@ 2013-02-27 14:24           ` Kevin Hilman
  -1 siblings, 0 replies; 42+ messages in thread
From: Kevin Hilman @ 2013-02-27 14:24 UTC (permalink / raw)
  To: Namhyung Kim
  Cc: Frederic Weisbecker, Mats Liljegren, linaro-dev, linux-kernel,
	linux-arm-kernel

Namhyung Kim <namhyung@kernel.org> writes:

> 2013-02-26 (화), 11:29 -0800, Kevin Hilman:
>> Frederic Weisbecker <fweisbec@gmail.com> writes:
>> 
>> > On Wed, Feb 20, 2013 at 11:41:38AM -0800, Kevin Hilman wrote:
>> >> So that it can build on !KVM systems too.
>> >> 
>> >> Signed-off-by: Kevin Hilman <khilman@linaro.org>
>> >> ---
>> >>  kernel/context_tracking.c | 4 ++++
>> >>  1 file changed, 4 insertions(+)
>> >> 
>> >> diff --git a/kernel/context_tracking.c b/kernel/context_tracking.c
>> >> index 74f68f4..6fe96b1 100644
>> >> --- a/kernel/context_tracking.c
>> >> +++ b/kernel/context_tracking.c
>> >> @@ -1,5 +1,7 @@
>> >>  #include <linux/context_tracking.h>
>> >> +#ifdef CONFIG_KVM
>> >>  #include <linux/kvm_host.h>
>> >> +#endif
>> >
>> > The header should take care of the off-case, no need to ifdef its inclusion.
>> 
>> In their current form, the headers will not build on platforms without
>> KVM support.  For example, these platforms (like ARM) don't even have
>> <asm/kvm.h> or <asm/kvm_host.h>.
>> 
>> >>  #include <linux/rcupdate.h>
>> >>  #include <linux/sched.h>
>> >>  #include <linux/hardirq.h>
>> >> @@ -62,6 +64,7 @@ void user_exit(void)
>> >>  	local_irq_restore(flags);
>> >>  }
>> >>  
>> >> +#ifdef CONFIG_KVM
>> >>  void guest_enter(void)
>> >>  {
>> >>  	if (vtime_accounting_enabled())
>> >> @@ -79,6 +82,7 @@ void guest_exit(void)
>> >>  		__guest_exit();
>> >>  }
>> >>  EXPORT_SYMBOL_GPL(guest_exit);
>> >> +#endif
>> >
>> > Kvm might be built as a module so we can't actually do this unfortunately.
>> 
>> Ah, right.  How bout using 
>> 
>> #if defined(CONFIG_KVM) || defined(CONFIG_KVM_MODULE)
>> 
>> for both conditionals above?  Updated patch below.
>
> This is what IS_ENABLED(CONFIG_KVM) is for.

Right, updated patch below.

Kevin


>From 61e35f069a64c03a2bce348487d41072aeb9f36b Mon Sep 17 00:00:00 2001
From: Kevin Hilman <khilman@linaro.org>
Date: Thu, 14 Feb 2013 10:17:37 -0800
Subject: [PATCH] context tracking: conditionalize guest support based on
 CONFIG_KVM

So that it can build on !KVM systems too.

Signed-off-by: Kevin Hilman <khilman@linaro.org>
---
 kernel/context_tracking.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/kernel/context_tracking.c b/kernel/context_tracking.c
index 74f68f4..fda6cd6 100644
--- a/kernel/context_tracking.c
+++ b/kernel/context_tracking.c
@@ -1,5 +1,7 @@
 #include <linux/context_tracking.h>
+#if IS_ENABLED(CONFIG_KVM)
 #include <linux/kvm_host.h>
+#endif
 #include <linux/rcupdate.h>
 #include <linux/sched.h>
 #include <linux/hardirq.h>
@@ -62,6 +64,7 @@ void user_exit(void)
 	local_irq_restore(flags);
 }
 
+#if IS_ENABLED(CONFIG_KVM)
 void guest_enter(void)
 {
 	if (vtime_accounting_enabled())
@@ -79,6 +82,7 @@ void guest_exit(void)
 		__guest_exit();
 }
 EXPORT_SYMBOL_GPL(guest_exit);
+#endif
 
 void context_tracking_task_switch(struct task_struct *prev,
 			     struct task_struct *next)
-- 
1.8.1.2


^ permalink raw reply related	[flat|nested] 42+ messages in thread

* [RFC/PATCH 1/5] context tracking: conditionalize guest support based on CONFIG_KVM
@ 2013-02-27 14:24           ` Kevin Hilman
  0 siblings, 0 replies; 42+ messages in thread
From: Kevin Hilman @ 2013-02-27 14:24 UTC (permalink / raw)
  To: linux-arm-kernel

Namhyung Kim <namhyung@kernel.org> writes:

> 2013-02-26 (?), 11:29 -0800, Kevin Hilman:
>> Frederic Weisbecker <fweisbec@gmail.com> writes:
>> 
>> > On Wed, Feb 20, 2013 at 11:41:38AM -0800, Kevin Hilman wrote:
>> >> So that it can build on !KVM systems too.
>> >> 
>> >> Signed-off-by: Kevin Hilman <khilman@linaro.org>
>> >> ---
>> >>  kernel/context_tracking.c | 4 ++++
>> >>  1 file changed, 4 insertions(+)
>> >> 
>> >> diff --git a/kernel/context_tracking.c b/kernel/context_tracking.c
>> >> index 74f68f4..6fe96b1 100644
>> >> --- a/kernel/context_tracking.c
>> >> +++ b/kernel/context_tracking.c
>> >> @@ -1,5 +1,7 @@
>> >>  #include <linux/context_tracking.h>
>> >> +#ifdef CONFIG_KVM
>> >>  #include <linux/kvm_host.h>
>> >> +#endif
>> >
>> > The header should take care of the off-case, no need to ifdef its inclusion.
>> 
>> In their current form, the headers will not build on platforms without
>> KVM support.  For example, these platforms (like ARM) don't even have
>> <asm/kvm.h> or <asm/kvm_host.h>.
>> 
>> >>  #include <linux/rcupdate.h>
>> >>  #include <linux/sched.h>
>> >>  #include <linux/hardirq.h>
>> >> @@ -62,6 +64,7 @@ void user_exit(void)
>> >>  	local_irq_restore(flags);
>> >>  }
>> >>  
>> >> +#ifdef CONFIG_KVM
>> >>  void guest_enter(void)
>> >>  {
>> >>  	if (vtime_accounting_enabled())
>> >> @@ -79,6 +82,7 @@ void guest_exit(void)
>> >>  		__guest_exit();
>> >>  }
>> >>  EXPORT_SYMBOL_GPL(guest_exit);
>> >> +#endif
>> >
>> > Kvm might be built as a module so we can't actually do this unfortunately.
>> 
>> Ah, right.  How bout using 
>> 
>> #if defined(CONFIG_KVM) || defined(CONFIG_KVM_MODULE)
>> 
>> for both conditionals above?  Updated patch below.
>
> This is what IS_ENABLED(CONFIG_KVM) is for.

Right, updated patch below.

Kevin


>From 61e35f069a64c03a2bce348487d41072aeb9f36b Mon Sep 17 00:00:00 2001
From: Kevin Hilman <khilman@linaro.org>
Date: Thu, 14 Feb 2013 10:17:37 -0800
Subject: [PATCH] context tracking: conditionalize guest support based on
 CONFIG_KVM

So that it can build on !KVM systems too.

Signed-off-by: Kevin Hilman <khilman@linaro.org>
---
 kernel/context_tracking.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/kernel/context_tracking.c b/kernel/context_tracking.c
index 74f68f4..fda6cd6 100644
--- a/kernel/context_tracking.c
+++ b/kernel/context_tracking.c
@@ -1,5 +1,7 @@
 #include <linux/context_tracking.h>
+#if IS_ENABLED(CONFIG_KVM)
 #include <linux/kvm_host.h>
+#endif
 #include <linux/rcupdate.h>
 #include <linux/sched.h>
 #include <linux/hardirq.h>
@@ -62,6 +64,7 @@ void user_exit(void)
 	local_irq_restore(flags);
 }
 
+#if IS_ENABLED(CONFIG_KVM)
 void guest_enter(void)
 {
 	if (vtime_accounting_enabled())
@@ -79,6 +82,7 @@ void guest_exit(void)
 		__guest_exit();
 }
 EXPORT_SYMBOL_GPL(guest_exit);
+#endif
 
 void context_tracking_task_switch(struct task_struct *prev,
 			     struct task_struct *next)
-- 
1.8.1.2

^ permalink raw reply related	[flat|nested] 42+ messages in thread

* Re: [RFC/PATCH 1/5] context tracking: conditionalize guest support based on CONFIG_KVM
  2013-02-27 14:24           ` Kevin Hilman
@ 2013-02-27 15:21             ` Frederic Weisbecker
  -1 siblings, 0 replies; 42+ messages in thread
From: Frederic Weisbecker @ 2013-02-27 15:21 UTC (permalink / raw)
  To: Kevin Hilman
  Cc: Namhyung Kim, Mats Liljegren, linaro-dev, linux-kernel, linux-arm-kernel

2013/2/27 Kevin Hilman <khilman@linaro.org>:
> From 61e35f069a64c03a2bce348487d41072aeb9f36b Mon Sep 17 00:00:00 2001
> From: Kevin Hilman <khilman@linaro.org>
> Date: Thu, 14 Feb 2013 10:17:37 -0800
> Subject: [PATCH] context tracking: conditionalize guest support based on
>  CONFIG_KVM
>
> So that it can build on !KVM systems too.
>
> Signed-off-by: Kevin Hilman <khilman@linaro.org>
> ---
>  kernel/context_tracking.c | 4 ++++
>  1 file changed, 4 insertions(+)
>
> diff --git a/kernel/context_tracking.c b/kernel/context_tracking.c
> index 74f68f4..fda6cd6 100644
> --- a/kernel/context_tracking.c
> +++ b/kernel/context_tracking.c
> @@ -1,5 +1,7 @@
>  #include <linux/context_tracking.h>
> +#if IS_ENABLED(CONFIG_KVM)
>  #include <linux/kvm_host.h>
> +#endif

Please fix the header rather than its inclusion.
General purpose headers like this must be able to handle the off-cases.

Thanks.

^ permalink raw reply	[flat|nested] 42+ messages in thread

* [RFC/PATCH 1/5] context tracking: conditionalize guest support based on CONFIG_KVM
@ 2013-02-27 15:21             ` Frederic Weisbecker
  0 siblings, 0 replies; 42+ messages in thread
From: Frederic Weisbecker @ 2013-02-27 15:21 UTC (permalink / raw)
  To: linux-arm-kernel

2013/2/27 Kevin Hilman <khilman@linaro.org>:
> From 61e35f069a64c03a2bce348487d41072aeb9f36b Mon Sep 17 00:00:00 2001
> From: Kevin Hilman <khilman@linaro.org>
> Date: Thu, 14 Feb 2013 10:17:37 -0800
> Subject: [PATCH] context tracking: conditionalize guest support based on
>  CONFIG_KVM
>
> So that it can build on !KVM systems too.
>
> Signed-off-by: Kevin Hilman <khilman@linaro.org>
> ---
>  kernel/context_tracking.c | 4 ++++
>  1 file changed, 4 insertions(+)
>
> diff --git a/kernel/context_tracking.c b/kernel/context_tracking.c
> index 74f68f4..fda6cd6 100644
> --- a/kernel/context_tracking.c
> +++ b/kernel/context_tracking.c
> @@ -1,5 +1,7 @@
>  #include <linux/context_tracking.h>
> +#if IS_ENABLED(CONFIG_KVM)
>  #include <linux/kvm_host.h>
> +#endif

Please fix the header rather than its inclusion.
General purpose headers like this must be able to handle the off-cases.

Thanks.

^ permalink raw reply	[flat|nested] 42+ messages in thread

end of thread, other threads:[~2013-02-27 15:21 UTC | newest]

Thread overview: 42+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2013-02-20 19:41 [RFC/PATCH 0/5] context_tracking: prerequisites for ARM support Kevin Hilman
2013-02-20 19:41 ` Kevin Hilman
2013-02-20 19:41 ` [RFC/PATCH 1/5] context tracking: conditionalize guest support based on CONFIG_KVM Kevin Hilman
2013-02-20 19:41   ` Kevin Hilman
2013-02-22 13:27   ` Frederic Weisbecker
2013-02-22 13:27     ` Frederic Weisbecker
2013-02-26 19:29     ` Kevin Hilman
2013-02-26 19:29       ` Kevin Hilman
2013-02-26 22:13       ` Namhyung Kim
2013-02-26 22:13         ` Namhyung Kim
2013-02-27 14:24         ` Kevin Hilman
2013-02-27 14:24           ` Kevin Hilman
2013-02-27 15:21           ` Frederic Weisbecker
2013-02-27 15:21             ` Frederic Weisbecker
2013-02-20 19:41 ` [RFC/PATCH 2/5] kernel_cpustat: convert to atomic 64-bit accessors Kevin Hilman
2013-02-20 19:41   ` Kevin Hilman
2013-02-21 19:38   ` Kevin Hilman
2013-02-21 19:38     ` Kevin Hilman
2013-02-21 21:53     ` Frederic Weisbecker
2013-02-21 21:53       ` Frederic Weisbecker
2013-02-21 21:54       ` Frederic Weisbecker
2013-02-21 21:54         ` Frederic Weisbecker
2013-02-22  5:57         ` Kevin Hilman
2013-02-22  5:57           ` Kevin Hilman
2013-02-21 21:58       ` Russell King - ARM Linux
2013-02-21 21:58         ` Russell King - ARM Linux
2013-02-21 22:15         ` Frederic Weisbecker
2013-02-21 22:15           ` Frederic Weisbecker
2013-02-20 19:41 ` [RFC/PATCH 3/5] virt CPU accounting: Kconfig: drop 64-bit requirment Kevin Hilman
2013-02-20 19:41   ` Kevin Hilman
2013-02-20 19:41 ` [RFC/PATCH 4/5] cputime: use do_div() for nsec resolution conversion helpers Kevin Hilman
2013-02-20 19:41   ` Kevin Hilman
2013-02-21 16:24   ` Frederic Weisbecker
2013-02-21 16:24     ` Frederic Weisbecker
2013-02-21 17:58   ` Namhyung Kim
2013-02-21 17:58     ` Namhyung Kim
2013-02-21 19:21     ` Kevin Hilman
2013-02-21 19:21       ` Kevin Hilman
2013-02-26 15:21       ` Frederic Weisbecker
2013-02-26 15:21         ` Frederic Weisbecker
2013-02-20 19:41 ` [RFC/PATCH 5/5] ARM: Kconfig: allow virt CPU accounting Kevin Hilman
2013-02-20 19:41   ` Kevin Hilman

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.