2023-05-25 21:03:49

by Nadav Amit

[permalink] [raw]
Subject: [PATCH v2 2/3] compiler: inline does not imply notrace

From: Nadav Amit <[email protected]>

Functions that are marked as "inline" are currently also not tracable.
This limits tracing functionality for many functions for no reason.
Apparently, this has been done for two reasons.

First, as described in commit 5963e317b1e9d2a ("ftrace/x86: Do not
change stacks in DEBUG when calling lockdep"), it was intended to
prevent some functions that cannot be traced from being traced as these
functions were marked as inline (among others).

Yet, this change has been done a decade ago, and according to Steven
Rostedt, ftrace should have improved and hopefully resolved nested
tracing issues by now. Arguably, if functions that should be traced -
for instance since they are used during tracing - still exist, they
should be marked as notrace explicitly.

The second reason, which Steven raised, is that attaching "notrace" to
"inline" prevented tracing differences between different configs, which
caused various problem. This consideration is not very strong, and tying
"inline" and "notrace" does not seem very beneficial. The "inline"
keyword is just a hint, and many functions are currently not tracable
due to this reason.

Disconnect "inline" from "notrace".

Signed-off-by: Nadav Amit <[email protected]>
---
include/linux/compiler_types.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/include/linux/compiler_types.h b/include/linux/compiler_types.h
index 547ea1ff806e..bab3e25bbe3f 100644
--- a/include/linux/compiler_types.h
+++ b/include/linux/compiler_types.h
@@ -184,7 +184,7 @@ struct ftrace_likely_data {
* of extern inline functions at link time.
* A lot of inline functions can cause havoc with function tracing.
*/
-#define inline inline __gnu_inline __inline_maybe_unused notrace
+#define inline inline __gnu_inline __inline_maybe_unused

/*
* gcc provides both __inline__ and __inline as alternate spellings of
--
2.25.1



2023-05-26 02:31:03

by Steven Rostedt

[permalink] [raw]
Subject: Re: [PATCH v2 2/3] compiler: inline does not imply notrace

On Thu, 25 May 2023 14:00:39 -0700
Nadav Amit <[email protected]> wrote:

> From: Nadav Amit <[email protected]>
>
> Functions that are marked as "inline" are currently also not tracable.
> This limits tracing functionality for many functions for no reason.
> Apparently, this has been done for two reasons.
>
> First, as described in commit 5963e317b1e9d2a ("ftrace/x86: Do not
> change stacks in DEBUG when calling lockdep"), it was intended to
> prevent some functions that cannot be traced from being traced as these
> functions were marked as inline (among others).
>
> Yet, this change has been done a decade ago, and according to Steven
> Rostedt, ftrace should have improved and hopefully resolved nested
> tracing issues by now. Arguably, if functions that should be traced -
> for instance since they are used during tracing - still exist, they
> should be marked as notrace explicitly.
>
> The second reason, which Steven raised, is that attaching "notrace" to
> "inline" prevented tracing differences between different configs, which
> caused various problem. This consideration is not very strong, and tying
> "inline" and "notrace" does not seem very beneficial. The "inline"
> keyword is just a hint, and many functions are currently not tracable
> due to this reason.
>
> Disconnect "inline" from "notrace".

FYI, I have a patch queued (still needs to go through testing) that
already does this ;-)

https://lore.kernel.org/all/[email protected]/

-- Steve

2023-05-26 05:36:11

by Nadav Amit

[permalink] [raw]
Subject: Re: [PATCH v2 2/3] compiler: inline does not imply notrace



> On May 25, 2023, at 7:28 PM, Steven Rostedt <[email protected]> wrote:
>
> On Thu, 25 May 2023 14:00:39 -0700
> Nadav Amit <[email protected]> wrote:
>
>> From: Nadav Amit <[email protected]>
>>
>> Functions that are marked as "inline" are currently also not tracable.
>> This limits tracing functionality for many functions for no reason.
>> Apparently, this has been done for two reasons.
>>
>> First, as described in commit 5963e317b1e9d2a ("ftrace/x86: Do not
>> change stacks in DEBUG when calling lockdep"), it was intended to
>> prevent some functions that cannot be traced from being traced as these
>> functions were marked as inline (among others).
>>
>> Yet, this change has been done a decade ago, and according to Steven
>> Rostedt, ftrace should have improved and hopefully resolved nested
>> tracing issues by now. Arguably, if functions that should be traced -
>> for instance since they are used during tracing - still exist, they
>> should be marked as notrace explicitly.
>>
>> The second reason, which Steven raised, is that attaching "notrace" to
>> "inline" prevented tracing differences between different configs, which
>> caused various problem. This consideration is not very strong, and tying
>> "inline" and "notrace" does not seem very beneficial. The "inline"
>> keyword is just a hint, and many functions are currently not tracable
>> due to this reason.
>>
>> Disconnect "inline" from "notrace".
>
> FYI, I have a patch queued (still needs to go through testing) that
> already does this ;-)
>
> https://lore.kernel.org/all/[email protected]/

Ugh. If you cc’d me, I wouldn’t bother you during your vacation. :)

I think you may like the first patch in my series to precede this patch
though as some of the function I marked as “notrace" are currently “inline”.

Let me know how you want to proceed, so I would know how to break this
series.


2023-05-26 05:40:43

by Steven Rostedt

[permalink] [raw]
Subject: Re: [PATCH v2 2/3] compiler: inline does not imply notrace

On Thu, 25 May 2023 22:17:33 -0700
Nadav Amit <[email protected]> wrote:


> > FYI, I have a patch queued (still needs to go through testing) that
> > already does this ;-)
> >
> > https://lore.kernel.org/all/[email protected]/
>
> Ugh. If you cc’d me, I wouldn’t bother you during your vacation. :)

I'm currently passed the vacation part and now in Taiwan for work.

>
> I think you may like the first patch in my series to precede this patch
> though as some of the function I marked as “notrace" are currently “inline”.
>
> Let me know how you want to proceed, so I would know how to break this
> series.

Currently there's a nasty bug in v6.4-rc3 I'm fighting where I can't
proceed on anything until it's resolved. But I could also just pull
your first and third patch too. I'll let you know when I'm finished
debugging.

-- Steve

2023-05-26 05:49:01

by Steven Rostedt

[permalink] [raw]
Subject: Re: [PATCH v2 2/3] compiler: inline does not imply notrace

On Thu, 25 May 2023 22:17:33 -0700
Nadav Amit <[email protected]> wrote:

> Ugh. If you cc’d me, I wouldn’t bother you during your vacation. :)

Oh, and if you are interested in tracing patches, just subscribe to
[email protected].

-- Steve