linux-kernel - Re: time / gtod seconds value out of sync?

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]

Date:	Thu, 19 Feb 2015 11:03:26 -0800
From:	John Stultz <john.stultz@...aro.org>
To:	Nishanth Aravamudan <nacc@...ux.vnet.ibm.com>
Cc:	Thomas Gleixner <tglx@...utronix.de>,
	lkml <linux-kernel@...r.kernel.org>, jstancek@...hat.com,
	Ingo Molnar <mingo@...nel.org>
Subject: Re: time / gtod seconds value out of sync?

Hey Nish! Long time!

On Thu, Feb 19, 2015 at 10:35 AM, Nishanth Aravamudan
<nacc@...ux.vnet.ibm.com> wrote:
> Hi John,
>
> We're seeing an interesting issue with the openposix testcase
> difftime/1-1, which basically calls gtod/time, sleeps, calls time/gtod,

I'm not familiar with the test... Do you have a link?

> then difftime and sees if they disagree. The issue occurs with either
> vDSO implementations or direct syscalls.
>
> We are seeing failures on ppc64le and x86_64 (probably other places too,
> just not tested yet), because (I'm pretty sure), the time() syscalls
> granularity is not accounting for the nsecs value at all. Instead, it
> just returns get_seconds().

Right, so there is always a problem mixing calls of different
granularity (similar issues crop up with gettimeofday() and filesystem
timestamps), so the basis of the test worries me a little bit from the
description, but I'd have to look at it to really get a sense.

> In one case, I see, in sys_time():
>
> [  313.001823] NACC: timekeeping_get_ns = 1000121642
> [  314.001889] NACC: timekeeping_get_ns = 188401
>
> gtod correctly accumulates those nsecs into the secs value:
>
>                 ts->tv_sec = tk->xtime_sec;
>                 nsecs = timekeeping_get_ns(&tk->tkr);
>                 ts->tv_nsec = 0;
>                 timespec64_add_ns(ts, nsecs);
>
> but time() does:
>
>                 return tk->xtime_sec;
>
> It seems like overkill to do the full timekeeping_get_ns() in time(),

Right, so looking into the git history,
f20bf6125605acbbc7eb8c9420d7221c91aa83eb (time: introduce
xtime_seconds) changed this specifically for performance reasons
(cc'ed Ingo here, in case he remembers more context).

The idea that time() would be ok as being HZ granular, and its been
this way since 2.6.23.  Thus you have a < HZ sized window where
gettimeofday() will return the next second before time() gets updated
by the tick.

> but maybe it's also necessary to account for leap seconds? That is, we
> need to ensure that accumulate_nsecs_to_secs() has been called before
> return tk->xtime_sec?

So leapseconds are also applied at tick time, so I don't think you'd
see any different behavior with them.

There was a thread on this quite awhile back and I if I recall I think
the general consensus was to keep time() tick granular  (so it aligns
with filesystem timestamps) and gettimeofday() hardware granular.
Though we also introduced the CLOCK_REALTIME_COARSE to match
sub-second filesystem timestamps as well.

So yea... I don't think we want to make a change here, but maybe I'm
not understanding the underlying issue... so feel free to push back
here. :)

thanks
-john
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/