clock_getres in the vDSO library has to preserve the same behaviour
of posix_get_hrtimer_res().
In particular, posix_get_hrtimer_res() does:
sec = 0;
ns = hrtimer_resolution;
and hrtimer_resolution depends on the enablement of the high
resolution timers that can happen either at compile or at run time.
Fix the s390 vdso implementation of clock_getres keeping a copy of
hrtimer_resolution in vdso data and using that directly.
Cc: Heiko Carstens <[email protected]>
Cc: Vasily Gorbik <[email protected]>
Cc: Christian Borntraeger <[email protected]>
Signed-off-by: Vincenzo Frascino <[email protected]>
Acked-by: Martin Schwidefsky <[email protected]>
---
arch/s390/include/asm/vdso.h | 1 +
arch/s390/kernel/asm-offsets.c | 2 +-
arch/s390/kernel/time.c | 1 +
arch/s390/kernel/vdso64/clock_getres.S | 10 +++++-----
4 files changed, 8 insertions(+), 6 deletions(-)
diff --git a/arch/s390/include/asm/vdso.h b/arch/s390/include/asm/vdso.h
index 3bcfdeb01395..0cd085cdeb4f 100644
--- a/arch/s390/include/asm/vdso.h
+++ b/arch/s390/include/asm/vdso.h
@@ -36,6 +36,7 @@ struct vdso_data {
__u32 tk_shift; /* Shift used for xtime_nsec 0x60 */
__u32 ts_dir; /* TOD steering direction 0x64 */
__u64 ts_end; /* TOD steering end 0x68 */
+ __u32 hrtimer_res; /* hrtimer resolution 0x70 */
};
struct vdso_per_cpu_data {
diff --git a/arch/s390/kernel/asm-offsets.c b/arch/s390/kernel/asm-offsets.c
index ce33406cfe83..19b825d0b93d 100644
--- a/arch/s390/kernel/asm-offsets.c
+++ b/arch/s390/kernel/asm-offsets.c
@@ -76,6 +76,7 @@ int main(void)
OFFSET(__VDSO_TK_SHIFT, vdso_data, tk_shift);
OFFSET(__VDSO_TS_DIR, vdso_data, ts_dir);
OFFSET(__VDSO_TS_END, vdso_data, ts_end);
+ OFFSET(__VDSO_CLOCK_REALTIME_RES, vdso_data, hrtimer_res);
OFFSET(__VDSO_ECTG_BASE, vdso_per_cpu_data, ectg_timer_base);
OFFSET(__VDSO_ECTG_USER, vdso_per_cpu_data, ectg_user_time);
OFFSET(__VDSO_GETCPU_VAL, vdso_per_cpu_data, getcpu_val);
@@ -86,7 +87,6 @@ int main(void)
DEFINE(__CLOCK_REALTIME_COARSE, CLOCK_REALTIME_COARSE);
DEFINE(__CLOCK_MONOTONIC_COARSE, CLOCK_MONOTONIC_COARSE);
DEFINE(__CLOCK_THREAD_CPUTIME_ID, CLOCK_THREAD_CPUTIME_ID);
- DEFINE(__CLOCK_REALTIME_RES, MONOTONIC_RES_NSEC);
DEFINE(__CLOCK_COARSE_RES, LOW_RES_NSEC);
BLANK();
/* idle data offsets */
diff --git a/arch/s390/kernel/time.c b/arch/s390/kernel/time.c
index f9d070d016e3..b1113b519432 100644
--- a/arch/s390/kernel/time.c
+++ b/arch/s390/kernel/time.c
@@ -301,6 +301,7 @@ void update_vsyscall(struct timekeeper *tk)
vdso_data->tk_mult = tk->tkr_mono.mult;
vdso_data->tk_shift = tk->tkr_mono.shift;
+ vdso_data->hrtimer_res = hrtimer_resolution;
smp_wmb();
++vdso_data->tb_update_count;
}
diff --git a/arch/s390/kernel/vdso64/clock_getres.S b/arch/s390/kernel/vdso64/clock_getres.S
index 081435398e0a..022b58c980db 100644
--- a/arch/s390/kernel/vdso64/clock_getres.S
+++ b/arch/s390/kernel/vdso64/clock_getres.S
@@ -17,12 +17,14 @@
.type __kernel_clock_getres,@function
__kernel_clock_getres:
CFI_STARTPROC
- larl %r1,4f
+ larl %r1,3f
+ lg %r0,0(%r1)
cghi %r2,__CLOCK_REALTIME_COARSE
je 0f
cghi %r2,__CLOCK_MONOTONIC_COARSE
je 0f
- larl %r1,3f
+ larl %r1,_vdso_data
+ l %r0,__VDSO_CLOCK_REALTIME_RES(%r1)
cghi %r2,__CLOCK_REALTIME
je 0f
cghi %r2,__CLOCK_MONOTONIC
@@ -36,7 +38,6 @@ __kernel_clock_getres:
jz 2f
0: ltgr %r3,%r3
jz 1f /* res == NULL */
- lg %r0,0(%r1)
xc 0(8,%r3),0(%r3) /* set tp->tv_sec to zero */
stg %r0,8(%r3) /* store tp->tv_usec */
1: lghi %r2,0
@@ -45,6 +46,5 @@ __kernel_clock_getres:
svc 0
br %r14
CFI_ENDPROC
-3: .quad __CLOCK_REALTIME_RES
-4: .quad __CLOCK_COARSE_RES
+3: .quad __CLOCK_COARSE_RES
.size __kernel_clock_getres,.-__kernel_clock_getres
--
2.25.2
Hello Vincenzo,
sorry it took such a long time to answer!
> clock_getres in the vDSO library has to preserve the same behaviour
> of posix_get_hrtimer_res().
>
> In particular, posix_get_hrtimer_res() does:
> sec = 0;
> ns = hrtimer_resolution;
> and hrtimer_resolution depends on the enablement of the high
> resolution timers that can happen either at compile or at run time.
>
> Fix the s390 vdso implementation of clock_getres keeping a copy of
> hrtimer_resolution in vdso data and using that directly.
>
> Cc: Heiko Carstens <[email protected]>
> Cc: Vasily Gorbik <[email protected]>
> Cc: Christian Borntraeger <[email protected]>
> Signed-off-by: Vincenzo Frascino <[email protected]>
> Acked-by: Martin Schwidefsky <[email protected]>
> ---
> arch/s390/include/asm/vdso.h | 1 +
> arch/s390/kernel/asm-offsets.c | 2 +-
> arch/s390/kernel/time.c | 1 +
> arch/s390/kernel/vdso64/clock_getres.S | 10 +++++-----
> 4 files changed, 8 insertions(+), 6 deletions(-)
...
> diff --git a/arch/s390/kernel/vdso64/clock_getres.S b/arch/s390/kernel/vdso64/clock_getres.S
> index 081435398e0a..022b58c980db 100644
> --- a/arch/s390/kernel/vdso64/clock_getres.S
> +++ b/arch/s390/kernel/vdso64/clock_getres.S
> @@ -17,12 +17,14 @@
> .type __kernel_clock_getres,@function
> __kernel_clock_getres:
> CFI_STARTPROC
> - larl %r1,4f
> + larl %r1,3f
> + lg %r0,0(%r1)
> cghi %r2,__CLOCK_REALTIME_COARSE
> je 0f
> cghi %r2,__CLOCK_MONOTONIC_COARSE
> je 0f
> - larl %r1,3f
> + larl %r1,_vdso_data
> + l %r0,__VDSO_CLOCK_REALTIME_RES(%r1)
This should be llgf for proper zero extension. The code works anyway,
since the upper lg would clear the high order bits, however this looks
like it works more or less by accident ;)
I changed it and applied your patch. Thanks a lot!
Hi Heiko,
On 6/9/20 3:46 PM, Heiko Carstens wrote:
> Hello Vincenzo,
>
> sorry it took such a long time to answer!
>
>> clock_getres in the vDSO library has to preserve the same behaviour
>> of posix_get_hrtimer_res().
>>
>> In particular, posix_get_hrtimer_res() does:
>> sec = 0;
>> ns = hrtimer_resolution;
>> and hrtimer_resolution depends on the enablement of the high
>> resolution timers that can happen either at compile or at run time.
>>
>> Fix the s390 vdso implementation of clock_getres keeping a copy of
>> hrtimer_resolution in vdso data and using that directly.
>>
>> Cc: Heiko Carstens <[email protected]>
>> Cc: Vasily Gorbik <[email protected]>
>> Cc: Christian Borntraeger <[email protected]>
>> Signed-off-by: Vincenzo Frascino <[email protected]>
>> Acked-by: Martin Schwidefsky <[email protected]>
>> ---
>> arch/s390/include/asm/vdso.h | 1 +
>> arch/s390/kernel/asm-offsets.c | 2 +-
>> arch/s390/kernel/time.c | 1 +
>> arch/s390/kernel/vdso64/clock_getres.S | 10 +++++-----
>> 4 files changed, 8 insertions(+), 6 deletions(-)
> ...
>
>> diff --git a/arch/s390/kernel/vdso64/clock_getres.S b/arch/s390/kernel/vdso64/clock_getres.S
>> index 081435398e0a..022b58c980db 100644
>> --- a/arch/s390/kernel/vdso64/clock_getres.S
>> +++ b/arch/s390/kernel/vdso64/clock_getres.S
>> @@ -17,12 +17,14 @@
>> .type __kernel_clock_getres,@function
>> __kernel_clock_getres:
>> CFI_STARTPROC
>> - larl %r1,4f
>> + larl %r1,3f
>> + lg %r0,0(%r1)
>> cghi %r2,__CLOCK_REALTIME_COARSE
>> je 0f
>> cghi %r2,__CLOCK_MONOTONIC_COARSE
>> je 0f
>> - larl %r1,3f
>> + larl %r1,_vdso_data
>> + l %r0,__VDSO_CLOCK_REALTIME_RES(%r1)
>
> This should be llgf for proper zero extension. The code works anyway,
> since the upper lg would clear the high order bits, however this looks
> like it works more or less by accident ;)
>
> I changed it and applied your patch. Thanks a lot!
Thank you for this and for the explanation. I must admit that this was the first
attempt to write s390 assembly. I had to go through most of the "Principles of
Operation" before I could get something meaningful going ;)
>
--
Regards,
Vincenzo
P.S. Now I can finally say that once in my life I wrote s390 assembler ;)