[PATCH 0/6] ftrace: Minor fixes for sparse and kernel test robot

[PATCH 0/6] ftrace: Minor fixes for sparse and kernel test robot

[Steven Rostedt]

Recieved some minor bug reports from the kernel test robot. First I started
cleaning up some of the sparse warnings. There's many more, but most changes
are not really helping anything, but just quieting the warnings.

But the reports from kernel test robot need to be fixed.

Steven Rostedt (Google) (6):
      ftrace: Declare function_trace_op in header to quiet sparse warning
      ftrace: Assign ftrace_list_end to ftrace_ops_list type cast to RCU
      ftrace: Assign RCU list variable with rcu_assign_ptr()
      ftrace: Fix prototypes for ftrace_startup/shutdown_subops()
      function_graph: Make fgraph_do_direct static key static
      function_graph: Do not update pid func if CONFIG_DYNAMIC_FTRACE not enabled

----
 include/linux/ftrace.h         | 3 +++
 kernel/trace/fgraph.c          | 4 +++-
 kernel/trace/ftrace.c          | 4 ++--
 kernel/trace/ftrace_internal.h | 9 +++++++++
 kernel/trace/trace.h           | 1 -
 5 files changed, 17 insertions(+), 4 deletions(-)

[PATCH 1/6] ftrace: Declare function_trace_op in header to quiet sparse warning

[Steven Rostedt]

From: "Steven Rostedt (Google)" <rostedt@goodmis.org>

Sparse complains that function_trace_op is not static but is not declared
in a header file. It is used only in assembly code. But add it to a header
so that sparse no longer complains:

 kernel/trace/ftrace.c:99:19: warning: symbol 'function_trace_op' was not declared. Should it be static?

Signed-off-by: Steven Rostedt (Google) <rostedt@goodmis.org>
---
 include/linux/ftrace.h | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/include/linux/ftrace.h b/include/linux/ftrace.h
index 9f61556a9491..4135dc171447 100644
--- a/include/linux/ftrace.h
+++ b/include/linux/ftrace.h
@@ -1131,6 +1131,9 @@ extern void ftrace_graph_init_task(struct task_struct *t);
 extern void ftrace_graph_exit_task(struct task_struct *t);
 extern void ftrace_graph_init_idle_task(struct task_struct *t, int cpu);
 
+/* Used by assembly, but to quiet sparse warnings */
+extern struct ftrace_ops *function_trace_op;
+
 static inline void pause_graph_tracing(void)
 {
 	atomic_inc(&current->tracing_graph_pause);
-- 
2.43.0

[PATCH 2/6] ftrace: Assign ftrace_list_end to ftrace_ops_list type cast to RCU

[Steven Rostedt]

From: "Steven Rostedt (Google)" <rostedt@goodmis.org>

Use a type cast to convert ftrace_list_end to RCU when assigning
ftrace_ops_list. This will quiet the sparse warning:

 kernel/trace/ftrace.c:125:59: warning: incorrect type in initializer (different address spaces)
 kernel/trace/ftrace.c:125:59:    expected struct ftrace_ops [noderef] __rcu *[addressable] [toplevel] ftrace_ops_list
 kernel/trace/ftrace.c:125:59:    got struct ftrace_ops *

Signed-off-by: Steven Rostedt (Google) <rostedt@goodmis.org>
---
 kernel/trace/ftrace.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/kernel/trace/ftrace.c b/kernel/trace/ftrace.c
index da7e6abf48b4..3af0b1feb873 100644
--- a/kernel/trace/ftrace.c
+++ b/kernel/trace/ftrace.c
@@ -122,7 +122,7 @@ static int ftrace_disabled __read_mostly;
 
 DEFINE_MUTEX(ftrace_lock);
 
-struct ftrace_ops __rcu *ftrace_ops_list __read_mostly = &ftrace_list_end;
+struct ftrace_ops __rcu *ftrace_ops_list __read_mostly = (struct ftrace_ops __rcu *)&ftrace_list_end;
 ftrace_func_t ftrace_trace_function __read_mostly = ftrace_stub;
 struct ftrace_ops global_ops;
 
-- 
2.43.0

[PATCH 3/6] ftrace: Assign RCU list variable with rcu_assign_ptr()

[Steven Rostedt]

From: "Steven Rostedt (Google)" <rostedt@goodmis.org>

Use rcu_assign_ptr() to assign the list pointer as it is marked as RCU,
and this quiets the sparse warning:

   kernel/trace/ftrace.c:313:23: warning: incorrect type in assignment (different address spaces)
   kernel/trace/ftrace.c:313:23:    expected struct ftrace_ops [noderef] __rcu *
   kernel/trace/ftrace.c:313:23:    got struct ftrace_ops *

Signed-off-by: Steven Rostedt (Google) <rostedt@goodmis.org>
---
 kernel/trace/ftrace.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/kernel/trace/ftrace.c b/kernel/trace/ftrace.c
index 3af0b1feb873..a39842a53444 100644
--- a/kernel/trace/ftrace.c
+++ b/kernel/trace/ftrace.c
@@ -310,7 +310,7 @@ static int remove_ftrace_ops(struct ftrace_ops __rcu **list,
 			lockdep_is_held(&ftrace_lock)) == ops &&
 	    rcu_dereference_protected(ops->next,
 			lockdep_is_held(&ftrace_lock)) == &ftrace_list_end) {
-		*list = &ftrace_list_end;
+		rcu_assign_pointer(*list, &ftrace_list_end);
 		return 0;
 	}
 
-- 
2.43.0

[PATCH 4/6] ftrace: Fix prototypes for ftrace_startup/shutdown_subops()

[Steven Rostedt]

From: "Steven Rostedt (Google)" <rostedt@goodmis.org>

The ftrace_startup_subops() was in the wrong header, and both functions
were not defined on !CONFIG_DYNAMIC_FTRACE.

Fixes: 5fccc7552ccbc ("ftrace: Add subops logic to allow one ops to manage many")
Reported-by: kernel test robot <lkp@intel.com>
Closes: https://lore.kernel.org/oe-kbuild-all/202406051524.a12JqLqx-lkp@intel.com/
Signed-off-by: Steven Rostedt (Google) <rostedt@goodmis.org>
---
 kernel/trace/ftrace_internal.h | 9 +++++++++
 kernel/trace/trace.h           | 1 -
 2 files changed, 9 insertions(+), 1 deletion(-)

diff --git a/kernel/trace/ftrace_internal.h b/kernel/trace/ftrace_internal.h
index bfba10c2fcf1..4bb1e881154a 100644
--- a/kernel/trace/ftrace_internal.h
+++ b/kernel/trace/ftrace_internal.h
@@ -15,6 +15,7 @@ extern struct ftrace_ops global_ops;
 int ftrace_startup(struct ftrace_ops *ops, int command);
 int ftrace_shutdown(struct ftrace_ops *ops, int command);
 int ftrace_ops_test(struct ftrace_ops *ops, unsigned long ip, void *regs);
+int ftrace_startup_subops(struct ftrace_ops *ops, struct ftrace_ops *subops, int command);
 int ftrace_shutdown_subops(struct ftrace_ops *ops, struct ftrace_ops *subops, int command);
 
 #else /* !CONFIG_DYNAMIC_FTRACE */
@@ -39,6 +40,14 @@ ftrace_ops_test(struct ftrace_ops *ops, unsigned long ip, void *regs)
 {
 	return 1;
 }
+static inline int ftrace_startup_subops(struct ftrace_ops *ops, struct ftrace_ops *subops, int command)
+{
+	return -EINVAL;
+}
+static inline int ftrace_shutdown_subops(struct ftrace_ops *ops, struct ftrace_ops *subops, int command)
+{
+	return -EINVAL;
+}
 #endif /* CONFIG_DYNAMIC_FTRACE */
 
 #ifdef CONFIG_FUNCTION_GRAPH_TRACER
diff --git a/kernel/trace/trace.h b/kernel/trace/trace.h
index b37402e3f0c9..8783bebd0562 100644
--- a/kernel/trace/trace.h
+++ b/kernel/trace/trace.h
@@ -1186,7 +1186,6 @@ extern int ftrace_set_filter(struct ftrace_ops *ops, unsigned char *buf,
 			     int len, int reset);
 extern int ftrace_set_notrace(struct ftrace_ops *ops, unsigned char *buf,
 			      int len, int reset);
-extern int ftrace_startup_subops(struct ftrace_ops *ops, struct ftrace_ops *subops, int command);
 #else
 struct ftrace_func_command;
 
-- 
2.43.0

[PATCH 5/6] function_graph: Make fgraph_do_direct static key static

[Steven Rostedt]

From: "Steven Rostedt (Google)" <rostedt@goodmis.org>

The static branch key "fgraph_do_direct" was not declared static but is
only used in one file. Change it to a static variable.

Fixes: cc60ee813b503 ("function_graph: Use static_call and branch to optimize entry function")
Reported-by: kernel test robot <lkp@intel.com>
Closes: https://lore.kernel.org/oe-kbuild-all/202406051711.dS1sQZ9n-lkp@intel.com/
Signed-off-by: Steven Rostedt (Google) <rostedt@goodmis.org>
---
 kernel/trace/fgraph.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/kernel/trace/fgraph.c b/kernel/trace/fgraph.c
index 4bf91eebbb08..63d828054c79 100644
--- a/kernel/trace/fgraph.c
+++ b/kernel/trace/fgraph.c
@@ -515,7 +515,7 @@ static struct fgraph_ops fgraph_stub = {
 static struct fgraph_ops *fgraph_direct_gops = &fgraph_stub;
 DEFINE_STATIC_CALL(fgraph_func, ftrace_graph_entry_stub);
 DEFINE_STATIC_CALL(fgraph_retfunc, ftrace_graph_ret_stub);
-DEFINE_STATIC_KEY_TRUE(fgraph_do_direct);
+static DEFINE_STATIC_KEY_TRUE(fgraph_do_direct);
 
 /**
  * ftrace_graph_stop - set to permanently disable function graph tracing
-- 
2.43.0

[PATCH 6/6] function_graph: Do not update pid func if CONFIG_DYNAMIC_FTRACE not enabled

[Steven Rostedt]

From: "Steven Rostedt (Google)" <rostedt@goodmis.org>

The ftrace subops is only defined if CONFIG_DYNAMIC_FTRACE is enabled. If
it is not, function tracing is extremely limited, and the subops in the
ftrace_ops structure is not defined (and will fail to compile). If
DYNAMIC_FTRACE is not enabled, then function graph filtering will not
work (as it shouldn't).

Fixes: df3ec5da6a1e7 ("function_graph: Add pid tracing back to function graph tracer")
Reported-by: kernel test robot <lkp@intel.com>
Closes: https://lore.kernel.org/oe-kbuild-all/202406051855.9VIYXbTB-lkp@intel.com/
Signed-off-by: Steven Rostedt (Google) <rostedt@goodmis.org>
---
 kernel/trace/fgraph.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/kernel/trace/fgraph.c b/kernel/trace/fgraph.c
index 63d828054c79..c0e428c87ea5 100644
--- a/kernel/trace/fgraph.c
+++ b/kernel/trace/fgraph.c
@@ -1177,6 +1177,7 @@ void fgraph_update_pid_func(void)
 	if (!(graph_ops.flags & FTRACE_OPS_FL_INITIALIZED))
 		return;
 
+#ifdef CONFIG_DYNAMIC_FTRACE
 	list_for_each_entry(op, &graph_ops.subop_list, list) {
 		if (op->flags & FTRACE_OPS_FL_PID) {
 			gops = container_of(op, struct fgraph_ops, ops);
@@ -1186,6 +1187,7 @@ void fgraph_update_pid_func(void)
 				static_call_update(fgraph_func, gops->entryfunc);
 		}
 	}
+#endif
 }
 
 /* Allocate a return stack for each task */
-- 
2.43.0

Re: [PATCH 0/6] ftrace: Minor fixes for sparse and kernel test robot

[Masami Hiramatsu]

On Wed, 05 Jun 2024 16:26:44 -0400
Steven Rostedt <rostedt@goodmis.org> wrote:

> 
> Recieved some minor bug reports from the kernel test robot. First I started
> cleaning up some of the sparse warnings. There's many more, but most changes
> are not really helping anything, but just quieting the warnings.
> 
> But the reports from kernel test robot need to be fixed.

All looks good to me.

Acked-by: Masami Hiramatsu (Google) <mhiramat@kernel.org>

Thank you!

> 
> Steven Rostedt (Google) (6):
>       ftrace: Declare function_trace_op in header to quiet sparse warning
>       ftrace: Assign ftrace_list_end to ftrace_ops_list type cast to RCU
>       ftrace: Assign RCU list variable with rcu_assign_ptr()
>       ftrace: Fix prototypes for ftrace_startup/shutdown_subops()
>       function_graph: Make fgraph_do_direct static key static
>       function_graph: Do not update pid func if CONFIG_DYNAMIC_FTRACE not enabled
> 
> ----
>  include/linux/ftrace.h         | 3 +++
>  kernel/trace/fgraph.c          | 4 +++-
>  kernel/trace/ftrace.c          | 4 ++--
>  kernel/trace/ftrace_internal.h | 9 +++++++++
>  kernel/trace/trace.h           | 1 -
>  5 files changed, 17 insertions(+), 4 deletions(-)


-- 
Masami Hiramatsu (Google) <mhiramat@kernel.org>