* [PATCH 0/2] arm64: vdso fixes for coarse clocks @ 2014-02-03 19:48 Nathan Lynch 2014-02-03 19:48 ` [PATCH 1/2] arm64: vdso: fix coarse clock handling Nathan Lynch 2014-02-03 19:48 ` [PATCH 2/2] arm64: vdso: update wtm fields for CLOCK_MONOTONIC_COARSE Nathan Lynch 0 siblings, 2 replies; 9+ messages in thread From: Nathan Lynch @ 2014-02-03 19:48 UTC (permalink / raw) To: linux-arm-kernel While working on vdso for 32-bit ARM I have referred heavily to the arm64 implementation. In arm64 I found a couple of issues with the handling of CLOCK_MONOTONIC_COARSE and CLOCK_REALTIME_COARSE, which are addressed in this series. Nathan Lynch (2): arm64: vdso: fix coarse clock handling arm64: vdso: update wtm fields for CLOCK_MONOTONIC_COARSE arch/arm64/kernel/vdso.c | 4 ++-- arch/arm64/kernel/vdso/gettimeofday.S | 9 ++++++--- 2 files changed, 8 insertions(+), 5 deletions(-) -- 1.8.3.1 ^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH 1/2] arm64: vdso: fix coarse clock handling 2014-02-03 19:48 [PATCH 0/2] arm64: vdso fixes for coarse clocks Nathan Lynch @ 2014-02-03 19:48 ` Nathan Lynch 2014-02-04 18:26 ` Will Deacon 2014-02-05 5:53 ` [PATCH v2 " Nathan Lynch 2014-02-03 19:48 ` [PATCH 2/2] arm64: vdso: update wtm fields for CLOCK_MONOTONIC_COARSE Nathan Lynch 1 sibling, 2 replies; 9+ messages in thread From: Nathan Lynch @ 2014-02-03 19:48 UTC (permalink / raw) To: linux-arm-kernel When __kernel_clock_gettime is called with a CLOCK_MONOTONIC_COARSE or CLOCK_REALTIME_COARSE clock id, it returns incorrectly to whatever the caller has placed in x2. Fix this by saving x30/LR to x2 unconditionally. Also: the tv_nsec field in the result is shifted by the value in x12. In the high-precision case x12 is cs_shift from the data page, but for coarse clocks x12 is uninitialized. Fix this by setting x12 to 0 once we know we are dealing with a coarse clock. Signed-off-by: Nathan Lynch <nathan_lynch@mentor.com> --- arch/arm64/kernel/vdso/gettimeofday.S | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/arch/arm64/kernel/vdso/gettimeofday.S b/arch/arm64/kernel/vdso/gettimeofday.S index f0a6d10b5211..6c37ae4a70c0 100644 --- a/arch/arm64/kernel/vdso/gettimeofday.S +++ b/arch/arm64/kernel/vdso/gettimeofday.S @@ -88,13 +88,13 @@ ENDPROC(__kernel_gettimeofday) /* int __kernel_clock_gettime(clockid_t clock_id, struct timespec *tp); */ ENTRY(__kernel_clock_gettime) .cfi_startproc + mov x2, x30 + .cfi_register x30, x2 + cmp w0, #CLOCK_REALTIME ccmp w0, #CLOCK_MONOTONIC, #0x4, ne b.ne 2f - mov x2, x30 - .cfi_register x30, x2 - /* Get kernel timespec. */ adr vdso_data, _vdso_data 1: seqcnt_acquire @@ -118,6 +118,9 @@ ENTRY(__kernel_clock_gettime) ccmp w0, #CLOCK_MONOTONIC_COARSE, #0x4, ne b.ne 8f + /* Set shift to 0 for coarse clocks */ + mov x12, #0 + /* Get coarse timespec. */ adr vdso_data, _vdso_data 3: seqcnt_acquire -- 1.8.3.1 ^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH 1/2] arm64: vdso: fix coarse clock handling 2014-02-03 19:48 ` [PATCH 1/2] arm64: vdso: fix coarse clock handling Nathan Lynch @ 2014-02-04 18:26 ` Will Deacon 2014-02-05 5:52 ` Nathan Lynch 2014-02-05 5:53 ` [PATCH v2 " Nathan Lynch 1 sibling, 1 reply; 9+ messages in thread From: Will Deacon @ 2014-02-04 18:26 UTC (permalink / raw) To: linux-arm-kernel Hi Nathan, Thanks for the patch! On Mon, Feb 03, 2014 at 07:48:51PM +0000, Nathan Lynch wrote: > When __kernel_clock_gettime is called with a CLOCK_MONOTONIC_COARSE or > CLOCK_REALTIME_COARSE clock id, it returns incorrectly to whatever the > caller has placed in x2. Fix this by saving x30/LR to x2 > unconditionally. > > Also: the tv_nsec field in the result is shifted by the value in x12. > In the high-precision case x12 is cs_shift from the data page, but > for coarse clocks x12 is uninitialized. Fix this by setting x12 to 0 > once we know we are dealing with a coarse clock. How are you managing to see these failures? It's clear that our LTP testing isn't hitting this path... > Signed-off-by: Nathan Lynch <nathan_lynch@mentor.com> > --- > arch/arm64/kernel/vdso/gettimeofday.S | 9 ++++++--- > 1 file changed, 6 insertions(+), 3 deletions(-) > > diff --git a/arch/arm64/kernel/vdso/gettimeofday.S b/arch/arm64/kernel/vdso/gettimeofday.S > index f0a6d10b5211..6c37ae4a70c0 100644 > --- a/arch/arm64/kernel/vdso/gettimeofday.S > +++ b/arch/arm64/kernel/vdso/gettimeofday.S > @@ -88,13 +88,13 @@ ENDPROC(__kernel_gettimeofday) > /* int __kernel_clock_gettime(clockid_t clock_id, struct timespec *tp); */ > ENTRY(__kernel_clock_gettime) > .cfi_startproc > + mov x2, x30 > + .cfi_register x30, x2 > + > cmp w0, #CLOCK_REALTIME > ccmp w0, #CLOCK_MONOTONIC, #0x4, ne > b.ne 2f > > - mov x2, x30 > - .cfi_register x30, x2 > - Could we avoid the redundant moves by instead doing this around the bl __do_get_tspec? > /* Get kernel timespec. */ > adr vdso_data, _vdso_data > 1: seqcnt_acquire > @@ -118,6 +118,9 @@ ENTRY(__kernel_clock_gettime) > ccmp w0, #CLOCK_MONOTONIC_COARSE, #0x4, ne > b.ne 8f > > + /* Set shift to 0 for coarse clocks */ > + mov x12, #0 Worth mentioning that xtime_coarse_nsec is pre-shifted for us, rather than shifting not actually being required. Also, rather than shift by #0, can we move the lsl instruction immediately before the b 4f earlier on? Will ^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH 1/2] arm64: vdso: fix coarse clock handling 2014-02-04 18:26 ` Will Deacon @ 2014-02-05 5:52 ` Nathan Lynch 0 siblings, 0 replies; 9+ messages in thread From: Nathan Lynch @ 2014-02-05 5:52 UTC (permalink / raw) To: linux-arm-kernel Hi Will, On 02/04/2014 12:26 PM, Will Deacon wrote: > > On Mon, Feb 03, 2014 at 07:48:51PM +0000, Nathan Lynch wrote: >> When __kernel_clock_gettime is called with a CLOCK_MONOTONIC_COARSE or >> CLOCK_REALTIME_COARSE clock id, it returns incorrectly to whatever the >> caller has placed in x2. Fix this by saving x30/LR to x2 >> unconditionally. >> >> Also: the tv_nsec field in the result is shifted by the value in x12. >> In the high-precision case x12 is cs_shift from the data page, but >> for coarse clocks x12 is uninitialized. Fix this by setting x12 to 0 >> once we know we are dealing with a coarse clock. > > How are you managing to see these failures? It's clear that our LTP testing > isn't hitting this path... I'm using a custom test program that I wrote to check the vdso implementation I've been working on for 32-bit ARM. Briefly looking through LTP sources it's not apparent to me that it tests clock_gettime() with the coarse ids. > >> Signed-off-by: Nathan Lynch <nathan_lynch@mentor.com> >> --- >> arch/arm64/kernel/vdso/gettimeofday.S | 9 ++++++--- >> 1 file changed, 6 insertions(+), 3 deletions(-) >> >> diff --git a/arch/arm64/kernel/vdso/gettimeofday.S b/arch/arm64/kernel/vdso/gettimeofday.S >> index f0a6d10b5211..6c37ae4a70c0 100644 >> --- a/arch/arm64/kernel/vdso/gettimeofday.S >> +++ b/arch/arm64/kernel/vdso/gettimeofday.S >> @@ -88,13 +88,13 @@ ENDPROC(__kernel_gettimeofday) >> /* int __kernel_clock_gettime(clockid_t clock_id, struct timespec *tp); */ >> ENTRY(__kernel_clock_gettime) >> .cfi_startproc >> + mov x2, x30 >> + .cfi_register x30, x2 >> + >> cmp w0, #CLOCK_REALTIME >> ccmp w0, #CLOCK_MONOTONIC, #0x4, ne >> b.ne 2f >> >> - mov x2, x30 >> - .cfi_register x30, x2 >> - > > Could we avoid the redundant moves by instead doing this around the bl > __do_get_tspec? In v2 I've restored x30 upon returning from __do_get_tspec and adjusted the return from __kernel_clock_gettime accordingly. Let me know if you were suggesting something different here. > >> /* Get kernel timespec. */ >> adr vdso_data, _vdso_data >> 1: seqcnt_acquire >> @@ -118,6 +118,9 @@ ENTRY(__kernel_clock_gettime) >> ccmp w0, #CLOCK_MONOTONIC_COARSE, #0x4, ne >> b.ne 8f >> >> + /* Set shift to 0 for coarse clocks */ >> + mov x12, #0 > > Worth mentioning that xtime_coarse_nsec is pre-shifted for us, rather than > shifting not actually being required. Agreed. > Also, rather than shift by #0, can we move the lsl instruction immediately > before the b 4f earlier on? There are two more shift operations later in the routine which would also need accounting for, so it's not quite that simple. Okay to leave this alone for the purpose of this patch? ^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH v2 1/2] arm64: vdso: fix coarse clock handling 2014-02-03 19:48 ` [PATCH 1/2] arm64: vdso: fix coarse clock handling Nathan Lynch 2014-02-04 18:26 ` Will Deacon @ 2014-02-05 5:53 ` Nathan Lynch 2014-02-05 10:09 ` Will Deacon 1 sibling, 1 reply; 9+ messages in thread From: Nathan Lynch @ 2014-02-05 5:53 UTC (permalink / raw) To: linux-arm-kernel When __kernel_clock_gettime is called with a CLOCK_MONOTONIC_COARSE or CLOCK_REALTIME_COARSE clock id, it returns incorrectly to whatever the caller has placed in x2 ("ret x2" to return from the fast path). Fix this by saving x30/LR to x2 only in code that will call __do_get_tspec, restoring x30 afterward, and using a plain "ret" to return from the routine. Also: while the resulting tv_nsec value for CLOCK_REALTIME and CLOCK_MONOTONIC must be computed using intermediate values that are left-shifted by cs_shift (x12, set by __do_get_tspec), the results for coarse clocks should be calculated using unshifted values (xtime_coarse_nsec is in units of actual nanoseconds). The current code shifts intermediate values by x12 unconditionally, but x12 is uninitialized when servicing a coarse clock. Fix this by setting x12 to 0 once we know we are dealing with a coarse clock id. Signed-off-by: Nathan Lynch <nathan_lynch@mentor.com> --- Changes from v1: - Save x30/lr only when branching to __do_get_tspec, and restore it afterward. Use plain "ret" to return instead of "ret x2". - Better explanation for setting x12 (shift) to zero. arch/arm64/kernel/vdso/gettimeofday.S | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/arch/arm64/kernel/vdso/gettimeofday.S b/arch/arm64/kernel/vdso/gettimeofday.S index f0a6d10b5211..fe652ffd34c2 100644 --- a/arch/arm64/kernel/vdso/gettimeofday.S +++ b/arch/arm64/kernel/vdso/gettimeofday.S @@ -103,6 +103,8 @@ ENTRY(__kernel_clock_gettime) bl __do_get_tspec seqcnt_check w9, 1b + mov x30, x2 + cmp w0, #CLOCK_MONOTONIC b.ne 6f @@ -118,6 +120,9 @@ ENTRY(__kernel_clock_gettime) ccmp w0, #CLOCK_MONOTONIC_COARSE, #0x4, ne b.ne 8f + /* xtime_coarse_nsec is already right-shifted */ + mov x12, #0 + /* Get coarse timespec. */ adr vdso_data, _vdso_data 3: seqcnt_acquire @@ -156,7 +161,7 @@ ENTRY(__kernel_clock_gettime) lsr x11, x11, x12 stp x10, x11, [x1, #TSPEC_TV_SEC] mov x0, xzr - ret x2 + ret 7: mov x30, x2 8: /* Syscall fallback. */ -- 1.8.3.1 ^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH v2 1/2] arm64: vdso: fix coarse clock handling 2014-02-05 5:53 ` [PATCH v2 " Nathan Lynch @ 2014-02-05 10:09 ` Will Deacon 2014-02-05 11:54 ` Catalin Marinas 0 siblings, 1 reply; 9+ messages in thread From: Will Deacon @ 2014-02-05 10:09 UTC (permalink / raw) To: linux-arm-kernel On Wed, Feb 05, 2014 at 05:53:04AM +0000, Nathan Lynch wrote: > When __kernel_clock_gettime is called with a CLOCK_MONOTONIC_COARSE or > CLOCK_REALTIME_COARSE clock id, it returns incorrectly to whatever the > caller has placed in x2 ("ret x2" to return from the fast path). Fix > this by saving x30/LR to x2 only in code that will call > __do_get_tspec, restoring x30 afterward, and using a plain "ret" to > return from the routine. > > Also: while the resulting tv_nsec value for CLOCK_REALTIME and > CLOCK_MONOTONIC must be computed using intermediate values that are > left-shifted by cs_shift (x12, set by __do_get_tspec), the results for > coarse clocks should be calculated using unshifted values > (xtime_coarse_nsec is in units of actual nanoseconds). The current > code shifts intermediate values by x12 unconditionally, but x12 is > uninitialized when servicing a coarse clock. Fix this by setting x12 > to 0 once we know we are dealing with a coarse clock id. > > Signed-off-by: Nathan Lynch <nathan_lynch@mentor.com> > --- Thanks for the quick update Nathan! Acked-by: Will Deacon <will.deacon@arm.com> Catalin: both of these are candidates for stable. Will ^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH v2 1/2] arm64: vdso: fix coarse clock handling 2014-02-05 10:09 ` Will Deacon @ 2014-02-05 11:54 ` Catalin Marinas 0 siblings, 0 replies; 9+ messages in thread From: Catalin Marinas @ 2014-02-05 11:54 UTC (permalink / raw) To: linux-arm-kernel On Wed, Feb 05, 2014 at 10:09:45AM +0000, Will Deacon wrote: > On Wed, Feb 05, 2014 at 05:53:04AM +0000, Nathan Lynch wrote: > > When __kernel_clock_gettime is called with a CLOCK_MONOTONIC_COARSE or > > CLOCK_REALTIME_COARSE clock id, it returns incorrectly to whatever the > > caller has placed in x2 ("ret x2" to return from the fast path). Fix > > this by saving x30/LR to x2 only in code that will call > > __do_get_tspec, restoring x30 afterward, and using a plain "ret" to > > return from the routine. > > > > Also: while the resulting tv_nsec value for CLOCK_REALTIME and > > CLOCK_MONOTONIC must be computed using intermediate values that are > > left-shifted by cs_shift (x12, set by __do_get_tspec), the results for > > coarse clocks should be calculated using unshifted values > > (xtime_coarse_nsec is in units of actual nanoseconds). The current > > code shifts intermediate values by x12 unconditionally, but x12 is > > uninitialized when servicing a coarse clock. Fix this by setting x12 > > to 0 once we know we are dealing with a coarse clock id. > > > > Signed-off-by: Nathan Lynch <nathan_lynch@mentor.com> > > --- > > Thanks for the quick update Nathan! > > Acked-by: Will Deacon <will.deacon@arm.com> > > Catalin: both of these are candidates for stable. And by this you mean Cc: stable... Applied, thanks. -- Catalin ^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH 2/2] arm64: vdso: update wtm fields for CLOCK_MONOTONIC_COARSE 2014-02-03 19:48 [PATCH 0/2] arm64: vdso fixes for coarse clocks Nathan Lynch 2014-02-03 19:48 ` [PATCH 1/2] arm64: vdso: fix coarse clock handling Nathan Lynch @ 2014-02-03 19:48 ` Nathan Lynch 2014-02-04 18:28 ` Will Deacon 1 sibling, 1 reply; 9+ messages in thread From: Nathan Lynch @ 2014-02-03 19:48 UTC (permalink / raw) To: linux-arm-kernel Update wall-to-monotonic fields in the VDSO data page unconditionally. These are used to service CLOCK_MONOTONIC_COARSE, which is not guarded by use_syscall. Signed-off-by: Nathan Lynch <nathan_lynch@mentor.com> --- arch/arm64/kernel/vdso.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/arch/arm64/kernel/vdso.c b/arch/arm64/kernel/vdso.c index 65d40cf6945a..a7149cae1615 100644 --- a/arch/arm64/kernel/vdso.c +++ b/arch/arm64/kernel/vdso.c @@ -238,6 +238,8 @@ void update_vsyscall(struct timekeeper *tk) vdso_data->use_syscall = use_syscall; vdso_data->xtime_coarse_sec = xtime_coarse.tv_sec; vdso_data->xtime_coarse_nsec = xtime_coarse.tv_nsec; + vdso_data->wtm_clock_sec = tk->wall_to_monotonic.tv_sec; + vdso_data->wtm_clock_nsec = tk->wall_to_monotonic.tv_nsec; if (!use_syscall) { vdso_data->cs_cycle_last = tk->clock->cycle_last; @@ -245,8 +247,6 @@ void update_vsyscall(struct timekeeper *tk) vdso_data->xtime_clock_nsec = tk->xtime_nsec; vdso_data->cs_mult = tk->mult; vdso_data->cs_shift = tk->shift; - vdso_data->wtm_clock_sec = tk->wall_to_monotonic.tv_sec; - vdso_data->wtm_clock_nsec = tk->wall_to_monotonic.tv_nsec; } smp_wmb(); -- 1.8.3.1 ^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH 2/2] arm64: vdso: update wtm fields for CLOCK_MONOTONIC_COARSE 2014-02-03 19:48 ` [PATCH 2/2] arm64: vdso: update wtm fields for CLOCK_MONOTONIC_COARSE Nathan Lynch @ 2014-02-04 18:28 ` Will Deacon 0 siblings, 0 replies; 9+ messages in thread From: Will Deacon @ 2014-02-04 18:28 UTC (permalink / raw) To: linux-arm-kernel On Mon, Feb 03, 2014 at 07:48:52PM +0000, Nathan Lynch wrote: > Update wall-to-monotonic fields in the VDSO data page > unconditionally. These are used to service CLOCK_MONOTONIC_COARSE, > which is not guarded by use_syscall. > > Signed-off-by: Nathan Lynch <nathan_lynch@mentor.com> Acked-by: Will Deacon <will.deacon@arm.com> Will ^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2014-02-05 11:54 UTC | newest] Thread overview: 9+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2014-02-03 19:48 [PATCH 0/2] arm64: vdso fixes for coarse clocks Nathan Lynch 2014-02-03 19:48 ` [PATCH 1/2] arm64: vdso: fix coarse clock handling Nathan Lynch 2014-02-04 18:26 ` Will Deacon 2014-02-05 5:52 ` Nathan Lynch 2014-02-05 5:53 ` [PATCH v2 " Nathan Lynch 2014-02-05 10:09 ` Will Deacon 2014-02-05 11:54 ` Catalin Marinas 2014-02-03 19:48 ` [PATCH 2/2] arm64: vdso: update wtm fields for CLOCK_MONOTONIC_COARSE Nathan Lynch 2014-02-04 18:28 ` Will Deacon
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).