From: Heiko Carstens <hca@linux.ibm.com>
To: Heiko Carstens <hca@linux.ibm.com>
Cc: Li Wang <liwang@redhat.com>,
Alexander Gordeev <agordeev@linux.ibm.com>,
Vasily Gorbik <gor@linux.ibm.com>,
Sven Schnelle <svens@linux.ibm.com>,
Christian Borntraeger <borntraeger@de.ibm.com>,
Viresh Kumar <viresh.kumar@linaro.org>,
Thomas Gleixner <tglx@linutronix.de>,
ltp@lists.linux.it, linux-kernel@vger.kernel.org,
linux-s390@vger.kernel.org
Subject: Re: [PATCH 2/3] s390/vdso: fix arch_data access for __arch_get_hw_counter()
Date: Wed, 24 Mar 2021 06:53:35 +0100 [thread overview]
Message-ID: <YFrT3x7sa49EPYFx@osiris> (raw)
In-Reply-To: <20210323215819.4161164-3-hca@linux.ibm.com>
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)
next prev parent reply other threads:[~2021-03-24 5:54 UTC|newest]
Thread overview: 11+ messages / expand[flat|nested] mbox.gz Atom feed top
[not found] <CAEemH2cELFSMzEYM-Gd1LxNuFzVE2PcG1chzyaVhW2YCJjjzdw@mail.gmail.com>
2021-03-23 7:11 ` [s390x vDSO Bug?] clock_gettime(CLOCK_MONOTONIC_RAW, ...) gets abnormal ts value Heiko Carstens
2021-03-23 13:48 ` Heiko Carstens
2021-03-23 21:58 ` [PATCH 0/3] s390 vdso fixes Heiko Carstens
2021-03-23 21:58 ` [PATCH 1/3] s390/vdso: fix tod clock steering Heiko Carstens
2021-03-24 9:50 ` Heiko Carstens
2021-03-23 21:58 ` [PATCH 2/3] s390/vdso: fix arch_data access for __arch_get_hw_counter() Heiko Carstens
2021-03-24 5:53 ` Heiko Carstens [this message]
2021-03-23 21:58 ` [PATCH 3/3] lib/vdso: remove struct arch_vdso_data from vdso data struct Heiko Carstens
2021-03-25 17:55 ` Thomas Gleixner
2021-03-25 17:57 ` Thomas Gleixner
[not found] ` <CAEemH2cSk=doHL51uD5Qu6uCRRTCgYd0EN0iij=X+538J53XsQ@mail.gmail.com>
2021-03-25 12:33 ` [PATCH 0/3] s390 vdso fixes Heiko Carstens
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=YFrT3x7sa49EPYFx@osiris \
--to=hca@linux.ibm.com \
--cc=agordeev@linux.ibm.com \
--cc=borntraeger@de.ibm.com \
--cc=gor@linux.ibm.com \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-s390@vger.kernel.org \
--cc=liwang@redhat.com \
--cc=ltp@lists.linux.it \
--cc=svens@linux.ibm.com \
--cc=tglx@linutronix.de \
--cc=viresh.kumar@linaro.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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).