One can enable dyanmic tracing but disable sysctls.
When this is doen we get the compile kernel warning:
CC kernel/trace/ftrace.o
kernel/trace/ftrace.c:3086:13: warning: ‘ftrace_shutdown_sysctl’ defined
but not used [-Wunused-function]
3086 | static void ftrace_shutdown_sysctl(void)
| ^~~~~~~~~~~~~~~~~~~~~~
kernel/trace/ftrace.c:3068:13: warning: ‘ftrace_startup_sysctl’ defined
but not used [-Wunused-function]
3068 | static void ftrace_startup_sysctl(void)
When CONFIG_DYNAMIC_FTRACE=n the ftrace_startup_sysctl() and
routines ftrace_shutdown_sysctl() still compiles, so these
are actually more just used for when SYSCTL=y.
Fix this then by just moving these routines to when sysctls
are enabled.
Fixes: 7cde53da38a3 ("ftrace: move sysctl_ftrace_enabled to ftrace.c")
Reported-by: kernel test robot <[email protected]>
Signed-off-by: Luis Chamberlain <[email protected]>
---
Steven,
As with the other fixes I can take this in through sysctl-next if you
are OK with that, please let me know.
kernel/trace/ftrace.c | 71 +++++++++++++++++++++----------------------
1 file changed, 34 insertions(+), 37 deletions(-)
diff --git a/kernel/trace/ftrace.c b/kernel/trace/ftrace.c
index db8d553728b6..d9424fd9a183 100644
--- a/kernel/trace/ftrace.c
+++ b/kernel/trace/ftrace.c
@@ -3065,40 +3065,6 @@ int ftrace_shutdown(struct ftrace_ops *ops, int command)
return 0;
}
-static void ftrace_startup_sysctl(void)
-{
- int command;
-
- if (unlikely(ftrace_disabled))
- return;
-
- /* Force update next time */
- saved_ftrace_func = NULL;
- /* ftrace_start_up is true if we want ftrace running */
- if (ftrace_start_up) {
- command = FTRACE_UPDATE_CALLS;
- if (ftrace_graph_active)
- command |= FTRACE_START_FUNC_RET;
- ftrace_startup_enable(command);
- }
-}
-
-static void ftrace_shutdown_sysctl(void)
-{
- int command;
-
- if (unlikely(ftrace_disabled))
- return;
-
- /* ftrace_start_up is true if ftrace is running */
- if (ftrace_start_up) {
- command = FTRACE_DISABLE_CALLS;
- if (ftrace_graph_active)
- command |= FTRACE_STOP_FUNC_RET;
- ftrace_run_update_code(command);
- }
-}
-
static u64 ftrace_update_time;
unsigned long ftrace_update_tot_cnt;
unsigned long ftrace_number_of_pages;
@@ -7267,9 +7233,6 @@ core_initcall(ftrace_nodyn_init);
static inline int ftrace_init_dyn_tracefs(struct dentry *d_tracer) { return 0; }
static inline void ftrace_startup_all(int command) { }
-# define ftrace_startup_sysctl() do { } while (0)
-# define ftrace_shutdown_sysctl() do { } while (0)
-
static void ftrace_update_trampoline(struct ftrace_ops *ops)
{
}
@@ -7910,6 +7873,40 @@ int unregister_ftrace_function(struct ftrace_ops *ops)
EXPORT_SYMBOL_GPL(unregister_ftrace_function);
#ifdef CONFIG_SYSCTL
+static void ftrace_startup_sysctl(void)
+{
+ int command;
+
+ if (unlikely(ftrace_disabled))
+ return;
+
+ /* Force update next time */
+ saved_ftrace_func = NULL;
+ /* ftrace_start_up is true if we want ftrace running */
+ if (ftrace_start_up) {
+ command = FTRACE_UPDATE_CALLS;
+ if (ftrace_graph_active)
+ command |= FTRACE_START_FUNC_RET;
+ ftrace_startup_enable(command);
+ }
+}
+
+static void ftrace_shutdown_sysctl(void)
+{
+ int command;
+
+ if (unlikely(ftrace_disabled))
+ return;
+
+ /* ftrace_start_up is true if ftrace is running */
+ if (ftrace_start_up) {
+ command = FTRACE_DISABLE_CALLS;
+ if (ftrace_graph_active)
+ command |= FTRACE_STOP_FUNC_RET;
+ ftrace_run_update_code(command);
+ }
+}
+
static bool is_permanent_ops_registered(void)
{
struct ftrace_ops *op;
--
2.30.2
On Wed, Apr 20, 2022 at 07:28:07PM -0400, Steven Rostedt wrote:
> On Fri, 15 Apr 2022 14:54:04 -0700
> Luis Chamberlain <[email protected]> wrote:
>
> > Steven,
> >
> > As with the other fixes I can take this in through sysctl-next if you
> > are OK with that, please let me know.
>
> Honestly I would love to just nuke the ftrace_enabled sysctl, it's totally
> obsolete. Perhaps I should have it trigger some kind of warning message
> that it will be going away soon, and perhaps we can remove it?
I'm not sure if we can remove proc sysctls, that can break userpace
interaction. But deprecating it.. yeah I think it's fine to strive
for it, and just make the code do at least what it used to. In fact
sadly I don't see this documented well, we do have the documentation in
Documentation/admin-guide/sysctl/ but we don't really spell it out if
we can nuke some. Eric, do you have any thoughts on eventually
*removing* sysctls? Maybe we should document this.
BTW yeah another thing I noticed while looking at ftrace.c file was it could
probably use some spring cleaning by splitting the file up into multiple
files. Hey I'm not volunteering :) just an idea.
> Anyway, yeah, take this through your tree.
OK thanks, 0day found one last issue, I'll send a patch for that just
now. Let me know if you're OK for me to just squash that into this
patch.
> Acked-by: Steven Rostedt (Google) <[email protected]>
Luis
On Fri, 15 Apr 2022 14:54:04 -0700
Luis Chamberlain <[email protected]> wrote:
> Steven,
>
> As with the other fixes I can take this in through sysctl-next if you
> are OK with that, please let me know.
Honestly I would love to just nuke the ftrace_enabled sysctl, it's totally
obsolete. Perhaps I should have it trigger some kind of warning message
that it will be going away soon, and perhaps we can remove it?
Anyway, yeah, take this through your tree.
Acked-by: Steven Rostedt (Google) <[email protected]>
-- Steve