All of lore.kernel.org
 help / color / mirror / Atom feed
From: Peter Zijlstra <peterz@infradead.org>
To: "Jason A. Donenfeld" <Jason@zx2c4.com>
Cc: Thomas Gleixner <tglx@linutronix.de>,
	LKML <linux-kernel@vger.kernel.org>,
	clemens@ladisch.de, Sultan Alsawaf <sultan@kerneltoast.com>,
	Waiman Long <longman@redhat.com>, X86 ML <x86@kernel.org>
Subject: Re: infinite loop in read_hpet from ktime_get_boot_fast_ns
Date: Wed, 12 Jun 2019 14:28:43 +0200	[thread overview]
Message-ID: <20190612122843.GJ3436@hirez.programming.kicks-ass.net> (raw)
In-Reply-To: <CAHmME9obwzZ5x=p3twDfNYux+kg0h4QAGe0ePAkZ2KqvguBK3g@mail.gmail.com>

On Wed, Jun 12, 2019 at 11:44:35AM +0200, Jason A. Donenfeld wrote:
> Hey Peter,
> 
> On Wed, Jun 12, 2019 at 11:03 AM Peter Zijlstra <peterz@infradead.org> wrote:
> > How quasi? Do the comments in kernel/sched/clock.c look like something
> > you could use?
> >
> > As already mentioned in the other tasks, anything ktime will be
> > horrifically crap when it ends up using the HPET, the code in
> > kernel/sched/clock.c is a best effort to keep using TSC even when it is
> > deemed unusable for timekeeping.
> 
> Thanks for pointing that out. Indeed the HPET path is a bummer and I'd
> like to just escape using ktime all together.
> 
> In fact, my accuracy requirements are very lax. I could probably even
> deal with an inaccuracy as huge as ~200 milliseconds. But what I do
> need is 64-bit, so that it doesn't wrap, allowing me to compare two
> stamps taken a long time apart, and for it to take into account sleep
> time, like CLOCK_BOOTTIME does, which means get_jiffies_64() doesn't
> fit the bill. I was under the impression that I could only get this
> with ktime_get_boot & co, because those add the sleep offset.
> 
> It looks like, though, kernel/sched/clock.c keeps track of some
> offsets too -- __sched_clock_offset and __gtod_offset,

Right, those are used to keep the clock values coherent (as best as
possible) when we switch modes.

When the TSC is stable sched_clock_cpu() is mapped directly to
sched_clock() for performance reasons. The moment the TSC is detected to
be unsuitable, we switch to the unstable mode, where we take a GTOD
timestamp every tick and add resolution with the CPU local TSC (plus
filters etc..).

To make this mode-switch as smooth as possible, we track those offsets.

> and the comment at the top mentions explicit sleep hooks. I wasn't
> sure which function to use from here, though. 

Either local_clock() or cpu_clock(cpu). The sleep hooks are not
something the consumer has to worry about.

> sched_clock() seems based on jiffies, which
> has the 32-bit wraparound issue, and the base implementation doesn't
> seem to take into account sleeptime. The x86 implementation seems use
> rdtsc and then adds cyc2ns_offset which looks to be based on
> cyc2ns_suspend, which I assume is what I want. 

Yes.

> But there's still the
> issue of the 32-bit wraparound on the base implementation.

If an architecture doesn't provide a sched_clock(), you're on a
seriously handicapped arch. It wraps in ~500 days, and aside from
changing jiffies_lock to a latch, I don't think we can do much about it.

(the scheduler too expects sched_clock() to not wrap short of the u64
and so having those machines online for 500 days will get you 'funny'
results)

AFAICT only: alpha, h8300, hexagon, m68knommu, nds32, nios2, openrisc
are lacking any form of sched_clock(), the rest has it either natively
or through sched_clock_register().

> I guess you know this code better than my quick perusal. Is there some
> clock in here that doesn't have a wrap around issue and takes into
> account sleeptime, without being super slow like ktime/hpet?

You probably want to use local_clock().


  reply	other threads:[~2019-06-12 12:29 UTC|newest]

Thread overview: 26+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-06-07 14:14 infinite loop in read_hpet from ktime_get_boot_fast_ns Jason A. Donenfeld
2019-06-11 21:09 ` Thomas Gleixner
2019-06-11 21:40   ` Waiman Long
2019-06-12  9:02   ` Peter Zijlstra
2019-06-12  9:44     ` Jason A. Donenfeld
2019-06-12 12:28       ` Peter Zijlstra [this message]
2019-06-12 12:31         ` Peter Zijlstra
2019-06-12 12:58         ` Jason A. Donenfeld
2019-06-12 15:27           ` Peter Zijlstra
2019-06-12 19:46         ` Arnd Bergmann
2019-06-18 17:34         ` Jason A. Donenfeld
2019-06-12 14:01       ` Arnd Bergmann
2019-06-13 15:18         ` Jason A. Donenfeld
2019-06-13 15:40           ` Arnd Bergmann
2019-06-13 16:17             ` Jason A. Donenfeld
2019-06-13 16:26               ` Thomas Gleixner
2019-06-13 16:34                 ` Jason A. Donenfeld
2019-06-13 16:41                   ` Jason A. Donenfeld
2019-06-13 19:53                   ` Thomas Gleixner
2019-06-14  9:14                     ` Jason A. Donenfeld
2019-06-14  9:44                       ` Thomas Gleixner
2019-06-14  9:56                         ` Jason A. Donenfeld
2019-06-14  9:48                       ` [PATCH] timekeeping: add get_jiffies_boot_64() for jiffies including sleep Jason A. Donenfeld
2019-06-14  9:55                     ` [tip:timers/urgent] timekeeping: Repair ktime_get_coarse*() granularity tip-bot for Thomas Gleixner
2019-06-14 11:18                       ` Arnd Bergmann
2019-06-12  9:29   ` infinite loop in read_hpet from ktime_get_boot_fast_ns Peter Zijlstra

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=20190612122843.GJ3436@hirez.programming.kicks-ass.net \
    --to=peterz@infradead.org \
    --cc=Jason@zx2c4.com \
    --cc=clemens@ladisch.de \
    --cc=linux-kernel@vger.kernel.org \
    --cc=longman@redhat.com \
    --cc=sultan@kerneltoast.com \
    --cc=tglx@linutronix.de \
    --cc=x86@kernel.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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.