Commit 9182b519 authored by Steven Rostedt (Google)'s avatar Steven Rostedt (Google)
Browse files

tracing: Remove unnecessary copying of tr->current_trace 

The iterator allocated a descriptor to copy the current_trace. This was done
with the assumption that the function pointers might change. But this was a
false assuption, as it does not change. There's no reason to make a copy of the
current_trace and just use the pointer it points to. This removes needing to
manage freeing the descriptor. Worse yet, there's locations that the iterator
is used but does make a copy and just uses the pointer. This could cause the
actual pointer to the trace descriptor to be freed and not the allocated copy.

This is more of a clean up than a fix.

Link: https://lkml.kernel.org/r/20230715141348.135792275@goodmis.org



Cc: Mark Rutland <mark.rutland@arm.com>
Cc: Andrew Morton <akpm@linux-foundation.org>
Fixes: d7350c3f ("tracing/core: make the read callbacks reentrants")
Reviewed-by: default avatarMasami Hiramatsu (Google) <mhiramat@kernel.org>
Signed-off-by: default avatarSteven Rostedt (Google) <rostedt@goodmis.org>
parent 00a8478f

kernel/trace/trace.c

+3 −19
Original line number Diff line number Diff line
@@ -4189,15 +4189,9 @@ static void *s_start(struct seq_file *m, loff_t *pos) 
	loff_t l = 0;

	int cpu;



	/*

	 * copy the tracer to avoid using a global lock all around.

	 * iter->trace is a copy of current_trace, the pointer to the

	 * name may be used instead of a strcmp(), as iter->trace->name

	 * will point to the same string as current_trace->name.

	 */

	mutex_lock(&trace_types_lock);

	if (unlikely(tr->current_trace && iter->trace->name != tr->current_trace->name))

		*iter->trace = *tr->current_trace;

	if (unlikely(tr->current_trace != iter->trace))

		iter->trace = tr->current_trace;

	mutex_unlock(&trace_types_lock);



#ifdef CONFIG_TRACER_MAX_TRACE
@@ -4846,16 +4840,8 @@ __tracing_open(struct inode *inode, struct file *file, bool snapshot) 
	iter->fmt = NULL;

	iter->fmt_size = 0;



	/*

	 * We make a copy of the current tracer to avoid concurrent

	 * changes on it while we are reading.

	 */

	mutex_lock(&trace_types_lock);

	iter->trace = kzalloc(sizeof(*iter->trace), GFP_KERNEL);

	if (!iter->trace)

		goto fail;



	*iter->trace = *tr->current_trace;

	iter->trace = tr->current_trace;



	if (!zalloc_cpumask_var(&iter->started, GFP_KERNEL))

		goto fail;
@@ -4920,7 +4906,6 @@ __tracing_open(struct inode *inode, struct file *file, bool snapshot) 


 fail:

	mutex_unlock(&trace_types_lock);

	kfree(iter->trace);

	kfree(iter->temp);

	kfree(iter->buffer_iter);

release:
@@ -5005,7 +4990,6 @@ static int tracing_release(struct inode *inode, struct file *file) 
	free_cpumask_var(iter->started);

	kfree(iter->fmt);

	kfree(iter->temp);

	kfree(iter->trace);

	kfree(iter->buffer_iter);

	seq_release_private(inode, file);