linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v6] s390: Fix vDSO clock_getres()
@ 2020-03-24 12:10 Vincenzo Frascino
  2020-06-09 14:46 ` Heiko Carstens
  0 siblings, 1 reply; 3+ messages in thread
From: Vincenzo Frascino @ 2020-03-24 12:10 UTC (permalink / raw)
  To: linux-s390, linux-kernel
  Cc: tglx, Vincenzo Frascino, Heiko Carstens, Vasily Gorbik,
	Christian Borntraeger, Martin Schwidefsky

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 <heiko.carstens@de.ibm.com>
Cc: Vasily Gorbik <gor@linux.ibm.com>
Cc: Christian Borntraeger <borntraeger@de.ibm.com>
Signed-off-by: Vincenzo Frascino <vincenzo.frascino@arm.com>
Acked-by: Martin Schwidefsky <schwidefsky@de.ibm.com>
---
 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


^ permalink raw reply related	[flat|nested] 3+ messages in thread
* Re: [PATCH v6] s390: Fix vDSO clock_getres()
  2020-03-24 12:10 [PATCH v6] s390: Fix vDSO clock_getres() Vincenzo Frascino
@ 2020-06-09 14:46 ` Heiko Carstens
  2020-06-09 15:12   ` Vincenzo Frascino
  0 siblings, 1 reply; 3+ messages in thread
From: Heiko Carstens @ 2020-06-09 14:46 UTC (permalink / raw)
  To: Vincenzo Frascino
  Cc: linux-s390, linux-kernel, tglx, Vasily Gorbik,
	Christian Borntraeger, Martin Schwidefsky

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 <heiko.carstens@de.ibm.com>
> Cc: Vasily Gorbik <gor@linux.ibm.com>
> Cc: Christian Borntraeger <borntraeger@de.ibm.com>
> Signed-off-by: Vincenzo Frascino <vincenzo.frascino@arm.com>
> Acked-by: Martin Schwidefsky <schwidefsky@de.ibm.com>
> ---
>  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!

^ permalink raw reply	[flat|nested] 3+ messages in thread
* Re: [PATCH v6] s390: Fix vDSO clock_getres()
  2020-06-09 14:46 ` Heiko Carstens
@ 2020-06-09 15:12   ` Vincenzo Frascino
  0 siblings, 0 replies; 3+ messages in thread
From: Vincenzo Frascino @ 2020-06-09 15:12 UTC (permalink / raw)
  To: Heiko Carstens
  Cc: linux-s390, linux-kernel, tglx, Vasily Gorbik,
	Christian Borntraeger, Martin Schwidefsky

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 <heiko.carstens@de.ibm.com>
>> Cc: Vasily Gorbik <gor@linux.ibm.com>
>> Cc: Christian Borntraeger <borntraeger@de.ibm.com>
>> Signed-off-by: Vincenzo Frascino <vincenzo.frascino@arm.com>
>> Acked-by: Martin Schwidefsky <schwidefsky@de.ibm.com>
>> ---
>>  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 ;)

^ permalink raw reply	[flat|nested] 3+ messages in thread
end of thread, other threads:[~2020-06-09 15:10 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-03-24 12:10 [PATCH v6] s390: Fix vDSO clock_getres() Vincenzo Frascino
2020-06-09 14:46 ` Heiko Carstens
2020-06-09 15:12   ` Vincenzo Frascino

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).