* [PATCH] cputime: Fix timeval-->cputime conversion
@ 2016-01-28 7:02 zengtao
2016-01-28 8:22 ` Thomas Gleixner
0 siblings, 1 reply; 9+ messages in thread
From: zengtao @ 2016-01-28 7:02 UTC (permalink / raw)
To: tglx; +Cc: linux-kernel
The structure:
struct timeval {
__kernel_time_t tv_sec; /* seconds */
__kernel_suseconds_t tv_usec; /* microseconds */
};
both __kernel_time_t and __kernel_suseconds_t are short than u64
when it is 32bit platform, so force u64 conversion here.
Signed-off-by: zengtao <prime.zeng@huawei.com>
---
include/asm-generic/cputime_nsecs.h | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/include/asm-generic/cputime_nsecs.h b/include/asm-generic/cputime_nsecs.h
index 0419485..e2f7ff9 100644
--- a/include/asm-generic/cputime_nsecs.h
+++ b/include/asm-generic/cputime_nsecs.h
@@ -91,7 +91,8 @@ static inline void cputime_to_timespec(const cputime_t ct, struct timespec *val)
*/
static inline cputime_t timeval_to_cputime(const struct timeval *val)
{
- u64 ret = val->tv_sec * NSEC_PER_SEC + val->tv_usec * NSEC_PER_USEC;
+ u64 ret = (u64)val->tv_sec * NSEC_PER_SEC +
+ val->tv_usec * NSEC_PER_USEC;
return (__force cputime_t) ret;
}
static inline void cputime_to_timeval(const cputime_t ct, struct timeval *val)
--
1.9.1
^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH] cputime: Fix timeval-->cputime conversion
2016-01-28 7:02 [PATCH] cputime: Fix timeval-->cputime conversion zengtao
@ 2016-01-28 8:22 ` Thomas Gleixner
2016-01-28 11:52 ` Arnd Bergmann
0 siblings, 1 reply; 9+ messages in thread
From: Thomas Gleixner @ 2016-01-28 8:22 UTC (permalink / raw)
To: zengtao; +Cc: LKML, Arnd Bergmann
Cc'ing Arnd
On Thu, 28 Jan 2016, zengtao wrote:
> The structure:
> struct timeval {
> __kernel_time_t tv_sec; /* seconds */
> __kernel_suseconds_t tv_usec; /* microseconds */
> };
> both __kernel_time_t and __kernel_suseconds_t are short than u64
> when it is 32bit platform, so force u64 conversion here.
>
> Signed-off-by: zengtao <prime.zeng@huawei.com>
Reviewed-by: Thomas Gleixner <tglx@linutronix.de>
> ---
> include/asm-generic/cputime_nsecs.h | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/include/asm-generic/cputime_nsecs.h b/include/asm-generic/cputime_nsecs.h
> index 0419485..e2f7ff9 100644
> --- a/include/asm-generic/cputime_nsecs.h
> +++ b/include/asm-generic/cputime_nsecs.h
> @@ -91,7 +91,8 @@ static inline void cputime_to_timespec(const cputime_t ct, struct timespec *val)
> */
> static inline cputime_t timeval_to_cputime(const struct timeval *val)
> {
> - u64 ret = val->tv_sec * NSEC_PER_SEC + val->tv_usec * NSEC_PER_USEC;
> + u64 ret = (u64)val->tv_sec * NSEC_PER_SEC +
> + val->tv_usec * NSEC_PER_USEC;
> return (__force cputime_t) ret;
> }
> static inline void cputime_to_timeval(const cputime_t ct, struct timeval *val)
> --
> 1.9.1
>
>
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] cputime: Fix timeval-->cputime conversion
2016-01-28 8:22 ` Thomas Gleixner
@ 2016-01-28 11:52 ` Arnd Bergmann
2016-01-29 3:12 ` Zengtao (B)
0 siblings, 1 reply; 9+ messages in thread
From: Arnd Bergmann @ 2016-01-28 11:52 UTC (permalink / raw)
To: Thomas Gleixner; +Cc: zengtao, LKML, Frederic Weisbecker
On Thursday 28 January 2016 09:22:04 Thomas Gleixner wrote:
> Cc'ing Arnd
>
> On Thu, 28 Jan 2016, zengtao wrote:
>
> > The structure:
> > struct timeval {
> > __kernel_time_t tv_sec; /* seconds */
> > __kernel_suseconds_t tv_usec; /* microseconds */
> > };
> > both __kernel_time_t and __kernel_suseconds_t are short than u64
> > when it is 32bit platform, so force u64 conversion here.
> >
> > Signed-off-by: zengtao <prime.zeng@huawei.com>
>
> Reviewed-by: Thomas Gleixner <tglx@linutronix.de>
This seems to miss timespec_to_cputime(), which has the same problem,
so only setitimer() is fixed, but not nanosleep() or timer_settime().
There should probably be some explanation in which cases this happens,
my reading is that can only occur on MIPS32 and ARM32 with "Full dynticks
CPU time accounting" enabled, which is required for CONFIG_NO_HZ_FULL,
so we need this backported to any kernel that includes
31c1fc818715 ("ARM: Kconfig: allow full nohz CPU accounting"), i.e.
v3.13 or higher, correct?
Arnd
> > include/asm-generic/cputime_nsecs.h | 3 ++-
> > 1 file changed, 2 insertions(+), 1 deletion(-)
> >
> > diff --git a/include/asm-generic/cputime_nsecs.h b/include/asm-generic/cputime_nsecs.h
> > index 0419485..e2f7ff9 100644
> > --- a/include/asm-generic/cputime_nsecs.h
> > +++ b/include/asm-generic/cputime_nsecs.h
> > @@ -91,7 +91,8 @@ static inline void cputime_to_timespec(const cputime_t ct, struct timespec *val)
> > */
> > static inline cputime_t timeval_to_cputime(const struct timeval *val)
> > {
> > - u64 ret = val->tv_sec * NSEC_PER_SEC + val->tv_usec * NSEC_PER_USEC;
> > + u64 ret = (u64)val->tv_sec * NSEC_PER_SEC +
> > + val->tv_usec * NSEC_PER_USEC;
> > return (__force cputime_t) ret;
> > }
> > static inline void cputime_to_timeval(const cputime_t ct, struct timeval *val)
> > --
> > 1.9.1
> >
> >
^ permalink raw reply [flat|nested] 9+ messages in thread
* RE: [PATCH] cputime: Fix timeval-->cputime conversion
2016-01-28 11:52 ` Arnd Bergmann
@ 2016-01-29 3:12 ` Zengtao (B)
2016-01-29 8:46 ` Arnd Bergmann
0 siblings, 1 reply; 9+ messages in thread
From: Zengtao (B) @ 2016-01-29 3:12 UTC (permalink / raw)
To: Arnd Bergmann, Thomas Gleixner; +Cc: LKML, Frederic Weisbecker
> -----Original Message-----
> From: Arnd Bergmann [mailto:arnd@arndb.de]
> Sent: Thursday, January 28, 2016 7:52 PM
> To: Thomas Gleixner
> Cc: Zengtao (B); LKML; Frederic Weisbecker
> Subject: Re: [PATCH] cputime: Fix timeval-->cputime conversion
>
> On Thursday 28 January 2016 09:22:04 Thomas Gleixner wrote:
> > Cc'ing Arnd
> >
> > On Thu, 28 Jan 2016, zengtao wrote:
> >
> > > The structure:
> > > struct timeval {
> > > __kernel_time_t tv_sec; /* seconds */
> > > __kernel_suseconds_t tv_usec; /* microseconds */
> > > };
> > > both __kernel_time_t and __kernel_suseconds_t are short than u64
> > > when it is 32bit platform, so force u64 conversion here.
> > >
> > > Signed-off-by: zengtao <prime.zeng@huawei.com>
> >
> > Reviewed-by: Thomas Gleixner <tglx@linutronix.de>
>
> This seems to miss timespec_to_cputime(), which has the same problem,
> so only setitimer() is fixed, but not nanosleep() or timer_settime().
Yes, I have checked the code just now, the timespec_to_cputime() has the
same problem.I found the origin issue through setitimer().And I think the
timespec_to_cputime() only affects timer_settime(),by which means it affects
nanosleep?
>
> There should probably be some explanation in which cases this happens,
> my reading is that can only occur on MIPS32 and ARM32 with "Full dynticks
> CPU time accounting" enabled, which is required for CONFIG_NO_HZ_FULL,
> so we need this backported to any kernel that includes
> 31c1fc818715 ("ARM: Kconfig: allow full nohz CPU accounting"), i.e.
> v3.13 or higher, correct?
Yes.
>
> Arnd
>
> > > include/asm-generic/cputime_nsecs.h | 3 ++-
> > > 1 file changed, 2 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/include/asm-generic/cputime_nsecs.h
> b/include/asm-generic/cputime_nsecs.h
> > > index 0419485..e2f7ff9 100644
> > > --- a/include/asm-generic/cputime_nsecs.h
> > > +++ b/include/asm-generic/cputime_nsecs.h
> > > @@ -91,7 +91,8 @@ static inline void cputime_to_timespec(const
> cputime_t ct, struct timespec *val)
> > > */
> > > static inline cputime_t timeval_to_cputime(const struct timeval *val)
> > > {
> > > - u64 ret = val->tv_sec * NSEC_PER_SEC + val->tv_usec *
> NSEC_PER_USEC;
> > > + u64 ret = (u64)val->tv_sec * NSEC_PER_SEC +
> > > + val->tv_usec * NSEC_PER_USEC;
> > > return (__force cputime_t) ret;
> > > }
> > > static inline void cputime_to_timeval(const cputime_t ct, struct timeval
> *val)
> > > --
> > > 1.9.1
> > >
> > >
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] cputime: Fix timeval-->cputime conversion
2016-01-29 3:12 ` Zengtao (B)
@ 2016-01-29 8:46 ` Arnd Bergmann
2016-01-30 2:31 ` Zengtao (B)
2016-02-01 3:51 ` Zengtao (B)
0 siblings, 2 replies; 9+ messages in thread
From: Arnd Bergmann @ 2016-01-29 8:46 UTC (permalink / raw)
To: Zengtao (B); +Cc: Thomas Gleixner, LKML, Frederic Weisbecker
On Friday 29 January 2016 03:12:37 Zengtao wrote:
> > -----Original Message-----
> > From: Arnd Bergmann [mailto:arnd@arndb.de]
> > Sent: Thursday, January 28, 2016 7:52 PM
> > To: Thomas Gleixner
> > Cc: Zengtao (B); LKML; Frederic Weisbecker
> > Subject: Re: [PATCH] cputime: Fix timeval-->cputime conversion
> >
> > On Thursday 28 January 2016 09:22:04 Thomas Gleixner wrote:
> > > Cc'ing Arnd
> > >
> > > On Thu, 28 Jan 2016, zengtao wrote:
> > >
> > > > The structure:
> > > > struct timeval {
> > > > __kernel_time_t tv_sec; /* seconds */
> > > > __kernel_suseconds_t tv_usec; /* microseconds */
> > > > };
> > > > both __kernel_time_t and __kernel_suseconds_t are short than u64
> > > > when it is 32bit platform, so force u64 conversion here.
> > > >
> > > > Signed-off-by: zengtao <prime.zeng@huawei.com>
> > >
> > > Reviewed-by: Thomas Gleixner <tglx@linutronix.de>
> >
> > This seems to miss timespec_to_cputime(), which has the same problem,
> > so only setitimer() is fixed, but not nanosleep() or timer_settime().
> Yes, I have checked the code just now, the timespec_to_cputime() has the
> same problem.I found the origin issue through setitimer().And I think the
> timespec_to_cputime() only affects timer_settime(),by which means it affects
> nanosleep?
Reading that code again, I think it does not affect sys_nanosleep, but
it does affect sys_clock_nanosleep(CLOCK_PROCESS_CPUTIME_ID, ...)
along with timer_create/timer_settime with CLOCK_PROCESS_CPUTIME_ID.
Arnd
^ permalink raw reply [flat|nested] 9+ messages in thread
* RE: [PATCH] cputime: Fix timeval-->cputime conversion
2016-01-29 8:46 ` Arnd Bergmann
@ 2016-01-30 2:31 ` Zengtao (B)
2016-02-01 3:51 ` Zengtao (B)
1 sibling, 0 replies; 9+ messages in thread
From: Zengtao (B) @ 2016-01-30 2:31 UTC (permalink / raw)
To: Arnd Bergmann; +Cc: Thomas Gleixner, LKML, Frederic Weisbecker
> -----Original Message-----
> From: Arnd Bergmann [mailto:arnd@arndb.de]
> Sent: Friday, January 29, 2016 4:46 PM
> To: Zengtao (B)
> Cc: Thomas Gleixner; LKML; Frederic Weisbecker
> Subject: Re: [PATCH] cputime: Fix timeval-->cputime conversion
>
> On Friday 29 January 2016 03:12:37 Zengtao wrote:
> > > -----Original Message-----
> > > From: Arnd Bergmann [mailto:arnd@arndb.de]
> > > Sent: Thursday, January 28, 2016 7:52 PM
> > > To: Thomas Gleixner
> > > Cc: Zengtao (B); LKML; Frederic Weisbecker
> > > Subject: Re: [PATCH] cputime: Fix timeval-->cputime conversion
> > >
> > > On Thursday 28 January 2016 09:22:04 Thomas Gleixner wrote:
> > > > Cc'ing Arnd
> > > >
> > > > On Thu, 28 Jan 2016, zengtao wrote:
> > > >
> > > > > The structure:
> > > > > struct timeval {
> > > > > __kernel_time_t tv_sec; /* seconds */
> > > > > __kernel_suseconds_t tv_usec; /* microseconds
> */
> > > > > };
> > > > > both __kernel_time_t and __kernel_suseconds_t are short than u64
> > > > > when it is 32bit platform, so force u64 conversion here.
> > > > >
> > > > > Signed-off-by: zengtao <prime.zeng@huawei.com>
> > > >
> > > > Reviewed-by: Thomas Gleixner <tglx@linutronix.de>
> > >
> > > This seems to miss timespec_to_cputime(), which has the same problem,
> > > so only setitimer() is fixed, but not nanosleep() or timer_settime().
> > Yes, I have checked the code just now, the timespec_to_cputime() has the
> > same problem.I found the origin issue through setitimer().And I think the
> > timespec_to_cputime() only affects timer_settime(),by which means it
> affects
> > nanosleep?
>
> Reading that code again, I think it does not affect sys_nanosleep, but
> it does affect sys_clock_nanosleep(CLOCK_PROCESS_CPUTIME_ID, ...)
> along with timer_create/timer_settime with CLOCK_PROCESS_CPUTIME_ID.
>
Got it, I will fix the timespec_to_cputime and resend the patch later.
> Arnd
^ permalink raw reply [flat|nested] 9+ messages in thread
* RE: [PATCH] cputime: Fix timeval-->cputime conversion
2016-01-29 8:46 ` Arnd Bergmann
2016-01-30 2:31 ` Zengtao (B)
@ 2016-02-01 3:51 ` Zengtao (B)
2016-02-01 8:43 ` Thomas Gleixner
1 sibling, 1 reply; 9+ messages in thread
From: Zengtao (B) @ 2016-02-01 3:51 UTC (permalink / raw)
To: Zengtao (B), Arnd Bergmann; +Cc: Thomas Gleixner, LKML, Frederic Weisbecker
Hi Arnd:
I have got a new idea about the problem:
In include/linux/time64.h
#define NSEC_PER_SEC 1000000000L
I think we should change it to
#define NSEC_PER_SEC 1000000000LL
My reason is :
1. when it is used in a multiplication, it will easily get overflow.
2. when it don't get overflow, the change has no side affect.
Thanks.
Zengtao
> -----Original Message-----
> From: Zengtao (B)
> Sent: Saturday, January 30, 2016 10:31 AM
> To: 'Arnd Bergmann'
> Cc: Thomas Gleixner; LKML; Frederic Weisbecker
> Subject: RE: [PATCH] cputime: Fix timeval-->cputime conversion
>
> > -----Original Message-----
> > From: Arnd Bergmann [mailto:arnd@arndb.de]
> > Sent: Friday, January 29, 2016 4:46 PM
> > To: Zengtao (B)
> > Cc: Thomas Gleixner; LKML; Frederic Weisbecker
> > Subject: Re: [PATCH] cputime: Fix timeval-->cputime conversion
> >
> > On Friday 29 January 2016 03:12:37 Zengtao wrote:
> > > > -----Original Message-----
> > > > From: Arnd Bergmann [mailto:arnd@arndb.de]
> > > > Sent: Thursday, January 28, 2016 7:52 PM
> > > > To: Thomas Gleixner
> > > > Cc: Zengtao (B); LKML; Frederic Weisbecker
> > > > Subject: Re: [PATCH] cputime: Fix timeval-->cputime conversion
> > > >
> > > > On Thursday 28 January 2016 09:22:04 Thomas Gleixner wrote:
> > > > > Cc'ing Arnd
> > > > >
> > > > > On Thu, 28 Jan 2016, zengtao wrote:
> > > > >
> > > > > > The structure:
> > > > > > struct timeval {
> > > > > > __kernel_time_t tv_sec; /* seconds */
> > > > > > __kernel_suseconds_t tv_usec; /* microseconds
> > */
> > > > > > };
> > > > > > both __kernel_time_t and __kernel_suseconds_t are short than u64
> > > > > > when it is 32bit platform, so force u64 conversion here.
> > > > > >
> > > > > > Signed-off-by: zengtao <prime.zeng@huawei.com>
> > > > >
> > > > > Reviewed-by: Thomas Gleixner <tglx@linutronix.de>
> > > >
> > > > This seems to miss timespec_to_cputime(), which has the same problem,
> > > > so only setitimer() is fixed, but not nanosleep() or timer_settime().
> > > Yes, I have checked the code just now, the timespec_to_cputime() has the
> > > same problem.I found the origin issue through setitimer().And I think the
> > > timespec_to_cputime() only affects timer_settime(),by which means it
> > affects
> > > nanosleep?
> >
> > Reading that code again, I think it does not affect sys_nanosleep, but
> > it does affect sys_clock_nanosleep(CLOCK_PROCESS_CPUTIME_ID, ...)
> > along with timer_create/timer_settime with CLOCK_PROCESS_CPUTIME_ID.
> >
> Got it, I will fix the timespec_to_cputime and resend the patch later.
> > Arnd
^ permalink raw reply [flat|nested] 9+ messages in thread
* RE: [PATCH] cputime: Fix timeval-->cputime conversion
2016-02-01 3:51 ` Zengtao (B)
@ 2016-02-01 8:43 ` Thomas Gleixner
2016-02-02 2:57 ` Zengtao (B)
0 siblings, 1 reply; 9+ messages in thread
From: Thomas Gleixner @ 2016-02-01 8:43 UTC (permalink / raw)
To: Zengtao (B); +Cc: Arnd Bergmann, LKML, Frederic Weisbecker
On Mon, 1 Feb 2016, Zengtao (B) wrote:
> Hi Arnd:
> I have got a new idea about the problem:
> In include/linux/time64.h
> #define NSEC_PER_SEC 1000000000L
> I think we should change it to
> #define NSEC_PER_SEC 1000000000LL
>
> My reason is :
> 1. when it is used in a multiplication, it will easily get overflow.
> 2. when it don't get overflow, the change has no side affect.
That's not a good idea. NSEC_PER_SEC is used in lots of places with 32bit
storage. I really don't want to mop up all the fallout.
Thanks,
tglx
^ permalink raw reply [flat|nested] 9+ messages in thread
* RE: [PATCH] cputime: Fix timeval-->cputime conversion
2016-02-01 8:43 ` Thomas Gleixner
@ 2016-02-02 2:57 ` Zengtao (B)
0 siblings, 0 replies; 9+ messages in thread
From: Zengtao (B) @ 2016-02-02 2:57 UTC (permalink / raw)
To: Thomas Gleixner; +Cc: Arnd Bergmann, LKML, Frederic Weisbecker
> -----Original Message-----
> From: Thomas Gleixner [mailto:tglx@linutronix.de]
> Sent: Monday, February 01, 2016 4:43 PM
> To: Zengtao (B)
> Cc: Arnd Bergmann; LKML; Frederic Weisbecker
> Subject: RE: [PATCH] cputime: Fix timeval-->cputime conversion
>
> On Mon, 1 Feb 2016, Zengtao (B) wrote:
>
> > Hi Arnd:
> > I have got a new idea about the problem:
> > In include/linux/time64.h
> > #define NSEC_PER_SEC 1000000000L
> > I think we should change it to
> > #define NSEC_PER_SEC 1000000000LL
> >
> > My reason is :
> > 1. when it is used in a multiplication, it will easily get overflow.
> > 2. when it don't get overflow, the change has no side affect.
>
> That's not a good idea. NSEC_PER_SEC is used in lots of places with 32bit
> storage. I really don't want to mop up all the fallout.
Yes, agree, a lot of places it has been used as 32bit value. Beyond that, I think
it is more reasonable to use 64bit.
>
> Thanks,
>
> tglx
>
^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2016-02-02 2:58 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-01-28 7:02 [PATCH] cputime: Fix timeval-->cputime conversion zengtao
2016-01-28 8:22 ` Thomas Gleixner
2016-01-28 11:52 ` Arnd Bergmann
2016-01-29 3:12 ` Zengtao (B)
2016-01-29 8:46 ` Arnd Bergmann
2016-01-30 2:31 ` Zengtao (B)
2016-02-01 3:51 ` Zengtao (B)
2016-02-01 8:43 ` Thomas Gleixner
2016-02-02 2:57 ` Zengtao (B)
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.