Re: [RFC][PATCH 7/9] x86/tsc: Provide sched_clock_noinstr()

[Peter Zijlstra]

On Wed, May 17, 2023 at 02:26:35AM +0000, Michael Kelley (LINUX) wrote:

> Peter -- I've sent you an RFC patch to incorporate into your broader
> patch set.  I think it probably makes sense for all the Hyper-V
> stuff to be a separate patch.

Perhaps, it's not that much.

> I haven't previously worked with the details of notrace vs. noinstr,
> but I followed the patterns elsewhere in patch set. Please review
> to see if it seems correct.

notrace inhibits the "call __fentry__" at the start of the symbol.

The __fentry__ call is mostly for ftrace, there's a few sites where
inhibiting tracing is critical -- stuff that happens before the ftrace
recursion handling, but mostly it's about performance these days,
constantly hitting the recusion code isn't very good.

noinstr inhibits any and all compiler generated 'extra' -- it is for the
C as a portable assembler usage. This very much includes notrace, but it
also covers all the *SAN nonsense. Basically, if it does not directly
reflect the code as written, it shouldn't be emitted.

Additionally, and for validation purposes, it also ensures all these
symbols end up in a special text section.

But yeah, you seem to have gotten it right.

> One thing:  In the cases where I added __always_inline, I dropped
> any notrace or noinstr annotations.  I presume such code always
> takes on the attributes of the caller.  If that's not correct, let me know.

Correct; noinstr actually has an explicit noinline because compilers
suck :/