Re: [PATCH] tracing: Reset the function filter after completing trampoline/graph selftest

[Steven Rostedt]

On Mon, 18 Apr 2022 15:39:58 +0800
Li Huafei <lihuafei1@xxxxxxxxxx> wrote:

> The direct trampoline and graph coexistence test sets global_ops to
> trace only 'trace_selftest_dynamic_test_func', but does not reset it
> after the test is completed, resulting in the function filter being set
> already after the system starts. Although it can be reset through the
> tracefs interface, it is more or less confusing to the user, and we
> should reset it to trace all functions after the trampoline/graph test
> completes.
> 
> Fixes: 130c08065848 ("tracing: Add trampoline/graph selftest")
> Signed-off-by: Li Huafei <lihuafei1@xxxxxxxxxx>
> ---
>  kernel/trace/trace_selftest.c | 12 ++++++++----
>  1 file changed, 8 insertions(+), 4 deletions(-)
> 
> diff --git a/kernel/trace/trace_selftest.c b/kernel/trace/trace_selftest.c
> index abcadbe933bb..e89f72571f8e 100644
> --- a/kernel/trace/trace_selftest.c
> +++ b/kernel/trace/trace_selftest.c
> @@ -866,12 +866,12 @@ trace_selftest_startup_function_graph(struct tracer *trace,
>  	ret = register_ftrace_direct((unsigned long) DYN_FTRACE_TEST_NAME,
>  				     (unsigned long) trace_direct_tramp);
>  	if (ret)
> -		goto out;
> +		goto reset_filter;
>  
>  	ret = register_ftrace_graph(&fgraph_ops);
>  	if (ret) {
>  		warn_failed_init_tracer(trace, ret);
> -		goto out;
> +		goto reset_filter;
>  	}
>  
>  	DYN_FTRACE_TEST_NAME();
> @@ -887,14 +887,18 @@ trace_selftest_startup_function_graph(struct tracer *trace,
>  	ret = unregister_ftrace_direct((unsigned long) DYN_FTRACE_TEST_NAME,
>  				       (unsigned long) trace_direct_tramp);
>  	if (ret)
> -		goto out;
> +		goto reset_filter;
>  
>  	tracing_start();
>  
>  	if (!ret && !count) {
>  		ret = -1;
> -		goto out;
> +		goto reset_filter;

No need for all the 'goto reset_filter', if this function fails, then the
tracer is disabled, and there's no reason to clear the filter. In fact, it
may cause a crash (because something bad happened).

-- Steve


>  	}
> +
> +reset_filter:
> +	/* Enable tracing on all functions again */
> +	ftrace_set_global_filter(NULL, 0, 1);
>  #endif
>  
>  	/* Don't test dynamic tracing, the function tracer already did */