* [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