Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Message-ID: <d3004368d5784e3c2a40e3229d3618a88924b11e.camel@kernel.org>
Subject: Re: [PATCH] tracing: Have traceon and traceoff trigger honor the
 instance
From:   Tom Zanussi <zanussi@kernel.org>
To:     Steven Rostedt <rostedt@goodmis.org>,
        LKML <linux-kernel@vger.kernel.org>
Cc:     Ingo Molnar <mingo@kernel.org>,
        Andrew Morton <akpm@linux-foundation.org>,
        Daniel Bristot de Oliveira <bristot@redhat.com>
Date:   Fri, 25 Feb 2022 11:00:18 -0600
In-Reply-To: <20220223223837.14f94ec3@rorschach.local.home>
References: <20220223223837.14f94ec3@rorschach.local.home>
Content-Type: text/plain; charset="UTF-8"
Mime-Version: 1.0
Content-Transfer-Encoding: 7bit
Precedence: bulk

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