linux-kernel - Re: [PATCH] clocksource: tsc: respect tsc bootparam for clocksource_tsc

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 for Android: free password hash cracker in your pocket

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

Date:   Fri, 18 Oct 2019 20:41:14 +0200 (CEST)
From:   Thomas Gleixner <tglx@...utronix.de>
To:     Michael Zhivich <mzhivich@...mai.com>
cc:     linux-kernel@...r.kernel.org, mingo@...hat.com, bp@...en8.de,
        hpa@...or.com, rafael.j.wysocki@...el.com
Subject: Re: [PATCH] clocksource: tsc: respect tsc bootparam for
 clocksource_tsc_early

On Wed, 2 Oct 2019, Michael Zhivich wrote:
> Introduction of clocksource_tsc_early broke functionality of "tsc=reliable"
> and "tsc=nowatchdog" boot params, since clocksource_tsc_early is *always*
> registered with CLOCK_SOURCE_MUST_VERIFY and thus put on the watchdog list.
> 
> If CPU is very busy during boot, the watchdog clocksource may not be
> read frequently enough, resulting in a large difference that is treated as
> "negative" by clocksource_delta() and incorrectly truncated to 0.

What? 

>   clocksource: timekeeping watchdog on CPU1: Marking clocksource
>                'tsc-early' as unstable because the skew is too large:
>   clocksource: 'refined-jiffies' wd_now: fffb7019 wd_last: fffb6e28

    0xfffb7019 - 0xfffb6e28 = 497

What's treated negative there? And what would truncate that to 0?

>                 mask: ffffffff

A 'negative delta' value can only happen when the clocksource is not read
before it advanced more than mask/2. For jiffies this means 2^31
ticks. That would be ~248 days for HZ=100 or ~24 days for HZ=1000.

>   clocksource: 'tsc-early' cs_now: 52c3918b89d6 cs_last: 52c31d736d2e

  0x52c3918b89d6 - 0x52c31d736d2e = 1.94774e+09

Again nothing is treated negative here, but the point is that the TSC
advanced by ~2e9 cycles while jiffies advanced by 497.

How that's related, I can't tell because I don't know the TSC frequency of
your machine. HZ is probably 1000 as the watchdog period is HZ/2.

>                 mask: ffffffffffffffff
>   tsc: Marking TSC unstable due to clocksource watchdog

Even if the watchdog is not read out for a quite some time, the math in
there and in clocksource_delta() can handle pretty large deltas.

The watchdog triggers when

    abs(delta_watchdog - delta_tsc) > 0.0625 seconds

So that has absolutely nothing to do with CPU being busy and watchdog not
being serviced. jiffies and TSC drift apart for some reason, and that
reason wants to be documented in the changelog.

That said, I have no objections against the patch itself, but I'm not going
to apply a patch with a fairy tale changelog.

Thanks,

	tglx