tracing/osnoise: Fix notify new tracing_max_latency

[PATCH] tracing/osnoise: Fix notify new tracing_max_latency

Posted by Daniel Bristot de Oliveira 1 year, 1 month ago

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(-)

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;
-- 
2.38.1

Re: [PATCH] tracing/osnoise: Fix notify new tracing_max_latency

Posted by Steven Rostedt 1 year ago

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;

Re: [PATCH] tracing/osnoise: Fix notify new tracing_max_latency

Posted by Daniel Bristot de Oliveira 1 year ago

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;
>