Re: [PATCH] tracing: Decrement trace_array when bootconfig creates an instance

[Masami Hiramatsu <mhiramat@kernel.org>]

On Fri, 24 Jan 2020 20:59:27 -0500
Steven Rostedt <rostedt@goodmis.org> wrote:

> 
> From: "Steven Rostedt (VMware)" <rostedt@goodmis.org>
> 
> The trace_array_get_by_name() creates a ftrace instance and
> trace_array_put() is used to remove the reference. Even though the
> trace_array_get_by_name() creates the instance, it also adds a reference
> count to it, that prevents user space from removing it.
> 
> As the bootconfig just creates the instance on boot up, it should still be
> used where it can be deleted by user space after boot. A trace_array_put()
> is required to let that happen. Otherwise, the created instance can not
> be removed.
> 
> Also, change the documentation on trace_array_get_by_name() to make this not
> be so confusing.

Good catch! I misunderstood trace_array_put() would remove the instance.

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

Thank you!

> 
> Fixes: 4f712a4d04a4e ("tracing/boot: Add instance node support")
> Signed-off-by: Steven Rostedt (VMware) <rostedt@goodmis.org>
> ---
>  kernel/trace/trace.c      | 4 ++++
>  kernel/trace/trace_boot.c | 1 +
>  2 files changed, 5 insertions(+)
> 
> diff --git a/kernel/trace/trace.c b/kernel/trace/trace.c
> index 6fed9b0a8d58..0a5569b1cace 100644
> --- a/kernel/trace/trace.c
> +++ b/kernel/trace/trace.c
> @@ -8602,6 +8602,10 @@ static int instance_mkdir(const char *name)
>   * NOTE: This function increments the reference counter associated with the
>   * trace array returned. This makes sure it cannot be freed while in use.
>   * Use trace_array_put() once the trace array is no longer needed.
> + * If the trace_array is to be freed, trace_array_destroy() needs to
> + * be called after the trace_array_put(), or simply let user space delete
> + * it from the tracefs instances directory. But until the
> + * trace_array_put() is called, user space can not delete it.
>   *
>   */
>  struct trace_array *trace_array_get_by_name(const char *name)
> diff --git a/kernel/trace/trace_boot.c b/kernel/trace/trace_boot.c
> index cd541ac1cbc1..2f616cd926b0 100644
> --- a/kernel/trace/trace_boot.c
> +++ b/kernel/trace/trace_boot.c
> @@ -327,6 +327,7 @@ trace_boot_init_instances(struct xbc_node *node)
>  			continue;
>  		}
>  		trace_boot_init_one_instance(tr, inode);
> +		trace_array_put(tr);
>  	}
>  }
>  
> -- 
> 2.20.1
> 


-- 
Masami Hiramatsu <mhiramat@kernel.org>