[PATCH] ftrace: allow dumping traces without tracking trace started cpus

[PATCH] ftrace: allow dumping traces without tracking trace started cpus

[Sasha Levin]

We don't init iter->started when dumping the ftrace buffer, and there's no
real need to do so - so allow skipping that check if the iter doesn't have
an initialized ->started cpumask.

Signed-off-by: Sasha Levin <sasha.levin@oracle.com>
---
 kernel/trace/trace.c |    5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/kernel/trace/trace.c b/kernel/trace/trace.c
index 6e79408..0dc647f 100644
--- a/kernel/trace/trace.c
+++ b/kernel/trace/trace.c
@@ -2671,13 +2671,14 @@ static void test_cpu_buff_start(struct trace_iterator *iter)
 	if (!(iter->iter_flags & TRACE_FILE_ANNOTATE))
 		return;
 
-	if (cpumask_test_cpu(iter->cpu, iter->started))
+	if (iter->started && cpumask_test_cpu(iter->cpu, iter->started))
 		return;
 
 	if (per_cpu_ptr(iter->trace_buffer->data, iter->cpu)->skipped_entries)
 		return;
 
-	cpumask_set_cpu(iter->cpu, iter->started);
+	if (iter->started)
+		cpumask_set_cpu(iter->cpu, iter->started);
 
 	/* Don't print started cpu buffer for the first entry of the trace */
 	if (iter->idx > 1)
-- 
1.7.10.4

Re: [PATCH] ftrace: allow dumping traces without tracking trace started cpus

[Minfei Huang]

On 09/04/15 at 12:45pm, Sasha Levin wrote:
> We don't init iter->started when dumping the ftrace buffer, and there's no
> real need to do so - so allow skipping that check if the iter doesn't have
> an initialized ->started cpumask.
> 
> Signed-off-by: Sasha Levin <sasha.levin@oracle.com>
> ---
>  kernel/trace/trace.c |    5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/kernel/trace/trace.c b/kernel/trace/trace.c
> index 6e79408..0dc647f 100644
> --- a/kernel/trace/trace.c
> +++ b/kernel/trace/trace.c
> @@ -2671,13 +2671,14 @@ static void test_cpu_buff_start(struct trace_iterator *iter)
>  	if (!(iter->iter_flags & TRACE_FILE_ANNOTATE))
>  		return;
>  
> -	if (cpumask_test_cpu(iter->cpu, iter->started))
> +	if (iter->started && cpumask_test_cpu(iter->cpu, iter->started))
>  		return;

Ftrace will initialize the variable iter->started in the function
trace_init_global_iter. Otherwise kernel will panic during calling the
function cpumask_test_cpu.

So it is safe to call the function cpumask_test_cpu without doing
checking.

Thanks
Minfei

Re: [PATCH] ftrace: allow dumping traces without tracking trace started cpus

[Sasha Levin]

On 09/06/2015 05:20 AM, Minfei Huang wrote:
> On 09/04/15 at 12:45pm, Sasha Levin wrote:
>> > We don't init iter->started when dumping the ftrace buffer, and there's no
>> > real need to do so - so allow skipping that check if the iter doesn't have
>> > an initialized ->started cpumask.
>> > 
>> > Signed-off-by: Sasha Levin <sasha.levin@oracle.com>
>> > ---
>> >  kernel/trace/trace.c |    5 +++--
>> >  1 file changed, 3 insertions(+), 2 deletions(-)
>> > 
>> > diff --git a/kernel/trace/trace.c b/kernel/trace/trace.c
>> > index 6e79408..0dc647f 100644
>> > --- a/kernel/trace/trace.c
>> > +++ b/kernel/trace/trace.c
>> > @@ -2671,13 +2671,14 @@ static void test_cpu_buff_start(struct trace_iterator *iter)
>> >  	if (!(iter->iter_flags & TRACE_FILE_ANNOTATE))
>> >  		return;
>> >  
>> > -	if (cpumask_test_cpu(iter->cpu, iter->started))
>> > +	if (iter->started && cpumask_test_cpu(iter->cpu, iter->started))
>> >  		return;
> Ftrace will initialize the variable iter->started in the function
> trace_init_global_iter. Otherwise kernel will panic during calling the
> function cpumask_test_cpu.
> 
> So it is safe to call the function cpumask_test_cpu without doing
> checking.

Can you point me to exactly where trace_init_global_iter() initializes
iter->started?


Thanks,
Sasha

Re: [PATCH] ftrace: allow dumping traces without tracking trace started cpus

[Steven Rostedt]

On Sun, 06 Sep 2015 10:29:43 -0400
Sasha Levin <sasha.levin@oracle.com> wrote:


> > So it is safe to call the function cpumask_test_cpu without doing
> > checking.
> 
> Can you point me to exactly where trace_init_global_iter() initializes
> iter->started?

Wouldn't the better solution be to initialize it in that function,
instead of checking if it is NULL? I think that's the true fix.
"started" should not be ignored.

-- Steve

Re: [PATCH] ftrace: allow dumping traces without tracking trace started cpus

[Sasha Levin]

On 09/08/2015 10:13 AM, Steven Rostedt wrote:
> On Sun, 06 Sep 2015 10:29:43 -0400
> Sasha Levin <sasha.levin@oracle.com> wrote:
> 
> 
>>> So it is safe to call the function cpumask_test_cpu without doing
>>> checking.
>>
>> Can you point me to exactly where trace_init_global_iter() initializes
>> iter->started?
> 
> Wouldn't the better solution be to initialize it in that function,
> instead of checking if it is NULL? I think that's the true fix.
> "started" should not be ignored.

Yes, I agree that it would be nicer if we init it rather than ignore it,
but I wanted to avoid trying to do an extra allocation on this path since
it usually happens when the system oopsed, so allocations might be reliable
and we want to get the ftrace buffer out as reliably as we can.


Thanks,
Sasha

Re: [PATCH] ftrace: allow dumping traces without tracking trace started cpus

[Steven Rostedt]

On Tue, 08 Sep 2015 10:17:15 -0400
Sasha Levin <sasha.levin@oracle.com> wrote:


> Yes, I agree that it would be nicer if we init it rather than ignore it,
> but I wanted to avoid trying to do an extra allocation on this path since
> it usually happens when the system oopsed, so allocations might be reliable
> and we want to get the ftrace buffer out as reliably as we can.

Fair enough. I'll keep the patch as is then.

-- Steve