On Tue, Mar 23, 2021 at 02:21:52PM +0800, Li Wang wrote:
> Hi linux-s390 experts,
>
> We observed that LTP/clock_gettime04 always FAIL on s390x with
> kernel-v5.12-rc3.
> To simply show the problem, I rewrite the LTP reproducer as a simple C
> below.
> Maybe it's a new bug introduced from the kernel-5.12 series branch?
>
> PASS:
> ------------
> # uname -r
> 5.11.0-*.s390x
>
> # grep TIME_NS /boot/config-5.11.0-*.s390x
> no TIME_NS enabled
>
> ## ./test-timer
> vdso_ts_nsec = 898169901815, vdso_ts.tv_sec = 898, vdso_ts.tv_nsec =
> 169901815
> sys_ts_nsec = 898169904269, sys_ts.tv_sec = 898, sys_ts.tv_nsec =
> 169904269
> ===> PASS
>
> FAIL:
> ----------
> # uname -r
> 5.12.0-0.rc3.*.s390x
>
> # grep TIME_NS /boot/config-5.12.0-0.rc3.s390x
> CONFIG_TIME_NS=y
> CONFIG_GENERIC_VDSO_TIME_NS=y
>
> # ./test-timer
> vdso_ts_nsec = 4484351380985507, vdso_ts.tv_sec = 4484351, vdso_ts.tv_nsec
> = 380985507
> sys_ts_nsec = 1446923235377, sys_ts.tv_sec = 1446, sys_ts.tv_nsec =
> 923235377
> ===> FAIL
Thanks for reporting!
I'll look later today into this. I would nearly bet that I broke it
with commit f8d8977a3d97 ("s390/time: convert tod_clock_base to
union")
On Tue, Mar 23, 2021 at 08:11:41AM +0100, Heiko Carstens wrote:
> On Tue, Mar 23, 2021 at 02:21:52PM +0800, Li Wang wrote:
> > Hi linux-s390 experts,
> >
> > We observed that LTP/clock_gettime04 always FAIL on s390x with
> > kernel-v5.12-rc3.
> > To simply show the problem, I rewrite the LTP reproducer as a simple C
> > below.
> > Maybe it's a new bug introduced from the kernel-5.12 series branch?
> >
> > PASS:
> > ------------
> > # uname -r
> > 5.11.0-*.s390x
> >
> > # grep TIME_NS /boot/config-5.11.0-*.s390x
> > no TIME_NS enabled
> >
> > ## ./test-timer
> > vdso_ts_nsec = 898169901815, vdso_ts.tv_sec = 898, vdso_ts.tv_nsec =
> > 169901815
> > sys_ts_nsec = 898169904269, sys_ts.tv_sec = 898, sys_ts.tv_nsec =
> > 169904269
> > ===> PASS
> >
> > FAIL:
> > ----------
> > # uname -r
> > 5.12.0-0.rc3.*.s390x
> >
> > # grep TIME_NS /boot/config-5.12.0-0.rc3.s390x
> > CONFIG_TIME_NS=y
> > CONFIG_GENERIC_VDSO_TIME_NS=y
> >
> > # ./test-timer
> > vdso_ts_nsec = 4484351380985507, vdso_ts.tv_sec = 4484351, vdso_ts.tv_nsec
> > = 380985507
> > sys_ts_nsec = 1446923235377, sys_ts.tv_sec = 1446, sys_ts.tv_nsec =
> > 923235377
> > ===> FAIL
>
> Thanks for reporting!
>
> I'll look later today into this. I would nearly bet that I broke it
> with commit f8d8977a3d97 ("s390/time: convert tod_clock_base to
> union")
So, I broke it with commit 1ba2d6c0fd4e ("s390/vdso: simplify
__arch_get_hw_counter()"). Reverting that patch will fix it for non
time namespace processes only.
The problem is that the vdso data page contains an array of struct
vdso_data's for each clock source. However only the first member of
that array contains a/the valid struct arch_vdso_data, which is
required for __arch_get_hw_counter(). Which alone is a bit odd...
However for a process which is within a time namespace there is no
(easy) way to access that page (the time namespace specific vdso data
page does not contain valid arch_vdso_data). I guess the real fix is
to simply map yet another page into the vvar mapping and put the
arch_data there. What a mess... :/
Li Wang reported that clock_gettime(CLOCK_MONOTONIC_RAW, ...) does not
work correctly on s390 via vdso. Debugging this also revealed an
unrelated bug (first patch).
The second patch fixes the problem: the tod clock steering parameters
required by __arch_get_hw_counter() are only present within the first
element of the _vdso_data array and not at all within the _timens_data
array.
Instead of working around this simply provide an s390 specific vdso
data page which contains the tod clock steering parameters.
This allows also to remove ARCH_HAS_VDSO_DATA again.
Heiko Carstens (3):
s390/vdso: fix tod clock steering
s390/vdso: fix arch_data access for __arch_get_hw_counter()
lib/vdso: remove struct arch_vdso_data from vdso data struct
arch/Kconfig | 3 ---
arch/s390/Kconfig | 1 -
arch/s390/include/asm/vdso.h | 4 +++-
arch/s390/include/asm/vdso/data.h | 13 ------------
arch/s390/include/asm/vdso/datapage.h | 17 +++++++++++++++
arch/s390/include/asm/vdso/gettimeofday.h | 11 ++++++++--
arch/s390/kernel/time.c | 5 +++--
arch/s390/kernel/vdso.c | 25 ++++++++++++++++++++---
arch/s390/kernel/vdso64/vdso64.lds.S | 3 ++-
include/vdso/datapage.h | 10 ---------
10 files changed, 56 insertions(+), 36 deletions(-)
delete mode 100644 arch/s390/include/asm/vdso/data.h
create mode 100644 arch/s390/include/asm/vdso/datapage.h
--
2.25.1
On Thu, Mar 25, 2021 at 04:56:18PM +0800, Li Wang wrote:
> Hi Heiko,
>
> On Wed, Mar 24, 2021 at 5:58 AM Heiko Carstens <[email protected]> wrote:
>
> > Li Wang reported that clock_gettime(CLOCK_MONOTONIC_RAW, ...) does not
> > work correctly on s390 via vdso. Debugging this also revealed an
> > unrelated bug (first patch).
> >
> > The second patch fixes the problem: the tod clock steering parameters
> > required by __arch_get_hw_counter() are only present within the first
> > element of the _vdso_data array and not at all within the _timens_data
> > array.
> >
> > Instead of working around this simply provide an s390 specific vdso
> > data page which contains the tod clock steering parameters.
> >
> > This allows also to remove ARCH_HAS_VDSO_DATA again.
> >
> > Heiko Carstens (3):
> > s390/vdso: fix tod clock steering
> > s390/vdso: fix arch_data access for __arch_get_hw_counter()
> > lib/vdso: remove struct arch_vdso_data from vdso data struct
> >
>
> Thanks for the quick fix! I confirmed these patches work for me.
> (tested with latest mainline kernel v5.12-rc4)
>
> Tested-by: Li Wang <[email protected]>
Thanks a lot for confirming! However I decided to go with the simple variant:
https://lore.kernel.org/linux-s390/YFnxr1ZlMIOIqjfq@osiris/T/#m26f94fd8ac048421a4a8870e7259a09f97840a3e
May I add your Tested-by there as well?