* [LTP] [PATCH 0/3] s390 vdso fixes
@ 2021-03-23 21:58 ` Heiko Carstens
0 siblings, 0 replies; 25+ messages in thread
From: Heiko Carstens @ 2021-03-23 21:58 UTC (permalink / raw)
To: ltp
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
^ permalink raw reply [flat|nested] 25+ messages in thread
* [PATCH 1/3] s390/vdso: fix tod clock steering
2021-03-23 21:58 ` [LTP] " Heiko Carstens
@ 2021-03-23 21:58 ` Heiko Carstens
-1 siblings, 0 replies; 25+ messages in thread
From: Heiko Carstens @ 2021-03-23 21:58 UTC (permalink / raw)
To: Li Wang, Alexander Gordeev, Vasily Gorbik, Sven Schnelle,
Christian Borntraeger, Viresh Kumar, Thomas Gleixner
Cc: ltp, linux-kernel, linux-s390
The s390 specific vdso function __arch_get_hw_counter() is supposed to
consider tod clock steering.
If a tod clock steering event happens and the tod clock is set to a
new value __arch_get_hw_counter() will not return the real tod clock
value but slowly drift it from the old delta until the returned value
finally matches the real tod clock value again.
When converting the assembler code to C it was forgotten to tell user
space in which direction the clock has to be adjusted.
Worst case is now that instead of drifting the clock slowly it will
jump into the opposite direction by a factor of two.
Fix this by simply providing the missing value to user space.
Fixes: 4bff8cb54502 ("s390: convert to GENERIC_VDSO")
Cc: <stable@vger.kernel.org> # 5.10
Signed-off-by: Heiko Carstens <hca@linux.ibm.com>
---
arch/s390/kernel/time.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/arch/s390/kernel/time.c b/arch/s390/kernel/time.c
index 165da961f901..e37285a5101b 100644
--- a/arch/s390/kernel/time.c
+++ b/arch/s390/kernel/time.c
@@ -382,6 +382,7 @@ static void clock_sync_global(unsigned long delta)
tod_steering_delta);
tod_steering_end = now + (abs(tod_steering_delta) << 15);
vdso_data->arch_data.tod_steering_end = tod_steering_end;
+ vdso_data->arch_data.tod_steering_delta = tod_steering_delta;
/* Update LPAR offset. */
if (ptff_query(PTFF_QTO) && ptff(&qto, sizeof(qto), PTFF_QTO) == 0)
--
2.25.1
^ permalink raw reply related [flat|nested] 25+ messages in thread
* [LTP] [PATCH 1/3] s390/vdso: fix tod clock steering
@ 2021-03-23 21:58 ` Heiko Carstens
0 siblings, 0 replies; 25+ messages in thread
From: Heiko Carstens @ 2021-03-23 21:58 UTC (permalink / raw)
To: ltp
The s390 specific vdso function __arch_get_hw_counter() is supposed to
consider tod clock steering.
If a tod clock steering event happens and the tod clock is set to a
new value __arch_get_hw_counter() will not return the real tod clock
value but slowly drift it from the old delta until the returned value
finally matches the real tod clock value again.
When converting the assembler code to C it was forgotten to tell user
space in which direction the clock has to be adjusted.
Worst case is now that instead of drifting the clock slowly it will
jump into the opposite direction by a factor of two.
Fix this by simply providing the missing value to user space.
Fixes: 4bff8cb54502 ("s390: convert to GENERIC_VDSO")
Cc: <stable@vger.kernel.org> # 5.10
Signed-off-by: Heiko Carstens <hca@linux.ibm.com>
---
arch/s390/kernel/time.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/arch/s390/kernel/time.c b/arch/s390/kernel/time.c
index 165da961f901..e37285a5101b 100644
--- a/arch/s390/kernel/time.c
+++ b/arch/s390/kernel/time.c
@@ -382,6 +382,7 @@ static void clock_sync_global(unsigned long delta)
tod_steering_delta);
tod_steering_end = now + (abs(tod_steering_delta) << 15);
vdso_data->arch_data.tod_steering_end = tod_steering_end;
+ vdso_data->arch_data.tod_steering_delta = tod_steering_delta;
/* Update LPAR offset. */
if (ptff_query(PTFF_QTO) && ptff(&qto, sizeof(qto), PTFF_QTO) == 0)
--
2.25.1
^ permalink raw reply related [flat|nested] 25+ messages in thread
* Re: [PATCH 1/3] s390/vdso: fix tod clock steering
2021-03-23 21:58 ` [LTP] " Heiko Carstens
@ 2021-03-24 9:50 ` Heiko Carstens
-1 siblings, 0 replies; 25+ messages in thread
From: Heiko Carstens @ 2021-03-24 9:50 UTC (permalink / raw)
To: Heiko Carstens
Cc: Li Wang, Alexander Gordeev, Vasily Gorbik, Sven Schnelle,
Christian Borntraeger, Viresh Kumar, Thomas Gleixner, ltp,
linux-kernel, linux-s390
On Tue, Mar 23, 2021 at 10:58:17PM +0100, Heiko Carstens wrote:
> The s390 specific vdso function __arch_get_hw_counter() is supposed to
> consider tod clock steering.
>
> If a tod clock steering event happens and the tod clock is set to a
> new value __arch_get_hw_counter() will not return the real tod clock
> value but slowly drift it from the old delta until the returned value
> finally matches the real tod clock value again.
>
> When converting the assembler code to C it was forgotten to tell user
> space in which direction the clock has to be adjusted.
>
> Worst case is now that instead of drifting the clock slowly it will
> jump into the opposite direction by a factor of two.
>
> Fix this by simply providing the missing value to user space.
>
> Fixes: 4bff8cb54502 ("s390: convert to GENERIC_VDSO")
> Cc: <stable@vger.kernel.org> # 5.10
> Signed-off-by: Heiko Carstens <hca@linux.ibm.com>
> ---
> arch/s390/kernel/time.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/arch/s390/kernel/time.c b/arch/s390/kernel/time.c
> index 165da961f901..e37285a5101b 100644
> --- a/arch/s390/kernel/time.c
> +++ b/arch/s390/kernel/time.c
> @@ -382,6 +382,7 @@ static void clock_sync_global(unsigned long delta)
> tod_steering_delta);
> tod_steering_end = now + (abs(tod_steering_delta) << 15);
> vdso_data->arch_data.tod_steering_end = tod_steering_end;
> + vdso_data->arch_data.tod_steering_delta = tod_steering_delta;
..and yet another bug: __arch_get_hw_counter() tests if
tod_steering_delta is negative.
It makes sense to give tod_steering_delta the correct type:
diff --git a/arch/s390/include/asm/vdso/data.h b/arch/s390/include/asm/vdso/data.h
index 7b3cdb4a5f48..73ee89142666 100644
--- a/arch/s390/include/asm/vdso/data.h
+++ b/arch/s390/include/asm/vdso/data.h
@@ -6,7 +6,7 @@
#include <vdso/datapage.h>
struct arch_vdso_data {
- __u64 tod_steering_delta;
+ __s64 tod_steering_delta;
__u64 tod_steering_end;
};
^ permalink raw reply related [flat|nested] 25+ messages in thread
* [LTP] [PATCH 1/3] s390/vdso: fix tod clock steering
@ 2021-03-24 9:50 ` Heiko Carstens
0 siblings, 0 replies; 25+ messages in thread
From: Heiko Carstens @ 2021-03-24 9:50 UTC (permalink / raw)
To: ltp
On Tue, Mar 23, 2021 at 10:58:17PM +0100, Heiko Carstens wrote:
> The s390 specific vdso function __arch_get_hw_counter() is supposed to
> consider tod clock steering.
>
> If a tod clock steering event happens and the tod clock is set to a
> new value __arch_get_hw_counter() will not return the real tod clock
> value but slowly drift it from the old delta until the returned value
> finally matches the real tod clock value again.
>
> When converting the assembler code to C it was forgotten to tell user
> space in which direction the clock has to be adjusted.
>
> Worst case is now that instead of drifting the clock slowly it will
> jump into the opposite direction by a factor of two.
>
> Fix this by simply providing the missing value to user space.
>
> Fixes: 4bff8cb54502 ("s390: convert to GENERIC_VDSO")
> Cc: <stable@vger.kernel.org> # 5.10
> Signed-off-by: Heiko Carstens <hca@linux.ibm.com>
> ---
> arch/s390/kernel/time.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/arch/s390/kernel/time.c b/arch/s390/kernel/time.c
> index 165da961f901..e37285a5101b 100644
> --- a/arch/s390/kernel/time.c
> +++ b/arch/s390/kernel/time.c
> @@ -382,6 +382,7 @@ static void clock_sync_global(unsigned long delta)
> tod_steering_delta);
> tod_steering_end = now + (abs(tod_steering_delta) << 15);
> vdso_data->arch_data.tod_steering_end = tod_steering_end;
> + vdso_data->arch_data.tod_steering_delta = tod_steering_delta;
..and yet another bug: __arch_get_hw_counter() tests if
tod_steering_delta is negative.
It makes sense to give tod_steering_delta the correct type:
diff --git a/arch/s390/include/asm/vdso/data.h b/arch/s390/include/asm/vdso/data.h
index 7b3cdb4a5f48..73ee89142666 100644
--- a/arch/s390/include/asm/vdso/data.h
+++ b/arch/s390/include/asm/vdso/data.h
@@ -6,7 +6,7 @@
#include <vdso/datapage.h>
struct arch_vdso_data {
- __u64 tod_steering_delta;
+ __s64 tod_steering_delta;
__u64 tod_steering_end;
};
^ permalink raw reply related [flat|nested] 25+ messages in thread
* [PATCH 2/3] s390/vdso: fix arch_data access for __arch_get_hw_counter()
2021-03-23 21:58 ` [LTP] " Heiko Carstens
@ 2021-03-23 21:58 ` Heiko Carstens
-1 siblings, 0 replies; 25+ messages in thread
From: Heiko Carstens @ 2021-03-23 21:58 UTC (permalink / raw)
To: Li Wang, Alexander Gordeev, Vasily Gorbik, Sven Schnelle,
Christian Borntraeger, Viresh Kumar, Thomas Gleixner
Cc: ltp, linux-kernel, linux-s390
Li Wang reported that clock_gettime(CLOCK_MONOTONIC_RAW, ...) returns
incorrect values when time is provided via vdso instead of system call:
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
Within the s390 specific vdso function __arch_get_hw_counter() tries
to read tod clock steering values from the arch_data member of the
passed in vdso_data structure.
However only the arch_data member of the first clock source base
(CS_HRES_COARSE) is initialized. For CS_RAW arch_data is not at all
initialized, which explains the incorrect returned values.
It is a bit odd to provide the required tod clock steering parameters
only within the first element of the _vdso_data array. However for
time namespaces even no member of the _timens_data array contains the
required data, which would make fixing __arch_get_hw_counter() quite
complicated.
Therefore simply add an s390 specific vdso data page which contains
the tod clock steering parameters. Everything else seems to be
unnecessary complex.
Reported-by: Li Wang <liwang@redhat.com>
Fixes: 1ba2d6c0fd4e ("s390/vdso: simplify __arch_get_hw_counter()")
Fixes: eeab78b05d20 ("s390/vdso: implement generic vdso time namespace support")
Link: https://lore.kernel.org/linux-s390/YFnxr1ZlMIOIqjfq@osiris
Signed-off-by: Heiko Carstens <hca@linux.ibm.com>
---
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 | 6 +++---
arch/s390/kernel/vdso.c | 25 ++++++++++++++++++++---
arch/s390/kernel/vdso64/vdso64.lds.S | 3 ++-
8 files changed, 56 insertions(+), 24 deletions(-)
delete mode 100644 arch/s390/include/asm/vdso/data.h
create mode 100644 arch/s390/include/asm/vdso/datapage.h
diff --git a/arch/s390/Kconfig b/arch/s390/Kconfig
index c1ff874e6c2e..532ce0fcc659 100644
--- a/arch/s390/Kconfig
+++ b/arch/s390/Kconfig
@@ -77,7 +77,6 @@ config S390
select ARCH_HAS_STRICT_MODULE_RWX
select ARCH_HAS_SYSCALL_WRAPPER
select ARCH_HAS_UBSAN_SANITIZE_ALL
- select ARCH_HAS_VDSO_DATA
select ARCH_HAVE_NMI_SAFE_CMPXCHG
select ARCH_INLINE_READ_LOCK
select ARCH_INLINE_READ_LOCK_BH
diff --git a/arch/s390/include/asm/vdso.h b/arch/s390/include/asm/vdso.h
index b45e3dddd2c2..0d047f519df6 100644
--- a/arch/s390/include/asm/vdso.h
+++ b/arch/s390/include/asm/vdso.h
@@ -3,17 +3,19 @@
#define __S390_VDSO_H__
#include <vdso/datapage.h>
+#include <asm/vdso/datapage.h>
/* Default link address for the vDSO */
#define VDSO64_LBASE 0
-#define __VVAR_PAGES 2
+#define __VVAR_PAGES 3
#define VDSO_VERSION_STRING LINUX_2.6.29
#ifndef __ASSEMBLY__
extern struct vdso_data *vdso_data;
+extern struct s390_vdso_data *s390_vdso_data;
int vdso_getcpu_init(void);
diff --git a/arch/s390/include/asm/vdso/data.h b/arch/s390/include/asm/vdso/data.h
deleted file mode 100644
index 7b3cdb4a5f48..000000000000
--- a/arch/s390/include/asm/vdso/data.h
+++ /dev/null
@@ -1,13 +0,0 @@
-/* SPDX-License-Identifier: GPL-2.0 */
-#ifndef __S390_ASM_VDSO_DATA_H
-#define __S390_ASM_VDSO_DATA_H
-
-#include <linux/types.h>
-#include <vdso/datapage.h>
-
-struct arch_vdso_data {
- __u64 tod_steering_delta;
- __u64 tod_steering_end;
-};
-
-#endif /* __S390_ASM_VDSO_DATA_H */
diff --git a/arch/s390/include/asm/vdso/datapage.h b/arch/s390/include/asm/vdso/datapage.h
new file mode 100644
index 000000000000..bfae78d814af
--- /dev/null
+++ b/arch/s390/include/asm/vdso/datapage.h
@@ -0,0 +1,17 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+#ifndef __S390_ASM_VDSO_DATAPAGE_H
+#define __S390_ASM_VDSO_DATAPAGE_H
+
+#include <linux/types.h>
+
+#ifndef __ASSEMBLY__
+
+struct s390_vdso_data {
+ __u64 tod_steering_delta;
+ __u64 tod_steering_end;
+};
+
+extern struct s390_vdso_data _s390_data __attribute__((visibility("hidden")));
+
+#endif /* __ASSEMBLY__ */
+#endif /* __S390_ASM_VDSO_DATAPAGE_H */
diff --git a/arch/s390/include/asm/vdso/gettimeofday.h b/arch/s390/include/asm/vdso/gettimeofday.h
index ed89ef742530..bbd6da6b1651 100644
--- a/arch/s390/include/asm/vdso/gettimeofday.h
+++ b/arch/s390/include/asm/vdso/gettimeofday.h
@@ -9,6 +9,7 @@
#include <asm/timex.h>
#include <asm/unistd.h>
#include <asm/vdso.h>
+#include <asm/vdso/datapage.h>
#include <linux/compiler.h>
#define vdso_calc_delta __arch_vdso_calc_delta
@@ -22,14 +23,20 @@ static __always_inline const struct vdso_data *__arch_get_vdso_data(void)
return _vdso_data;
}
+static __always_inline const struct s390_vdso_data *__get_s390_vdso_data(void)
+{
+ return &_s390_data;
+}
+
static inline u64 __arch_get_hw_counter(s32 clock_mode, const struct vdso_data *vd)
{
+ const struct s390_vdso_data *svd = __get_s390_vdso_data();
u64 adj, now;
now = get_tod_clock();
- adj = vd->arch_data.tod_steering_end - now;
+ adj = svd->tod_steering_end - now;
if (unlikely((s64) adj > 0))
- now += (vd->arch_data.tod_steering_delta < 0) ? (adj >> 15) : -(adj >> 15);
+ now += (svd->tod_steering_delta < 0) ? (adj >> 15) : -(adj >> 15);
return now;
}
diff --git a/arch/s390/kernel/time.c b/arch/s390/kernel/time.c
index e37285a5101b..ec5c12e541aa 100644
--- a/arch/s390/kernel/time.c
+++ b/arch/s390/kernel/time.c
@@ -83,7 +83,7 @@ void __init time_early_init(void)
/* Initialize TOD steering parameters */
tod_steering_end = tod_clock_base.tod;
- vdso_data->arch_data.tod_steering_end = tod_steering_end;
+ s390_vdso_data->tod_steering_end = tod_steering_end;
if (!test_facility(28))
return;
@@ -381,8 +381,8 @@ static void clock_sync_global(unsigned long delta)
panic("TOD clock sync offset %li is too large to drift\n",
tod_steering_delta);
tod_steering_end = now + (abs(tod_steering_delta) << 15);
- vdso_data->arch_data.tod_steering_end = tod_steering_end;
- vdso_data->arch_data.tod_steering_delta = tod_steering_delta;
+ s390_vdso_data->tod_steering_end = tod_steering_end;
+ s390_vdso_data->tod_steering_delta = tod_steering_delta;
/* Update LPAR offset. */
if (ptff_query(PTFF_QTO) && ptff(&qto, sizeof(qto), PTFF_QTO) == 0)
diff --git a/arch/s390/kernel/vdso.c b/arch/s390/kernel/vdso.c
index 8c4e07d533c8..4f1dba52b240 100644
--- a/arch/s390/kernel/vdso.c
+++ b/arch/s390/kernel/vdso.c
@@ -29,9 +29,16 @@ static union {
u8 page[PAGE_SIZE];
} vdso_data_store __page_aligned_data;
+static union {
+ struct s390_vdso_data data;
+ u8 page[PAGE_SIZE];
+} s390_vdso_page __page_aligned_data;
+
struct vdso_data *vdso_data = vdso_data_store.data;
+struct s390_vdso_data *s390_vdso_data = &s390_vdso_page.data;
enum vvar_pages {
+ VVAR_S390_PAGE_OFFSET,
VVAR_DATA_PAGE_OFFSET,
VVAR_TIMENS_PAGE_OFFSET,
VVAR_NR_PAGES,
@@ -109,14 +116,26 @@ static vm_fault_t vvar_fault(const struct vm_special_mapping *sm,
vm_fault_t err;
switch (vmf->pgoff) {
+ case VVAR_S390_PAGE_OFFSET:
+ pfn = virt_to_pfn(s390_vdso_data);
+ break;
case VVAR_DATA_PAGE_OFFSET:
+ /*
+ * Fault in VVAR s390 page too, since it will be
+ * accessed to get tod clock steering data anyway.
+ */
+ addr = vma->vm_start + VVAR_S390_PAGE_OFFSET * PAGE_SIZE;
+ pfn = virt_to_pfn(s390_vdso_data);
+ err = vmf_insert_pfn(vma, addr, pfn);
+ if (unlikely(err & VM_FAULT_ERROR))
+ return err;
+ addr = vma->vm_start + VVAR_TIMENS_PAGE_OFFSET * PAGE_SIZE;
pfn = virt_to_pfn(vdso_data);
if (timens_page) {
/*
- * Fault in VVAR page too, since it will be accessed
- * to get clock data anyway.
+ * Fault in VVAR data page too, since it will be
+ * accessed to get clock data anyway.
*/
- addr = vmf->address + VVAR_TIMENS_PAGE_OFFSET * PAGE_SIZE;
err = vmf_insert_pfn(vma, addr, pfn);
if (unlikely(err & VM_FAULT_ERROR))
return err;
diff --git a/arch/s390/kernel/vdso64/vdso64.lds.S b/arch/s390/kernel/vdso64/vdso64.lds.S
index 518f1ea405f4..d38e5475df65 100644
--- a/arch/s390/kernel/vdso64/vdso64.lds.S
+++ b/arch/s390/kernel/vdso64/vdso64.lds.S
@@ -13,7 +13,8 @@ ENTRY(_start)
SECTIONS
{
- PROVIDE(_vdso_data = . - __VVAR_PAGES * PAGE_SIZE);
+ PROVIDE(_s390_data = . - __VVAR_PAGES * PAGE_SIZE);
+ PROVIDE(_vdso_data = _s390_data + PAGE_SIZE);
#ifdef CONFIG_TIME_NS
PROVIDE(_timens_data = _vdso_data + PAGE_SIZE);
#endif
--
2.25.1
^ permalink raw reply related [flat|nested] 25+ messages in thread
* [LTP] [PATCH 2/3] s390/vdso: fix arch_data access for __arch_get_hw_counter()
@ 2021-03-23 21:58 ` Heiko Carstens
0 siblings, 0 replies; 25+ messages in thread
From: Heiko Carstens @ 2021-03-23 21:58 UTC (permalink / raw)
To: ltp
Li Wang reported that clock_gettime(CLOCK_MONOTONIC_RAW, ...) returns
incorrect values when time is provided via vdso instead of system call:
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
Within the s390 specific vdso function __arch_get_hw_counter() tries
to read tod clock steering values from the arch_data member of the
passed in vdso_data structure.
However only the arch_data member of the first clock source base
(CS_HRES_COARSE) is initialized. For CS_RAW arch_data is not at all
initialized, which explains the incorrect returned values.
It is a bit odd to provide the required tod clock steering parameters
only within the first element of the _vdso_data array. However for
time namespaces even no member of the _timens_data array contains the
required data, which would make fixing __arch_get_hw_counter() quite
complicated.
Therefore simply add an s390 specific vdso data page which contains
the tod clock steering parameters. Everything else seems to be
unnecessary complex.
Reported-by: Li Wang <liwang@redhat.com>
Fixes: 1ba2d6c0fd4e ("s390/vdso: simplify __arch_get_hw_counter()")
Fixes: eeab78b05d20 ("s390/vdso: implement generic vdso time namespace support")
Link: https://lore.kernel.org/linux-s390/YFnxr1ZlMIOIqjfq@osiris
Signed-off-by: Heiko Carstens <hca@linux.ibm.com>
---
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 | 6 +++---
arch/s390/kernel/vdso.c | 25 ++++++++++++++++++++---
arch/s390/kernel/vdso64/vdso64.lds.S | 3 ++-
8 files changed, 56 insertions(+), 24 deletions(-)
delete mode 100644 arch/s390/include/asm/vdso/data.h
create mode 100644 arch/s390/include/asm/vdso/datapage.h
diff --git a/arch/s390/Kconfig b/arch/s390/Kconfig
index c1ff874e6c2e..532ce0fcc659 100644
--- a/arch/s390/Kconfig
+++ b/arch/s390/Kconfig
@@ -77,7 +77,6 @@ config S390
select ARCH_HAS_STRICT_MODULE_RWX
select ARCH_HAS_SYSCALL_WRAPPER
select ARCH_HAS_UBSAN_SANITIZE_ALL
- select ARCH_HAS_VDSO_DATA
select ARCH_HAVE_NMI_SAFE_CMPXCHG
select ARCH_INLINE_READ_LOCK
select ARCH_INLINE_READ_LOCK_BH
diff --git a/arch/s390/include/asm/vdso.h b/arch/s390/include/asm/vdso.h
index b45e3dddd2c2..0d047f519df6 100644
--- a/arch/s390/include/asm/vdso.h
+++ b/arch/s390/include/asm/vdso.h
@@ -3,17 +3,19 @@
#define __S390_VDSO_H__
#include <vdso/datapage.h>
+#include <asm/vdso/datapage.h>
/* Default link address for the vDSO */
#define VDSO64_LBASE 0
-#define __VVAR_PAGES 2
+#define __VVAR_PAGES 3
#define VDSO_VERSION_STRING LINUX_2.6.29
#ifndef __ASSEMBLY__
extern struct vdso_data *vdso_data;
+extern struct s390_vdso_data *s390_vdso_data;
int vdso_getcpu_init(void);
diff --git a/arch/s390/include/asm/vdso/data.h b/arch/s390/include/asm/vdso/data.h
deleted file mode 100644
index 7b3cdb4a5f48..000000000000
--- a/arch/s390/include/asm/vdso/data.h
+++ /dev/null
@@ -1,13 +0,0 @@
-/* SPDX-License-Identifier: GPL-2.0 */
-#ifndef __S390_ASM_VDSO_DATA_H
-#define __S390_ASM_VDSO_DATA_H
-
-#include <linux/types.h>
-#include <vdso/datapage.h>
-
-struct arch_vdso_data {
- __u64 tod_steering_delta;
- __u64 tod_steering_end;
-};
-
-#endif /* __S390_ASM_VDSO_DATA_H */
diff --git a/arch/s390/include/asm/vdso/datapage.h b/arch/s390/include/asm/vdso/datapage.h
new file mode 100644
index 000000000000..bfae78d814af
--- /dev/null
+++ b/arch/s390/include/asm/vdso/datapage.h
@@ -0,0 +1,17 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+#ifndef __S390_ASM_VDSO_DATAPAGE_H
+#define __S390_ASM_VDSO_DATAPAGE_H
+
+#include <linux/types.h>
+
+#ifndef __ASSEMBLY__
+
+struct s390_vdso_data {
+ __u64 tod_steering_delta;
+ __u64 tod_steering_end;
+};
+
+extern struct s390_vdso_data _s390_data __attribute__((visibility("hidden")));
+
+#endif /* __ASSEMBLY__ */
+#endif /* __S390_ASM_VDSO_DATAPAGE_H */
diff --git a/arch/s390/include/asm/vdso/gettimeofday.h b/arch/s390/include/asm/vdso/gettimeofday.h
index ed89ef742530..bbd6da6b1651 100644
--- a/arch/s390/include/asm/vdso/gettimeofday.h
+++ b/arch/s390/include/asm/vdso/gettimeofday.h
@@ -9,6 +9,7 @@
#include <asm/timex.h>
#include <asm/unistd.h>
#include <asm/vdso.h>
+#include <asm/vdso/datapage.h>
#include <linux/compiler.h>
#define vdso_calc_delta __arch_vdso_calc_delta
@@ -22,14 +23,20 @@ static __always_inline const struct vdso_data *__arch_get_vdso_data(void)
return _vdso_data;
}
+static __always_inline const struct s390_vdso_data *__get_s390_vdso_data(void)
+{
+ return &_s390_data;
+}
+
static inline u64 __arch_get_hw_counter(s32 clock_mode, const struct vdso_data *vd)
{
+ const struct s390_vdso_data *svd = __get_s390_vdso_data();
u64 adj, now;
now = get_tod_clock();
- adj = vd->arch_data.tod_steering_end - now;
+ adj = svd->tod_steering_end - now;
if (unlikely((s64) adj > 0))
- now += (vd->arch_data.tod_steering_delta < 0) ? (adj >> 15) : -(adj >> 15);
+ now += (svd->tod_steering_delta < 0) ? (adj >> 15) : -(adj >> 15);
return now;
}
diff --git a/arch/s390/kernel/time.c b/arch/s390/kernel/time.c
index e37285a5101b..ec5c12e541aa 100644
--- a/arch/s390/kernel/time.c
+++ b/arch/s390/kernel/time.c
@@ -83,7 +83,7 @@ void __init time_early_init(void)
/* Initialize TOD steering parameters */
tod_steering_end = tod_clock_base.tod;
- vdso_data->arch_data.tod_steering_end = tod_steering_end;
+ s390_vdso_data->tod_steering_end = tod_steering_end;
if (!test_facility(28))
return;
@@ -381,8 +381,8 @@ static void clock_sync_global(unsigned long delta)
panic("TOD clock sync offset %li is too large to drift\n",
tod_steering_delta);
tod_steering_end = now + (abs(tod_steering_delta) << 15);
- vdso_data->arch_data.tod_steering_end = tod_steering_end;
- vdso_data->arch_data.tod_steering_delta = tod_steering_delta;
+ s390_vdso_data->tod_steering_end = tod_steering_end;
+ s390_vdso_data->tod_steering_delta = tod_steering_delta;
/* Update LPAR offset. */
if (ptff_query(PTFF_QTO) && ptff(&qto, sizeof(qto), PTFF_QTO) == 0)
diff --git a/arch/s390/kernel/vdso.c b/arch/s390/kernel/vdso.c
index 8c4e07d533c8..4f1dba52b240 100644
--- a/arch/s390/kernel/vdso.c
+++ b/arch/s390/kernel/vdso.c
@@ -29,9 +29,16 @@ static union {
u8 page[PAGE_SIZE];
} vdso_data_store __page_aligned_data;
+static union {
+ struct s390_vdso_data data;
+ u8 page[PAGE_SIZE];
+} s390_vdso_page __page_aligned_data;
+
struct vdso_data *vdso_data = vdso_data_store.data;
+struct s390_vdso_data *s390_vdso_data = &s390_vdso_page.data;
enum vvar_pages {
+ VVAR_S390_PAGE_OFFSET,
VVAR_DATA_PAGE_OFFSET,
VVAR_TIMENS_PAGE_OFFSET,
VVAR_NR_PAGES,
@@ -109,14 +116,26 @@ static vm_fault_t vvar_fault(const struct vm_special_mapping *sm,
vm_fault_t err;
switch (vmf->pgoff) {
+ case VVAR_S390_PAGE_OFFSET:
+ pfn = virt_to_pfn(s390_vdso_data);
+ break;
case VVAR_DATA_PAGE_OFFSET:
+ /*
+ * Fault in VVAR s390 page too, since it will be
+ * accessed to get tod clock steering data anyway.
+ */
+ addr = vma->vm_start + VVAR_S390_PAGE_OFFSET * PAGE_SIZE;
+ pfn = virt_to_pfn(s390_vdso_data);
+ err = vmf_insert_pfn(vma, addr, pfn);
+ if (unlikely(err & VM_FAULT_ERROR))
+ return err;
+ addr = vma->vm_start + VVAR_TIMENS_PAGE_OFFSET * PAGE_SIZE;
pfn = virt_to_pfn(vdso_data);
if (timens_page) {
/*
- * Fault in VVAR page too, since it will be accessed
- * to get clock data anyway.
+ * Fault in VVAR data page too, since it will be
+ * accessed to get clock data anyway.
*/
- addr = vmf->address + VVAR_TIMENS_PAGE_OFFSET * PAGE_SIZE;
err = vmf_insert_pfn(vma, addr, pfn);
if (unlikely(err & VM_FAULT_ERROR))
return err;
diff --git a/arch/s390/kernel/vdso64/vdso64.lds.S b/arch/s390/kernel/vdso64/vdso64.lds.S
index 518f1ea405f4..d38e5475df65 100644
--- a/arch/s390/kernel/vdso64/vdso64.lds.S
+++ b/arch/s390/kernel/vdso64/vdso64.lds.S
@@ -13,7 +13,8 @@ ENTRY(_start)
SECTIONS
{
- PROVIDE(_vdso_data = . - __VVAR_PAGES * PAGE_SIZE);
+ PROVIDE(_s390_data = . - __VVAR_PAGES * PAGE_SIZE);
+ PROVIDE(_vdso_data = _s390_data + PAGE_SIZE);
#ifdef CONFIG_TIME_NS
PROVIDE(_timens_data = _vdso_data + PAGE_SIZE);
#endif
--
2.25.1
^ permalink raw reply related [flat|nested] 25+ messages in thread
* Re: [PATCH 2/3] s390/vdso: fix arch_data access for __arch_get_hw_counter()
2021-03-23 21:58 ` [LTP] " Heiko Carstens
@ 2021-03-24 5:53 ` Heiko Carstens
-1 siblings, 0 replies; 25+ messages in thread
From: Heiko Carstens @ 2021-03-24 5:53 UTC (permalink / raw)
To: Heiko Carstens
Cc: Li Wang, Alexander Gordeev, Vasily Gorbik, Sven Schnelle,
Christian Borntraeger, Viresh Kumar, Thomas Gleixner, ltp,
linux-kernel, linux-s390
On Tue, Mar 23, 2021 at 10:58:18PM +0100, Heiko Carstens wrote:
> Li Wang reported that clock_gettime(CLOCK_MONOTONIC_RAW, ...) returns
> incorrect values when time is provided via vdso instead of system call:
>
> 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
>
> Within the s390 specific vdso function __arch_get_hw_counter() tries
> to read tod clock steering values from the arch_data member of the
> passed in vdso_data structure.
> However only the arch_data member of the first clock source base
> (CS_HRES_COARSE) is initialized. For CS_RAW arch_data is not at all
> initialized, which explains the incorrect returned values.
>
> It is a bit odd to provide the required tod clock steering parameters
> only within the first element of the _vdso_data array. However for
> time namespaces even no member of the _timens_data array contains the
> required data, which would make fixing __arch_get_hw_counter() quite
> complicated.
>
> Therefore simply add an s390 specific vdso data page which contains
> the tod clock steering parameters. Everything else seems to be
> unnecessary complex.
>
> Reported-by: Li Wang <liwang@redhat.com>
> Fixes: 1ba2d6c0fd4e ("s390/vdso: simplify __arch_get_hw_counter()")
> Fixes: eeab78b05d20 ("s390/vdso: implement generic vdso time namespace support")
> Link: https://lore.kernel.org/linux-s390/YFnxr1ZlMIOIqjfq@osiris
> Signed-off-by: Heiko Carstens <hca@linux.ibm.com>
> ---
> 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 | 6 +++---
> arch/s390/kernel/vdso.c | 25 ++++++++++++++++++++---
> arch/s390/kernel/vdso64/vdso64.lds.S | 3 ++-
> 8 files changed, 56 insertions(+), 24 deletions(-)
> delete mode 100644 arch/s390/include/asm/vdso/data.h
> create mode 100644 arch/s390/include/asm/vdso/datapage.h
FWIW, alternatively to this and the third patch we could also do the
much shorter and simpler variant below. What I personally don't like
is that data is duplicated.
But on the other hand it is much shorter, and the more I think of it
this seems to be the way to go.
Opinions?
diff --git a/arch/s390/kernel/time.c b/arch/s390/kernel/time.c
index e37285a5101b..fa095ecf0349 100644
--- a/arch/s390/kernel/time.c
+++ b/arch/s390/kernel/time.c
@@ -80,10 +80,12 @@ void __init time_early_init(void)
{
struct ptff_qto qto;
struct ptff_qui qui;
+ int i;
/* Initialize TOD steering parameters */
tod_steering_end = tod_clock_base.tod;
- vdso_data->arch_data.tod_steering_end = tod_steering_end;
+ for (i = 0; i < CS_BASES; i++)
+ vdso_data[i].arch_data.tod_steering_end = tod_steering_end;
if (!test_facility(28))
return;
@@ -366,6 +368,7 @@ static void clock_sync_global(unsigned long delta)
{
unsigned long now, adj;
struct ptff_qto qto;
+ int i;
/* Fixup the monotonic sched clock. */
tod_clock_base.eitod += delta;
@@ -381,8 +384,10 @@ static void clock_sync_global(unsigned long delta)
panic("TOD clock sync offset %li is too large to drift\n",
tod_steering_delta);
tod_steering_end = now + (abs(tod_steering_delta) << 15);
- vdso_data->arch_data.tod_steering_end = tod_steering_end;
- vdso_data->arch_data.tod_steering_delta = tod_steering_delta;
+ for (i = 0; i < CS_BASES; i++) {
+ vdso_data[i].arch_data.tod_steering_end = tod_steering_end;
+ vdso_data[i].arch_data.tod_steering_delta = tod_steering_delta;
+ }
/* Update LPAR offset. */
if (ptff_query(PTFF_QTO) && ptff(&qto, sizeof(qto), PTFF_QTO) == 0)
^ permalink raw reply related [flat|nested] 25+ messages in thread
* [LTP] [PATCH 2/3] s390/vdso: fix arch_data access for __arch_get_hw_counter()
@ 2021-03-24 5:53 ` Heiko Carstens
0 siblings, 0 replies; 25+ messages in thread
From: Heiko Carstens @ 2021-03-24 5:53 UTC (permalink / raw)
To: ltp
On Tue, Mar 23, 2021 at 10:58:18PM +0100, Heiko Carstens wrote:
> Li Wang reported that clock_gettime(CLOCK_MONOTONIC_RAW, ...) returns
> incorrect values when time is provided via vdso instead of system call:
>
> 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
>
> Within the s390 specific vdso function __arch_get_hw_counter() tries
> to read tod clock steering values from the arch_data member of the
> passed in vdso_data structure.
> However only the arch_data member of the first clock source base
> (CS_HRES_COARSE) is initialized. For CS_RAW arch_data is not at all
> initialized, which explains the incorrect returned values.
>
> It is a bit odd to provide the required tod clock steering parameters
> only within the first element of the _vdso_data array. However for
> time namespaces even no member of the _timens_data array contains the
> required data, which would make fixing __arch_get_hw_counter() quite
> complicated.
>
> Therefore simply add an s390 specific vdso data page which contains
> the tod clock steering parameters. Everything else seems to be
> unnecessary complex.
>
> Reported-by: Li Wang <liwang@redhat.com>
> Fixes: 1ba2d6c0fd4e ("s390/vdso: simplify __arch_get_hw_counter()")
> Fixes: eeab78b05d20 ("s390/vdso: implement generic vdso time namespace support")
> Link: https://lore.kernel.org/linux-s390/YFnxr1ZlMIOIqjfq@osiris
> Signed-off-by: Heiko Carstens <hca@linux.ibm.com>
> ---
> 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 | 6 +++---
> arch/s390/kernel/vdso.c | 25 ++++++++++++++++++++---
> arch/s390/kernel/vdso64/vdso64.lds.S | 3 ++-
> 8 files changed, 56 insertions(+), 24 deletions(-)
> delete mode 100644 arch/s390/include/asm/vdso/data.h
> create mode 100644 arch/s390/include/asm/vdso/datapage.h
FWIW, alternatively to this and the third patch we could also do the
much shorter and simpler variant below. What I personally don't like
is that data is duplicated.
But on the other hand it is much shorter, and the more I think of it
this seems to be the way to go.
Opinions?
diff --git a/arch/s390/kernel/time.c b/arch/s390/kernel/time.c
index e37285a5101b..fa095ecf0349 100644
--- a/arch/s390/kernel/time.c
+++ b/arch/s390/kernel/time.c
@@ -80,10 +80,12 @@ void __init time_early_init(void)
{
struct ptff_qto qto;
struct ptff_qui qui;
+ int i;
/* Initialize TOD steering parameters */
tod_steering_end = tod_clock_base.tod;
- vdso_data->arch_data.tod_steering_end = tod_steering_end;
+ for (i = 0; i < CS_BASES; i++)
+ vdso_data[i].arch_data.tod_steering_end = tod_steering_end;
if (!test_facility(28))
return;
@@ -366,6 +368,7 @@ static void clock_sync_global(unsigned long delta)
{
unsigned long now, adj;
struct ptff_qto qto;
+ int i;
/* Fixup the monotonic sched clock. */
tod_clock_base.eitod += delta;
@@ -381,8 +384,10 @@ static void clock_sync_global(unsigned long delta)
panic("TOD clock sync offset %li is too large to drift\n",
tod_steering_delta);
tod_steering_end = now + (abs(tod_steering_delta) << 15);
- vdso_data->arch_data.tod_steering_end = tod_steering_end;
- vdso_data->arch_data.tod_steering_delta = tod_steering_delta;
+ for (i = 0; i < CS_BASES; i++) {
+ vdso_data[i].arch_data.tod_steering_end = tod_steering_end;
+ vdso_data[i].arch_data.tod_steering_delta = tod_steering_delta;
+ }
/* Update LPAR offset. */
if (ptff_query(PTFF_QTO) && ptff(&qto, sizeof(qto), PTFF_QTO) == 0)
^ permalink raw reply related [flat|nested] 25+ messages in thread
* [PATCH 3/3] lib/vdso: remove struct arch_vdso_data from vdso data struct
2021-03-23 21:58 ` [LTP] " Heiko Carstens
@ 2021-03-23 21:58 ` Heiko Carstens
-1 siblings, 0 replies; 25+ messages in thread
From: Heiko Carstens @ 2021-03-23 21:58 UTC (permalink / raw)
To: Li Wang, Alexander Gordeev, Vasily Gorbik, Sven Schnelle,
Christian Borntraeger, Viresh Kumar, Thomas Gleixner
Cc: ltp, linux-kernel, linux-s390
Since commit d60d7de3e16d ("lib/vdso: Allow to add architecture-specific
vdso data") it is possible to provide arch specific VDSO data.
This was only added for s390, which doesn't make use this anymore.
Therefore remove it again.
Signed-off-by: Heiko Carstens <hca@linux.ibm.com>
---
arch/Kconfig | 3 ---
include/vdso/datapage.h | 10 ----------
2 files changed, 13 deletions(-)
diff --git a/arch/Kconfig b/arch/Kconfig
index ecfd3520b676..35c7114f7ea3 100644
--- a/arch/Kconfig
+++ b/arch/Kconfig
@@ -1147,9 +1147,6 @@ config HAVE_SPARSE_SYSCALL_NR
entries at 4000, 5000 and 6000 locations. This option turns on syscall
related optimizations for a given architecture.
-config ARCH_HAS_VDSO_DATA
- bool
-
config HAVE_STATIC_CALL
bool
diff --git a/include/vdso/datapage.h b/include/vdso/datapage.h
index 73eb622e7663..ee810cae4e1e 100644
--- a/include/vdso/datapage.h
+++ b/include/vdso/datapage.h
@@ -19,12 +19,6 @@
#include <vdso/time32.h>
#include <vdso/time64.h>
-#ifdef CONFIG_ARCH_HAS_VDSO_DATA
-#include <asm/vdso/data.h>
-#else
-struct arch_vdso_data {};
-#endif
-
#define VDSO_BASES (CLOCK_TAI + 1)
#define VDSO_HRES (BIT(CLOCK_REALTIME) | \
BIT(CLOCK_MONOTONIC) | \
@@ -70,8 +64,6 @@ struct vdso_timestamp {
* @tz_dsttime: type of DST correction
* @hrtimer_res: hrtimer resolution
* @__unused: unused
- * @arch_data: architecture specific data (optional, defaults
- * to an empty struct)
*
* vdso_data will be accessed by 64 bit and compat code at the same time
* so we should be careful before modifying this structure.
@@ -105,8 +97,6 @@ struct vdso_data {
s32 tz_dsttime;
u32 hrtimer_res;
u32 __unused;
-
- struct arch_vdso_data arch_data;
};
/*
--
2.25.1
^ permalink raw reply related [flat|nested] 25+ messages in thread
* [LTP] [PATCH 3/3] lib/vdso: remove struct arch_vdso_data from vdso data struct
@ 2021-03-23 21:58 ` Heiko Carstens
0 siblings, 0 replies; 25+ messages in thread
From: Heiko Carstens @ 2021-03-23 21:58 UTC (permalink / raw)
To: ltp
Since commit d60d7de3e16d ("lib/vdso: Allow to add architecture-specific
vdso data") it is possible to provide arch specific VDSO data.
This was only added for s390, which doesn't make use this anymore.
Therefore remove it again.
Signed-off-by: Heiko Carstens <hca@linux.ibm.com>
---
arch/Kconfig | 3 ---
include/vdso/datapage.h | 10 ----------
2 files changed, 13 deletions(-)
diff --git a/arch/Kconfig b/arch/Kconfig
index ecfd3520b676..35c7114f7ea3 100644
--- a/arch/Kconfig
+++ b/arch/Kconfig
@@ -1147,9 +1147,6 @@ config HAVE_SPARSE_SYSCALL_NR
entries at 4000, 5000 and 6000 locations. This option turns on syscall
related optimizations for a given architecture.
-config ARCH_HAS_VDSO_DATA
- bool
-
config HAVE_STATIC_CALL
bool
diff --git a/include/vdso/datapage.h b/include/vdso/datapage.h
index 73eb622e7663..ee810cae4e1e 100644
--- a/include/vdso/datapage.h
+++ b/include/vdso/datapage.h
@@ -19,12 +19,6 @@
#include <vdso/time32.h>
#include <vdso/time64.h>
-#ifdef CONFIG_ARCH_HAS_VDSO_DATA
-#include <asm/vdso/data.h>
-#else
-struct arch_vdso_data {};
-#endif
-
#define VDSO_BASES (CLOCK_TAI + 1)
#define VDSO_HRES (BIT(CLOCK_REALTIME) | \
BIT(CLOCK_MONOTONIC) | \
@@ -70,8 +64,6 @@ struct vdso_timestamp {
* @tz_dsttime: type of DST correction
* @hrtimer_res: hrtimer resolution
* @__unused: unused
- * @arch_data: architecture specific data (optional, defaults
- * to an empty struct)
*
* vdso_data will be accessed by 64 bit and compat code at the same time
* so we should be careful before modifying this structure.
@@ -105,8 +97,6 @@ struct vdso_data {
s32 tz_dsttime;
u32 hrtimer_res;
u32 __unused;
-
- struct arch_vdso_data arch_data;
};
/*
--
2.25.1
^ permalink raw reply related [flat|nested] 25+ messages in thread
* Re: [PATCH 3/3] lib/vdso: remove struct arch_vdso_data from vdso data struct
2021-03-23 21:58 ` [LTP] " Heiko Carstens
@ 2021-03-25 17:55 ` Thomas Gleixner
-1 siblings, 0 replies; 25+ messages in thread
From: Thomas Gleixner @ 2021-03-25 17:55 UTC (permalink / raw)
To: Heiko Carstens, Li Wang, Alexander Gordeev, Vasily Gorbik,
Sven Schnelle, Christian Borntraeger, Viresh Kumar
Cc: ltp, linux-kernel, linux-s390
On Tue, Mar 23 2021 at 22:58, Heiko Carstens wrote:
> Since commit d60d7de3e16d ("lib/vdso: Allow to add architecture-specific
> vdso data") it is possible to provide arch specific VDSO data.
>
> This was only added for s390, which doesn't make use this anymore.
> Therefore remove it again.
>
> Signed-off-by: Heiko Carstens <hca@linux.ibm.com>
Please route that with the rest of the fixes.
Reviewed-by: Thomas Gleixner <tglx@linutronix.de>
^ permalink raw reply [flat|nested] 25+ messages in thread
* [LTP] [PATCH 3/3] lib/vdso: remove struct arch_vdso_data from vdso data struct
@ 2021-03-25 17:55 ` Thomas Gleixner
0 siblings, 0 replies; 25+ messages in thread
From: Thomas Gleixner @ 2021-03-25 17:55 UTC (permalink / raw)
To: ltp
On Tue, Mar 23 2021 at 22:58, Heiko Carstens wrote:
> Since commit d60d7de3e16d ("lib/vdso: Allow to add architecture-specific
> vdso data") it is possible to provide arch specific VDSO data.
>
> This was only added for s390, which doesn't make use this anymore.
> Therefore remove it again.
>
> Signed-off-by: Heiko Carstens <hca@linux.ibm.com>
Please route that with the rest of the fixes.
Reviewed-by: Thomas Gleixner <tglx@linutronix.de>
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH 3/3] lib/vdso: remove struct arch_vdso_data from vdso data struct
2021-03-25 17:55 ` [LTP] " Thomas Gleixner
@ 2021-03-25 17:57 ` Thomas Gleixner
-1 siblings, 0 replies; 25+ messages in thread
From: Thomas Gleixner @ 2021-03-25 17:57 UTC (permalink / raw)
To: Heiko Carstens, Li Wang, Alexander Gordeev, Vasily Gorbik,
Sven Schnelle, Christian Borntraeger, Viresh Kumar
Cc: ltp, linux-kernel, linux-s390
On Thu, Mar 25 2021 at 18:55, Thomas Gleixner wrote:
> On Tue, Mar 23 2021 at 22:58, Heiko Carstens wrote:
>> Since commit d60d7de3e16d ("lib/vdso: Allow to add architecture-specific
>> vdso data") it is possible to provide arch specific VDSO data.
>>
>> This was only added for s390, which doesn't make use this anymore.
>> Therefore remove it again.
>>
>> Signed-off-by: Heiko Carstens <hca@linux.ibm.com>
>
> Please route that with the rest of the fixes.
>
> Reviewed-by: Thomas Gleixner <tglx@linutronix.de>
Ah, you decided for the simpler variant. Fine with me.
^ permalink raw reply [flat|nested] 25+ messages in thread
* [LTP] [PATCH 3/3] lib/vdso: remove struct arch_vdso_data from vdso data struct
@ 2021-03-25 17:57 ` Thomas Gleixner
0 siblings, 0 replies; 25+ messages in thread
From: Thomas Gleixner @ 2021-03-25 17:57 UTC (permalink / raw)
To: ltp
On Thu, Mar 25 2021 at 18:55, Thomas Gleixner wrote:
> On Tue, Mar 23 2021 at 22:58, Heiko Carstens wrote:
>> Since commit d60d7de3e16d ("lib/vdso: Allow to add architecture-specific
>> vdso data") it is possible to provide arch specific VDSO data.
>>
>> This was only added for s390, which doesn't make use this anymore.
>> Therefore remove it again.
>>
>> Signed-off-by: Heiko Carstens <hca@linux.ibm.com>
>
> Please route that with the rest of the fixes.
>
> Reviewed-by: Thomas Gleixner <tglx@linutronix.de>
Ah, you decided for the simpler variant. Fine with me.
^ permalink raw reply [flat|nested] 25+ messages in thread
* [LTP] [PATCH 0/3] s390 vdso fixes
2021-03-23 21:58 ` [LTP] " Heiko Carstens
` (3 preceding siblings ...)
(?)
@ 2021-03-25 8:56 ` Li Wang
2021-03-25 12:33 ` [LTP] " Heiko Carstens
-1 siblings, 1 reply; 25+ messages in thread
From: Li Wang @ 2021-03-25 8:56 UTC (permalink / raw)
To: ltp
Hi Heiko,
On Wed, Mar 24, 2021 at 5:58 AM Heiko Carstens <hca@linux.ibm.com> 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 <liwang@redhat.com>
--
Regards,
Li Wang
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.linux.it/pipermail/ltp/attachments/20210325/ef168269/attachment-0001.htm>
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH 0/3] s390 vdso fixes
2021-03-25 8:56 ` [LTP] [PATCH 0/3] s390 vdso fixes Li Wang
@ 2021-03-25 12:33 ` Heiko Carstens
0 siblings, 0 replies; 25+ messages in thread
From: Heiko Carstens @ 2021-03-25 12:33 UTC (permalink / raw)
To: Li Wang
Cc: Alexander Gordeev, Vasily Gorbik, Sven Schnelle,
Christian Borntraeger, Viresh Kumar, Thomas Gleixner, LTP List,
linux-kernel, linux-s390
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 <hca@linux.ibm.com> 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 <liwang@redhat.com>
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?
^ permalink raw reply [flat|nested] 25+ messages in thread
* [LTP] [PATCH 0/3] s390 vdso fixes
@ 2021-03-25 12:33 ` Heiko Carstens
0 siblings, 0 replies; 25+ messages in thread
From: Heiko Carstens @ 2021-03-25 12:33 UTC (permalink / raw)
To: ltp
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 <hca@linux.ibm.com> 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 <liwang@redhat.com>
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?
^ permalink raw reply [flat|nested] 25+ messages in thread
* [LTP] [PATCH 0/3] s390 vdso fixes
2021-03-25 12:33 ` [LTP] " Heiko Carstens
(?)
@ 2021-03-25 14:11 ` Li Wang
-1 siblings, 0 replies; 25+ messages in thread
From: Li Wang @ 2021-03-25 14:11 UTC (permalink / raw)
To: ltp
Heiko Carstens <hca@linux.ibm.com> wrote:
> ...
> > > 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 <liwang@redhat.com>
>
> 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?
>
Sure, I just reverted the 2/3 and 3/3, then apply the simple variant
scratch-patch. It also works well.
--
Regards,
Li Wang
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.linux.it/pipermail/ltp/attachments/20210325/bf84e964/attachment.htm>
^ permalink raw reply [flat|nested] 25+ messages in thread