All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] timekeeping: Fixup typo in update_vsyscall_old definition
@ 2014-07-26  4:37 John Stultz
  2014-07-30  7:31 ` [tip:timers/core] " tip-bot for John Stultz
  0 siblings, 1 reply; 8+ messages in thread
From: John Stultz @ 2014-07-26  4:37 UTC (permalink / raw)
  To: linux-kernel
  Cc: John Stultz, Stephen Rothwell, Thomas Gleixner, Ingo Molnar,
	H. Peter Anvin, Peter Zijlstra

In commit 4a0e637738f0 ("clocksource: Get rid of cycle_last"),
currently in the -tip tree, there was a small typo where cycles_t
was used intstead of cycle_t. This broke ppc64 builds.

Fix this by using the proper cycle_t type for this usage, in
both the definition and the ia64 implementation.

Now, having both cycle_t and cycles_t types seems like a very
bad idea just asking for these sorts of issues. But that
will be a cleanup for another day.

Cc: Stephen Rothwell <sfr@canb.auug.org.au>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Ingo Molnar <mingo@elte.hu>
Cc: "H. Peter Anvin" <hpa@zytor.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Reported-by: Stephen Rothwell <sfr@canb.auug.org.au>
Signed-off-by: John Stultz <john.stultz@linaro.org>
---

Note: This should be visibly correct, and I've test built on ppc64,
but I don't have an ia64 toolchain, so if anyone could give this a
build whirl on ia64, I'd appreciate it.

 arch/ia64/kernel/time.c             | 2 +-
 include/linux/timekeeper_internal.h | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/ia64/kernel/time.c b/arch/ia64/kernel/time.c
index 11dc42d..3e71ef8 100644
--- a/arch/ia64/kernel/time.c
+++ b/arch/ia64/kernel/time.c
@@ -441,7 +441,7 @@ void update_vsyscall_tz(void)
 }
 
 void update_vsyscall_old(struct timespec *wall, struct timespec *wtm,
-			 struct clocksource *c, u32 mult, cycles_t cycle_last)
+			 struct clocksource *c, u32 mult, cycle_t cycle_last)
 {
 	write_seqcount_begin(&fsyscall_gtod_data.seq);
 
diff --git a/include/linux/timekeeper_internal.h b/include/linux/timekeeper_internal.h
index e9660e5..95640dc 100644
--- a/include/linux/timekeeper_internal.h
+++ b/include/linux/timekeeper_internal.h
@@ -113,7 +113,7 @@ extern void update_vsyscall_tz(void);
 
 extern void update_vsyscall_old(struct timespec *ts, struct timespec *wtm,
 				struct clocksource *c, u32 mult,
-				cycles_t cycle_last);
+				cycle_t cycle_last);
 extern void update_vsyscall_tz(void);
 
 #else
-- 
1.9.1


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [tip:timers/core] timekeeping: Fixup typo in update_vsyscall_old definition
  2014-07-26  4:37 [PATCH] timekeeping: Fixup typo in update_vsyscall_old definition John Stultz
@ 2014-07-30  7:31 ` tip-bot for John Stultz
  2014-08-11  4:19     ` Benjamin Herrenschmidt
  0 siblings, 1 reply; 8+ messages in thread
From: tip-bot for John Stultz @ 2014-07-30  7:31 UTC (permalink / raw)
  To: linux-tip-commits
  Cc: linux-kernel, john.stultz, hpa, mingo, peterz, sfr, tglx

Commit-ID:  953dec21aed4038464fec02f96a2f1b8701a5bce
Gitweb:     http://git.kernel.org/tip/953dec21aed4038464fec02f96a2f1b8701a5bce
Author:     John Stultz <john.stultz@linaro.org>
AuthorDate: Fri, 25 Jul 2014 21:37:19 -0700
Committer:  Thomas Gleixner <tglx@linutronix.de>
CommitDate: Wed, 30 Jul 2014 09:26:25 +0200

timekeeping: Fixup typo in update_vsyscall_old definition

In commit 4a0e637738f0 ("clocksource: Get rid of cycle_last"),
currently in the -tip tree, there was a small typo where cycles_t
was used intstead of cycle_t. This broke ppc64 builds.

Fix this by using the proper cycle_t type for this usage, in
both the definition and the ia64 implementation.

Now, having both cycle_t and cycles_t types seems like a very
bad idea just asking for these sorts of issues. But that
will be a cleanup for another day.

Reported-by: Stephen Rothwell <sfr@canb.auug.org.au>
Signed-off-by: John Stultz <john.stultz@linaro.org>
Cc: Ingo Molnar <mingo@kernel.org>
Cc: "H. Peter Anvin" <hpa@zytor.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Link: http://lkml.kernel.org/r/1406349439-11785-1-git-send-email-john.stultz@linaro.org
Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
---
 arch/ia64/kernel/time.c             | 2 +-
 include/linux/timekeeper_internal.h | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/ia64/kernel/time.c b/arch/ia64/kernel/time.c
index 11dc42d..3e71ef8 100644
--- a/arch/ia64/kernel/time.c
+++ b/arch/ia64/kernel/time.c
@@ -441,7 +441,7 @@ void update_vsyscall_tz(void)
 }
 
 void update_vsyscall_old(struct timespec *wall, struct timespec *wtm,
-			 struct clocksource *c, u32 mult, cycles_t cycle_last)
+			 struct clocksource *c, u32 mult, cycle_t cycle_last)
 {
 	write_seqcount_begin(&fsyscall_gtod_data.seq);
 
diff --git a/include/linux/timekeeper_internal.h b/include/linux/timekeeper_internal.h
index e9660e5..95640dc 100644
--- a/include/linux/timekeeper_internal.h
+++ b/include/linux/timekeeper_internal.h
@@ -113,7 +113,7 @@ extern void update_vsyscall_tz(void);
 
 extern void update_vsyscall_old(struct timespec *ts, struct timespec *wtm,
 				struct clocksource *c, u32 mult,
-				cycles_t cycle_last);
+				cycle_t cycle_last);
 extern void update_vsyscall_tz(void);
 
 #else

^ permalink raw reply related	[flat|nested] 8+ messages in thread

* Re: [tip:timers/core] timekeeping: Fixup typo in update_vsyscall_old definition
  2014-07-30  7:31 ` [tip:timers/core] " tip-bot for John Stultz
@ 2014-08-11  4:19     ` Benjamin Herrenschmidt
  0 siblings, 0 replies; 8+ messages in thread
From: Benjamin Herrenschmidt @ 2014-08-11  4:19 UTC (permalink / raw)
  To: john.stultz; +Cc: mingo, hpa, linux-kernel, peterz, tglx, sfr, linuxppc-dev

On Wed, 2014-07-30 at 00:31 -0700, tip-bot for John Stultz wrote:
> Commit-ID:  953dec21aed4038464fec02f96a2f1b8701a5bce
> Gitweb:     http://git.kernel.org/tip/953dec21aed4038464fec02f96a2f1b8701a5bce
> Author:     John Stultz <john.stultz@linaro.org>
> AuthorDate: Fri, 25 Jul 2014 21:37:19 -0700
> Committer:  Thomas Gleixner <tglx@linutronix.de>
> CommitDate: Wed, 30 Jul 2014 09:26:25 +0200
> 
> timekeeping: Fixup typo in update_vsyscall_old definition
> 
> In commit 4a0e637738f0 ("clocksource: Get rid of cycle_last"),
> currently in the -tip tree, there was a small typo where cycles_t
> was used intstead of cycle_t. This broke ppc64 builds.

There's another bug in there... You fix timespec vs. timespec64 for the
first argument of update_vsyscall_old but not the second one ...
(wall_to_monotonic).

Also, in e2dff1ec0 you claim this is "minor", you seem to forget that
arch/powerpc also deals with 32-bit kernels which use the same time
keeping code, so we have a pretty serious regressions here...

BTW. Is there some documentation you can point me to to figure out what
replace that "_OLD" stuff so we can update to whatever is "new" ?

Cheers,
Ben.



^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [tip:timers/core] timekeeping: Fixup typo in update_vsyscall_old definition
@ 2014-08-11  4:19     ` Benjamin Herrenschmidt
  0 siblings, 0 replies; 8+ messages in thread
From: Benjamin Herrenschmidt @ 2014-08-11  4:19 UTC (permalink / raw)
  To: john.stultz; +Cc: sfr, peterz, linux-kernel, linuxppc-dev, hpa, tglx, mingo

On Wed, 2014-07-30 at 00:31 -0700, tip-bot for John Stultz wrote:
> Commit-ID:  953dec21aed4038464fec02f96a2f1b8701a5bce
> Gitweb:     http://git.kernel.org/tip/953dec21aed4038464fec02f96a2f1b8701a5bce
> Author:     John Stultz <john.stultz@linaro.org>
> AuthorDate: Fri, 25 Jul 2014 21:37:19 -0700
> Committer:  Thomas Gleixner <tglx@linutronix.de>
> CommitDate: Wed, 30 Jul 2014 09:26:25 +0200
> 
> timekeeping: Fixup typo in update_vsyscall_old definition
> 
> In commit 4a0e637738f0 ("clocksource: Get rid of cycle_last"),
> currently in the -tip tree, there was a small typo where cycles_t
> was used intstead of cycle_t. This broke ppc64 builds.

There's another bug in there... You fix timespec vs. timespec64 for the
first argument of update_vsyscall_old but not the second one ...
(wall_to_monotonic).

Also, in e2dff1ec0 you claim this is "minor", you seem to forget that
arch/powerpc also deals with 32-bit kernels which use the same time
keeping code, so we have a pretty serious regressions here...

BTW. Is there some documentation you can point me to to figure out what
replace that "_OLD" stuff so we can update to whatever is "new" ?

Cheers,
Ben.

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [tip:timers/core] timekeeping: Fixup typo in update_vsyscall_old definition
  2014-08-11  4:19     ` Benjamin Herrenschmidt
@ 2014-08-12 15:30       ` John Stultz
  -1 siblings, 0 replies; 8+ messages in thread
From: John Stultz @ 2014-08-12 15:30 UTC (permalink / raw)
  To: Benjamin Herrenschmidt
  Cc: Ingo Molnar, hpa, LKML, peterz, tglx, sfr, linuxppc-dev, Paul Mackerras

On 08/10/2014 09:19 PM, Benjamin Herrenschmidt wrote:
> On Wed, 2014-07-30 at 00:31 -0700, tip-bot for John Stultz wrote:
>> Commit-ID:  953dec21aed4038464fec02f96a2f1b8701a5bce
>> Gitweb:     http://git.kernel.org/tip/953dec21aed4038464fec02f96a2f1b8701a5bce
>> Author:     John Stultz <john.stultz@linaro.org>
>> AuthorDate: Fri, 25 Jul 2014 21:37:19 -0700
>> Committer:  Thomas Gleixner <tglx@linutronix.de>
>> CommitDate: Wed, 30 Jul 2014 09:26:25 +0200
>>
>> timekeeping: Fixup typo in update_vsyscall_old definition
>>
>> In commit 4a0e637738f0 ("clocksource: Get rid of cycle_last"),
>> currently in the -tip tree, there was a small typo where cycles_t
>> was used intstead of cycle_t. This broke ppc64 builds.
> There's another bug in there... You fix timespec vs. timespec64 for the
> first argument of update_vsyscall_old but not the second one ...
> (wall_to_monotonic).
>
> Also, in e2dff1ec0 you claim this is "minor", you seem to forget that
> arch/powerpc also deals with 32-bit kernels which use the same time
> keeping code, so we have a pretty serious regressions here...
Yikes. My apologies. I had missed that issue and had forgotten ppc32 has
the vsyscall support as well.

Thanks for pointing it out. I'll send a fix here shortly (though I only
have the ppc64le toolchain handy, so forgive me if its not quite right).


> BTW. Is there some documentation you can point me to to figure out what
> replace that "_OLD" stuff so we can update to whatever is "new" ?

So there's not exactly documentation, but the idea is rather then doing:

nsecs +  (mult*(now - cycle_last) >>shift);

We're preserving the sub-nanosecond precision, and doing:

(shifted_nsecs + mult*(now-cycle_last)) >> shift

This avoids the rounding up 1ns every tick, which we did to avoid errors
from truncating the precision.

I think the hard part for ppc, is if I recall, ppc's vsyscall exports
the xsec unit, which less granular then nanoseconds, so it had its own
version of the same precision truncation issues. I recall Paul working
on that, and I thought his solution was to reduce the multiplier by one
so the inter-tick time was slightly slower, then the tick update would
catch up causing a slight stair-step, which isn't ideal but is better
then the inconsistencies that came out of not-handling the precision
properly. That said, skimming the ppc code, I don't see that logic right
off, and have a fuzzy memory that maybe that solution was RHEL5 specific
or something like that.

thanks
-john







^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [tip:timers/core] timekeeping: Fixup typo in update_vsyscall_old definition
@ 2014-08-12 15:30       ` John Stultz
  0 siblings, 0 replies; 8+ messages in thread
From: John Stultz @ 2014-08-12 15:30 UTC (permalink / raw)
  To: Benjamin Herrenschmidt
  Cc: sfr, peterz, LKML, linuxppc-dev, Paul Mackerras, hpa, tglx, Ingo Molnar

On 08/10/2014 09:19 PM, Benjamin Herrenschmidt wrote:
> On Wed, 2014-07-30 at 00:31 -0700, tip-bot for John Stultz wrote:
>> Commit-ID:  953dec21aed4038464fec02f96a2f1b8701a5bce
>> Gitweb:     http://git.kernel.org/tip/953dec21aed4038464fec02f96a2f1b8=
701a5bce
>> Author:     John Stultz <john.stultz@linaro.org>
>> AuthorDate: Fri, 25 Jul 2014 21:37:19 -0700
>> Committer:  Thomas Gleixner <tglx@linutronix.de>
>> CommitDate: Wed, 30 Jul 2014 09:26:25 +0200
>>
>> timekeeping: Fixup typo in update_vsyscall_old definition
>>
>> In commit 4a0e637738f0 ("clocksource: Get rid of cycle_last"),
>> currently in the -tip tree, there was a small typo where cycles_t
>> was used intstead of cycle_t. This broke ppc64 builds.
> There's another bug in there... You fix timespec vs. timespec64 for the=

> first argument of update_vsyscall_old but not the second one ...
> (wall_to_monotonic).
>
> Also, in e2dff1ec0 you claim this is "minor", you seem to forget that
> arch/powerpc also deals with 32-bit kernels which use the same time
> keeping code, so we have a pretty serious regressions here...
Yikes. My apologies. I had missed that issue and had forgotten ppc32 has
the vsyscall support as well.

Thanks for pointing it out. I'll send a fix here shortly (though I only
have the ppc64le toolchain handy, so forgive me if its not quite right).


> BTW. Is there some documentation you can point me to to figure out what=

> replace that "_OLD" stuff so we can update to whatever is "new" ?

So there's not exactly documentation, but the idea is rather then doing:

nsecs +  (mult*(now - cycle_last) >>shift);

We're preserving the sub-nanosecond precision, and doing:

(shifted_nsecs + mult*(now-cycle_last)) >> shift

This avoids the rounding up 1ns every tick, which we did to avoid errors
from truncating the precision.

I think the hard part for ppc, is if I recall, ppc's vsyscall exports
the xsec unit, which less granular then nanoseconds, so it had its own
version of the same precision truncation issues. I recall Paul working
on that, and I thought his solution was to reduce the multiplier by one
so the inter-tick time was slightly slower, then the tick update would
catch up causing a slight stair-step, which isn't ideal but is better
then the inconsistencies that came out of not-handling the precision
properly. That said, skimming the ppc code, I don't see that logic right
off, and have a fuzzy memory that maybe that solution was RHEL5 specific
or something like that.

thanks
-john

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [tip:timers/core] timekeeping: Fixup typo in update_vsyscall_old definition
  2014-08-12 15:30       ` John Stultz
@ 2014-08-13  4:03         ` Tony Breeds
  -1 siblings, 0 replies; 8+ messages in thread
From: Tony Breeds @ 2014-08-13  4:03 UTC (permalink / raw)
  To: John Stultz
  Cc: Benjamin Herrenschmidt, sfr, peterz, LKML, linuxppc-dev,
	Paul Mackerras, hpa, tglx, Ingo Molnar

[-- Attachment #1: Type: text/plain, Size: 401 bytes --]

On Tue, Aug 12, 2014 at 08:30:40AM -0700, John Stultz wrote:

> Thanks for pointing it out. I'll send a fix here shortly (though I only
> have the ppc64le toolchain handy, so forgive me if its not quite right).

/me pimps the kernel.org toolchains:
https://www.kernel.org/pub/tools/crosstool/files/bin/x86_64/4.9.0/x86_64-gcc-4.9.0-nolibc_powerpc64-linux.tar.xz

and gets back in his box.

Yours Tony

[-- Attachment #2: Type: application/pgp-signature, Size: 819 bytes --]

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [tip:timers/core] timekeeping: Fixup typo in update_vsyscall_old definition
@ 2014-08-13  4:03         ` Tony Breeds
  0 siblings, 0 replies; 8+ messages in thread
From: Tony Breeds @ 2014-08-13  4:03 UTC (permalink / raw)
  To: John Stultz
  Cc: sfr, peterz, LKML, linuxppc-dev, Paul Mackerras, hpa, tglx, Ingo Molnar

[-- Attachment #1: Type: text/plain, Size: 401 bytes --]

On Tue, Aug 12, 2014 at 08:30:40AM -0700, John Stultz wrote:

> Thanks for pointing it out. I'll send a fix here shortly (though I only
> have the ppc64le toolchain handy, so forgive me if its not quite right).

/me pimps the kernel.org toolchains:
https://www.kernel.org/pub/tools/crosstool/files/bin/x86_64/4.9.0/x86_64-gcc-4.9.0-nolibc_powerpc64-linux.tar.xz

and gets back in his box.

Yours Tony

[-- Attachment #2: Type: application/pgp-signature, Size: 819 bytes --]

^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2014-08-13  4:03 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-07-26  4:37 [PATCH] timekeeping: Fixup typo in update_vsyscall_old definition John Stultz
2014-07-30  7:31 ` [tip:timers/core] " tip-bot for John Stultz
2014-08-11  4:19   ` Benjamin Herrenschmidt
2014-08-11  4:19     ` Benjamin Herrenschmidt
2014-08-12 15:30     ` John Stultz
2014-08-12 15:30       ` John Stultz
2014-08-13  4:03       ` Tony Breeds
2014-08-13  4:03         ` Tony Breeds

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.