* [PATCH] tracing: Have synthetic event test use raw_smp_processor_id()
@ 2020-02-20 21:29 Steven Rostedt
2020-02-20 22:14 ` Tom Zanussi
0 siblings, 1 reply; 5+ messages in thread
From: Steven Rostedt @ 2020-02-20 21:29 UTC (permalink / raw)
To: LKML; +Cc: Tom Zanussi, Masami Hiramatsu
From: "Steven Rostedt (VMware)" <rostedt@goodmis.org>
The test code that tests synthetic event creation pushes in as one of its
test fields the current CPU using "smp_processor_id()". As this is just
something to see if the value is correctly passed in, and the actual CPU
used does not matter, use raw_smp_processor_id(), otherwise with debug
preemption enabled, a warning happens as the smp_processor_id() is called
without preemption enabled.
Signed-off-by: Steven Rostedt (VMware) <rostedt@goodmis.org>
---
kernel/trace/synth_event_gen_test.c | 12 ++++++------
1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/kernel/trace/synth_event_gen_test.c b/kernel/trace/synth_event_gen_test.c
index 6866280a9b10..7d56d621ffea 100644
--- a/kernel/trace/synth_event_gen_test.c
+++ b/kernel/trace/synth_event_gen_test.c
@@ -114,7 +114,7 @@ static int __init test_gen_synth_cmd(void)
vals[1] = (u64)(long)"hula hoops"; /* next_comm_field */
vals[2] = 1000000; /* ts_ns */
vals[3] = 1000; /* ts_ms */
- vals[4] = smp_processor_id(); /* cpu */
+ vals[4] = raw_smp_processor_id(); /* cpu */
vals[5] = (u64)(long)"thneed"; /* my_string_field */
vals[6] = 598; /* my_int_field */
@@ -221,7 +221,7 @@ static int __init test_empty_synth_event(void)
vals[1] = (u64)(long)"tiddlywinks"; /* next_comm_field */
vals[2] = 1000000; /* ts_ns */
vals[3] = 1000; /* ts_ms */
- vals[4] = smp_processor_id(); /* cpu */
+ vals[4] = raw_smp_processor_id(); /* cpu */
vals[5] = (u64)(long)"thneed_2.0"; /* my_string_field */
vals[6] = 399; /* my_int_field */
@@ -293,7 +293,7 @@ static int __init test_create_synth_event(void)
vals[1] = (u64)(long)"tiddlywinks"; /* next_comm_field */
vals[2] = 1000000; /* ts_ns */
vals[3] = 1000; /* ts_ms */
- vals[4] = smp_processor_id(); /* cpu */
+ vals[4] = raw_smp_processor_id(); /* cpu */
vals[5] = (u64)(long)"thneed"; /* my_string_field */
vals[6] = 398; /* my_int_field */
@@ -345,7 +345,7 @@ static int __init test_add_next_synth_val(void)
goto out;
/* cpu */
- ret = synth_event_add_next_val(smp_processor_id(), &trace_state);
+ ret = synth_event_add_next_val(raw_smp_processor_id(), &trace_state);
if (ret)
goto out;
@@ -388,7 +388,7 @@ static int __init test_add_synth_val(void)
if (ret)
goto out;
- ret = synth_event_add_val("cpu", smp_processor_id(), &trace_state);
+ ret = synth_event_add_val("cpu", raw_smp_processor_id(), &trace_state);
if (ret)
goto out;
@@ -427,7 +427,7 @@ static int __init test_trace_synth_event(void)
(u64)(long)"clackers", /* next_comm_field */
(u64)1000000, /* ts_ns */
(u64)1000, /* ts_ms */
- (u64)smp_processor_id(),/* cpu */
+ (u64)raw_smp_processor_id(), /* cpu */
(u64)(long)"Thneed", /* my_string_field */
(u64)999); /* my_int_field */
return ret;
--
2.20.1
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH] tracing: Have synthetic event test use raw_smp_processor_id()
2020-02-20 21:29 [PATCH] tracing: Have synthetic event test use raw_smp_processor_id() Steven Rostedt
@ 2020-02-20 22:14 ` Tom Zanussi
2020-02-21 0:22 ` Masami Hiramatsu
0 siblings, 1 reply; 5+ messages in thread
From: Tom Zanussi @ 2020-02-20 22:14 UTC (permalink / raw)
To: Steven Rostedt, LKML; +Cc: Masami Hiramatsu
Hi Steve,
On Thu, 2020-02-20 at 16:29 -0500, Steven Rostedt wrote:
> From: "Steven Rostedt (VMware)" <rostedt@goodmis.org>
>
> The test code that tests synthetic event creation pushes in as one of
> its
> test fields the current CPU using "smp_processor_id()". As this is
> just
> something to see if the value is correctly passed in, and the actual
> CPU
> used does not matter, use raw_smp_processor_id(), otherwise with
> debug
> preemption enabled, a warning happens as the smp_processor_id() is
> called
> without preemption enabled.
>
> Signed-off-by: Steven Rostedt (VMware) <rostedt@goodmis.org>
Makes sense - I guess it's simpler than Masami's and fine for this
purpose.
Reviewed-by: Tom Zanussi <zanussi@kernel.org>
Thanks,
Tom
> ---
> kernel/trace/synth_event_gen_test.c | 12 ++++++------
> 1 file changed, 6 insertions(+), 6 deletions(-)
>
> diff --git a/kernel/trace/synth_event_gen_test.c
> b/kernel/trace/synth_event_gen_test.c
> index 6866280a9b10..7d56d621ffea 100644
> --- a/kernel/trace/synth_event_gen_test.c
> +++ b/kernel/trace/synth_event_gen_test.c
> @@ -114,7 +114,7 @@ static int __init test_gen_synth_cmd(void)
> vals[1] = (u64)(long)"hula hoops"; /* next_comm_field
> */
> vals[2] = 1000000; /* ts_ns */
> vals[3] = 1000; /* ts_ms */
> - vals[4] = smp_processor_id(); /* cpu */
> + vals[4] = raw_smp_processor_id(); /* cpu */
> vals[5] = (u64)(long)"thneed"; /* my_string_field */
> vals[6] = 598; /* my_int_field */
>
> @@ -221,7 +221,7 @@ static int __init test_empty_synth_event(void)
> vals[1] = (u64)(long)"tiddlywinks"; /*
> next_comm_field */
> vals[2] = 1000000; /* ts_ns */
> vals[3] = 1000; /* ts_ms */
> - vals[4] = smp_processor_id(); /* cpu */
> + vals[4] = raw_smp_processor_id(); /* cpu */
> vals[5] = (u64)(long)"thneed_2.0"; /* my_string_field
> */
> vals[6] = 399; /* my_int_field */
>
> @@ -293,7 +293,7 @@ static int __init test_create_synth_event(void)
> vals[1] = (u64)(long)"tiddlywinks"; /*
> next_comm_field */
> vals[2] = 1000000; /* ts_ns */
> vals[3] = 1000; /* ts_ms */
> - vals[4] = smp_processor_id(); /* cpu */
> + vals[4] = raw_smp_processor_id(); /* cpu */
> vals[5] = (u64)(long)"thneed"; /* my_string_field */
> vals[6] = 398; /* my_int_field */
>
> @@ -345,7 +345,7 @@ static int __init test_add_next_synth_val(void)
> goto out;
>
> /* cpu */
> - ret = synth_event_add_next_val(smp_processor_id(),
> &trace_state);
> + ret = synth_event_add_next_val(raw_smp_processor_id(),
> &trace_state);
> if (ret)
> goto out;
>
> @@ -388,7 +388,7 @@ static int __init test_add_synth_val(void)
> if (ret)
> goto out;
>
> - ret = synth_event_add_val("cpu", smp_processor_id(),
> &trace_state);
> + ret = synth_event_add_val("cpu", raw_smp_processor_id(),
> &trace_state);
> if (ret)
> goto out;
>
> @@ -427,7 +427,7 @@ static int __init test_trace_synth_event(void)
> (u64)(long)"clackers", /*
> next_comm_field */
> (u64)1000000, /*
> ts_ns */
> (u64)1000, /* ts_ms
> */
> - (u64)smp_processor_id(),/* cpu */
> + (u64)raw_smp_processor_id(), /* cpu
> */
> (u64)(long)"Thneed", /*
> my_string_field */
> (u64)999); /*
> my_int_field */
> return ret;
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] tracing: Have synthetic event test use raw_smp_processor_id()
2020-02-20 22:14 ` Tom Zanussi
@ 2020-02-21 0:22 ` Masami Hiramatsu
2020-02-21 2:52 ` Steven Rostedt
0 siblings, 1 reply; 5+ messages in thread
From: Masami Hiramatsu @ 2020-02-21 0:22 UTC (permalink / raw)
To: Tom Zanussi; +Cc: Steven Rostedt, LKML, Masami Hiramatsu
On Thu, 20 Feb 2020 16:14:40 -0600
Tom Zanussi <zanussi@kernel.org> wrote:
> Hi Steve,
>
> On Thu, 2020-02-20 at 16:29 -0500, Steven Rostedt wrote:
> > From: "Steven Rostedt (VMware)" <rostedt@goodmis.org>
> >
> > The test code that tests synthetic event creation pushes in as one of
> > its
> > test fields the current CPU using "smp_processor_id()". As this is
> > just
> > something to see if the value is correctly passed in, and the actual
> > CPU
> > used does not matter, use raw_smp_processor_id(), otherwise with
> > debug
> > preemption enabled, a warning happens as the smp_processor_id() is
> > called
> > without preemption enabled.
> >
> > Signed-off-by: Steven Rostedt (VMware) <rostedt@goodmis.org>
>
> Makes sense - I guess it's simpler than Masami's and fine for this
> purpose.
>
> Reviewed-by: Tom Zanussi <zanussi@kernel.org>
Hmm, can we reserve ring buffer on CPU1 and commit it on CPU2?
Shouldn't we disable preemption between them?
Thank you,
--
Masami Hiramatsu <mhiramat@kernel.org>
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] tracing: Have synthetic event test use raw_smp_processor_id()
2020-02-21 0:22 ` Masami Hiramatsu
@ 2020-02-21 2:52 ` Steven Rostedt
2020-02-21 10:22 ` Masami Hiramatsu
0 siblings, 1 reply; 5+ messages in thread
From: Steven Rostedt @ 2020-02-21 2:52 UTC (permalink / raw)
To: Masami Hiramatsu; +Cc: Tom Zanussi, LKML
On Fri, 21 Feb 2020 09:22:30 +0900
Masami Hiramatsu <mhiramat@kernel.org> wrote:
> On Thu, 20 Feb 2020 16:14:40 -0600
> Tom Zanussi <zanussi@kernel.org> wrote:
>
> > Hi Steve,
> >
> > On Thu, 2020-02-20 at 16:29 -0500, Steven Rostedt wrote:
> > > From: "Steven Rostedt (VMware)" <rostedt@goodmis.org>
> > >
> > > The test code that tests synthetic event creation pushes in as one of
> > > its
> > > test fields the current CPU using "smp_processor_id()". As this is
> > > just
> > > something to see if the value is correctly passed in, and the actual
> > > CPU
> > > used does not matter, use raw_smp_processor_id(), otherwise with
> > > debug
> > > preemption enabled, a warning happens as the smp_processor_id() is
> > > called
> > > without preemption enabled.
> > >
> > > Signed-off-by: Steven Rostedt (VMware) <rostedt@goodmis.org>
> >
> > Makes sense - I guess it's simpler than Masami's and fine for this
> > purpose.
> >
> > Reviewed-by: Tom Zanussi <zanussi@kernel.org>
>
> Hmm, can we reserve ring buffer on CPU1 and commit it on CPU2?
> Shouldn't we disable preemption between them?
>
I don't think it matters. There's nothing that checks it. And if one
was concerned about the content of the CPU, one could always enable
scheduling events and see the task migrate.
-- Steve
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] tracing: Have synthetic event test use raw_smp_processor_id()
2020-02-21 2:52 ` Steven Rostedt
@ 2020-02-21 10:22 ` Masami Hiramatsu
0 siblings, 0 replies; 5+ messages in thread
From: Masami Hiramatsu @ 2020-02-21 10:22 UTC (permalink / raw)
To: Steven Rostedt; +Cc: Tom Zanussi, LKML
On Thu, 20 Feb 2020 21:52:36 -0500
Steven Rostedt <rostedt@goodmis.org> wrote:
> On Fri, 21 Feb 2020 09:22:30 +0900
> Masami Hiramatsu <mhiramat@kernel.org> wrote:
>
> > On Thu, 20 Feb 2020 16:14:40 -0600
> > Tom Zanussi <zanussi@kernel.org> wrote:
> >
> > > Hi Steve,
> > >
> > > On Thu, 2020-02-20 at 16:29 -0500, Steven Rostedt wrote:
> > > > From: "Steven Rostedt (VMware)" <rostedt@goodmis.org>
> > > >
> > > > The test code that tests synthetic event creation pushes in as one of
> > > > its
> > > > test fields the current CPU using "smp_processor_id()". As this is
> > > > just
> > > > something to see if the value is correctly passed in, and the actual
> > > > CPU
> > > > used does not matter, use raw_smp_processor_id(), otherwise with
> > > > debug
> > > > preemption enabled, a warning happens as the smp_processor_id() is
> > > > called
> > > > without preemption enabled.
> > > >
> > > > Signed-off-by: Steven Rostedt (VMware) <rostedt@goodmis.org>
> > >
> > > Makes sense - I guess it's simpler than Masami's and fine for this
> > > purpose.
> > >
> > > Reviewed-by: Tom Zanussi <zanussi@kernel.org>
> >
> > Hmm, can we reserve ring buffer on CPU1 and commit it on CPU2?
> > Shouldn't we disable preemption between them?
> >
>
> I don't think it matters. There's nothing that checks it. And if one
> was concerned about the content of the CPU, one could always enable
> scheduling events and see the task migrate.
Ah, OK I confirmed that the preemption is disabled in ring_buffer_nest_start()
so it should not happen.
Reviewed-by: Masami Hiramatsu <mhiramat@kernel.org>
Thank you!
>
> -- Steve
>
--
Masami Hiramatsu <mhiramat@kernel.org>
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2020-02-21 10:22 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2020-02-20 21:29 [PATCH] tracing: Have synthetic event test use raw_smp_processor_id() Steven Rostedt
2020-02-20 22:14 ` Tom Zanussi
2020-02-21 0:22 ` Masami Hiramatsu
2020-02-21 2:52 ` Steven Rostedt
2020-02-21 10:22 ` Masami Hiramatsu
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.