search for: cycle_last

...x86: tsc prevent time going backwards") > > I still have one of the machines which is affected by this. Are we sure this isn't a load vs rdtsc reorder? Because if I look at the current code: notrace static u64 vread_tsc(void) { u64 ret = (u64)rdtsc_ordered(); u64 last = gtod->cycle_last; if (likely(ret >= last)) return ret; /* * GCC likes to generate cmov here, but this branch is extremely * predictable (it's just a function of time and the likely is * very likely) and there's a data dependence, so force GCC * to generate a branch instead. I don't bar...

...x86: tsc prevent time going backwards") > > I still have one of the machines which is affected by this. Are we sure this isn't a load vs rdtsc reorder? Because if I look at the current code: notrace static u64 vread_tsc(void) { u64 ret = (u64)rdtsc_ordered(); u64 last = gtod->cycle_last; if (likely(ret >= last)) return ret; /* * GCC likes to generate cmov here, but this branch is extremely * predictable (it's just a function of time and the likely is * very likely) and there's a data dependence, so force GCC * to generate a branch instead. I don't bar...

Dereferencing gtod->cycle_last all over the place and foing the cycles < last comparison in the vclock read functions generates horrible code. Doing it at the call site is much better and gains a few cycles both for TSC and pvclock. Caveat: This adds the comparison to the hyperv vclock as well, but I have no way to test that...

On Tue, 18 Sep 2018, Thomas Gleixner wrote: > On Tue, 18 Sep 2018, Peter Zijlstra wrote: > > > Your memory serves you right. That's indeed observable on CPUs which > > > lack TSC_ADJUST. > > > > But, if the gtod code can observe this, then why doesn't the code that > > checks the sync? > > Because it depends where the involved CPUs are in the

On Tue, 18 Sep 2018, Thomas Gleixner wrote: > On Tue, 18 Sep 2018, Peter Zijlstra wrote: > > > Your memory serves you right. That's indeed observable on CPUs which > > > lack TSC_ADJUST. > > > > But, if the gtod code can observe this, then why doesn't the code that > > checks the sync? > > Because it depends where the involved CPUs are in the

...r wrote: > > I still have one of the machines which is affected by this. > > Are we sure this isn't a load vs rdtsc reorder? Because if I look at the > current code: The load order of last vs. rdtsc does not matter at all. CPU0 CPU1 .... now0 = rdtsc_ordered(); ... tk->cycle_last = now0; gtod->seq++; gtod->cycle_last = tk->cycle_last; ... gtod->seq++; seq_begin(gtod->seq); now1 = rdtsc_ordered(); So if the TSC on CPU1 is slightly behind the TSC on CPU0 then now1 can be smaller than cycle_last. The TSC sync stuff does not catch the small delta for...

Matt attempted to add CLOCK_TAI support to the VDSO clock_gettime() implementation, which extended the clockid switch case and added yet another slightly different copy of the same code. Especially the extended switch case is problematic as the compiler tends to generate a jump table which then requires to use retpolines. If jump tables are disabled it adds yet another conditional to the existing

Matt attempted to add CLOCK_TAI support to the VDSO clock_gettime() implementation, which extended the clockid switch case and added yet another slightly different copy of the same code. Especially the extended switch case is problematic as the compiler tends to generate a jump table which then requires to use retpolines. If jump tables are disabled it adds yet another conditional to the existing

Matt attempted to add CLOCK_TAI support to the VDSO clock_gettime() implementation, which extended the clockid switch case and added yet another slightly different copy of the same code. Especially the extended switch case is problematic as the compiler tends to generate a jump table which then requires to use retpolines. If jump tables are disabled it adds yet another conditional to the existing

...; I'll have a look into that. It needs some thought vs. the fractional part > > of the base time, but it should be not rocket science to get that > > correct. Famous last words... > > Does the sentinel need to be U64_MAX? What if vgetcyc and its minions > returned gtod->cycle_last-1 (for some value of 1), and the caller just > does "if ((s64)cycles - (s64)last < 0) return fallback; ns += > (cycles-last)* ...". That should just be a "sub ; js ; ". It's an extra > load of ->cycle_last, but only on the path where we're heading for the...

On Tue, 18 Sep 2018, Thomas Gleixner wrote: > So if the TSC on CPU1 is slightly behind the TSC on CPU0 then now1 can be > smaller than cycle_last. The TSC sync stuff does not catch the small delta > for unknown raisins. I'll go and find that machine and test that again. Of course it does not trigger anymore. We accumulated code between the point in timekeeping_advance() where the TSC is read and the update of the VDSO data. I'll...

..._accumulation(struct timekeeper *tk, cycle_t offset, - u32 shift) + u32 shift, unsigned int *action) { cycle_t interval = tk->cycle_interval << shift; u64 raw_nsecs; @@ -1309,7 +1309,7 @@ static cycle_t logarithmic_accumulation(struct timekeeper *tk, cycle_t offset, tk->cycle_last += interval; tk->xtime_nsec += tk->xtime_interval << shift; - accumulate_nsecs_to_secs(tk); + *action |= accumulate_nsecs_to_secs(tk); /* Accumulate raw time */ raw_nsecs = (u64)tk->raw_interval << shift; @@ -1367,7 +1367,7 @@ static void update_wall_time(void) stru...

...ekeeping_inject_sleeptime(tk, delta); - timekeeping_update(tk, true, true); + timekeeping_update(tk, TK_CLEAR_NTP | TK_MIRROR); write_seqcount_end(&timekeeper_seq); raw_spin_unlock_irqrestore(&timekeeper_lock, flags); @@ -954,7 +957,7 @@ static void timekeeping_resume(void) tk->cycle_last = clock->cycle_last = cycle_now; tk->ntp_error = 0; timekeeping_suspended = 0; - timekeeping_update(tk, false, true); + timekeeping_update(tk, TK_MIRROR); write_seqcount_end(&timekeeper_seq); raw_spin_unlock_irqrestore(&timekeeper_lock, flags); @@ -1415,7 +1418,7 @@ static v...

...garithmic_accumulation(struct timekeeper *tk, cycle_t offset, - u32 shift) + u32 shift, int *action) { cycle_t interval = tk->cycle_interval << shift; u64 raw_nsecs; @@ -1311,7 +1311,7 @@ static cycle_t logarithmic_accumulation(struct timekeeper *tk, cycle_t offset, tk->cycle_last += interval; tk->xtime_nsec += tk->xtime_interval << shift; - accumulate_nsecs_to_secs(tk); + *action |= accumulate_nsecs_to_secs(tk); /* Accumulate raw time */ raw_nsecs = (u64)tk->raw_interval << shift; @@ -1369,7 +1369,7 @@ static void update_wall_time(void) stru...

...; + else if (mode == VCLOCK_PVCLOCK) + return vread_pvclock(); #endif #ifdef CONFIG_HYPERV_TSCPAGE - else if (gtod->vclock_mode == VCLOCK_HVCLOCK) - cycles = vread_hvclock(mode); + else if (mode == VCLOCK_HVCLOCK) + return vread_hvclock(); #endif - else - return 0; - v = cycles - gtod->cycle_last; - return v * gtod->mult; + return U64_MAX; } notrace static int do_hres(clockid_t clk, struct timespec *ts) { struct vgtod_ts *base = &gtod->basetime[clk]; unsigned int seq; - int mode; - u64 ns; + u64 cycles, ns; do { seq = gtod_read_begin(gtod); - mode = gtod->vcloc...

> On Sep 18, 2018, at 12:52 AM, Thomas Gleixner <tglx at linutronix.de> wrote: > >> On Mon, 17 Sep 2018, John Stultz wrote: >>> On Mon, Sep 17, 2018 at 12:25 PM, Andy Lutomirski <luto at kernel.org> wrote: >>> Also, I'm not entirely convinced that this "last" thing is needed at >>> all. John, what's the scenario under which we

> On Sep 18, 2018, at 12:52 AM, Thomas Gleixner <tglx at linutronix.de> wrote: > >> On Mon, 17 Sep 2018, John Stultz wrote: >>> On Mon, Sep 17, 2018 at 12:25 PM, Andy Lutomirski <luto at kernel.org> wrote: >>> Also, I'm not entirely convinced that this "last" thing is needed at >>> all. John, what's the scenario under which we

...cycles = vread_pvclock(mode); > #endif > +#ifdef CONFIG_HYPERV_CLOCK > + else if (gtod->vclock_mode == VCLOCK_HVCLOCK) > + cycles = vread_hvclock(mode); > +#endif > else > return 0; > v = (cycles - gtod->cycle_last) & gtod->mask; > diff --git a/arch/x86/entry/vdso/vma.c b/arch/x86/entry/vdso/vma.c > index 10820f6..4b9d90c 100644 > --- a/arch/x86/entry/vdso/vma.c > +++ b/arch/x86/entry/vdso/vma.c > @@ -21,6 +21,7 @@ > #include <asm/page.h> > #include <asm/desc.h> >...

...cycles = vread_pvclock(mode); > #endif > +#ifdef CONFIG_HYPERV_CLOCK > + else if (gtod->vclock_mode == VCLOCK_HVCLOCK) > + cycles = vread_hvclock(mode); > +#endif > else > return 0; > v = (cycles - gtod->cycle_last) & gtod->mask; > diff --git a/arch/x86/entry/vdso/vma.c b/arch/x86/entry/vdso/vma.c > index 10820f6..4b9d90c 100644 > --- a/arch/x86/entry/vdso/vma.c > +++ b/arch/x86/entry/vdso/vma.c > @@ -21,6 +21,7 @@ > #include <asm/page.h> > #include <asm/desc.h> >...

Hi, Hyper-V TSC page clocksource is suitable for vDSO, however, the protocol defined by the hypervisor is different from VCLOCK_PVCLOCK. I implemented the required support re-using pvclock_page VVAR. Simple sysbench test shows the following results: Before: # time sysbench --test=memory --max-requests=500000 run ... real 1m22.618s user 0m50.193s sys 0m32.268s After: # time sysbench