tracing: make ftrace_likely_update() declaration visible

Commit Message

Arnd Bergmann May 17, 2023, 12:47 p.m. UTC

From: Arnd Bergmann <[e-mail]>

This function is only used when CONFIG_TRACE_BRANCH_PROFILING is
set, and the declaration is hidden behind this Kconfig symbol,
which causes a warning if disabled:

kernel/trace/trace_branch.c:205:6: error: no previous prototype for 'ftrace_likely_update' [-Werror=missing-prototypes]

Move the declaration out of the #ifdef to avoid the warning.

Signed-off-by: Arnd Bergmann <[e-mail]>
---
 include/linux/compiler.h | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

Comments

Linus Torvalds May 17, 2023, 4:39 p.m. UTC | #1

On Wed, May 17, 2023 at 5:47 AM Arnd Bergmann <[e-mail]> wrote:
>
> From: Arnd Bergmann <[e-mail]>
>
> This function is only used when CONFIG_TRACE_BRANCH_PROFILING is
> set, and the declaration is hidden behind this Kconfig symbol,
> which causes a warning if disabled:

That explanation is not very clear.

The problem is that the tracing code itself is built with
DISABLE_BRANCH_PROFILING, in order to not recurse. And that hides the
declaration when the definition is compiled, leading to the problem.

CONFIG_TRACE_BRANCH_PROFILING isn't the problem in itself - *that*
part of the test is consistent (not used, not declared, and not
compiled if it is off).

The problem is that DISABLE_BRANCH_PROFILING case, where it is used
elsewhere, but not declared when it is itself compiled.

I applied the patch, but I tried to reword the explanation to be more
clear. I may have failed.

          Linus

Steven Rostedt May 17, 2023, 6:44 p.m. UTC | #2

On Wed, 17 May 2023 09:39:41 -0700
Linus Torvalds <[e-mail]> wrote:

> On Wed, May 17, 2023 at 5:47 AM Arnd Bergmann <[e-mail]> wrote:
> >
> > From: Arnd Bergmann <[e-mail]>
> >
> > This function is only used when CONFIG_TRACE_BRANCH_PROFILING is
> > set, and the declaration is hidden behind this Kconfig symbol,
> > which causes a warning if disabled:  
> 
> That explanation is not very clear.
> 
> The problem is that the tracing code itself is built with
> DISABLE_BRANCH_PROFILING, in order to not recurse. And that hides the
> declaration when the definition is compiled, leading to the problem.
> 
> CONFIG_TRACE_BRANCH_PROFILING isn't the problem in itself - *that*
> part of the test is consistent (not used, not declared, and not
> compiled if it is off).
> 
> The problem is that DISABLE_BRANCH_PROFILING case, where it is used
> elsewhere, but not declared when it is itself compiled.
> 
> I applied the patch, but I tried to reword the explanation to be more
> clear. I may have failed.
> 

That's pretty much it. The DISABLE_BRANCH_PROFILING is to disable it in
various places where it will crash if used (vdso is one of them). It is
also used to prevent the code that declares it from recursing on itself (as
you stated).

Thus, the issue is simply that the declaration is hidden by the recursion
protection where the function is actually defined.

Reviewed-by: Steven Rostedt (Google) <[e-mail]>

-- Steve

Patch

diff --git a/include/linux/compiler.h b/include/linux/compiler.h
index 947a60b801db..d7779a18b24f 100644
--- a/include/linux/compiler.h
+++ b/include/linux/compiler.h
@@ -12,11 +12,10 @@ 
  * Note: DISABLE_BRANCH_PROFILING can be used by special lowlevel code
  * to disable branch tracing on a per file basis.
  */
-#if defined(CONFIG_TRACE_BRANCH_PROFILING) \
-    && !defined(DISABLE_BRANCH_PROFILING) && !defined(__CHECKER__)
 void ftrace_likely_update(struct ftrace_likely_data *f, int val,
 			  int expect, int is_constant);
-
+#if defined(CONFIG_TRACE_BRANCH_PROFILING) \
+    && !defined(DISABLE_BRANCH_PROFILING) && !defined(__CHECKER__)
 #define likely_notrace(x)	__builtin_expect(!!(x), 1)
 #define unlikely_notrace(x)	__builtin_expect(!!(x), 0)