linux-kernel - Re: [PATCH v4.16-rc4 1/1] x86/vdso: on Intel, VDSO should handle CLOCK_MONOTONIC

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] [day] [month] [year] [list]

Message-ID: <alpine.DEB.2.21.1803111513110.1625@nanos.tec.linutronix.de>
Date:   Sun, 11 Mar 2018 15:51:21 -0700 (PDT)
From:   Thomas Gleixner <tglx@...utronix.de>
To:     Jason Vas Dias <jason.vas.dias@...il.com>
cc:     x86@...nel.org, LKML <linux-kernel@...r.kernel.org>,
        andi <andi@...stfloor.org>, Peter Zijlstra <peterz@...radead.org>
Subject: Re: [PATCH v4.16-rc4 1/1] x86/vdso: on Intel, VDSO should handle
 CLOCK_MONOTONIC_RAW

On Sun, 11 Mar 2018, Jason Vas Dias wrote:
> On 11/03/2018, Thomas Gleixner <tglx@...utronix.de> wrote:
> > On Sun, 11 Mar 2018, Jason Vas Dias wrote:
> >
> > This looks better now. Though running that patch through checkpatch.pl
> > results in:
> >
> > total: 28 errors, 20 warnings, 139 lines checked
> >
> 
> Hmm, I was unaware of that script, I'll run and find out why -

Documentation/process/* which I recommended for reading before definitely
mentions it.

> probably because whitespace is not visible in emacs with
> my monospace font and it is very difficult to see if tabs
> are used if somehow a '\t\ ' or ' \t' has slipped in .

It's not only about whitespace ....

> >> +notrace static u64 vread_tsc_raw(void)
> >
> > Why do you need a separate function? I asked you to use vread_tsc(). So you
> > might have reasons for doing that, but please then explain WHY and not just
> > throw the stuff in my direction w/o any comment.
> >
> 
> mainly, because vread_tsc() makes its comparison against gtod->cycles_last ,
> a copy of tk->tkr_mono.cycle_last, while vread_tsc_raw() uses
> gtod->raw_cycle_last, a copy of tk->tkr_raw.cycle_last .

Well, if you look at the timekeeping core then you'll notice that it's
actually the same value. It's updated in both tkr_mono and tkr_raw so its
available when either of the tkr_* structs is handed to the relevant
functions.

> A complete patch , against 4.15.9, is attached , that I am using ,
> including a suggested '__vdso_linux_tsc_calibration()'
> function and arch/x86/include/uapi/asm/vdso_tsc_calibration.h file
> that does not return any pointers into the VDSO .

I'm not going to look at attached complete patches. The development process
is well defined for a reason and it's not optional.

Thanks,

	tglx