Messages in this thread | | | Date | Wed, 02 Jan 2008 16:36:47 -0800 | From | "H. Peter Anvin" <> | Subject | Re: [PATCH] Provide u64 version of jiffies_to_usecs() in kernel/tsacct.c |
| |
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
| |