procfs: do not confuse jiffies with cputime64_t
Subsystems (4)
Michal Hocko
Acked-byQuoted 3 lines · Andreas Schwab
get_{idle,iowait}_time are supposed to return
cputime64_tvalues, not jiffies.Signed-off-by Andreas Schwab <[email protected]>
Ahhh, I have missed that jiffies64_to_cputime64 is doing something for ia64 and powerpc.
Thanks for noticing and fixing that up.
Quoted 34 lines
--- fs/proc/stat.c | 4 ++-- 1 files changed, 2 insertions(+), 2 deletions(-)fs/proc/stat.c +2
-2@@ -32,7 +32,7 @@ static cputime64_t get_idle_time(int cpu) idle =kstat_cpu (cpu).cpustat.idle; idle =cputime64_add (idle,arch_idle_time (cpu)); }else - idle =nsecs_to_jiffies64 (1000 * idle_time); + idle =jiffies64_to_cputime64 (nsecs_to_jiffies64 (1000 * idle_time));return idle; }@@ -46,7 +46,7 @@ static cputime64_t get_iowait_time(int cpu)/* !NO_HZ so we can rely on cpustat.iowait */ iowait =kstat_cpu (cpu).cpustat.iowait; else - iowait =nsecs_to_jiffies64 (1000 * iowait_time); + iowait =jiffies64_to_cputime64 (nsecs_to_jiffies64 (1000 * iowait_time));return iowait; }Signature · 7 lines
1.7.8 -- Andreas Schwab, [email protected] GPG Key fingerprint = 58CA 54C7 6D53 942B 1756 01D3 44D5 214B 8276 4ED5 "And now for something completely different."
Signature · 6 lines
Michal Hocko SUSE Labs SUSE LINUX s.r.o. Lihovarska 1060/12 190 00 Praha 9 Czech Republic
Arnd Bergmann
maintainerReview static cputime64_t get_iowait_time(int cpu) · line 51
Quoted 8 lines · Andreas Schwab
(quoted diff) +1
-1@@ -46,7 +46,7 @@ static cputime64_t get_iowait_time(int cpu) /* !NO_HZ so we can rely on cpustat.iowait */ iowait = kstat_cpu(cpu).cpustat.iowait; else - iowait = nsecs_to_jiffies64(1000 * iowait_time); + iowait = jiffies64_to_cputime64(nsecs_to_jiffies64(1000 * iowait_time)); return iowait;
Hmm, shouldn't this be using nsecs_to_cputime64()? For some reason however, that function is (incorrectly?) defined as
include/asm-generic/cputime.h:#define nsecs_to_cputime64(__ct) nsecs_to_jiffies64(__ct)
and only used in one place, in
kernel/sched.c: if (cputime64_gt(nsecs_to_cputime64(latest_ns), cpustat->irq)) kernel/sched.c: if (cputime64_gt(nsecs_to_cputime64(latest_ns), cpustat->softirq))
I'm not sure what the correct solution is, but I would assume that ia64 and powerpc should fix their definitions of nsecs_to_cputime64() anyway.
Arnd
Quoted 15 lines · Arnd Bergmann
Review static cputime64_t get_iowait_time(int cpu) · line 51
Quoted 8 lines · Andreas Schwab
(quoted diff) +1
-1@@ -46,7 +46,7 @@ static cputime64_t get_iowait_time(int cpu) /* !NO_HZ so we can rely on cpustat.iowait */ iowait = kstat_cpu(cpu).cpustat.iowait; else - iowait = nsecs_to_jiffies64(1000 * iowait_time); + iowait = jiffies64_to_cputime64(nsecs_to_jiffies64(1000 * iowait_time)); return iowait;Hmm, shouldn't this be using
nsecs_to_cputime64()? For some reason however, that function is (incorrectly?) defined asinclude/asm-generic/cputime.h:#define
nsecs_to_cputime64(__ct)nsecs_to_jiffies64(__ct)
Quoted 7 lines
and only used in one place, in
kernel/sched.c: if (cputime64_gt(nsecs_to_cputime64(latest_ns), cpustat->irq))kernel/sched.c: if (cputime64_gt(nsecs_to_cputime64(latest_ns), cpustat->softirq))I'm not sure what the correct solution is, but I would assume that ia64 and powerpc should fix their definitions of
nsecs_to_cputime64()anyway.
There is no definition of nsecs_to_cputime64 on ia64/powerpc64 with CONFIG_VIRT_CPU_ACCOUNTING yet, probably because CONFIG_IRQ_TIME_ACCOUNTING is x86-only.
Andreas.
Signature · 3 lines
Andreas Schwab, [email protected] GPG Key fingerprint = 58CA 54C7 6D53 942B 1756 01D3 44D5 214B 8276 4ED5 "And now for something completely different."
PATCH v2 1/1 procfs: do not confuse jiffies with cputime64_t
In reply to Arnd Bergmannget_{idle,iowait}_time are supposed to return cputime64_t values, not jiffies. Add usecs_to_cputime64 for this.
--- arch/ia64/include/asm/cputime.h | 1 + arch/powerpc/include/asm/cputime.h | 2 ++ fs/proc/stat.c | 4 ++-- include/asm-generic/cputime.h | 1 + 4 files changed, 6 insertions(+), 2 deletions(-)
arch/ia64/include/asm/cputime.h +1 -0
@@ -60,6 +60,7 @@ typedef u64 cputime64_t;*/ #define cputime_to_usecs (__ct) ((__ct) / NSEC_PER_USEC)#define usecs_to_cputime (__usecs) ((__usecs) * NSEC_PER_USEC) +#define usecs_to_cputime64 (__usecs) usecs_to_cputime(__usecs)/* * Convert cputime <-> seconds
arch/powerpc/include/asm/cputime.h +2 -0
@@ -150,6 +150,8 @@ static inline cputime_t usecs_to_cputime(const unsigned long us)return ct; } +#define usecs_to_cputime64 (us) usecs_to_cputime(us) +/* * Convert cputime <-> seconds */
fs/proc/stat.c +2 -2
@@ -32,7 +32,7 @@ static cputime64_t get_idle_time(int cpu) idle =kstat_cpu (cpu).cpustat.idle; idle =cputime64_add (idle,arch_idle_time (cpu)); }else - idle =nsecs_to_jiffies64 (1000 * idle_time); + idle =usecs_to_cputime64 (idle_time);return idle; }
@@ -46,7 +46,7 @@ static cputime64_t get_iowait_time(int cpu)/* !NO_HZ so we can rely on cpustat.iowait */ iowait =kstat_cpu (cpu).cpustat.iowait; else - iowait =nsecs_to_jiffies64 (1000 * iowait_time); + iowait =usecs_to_cputime64 (iowait_time);return iowait; }
include/asm-generic/cputime.h +1 -0
@@ -40,6 +40,7 @@ typedef u64 cputime64_t;*/ #define cputime_to_usecs (__ct) jiffies_to_usecs(__ct)#define usecs_to_cputime (__msecs) usecs_to_jiffies(__msecs) +#define usecs_to_cputime64 (__msecs) nsecs_to_jiffies64((__msecs) * 1000)/* * Convert cputime to seconds and back.
Signature · 7 lines
1.7.8 -- Andreas Schwab, [email protected] GPG Key fingerprint = 58CA 54C7 6D53 942B 1756 01D3 44D5 214B 8276 4ED5 "And now for something completely different."
Quoted 2 lines · Andreas Schwab
get_{idle,iowait}_time are supposed to return
cputime64_tvalues, not jiffies. Addusecs_to_cputime64for this.
OK, this looks much better than doing an artificial us -> ns -> jiffies -> cputime64
Could you also add usecs_to_cputime64 for s390 so that all this is consistent?
This should cover all architectures AFAICS.
Thanks
Quoted 2 lines
Signed-off-by Andreas Schwab <[email protected]>
Quoted 79 lines
--- arch/ia64/include/asm/cputime.h | 1 + arch/powerpc/include/asm/cputime.h | 2 ++ fs/proc/stat.c | 4 ++-- include/asm-generic/cputime.h | 1 + 4 files changed, 6 insertions(+), 2 deletions(-)arch/ia64/include/asm/cputime.h +1
-0@@ -60,6 +60,7 @@ typedef u64 cputime64_t;*/ #define cputime_to_usecs (__ct) ((__ct) / NSEC_PER_USEC)#define usecs_to_cputime (__usecs) ((__usecs) * NSEC_PER_USEC) +#define usecs_to_cputime64 (__usecs) usecs_to_cputime(__usecs)/* * Convert cputime <-> seconds arch/powerpc/include/asm/cputime.h +2
-0@@ -150,6 +150,8 @@ static inline cputime_t usecs_to_cputime(const unsigned long us)return ct; } +#define usecs_to_cputime64 (us) usecs_to_cputime(us) +/* * Convert cputime <-> seconds */ fs/proc/stat.c +2
-2@@ -32,7 +32,7 @@ static cputime64_t get_idle_time(int cpu) idle =kstat_cpu (cpu).cpustat.idle; idle =cputime64_add (idle,arch_idle_time (cpu)); }else - idle =nsecs_to_jiffies64 (1000 * idle_time); + idle =usecs_to_cputime64 (idle_time);return idle; }@@ -46,7 +46,7 @@ static cputime64_t get_iowait_time(int cpu)/* !NO_HZ so we can rely on cpustat.iowait */ iowait =kstat_cpu (cpu).cpustat.iowait; else - iowait =nsecs_to_jiffies64 (1000 * iowait_time); + iowait =usecs_to_cputime64 (iowait_time);return iowait; }include/asm-generic/cputime.h +1
-0@@ -40,6 +40,7 @@ typedef u64 cputime64_t;*/ #define cputime_to_usecs (__ct) jiffies_to_usecs(__ct)#define usecs_to_cputime (__msecs) usecs_to_jiffies(__msecs) +#define usecs_to_cputime64 (__msecs) nsecs_to_jiffies64((__msecs) * 1000)/* * Convert cputime to seconds and back. Signature · 12 lines
1.7.8 -- Andreas Schwab, [email protected] GPG Key fingerprint = 58CA 54C7 6D53 942B 1756 01D3 44D5 214B 8276 4ED5 "And now for something completely different." -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Signature · 6 lines
Michal Hocko SUSE Labs SUSE LINUX s.r.o. Lihovarska 1060/12 190 00 Praha 9 Czech Republic
PATCH v3 1/1 procfs: do not confuse jiffies with cputime64_t
get_{idle,iowait}_time are supposed to return cputime64_t values, not jiffies. Add usecs_to_cputime64 for this.
--- arch/ia64/include/asm/cputime.h | 1 + arch/powerpc/include/asm/cputime.h | 2 ++ arch/s390/include/asm/cputime.h | 2 ++ fs/proc/stat.c | 4 ++-- include/asm-generic/cputime.h | 1 + 5 files changed, 8 insertions(+), 2 deletions(-)
arch/ia64/include/asm/cputime.h +1 -0
@@ -60,6 +60,7 @@ typedef u64 cputime64_t;*/ #define cputime_to_usecs (__ct) ((__ct) / NSEC_PER_USEC)#define usecs_to_cputime (__usecs) ((__usecs) * NSEC_PER_USEC) +#define usecs_to_cputime64 (__usecs) usecs_to_cputime(__usecs)/* * Convert cputime <-> seconds
arch/powerpc/include/asm/cputime.h +2 -0
@@ -150,6 +150,8 @@ static inline cputime_t usecs_to_cputime(const unsigned long us)return ct; } +#define usecs_to_cputime64 (us) usecs_to_cputime(us) +/* * Convert cputime <-> seconds */
arch/s390/include/asm/cputime.h +2 -0
@@ -87,6 +87,8 @@ usecs_to_cputime(const unsigned int m)return (cputime_t ) m *4096 ; } +#define usecs_to_cputime64 (m) usecs_to_cputime(m) +/* * Convert cputime to milliseconds and back. */
fs/proc/stat.c +2 -2
@@ -32,7 +32,7 @@ static cputime64_t get_idle_time(int cpu) idle =kstat_cpu (cpu).cpustat.idle; idle =cputime64_add (idle,arch_idle_time (cpu)); }else - idle =nsecs_to_jiffies64 (1000 * idle_time); + idle =usecs_to_cputime64 (idle_time);return idle; }
@@ -46,7 +46,7 @@ static cputime64_t get_iowait_time(int cpu)/* !NO_HZ so we can rely on cpustat.iowait */ iowait =kstat_cpu (cpu).cpustat.iowait; else - iowait =nsecs_to_jiffies64 (1000 * iowait_time); + iowait =usecs_to_cputime64 (iowait_time);return iowait; }
include/asm-generic/cputime.h +1 -0
@@ -40,6 +40,7 @@ typedef u64 cputime64_t;*/ #define cputime_to_usecs (__ct) jiffies_to_usecs(__ct)#define usecs_to_cputime (__msecs) usecs_to_jiffies(__msecs) +#define usecs_to_cputime64 (__msecs) nsecs_to_jiffies64((__msecs) * 1000)/* * Convert cputime to seconds and back.
Signature · 6 lines
1.7.8 -- Andreas Schwab, [email protected] GPG Key fingerprint = 58CA 54C7 6D53 942B 1756 01D3 44D5 214B 8276 4ED5 "And now for something completely different."
Quoted 4 lines · Andreas Schwab
get_{idle,iowait}_time are supposed to return
cputime64_tvalues, not jiffies. Addusecs_to_cputime64for this.Signed-off-by Andreas Schwab <[email protected]>
Thanks!
Quoted 92 lines
--- arch/ia64/include/asm/cputime.h | 1 + arch/powerpc/include/asm/cputime.h | 2 ++ arch/s390/include/asm/cputime.h | 2 ++ fs/proc/stat.c | 4 ++-- include/asm-generic/cputime.h | 1 + 5 files changed, 8 insertions(+), 2 deletions(-)arch/ia64/include/asm/cputime.h +1
-0@@ -60,6 +60,7 @@ typedef u64 cputime64_t;*/ #define cputime_to_usecs (__ct) ((__ct) / NSEC_PER_USEC)#define usecs_to_cputime (__usecs) ((__usecs) * NSEC_PER_USEC) +#define usecs_to_cputime64 (__usecs) usecs_to_cputime(__usecs)/* * Convert cputime <-> seconds arch/powerpc/include/asm/cputime.h +2
-0@@ -150,6 +150,8 @@ static inline cputime_t usecs_to_cputime(const unsigned long us)return ct; } +#define usecs_to_cputime64 (us) usecs_to_cputime(us) +/* * Convert cputime <-> seconds */ arch/s390/include/asm/cputime.h +2
-0@@ -87,6 +87,8 @@ usecs_to_cputime(const unsigned int m)return (cputime_t ) m *4096 ; } +#define usecs_to_cputime64 (m) usecs_to_cputime(m) +/* * Convert cputime to milliseconds and back. */ fs/proc/stat.c +2
-2@@ -32,7 +32,7 @@ static cputime64_t get_idle_time(int cpu) idle =kstat_cpu (cpu).cpustat.idle; idle =cputime64_add (idle,arch_idle_time (cpu)); }else - idle =nsecs_to_jiffies64 (1000 * idle_time); + idle =usecs_to_cputime64 (idle_time);return idle; }@@ -46,7 +46,7 @@ static cputime64_t get_iowait_time(int cpu)/* !NO_HZ so we can rely on cpustat.iowait */ iowait =kstat_cpu (cpu).cpustat.iowait; else - iowait =nsecs_to_jiffies64 (1000 * iowait_time); + iowait =usecs_to_cputime64 (iowait_time);return iowait; }include/asm-generic/cputime.h +1
-0@@ -40,6 +40,7 @@ typedef u64 cputime64_t;*/ #define cputime_to_usecs (__ct) jiffies_to_usecs(__ct)#define usecs_to_cputime (__msecs) usecs_to_jiffies(__msecs) +#define usecs_to_cputime64 (__msecs) nsecs_to_jiffies64((__msecs) * 1000)/* * Convert cputime to seconds and back. Signature · 11 lines
1.7.8 -- Andreas Schwab, [email protected] GPG Key fingerprint = 58CA 54C7 6D53 942B 1756 01D3 44D5 214B 8276 4ED5 "And now for something completely different." -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Signature · 6 lines
Michal Hocko SUSE Labs SUSE LINUX s.r.o. Lihovarska 1060/12 190 00 Praha 9 Czech Republic
PATCH RESEND 1/1 procfs: do not confuse jiffies with cputime64_t
Hmm, it seems that this bugfix (for 3.2) stalled. I guess that it is primarily because it is multiarch fix.
I am sorry to bother you Andrew but could we push this through you, please?
The full patch for reference:
---
Quoted 1 line
From
1fca39b21f3b344c90c30d98db6dcdcdc6815797Mon Sep 17 00:00:00 2001
From: Andreas Schwab <[email protected]>
Date: Mon, 12 Dec 2011 14:07:53 +0100
Subject: [PATCH] procfs: do not confuse jiffies with cputime64_t
get_{idle,iowait}_time are supposed to return cputime64_t values, not jiffies. Add usecs_to_cputime64 for this.
--- arch/ia64/include/asm/cputime.h | 1 + arch/powerpc/include/asm/cputime.h | 2 ++ arch/s390/include/asm/cputime.h | 2 ++ fs/proc/stat.c | 4 ++-- include/asm-generic/cputime.h | 1 + 5 files changed, 8 insertions(+), 2 deletions(-)
arch/ia64/include/asm/cputime.h +1 -0
@@ -60,6 +60,7 @@ typedef u64 cputime64_t;*/ #define cputime_to_usecs (__ct) ((__ct) / NSEC_PER_USEC)#define usecs_to_cputime (__usecs) ((__usecs) * NSEC_PER_USEC) +#define usecs_to_cputime64 (__usecs) usecs_to_cputime(__usecs)/* * Convert cputime <-> seconds
arch/powerpc/include/asm/cputime.h +2 -0
@@ -150,6 +150,8 @@ static inline cputime_t usecs_to_cputime(const unsigned long us)return ct; } +#define usecs_to_cputime64 (us) usecs_to_cputime(us) +/* * Convert cputime <-> seconds */
arch/s390/include/asm/cputime.h +2 -0
@@ -87,6 +87,8 @@ usecs_to_cputime(const unsigned int m)return (cputime_t ) m *4096 ; } +#define usecs_to_cputime64 (m) usecs_to_cputime(m) +/* * Convert cputime to milliseconds and back. */
fs/proc/stat.c +2 -2
@@ -32,7 +32,7 @@ static cputime64_t get_idle_time(int cpu) idle =kstat_cpu (cpu).cpustat.idle; idle =cputime64_add (idle,arch_idle_time (cpu)); }else - idle =nsecs_to_jiffies64 (1000 * idle_time); + idle =usecs_to_cputime64 (idle_time);return idle; }
@@ -46,7 +46,7 @@ static cputime64_t get_iowait_time(int cpu)/* !NO_HZ so we can rely on cpustat.iowait */ iowait =kstat_cpu (cpu).cpustat.iowait; else - iowait =nsecs_to_jiffies64 (1000 * iowait_time); + iowait =usecs_to_cputime64 (iowait_time);return iowait; }
include/asm-generic/cputime.h +1 -0
@@ -40,6 +40,7 @@ typedef u64 cputime64_t;*/ #define cputime_to_usecs (__ct) jiffies_to_usecs(__ct)#define usecs_to_cputime (__msecs) usecs_to_jiffies(__msecs) +#define usecs_to_cputime64 (__msecs) nsecs_to_jiffies64((__msecs) * 1000)/* * Convert cputime to seconds and back.
Signature · 10 lines
1.7.7.3 -- Michal Hocko SUSE Labs SUSE LINUX s.r.o. Lihovarska 1060/12 190 00 Praha 9 Czech Republic
Quoted 2 lines · On Wed, 21 Dec 2011 11:03:34 +0100 Michal Hocko <[email protected]> wrote:
Hmm, it seems that this bugfix (for 3.2) stalled. I guess that it is primarily because it is multiarch fix.
No, it's because I'm behind in my lkml reading and nobody cc'ed me on it. I'd have got there eventually.
Quoted 2 lines
I am sorry to bother you Andrew but could we push this through you, please?
Sure.
Quoted 9 lines
The full patch for reference:
---Quoted 1 line
From
1fca39b21f3b344c90c30d98db6dcdcdc6815797Mon Sep 17 00:00:00 2001From: Andreas Schwab <[email protected]>
Date: Mon, 12 Dec 2011 14:07:53 +0100
Subject: [PATCH] procfs: do not confuse jiffies withcputime64_tget_{idle,iowait}_time are supposed to return
cputime64_tvalues, not jiffies. Addusecs_to_cputime64for this.
Changelog is poor. The patch is described as a bugfix but there's no description of how the bug affects users.
Without that information I am unable to understand why you think the patch should be in 3.2, why it should not be in 3.1, etc. And without that information, others will find it hard to determine whether this patch will fix some problem which they or their users are experiencing.
(the patch doesn't apply successfully to 3.1 btw).
Quoted 7 lines · On Wed, 21 Dec 2011 11:03:34 +0100 Michal Hocko <[email protected]> wrote:
Hmm, it seems that this bugfix (for 3.2) stalled. I guess that it is primarily because it is multiarch fix.
I am sorry to bother you Andrew but could we push this through you, please?The full patch for reference:
---
Quoted 7 lines
Quoted 1 line
From
1fca39b21f3b344c90c30d98db6dcdcdc6815797Mon Sep 17 00:00:00 2001From: Andreas Schwab <[email protected]>
Date: Mon, 12 Dec 2011 14:07:53 +0100
Subject: [PATCH] procfs: do not confuse jiffies withcputime64_tget_{idle,iowait}_time are supposed to return
cputime64_tvalues, not jiffies. Addusecs_to_cputime64for this.
This makes a huge mess when mixed with Martin's "cputime: add sparse checking and cleanup" in linux-next. I think I got it fixed up but it needs careful checking - I don't _think_ I needed to add more __force thingies. The version against today's linux-next is below.
(I did party tricks with this so I could carry the against-mainline and against-next versions in the same tree).
Also, in include/asm-generic/cputime.h we have:
#define usecs_to_cputime64(__msecs) nsecs_to_jiffies64((__msecs) * 1000)
But it would be neater to have used nsecs_to_cputime64(), surely.
Also, the changelog still sucks
From: Andreas Schwab <[email protected]>
Subject: procfs: do not confuse jiffies with cputime64_t
get_{idle,iowait}_time are supposed to return cputime64_t values, not jiffies. Add usecs_to_cputime64 for this.
---
arch/ia64/include/asm/cputime.h | 1 + arch/powerpc/include/asm/cputime.h | 2 ++ arch/s390/include/asm/cputime.h | 2 ++ fs/proc/stat.c | 4 ++-- include/asm-generic/cputime.h | 1 + 5 files changed, 8 insertions(+), 2 deletions(-)
arch/ia64/include/asm/cputime.h renamed from arch/ia64/include/asm/cputime.h~reapply-procfs-do-not-confuse-jiffies-with-cputime64_t +1 -0
@@ -50,6 +50,7 @@ typedef u64 __nocast cputime64_t; ((__force u64)(__ct) / NSEC_PER_USEC)#define usecs_to_cputime (__usecs) \ (__force cputime_t)((__usecs) * NSEC_PER_USEC) +#define usecs_to_cputime64 (__usecs) usecs_to_cputime(__usecs)/* * Convert cputime <-> seconds
arch/powerpc/include/asm/cputime.h renamed from arch/powerpc/include/asm/cputime.h~reapply-procfs-do-not-confuse-jiffies-with-cputime64_t +2 -0
@@ -134,6 +134,8 @@ static inline cputime_t usecs_to_cputimereturn (__force cputime_t ) ct; } +#define usecs_to_cputime64 (us) usecs_to_cputime(us) +/* * Convert cputime <-> seconds */
arch/s390/include/asm/cputime.h renamed from arch/s390/include/asm/cputime.h~reapply-procfs-do-not-confuse-jiffies-with-cputime64_t +2 -0
@@ -72,6 +72,8 @@ static inline cputime_t usecs_to_cputimereturn (__force cputime_t)(m *4096ULL ); } +#define usecs_to_cputime64 (m) usecs_to_cputime(m) +/* * Convert cputime to milliseconds and back. */
fs/proc/stat.c renamed from fs/proc/stat.c~reapply-procfs-do-not-confuse-jiffies-with-cputime64_t +2 -2
@@ -31,7 +31,7 @@ static u64 get_idle_time(int cpu) idle =kcpustat_cpu (cpu).cpustat[CPUTIME_IDLE ]; idle +=arch_idle_time (cpu); }else - idle =nsecs_to_jiffies64 (1000 * idle_time); + idle =usecs_to_cputime64 (idle_time);return idle; }
@@ -44,7 +44,7 @@ static u64 get_iowait_time(int cpu)/* !NO_HZ so we can rely on cpustat.iowait */ iowait =kcpustat_cpu (cpu).cpustat[CPUTIME_IOWAIT ]; else - iowait =nsecs_to_jiffies64 (1000 * iowait_time); + iowait =usecs_to_cputime64 (iowait_time);return iowait; }
include/asm-generic/cputime.h renamed from include/asm-generic/cputime.h~reapply-procfs-do-not-confuse-jiffies-with-cputime64_t +1 -0
@@ -27,6 +27,7 @@ typedef u64 __nocast cputime64_t;jiffies_to_usecs (cputime_to_jiffies (__ct));#define usecs_to_cputime (__msecs) \ jiffies_to_cputime(usecs_to_jiffies(__msecs)); +#define usecs_to_cputime64 (__msecs) nsecs_to_jiffies64((__msecs) * 1000)/* * Convert cputime to seconds and back.
_
.
Quoted 5 lines · Andrew Morton
From: Andreas Schwab <[email protected]>
Subject: procfs: do not confuse jiffies withcputime64_tget_{idle,iowait}_time are supposed to return
cputime64_tvalues, not jiffies. Addusecs_to_cputime64for this.
For most architectures (which use the asm-generic/cputime.h) jiffies use the same unit as cputime64_t values, but ia64, powerpc and s390 make them different.
Andreas.
Signature · 3 lines
Andreas Schwab, [email protected] GPG Key fingerprint = 58CA 54C7 6D53 942B 1756 01D3 44D5 214B 8276 4ED5 "And now for something completely different."
Quoted 5 lines · Andrew Morton
Also, in
include/asm-generic/cputime.hwe have:#define
usecs_to_cputime64(__msecs)nsecs_to_jiffies64((__msecs) * 1000)But it would be neater to have used
nsecs_to_cputime64(), surely.
The procfs interface wants to convert usecs to cputime64, but generic cputime does not have usecs_to_jiffies64. Once someone writes the latter it can be used here.
Andreas.
Signature · 3 lines
Andreas Schwab, [email protected] GPG Key fingerprint = 58CA 54C7 6D53 942B 1756 01D3 44D5 214B 8276 4ED5 "And now for something completely different."
Quoted 5 lines · On Thu, 22 Dec 2011 00:50:06 +0100 Andreas Schwab <[email protected]> wrote:
Quoted 3 lines · Andrew Morton
From: Andreas Schwab <[email protected]>
Subject: procfs: do not confuse jiffies withcputime64_t
Quoted 7 lines
Quoted 2 lines
get_{idle,iowait}_time are supposed to return
cputime64_tvalues, not jiffies. Addusecs_to_cputime64for this.For most architectures (which use the
asm-generic/cputime.h) jiffies use the same unit ascputime64_tvalues, but ia64, powerpc and s390 make them different.
OK, but
a) what effect does this problem have upon users of the kernel?
b) which kernel version(s) are affected?
Quoted 4 lines · On Thu, 22 Dec 2011 00:55:07 +0100 Andreas Schwab <[email protected]> wrote:
Quoted 2 lines · Andrew Morton
Also, in
include/asm-generic/cputime.hwe have:
Quoted 8 lines
Quoted 3 lines
#define
usecs_to_cputime64(__msecs)nsecs_to_jiffies64((__msecs) * 1000)But it would be neater to have used
nsecs_to_cputime64(), surely.The procfs interface wants to convert usecs to cputime64, but generic cputime does not have
usecs_to_jiffies64. Once someone writes the latter it can be used here.
That doesn't address my suggestion.
I'm saying that this:
#define usecs_to_cputime64(__msecs) nsecs_to_jiffies64((__msecs) * 1000)
should have instead been
#define usecs_to_cputime64(__msecs) nsecs_to_cputime64((__msecs) * 1000)
Quoted 11 lines · Andrew Morton
Quoted 8 lines · On Thu, 22 Dec 2011 00:50:06 +0100 Andreas Schwab <[email protected]> wrote:
Quoted 5 lines · Andrew Morton
From: Andreas Schwab <[email protected]>
Subject: procfs: do not confuse jiffies withcputime64_tget_{idle,iowait}_time are supposed to return
cputime64_tvalues, not jiffies. Addusecs_to_cputime64for this.
Quoted 8 lines
Quoted 4 lines
For most architectures (which use the
asm-generic/cputime.h) jiffies use the same unit ascputime64_tvalues, but ia64, powerpc and s390 make them different.OK, but
a) what effect does this problem have upon users of the kernel?
get_idle_time returns the value in the wrong unit.
Quoted 1 line
b) which kernel version(s) are affected?
get_idle_time got broken in 2a95ea6c0d129b4568fb64e1deda16ceb20e6636.
$ git tag --contains a25cac5198d4ff2842ccca63b423962848ad24b2 v3.2-rc1 v3.2-rc2 v3.2-rc3 v3.2-rc4 v3.2-rc5 v3.2-rc6
Andreas.
Signature · 3 lines
Andreas Schwab, [email protected] GPG Key fingerprint = 58CA 54C7 6D53 942B 1756 01D3 44D5 214B 8276 4ED5 "And now for something completely different."
Quoted 3 lines · On Thu, 22 Dec 2011 01:14:39 +0100 Andreas Schwab <[email protected]> wrote:
Quoted 1 line
a) what effect does this problem have upon users of the kernel?
get_idle_timereturns the value in the wrong unit.
So the "idle" fields in /proc/stat are wrong.
We're getting there! Now, in what way are they wrong? How could a kernel maintainer recognise that this patch is needed?
Quoted 18 lines · Andrew Morton
Quoted 4 lines · On Thu, 22 Dec 2011 00:55:07 +0100 Andreas Schwab <[email protected]> wrote:
Quoted 2 lines · Andrew Morton
Also, in
include/asm-generic/cputime.hwe have:Quoted 8 lines
Quoted 3 lines
#define
usecs_to_cputime64(__msecs)nsecs_to_jiffies64((__msecs) * 1000)But it would be neater to have used
nsecs_to_cputime64(), surely.The procfs interface wants to convert usecs to cputime64, but generic cputime does not have
usecs_to_jiffies64. Once someone writes the latter it can be used here.That doesn't address my suggestion.
Quoted 7 lines
I'm saying that this:
#define
usecs_to_cputime64(__msecs)nsecs_to_jiffies64((__msecs) * 1000)should have instead been
#define
usecs_to_cputime64(__msecs)nsecs_to_cputime64((__msecs) * 1000)
It follows the style of the exitsting definitions. Generic cputime does not differentiate between jiffies and cputime, so adding a layer of indirection only here does not increase clarity, IMHO.
Andreas.
Signature · 3 lines
Andreas Schwab, [email protected] GPG Key fingerprint = 58CA 54C7 6D53 942B 1756 01D3 44D5 214B 8276 4ED5 "And now for something completely different."
Quoted 15 lines · On Wed, 21 Dec 2011 11:59:19 -0800 Andrew Morton <[email protected]>…
Quoted 12 lines · On Wed, 21 Dec 2011 11:03:34 +0100 Michal Hocko <[email protected]> wrote:
Hmm, it seems that this bugfix (for 3.2) stalled. I guess that it is primarily because it is multiarch fix.
I am sorry to bother you Andrew but could we push this through you, please?The full patch for reference:
---Quoted 1 line
From
1fca39b21f3b344c90c30d98db6dcdcdc6815797Mon Sep 17 00:00:00 2001From: Andreas Schwab <[email protected]>
Date: Mon, 12 Dec 2011 14:07:53 +0100
Subject: [PATCH] procfs: do not confuse jiffies withcputime64_t
Quoted 7 lines
Quoted 2 lines
get_{idle,iowait}_time are supposed to return
cputime64_tvalues, not jiffies. Addusecs_to_cputime64for this.This makes a huge mess when mixed with Martin's "cputime: add sparse checking and cleanup" in linux-next. I think I got it fixed up but it needs careful checking - I don't _think_ I needed to add more
__forcethingies. The version against today's linux-next is below.
As long as cputime_t and cputime64_t have the same base type my version of sparse does not warn if you mix the two types.
Quoted 9 lines
(I did party tricks with this so I could carry the against-mainline and against-next versions in the same tree).
Also, in
include/asm-generic/cputime.hwe have:#define
usecs_to_cputime64(__msecs)nsecs_to_jiffies64((__msecs) * 1000)But it would be neater to have used
nsecs_to_cputime64(), surely.
It would be cleaner to do an explicit cast to cputime64_t for all of the usecs_to_cputime64() definitions.
Signature · 4 lines
blue skies, Martin. "Reality continues to ruin my life." - Calvin.
Quoted 10 lines · Andrew Morton
Quoted 3 lines · On Thu, 22 Dec 2011 01:14:39 +0100 Andreas Schwab <[email protected]> wrote:
Quoted 1 line
a) what effect does this problem have upon users of the kernel?
get_idle_timereturns the value in the wrong unit.So the "idle" fields in
/proc/statare wrong.We're getting there! Now, in what way are they wrong?
They use the wrong unit, off by the factor between jiffies and cputime.
Quoted 1 line
How could a kernel maintainer recognise that this patch is needed?
By running the kernel on one of the architectures that are affected (did I already say that on most architectures jiffies and cputime are equivalent?).
Andreas.
Signature · 3 lines
Andreas Schwab, [email protected] GPG Key fingerprint = 58CA 54C7 6D53 942B 1756 01D3 44D5 214B 8276 4ED5 "And now for something completely different."
Quoted 5 lines · Andrew Morton
From: Andreas Schwab <[email protected]>
Subject: procfs: do not confuse jiffies withcputime64_tget_{idle,iowait}_time are supposed to return
cputime64_tvalues, not jiffies. Addusecs_to_cputime64for this.
Subject: procfs: do not confuse jiffies with cputime64_t
Commit 2a95ea6c0d129b4568fb64e1deda16ceb20e6636 ("procfs: do not overflow get_{idle,iowait}_time for nohz") did not take into account that one some architectures jiffies and cputime use different units. Instead of converting the usec value returned by get_cpu_{idle,iowait}_time_us to units of jiffies, use the new function usecs_to_cputime64 to convert it to the correct unit of cputime64_t.
Andreas.
Signature · 3 lines
Andreas Schwab, [email protected] GPG Key fingerprint = 58CA 54C7 6D53 942B 1756 01D3 44D5 214B 8276 4ED5 "And now for something completely different."