Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1219165 > unrolled thread

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

Back to article view | Back to linux.kernel

Contents

  [PATCH] ftrace: allow dumping traces without tracking trace started cpus Sasha Levin <sasha.levin@oracle.com> - 2015-09-04 18:50 +0200
    Re: [PATCH] ftrace: allow dumping traces without tracking trace  started cpus Minfei Huang <mhuang@redhat.com> - 2015-09-06 11:20 +0200
      Re: [PATCH] ftrace: allow dumping traces without tracking trace started  cpus Sasha Levin <sasha.levin@oracle.com> - 2015-09-06 16:40 +0200
        Re: [PATCH] ftrace: allow dumping traces without tracking trace  started cpus Steven Rostedt <rostedt@goodmis.org> - 2015-09-08 16:20 +0200
          Re: [PATCH] ftrace: allow dumping traces without tracking trace started  cpus Sasha Levin <sasha.levin@oracle.com> - 2015-09-08 16:20 +0200
            Re: [PATCH] ftrace: allow dumping traces without tracking trace  started cpus Steven Rostedt <rostedt@goodmis.org> - 2015-09-08 17:40 +0200

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

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

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [next] | [standalone]

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

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
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

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

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
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

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

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

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

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

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

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

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

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
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web