From: Glauber Costa <glommer@redhat.com>
To: linux-kernel@vger.kernel.org
Cc: avi@redhat.com, Thomas Gleixner <tglx@linutronix.de>,
Ingo Molnar <mingo@elte.hu>, "H. Peter Anvin" <hpa@zytor.com>,
Peter Zijlstra <peterz@infradead.org>
Subject: [PATCH] use a stable clock reference in vdso vgetns
Date: Fri, 1 Oct 2010 09:09:56 -0400 [thread overview]
Message-ID: <1285938596-2186-1-git-send-email-glommer@redhat.com> (raw)
When using vdso services for clock_gettime, we test for the ability
of a fine-grained measurement through the existance of a vread() function.
However, from the time we test it, to the time we use it, vread() reference
may not be valid anymore. It happens, for example, when we change the current
clocksource from one that provides vread (say tsc) to one that lacks it
(say acpi_pm), in the middle of clock_gettime routine.
seqlock does not really protect us, since readers here won't stop the writers
to change references. The proposed solution is to grab a copy of the clock
structure early, and use it as a stable reference onwards.
Since we don't free parts of memory associated with old clocksources, we
merely stop using it for a while, this is a safe thing to do.
Signed-off-by: Glauber Costa <glommer@redhat.com>
CC: Avi Kivity <avi@redhat.com>
CC: Thomas Gleixner <tglx@linutronix.de>
CC: Ingo Molnar <mingo@elte.hu>
CC: H. Peter Anvin <hpa@zytor.com>
CC: Peter Zijlstra <peterz@infradead.org>
---
arch/x86/include/asm/vgtod.h | 2 +-
arch/x86/vdso/vclock_gettime.c | 51 +++++++++++++++++++++++++++------------
2 files changed, 36 insertions(+), 17 deletions(-)
diff --git a/arch/x86/include/asm/vgtod.h b/arch/x86/include/asm/vgtod.h
index 3d61e20..7f813bc 100644
--- a/arch/x86/include/asm/vgtod.h
+++ b/arch/x86/include/asm/vgtod.h
@@ -13,7 +13,7 @@ struct vsyscall_gtod_data {
int sysctl_enabled;
struct timezone sys_tz;
- struct { /* extract of a clocksource struct */
+ struct vclock { /* extract of a clocksource struct */
cycle_t (*vread)(void);
cycle_t cycle_last;
cycle_t mask;
diff --git a/arch/x86/vdso/vclock_gettime.c b/arch/x86/vdso/vclock_gettime.c
index ee55754..c82ade0 100644
--- a/arch/x86/vdso/vclock_gettime.c
+++ b/arch/x86/vdso/vclock_gettime.c
@@ -34,23 +34,26 @@ notrace static long vdso_fallback_gettime(long clock, struct timespec *ts)
return ret;
}
-notrace static inline long vgetns(void)
+/*
+ * The clock structure must be provided from the outside, instead of accessed
+ * from gtod. The clock can have changed, leading to an invalid vread by the
+ * time we get here. All other values must also be consistent with vread result
+ */
+notrace static inline long vgetns(struct vclock *clock)
{
long v;
- cycles_t (*vread)(void);
- vread = gtod->clock.vread;
- v = (vread() - gtod->clock.cycle_last) & gtod->clock.mask;
- return (v * gtod->clock.mult) >> gtod->clock.shift;
+ v = (clock->vread() - clock->cycle_last) & clock->mask;
+ return (v * clock->mult) >> clock->shift;
}
-notrace static noinline int do_realtime(struct timespec *ts)
+notrace static noinline int do_realtime(struct timespec *ts, struct vclock *clock)
{
unsigned long seq, ns;
do {
seq = read_seqbegin(>od->lock);
ts->tv_sec = gtod->wall_time_sec;
ts->tv_nsec = gtod->wall_time_nsec;
- ns = vgetns();
+ ns = vgetns(clock);
} while (unlikely(read_seqretry(>od->lock, seq)));
timespec_add_ns(ts, ns);
return 0;
@@ -72,13 +75,13 @@ vset_normalized_timespec(struct timespec *ts, long sec, long nsec)
ts->tv_nsec = nsec;
}
-notrace static noinline int do_monotonic(struct timespec *ts)
+notrace static noinline int do_monotonic(struct timespec *ts, struct vclock *clock)
{
unsigned long seq, ns, secs;
do {
seq = read_seqbegin(>od->lock);
secs = gtod->wall_time_sec;
- ns = gtod->wall_time_nsec + vgetns();
+ ns = gtod->wall_time_nsec + vgetns(clock);
secs += gtod->wall_to_monotonic.tv_sec;
ns += gtod->wall_to_monotonic.tv_nsec;
} while (unlikely(read_seqretry(>od->lock, seq)));
@@ -113,21 +116,29 @@ notrace static noinline int do_monotonic_coarse(struct timespec *ts)
notrace int __vdso_clock_gettime(clockid_t clock, struct timespec *ts)
{
- if (likely(gtod->sysctl_enabled))
+ if (likely(gtod->sysctl_enabled)) {
+ struct vclock clk;
+ unsigned long seq;
+ do {
+ seq = read_seqbegin(>od->lock);
+ memcpy(&clk, >od->clock, sizeof(clk));
+ } while (unlikely(read_seqretry(>od->lock, seq)));
+
switch (clock) {
case CLOCK_REALTIME:
- if (likely(gtod->clock.vread))
- return do_realtime(ts);
+ if (likely(clk.vread))
+ return do_realtime(ts, &clk);
break;
case CLOCK_MONOTONIC:
- if (likely(gtod->clock.vread))
- return do_monotonic(ts);
+ if (likely(clk.vread))
+ return do_monotonic(ts, &clk);
break;
case CLOCK_REALTIME_COARSE:
return do_realtime_coarse(ts);
case CLOCK_MONOTONIC_COARSE:
return do_monotonic_coarse(ts);
}
+ }
return vdso_fallback_gettime(clock, ts);
}
int clock_gettime(clockid_t, struct timespec *)
@@ -136,12 +147,20 @@ int clock_gettime(clockid_t, struct timespec *)
notrace int __vdso_gettimeofday(struct timeval *tv, struct timezone *tz)
{
long ret;
- if (likely(gtod->sysctl_enabled && gtod->clock.vread)) {
+ struct vclock clock;
+ unsigned long seq;
+
+ do {
+ seq = read_seqbegin(>od->lock);
+ memcpy(&clock, >od->clock, sizeof(clock));
+ } while (unlikely(read_seqretry(>od->lock, seq)));
+
+ if (likely(gtod->sysctl_enabled && clock.vread)) {
if (likely(tv != NULL)) {
BUILD_BUG_ON(offsetof(struct timeval, tv_usec) !=
offsetof(struct timespec, tv_nsec) ||
sizeof(*tv) != sizeof(struct timespec));
- do_realtime((struct timespec *)tv);
+ do_realtime((struct timespec *)tv, &clock);
tv->tv_usec /= 1000;
}
if (unlikely(tz != NULL)) {
--
1.7.0.1
next reply other threads:[~2010-10-01 17:38 UTC|newest]
Thread overview: 9+ messages / expand[flat|nested] mbox.gz Atom feed top
2010-10-01 13:09 Glauber Costa [this message]
2010-10-01 17:49 ` [PATCH] use a stable clock reference in vdso vgetns john stultz
2010-10-04 12:40 ` Glauber Costa
2010-10-04 16:15 ` Glauber Costa
2010-10-13 14:07 ` Glauber Costa
2010-10-14 1:36 ` john stultz
2010-10-14 2:44 ` Glauber Costa
2010-10-14 18:16 ` john stultz
2010-10-15 15:29 ` Glauber Costa
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=1285938596-2186-1-git-send-email-glommer@redhat.com \
--to=glommer@redhat.com \
--cc=avi@redhat.com \
--cc=hpa@zytor.com \
--cc=linux-kernel@vger.kernel.org \
--cc=mingo@elte.hu \
--cc=peterz@infradead.org \
--cc=tglx@linutronix.de \
/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