Re: [PATCH] procfs: do not confuse jiffies with cputime64_t

Re: [PATCH] procfs: do not confuse jiffies with cputime64_t

[Michal Hocko]

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

Re: [PATCH] procfs: do not confuse jiffies with cputime64_t

[Arnd Bergmann]

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

Re: [PATCH] procfs: do not confuse jiffies with cputime64_t

[Andreas Schwab]

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."

[PATCH v2] procfs: do not confuse jiffies with cputime64_t

[Andreas Schwab]

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."

Re: [PATCH v2] procfs: do not confuse jiffies with cputime64_t

[Michal Hocko]

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

[PATCH v3] procfs: do not confuse jiffies with cputime64_t

[Andreas Schwab]

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."

Re: [PATCH v3] procfs: do not confuse jiffies with cputime64_t

[Michal Hocko]

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

[resend PATCH for 3.2] procfs: do not confuse jiffies with cputime64_t

[Michal Hocko]

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

Re: [resend PATCH for 3.2] procfs: do not confuse jiffies with cputime64_t

[Andrew Morton]

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).

Re: [resend PATCH for 3.2] procfs: do not confuse jiffies with cputime64_t

[Andrew Morton]

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.
_
.

Re: [resend PATCH for 3.2] procfs: do not confuse jiffies with cputime64_t

[Andreas Schwab]

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."

Re: [resend PATCH for 3.2] procfs: do not confuse jiffies with cputime64_t

[Andreas Schwab]

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."

Re: [resend PATCH for 3.2] procfs: do not confuse jiffies with cputime64_t

[Andrew Morton]

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?

Re: [resend PATCH for 3.2] procfs: do not confuse jiffies with cputime64_t

[Andrew Morton]

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)

Re: [resend PATCH for 3.2] procfs: do not confuse jiffies with cputime64_t

[Andreas Schwab]

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."

Re: [resend PATCH for 3.2] procfs: do not confuse jiffies with cputime64_t

[Andrew Morton]

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?

Re: [resend PATCH for 3.2] procfs: do not confuse jiffies with cputime64_t

[Andreas Schwab]

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."

Re: [resend PATCH for 3.2] procfs: do not confuse jiffies with cputime64_t

[Martin Schwidefsky]

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.

Re: [resend PATCH for 3.2] procfs: do not confuse jiffies with cputime64_t

[Andreas Schwab]

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."

Re: [resend PATCH for 3.2] procfs: do not confuse jiffies with cputime64_t

[Andreas Schwab]

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."