Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
From:   Michael Ellerman <mpe@ellerman.id.au>
To:     Christophe Leroy <christophe.leroy@c-s.fr>,
        Benjamin Herrenschmidt <benh@kernel.crashing.org>,
        Paul Mackerras <paulus@samba.org>
Cc:     linux-kernel@vger.kernel.org, linuxppc-dev@lists.ozlabs.org
Subject: Re: [PATCH v3 2/8] powerpc/vdso32: Add support for CLOCK_{REALTIME/MONOTONIC}_COARSE
In-Reply-To: <4644ccc9b4da78639ae9424db878c48711abf05a.1572342582.git.christophe.leroy@c-s.fr>
References: <cover.1572342582.git.christophe.leroy@c-s.fr> <4644ccc9b4da78639ae9424db878c48711abf05a.1572342582.git.christophe.leroy@c-s.fr>
Date:   Wed, 20 Nov 2019 23:42:33 +1100
Message-ID: <87eey2btxi.fsf@mpe.ellerman.id.au>
MIME-Version: 1.0
Content-Type: text/plain
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

Christophe Leroy <christophe.leroy@c-s.fr> writes:
> This is copied and adapted from commit 5c929885f1bb ("powerpc/vdso64:
> Add support for CLOCK_{REALTIME/MONOTONIC}_COARSE")
> from Santosh Sivaraj <santosh@fossix.org>
>
> Benchmark from vdsotest-all:
> clock-gettime-realtime: syscall: 3601 nsec/call
> clock-gettime-realtime:    libc: 1072 nsec/call
> clock-gettime-realtime:    vdso: 931 nsec/call
> clock-gettime-monotonic: syscall: 4034 nsec/call
> clock-gettime-monotonic:    libc: 1213 nsec/call
> clock-gettime-monotonic:    vdso: 1076 nsec/call
> clock-gettime-realtime-coarse: syscall: 2722 nsec/call
> clock-gettime-realtime-coarse:    libc: 805 nsec/call
> clock-gettime-realtime-coarse:    vdso: 668 nsec/call
> clock-gettime-monotonic-coarse: syscall: 2949 nsec/call
> clock-gettime-monotonic-coarse:    libc: 882 nsec/call
> clock-gettime-monotonic-coarse:    vdso: 745 nsec/call
>
> Additional test passed with:
> 	vdsotest -d 30 clock-gettime-monotonic-coarse verify

This broke on 64-bit big endian, which uses the 32-bit VDSO, with errors
like:

  clock-gettime-monotonic-coarse/verify: 10 failures/inconsistencies encountered
  timestamp obtained from libc/vDSO not normalized:
  	[-1574202155, 1061008673]
  timestamp obtained from libc/vDSO predates timestamp
  previously obtained from kernel:
  	[74, 261310747] (kernel)
  	[-1574202155, 1061008673] (vDSO)
  timestamp obtained from libc/vDSO not normalized:
  	[-1574202155, 1061008673]
  timestamp obtained from libc/vDSO predates timestamp
  previously obtained from kernel:
  	[74, 261310747] (kernel)
  	[-1574202155, 1061008673] (vDSO)
  timestamp obtained from libc/vDSO not normalized:
  	[-1574202155, 1061008673]
  timestamp obtained from libc/vDSO predates timestamp
  previously obtained from kernel:
  	[74, 261310747] (kernel)
  	[-1574202155, 1061008673] (vDSO)
  timestamp obtained from libc/vDSO not normalized:
  	[-1574202155, 1061008673]
  timestamp obtained from libc/vDSO predates timestamp
  previously obtained from kernel:
  	[74, 261310747] (kernel)
  	[-1574202155, 1061008673] (vDSO)
  timestamp obtained from libc/vDSO not normalized:
  	[-1574202155, 1061008673]
  timestamp obtained from libc/vDSO predates timestamp
  previously obtained from kernel:
  	[74, 261310747] (kernel)
  	[-1574202155, 1061008673] (vDSO)
  Failure threshold (10) reached; stopping test.


The diff below seems to fix it, but I'm not sure it's correct. ie. we
just ignore the top part of the values, how does that work?

cheers


diff --git a/arch/powerpc/kernel/vdso32/gettimeofday.S b/arch/powerpc/kernel/vdso32/gettimeofday.S
index decd263c16e0..58f7e1124d92 100644
--- a/arch/powerpc/kernel/vdso32/gettimeofday.S
+++ b/arch/powerpc/kernel/vdso32/gettimeofday.S
@@ -16,9 +16,11 @@
 #ifdef CONFIG_PPC64
 #define LOPART 4
 #define TSPEC_TV_SEC   TSPC64_TV_SEC+LOPART
+#define TSPEC_TV_NSEC  TSPC64_TV_NSEC+LOPART
 #else
 #define LOPART 0
 #define TSPEC_TV_SEC   TSPC32_TV_SEC
+#define TSPEC_TV_NSEC  TSPC32_TV_NSEC
 #endif
 
        .text
@@ -131,8 +133,8 @@ V_FUNCTION_BEGIN(__kernel_clock_gettime)
         * CLOCK_REALTIME_COARSE, below values are needed for MONOTONIC_COARSE
         * too
         */
-       lwz     r3,STAMP_XTIME+TSPC32_TV_SEC(r9)
-       lwz     r4,STAMP_XTIME+TSPC32_TV_NSEC(r9)
+       lwz     r3,STAMP_XTIME+TSPEC_TV_SEC(r9)
+       lwz     r4,STAMP_XTIME+TSPEC_TV_NSEC(r9)
        bne     cr6,1f
 
        /* CLOCK_MONOTONIC_COARSE */