* Re: [PATCH] procfs: do not confuse jiffies with cputime64_t [not found] <m28vmi7ip3.fsf@igel.home> @ 2011-12-12 8:16 ` Michal Hocko 2011-12-12 9:22 ` Arnd Bergmann 1 sibling, 0 replies; 20+ messages in thread From: Michal Hocko @ 2011-12-12 8:16 UTC (permalink / raw) To: Andreas Schwab Cc: linux-kernel, Artem S.Tashkinov, Dave Jones, Arnd Bergmann, Alexey Dobriyan, Thomas Gleixner On Mon 12-12-11 01:11:04, Andreas Schwab wrote: > get_{idle,iowait}_time are supposed to return cputime64_t values, not jiffies. > > Signed-off-by: Andreas Schwab <schwab@linux-m68k.org> Ahhh, I have missed that jiffies64_to_cputime64 is doing something for ia64 and powerpc. Thanks for noticing and fixing that up. Acked-by: Michal Hocko <mhocko@suse.cz> > --- > fs/proc/stat.c | 4 ++-- > 1 files changed, 2 insertions(+), 2 deletions(-) > > diff --git a/fs/proc/stat.c b/fs/proc/stat.c > index 2a30d67..88f8a55 100644 > --- a/fs/proc/stat.c > +++ b/fs/proc/stat.c > @@ -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; > } > -- > 1.7.8 > > > -- > Andreas Schwab, schwab@linux-m68k.org > GPG Key fingerprint = 58CA 54C7 6D53 942B 1756 01D3 44D5 214B 8276 4ED5 > "And now for something completely different." -- Michal Hocko SUSE Labs SUSE LINUX s.r.o. Lihovarska 1060/12 190 00 Praha 9 Czech Republic ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH] procfs: do not confuse jiffies with cputime64_t [not found] <m28vmi7ip3.fsf@igel.home> 2011-12-12 8:16 ` [PATCH] procfs: do not confuse jiffies with cputime64_t Michal Hocko @ 2011-12-12 9:22 ` Arnd Bergmann 2011-12-12 10:17 ` Andreas Schwab 2011-12-12 11:51 ` [PATCH v2] " Andreas Schwab 1 sibling, 2 replies; 20+ messages in thread From: Arnd Bergmann @ 2011-12-12 9:22 UTC (permalink / raw) To: Andreas Schwab Cc: linux-kernel, Michal Hocko, Artem S. Tashkinov, Dave Jones, Alexey Dobriyan, Thomas Gleixner On Monday 12 December 2011, Andreas Schwab wrote: > @@ -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 ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH] procfs: do not confuse jiffies with cputime64_t 2011-12-12 9:22 ` Arnd Bergmann @ 2011-12-12 10:17 ` Andreas Schwab 2011-12-12 11:51 ` [PATCH v2] " Andreas Schwab 1 sibling, 0 replies; 20+ messages in thread From: Andreas Schwab @ 2011-12-12 10:17 UTC (permalink / raw) To: Arnd Bergmann Cc: linux-kernel, Michal Hocko, Artem S. Tashkinov, Dave Jones, Alexey Dobriyan, Thomas Gleixner Arnd Bergmann <arnd@arndb.de> writes: > On Monday 12 December 2011, Andreas Schwab wrote: >> @@ -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. 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. -- Andreas Schwab, schwab@linux-m68k.org GPG Key fingerprint = 58CA 54C7 6D53 942B 1756 01D3 44D5 214B 8276 4ED5 "And now for something completely different." ^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH v2] procfs: do not confuse jiffies with cputime64_t 2011-12-12 9:22 ` Arnd Bergmann 2011-12-12 10:17 ` Andreas Schwab @ 2011-12-12 11:51 ` Andreas Schwab 2011-12-12 12:43 ` Michal Hocko 1 sibling, 1 reply; 20+ messages in thread From: Andreas Schwab @ 2011-12-12 11:51 UTC (permalink / raw) To: Arnd Bergmann Cc: linux-kernel, Michal Hocko, Artem S. Tashkinov, Dave Jones, Alexey Dobriyan, Thomas Gleixner get_{idle,iowait}_time are supposed to return cputime64_t values, not jiffies. Add usecs_to_cputime64 for this. Signed-off-by: Andreas Schwab <schwab@linux-m68k.org> --- 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(-) diff --git a/arch/ia64/include/asm/cputime.h b/arch/ia64/include/asm/cputime.h index 6073b18..5a274af 100644 --- a/arch/ia64/include/asm/cputime.h +++ b/arch/ia64/include/asm/cputime.h @@ -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 diff --git a/arch/powerpc/include/asm/cputime.h b/arch/powerpc/include/asm/cputime.h index 33a3580..fa3f921 100644 --- a/arch/powerpc/include/asm/cputime.h +++ b/arch/powerpc/include/asm/cputime.h @@ -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 */ diff --git a/fs/proc/stat.c b/fs/proc/stat.c index 2a30d67..0855e6f 100644 --- a/fs/proc/stat.c +++ b/fs/proc/stat.c @@ -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; } diff --git a/include/asm-generic/cputime.h b/include/asm-generic/cputime.h index 62ce682..12a1764 100644 --- a/include/asm-generic/cputime.h +++ b/include/asm-generic/cputime.h @@ -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. -- 1.7.8 -- Andreas Schwab, schwab@linux-m68k.org GPG Key fingerprint = 58CA 54C7 6D53 942B 1756 01D3 44D5 214B 8276 4ED5 "And now for something completely different." ^ permalink raw reply related [flat|nested] 20+ messages in thread
* Re: [PATCH v2] procfs: do not confuse jiffies with cputime64_t 2011-12-12 11:51 ` [PATCH v2] " Andreas Schwab @ 2011-12-12 12:43 ` Michal Hocko 2011-12-12 13:07 ` [PATCH v3] " Andreas Schwab 0 siblings, 1 reply; 20+ messages in thread From: Michal Hocko @ 2011-12-12 12:43 UTC (permalink / raw) To: Andreas Schwab Cc: Arnd Bergmann, linux-kernel, Artem S. Tashkinov, Dave Jones, Alexey Dobriyan, Thomas Gleixner On Mon 12-12-11 12:51:19, Andreas Schwab wrote: > get_{idle,iowait}_time are supposed to return cputime64_t values, not > jiffies. Add usecs_to_cputime64 for 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 > > Signed-off-by: Andreas Schwab <schwab@linux-m68k.org> Acked-by: Michal Hocko <mhocko@suse.cz> > --- > 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(-) > > diff --git a/arch/ia64/include/asm/cputime.h b/arch/ia64/include/asm/cputime.h > index 6073b18..5a274af 100644 > --- a/arch/ia64/include/asm/cputime.h > +++ b/arch/ia64/include/asm/cputime.h > @@ -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 > diff --git a/arch/powerpc/include/asm/cputime.h b/arch/powerpc/include/asm/cputime.h > index 33a3580..fa3f921 100644 > --- a/arch/powerpc/include/asm/cputime.h > +++ b/arch/powerpc/include/asm/cputime.h > @@ -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 > */ > diff --git a/fs/proc/stat.c b/fs/proc/stat.c > index 2a30d67..0855e6f 100644 > --- a/fs/proc/stat.c > +++ b/fs/proc/stat.c > @@ -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; > } > diff --git a/include/asm-generic/cputime.h b/include/asm-generic/cputime.h > index 62ce682..12a1764 100644 > --- a/include/asm-generic/cputime.h > +++ b/include/asm-generic/cputime.h > @@ -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. > -- > 1.7.8 > > > -- > Andreas Schwab, schwab@linux-m68k.org > 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 majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html > Please read the FAQ at http://www.tux.org/lkml/ -- Michal Hocko SUSE Labs SUSE LINUX s.r.o. Lihovarska 1060/12 190 00 Praha 9 Czech Republic ^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH v3] procfs: do not confuse jiffies with cputime64_t 2011-12-12 12:43 ` Michal Hocko @ 2011-12-12 13:07 ` Andreas Schwab 2011-12-12 13:12 ` Michal Hocko 0 siblings, 1 reply; 20+ messages in thread From: Andreas Schwab @ 2011-12-12 13:07 UTC (permalink / raw) To: Michal Hocko Cc: Arnd Bergmann, linux-kernel, Artem S. Tashkinov, Dave Jones, Alexey Dobriyan, Thomas Gleixner get_{idle,iowait}_time are supposed to return cputime64_t values, not jiffies. Add usecs_to_cputime64 for this. Signed-off-by: Andreas Schwab <schwab@linux-m68k.org> --- 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(-) diff --git a/arch/ia64/include/asm/cputime.h b/arch/ia64/include/asm/cputime.h index 6073b18..5a274af 100644 --- a/arch/ia64/include/asm/cputime.h +++ b/arch/ia64/include/asm/cputime.h @@ -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 diff --git a/arch/powerpc/include/asm/cputime.h b/arch/powerpc/include/asm/cputime.h index 33a3580..fa3f921 100644 --- a/arch/powerpc/include/asm/cputime.h +++ b/arch/powerpc/include/asm/cputime.h @@ -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 */ diff --git a/arch/s390/include/asm/cputime.h b/arch/s390/include/asm/cputime.h index 0814348..b9acaaa 100644 --- a/arch/s390/include/asm/cputime.h +++ b/arch/s390/include/asm/cputime.h @@ -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. */ diff --git a/fs/proc/stat.c b/fs/proc/stat.c index 2a30d67..0855e6f 100644 --- a/fs/proc/stat.c +++ b/fs/proc/stat.c @@ -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; } diff --git a/include/asm-generic/cputime.h b/include/asm-generic/cputime.h index 62ce682..12a1764 100644 --- a/include/asm-generic/cputime.h +++ b/include/asm-generic/cputime.h @@ -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. -- 1.7.8 -- Andreas Schwab, schwab@linux-m68k.org GPG Key fingerprint = 58CA 54C7 6D53 942B 1756 01D3 44D5 214B 8276 4ED5 "And now for something completely different." ^ permalink raw reply related [flat|nested] 20+ messages in thread
* Re: [PATCH v3] procfs: do not confuse jiffies with cputime64_t 2011-12-12 13:07 ` [PATCH v3] " Andreas Schwab @ 2011-12-12 13:12 ` Michal Hocko 2011-12-21 10:03 ` [resend PATCH for 3.2] " Michal Hocko 0 siblings, 1 reply; 20+ messages in thread From: Michal Hocko @ 2011-12-12 13:12 UTC (permalink / raw) To: Andreas Schwab Cc: Arnd Bergmann, linux-kernel, Artem S. Tashkinov, Dave Jones, Alexey Dobriyan, Thomas Gleixner On Mon 12-12-11 14:07:53, Andreas Schwab wrote: > get_{idle,iowait}_time are supposed to return cputime64_t values, not > jiffies. Add usecs_to_cputime64 for this. > > Signed-off-by: Andreas Schwab <schwab@linux-m68k.org> Acked-by: Michal Hocko <mhocko@suse.cz> Thanks! > --- > 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(-) > > diff --git a/arch/ia64/include/asm/cputime.h b/arch/ia64/include/asm/cputime.h > index 6073b18..5a274af 100644 > --- a/arch/ia64/include/asm/cputime.h > +++ b/arch/ia64/include/asm/cputime.h > @@ -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 > diff --git a/arch/powerpc/include/asm/cputime.h b/arch/powerpc/include/asm/cputime.h > index 33a3580..fa3f921 100644 > --- a/arch/powerpc/include/asm/cputime.h > +++ b/arch/powerpc/include/asm/cputime.h > @@ -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 > */ > diff --git a/arch/s390/include/asm/cputime.h b/arch/s390/include/asm/cputime.h > index 0814348..b9acaaa 100644 > --- a/arch/s390/include/asm/cputime.h > +++ b/arch/s390/include/asm/cputime.h > @@ -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. > */ > diff --git a/fs/proc/stat.c b/fs/proc/stat.c > index 2a30d67..0855e6f 100644 > --- a/fs/proc/stat.c > +++ b/fs/proc/stat.c > @@ -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; > } > diff --git a/include/asm-generic/cputime.h b/include/asm-generic/cputime.h > index 62ce682..12a1764 100644 > --- a/include/asm-generic/cputime.h > +++ b/include/asm-generic/cputime.h > @@ -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. > -- > 1.7.8 > > -- > Andreas Schwab, schwab@linux-m68k.org > 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 majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html > Please read the FAQ at http://www.tux.org/lkml/ -- Michal Hocko SUSE Labs SUSE LINUX s.r.o. Lihovarska 1060/12 190 00 Praha 9 Czech Republic ^ permalink raw reply [flat|nested] 20+ messages in thread
* [resend PATCH for 3.2] procfs: do not confuse jiffies with cputime64_t 2011-12-12 13:12 ` Michal Hocko @ 2011-12-21 10:03 ` Michal Hocko 2011-12-21 19:43 ` Andrew Morton 2011-12-21 19:59 ` Andrew Morton 0 siblings, 2 replies; 20+ messages in thread From: Michal Hocko @ 2011-12-21 10:03 UTC (permalink / raw) To: Andrew Morton Cc: Arnd Bergmann, linux-kernel, Artem S. Tashkinov, Dave Jones, Alexey Dobriyan, Thomas Gleixner, Andreas Schwab 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: --- >From 1fca39b21f3b344c90c30d98db6dcdcdc6815797 Mon Sep 17 00:00:00 2001 From: Andreas Schwab <schwab@linux-m68k.org> 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. Signed-off-by: Andreas Schwab <schwab@linux-m68k.org> Acked-by: Michal Hocko <mhocko@suse.cz> --- 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(-) diff --git a/arch/ia64/include/asm/cputime.h b/arch/ia64/include/asm/cputime.h index 6073b18..5a274af 100644 --- a/arch/ia64/include/asm/cputime.h +++ b/arch/ia64/include/asm/cputime.h @@ -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 diff --git a/arch/powerpc/include/asm/cputime.h b/arch/powerpc/include/asm/cputime.h index 1cf20bd..98b7c4b 100644 --- a/arch/powerpc/include/asm/cputime.h +++ b/arch/powerpc/include/asm/cputime.h @@ -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 */ diff --git a/arch/s390/include/asm/cputime.h b/arch/s390/include/asm/cputime.h index 0814348..b9acaaa 100644 --- a/arch/s390/include/asm/cputime.h +++ b/arch/s390/include/asm/cputime.h @@ -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. */ diff --git a/fs/proc/stat.c b/fs/proc/stat.c index 2a30d67..0855e6f 100644 --- a/fs/proc/stat.c +++ b/fs/proc/stat.c @@ -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; } diff --git a/include/asm-generic/cputime.h b/include/asm-generic/cputime.h index 62ce682..12a1764 100644 --- a/include/asm-generic/cputime.h +++ b/include/asm-generic/cputime.h @@ -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. -- 1.7.7.3 -- Michal Hocko SUSE Labs SUSE LINUX s.r.o. Lihovarska 1060/12 190 00 Praha 9 Czech Republic ^ permalink raw reply related [flat|nested] 20+ messages in thread
* Re: [resend PATCH for 3.2] procfs: do not confuse jiffies with cputime64_t 2011-12-21 10:03 ` [resend PATCH for 3.2] " Michal Hocko @ 2011-12-21 19:43 ` Andrew Morton 2011-12-21 19:59 ` Andrew Morton 1 sibling, 0 replies; 20+ messages in thread From: Andrew Morton @ 2011-12-21 19:43 UTC (permalink / raw) To: Michal Hocko Cc: Arnd Bergmann, linux-kernel, Artem S. Tashkinov, Dave Jones, Alexey Dobriyan, Thomas Gleixner, Andreas Schwab On Wed, 21 Dec 2011 11:03:34 +0100 Michal Hocko <mhocko@suse.cz> 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. > I am sorry to bother you Andrew but could we push this through you, > please? Sure. > The full patch for reference: > --- > >From 1fca39b21f3b344c90c30d98db6dcdcdc6815797 Mon Sep 17 00:00:00 2001 > From: Andreas Schwab <schwab@linux-m68k.org> > 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. 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). ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [resend PATCH for 3.2] procfs: do not confuse jiffies with cputime64_t 2011-12-21 10:03 ` [resend PATCH for 3.2] " Michal Hocko 2011-12-21 19:43 ` Andrew Morton @ 2011-12-21 19:59 ` Andrew Morton 2011-12-21 23:50 ` Andreas Schwab ` (3 more replies) 1 sibling, 4 replies; 20+ messages in thread From: Andrew Morton @ 2011-12-21 19:59 UTC (permalink / raw) To: Michal Hocko Cc: Arnd Bergmann, linux-kernel, Artem S. Tashkinov, Dave Jones, Alexey Dobriyan, Thomas Gleixner, Andreas Schwab, Martin Schwidefsky On Wed, 21 Dec 2011 11:03:34 +0100 Michal Hocko <mhocko@suse.cz> 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: > --- > >From 1fca39b21f3b344c90c30d98db6dcdcdc6815797 Mon Sep 17 00:00:00 2001 > From: Andreas Schwab <schwab@linux-m68k.org> > 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. 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 <schwab@linux-m68k.org> 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. Signed-off-by: Andreas Schwab <schwab@linux-m68k.org> Acked-by: Michal Hocko <mhocko@suse.cz> Cc: Arnd Bergmann <arnd@arndb.de> Cc: "Artem S. Tashkinov" <t.artem@mailcity.com> Cc: Dave Jones <davej@redhat.com> Cc: Alexey Dobriyan <adobriyan@gmail.com> Cc: Thomas Gleixner <tglx@linutronix.de> Cc: "Luck, Tony" <tony.luck@intel.com> Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org> Cc: Martin Schwidefsky <schwidefsky@de.ibm.com> Cc: Heiko Carstens <heiko.carstens@de.ibm.com> Signed-off-by: Andrew Morton <akpm@linux-foundation.org> --- 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(-) diff -puN arch/ia64/include/asm/cputime.h~reapply-procfs-do-not-confuse-jiffies-with-cputime64_t arch/ia64/include/asm/cputime.h --- a/arch/ia64/include/asm/cputime.h~reapply-procfs-do-not-confuse-jiffies-with-cputime64_t +++ a/arch/ia64/include/asm/cputime.h @@ -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 diff -puN arch/powerpc/include/asm/cputime.h~reapply-procfs-do-not-confuse-jiffies-with-cputime64_t arch/powerpc/include/asm/cputime.h --- a/arch/powerpc/include/asm/cputime.h~reapply-procfs-do-not-confuse-jiffies-with-cputime64_t +++ a/arch/powerpc/include/asm/cputime.h @@ -134,6 +134,8 @@ static inline cputime_t usecs_to_cputime return (__force cputime_t) ct; } +#define usecs_to_cputime64(us) usecs_to_cputime(us) + /* * Convert cputime <-> seconds */ diff -puN arch/s390/include/asm/cputime.h~reapply-procfs-do-not-confuse-jiffies-with-cputime64_t arch/s390/include/asm/cputime.h --- a/arch/s390/include/asm/cputime.h~reapply-procfs-do-not-confuse-jiffies-with-cputime64_t +++ a/arch/s390/include/asm/cputime.h @@ -72,6 +72,8 @@ static inline cputime_t usecs_to_cputime return (__force cputime_t)(m * 4096ULL); } +#define usecs_to_cputime64(m) usecs_to_cputime(m) + /* * Convert cputime to milliseconds and back. */ diff -puN fs/proc/stat.c~reapply-procfs-do-not-confuse-jiffies-with-cputime64_t fs/proc/stat.c --- a/fs/proc/stat.c~reapply-procfs-do-not-confuse-jiffies-with-cputime64_t +++ a/fs/proc/stat.c @@ -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; } diff -puN include/asm-generic/cputime.h~reapply-procfs-do-not-confuse-jiffies-with-cputime64_t include/asm-generic/cputime.h --- a/include/asm-generic/cputime.h~reapply-procfs-do-not-confuse-jiffies-with-cputime64_t +++ a/include/asm-generic/cputime.h @@ -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. _ . ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [resend PATCH for 3.2] procfs: do not confuse jiffies with cputime64_t 2011-12-21 19:59 ` Andrew Morton @ 2011-12-21 23:50 ` Andreas Schwab 2011-12-21 23:56 ` Andrew Morton 2011-12-21 23:55 ` Andreas Schwab ` (2 subsequent siblings) 3 siblings, 1 reply; 20+ messages in thread From: Andreas Schwab @ 2011-12-21 23:50 UTC (permalink / raw) To: Andrew Morton Cc: Michal Hocko, Arnd Bergmann, linux-kernel, Artem S. Tashkinov, Dave Jones, Alexey Dobriyan, Thomas Gleixner, Martin Schwidefsky Andrew Morton <akpm@linux-foundation.org> writes: > From: Andreas Schwab <schwab@linux-m68k.org> > 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. 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. -- Andreas Schwab, schwab@linux-m68k.org GPG Key fingerprint = 58CA 54C7 6D53 942B 1756 01D3 44D5 214B 8276 4ED5 "And now for something completely different." ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [resend PATCH for 3.2] procfs: do not confuse jiffies with cputime64_t 2011-12-21 23:50 ` Andreas Schwab @ 2011-12-21 23:56 ` Andrew Morton 2011-12-22 0:14 ` Andreas Schwab 0 siblings, 1 reply; 20+ messages in thread From: Andrew Morton @ 2011-12-21 23:56 UTC (permalink / raw) To: Andreas Schwab Cc: Michal Hocko, Arnd Bergmann, linux-kernel, Artem S. Tashkinov, Dave Jones, Alexey Dobriyan, Thomas Gleixner, Martin Schwidefsky On Thu, 22 Dec 2011 00:50:06 +0100 Andreas Schwab <schwab@linux-m68k.org> wrote: > Andrew Morton <akpm@linux-foundation.org> writes: > > > From: Andreas Schwab <schwab@linux-m68k.org> > > 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. > > 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. > OK, but a) what effect does this problem have upon users of the kernel? b) which kernel version(s) are affected? ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [resend PATCH for 3.2] procfs: do not confuse jiffies with cputime64_t 2011-12-21 23:56 ` Andrew Morton @ 2011-12-22 0:14 ` Andreas Schwab 2011-12-22 0:19 ` Andrew Morton 0 siblings, 1 reply; 20+ messages in thread From: Andreas Schwab @ 2011-12-22 0:14 UTC (permalink / raw) To: Andrew Morton Cc: Michal Hocko, Arnd Bergmann, linux-kernel, Artem S. Tashkinov, Dave Jones, Alexey Dobriyan, Thomas Gleixner, Martin Schwidefsky Andrew Morton <akpm@linux-foundation.org> writes: > On Thu, 22 Dec 2011 00:50:06 +0100 > Andreas Schwab <schwab@linux-m68k.org> wrote: > >> Andrew Morton <akpm@linux-foundation.org> writes: >> >> > From: Andreas Schwab <schwab@linux-m68k.org> >> > 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. >> >> 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. >> > > OK, but > > a) what effect does this problem have upon users of the kernel? get_idle_time returns the value in the wrong unit. > 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. -- Andreas Schwab, schwab@linux-m68k.org GPG Key fingerprint = 58CA 54C7 6D53 942B 1756 01D3 44D5 214B 8276 4ED5 "And now for something completely different." ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [resend PATCH for 3.2] procfs: do not confuse jiffies with cputime64_t 2011-12-22 0:14 ` Andreas Schwab @ 2011-12-22 0:19 ` Andrew Morton 2011-12-22 10:18 ` Andreas Schwab 0 siblings, 1 reply; 20+ messages in thread From: Andrew Morton @ 2011-12-22 0:19 UTC (permalink / raw) To: Andreas Schwab Cc: Michal Hocko, Arnd Bergmann, linux-kernel, Artem S. Tashkinov, Dave Jones, Alexey Dobriyan, Thomas Gleixner, Martin Schwidefsky On Thu, 22 Dec 2011 01:14:39 +0100 Andreas Schwab <schwab@linux-m68k.org> wrote: > > a) what effect does this problem have upon users of the kernel? > > get_idle_time returns 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? ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [resend PATCH for 3.2] procfs: do not confuse jiffies with cputime64_t 2011-12-22 0:19 ` Andrew Morton @ 2011-12-22 10:18 ` Andreas Schwab 0 siblings, 0 replies; 20+ messages in thread From: Andreas Schwab @ 2011-12-22 10:18 UTC (permalink / raw) To: Andrew Morton Cc: Michal Hocko, Arnd Bergmann, linux-kernel, Artem S. Tashkinov, Dave Jones, Alexey Dobriyan, Thomas Gleixner, Martin Schwidefsky Andrew Morton <akpm@linux-foundation.org> writes: > On Thu, 22 Dec 2011 01:14:39 +0100 > Andreas Schwab <schwab@linux-m68k.org> wrote: > >> > a) what effect does this problem have upon users of the kernel? >> >> get_idle_time returns 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? They use the wrong unit, off by the factor between jiffies and cputime. > 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. -- Andreas Schwab, schwab@linux-m68k.org GPG Key fingerprint = 58CA 54C7 6D53 942B 1756 01D3 44D5 214B 8276 4ED5 "And now for something completely different." ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [resend PATCH for 3.2] procfs: do not confuse jiffies with cputime64_t 2011-12-21 19:59 ` Andrew Morton 2011-12-21 23:50 ` Andreas Schwab @ 2011-12-21 23:55 ` Andreas Schwab 2011-12-21 23:59 ` Andrew Morton 2011-12-22 9:55 ` Martin Schwidefsky 2011-12-22 10:19 ` Andreas Schwab 3 siblings, 1 reply; 20+ messages in thread From: Andreas Schwab @ 2011-12-21 23:55 UTC (permalink / raw) To: Andrew Morton Cc: Michal Hocko, Arnd Bergmann, linux-kernel, Artem S. Tashkinov, Dave Jones, Alexey Dobriyan, Thomas Gleixner, Martin Schwidefsky Andrew Morton <akpm@linux-foundation.org> writes: > 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. 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. -- Andreas Schwab, schwab@linux-m68k.org GPG Key fingerprint = 58CA 54C7 6D53 942B 1756 01D3 44D5 214B 8276 4ED5 "And now for something completely different." ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [resend PATCH for 3.2] procfs: do not confuse jiffies with cputime64_t 2011-12-21 23:55 ` Andreas Schwab @ 2011-12-21 23:59 ` Andrew Morton 2011-12-22 0:20 ` Andreas Schwab 0 siblings, 1 reply; 20+ messages in thread From: Andrew Morton @ 2011-12-21 23:59 UTC (permalink / raw) To: Andreas Schwab Cc: Michal Hocko, Arnd Bergmann, linux-kernel, Artem S. Tashkinov, Dave Jones, Alexey Dobriyan, Thomas Gleixner, Martin Schwidefsky On Thu, 22 Dec 2011 00:55:07 +0100 Andreas Schwab <schwab@linux-m68k.org> wrote: > Andrew Morton <akpm@linux-foundation.org> writes: > > > 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. > > 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) ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [resend PATCH for 3.2] procfs: do not confuse jiffies with cputime64_t 2011-12-21 23:59 ` Andrew Morton @ 2011-12-22 0:20 ` Andreas Schwab 0 siblings, 0 replies; 20+ messages in thread From: Andreas Schwab @ 2011-12-22 0:20 UTC (permalink / raw) To: Andrew Morton Cc: Michal Hocko, Arnd Bergmann, linux-kernel, Artem S. Tashkinov, Dave Jones, Alexey Dobriyan, Thomas Gleixner, Martin Schwidefsky Andrew Morton <akpm@linux-foundation.org> writes: > On Thu, 22 Dec 2011 00:55:07 +0100 > Andreas Schwab <schwab@linux-m68k.org> wrote: > >> Andrew Morton <akpm@linux-foundation.org> writes: >> >> > 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. >> >> 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) 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. -- Andreas Schwab, schwab@linux-m68k.org GPG Key fingerprint = 58CA 54C7 6D53 942B 1756 01D3 44D5 214B 8276 4ED5 "And now for something completely different." ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [resend PATCH for 3.2] procfs: do not confuse jiffies with cputime64_t 2011-12-21 19:59 ` Andrew Morton 2011-12-21 23:50 ` Andreas Schwab 2011-12-21 23:55 ` Andreas Schwab @ 2011-12-22 9:55 ` Martin Schwidefsky 2011-12-22 10:19 ` Andreas Schwab 3 siblings, 0 replies; 20+ messages in thread From: Martin Schwidefsky @ 2011-12-22 9:55 UTC (permalink / raw) To: Andrew Morton Cc: Michal Hocko, Arnd Bergmann, linux-kernel, Artem S. Tashkinov, Dave Jones, Alexey Dobriyan, Thomas Gleixner, Andreas Schwab On Wed, 21 Dec 2011 11:59:19 -0800 Andrew Morton <akpm@linux-foundation.org> wrote: > On Wed, 21 Dec 2011 11:03:34 +0100 > Michal Hocko <mhocko@suse.cz> 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: > > --- > > >From 1fca39b21f3b344c90c30d98db6dcdcdc6815797 Mon Sep 17 00:00:00 2001 > > From: Andreas Schwab <schwab@linux-m68k.org> > > 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. > > 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. 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. > (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. It would be cleaner to do an explicit cast to cputime64_t for all of the usecs_to_cputime64() definitions. -- blue skies, Martin. "Reality continues to ruin my life." - Calvin. ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [resend PATCH for 3.2] procfs: do not confuse jiffies with cputime64_t 2011-12-21 19:59 ` Andrew Morton ` (2 preceding siblings ...) 2011-12-22 9:55 ` Martin Schwidefsky @ 2011-12-22 10:19 ` Andreas Schwab 3 siblings, 0 replies; 20+ messages in thread From: Andreas Schwab @ 2011-12-22 10:19 UTC (permalink / raw) To: Andrew Morton Cc: Michal Hocko, Arnd Bergmann, linux-kernel, Artem S. Tashkinov, Dave Jones, Alexey Dobriyan, Thomas Gleixner, Martin Schwidefsky Andrew Morton <akpm@linux-foundation.org> writes: > From: Andreas Schwab <schwab@linux-m68k.org> > 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. 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. Signed-off-by: Andreas Schwab <schwab@linux-m68k.org> Andreas. -- Andreas Schwab, schwab@linux-m68k.org GPG Key fingerprint = 58CA 54C7 6D53 942B 1756 01D3 44D5 214B 8276 4ED5 "And now for something completely different." ^ permalink raw reply [flat|nested] 20+ messages in thread
end of thread, other threads:[~2011-12-22 10:19 UTC | newest]
Thread overview: 20+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <m28vmi7ip3.fsf@igel.home>
2011-12-12 8:16 ` [PATCH] procfs: do not confuse jiffies with cputime64_t Michal Hocko
2011-12-12 9:22 ` Arnd Bergmann
2011-12-12 10:17 ` Andreas Schwab
2011-12-12 11:51 ` [PATCH v2] " Andreas Schwab
2011-12-12 12:43 ` Michal Hocko
2011-12-12 13:07 ` [PATCH v3] " Andreas Schwab
2011-12-12 13:12 ` Michal Hocko
2011-12-21 10:03 ` [resend PATCH for 3.2] " Michal Hocko
2011-12-21 19:43 ` Andrew Morton
2011-12-21 19:59 ` Andrew Morton
2011-12-21 23:50 ` Andreas Schwab
2011-12-21 23:56 ` Andrew Morton
2011-12-22 0:14 ` Andreas Schwab
2011-12-22 0:19 ` Andrew Morton
2011-12-22 10:18 ` Andreas Schwab
2011-12-21 23:55 ` Andreas Schwab
2011-12-21 23:59 ` Andrew Morton
2011-12-22 0:20 ` Andreas Schwab
2011-12-22 9:55 ` Martin Schwidefsky
2011-12-22 10:19 ` Andreas Schwab
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).