tracing/osnoise: Fix notify new tracing_max_latency

Message ID c22302a7db3cee6bcabf87462ed22a35dd38690f.1679675521.git.bristot@kernel.org
State New
Headers
Series tracing/osnoise: Fix notify new tracing_max_latency |

Commit Message

Daniel Bristot de Oliveira March 24, 2023, 5:51 p.m. UTC
  timerlat is not reporting a new tracing_max_latency for the thread
latency. Also, the tracer is reporting new max latency on instances
where the tracing is off, creating inconsistencies between the max
reported values in the trace and in the tracing_max_latency. Thus
only report new tracing_max_latency on active tracing instances.

Fixes: dae181349f1e ("tracing/osnoise: Support a list of trace_array *tr")
Cc: Daniel Bristot de Oliveira <bristot@kernel.org>
Cc: Steven Rostedt <rostedt@goodmis.org>
Cc: Masami Hiramatsu <mhiramat@kernel.org>
Signed-off-by: Daniel Bristot de Oliveira <bristot@kernel.org>
---
 kernel/trace/trace_osnoise.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)
  

Comments

Steven Rostedt March 28, 2023, 11:16 p.m. UTC | #1
On Fri, 24 Mar 2023 18:51:35 +0100
Daniel Bristot de Oliveira <bristot@kernel.org> wrote:

> timerlat is not reporting a new tracing_max_latency for the thread
> latency. Also, the tracer is reporting new max latency on instances
> where the tracing is off, creating inconsistencies between the max
> reported values in the trace and in the tracing_max_latency. Thus
> only report new tracing_max_latency on active tracing instances.

Sounds to me this patch fixes two different bugs. If so, can you please
break it up into two different patches?

Thanks!

-- Steve

> 
> Fixes: dae181349f1e ("tracing/osnoise: Support a list of trace_array *tr")
> Cc: Daniel Bristot de Oliveira <bristot@kernel.org>
> Cc: Steven Rostedt <rostedt@goodmis.org>
> Cc: Masami Hiramatsu <mhiramat@kernel.org>
> Signed-off-by: Daniel Bristot de Oliveira <bristot@kernel.org>
> ---
>  kernel/trace/trace_osnoise.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/kernel/trace/trace_osnoise.c b/kernel/trace/trace_osnoise.c
> index 210e1f168392..2a1b337ac643 100644
> --- a/kernel/trace/trace_osnoise.c
> +++ b/kernel/trace/trace_osnoise.c
> @@ -1296,7 +1296,7 @@ static void notify_new_max_latency(u64 latency)
>  	rcu_read_lock();
>  	list_for_each_entry_rcu(inst, &osnoise_instances, list) {
>  		tr = inst->tr;
> -		if (tr->max_latency < latency) {
> +		if (tracer_tracing_is_on(tr) && tr->max_latency < latency) {
>  			tr->max_latency = latency;
>  			latency_fsnotify(tr);
>  		}
> @@ -1738,6 +1738,8 @@ static int timerlat_main(void *data)
>  
>  		trace_timerlat_sample(&s);
>  
> +		notify_new_max_latency(diff);
> +
>  		timerlat_dump_stack(time_to_us(diff));
>  
>  		tlat->tracing_thread = false;
  
Daniel Bristot de Oliveira March 29, 2023, 6:31 a.m. UTC | #2
On 3/29/23 01:16, Steven Rostedt wrote:
> On Fri, 24 Mar 2023 18:51:35 +0100
> Daniel Bristot de Oliveira <bristot@kernel.org> wrote:
> 
>> timerlat is not reporting a new tracing_max_latency for the thread
>> latency. Also, the tracer is reporting new max latency on instances
>> where the tracing is off, creating inconsistencies between the max
>> reported values in the trace and in the tracing_max_latency. Thus
>> only report new tracing_max_latency on active tracing instances.
> 
> Sounds to me this patch fixes two different bugs. If so, can you please
> break it up into two different patches?

Yeah, I was dubious about sending one or two patches when I sent this one... But
it makes sense making two of them... so I will do.

Thanks!

-- Daniel

> Thanks!
> 
> -- Steve
> 
>>
>> Fixes: dae181349f1e ("tracing/osnoise: Support a list of trace_array *tr")
>> Cc: Daniel Bristot de Oliveira <bristot@kernel.org>
>> Cc: Steven Rostedt <rostedt@goodmis.org>
>> Cc: Masami Hiramatsu <mhiramat@kernel.org>
>> Signed-off-by: Daniel Bristot de Oliveira <bristot@kernel.org>
>> ---
>>  kernel/trace/trace_osnoise.c | 4 +++-
>>  1 file changed, 3 insertions(+), 1 deletion(-)
>>
>> diff --git a/kernel/trace/trace_osnoise.c b/kernel/trace/trace_osnoise.c
>> index 210e1f168392..2a1b337ac643 100644
>> --- a/kernel/trace/trace_osnoise.c
>> +++ b/kernel/trace/trace_osnoise.c
>> @@ -1296,7 +1296,7 @@ static void notify_new_max_latency(u64 latency)
>>  	rcu_read_lock();
>>  	list_for_each_entry_rcu(inst, &osnoise_instances, list) {
>>  		tr = inst->tr;
>> -		if (tr->max_latency < latency) {
>> +		if (tracer_tracing_is_on(tr) && tr->max_latency < latency) {
>>  			tr->max_latency = latency;
>>  			latency_fsnotify(tr);
>>  		}
>> @@ -1738,6 +1738,8 @@ static int timerlat_main(void *data)
>>  
>>  		trace_timerlat_sample(&s);
>>  
>> +		notify_new_max_latency(diff);
>> +
>>  		timerlat_dump_stack(time_to_us(diff));
>>  
>>  		tlat->tracing_thread = false;
>
  

Patch

diff --git a/kernel/trace/trace_osnoise.c b/kernel/trace/trace_osnoise.c
index 210e1f168392..2a1b337ac643 100644
--- a/kernel/trace/trace_osnoise.c
+++ b/kernel/trace/trace_osnoise.c
@@ -1296,7 +1296,7 @@  static void notify_new_max_latency(u64 latency)
 	rcu_read_lock();
 	list_for_each_entry_rcu(inst, &osnoise_instances, list) {
 		tr = inst->tr;
-		if (tr->max_latency < latency) {
+		if (tracer_tracing_is_on(tr) && tr->max_latency < latency) {
 			tr->max_latency = latency;
 			latency_fsnotify(tr);
 		}
@@ -1738,6 +1738,8 @@  static int timerlat_main(void *data)
 
 		trace_timerlat_sample(&s);
 
+		notify_new_max_latency(diff);
+
 		timerlat_dump_stack(time_to_us(diff));
 
 		tlat->tracing_thread = false;