Re: [PATCH] Provide u64 version of jiffies_to_usecs() in kernel/tsacct.c

Re: [PATCH] Provide u64 version of jiffies_to_usecs() in kernel/tsacct.c

[Andrew Morton]

On Fri, 28 Dec 2007 13:26:07 -0800 (PST) Jonathan Lim <jlim@sgi.com> wrote:

> It's possible that the values used in and returned from jiffies_to_usecs() are
> incorrect because of truncation when variables of type u64 are involved.  So a
> function specific to that type is used instead.
> 
> Diff'd against: linux/kernel/git/stable/linux-2.6.23.y.git
> 
> Signed-off-by: Jonathan Lim <jlim@sgi.com>
> 
> --- a/kernel/tsacct.c	2007-12-28 11:58:05.182065029 -0800
> +++ b/kernel/tsacct.c	2007-12-28 11:57:37.949013675 -0800
> @@ -71,6 +71,17 @@ void bacct_add_tsk(struct taskstats *sta
>  
>  #ifdef CONFIG_TASK_XACCT
>  
> +static inline u64 jiffies_to_usecs_u64(const u64 j)
> +{
> +#if HZ <= USEC_PER_SEC && !(USEC_PER_SEC % HZ)
> +	return (USEC_PER_SEC / HZ) * j;
> +#elif HZ > USEC_PER_SEC && !(HZ % USEC_PER_SEC)
> +	return (j + (HZ / USEC_PER_SEC) - 1)/(HZ / USEC_PER_SEC);
> +#else
> +	return (j * USEC_PER_SEC) / HZ;
> +#endif
> +}
> +
>  #define KB 1024
>  #define MB (1024*KB)
>  /*
> @@ -81,8 +92,8 @@ void xacct_add_tsk(struct taskstats *sta
>  	struct mm_struct *mm;
>  
>  	/* convert pages-jiffies to Mbyte-usec */
> -	stats->coremem = jiffies_to_usecs(p->acct_rss_mem1) * PAGE_SIZE / MB;
> -	stats->virtmem = jiffies_to_usecs(p->acct_vm_mem1) * PAGE_SIZE / MB;
> +	stats->coremem = jiffies_to_usecs_u64(p->acct_rss_mem1) * PAGE_SIZE / MB;
> +	stats->virtmem = jiffies_to_usecs_u64(p->acct_vm_mem1) * PAGE_SIZE / MB;
>  	mm = get_task_mm(p);
>  	if (mm) {
>  		/* adjust to KB unit */

Fair enough.  But I guess that new function should be a kernel-wide thing
because surely other users will turn up.

Peter has been working on the accuracy of some of these conversion
functions and might need to know about this change?

Re: [PATCH] Provide u64 version of jiffies_to_usecs() in kernel/tsacct.c

[H. Peter Anvin]

Andrew Morton wrote:
> On Fri, 28 Dec 2007 13:26:07 -0800 (PST) Jonathan Lim <jlim@sgi.com> wrote:
> 
>> It's possible that the values used in and returned from jiffies_to_usecs() are
>> incorrect because of truncation when variables of type u64 are involved.  So a
>> function specific to that type is used instead.
>>
>> Diff'd against: linux/kernel/git/stable/linux-2.6.23.y.git
>>
>> Signed-off-by: Jonathan Lim <jlim@sgi.com>
>>
>> --- a/kernel/tsacct.c	2007-12-28 11:58:05.182065029 -0800
>> +++ b/kernel/tsacct.c	2007-12-28 11:57:37.949013675 -0800
>> @@ -71,6 +71,17 @@ void bacct_add_tsk(struct taskstats *sta
>>  
>>  #ifdef CONFIG_TASK_XACCT
>>  
>> +static inline u64 jiffies_to_usecs_u64(const u64 j)
>> +{
>> +#if HZ <= USEC_PER_SEC && !(USEC_PER_SEC % HZ)
>> +	return (USEC_PER_SEC / HZ) * j;
>> +#elif HZ > USEC_PER_SEC && !(HZ % USEC_PER_SEC)
>> +	return (j + (HZ / USEC_PER_SEC) - 1)/(HZ / USEC_PER_SEC);
>> +#else
>> +	return (j * USEC_PER_SEC) / HZ;
>> +#endif
>> +}
>> +
>>  #define KB 1024
>>  #define MB (1024*KB)
>>  /*
>> @@ -81,8 +92,8 @@ void xacct_add_tsk(struct taskstats *sta
>>  	struct mm_struct *mm;
>>  
>>  	/* convert pages-jiffies to Mbyte-usec */
>> -	stats->coremem = jiffies_to_usecs(p->acct_rss_mem1) * PAGE_SIZE / MB;
>> -	stats->virtmem = jiffies_to_usecs(p->acct_vm_mem1) * PAGE_SIZE / MB;
>> +	stats->coremem = jiffies_to_usecs_u64(p->acct_rss_mem1) * PAGE_SIZE / MB;
>> +	stats->virtmem = jiffies_to_usecs_u64(p->acct_vm_mem1) * PAGE_SIZE / MB;
>>  	mm = get_task_mm(p);
>>  	if (mm) {
>>  		/* adjust to KB unit */
> 
> Fair enough.  But I guess that new function should be a kernel-wide thing
> because surely other users will turn up.
> 
> Peter has been working on the accuracy of some of these conversion
> functions and might need to know about this change?

Yes, the function should be coded using the new #defines produced by 
timeconst.h; that way you end up avoiding a possible overflow in the 
multiplication.

I believe all three cases can be folded, then, to:

	return (j*HZ_TO_USEC_NUM + HZ_TO_USEC_DEN-1) / HZ_TO_USEC_DEN;

I would also like to observe that the roundoff behaviour of the function 
  above is inconsistent; in case 2 it will round up, but in case 3 it 
will round down.  The line proposed above has round up behaviour.

	-hpa

Re: [PATCH] Provide u64 version of jiffies_to_usecs() in kernel/tsacct.c

[Jonathan Lim]

On Wed Jan  2 16:36:47 2008, hpa@zytor.com wrote:
> 
> Andrew Morton wrote:
> > On Fri, 28 Dec 2007 13:26:07 -0800 (PST) Jonathan Lim <jlim@sgi.com> wrote:
> > 
> >> It's possible that the values used in and returned from jiffies_to_usecs()
> >> are incorrect because of truncation when variables of type u64 are
> >> involved.  So a function specific to that type is used instead.
> >>
> >> Diff'd against: linux/kernel/git/stable/linux-2.6.23.y.git
> >>
> >> Signed-off-by: Jonathan Lim <jlim@sgi.com>
> >>
> >> --- a/kernel/tsacct.c	2007-12-28 11:58:05.182065029 -0800
> >> +++ b/kernel/tsacct.c	2007-12-28 11:57:37.949013675 -0800
> >> @@ -71,6 +71,17 @@ void bacct_add_tsk(struct taskstats *sta
> >>  
> >>  #ifdef CONFIG_TASK_XACCT
> >>  
> >> +static inline u64 jiffies_to_usecs_u64(const u64 j)
> >> +{
> >> +#if HZ <= USEC_PER_SEC && !(USEC_PER_SEC % HZ)
> >> +	return (USEC_PER_SEC / HZ) * j;
> >> +#elif HZ > USEC_PER_SEC && !(HZ % USEC_PER_SEC)
> >> +	return (j + (HZ / USEC_PER_SEC) - 1)/(HZ / USEC_PER_SEC);
> >> +#else
> >> +	return (j * USEC_PER_SEC) / HZ;
> >> +#endif
> >> +}
> >> +
> >>  #define KB 1024
> >>  #define MB (1024*KB)
> >>  /*
> >> @@ -81,8 +92,8 @@ void xacct_add_tsk(struct taskstats *sta
> >>  	struct mm_struct *mm;
> >>  
> >>  	/* convert pages-jiffies to Mbyte-usec */
> >> -	stats->coremem = jiffies_to_usecs(p->acct_rss_mem1) * PAGE_SIZE / MB;
> >> -	stats->virtmem = jiffies_to_usecs(p->acct_vm_mem1) * PAGE_SIZE / MB;
> >> +	stats->coremem = jiffies_to_usecs_u64(p->acct_rss_mem1) * PAGE_SIZE / MB;
> >> +	stats->virtmem = jiffies_to_usecs_u64(p->acct_vm_mem1) * PAGE_SIZE / MB;
> >>  	mm = get_task_mm(p);
> >>  	if (mm) {
> >>  		/* adjust to KB unit */
> > 
> > Fair enough.  But I guess that new function should be a kernel-wide thing
> > because surely other users will turn up.
> > 
> > Peter has been working on the accuracy of some of these conversion
> > functions and might need to know about this change?
> 
> Yes, the function should be coded using the new #defines produced by 
> timeconst.h; that way you end up avoiding a possible overflow in the 
> multiplication.
> 
> I believe all three cases can be folded, then, to:
> 
> 	return (j*HZ_TO_USEC_NUM + HZ_TO_USEC_DEN-1) / HZ_TO_USEC_DEN;
> 
> I would also like to observe that the roundoff behaviour of the function 
>   above is inconsistent; in case 2 it will round up, but in case 3 it 
> will round down.  The line proposed above has round up behaviour.
> 
> 	-hpa

Peter,

Would you be willing to include the u64 function as part of your patch to make
it available kernel-wide?  It just needs:

  u64 inline jiffies_to_usecs_u64(const u64 j)

and for the symbol to be exported.  Thanks.

Jonathan

Re: [PATCH] Provide u64 version of jiffies_to_usecs() in kernel/tsacct.c

[H. Peter Anvin]

Jonathan Lim wrote:
> 
> Peter,
> 
> Would you be willing to include the u64 function as part of your patch to make
> it available kernel-wide?  It just needs:
> 
>   u64 inline jiffies_to_usecs_u64(const u64 j)
> 
> and for the symbol to be exported.  Thanks.
> 

It should be a separate patch (new functionality versus change of 
implementation); I'd just do a small patch on top of mine.

	-hpa

Re: [PATCH] Provide u64 version of jiffies_to_usecs() in kernel/tsacct.c

[Jonathan Lim]

It's possible that the values used in and returned from jiffies_to_usecs() are
incorrect because of truncation when variables of type u64 are involved.  So a
function specific to that type is used instead.

This version implements a correction to jiffies_64_to_usecs() based on feedback
from Randy Dunlap and Roman Zippel.

Diff'd against: linux/kernel/git/torvalds/linux-2.6.git

Signed-off-by: Jonathan Lim <jlim@sgi.com>

--- a/include/linux/jiffies.h	Thu Feb 14 18:04:14 PST 2008
+++ b/include/linux/jiffies.h	Thu Feb 14 18:07:17 PST 2008
@@ -42,7 +42,7 @@
 /* LATCH is used in the interval timer and ftape setup. */
 #define LATCH  ((CLOCK_TICK_RATE + HZ/2) / HZ)	/* For divider */
 
-/* Suppose we want to devide two numbers NOM and DEN: NOM/DEN, then we can
+/* Suppose we want to divide two numbers NOM and DEN: NOM/DEN, then we can
  * improve accuracy by shifting LSH bits, hence calculating:
  *     (NOM << LSH) / DEN
  * This however means trouble for large NOM, because (NOM << LSH) may no
@@ -204,7 +204,7 @@ extern unsigned long preset_lpj;
  * operator if the result is a long long AND at least one of the
  * operands is cast to long long (usually just prior to the "*" so as
  * not to confuse it into thinking it really has a 64-bit operand,
- * which, buy the way, it can do, but it takes more code and at least 2
+ * which, by the way, it can do, but it takes more code and at least 2
  * mpys).
 
  * We also need to be aware that one second in nanoseconds is only a
@@ -269,6 +269,7 @@ extern unsigned long preset_lpj;
  */
 extern unsigned int jiffies_to_msecs(const unsigned long j);
 extern unsigned int jiffies_to_usecs(const unsigned long j);
+extern u64 jiffies_64_to_usecs(const u64 j);
 extern unsigned long msecs_to_jiffies(const unsigned int m);
 extern unsigned long usecs_to_jiffies(const unsigned int u);
 extern unsigned long timespec_to_jiffies(const struct timespec *value);
--- a/kernel/time.c	Thu Feb 14 18:05:12 PST 2008
+++ b/kernel/time.c	Fri Apr 18 14:44:54 PDT 2008
@@ -38,6 +38,7 @@
 
 #include <asm/uaccess.h>
 #include <asm/unistd.h>
+#include <asm/div64.h>
 
 #include "timeconst.h"
 
@@ -267,6 +268,16 @@ unsigned int inline jiffies_to_usecs(con
 #endif
 }
 EXPORT_SYMBOL(jiffies_to_usecs);
+
+u64 jiffies_64_to_usecs(const u64 j)
+{
+# if BITS_PER_LONG == 32
+	return ((u64)HZ_TO_USEC_MUL32 * j) >> HZ_TO_USEC_SHR32;
+# else
+	return div64_64(j*HZ_TO_USEC_NUM + HZ_TO_USEC_DEN-1, HZ_TO_USEC_DEN);
+# endif
+}
+EXPORT_SYMBOL(jiffies_64_to_usecs);
 
 /**
  * timespec_trunc - Truncate timespec to a granularity
--- a/kernel/tsacct.c	Thu Feb 14 18:06:17 PST 2008
+++ b/kernel/tsacct.c	Thu Feb 14 18:08:47 PST 2008
@@ -85,8 +85,8 @@ void xacct_add_tsk(struct taskstats *sta
 	struct mm_struct *mm;
 
 	/* convert pages-jiffies to Mbyte-usec */
-	stats->coremem = jiffies_to_usecs(p->acct_rss_mem1) * PAGE_SIZE / MB;
-	stats->virtmem = jiffies_to_usecs(p->acct_vm_mem1) * PAGE_SIZE / MB;
+	stats->coremem = jiffies_64_to_usecs(p->acct_rss_mem1) * PAGE_SIZE / MB;
+	stats->virtmem = jiffies_64_to_usecs(p->acct_vm_mem1) * PAGE_SIZE / MB;
 	mm = get_task_mm(p);
 	if (mm) {
 		/* adjust to KB unit */

Re: [PATCH] Provide u64 version of jiffies_to_usecs() in kernel/tsacct.c

[Roman Zippel]

Hi,

On Monday 25. February 2008, Jonathan Lim wrote:

> +u64 jiffies_64_to_usecs(const u64 j)
> +{
> +	return div64_64(j*HZ_TO_USEC_NUM + HZ_TO_USEC_DEN-1, HZ_TO_USEC_DEN);
> +}
> +EXPORT_SYMBOL(jiffies_64_to_usecs);

Please also adapt the "BITS_PER_LONG == 32" part from jiffies_to_usecs(), it 
should provide enough resolution to avoid the div64_64() here.

bye, Roman

Re: [PATCH] Provide u64 version of jiffies_to_usecs() in kernel/tsacct.c

[Jonathan Lim]

It's possible that the values used in and returned from jiffies_to_usecs() are
incorrect because of truncation when variables of type u64 are involved.  So a
function specific to that type is used instead.

This version implements a correction to jiffies_64_to_usecs() based on feedback
from Randy Dunlap.

Diff'd against: linux/kernel/git/torvalds/linux-2.6.git

Signed-off-by: Jonathan Lim <jlim@sgi.com>

--- a/include/linux/jiffies.h	Thu Feb 14 18:04:14 PST 2008
+++ b/include/linux/jiffies.h	Thu Feb 14 18:07:17 PST 2008
@@ -42,7 +42,7 @@
 /* LATCH is used in the interval timer and ftape setup. */
 #define LATCH  ((CLOCK_TICK_RATE + HZ/2) / HZ)	/* For divider */
 
-/* Suppose we want to devide two numbers NOM and DEN: NOM/DEN, then we can
+/* Suppose we want to divide two numbers NOM and DEN: NOM/DEN, then we can
  * improve accuracy by shifting LSH bits, hence calculating:
  *     (NOM << LSH) / DEN
  * This however means trouble for large NOM, because (NOM << LSH) may no
@@ -204,7 +204,7 @@ extern unsigned long preset_lpj;
  * operator if the result is a long long AND at least one of the
  * operands is cast to long long (usually just prior to the "*" so as
  * not to confuse it into thinking it really has a 64-bit operand,
- * which, buy the way, it can do, but it takes more code and at least 2
+ * which, by the way, it can do, but it takes more code and at least 2
  * mpys).
 
  * We also need to be aware that one second in nanoseconds is only a
@@ -269,6 +269,7 @@ extern unsigned long preset_lpj;
  */
 extern unsigned int jiffies_to_msecs(const unsigned long j);
 extern unsigned int jiffies_to_usecs(const unsigned long j);
+extern u64 jiffies_64_to_usecs(const u64 j);
 extern unsigned long msecs_to_jiffies(const unsigned int m);
 extern unsigned long usecs_to_jiffies(const unsigned int u);
 extern unsigned long timespec_to_jiffies(const struct timespec *value);
--- a/kernel/time.c	Thu Feb 14 18:05:12 PST 2008
+++ b/kernel/time.c	Mon Feb 25 14:22:27 PST 2008
@@ -38,6 +38,7 @@
 
 #include <asm/uaccess.h>
 #include <asm/unistd.h>
+#include <asm/div64.h>
 
 #include "timeconst.h"
 
@@ -267,6 +268,12 @@ unsigned int inline jiffies_to_usecs(con
 #endif
 }
 EXPORT_SYMBOL(jiffies_to_usecs);
+
+u64 jiffies_64_to_usecs(const u64 j)
+{
+	return div64_64(j*HZ_TO_USEC_NUM + HZ_TO_USEC_DEN-1, HZ_TO_USEC_DEN);
+}
+EXPORT_SYMBOL(jiffies_64_to_usecs);
 
 /**
  * timespec_trunc - Truncate timespec to a granularity
--- a/kernel/tsacct.c	Thu Feb 14 18:06:17 PST 2008
+++ b/kernel/tsacct.c	Thu Feb 14 18:08:47 PST 2008
@@ -85,8 +85,8 @@ void xacct_add_tsk(struct taskstats *sta
 	struct mm_struct *mm;
 
 	/* convert pages-jiffies to Mbyte-usec */
-	stats->coremem = jiffies_to_usecs(p->acct_rss_mem1) * PAGE_SIZE / MB;
-	stats->virtmem = jiffies_to_usecs(p->acct_vm_mem1) * PAGE_SIZE / MB;
+	stats->coremem = jiffies_64_to_usecs(p->acct_rss_mem1) * PAGE_SIZE / MB;
+	stats->virtmem = jiffies_64_to_usecs(p->acct_vm_mem1) * PAGE_SIZE / MB;
 	mm = get_task_mm(p);
 	if (mm) {
 		/* adjust to KB unit */

Re: [PATCH] Provide u64 version of jiffies_to_usecs() in kernel/tsacct.c

[Randy Dunlap]

Jonathan Lim wrote:
> It's possible that the values used in and returned from jiffies_to_usecs() are
> incorrect because of truncation when variables of type u64 are involved.  So a
> function specific to that type is used instead.
> 
> This version implements a correction to jiffies_64_to_usecs() based on feedback
> from Randy Dunlap.
> 
> Diff'd against: linux/kernel/git/torvalds/linux-2.6.git
> 
> Signed-off-by: Jonathan Lim <jlim@sgi.com>
> 
> --- a/include/linux/jiffies.h	Thu Feb 14 18:04:14 PST 2008
> +++ b/include/linux/jiffies.h	Thu Feb 14 18:07:17 PST 2008
> @@ -42,7 +42,7 @@
>  /* LATCH is used in the interval timer and ftape setup. */
>  #define LATCH  ((CLOCK_TICK_RATE + HZ/2) / HZ)	/* For divider */
>  
> -/* Suppose we want to devide two numbers NOM and DEN: NOM/DEN, then we can
> +/* Suppose we want to divide two numbers NOM and DEN: NOM/DEN, then we can
>   * improve accuracy by shifting LSH bits, hence calculating:
>   *     (NOM << LSH) / DEN
>   * This however means trouble for large NOM, because (NOM << LSH) may no
> @@ -204,7 +204,7 @@ extern unsigned long preset_lpj;
>   * operator if the result is a long long AND at least one of the
>   * operands is cast to long long (usually just prior to the "*" so as
>   * not to confuse it into thinking it really has a 64-bit operand,
> - * which, buy the way, it can do, but it takes more code and at least 2
> + * which, by the way, it can do, but it takes more code and at least 2
>   * mpys).
>  
>   * We also need to be aware that one second in nanoseconds is only a
> @@ -269,6 +269,7 @@ extern unsigned long preset_lpj;
>   */
>  extern unsigned int jiffies_to_msecs(const unsigned long j);
>  extern unsigned int jiffies_to_usecs(const unsigned long j);
> +extern u64 jiffies_64_to_usecs(const u64 j);
>  extern unsigned long msecs_to_jiffies(const unsigned int m);
>  extern unsigned long usecs_to_jiffies(const unsigned int u);
>  extern unsigned long timespec_to_jiffies(const struct timespec *value);
> --- a/kernel/time.c	Thu Feb 14 18:05:12 PST 2008
> +++ b/kernel/time.c	Tue Feb 19 17:00:11 PST 2008

kernel/time.c needs:
#include <asm/div64.h>

After that, it's
Acked-by: Randy Dunlap <randy.dunlap@oracle.com>

> @@ -268,6 +268,12 @@ unsigned int inline jiffies_to_usecs(con
>  }
>  EXPORT_SYMBOL(jiffies_to_usecs);
>  
> +u64 jiffies_64_to_usecs(const u64 j)
> +{
> +	return div64_64(j*HZ_TO_USEC_NUM + HZ_TO_USEC_DEN-1, HZ_TO_USEC_DEN);
> +}
> +EXPORT_SYMBOL(jiffies_64_to_usecs);
> +
>  /**
>   * timespec_trunc - Truncate timespec to a granularity
>   * @t: Timespec
> --- a/kernel/tsacct.c	Thu Feb 14 18:06:17 PST 2008
> +++ b/kernel/tsacct.c	Thu Feb 14 18:08:47 PST 2008
> @@ -85,8 +85,8 @@ void xacct_add_tsk(struct taskstats *sta
>  	struct mm_struct *mm;
>  
>  	/* convert pages-jiffies to Mbyte-usec */
> -	stats->coremem = jiffies_to_usecs(p->acct_rss_mem1) * PAGE_SIZE / MB;
> -	stats->virtmem = jiffies_to_usecs(p->acct_vm_mem1) * PAGE_SIZE / MB;
> +	stats->coremem = jiffies_64_to_usecs(p->acct_rss_mem1) * PAGE_SIZE / MB;
> +	stats->virtmem = jiffies_64_to_usecs(p->acct_vm_mem1) * PAGE_SIZE / MB;
>  	mm = get_task_mm(p);
>  	if (mm) {
>  		/* adjust to KB unit */


-- 
~Randy

Re: [PATCH] Provide u64 version of jiffies_to_usecs() in kernel/tsacct.c

[Jonathan Lim]

It's possible that the values used in and returned from jiffies_to_usecs() are
incorrect because of truncation when variables of type u64 are involved.  So a
function specific to that type is used instead.

This version implements a correction to jiffies_64_to_usecs() based on feedback
from Randy Dunlap.

Diff'd against: linux/kernel/git/torvalds/linux-2.6.git

Signed-off-by: Jonathan Lim <jlim@sgi.com>

--- a/include/linux/jiffies.h	Thu Feb 14 18:04:14 PST 2008
+++ b/include/linux/jiffies.h	Thu Feb 14 18:07:17 PST 2008
@@ -42,7 +42,7 @@
 /* LATCH is used in the interval timer and ftape setup. */
 #define LATCH  ((CLOCK_TICK_RATE + HZ/2) / HZ)	/* For divider */
 
-/* Suppose we want to devide two numbers NOM and DEN: NOM/DEN, then we can
+/* Suppose we want to divide two numbers NOM and DEN: NOM/DEN, then we can
  * improve accuracy by shifting LSH bits, hence calculating:
  *     (NOM << LSH) / DEN
  * This however means trouble for large NOM, because (NOM << LSH) may no
@@ -204,7 +204,7 @@ extern unsigned long preset_lpj;
  * operator if the result is a long long AND at least one of the
  * operands is cast to long long (usually just prior to the "*" so as
  * not to confuse it into thinking it really has a 64-bit operand,
- * which, buy the way, it can do, but it takes more code and at least 2
+ * which, by the way, it can do, but it takes more code and at least 2
  * mpys).
 
  * We also need to be aware that one second in nanoseconds is only a
@@ -269,6 +269,7 @@ extern unsigned long preset_lpj;
  */
 extern unsigned int jiffies_to_msecs(const unsigned long j);
 extern unsigned int jiffies_to_usecs(const unsigned long j);
+extern u64 jiffies_64_to_usecs(const u64 j);
 extern unsigned long msecs_to_jiffies(const unsigned int m);
 extern unsigned long usecs_to_jiffies(const unsigned int u);
 extern unsigned long timespec_to_jiffies(const struct timespec *value);
--- a/kernel/time.c	Thu Feb 14 18:05:12 PST 2008
+++ b/kernel/time.c	Tue Feb 19 17:00:11 PST 2008
@@ -268,6 +268,12 @@ unsigned int inline jiffies_to_usecs(con
 }
 EXPORT_SYMBOL(jiffies_to_usecs);
 
+u64 jiffies_64_to_usecs(const u64 j)
+{
+	return div64_64(j*HZ_TO_USEC_NUM + HZ_TO_USEC_DEN-1, HZ_TO_USEC_DEN);
+}
+EXPORT_SYMBOL(jiffies_64_to_usecs);
+
 /**
  * timespec_trunc - Truncate timespec to a granularity
  * @t: Timespec
--- a/kernel/tsacct.c	Thu Feb 14 18:06:17 PST 2008
+++ b/kernel/tsacct.c	Thu Feb 14 18:08:47 PST 2008
@@ -85,8 +85,8 @@ void xacct_add_tsk(struct taskstats *sta
 	struct mm_struct *mm;
 
 	/* convert pages-jiffies to Mbyte-usec */
-	stats->coremem = jiffies_to_usecs(p->acct_rss_mem1) * PAGE_SIZE / MB;
-	stats->virtmem = jiffies_to_usecs(p->acct_vm_mem1) * PAGE_SIZE / MB;
+	stats->coremem = jiffies_64_to_usecs(p->acct_rss_mem1) * PAGE_SIZE / MB;
+	stats->virtmem = jiffies_64_to_usecs(p->acct_vm_mem1) * PAGE_SIZE / MB;
 	mm = get_task_mm(p);
 	if (mm) {
 		/* adjust to KB unit */

Re: [PATCH] Provide u64 version of jiffies_to_usecs() in kernel/tsacct.c

[Randy Dunlap]

On Tue, 19 Feb 2008 12:52:46 -0800 (PST) Jonathan Lim wrote:

> It's possible that the values used in and returned from jiffies_to_usecs() are
> incorrect because of truncation when variables of type u64 are involved.  So a
> function specific to that type is used instead.
> 
> Diff'd against: linux/kernel/git/torvalds/linux-2.6.git
> 
> Signed-off-by: Jonathan Lim <jlim@sgi.com>
> 
> --- a/include/linux/jiffies.h	Thu Feb 14 18:04:14 PST 2008
> +++ b/include/linux/jiffies.h	Thu Feb 14 18:07:17 PST 2008
> @@ -42,7 +42,7 @@
>  /* LATCH is used in the interval timer and ftape setup. */
>  #define LATCH  ((CLOCK_TICK_RATE + HZ/2) / HZ)	/* For divider */
>  
> -/* Suppose we want to devide two numbers NOM and DEN: NOM/DEN, then we can
> +/* Suppose we want to divide two numbers NOM and DEN: NOM/DEN, then we can
>   * improve accuracy by shifting LSH bits, hence calculating:
>   *     (NOM << LSH) / DEN
>   * This however means trouble for large NOM, because (NOM << LSH) may no
> @@ -204,7 +204,7 @@ extern unsigned long preset_lpj;
>   * operator if the result is a long long AND at least one of the
>   * operands is cast to long long (usually just prior to the "*" so as
>   * not to confuse it into thinking it really has a 64-bit operand,
> - * which, buy the way, it can do, but it takes more code and at least 2
> + * which, by the way, it can do, but it takes more code and at least 2
>   * mpys).
>  
>   * We also need to be aware that one second in nanoseconds is only a
> @@ -269,6 +269,7 @@ extern unsigned long preset_lpj;
>   */
>  extern unsigned int jiffies_to_msecs(const unsigned long j);
>  extern unsigned int jiffies_to_usecs(const unsigned long j);
> +extern u64 jiffies_64_to_usecs(const u64 j);
>  extern unsigned long msecs_to_jiffies(const unsigned int m);
>  extern unsigned long usecs_to_jiffies(const unsigned int u);
>  extern unsigned long timespec_to_jiffies(const struct timespec *value);
> --- a/kernel/time.c	Thu Feb 14 18:05:12 PST 2008
> +++ b/kernel/time.c	Tue Feb 19 12:44:03 PST 2008
> @@ -268,6 +268,14 @@ unsigned int inline jiffies_to_usecs(con
>  }
>  EXPORT_SYMBOL(jiffies_to_usecs);
>  
> +u64 inline jiffies_64_to_usecs(const u64 j)
> +{
> +	u64 tmp = j*HZ_TO_USEC_NUM + HZ_TO_USEC_DEN-1;
> +	do_div(tmp, HZ_TO_USEC_DEN);

do_div() is:
 * The semantics of do_div() are:
 *
 * uint32_t do_div(uint64_t *n, uint32_t base)

Maybe you want div64_64().

> +	return tmp;
> +}
> +EXPORT_SYMBOL(jiffies_64_to_usecs);
> +
>  /**
>   * timespec_trunc - Truncate timespec to a granularity
>   * @t: Timespec
> --- a/kernel/tsacct.c	Thu Feb 14 18:06:17 PST 2008
> +++ b/kernel/tsacct.c	Thu Feb 14 18:08:47 PST 2008
> @@ -85,8 +85,8 @@ void xacct_add_tsk(struct taskstats *sta
>  	struct mm_struct *mm;
>  
>  	/* convert pages-jiffies to Mbyte-usec */
> -	stats->coremem = jiffies_to_usecs(p->acct_rss_mem1) * PAGE_SIZE / MB;
> -	stats->virtmem = jiffies_to_usecs(p->acct_vm_mem1) * PAGE_SIZE / MB;
> +	stats->coremem = jiffies_64_to_usecs(p->acct_rss_mem1) * PAGE_SIZE / MB;
> +	stats->virtmem = jiffies_64_to_usecs(p->acct_vm_mem1) * PAGE_SIZE / MB;
>  	mm = get_task_mm(p);
>  	if (mm) {
>  		/* adjust to KB unit */
> --


---
~Randy

Re: [PATCH] Provide u64 version of jiffies_to_usecs() in kernel/tsacct.c

[Jonathan Lim]

It's possible that the values used in and returned from jiffies_to_usecs() are
incorrect because of truncation when variables of type u64 are involved.  So a
function specific to that type is used instead.

Diff'd against: linux/kernel/git/torvalds/linux-2.6.git

Signed-off-by: Jonathan Lim <jlim@sgi.com>

--- a/include/linux/jiffies.h	Thu Feb 14 18:04:14 PST 2008
+++ b/include/linux/jiffies.h	Thu Feb 14 18:07:17 PST 2008
@@ -42,7 +42,7 @@
 /* LATCH is used in the interval timer and ftape setup. */
 #define LATCH  ((CLOCK_TICK_RATE + HZ/2) / HZ)	/* For divider */
 
-/* Suppose we want to devide two numbers NOM and DEN: NOM/DEN, then we can
+/* Suppose we want to divide two numbers NOM and DEN: NOM/DEN, then we can
  * improve accuracy by shifting LSH bits, hence calculating:
  *     (NOM << LSH) / DEN
  * This however means trouble for large NOM, because (NOM << LSH) may no
@@ -204,7 +204,7 @@ extern unsigned long preset_lpj;
  * operator if the result is a long long AND at least one of the
  * operands is cast to long long (usually just prior to the "*" so as
  * not to confuse it into thinking it really has a 64-bit operand,
- * which, buy the way, it can do, but it takes more code and at least 2
+ * which, by the way, it can do, but it takes more code and at least 2
  * mpys).
 
  * We also need to be aware that one second in nanoseconds is only a
@@ -269,6 +269,7 @@ extern unsigned long preset_lpj;
  */
 extern unsigned int jiffies_to_msecs(const unsigned long j);
 extern unsigned int jiffies_to_usecs(const unsigned long j);
+extern u64 jiffies_64_to_usecs(const u64 j);
 extern unsigned long msecs_to_jiffies(const unsigned int m);
 extern unsigned long usecs_to_jiffies(const unsigned int u);
 extern unsigned long timespec_to_jiffies(const struct timespec *value);
--- a/kernel/time.c	Thu Feb 14 18:05:12 PST 2008
+++ b/kernel/time.c	Tue Feb 19 12:44:03 PST 2008
@@ -268,6 +268,14 @@ unsigned int inline jiffies_to_usecs(con
 }
 EXPORT_SYMBOL(jiffies_to_usecs);
 
+u64 inline jiffies_64_to_usecs(const u64 j)
+{
+	u64 tmp = j*HZ_TO_USEC_NUM + HZ_TO_USEC_DEN-1;
+	do_div(tmp, HZ_TO_USEC_DEN);
+	return tmp;
+}
+EXPORT_SYMBOL(jiffies_64_to_usecs);
+
 /**
  * timespec_trunc - Truncate timespec to a granularity
  * @t: Timespec
--- a/kernel/tsacct.c	Thu Feb 14 18:06:17 PST 2008
+++ b/kernel/tsacct.c	Thu Feb 14 18:08:47 PST 2008
@@ -85,8 +85,8 @@ void xacct_add_tsk(struct taskstats *sta
 	struct mm_struct *mm;
 
 	/* convert pages-jiffies to Mbyte-usec */
-	stats->coremem = jiffies_to_usecs(p->acct_rss_mem1) * PAGE_SIZE / MB;
-	stats->virtmem = jiffies_to_usecs(p->acct_vm_mem1) * PAGE_SIZE / MB;
+	stats->coremem = jiffies_64_to_usecs(p->acct_rss_mem1) * PAGE_SIZE / MB;
+	stats->virtmem = jiffies_64_to_usecs(p->acct_vm_mem1) * PAGE_SIZE / MB;
 	mm = get_task_mm(p);
 	if (mm) {
 		/* adjust to KB unit */

[PATCH] Provide u64 version of jiffies_to_usecs() in kernel/tsacct.c

[Jonathan Lim]

It's possible that the values used in and returned from jiffies_to_usecs() are
incorrect because of truncation when variables of type u64 are involved.  So a
function specific to that type is used instead.

Diff'd against: linux/kernel/git/torvalds/linux-2.6.git

Signed-off-by: Jonathan Lim <jlim@sgi.com>

--- a/include/linux/jiffies.h	Thu Feb 14 18:04:14 PST 2008
+++ b/include/linux/jiffies.h	Thu Feb 14 18:07:17 PST 2008
@@ -42,7 +42,7 @@
 /* LATCH is used in the interval timer and ftape setup. */
 #define LATCH  ((CLOCK_TICK_RATE + HZ/2) / HZ)	/* For divider */
 
-/* Suppose we want to devide two numbers NOM and DEN: NOM/DEN, then we can
+/* Suppose we want to divide two numbers NOM and DEN: NOM/DEN, then we can
  * improve accuracy by shifting LSH bits, hence calculating:
  *     (NOM << LSH) / DEN
  * This however means trouble for large NOM, because (NOM << LSH) may no
@@ -204,7 +204,7 @@ extern unsigned long preset_lpj;
  * operator if the result is a long long AND at least one of the
  * operands is cast to long long (usually just prior to the "*" so as
  * not to confuse it into thinking it really has a 64-bit operand,
- * which, buy the way, it can do, but it takes more code and at least 2
+ * which, by the way, it can do, but it takes more code and at least 2
  * mpys).
 
  * We also need to be aware that one second in nanoseconds is only a
@@ -269,6 +269,7 @@ extern unsigned long preset_lpj;
  */
 extern unsigned int jiffies_to_msecs(const unsigned long j);
 extern unsigned int jiffies_to_usecs(const unsigned long j);
+extern u64 jiffies_64_to_usecs(const u64 j);
 extern unsigned long msecs_to_jiffies(const unsigned int m);
 extern unsigned long usecs_to_jiffies(const unsigned int u);
 extern unsigned long timespec_to_jiffies(const struct timespec *value);
--- a/kernel/time.c	Thu Feb 14 18:05:12 PST 2008
+++ b/kernel/time.c	Thu Feb 14 18:08:08 PST 2008
@@ -268,6 +268,12 @@ unsigned int inline jiffies_to_usecs(con
 }
 EXPORT_SYMBOL(jiffies_to_usecs);
 
+u64 jiffies_64_to_usecs(const u64 j)
+{
+	return (j*HZ_TO_USEC_NUM + HZ_TO_USEC_DEN-1) / HZ_TO_USEC_DEN;
+}
+EXPORT_SYMBOL(jiffies_64_to_usecs);
+
 /**
  * timespec_trunc - Truncate timespec to a granularity
  * @t: Timespec
--- a/kernel/tsacct.c	Thu Feb 14 18:06:17 PST 2008
+++ b/kernel/tsacct.c	Thu Feb 14 18:08:47 PST 2008
@@ -85,8 +85,8 @@ void xacct_add_tsk(struct taskstats *sta
 	struct mm_struct *mm;
 
 	/* convert pages-jiffies to Mbyte-usec */
-	stats->coremem = jiffies_to_usecs(p->acct_rss_mem1) * PAGE_SIZE / MB;
-	stats->virtmem = jiffies_to_usecs(p->acct_vm_mem1) * PAGE_SIZE / MB;
+	stats->coremem = jiffies_64_to_usecs(p->acct_rss_mem1) * PAGE_SIZE / MB;
+	stats->virtmem = jiffies_64_to_usecs(p->acct_vm_mem1) * PAGE_SIZE / MB;
 	mm = get_task_mm(p);
 	if (mm) {
 		/* adjust to KB unit */

Re: [PATCH] Provide u64 version of jiffies_to_usecs() in kernel/tsacct.c

[Jonathan Lim]

It's possible that the values used in and returned from jiffies_to_usecs() are
incorrect because of truncation when variables of type u64 are involved.  So a
function specific to that type is used instead.

Updated from previous submission with feedback from Peter Anvin.

Diff'd against: linux/kernel/git/torvalds/linux-2.6.git

Signed-off-by: Jonathan Lim <jlim@sgi.com>

--- a/include/linux/jiffies.h	2008-01-07 16:09:17.737040981 -0800
+++ b/include/linux/jiffies.h	2008-01-07 16:09:17.693033003 -0800
@@ -36,7 +36,7 @@
 /* LATCH is used in the interval timer and ftape setup. */
 #define LATCH  ((CLOCK_TICK_RATE + HZ/2) / HZ)	/* For divider */
 
-/* Suppose we want to devide two numbers NOM and DEN: NOM/DEN, then we can
+/* Suppose we want to divide two numbers NOM and DEN: NOM/DEN, then we can
  * improve accuracy by shifting LSH bits, hence calculating:
  *     (NOM << LSH) / DEN
  * This however means trouble for large NOM, because (NOM << LSH) may no
@@ -198,7 +198,7 @@ extern unsigned long preset_lpj;
  * operator if the result is a long long AND at least one of the
  * operands is cast to long long (usually just prior to the "*" so as
  * not to confuse it into thinking it really has a 64-bit operand,
- * which, buy the way, it can do, but it takes more code and at least 2
+ * which, by the way, it can do, but it takes more code and at least 2
  * mpys).
 
  * We also need to be aware that one second in nanoseconds is only a
@@ -263,6 +263,7 @@ extern unsigned long preset_lpj;
  */
 extern unsigned int jiffies_to_msecs(const unsigned long j);
 extern unsigned int jiffies_to_usecs(const unsigned long j);
+extern u64 jiffies_64_to_usecs(const u64 j);
 extern unsigned long msecs_to_jiffies(const unsigned int m);
 extern unsigned long usecs_to_jiffies(const unsigned int u);
 extern unsigned long timespec_to_jiffies(const struct timespec *value);
--- a/kernel/time.c	2008-01-07 16:09:17.841059839 -0800
+++ b/kernel/time.c	2008-01-07 16:09:17.825056938 -0800
@@ -267,6 +267,12 @@ unsigned int inline jiffies_to_usecs(con
 }
 EXPORT_SYMBOL(jiffies_to_usecs);
 
+u64 inline jiffies_64_to_usecs(const u64 j)
+{
+	return (j*HZ_TO_USEC_NUM + HZ_TO_USEC_DEN-1) / HZ_TO_USEC_DEN;
+}
+EXPORT_SYMBOL(jiffies_64_to_usecs);
+
 /**
  * timespec_trunc - Truncate timespec to a granularity
  * @t: Timespec
--- a/kernel/tsacct.c	2008-01-07 16:09:17.833058389 -0800
+++ b/kernel/tsacct.c	2008-01-07 16:09:17.793051136 -0800
@@ -85,8 +85,8 @@ void xacct_add_tsk(struct taskstats *sta
 	struct mm_struct *mm;
 
 	/* convert pages-jiffies to Mbyte-usec */
-	stats->coremem = jiffies_to_usecs(p->acct_rss_mem1) * PAGE_SIZE / MB;
-	stats->virtmem = jiffies_to_usecs(p->acct_vm_mem1) * PAGE_SIZE / MB;
+	stats->coremem = jiffies_64_to_usecs(p->acct_rss_mem1) * PAGE_SIZE / MB;
+	stats->virtmem = jiffies_64_to_usecs(p->acct_vm_mem1) * PAGE_SIZE / MB;
 	mm = get_task_mm(p);
 	if (mm) {
 		/* adjust to KB unit */

Re: [PATCH] Provide u64 version of jiffies_to_usecs() in kernel/tsacct.c

[H. Peter Anvin]

Jonathan Lim wrote:
> It's possible that the values used in and returned from jiffies_to_usecs() are
> incorrect because of truncation when variables of type u64 are involved.  So a
> function specific to that type is used instead.

Much worse than that.  There are internal overflows in the conversions. 
See the patch I recently submitted to -mm.

	-hpa

[PATCH] Provide u64 version of jiffies_to_usecs() in kernel/tsacct.c

[Jonathan Lim]

It's possible that the values used in and returned from jiffies_to_usecs() are
incorrect because of truncation when variables of type u64 are involved.  So a
function specific to that type is used instead.

Diff'd against: linux/kernel/git/stable/linux-2.6.23.y.git

Signed-off-by: Jonathan Lim <jlim@sgi.com>

--- a/kernel/tsacct.c	2007-12-28 11:58:05.182065029 -0800
+++ b/kernel/tsacct.c	2007-12-28 11:57:37.949013675 -0800
@@ -71,6 +71,17 @@ void bacct_add_tsk(struct taskstats *sta
 
 #ifdef CONFIG_TASK_XACCT
 
+static inline u64 jiffies_to_usecs_u64(const u64 j)
+{
+#if HZ <= USEC_PER_SEC && !(USEC_PER_SEC % HZ)
+	return (USEC_PER_SEC / HZ) * j;
+#elif HZ > USEC_PER_SEC && !(HZ % USEC_PER_SEC)
+	return (j + (HZ / USEC_PER_SEC) - 1)/(HZ / USEC_PER_SEC);
+#else
+	return (j * USEC_PER_SEC) / HZ;
+#endif
+}
+
 #define KB 1024
 #define MB (1024*KB)
 /*
@@ -81,8 +92,8 @@ void xacct_add_tsk(struct taskstats *sta
 	struct mm_struct *mm;
 
 	/* convert pages-jiffies to Mbyte-usec */
-	stats->coremem = jiffies_to_usecs(p->acct_rss_mem1) * PAGE_SIZE / MB;
-	stats->virtmem = jiffies_to_usecs(p->acct_vm_mem1) * PAGE_SIZE / MB;
+	stats->coremem = jiffies_to_usecs_u64(p->acct_rss_mem1) * PAGE_SIZE / MB;
+	stats->virtmem = jiffies_to_usecs_u64(p->acct_vm_mem1) * PAGE_SIZE / MB;
 	mm = get_task_mm(p);
 	if (mm) {
 		/* adjust to KB unit */