* [PATCH 0/2] tracing: Fix synthetic event generation API and test
@ 2020-02-17 9:52 Masami Hiramatsu
2020-02-17 9:52 ` [PATCH 1/2] tracing: Fix synth event test to avoid using smp_processor_id() Masami Hiramatsu
` (2 more replies)
0 siblings, 3 replies; 7+ messages in thread
From: Masami Hiramatsu @ 2020-02-17 9:52 UTC (permalink / raw)
To: Steven Rostedt
Cc: Masami Hiramatsu, Tom Zanussi, artem.bityutskiy, linux-kernel,
linux-rt-users
Hi,
Here is a couple of patches to fix 2 issues LKP and I found on
synthetic event generation test.
[1/2] is for fixing warnings on smp_processor_id() without
disabling preemption, and [2/2] is for fixing a bug on the API
itself which LKP reported.
Thank you,
---
Masami Hiramatsu (2):
tracing: Fix synth event test to avoid using smp_processor_id()
tracing: Clear trace_state when starting trace
kernel/trace/synth_event_gen_test.c | 23 +++++++++++++++++------
kernel/trace/trace_events_hist.c | 4 ++--
2 files changed, 19 insertions(+), 8 deletions(-)
--
Masami Hiramatsu (Linaro) <mhiramat@kernel.org>
^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH 1/2] tracing: Fix synth event test to avoid using smp_processor_id()
2020-02-17 9:52 [PATCH 0/2] tracing: Fix synthetic event generation API and test Masami Hiramatsu
@ 2020-02-17 9:52 ` Masami Hiramatsu
2020-02-20 22:48 ` Steven Rostedt
2020-02-17 9:52 ` [PATCH 2/2] tracing: Clear trace_state when starting trace Masami Hiramatsu
2020-02-18 15:18 ` [PATCH 0/2] tracing: Fix synthetic event generation API and test Tom Zanussi
2 siblings, 1 reply; 7+ messages in thread
From: Masami Hiramatsu @ 2020-02-17 9:52 UTC (permalink / raw)
To: Steven Rostedt
Cc: Masami Hiramatsu, Tom Zanussi, artem.bityutskiy, linux-kernel,
linux-rt-users
Since smp_processor_id() requires irq-disabled or preempt-disabled,
synth event generation test module made some warnings. To prevent
that, use get_cpu()/put_cpu() instead of smp_processor_id().
Signed-off-by: Masami Hiramatsu <mhiramat@kernel.org>
---
kernel/trace/synth_event_gen_test.c | 23 +++++++++++++++++------
1 file changed, 17 insertions(+), 6 deletions(-)
diff --git a/kernel/trace/synth_event_gen_test.c b/kernel/trace/synth_event_gen_test.c
index 4aefe003cb7c..b7775fd6baf5 100644
--- a/kernel/trace/synth_event_gen_test.c
+++ b/kernel/trace/synth_event_gen_test.c
@@ -114,12 +114,13 @@ static int __init test_gen_synth_cmd(void)
vals[1] = (u64)"hula hoops"; /* next_comm_field */
vals[2] = 1000000; /* ts_ns */
vals[3] = 1000; /* ts_ms */
- vals[4] = smp_processor_id(); /* cpu */
+ vals[4] = get_cpu(); /* cpu */
vals[5] = (u64)"thneed"; /* my_string_field */
vals[6] = 598; /* my_int_field */
/* Now generate a gen_synth_test event */
ret = synth_event_trace_array(gen_synth_test, vals, ARRAY_SIZE(vals));
+ put_cpu();
out:
return ret;
delete:
@@ -221,12 +222,13 @@ static int __init test_empty_synth_event(void)
vals[1] = (u64)"tiddlywinks"; /* next_comm_field */
vals[2] = 1000000; /* ts_ns */
vals[3] = 1000; /* ts_ms */
- vals[4] = smp_processor_id(); /* cpu */
+ vals[4] = get_cpu(); /* cpu */
vals[5] = (u64)"thneed_2.0"; /* my_string_field */
vals[6] = 399; /* my_int_field */
/* Now trace an empty_synth_test event */
ret = synth_event_trace_array(empty_synth_test, vals, ARRAY_SIZE(vals));
+ put_cpu();
out:
return ret;
delete:
@@ -293,12 +295,13 @@ static int __init test_create_synth_event(void)
vals[1] = (u64)"tiddlywinks"; /* next_comm_field */
vals[2] = 1000000; /* ts_ns */
vals[3] = 1000; /* ts_ms */
- vals[4] = smp_processor_id(); /* cpu */
+ vals[4] = get_cpu(); /* cpu */
vals[5] = (u64)"thneed"; /* my_string_field */
vals[6] = 398; /* my_int_field */
/* Now generate a create_synth_test event */
ret = synth_event_trace_array(create_synth_test, vals, ARRAY_SIZE(vals));
+ put_cpu();
out:
return ret;
delete:
@@ -315,6 +318,7 @@ static int __init test_create_synth_event(void)
static int __init test_add_next_synth_val(void)
{
struct synth_event_trace_state trace_state;
+ unsigned int cpu;
int ret;
/* Start by reserving space in the trace buffer */
@@ -322,6 +326,8 @@ static int __init test_add_next_synth_val(void)
if (ret)
return ret;
+ cpu = get_cpu();
+
/* Write some bogus values into the trace buffer, one after another */
/* next_pid_field */
@@ -345,7 +351,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(cpu, &trace_state);
if (ret)
goto out;
@@ -357,6 +363,7 @@ static int __init test_add_next_synth_val(void)
/* my_int_field */
ret = synth_event_add_next_val(395, &trace_state);
out:
+ put_cpu();
/* Finally, commit the event */
ret = synth_event_trace_end(&trace_state);
@@ -371,6 +378,7 @@ static int __init test_add_next_synth_val(void)
static int __init test_add_synth_val(void)
{
struct synth_event_trace_state trace_state;
+ unsigned int cpu;
int ret;
/* Start by reserving space in the trace buffer */
@@ -378,6 +386,7 @@ static int __init test_add_synth_val(void)
if (ret)
return ret;
+ cpu = get_cpu();
/* Write some bogus values into the trace buffer, using field names */
ret = synth_event_add_val("ts_ns", 1000000, &trace_state);
@@ -388,7 +397,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", cpu, &trace_state);
if (ret)
goto out;
@@ -408,6 +417,7 @@ static int __init test_add_synth_val(void)
ret = synth_event_add_val("my_int_field", 3999, &trace_state);
out:
+ put_cpu();
/* Finally, commit the event */
ret = synth_event_trace_end(&trace_state);
@@ -427,9 +437,10 @@ static int __init test_trace_synth_event(void)
(u64)"clackers", /* next_comm_field */
1000000, /* ts_ns */
1000, /* ts_ms */
- smp_processor_id(), /* cpu */
+ get_cpu(), /* cpu */
(u64)"Thneed", /* my_string_field */
999); /* my_int_field */
+ put_cpu();
return ret;
}
^ permalink raw reply related [flat|nested] 7+ messages in thread
* [PATCH 2/2] tracing: Clear trace_state when starting trace
2020-02-17 9:52 [PATCH 0/2] tracing: Fix synthetic event generation API and test Masami Hiramatsu
2020-02-17 9:52 ` [PATCH 1/2] tracing: Fix synth event test to avoid using smp_processor_id() Masami Hiramatsu
@ 2020-02-17 9:52 ` Masami Hiramatsu
2020-02-18 15:18 ` [PATCH 0/2] tracing: Fix synthetic event generation API and test Tom Zanussi
2 siblings, 0 replies; 7+ messages in thread
From: Masami Hiramatsu @ 2020-02-17 9:52 UTC (permalink / raw)
To: Steven Rostedt
Cc: Masami Hiramatsu, Tom Zanussi, artem.bityutskiy, linux-kernel,
linux-rt-users
Clear trace_state data structure when starting trace
in __synth_event_trace_start() internal function.
Currently trace_state is initialized only in the
synth_event_trace_start() API, but the trace_state
in synth_event_trace() and synth_event_trace_array()
are on the stack without initialization.
This means those APIs will see wrong parameters and
wil skip closing process in __synth_event_trace_end()
because trace_state->disabled may be !0.
Signed-off-by: Masami Hiramatsu <mhiramat@kernel.org>
---
kernel/trace/trace_events_hist.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/kernel/trace/trace_events_hist.c b/kernel/trace/trace_events_hist.c
index 7f0e5cdf17ae..9ec0a7551d62 100644
--- a/kernel/trace/trace_events_hist.c
+++ b/kernel/trace/trace_events_hist.c
@@ -1798,6 +1798,8 @@ __synth_event_trace_start(struct trace_event_file *file,
int entry_size, fields_size = 0;
int ret = 0;
+ memset(trace_state, '\0', sizeof(*trace_state));
+
/*
* Normal event tracing doesn't get called at all unless the
* ENABLED bit is set (which attaches the probe thus allowing
@@ -1993,8 +1995,6 @@ int synth_event_trace_start(struct trace_event_file *file,
if (!trace_state)
return -EINVAL;
- memset(trace_state, '\0', sizeof(*trace_state));
-
ret = __synth_event_trace_start(file, trace_state);
if (ret == -ENOENT)
ret = 0; /* just disabled, not really an error */
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH 0/2] tracing: Fix synthetic event generation API and test
2020-02-17 9:52 [PATCH 0/2] tracing: Fix synthetic event generation API and test Masami Hiramatsu
2020-02-17 9:52 ` [PATCH 1/2] tracing: Fix synth event test to avoid using smp_processor_id() Masami Hiramatsu
2020-02-17 9:52 ` [PATCH 2/2] tracing: Clear trace_state when starting trace Masami Hiramatsu
@ 2020-02-18 15:18 ` Tom Zanussi
2 siblings, 0 replies; 7+ messages in thread
From: Tom Zanussi @ 2020-02-18 15:18 UTC (permalink / raw)
To: Masami Hiramatsu, Steven Rostedt
Cc: artem.bityutskiy, linux-kernel, linux-rt-users
Hi Masami,
On Mon, 2020-02-17 at 18:52 +0900, Masami Hiramatsu wrote:
> Hi,
>
> Here is a couple of patches to fix 2 issues LKP and I found on
> synthetic event generation test.
>
> [1/2] is for fixing warnings on smp_processor_id() without
> disabling preemption, and [2/2] is for fixing a bug on the API
> itself which LKP reported.
>
> Thank you,
Thank you for fixing these.
For both,
Reviewed-by: Tom Zanussi <zanussi@kernel.org>
Tom
>
> ---
>
> Masami Hiramatsu (2):
> tracing: Fix synth event test to avoid using smp_processor_id()
> tracing: Clear trace_state when starting trace
>
>
> kernel/trace/synth_event_gen_test.c | 23 +++++++++++++++++------
> kernel/trace/trace_events_hist.c | 4 ++--
> 2 files changed, 19 insertions(+), 8 deletions(-)
>
> --
> Masami Hiramatsu (Linaro) <mhiramat@kernel.org>
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 1/2] tracing: Fix synth event test to avoid using smp_processor_id()
2020-02-17 9:52 ` [PATCH 1/2] tracing: Fix synth event test to avoid using smp_processor_id() Masami Hiramatsu
@ 2020-02-20 22:48 ` Steven Rostedt
2020-02-20 22:56 ` Tom Zanussi
2020-02-21 10:24 ` Masami Hiramatsu
0 siblings, 2 replies; 7+ messages in thread
From: Steven Rostedt @ 2020-02-20 22:48 UTC (permalink / raw)
To: Masami Hiramatsu
Cc: Tom Zanussi, artem.bityutskiy, linux-kernel, linux-rt-users
On Mon, 17 Feb 2020 18:52:29 +0900
Masami Hiramatsu <mhiramat@kernel.org> wrote:
> Since smp_processor_id() requires irq-disabled or preempt-disabled,
> synth event generation test module made some warnings. To prevent
> that, use get_cpu()/put_cpu() instead of smp_processor_id().
>
> Signed-off-by: Masami Hiramatsu <mhiramat@kernel.org>
> ---
> kernel/trace/synth_event_gen_test.c | 23 +++++++++++++++++------
> 1 file changed, 17 insertions(+), 6 deletions(-)
>
I just noticed this patch, after applying my version that just uses the
raw_smp_processor_id(). We don't really care what CPU it is do we?
I didn't want a test to muck with preemption disabling and all that fun.
-- Steve
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 1/2] tracing: Fix synth event test to avoid using smp_processor_id()
2020-02-20 22:48 ` Steven Rostedt
@ 2020-02-20 22:56 ` Tom Zanussi
2020-02-21 10:24 ` Masami Hiramatsu
1 sibling, 0 replies; 7+ messages in thread
From: Tom Zanussi @ 2020-02-20 22:56 UTC (permalink / raw)
To: Steven Rostedt, Masami Hiramatsu
Cc: artem.bityutskiy, linux-kernel, linux-rt-users
Hi Steve,
On Thu, 2020-02-20 at 17:48 -0500, Steven Rostedt wrote:
> On Mon, 17 Feb 2020 18:52:29 +0900
> Masami Hiramatsu <mhiramat@kernel.org> wrote:
>
> > Since smp_processor_id() requires irq-disabled or preempt-disabled,
> > synth event generation test module made some warnings. To prevent
> > that, use get_cpu()/put_cpu() instead of smp_processor_id().
> >
> > Signed-off-by: Masami Hiramatsu <mhiramat@kernel.org>
> > ---
> > kernel/trace/synth_event_gen_test.c | 23 +++++++++++++++++------
> > 1 file changed, 17 insertions(+), 6 deletions(-)
> >
>
> I just noticed this patch, after applying my version that just uses
> the
> raw_smp_processor_id(). We don't really care what CPU it is do we?
>
> I didn't want a test to muck with preemption disabling and all that
> fun.
>
Right, we don't really care, it's just to test the trace API - for this
it could even be a constant. I just happened to pick that as an
example, and wasn't expecting unrelated complications.
Tom
> -- Steve
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 1/2] tracing: Fix synth event test to avoid using smp_processor_id()
2020-02-20 22:48 ` Steven Rostedt
2020-02-20 22:56 ` Tom Zanussi
@ 2020-02-21 10:24 ` Masami Hiramatsu
1 sibling, 0 replies; 7+ messages in thread
From: Masami Hiramatsu @ 2020-02-21 10:24 UTC (permalink / raw)
To: Steven Rostedt
Cc: Tom Zanussi, artem.bityutskiy, linux-kernel, linux-rt-users
On Thu, 20 Feb 2020 17:48:01 -0500
Steven Rostedt <rostedt@goodmis.org> wrote:
> On Mon, 17 Feb 2020 18:52:29 +0900
> Masami Hiramatsu <mhiramat@kernel.org> wrote:
>
> > Since smp_processor_id() requires irq-disabled or preempt-disabled,
> > synth event generation test module made some warnings. To prevent
> > that, use get_cpu()/put_cpu() instead of smp_processor_id().
> >
> > Signed-off-by: Masami Hiramatsu <mhiramat@kernel.org>
> > ---
> > kernel/trace/synth_event_gen_test.c | 23 +++++++++++++++++------
> > 1 file changed, 17 insertions(+), 6 deletions(-)
> >
>
> I just noticed this patch, after applying my version that just uses the
> raw_smp_processor_id(). We don't really care what CPU it is do we?
>
> I didn't want a test to muck with preemption disabling and all that fun.
OK, I confirmed that the ring_buffer_nest_start() ensures the preempt
disabled. So just using raw_smp_processor_id() is good to me.
Thank you,
--
Masami Hiramatsu <mhiramat@kernel.org>
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2020-02-21 10:24 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-02-17 9:52 [PATCH 0/2] tracing: Fix synthetic event generation API and test Masami Hiramatsu
2020-02-17 9:52 ` [PATCH 1/2] tracing: Fix synth event test to avoid using smp_processor_id() Masami Hiramatsu
2020-02-20 22:48 ` Steven Rostedt
2020-02-20 22:56 ` Tom Zanussi
2020-02-21 10:24 ` Masami Hiramatsu
2020-02-17 9:52 ` [PATCH 2/2] tracing: Clear trace_state when starting trace Masami Hiramatsu
2020-02-18 15:18 ` [PATCH 0/2] tracing: Fix synthetic event generation API and test Tom Zanussi
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).