2019-12-02 07:59:33

by Christophe Leroy

Subject: [PATCH v4 7/8] powerpc/vdso32: implement clock_getres entirely

clock_getres returns hrtimer_res for all clocks but coarse ones
for which it returns KTIME_LOW_RES.

return EINVAL for unknown clocks.

Signed-off-by: Christophe Leroy <[email protected]>
---
arch/powerpc/kernel/asm-offsets.c | 3 +++
arch/powerpc/kernel/vdso32/gettimeofday.S | 19 +++++++++++--------
2 files changed, 14 insertions(+), 8 deletions(-)

diff --git a/arch/powerpc/kernel/asm-offsets.c b/arch/powerpc/kernel/asm-offsets.c
index 0013197d89a6..90e53d432f2e 100644
--- a/arch/powerpc/kernel/asm-offsets.c
+++ b/arch/powerpc/kernel/asm-offsets.c
@@ -413,7 +413,10 @@ int main(void)
DEFINE(CLOCK_MONOTONIC, CLOCK_MONOTONIC);
DEFINE(CLOCK_REALTIME_COARSE, CLOCK_REALTIME_COARSE);
DEFINE(CLOCK_MONOTONIC_COARSE, CLOCK_MONOTONIC_COARSE);
+ DEFINE(CLOCK_MAX, CLOCK_TAI);
DEFINE(NSEC_PER_SEC, NSEC_PER_SEC);
+ DEFINE(EINVAL, EINVAL);
+ DEFINE(KTIME_LOW_RES, KTIME_LOW_RES);

#ifdef CONFIG_BUG
DEFINE(BUG_ENTRY_SIZE, sizeof(struct bug_entry));
diff --git a/arch/powerpc/kernel/vdso32/gettimeofday.S b/arch/powerpc/kernel/vdso32/gettimeofday.S
index 9aafacea9c4a..20ae38f3a5a3 100644
--- a/arch/powerpc/kernel/vdso32/gettimeofday.S
+++ b/arch/powerpc/kernel/vdso32/gettimeofday.S
@@ -196,17 +196,20 @@ V_FUNCTION_END(__kernel_clock_gettime)
V_FUNCTION_BEGIN(__kernel_clock_getres)
.cfi_startproc
/* Check for supported clock IDs */
- cmpwi cr0,r3,CLOCK_REALTIME
- cmpwi cr1,r3,CLOCK_MONOTONIC
- cror cr0*4+eq,cr0*4+eq,cr1*4+eq
- bne cr0,99f
+ cmplwi cr0, r3, CLOCK_MAX
+ cmpwi cr1, r3, CLOCK_REALTIME_COARSE
+ cmpwi cr7, r3, CLOCK_MONOTONIC_COARSE
+ bgt cr0, 99f
+ LOAD_REG_IMMEDIATE(r5, KTIME_LOW_RES)
+ beq cr1, 1f
+ beq cr7, 1f

mflr r12
.cfi_register lr,r12
get_datapage r3, r0
lwz r5, CLOCK_HRTIMER_RES(r3)
mtlr r12
- li r3,0
+1: li r3,0
cmpli cr0,r4,0
crclr cr0*4+so
beqlr
@@ -215,11 +218,11 @@ V_FUNCTION_BEGIN(__kernel_clock_getres)
blr

/*
- * syscall fallback
+ * invalid clock
*/
99:
- li r0,__NR_clock_getres
- sc
+ li r3, EINVAL
+ crset so
blr
.cfi_endproc
V_FUNCTION_END(__kernel_clock_getres)
--
2.13.3

2020-05-05 23:25:47

by Aurelien Jarno

Subject: Re: [PATCH v4 7/8] powerpc/vdso32: implement clock_getres entirely

Hi,

On 2019-12-02 07:57, Christophe Leroy wrote:
> clock_getres returns hrtimer_res for all clocks but coarse ones
> for which it returns KTIME_LOW_RES.
>
> return EINVAL for unknown clocks.
>
> Signed-off-by: Christophe Leroy <[email protected]>
> ---
> arch/powerpc/kernel/asm-offsets.c | 3 +++
> arch/powerpc/kernel/vdso32/gettimeofday.S | 19 +++++++++++--------
> 2 files changed, 14 insertions(+), 8 deletions(-)
>
> diff --git a/arch/powerpc/kernel/asm-offsets.c b/arch/powerpc/kernel/asm-offsets.c
> index 0013197d89a6..90e53d432f2e 100644
> --- a/arch/powerpc/kernel/asm-offsets.c
> +++ b/arch/powerpc/kernel/asm-offsets.c
> @@ -413,7 +413,10 @@ int main(void)
> DEFINE(CLOCK_MONOTONIC, CLOCK_MONOTONIC);
> DEFINE(CLOCK_REALTIME_COARSE, CLOCK_REALTIME_COARSE);
> DEFINE(CLOCK_MONOTONIC_COARSE, CLOCK_MONOTONIC_COARSE);
> + DEFINE(CLOCK_MAX, CLOCK_TAI);
> DEFINE(NSEC_PER_SEC, NSEC_PER_SEC);
> + DEFINE(EINVAL, EINVAL);
> + DEFINE(KTIME_LOW_RES, KTIME_LOW_RES);
>
> #ifdef CONFIG_BUG
> DEFINE(BUG_ENTRY_SIZE, sizeof(struct bug_entry));
> diff --git a/arch/powerpc/kernel/vdso32/gettimeofday.S b/arch/powerpc/kernel/vdso32/gettimeofday.S
> index 9aafacea9c4a..20ae38f3a5a3 100644
> --- a/arch/powerpc/kernel/vdso32/gettimeofday.S
> +++ b/arch/powerpc/kernel/vdso32/gettimeofday.S
> @@ -196,17 +196,20 @@ V_FUNCTION_END(__kernel_clock_gettime)
> V_FUNCTION_BEGIN(__kernel_clock_getres)
> .cfi_startproc
> /* Check for supported clock IDs */
> - cmpwi cr0,r3,CLOCK_REALTIME
> - cmpwi cr1,r3,CLOCK_MONOTONIC
> - cror cr0*4+eq,cr0*4+eq,cr1*4+eq
> - bne cr0,99f
> + cmplwi cr0, r3, CLOCK_MAX
> + cmpwi cr1, r3, CLOCK_REALTIME_COARSE
> + cmpwi cr7, r3, CLOCK_MONOTONIC_COARSE
> + bgt cr0, 99f
> + LOAD_REG_IMMEDIATE(r5, KTIME_LOW_RES)
> + beq cr1, 1f
> + beq cr7, 1f
>
> mflr r12
> .cfi_register lr,r12
> get_datapage r3, r0
> lwz r5, CLOCK_HRTIMER_RES(r3)
> mtlr r12
> - li r3,0
> +1: li r3,0
> cmpli cr0,r4,0
> crclr cr0*4+so
> beqlr
> @@ -215,11 +218,11 @@ V_FUNCTION_BEGIN(__kernel_clock_getres)
> blr
>
> /*
> - * syscall fallback
> + * invalid clock
> */
> 99:
> - li r0,__NR_clock_getres
> - sc
> + li r3, EINVAL
> + crset so
> blr
> .cfi_endproc
> V_FUNCTION_END(__kernel_clock_getres)

Removing the syscall fallback looks wrong, and broke access to
per-processes clocks. With this change a few glibc tests now fail.

This can be reproduced by the simple code below:

| #include <errno.h>
| #include <stdio.h>
| #include <string.h>
| #include <sys/types.h>
| #include <time.h>
| #include <unistd.h>
|
| int main()
| {
| struct timespec res;
| clockid_t ci;
| int e;
|
| e = clock_getcpuclockid(getpid(), &ci);
| if (e) {
| printf("clock_getcpuclockid returned %d\n", e);
| return e;
| }
| e = clock_getres (ci, &res);
| printf("clock_getres returned %d\n", e);
| if (e) {
| printf(" errno: %d, %s\n", errno, strerror(errno));
| }
|
| return e;
| }

Without this patch or with -m64, it returns:

| clock_getres returned 0

With this patch with -m32 it returns:

| clock_getres returned -1
| errno: 22, Invalid argument

Regards,
Aurelien

--
Aurelien Jarno GPG: 4096R/1DDD8C9B
[email protected] http://www.aurel32.net