Re: [PATCH] tracing: Have traceon and traceoff trigger honor the instance

[Tom Zanussi <zanussi@kernel.org>]

Hi Steve,

On Wed, 2022-02-23 at 22:38 -0500, Steven Rostedt wrote:
> From: "Steven Rostedt (Google)" <rostedt@goodmis.org>
> 
> If a trigger is set on an event to disable or enable tracing within
> an
> instance, then tracing should be disabled or enabled in the instance
> and
> not at the top level, which is confusing to users.
> 
> Cc: stable@vger.kernel.org
> Fixes: ae63b31e4d0e2 ("tracing: Separate out trace events from global
> variables")
> Signed-off-by: Steven Rostedt (Google) <rostedt@goodmis.org>
> ---
>  kernel/trace/trace_events_trigger.c | 52 +++++++++++++++++++++++++
> ----
>  1 file changed, 46 insertions(+), 6 deletions(-)
> 
> diff --git a/kernel/trace/trace_events_trigger.c
> b/kernel/trace/trace_events_trigger.c
> index e0d50c9577f3..efe563140f27 100644
> --- a/kernel/trace/trace_events_trigger.c
> +++ b/kernel/trace/trace_events_trigger.c
> @@ -1295,6 +1295,16 @@ traceon_trigger(struct event_trigger_data
> *data,
>  		struct trace_buffer *buffer, void *rec,
>  		struct ring_buffer_event *event)
>  {
> +	struct trace_event_file *file = data->private_data;
> +
> +	if (file) {
> +		if (tracer_tracing_is_on(file->tr))
> +			return;
> +
> +		tracer_tracing_on(file->tr);
> +		return;
> +	}
> +
>  	if (tracing_is_on())
>  		return;
>  
> @@ -1306,8 +1316,15 @@ traceon_count_trigger(struct
> event_trigger_data *data,
>  		      struct trace_buffer *buffer, void *rec,
>  		      struct ring_buffer_event *event)
>  {
> -	if (tracing_is_on())
> -		return;
> +	struct trace_event_file *file = data->private_data;
> +
> +	if (file) {
> +		if (tracer_tracing_is_on(file->tr))
> +			return;
> +	} else {
> +		if (tracing_is_on())
> +			return;
> +	}
>  
>  	if (!data->count)
>  		return;
> @@ -1315,7 +1332,10 @@ traceon_count_trigger(struct
> event_trigger_data *data,
>  	if (data->count != -1)
>  		(data->count)--;
>  
> -	tracing_on();
> +	if (file)
> +		tracer_tracing_on(file->tr);
> +	else
> +		tracing_on();
>  }
>  
>  static void
> @@ -1323,6 +1343,16 @@ traceoff_trigger(struct event_trigger_data
> *data,
>  		 struct trace_buffer *buffer, void *rec,
>  		 struct ring_buffer_event *event)
>  {
> +	struct trace_event_file *file = data->private_data;
> +
> +	if (file) {
> +		if (!tracer_tracing_is_on(file->tr))
> +			return;
> +
> +		tracer_tracing_off(file->tr);
> +		return;
> +	}
> +
>  	if (!tracing_is_on())
>  		return;
>  
> @@ -1334,8 +1364,15 @@ traceoff_count_trigger(struct
> event_trigger_data *data,
>  		       struct trace_buffer *buffer, void *rec,
>  		       struct ring_buffer_event *event)
>  {
> -	if (!tracing_is_on())
> -		return;
> +	struct trace_event_file *file = data->private_data;
> +
> +	if (file) {
> +		if (!tracer_tracing_is_on(file->tr))
> +			return;
> +	} else {
> +		if (!tracing_is_on())
> +			return;
> +	}
>  
>  	if (!data->count)
>  		return;
> @@ -1343,7 +1380,10 @@ traceoff_count_trigger(struct
> event_trigger_data *data,
>  	if (data->count != -1)
>  		(data->count)--;
>  
> -	tracing_off();
> +	if (file)
> +		tracer_tracing_off(file->tr);
> +	else
> +		tracing_off();
>  }
>  
>  static int


Looks good,

Reviewed-by: Tom Zanussi <zanussi@kernel.org>

Thanks,

Tom