linux-kernel - Re: infinite loop in read_hpet from ktime_get_boot_fast

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <alpine.DEB.2.21.1906141131570.1722@nanos.tec.linutronix.de>
Date:   Fri, 14 Jun 2019 11:44:03 +0200 (CEST)
From:   Thomas Gleixner <tglx@...utronix.de>
To:     "Jason A. Donenfeld" <Jason@...c4.com>
cc:     Arnd Bergmann <arnd@...db.de>,
        Peter Zijlstra <peterz@...radead.org>,
        LKML <linux-kernel@...r.kernel.org>,
        Clemens Ladisch <clemens@...isch.de>,
        Sultan Alsawaf <sultan@...neltoast.com>,
        Waiman Long <longman@...hat.com>, X86 ML <x86@...nel.org>
Subject: Re: infinite loop in read_hpet from ktime_get_boot_fast_ns

Jason,

On Fri, 14 Jun 2019, Jason A. Donenfeld wrote:

> Hey Thomas,
> 
> > --- a/kernel/time/timekeeping.c
> > +++ b/kernel/time/timekeeping.c
> >         } while (read_seqcount_retry(&tk_core.seq, seq));
> >
> > -       return base;
> > -
> > +       return base + nsecs;
> 
> The rest of the file seems to use `ktime_add_ns(base, nsecs)`. I
> realize, of course, that these days that macro is the same thing as
> what you wrote, though.

Yeah, historical raisins when ktime_t was special on 32bit.

> One thing I'm curious about is the performance comparison with various
> ways of using jiffies directly:
> 
> ktime_mono_to_any(ns_to_ktime(jiffies64_to_nsecs(get_jiffies_64())),
> TK_OFFS_BOOT)
> 
> Or really giving up on the locking:
> 
> ktime_to_ns(tk_core.timekeeper.offs_boot) + jiffies64_to_nsecs(get_jiffies_64())
> 
> Or keeping things in units of jiffies, though that incurs a div_u64:
> 
> nsecs_to_jiffies64(ktime_to_ns(tk_core.timekeeper.offs_boot)) + get_jiffies_64()

jiffies64 uses a seqcount on 32bit as well.
 
> But since offs_boot is updated somewhat rarely, that div_u64 could be
> precomputed each time offs_boot is updated, allowing hypothetically:
> 
> tk_core.timekeeper.offs_boot_jiffies + get_jiffies_64()

Hrm, I'm not a great fan of these shortcuts which cut corners based on
'somewhat rarely, so it should not matter'. Should not matter always
strikes back at some point. :)

> Which then could be remade into a wrapper such as:
> 
> get_jiffies_boot_64()
> 
> The speed is indeed an important factor to me in accessing this time
> value. Are any of these remotely interesting to you in that light?
> Maybe I'll send a patch for the latter.

So what you are looking for is jiffies based on boot time?

Thanks,

	tglx