* [for-next][PATCH 01/18] tracing: Fix a potential NULL dereference
2018-04-06 13:00 [for-next][PATCH 00/18] tracing: Last minute updates before pushing to Linus Steven Rostedt
@ 2018-04-06 13:00 ` Steven Rostedt
2018-04-06 13:00 ` [for-next][PATCH 02/18] init: Fix initcall0 name as it is "pure" not "early" Steven Rostedt
` (16 subsequent siblings)
17 siblings, 0 replies; 19+ messages in thread
From: Steven Rostedt @ 2018-04-06 13:00 UTC (permalink / raw)
To: linux-kernel; +Cc: Ingo Molnar, Andrew Morton, Tom Zanussi, Dan Carpenter
[-- Attachment #1: 0001-tracing-Fix-a-potential-NULL-dereference.patch --]
[-- Type: text/plain, Size: 966 bytes --]
From: Dan Carpenter <dan.carpenter@oracle.com>
We forgot to set the error code on this path so we return ERR_PTR(0)
which is NULL. It results in a NULL dereference in the caller.
Link: http://lkml.kernel.org/r/20180323113735.GC28518@mwanda
Fixes: 100719dcef44 ("tracing: Add simple expression support to hist triggers")
Acked-by: Tom Zanussi <tom.zanussi@linux.intel.com>
Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
Signed-off-by: Steven Rostedt (VMware) <rostedt@goodmis.org>
---
kernel/trace/trace_events_hist.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/kernel/trace/trace_events_hist.c b/kernel/trace/trace_events_hist.c
index 4f027642ceef..a02bc09d765a 100644
--- a/kernel/trace/trace_events_hist.c
+++ b/kernel/trace/trace_events_hist.c
@@ -2776,6 +2776,7 @@ static struct hist_field *parse_expr(struct hist_trigger_data *hist_data,
expr->fn = hist_field_plus;
break;
default:
+ ret = -EINVAL;
goto free;
}
--
2.15.1
^ permalink raw reply related [flat|nested] 19+ messages in thread* [for-next][PATCH 02/18] init: Fix initcall0 name as it is "pure" not "early"
2018-04-06 13:00 [for-next][PATCH 00/18] tracing: Last minute updates before pushing to Linus Steven Rostedt
2018-04-06 13:00 ` [for-next][PATCH 01/18] tracing: Fix a potential NULL dereference Steven Rostedt
@ 2018-04-06 13:00 ` Steven Rostedt
2018-04-06 13:00 ` [for-next][PATCH 03/18] tracing: Default to using trace_global_clock if sched_clock is unstable Steven Rostedt
` (15 subsequent siblings)
17 siblings, 0 replies; 19+ messages in thread
From: Steven Rostedt @ 2018-04-06 13:00 UTC (permalink / raw)
To: linux-kernel; +Cc: Ingo Molnar, Andrew Morton
[-- Attachment #1: 0002-init-Fix-initcall0-name-as-it-is-pure-not-early.patch --]
[-- Type: text/plain, Size: 1006 bytes --]
From: "Steven Rostedt (VMware)" <rostedt@goodmis.org>
The early_initcall() functions get assigned to __initcall_start[]. These are
called by do_pre_smp_initcalls(). The initcall_levels[] array starts with
__initcall0_start[], and initcall_levels[] are to match the
initcall_level_names[] array. The first name in that array is "early", but
that is not correct. As pure_initcall() functions get assigned to
__initcall0_start[] array.
Change the first name in initcall_level_names[] array to "pure".
Signed-off-by: Steven Rostedt (VMware) <rostedt@goodmis.org>
---
init/main.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/init/main.c b/init/main.c
index 969eaf140ef0..0ebdd5f15be8 100644
--- a/init/main.c
+++ b/init/main.c
@@ -874,7 +874,7 @@ static initcall_t *initcall_levels[] __initdata = {
/* Keep these in sync with initcalls in include/linux/init.h */
static char *initcall_level_names[] __initdata = {
- "early",
+ "pure",
"core",
"postcore",
"arch",
--
2.15.1
^ permalink raw reply related [flat|nested] 19+ messages in thread* [for-next][PATCH 03/18] tracing: Default to using trace_global_clock if sched_clock is unstable
2018-04-06 13:00 [for-next][PATCH 00/18] tracing: Last minute updates before pushing to Linus Steven Rostedt
2018-04-06 13:00 ` [for-next][PATCH 01/18] tracing: Fix a potential NULL dereference Steven Rostedt
2018-04-06 13:00 ` [for-next][PATCH 02/18] init: Fix initcall0 name as it is "pure" not "early" Steven Rostedt
@ 2018-04-06 13:00 ` Steven Rostedt
2018-04-06 13:00 ` [for-next][PATCH 04/18] tracing: Mention trace_clock=global when warning about unstable clocks Steven Rostedt
` (14 subsequent siblings)
17 siblings, 0 replies; 19+ messages in thread
From: Steven Rostedt @ 2018-04-06 13:00 UTC (permalink / raw)
To: linux-kernel; +Cc: Ingo Molnar, Andrew Morton, Chris Wilson
[-- Attachment #1: 0003-tracing-Default-to-using-trace_global_clock-if-sched.patch --]
[-- Type: text/plain, Size: 2076 bytes --]
From: Chris Wilson <chris@chris-wilson.co.uk>
Across suspend, we may see a very large drift in timestamps if the sched
clock is unstable, prompting the global trace's ringbuffer code to warn
and suggest switching to the global clock. Preempt this request by
detecting when the sched clock is unstable (determined during
late_initcall) and automatically switching the default clock over to
trace_global_clock.
This should prevent requiring user interaction to resolve warnings such
as:
Delta way too big! 18446743856563626466 ts=18446744054496180323 write stamp = 197932553857
If you just came from a suspend/resume,
please switch to the trace global clock:
echo global > /sys/kernel/debug/tracing/trace_clock
Link: http://lkml.kernel.org/r/20180330150132.16903-1-chris@chris-wilson.co.uk
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Signed-off-by: Steven Rostedt (VMware) <rostedt@goodmis.org>
---
kernel/trace/trace.c | 19 +++++++++++++++++++
1 file changed, 19 insertions(+)
diff --git a/kernel/trace/trace.c b/kernel/trace/trace.c
index 551a7cd0d705..0f47e653ffd8 100644
--- a/kernel/trace/trace.c
+++ b/kernel/trace/trace.c
@@ -41,6 +41,7 @@
#include <linux/nmi.h>
#include <linux/fs.h>
#include <linux/trace.h>
+#include <linux/sched/clock.h>
#include <linux/sched/rt.h>
#include "trace.h"
@@ -8596,3 +8597,21 @@ __init static int clear_boot_tracer(void)
fs_initcall(tracer_init_tracefs);
late_initcall_sync(clear_boot_tracer);
+
+#ifdef CONFIG_HAVE_UNSTABLE_SCHED_CLOCK
+__init static int tracing_set_default_clock(void)
+{
+ /* sched_clock_stable() is determined in late_initcall */
+ if (trace_boot_clock || sched_clock_stable()) {
+ printk(KERN_WARNING
+ "Unstable clock detected, switching default tracing clock to \"global\"\n"
+ "If you want to keep using the local clock, then add:\n"
+ " \"trace_clock=local\"\n"
+ "on the kernel command line\n");
+ tracing_set_clock(&global_trace, "global");
+ }
+
+ return 0;
+}
+late_initcall_sync(tracing_set_default_clock);
+#endif
--
2.15.1
^ permalink raw reply related [flat|nested] 19+ messages in thread* [for-next][PATCH 04/18] tracing: Mention trace_clock=global when warning about unstable clocks
2018-04-06 13:00 [for-next][PATCH 00/18] tracing: Last minute updates before pushing to Linus Steven Rostedt
` (2 preceding siblings ...)
2018-04-06 13:00 ` [for-next][PATCH 03/18] tracing: Default to using trace_global_clock if sched_clock is unstable Steven Rostedt
@ 2018-04-06 13:00 ` Steven Rostedt
2018-04-06 13:00 ` [for-next][PATCH 05/18] ftrace: Drop a VLA in module_exists() Steven Rostedt
` (13 subsequent siblings)
17 siblings, 0 replies; 19+ messages in thread
From: Steven Rostedt @ 2018-04-06 13:00 UTC (permalink / raw)
To: linux-kernel; +Cc: Ingo Molnar, Andrew Morton, Chris Wilson
[-- Attachment #1: 0004-tracing-Mention-trace_clock-global-when-warning-abou.patch --]
[-- Type: text/plain, Size: 1117 bytes --]
From: Chris Wilson <chris@chris-wilson.co.uk>
Mention the alternative of adding trace_clock=global to the kernel
command line when we detect that we've used an unstable clock across a
suspend/resume cycle.
Link: http://lkml.kernel.org/r/20180330150132.16903-2-chris@chris-wilson.co.uk
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Signed-off-by: Steven Rostedt (VMware) <rostedt@goodmis.org>
---
kernel/trace/ring_buffer.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/kernel/trace/ring_buffer.c b/kernel/trace/ring_buffer.c
index a2fd3893cc02..515be03e3009 100644
--- a/kernel/trace/ring_buffer.c
+++ b/kernel/trace/ring_buffer.c
@@ -2731,7 +2731,8 @@ rb_handle_timestamp(struct ring_buffer_per_cpu *cpu_buffer,
sched_clock_stable() ? "" :
"If you just came from a suspend/resume,\n"
"please switch to the trace global clock:\n"
- " echo global > /sys/kernel/debug/tracing/trace_clock\n");
+ " echo global > /sys/kernel/debug/tracing/trace_clock\n"
+ "or add trace_clock=global to the kernel command line\n");
info->add_timestamp = 1;
}
--
2.15.1
^ permalink raw reply related [flat|nested] 19+ messages in thread* [for-next][PATCH 05/18] ftrace: Drop a VLA in module_exists()
2018-04-06 13:00 [for-next][PATCH 00/18] tracing: Last minute updates before pushing to Linus Steven Rostedt
` (3 preceding siblings ...)
2018-04-06 13:00 ` [for-next][PATCH 04/18] tracing: Mention trace_clock=global when warning about unstable clocks Steven Rostedt
@ 2018-04-06 13:00 ` Steven Rostedt
2018-04-06 13:00 ` [for-next][PATCH 06/18] tracing: Fix display of hist trigger expressions containing timestamps Steven Rostedt
` (12 subsequent siblings)
17 siblings, 0 replies; 19+ messages in thread
From: Steven Rostedt @ 2018-04-06 13:00 UTC (permalink / raw)
To: linux-kernel; +Cc: Ingo Molnar, Andrew Morton, Salvatore Mesoraca
[-- Attachment #1: 0005-ftrace-Drop-a-VLA-in-module_exists.patch --]
[-- Type: text/plain, Size: 1535 bytes --]
From: Salvatore Mesoraca <s.mesoraca16@gmail.com>
Avoid a VLA by using a real constant expression instead of a variable.
The compiler should be able to optimize the original code and avoid using
an actual VLA. Anyway this change is useful because it will avoid a false
positive with -Wvla, it might also help the compiler generating better
code.
Link: http://lkml.kernel.org/r/CA+55aFzCG-zNmZwX4A2FQpadafLfEzK6CC=qPXydAacU1RqZWA@mail.gmail.com
Link: http://lkml.kernel.org/r/1522399988-8815-1-git-send-email-s.mesoraca16@gmail.com
Signed-off-by: Salvatore Mesoraca <s.mesoraca16@gmail.com>
Signed-off-by: Steven Rostedt (VMware) <rostedt@goodmis.org>
---
kernel/trace/ftrace.c | 7 +++----
1 file changed, 3 insertions(+), 4 deletions(-)
diff --git a/kernel/trace/ftrace.c b/kernel/trace/ftrace.c
index eac9ce2c57a2..16bbf062018f 100644
--- a/kernel/trace/ftrace.c
+++ b/kernel/trace/ftrace.c
@@ -3902,14 +3902,13 @@ static bool module_exists(const char *module)
{
/* All modules have the symbol __this_module */
const char this_mod[] = "__this_module";
- const int modname_size = MAX_PARAM_PREFIX_LEN + sizeof(this_mod) + 1;
- char modname[modname_size + 1];
+ char modname[MAX_PARAM_PREFIX_LEN + sizeof(this_mod) + 2];
unsigned long val;
int n;
- n = snprintf(modname, modname_size + 1, "%s:%s", module, this_mod);
+ n = snprintf(modname, sizeof(modname), "%s:%s", module, this_mod);
- if (n > modname_size)
+ if (n > sizeof(modname) - 1)
return false;
val = module_kallsyms_lookup_name(modname);
--
2.15.1
^ permalink raw reply related [flat|nested] 19+ messages in thread* [for-next][PATCH 06/18] tracing: Fix display of hist trigger expressions containing timestamps
2018-04-06 13:00 [for-next][PATCH 00/18] tracing: Last minute updates before pushing to Linus Steven Rostedt
` (4 preceding siblings ...)
2018-04-06 13:00 ` [for-next][PATCH 05/18] ftrace: Drop a VLA in module_exists() Steven Rostedt
@ 2018-04-06 13:00 ` Steven Rostedt
2018-04-06 13:00 ` [for-next][PATCH 07/18] tracing: Dont add flag strings when displaying variable references Steven Rostedt
` (11 subsequent siblings)
17 siblings, 0 replies; 19+ messages in thread
From: Steven Rostedt @ 2018-04-06 13:00 UTC (permalink / raw)
To: linux-kernel; +Cc: Ingo Molnar, Andrew Morton, Tom Zanussi
[-- Attachment #1: 0006-tracing-Fix-display-of-hist-trigger-expressions-cont.patch --]
[-- Type: text/plain, Size: 2871 bytes --]
From: Tom Zanussi <tom.zanussi@linux.intel.com>
When displaying hist triggers, variable references that have the
timestamp field flag set are erroneously displayed as common_timestamp
rather than the variable reference. Additionally, timestamp
expressions are displayed in the same way. Fix this by forcing the
timestamp flag handling to follow variable reference and expression
handling.
Before:
# cat /sys/kernel/debug/tracing/events/sched/sched_switch/trigger
hist:keys=next_pid:vals=hitcount:wakeup_lat=common_timestamp.usecs:...
After:
# cat /sys/kernel/debug/tracing/events/sched/sched_switch/trigger
hist:keys=next_pid:vals=hitcount:wakeup_lat=common_timestamp.usecs-$ts0.usecs:...
Link: http://lkml.kernel.org/r/92746b06be67499c2a6217bd55395b350ad18fad.1522256721.git.tom.zanussi@linux.intel.com
Signed-off-by: Tom Zanussi <tom.zanussi@linux.intel.com>
Signed-off-by: Steven Rostedt (VMware) <rostedt@goodmis.org>
---
kernel/trace/trace_events_hist.c | 19 +++++--------------
1 file changed, 5 insertions(+), 14 deletions(-)
diff --git a/kernel/trace/trace_events_hist.c b/kernel/trace/trace_events_hist.c
index a02bc09d765a..4f4792f4c83f 100644
--- a/kernel/trace/trace_events_hist.c
+++ b/kernel/trace/trace_events_hist.c
@@ -1686,8 +1686,6 @@ static const char *hist_field_name(struct hist_field *field,
else if (field->flags & HIST_FIELD_FL_LOG2 ||
field->flags & HIST_FIELD_FL_ALIAS)
field_name = hist_field_name(field->operands[0], ++level);
- else if (field->flags & HIST_FIELD_FL_TIMESTAMP)
- field_name = "common_timestamp";
else if (field->flags & HIST_FIELD_FL_CPU)
field_name = "cpu";
else if (field->flags & HIST_FIELD_FL_EXPR ||
@@ -1703,7 +1701,8 @@ static const char *hist_field_name(struct hist_field *field,
field_name = full_name;
} else
field_name = field->name;
- }
+ } else if (field->flags & HIST_FIELD_FL_TIMESTAMP)
+ field_name = "common_timestamp";
if (field_name == NULL)
field_name = "";
@@ -4858,23 +4857,15 @@ static void hist_field_print(struct seq_file *m, struct hist_field *hist_field)
if (hist_field->var.name)
seq_printf(m, "%s=", hist_field->var.name);
- if (hist_field->flags & HIST_FIELD_FL_TIMESTAMP)
- seq_puts(m, "common_timestamp");
- else if (hist_field->flags & HIST_FIELD_FL_CPU)
+ if (hist_field->flags & HIST_FIELD_FL_CPU)
seq_puts(m, "cpu");
else if (field_name) {
if (hist_field->flags & HIST_FIELD_FL_VAR_REF ||
hist_field->flags & HIST_FIELD_FL_ALIAS)
seq_putc(m, '$');
seq_printf(m, "%s", field_name);
- }
-
- if (hist_field->flags) {
- const char *flags_str = get_hist_field_flags(hist_field);
-
- if (flags_str)
- seq_printf(m, ".%s", flags_str);
- }
+ } else if (hist_field->flags & HIST_FIELD_FL_TIMESTAMP)
+ seq_puts(m, "common_timestamp");
}
static int event_hist_trigger_print(struct seq_file *m,
--
2.15.1
^ permalink raw reply related [flat|nested] 19+ messages in thread* [for-next][PATCH 07/18] tracing: Dont add flag strings when displaying variable references
2018-04-06 13:00 [for-next][PATCH 00/18] tracing: Last minute updates before pushing to Linus Steven Rostedt
` (5 preceding siblings ...)
2018-04-06 13:00 ` [for-next][PATCH 06/18] tracing: Fix display of hist trigger expressions containing timestamps Steven Rostedt
@ 2018-04-06 13:00 ` Steven Rostedt
2018-04-06 13:00 ` [for-next][PATCH 08/18] tracing: Add action comparisons when testing matching hist triggers Steven Rostedt
` (10 subsequent siblings)
17 siblings, 0 replies; 19+ messages in thread
From: Steven Rostedt @ 2018-04-06 13:00 UTC (permalink / raw)
To: linux-kernel; +Cc: Ingo Molnar, Andrew Morton, Tom Zanussi
[-- Attachment #1: 0007-tracing-Don-t-add-flag-strings-when-displaying-varia.patch --]
[-- Type: text/plain, Size: 1234 bytes --]
From: Tom Zanussi <tom.zanussi@linux.intel.com>
Variable references should never have flags appended when displayed -
prevent that from happening.
Before:
# cat /sys/kernel/debug/tracing/events/sched/sched_switch/trigger
hist:keys=next_pid:vals=hitcount:wakeup_lat=common_timestamp.usecs-$ts0.usecs:...
After:
hist:keys=next_pid:vals=hitcount:wakeup_lat=common_timestamp.usecs-$ts0:...
Link: http://lkml.kernel.org/r/913318a5610ef6b24af2522575f671fa6ee19b6b.1522256721.git.tom.zanussi@linux.intel.com
Signed-off-by: Tom Zanussi <tom.zanussi@linux.intel.com>
Signed-off-by: Steven Rostedt (VMware) <rostedt@goodmis.org>
---
kernel/trace/trace_events_hist.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/kernel/trace/trace_events_hist.c b/kernel/trace/trace_events_hist.c
index 4f4792f4c83f..d867502a56ba 100644
--- a/kernel/trace/trace_events_hist.c
+++ b/kernel/trace/trace_events_hist.c
@@ -2052,7 +2052,7 @@ static void expr_field_str(struct hist_field *field, char *expr)
strcat(expr, hist_field_name(field, 0));
- if (field->flags) {
+ if (field->flags && !(field->flags & HIST_FIELD_FL_VAR_REF)) {
const char *flags_str = get_hist_field_flags(field);
if (flags_str) {
--
2.15.1
^ permalink raw reply related [flat|nested] 19+ messages in thread* [for-next][PATCH 08/18] tracing: Add action comparisons when testing matching hist triggers
2018-04-06 13:00 [for-next][PATCH 00/18] tracing: Last minute updates before pushing to Linus Steven Rostedt
` (6 preceding siblings ...)
2018-04-06 13:00 ` [for-next][PATCH 07/18] tracing: Dont add flag strings when displaying variable references Steven Rostedt
@ 2018-04-06 13:00 ` Steven Rostedt
2018-04-06 13:00 ` [for-next][PATCH 09/18] tracing: Make sure variable string fields are NULL-terminated Steven Rostedt
` (9 subsequent siblings)
17 siblings, 0 replies; 19+ messages in thread
From: Steven Rostedt @ 2018-04-06 13:00 UTC (permalink / raw)
To: linux-kernel; +Cc: Ingo Molnar, Andrew Morton, Masami Hiramatsu, Tom Zanussi
[-- Attachment #1: 0008-tracing-Add-action-comparisons-when-testing-matching.patch --]
[-- Type: text/plain, Size: 5038 bytes --]
From: Tom Zanussi <tom.zanussi@linux.intel.com>
Actions also need to be considered when checking for matching triggers
- triggers differing only by action should be allowed, but currently
aren't because the matching check ignores the action and erroneously
returns -EEXIST.
Add and call an actions_match() function to address that.
Here's an example using onmatch() actions. The first -EEXIST shouldn't
occur because the onmatch() is different in the second wakeup_latency()
param. The second -EEXIST shouldn't occur because it's a different
action (in this case, it doesn't have an action, so shouldn't be seen
as being the same and therefore rejected).
In the after case, both are correctly accepted (and trying to add one of
them again returns -EEXIST as it should).
before:
# echo 'wakeup_latency u64 lat; pid_t pid' >> /sys/kernel/debug/tracing/synthetic_events
# echo 'hist:keys=pid:ts0=common_timestamp.usecs if comm=="cyclictest"' >> /sys/kernel/debug/tracing/events/sched/sched_wakeup/trigger
# echo 'hist:keys=next_pid:wakeup_lat=common_timestamp.usecs-$ts0 if next_comm=="cyclictest"' >> /sys/kernel/debug/tracing/events/sched/sched_switch/trigger
# echo 'hist:keys=next_pid:onmatch(sched.sched_wakeup).wakeup_latency(sched.sched_switch.$wakeup_lat,next_pid) if next_comm=="cyclictest"' >> /sys/kernel/debug/tracing/events/sched/sched_switch/trigger
# echo 'hist:keys=next_pid:onmatch(sched.sched_wakeup).wakeup_latency(sched.sched_switch.$wakeup_lat,prev_pid) if next_comm=="cyclictest"' >> /sys/kernel/debug/tracing/events/sched/sched_switch/trigger
-su: echo: write error: File exists
# echo 'hist:keys=next_pid if next_comm=="cyclictest"' >> /sys/kernel/debug/tracing/events/sched/sched_switch/trigger
-su: echo: write error: File exists
after:
# echo 'wakeup_latency u64 lat; pid_t pid' >> /sys/kernel/debug/tracing/synthetic_events
# echo 'hist:keys=pid:ts0=common_timestamp.usecs if comm=="cyclictest"' >> /sys/kernel/debug/tracing/events/sched/sched_wakeup/trigger
# echo 'hist:keys=next_pid:wakeup_lat=common_timestamp.usecs-$ts0 if next_comm=="cyclictest"' >> /sys/kernel/debug/tracing/events/sched/sched_switch/trigger
# echo 'hist:keys=next_pid:onmatch(sched.sched_wakeup).wakeup_latency(sched.sched_switch.$wakeup_lat,next_pid) if next_comm=="cyclictest"' >> /sys/kernel/debug/tracing/events/sched/sched_switch/trigger
# echo 'hist:keys=next_pid:onmatch(sched.sched_wakeup).wakeup_latency(sched.sched_switch.$wakeup_lat,prev_pid) if next_comm=="cyclictest"' >> /sys/kernel/debug/tracing/events/sched/sched_switch/trigger
# echo 'hist:keys=next_pid if next_comm=="cyclictest"' >> /sys/kernel/debug/tracing/events/sched/sched_switch/trigger
Link: http://lkml.kernel.org/r/a7fd668b87ec10736c8f016ac4279c8480d50c2b.1522256721.git.tom.zanussi@linux.intel.com
Tested-by: Masami Hiramatsu <mhiramat@kernel.org>
Signed-off-by: Tom Zanussi <tom.zanussi@linux.intel.com>
Signed-off-by: Steven Rostedt (VMware) <rostedt@goodmis.org>
---
kernel/trace/trace_events_hist.c | 50 ++++++++++++++++++++++++++++++++++++++++
1 file changed, 50 insertions(+)
diff --git a/kernel/trace/trace_events_hist.c b/kernel/trace/trace_events_hist.c
index d867502a56ba..6114939f065a 100644
--- a/kernel/trace/trace_events_hist.c
+++ b/kernel/trace/trace_events_hist.c
@@ -4364,6 +4364,53 @@ static void print_onmatch_spec(struct seq_file *m,
seq_puts(m, ")");
}
+static bool actions_match(struct hist_trigger_data *hist_data,
+ struct hist_trigger_data *hist_data_test)
+{
+ unsigned int i, j;
+
+ if (hist_data->n_actions != hist_data_test->n_actions)
+ return false;
+
+ for (i = 0; i < hist_data->n_actions; i++) {
+ struct action_data *data = hist_data->actions[i];
+ struct action_data *data_test = hist_data_test->actions[i];
+
+ if (data->fn != data_test->fn)
+ return false;
+
+ if (data->n_params != data_test->n_params)
+ return false;
+
+ for (j = 0; j < data->n_params; j++) {
+ if (strcmp(data->params[j], data_test->params[j]) != 0)
+ return false;
+ }
+
+ if (data->fn == action_trace) {
+ if (strcmp(data->onmatch.synth_event_name,
+ data_test->onmatch.synth_event_name) != 0)
+ return false;
+ if (strcmp(data->onmatch.match_event_system,
+ data_test->onmatch.match_event_system) != 0)
+ return false;
+ if (strcmp(data->onmatch.match_event,
+ data_test->onmatch.match_event) != 0)
+ return false;
+ } else if (data->fn == onmax_save) {
+ if (strcmp(data->onmax.var_str,
+ data_test->onmax.var_str) != 0)
+ return false;
+ if (strcmp(data->onmax.fn_name,
+ data_test->onmax.fn_name) != 0)
+ return false;
+ }
+ }
+
+ return true;
+}
+
+
static void print_actions_spec(struct seq_file *m,
struct hist_trigger_data *hist_data)
{
@@ -5174,6 +5221,9 @@ static bool hist_trigger_match(struct event_trigger_data *data,
(strcmp(data->filter_str, data_test->filter_str) != 0))
return false;
+ if (!actions_match(hist_data, hist_data_test))
+ return false;
+
return true;
}
--
2.15.1
^ permalink raw reply related [flat|nested] 19+ messages in thread* [for-next][PATCH 09/18] tracing: Make sure variable string fields are NULL-terminated
2018-04-06 13:00 [for-next][PATCH 00/18] tracing: Last minute updates before pushing to Linus Steven Rostedt
` (7 preceding siblings ...)
2018-04-06 13:00 ` [for-next][PATCH 08/18] tracing: Add action comparisons when testing matching hist triggers Steven Rostedt
@ 2018-04-06 13:00 ` Steven Rostedt
2018-04-06 13:00 ` [for-next][PATCH 10/18] tracing: Uninitialized variable in create_tracing_map_fields() Steven Rostedt
` (8 subsequent siblings)
17 siblings, 0 replies; 19+ messages in thread
From: Steven Rostedt @ 2018-04-06 13:00 UTC (permalink / raw)
To: linux-kernel; +Cc: Ingo Molnar, Andrew Morton, Tom Zanussi
[-- Attachment #1: 0009-tracing-Make-sure-variable-string-fields-are-NULL-te.patch --]
[-- Type: text/plain, Size: 1583 bytes --]
From: Tom Zanussi <tom.zanussi@linux.intel.com>
The strncpy() currently being used for variable string fields can
result in a lack of termination if the string length is equal to the
field size. Use the safer strscpy() instead, which will guarantee
termination.
Link: http://lkml.kernel.org/r/fb97c1e518fb358c12a4057d7445ba2c46956cd7.1522256721.git.tom.zanussi@linux.intel.com
Signed-off-by: Tom Zanussi <tom.zanussi@linux.intel.com>
Signed-off-by: Steven Rostedt (VMware) <rostedt@goodmis.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 6114939f065a..15ea11c29a51 100644
--- a/kernel/trace/trace_events_hist.c
+++ b/kernel/trace/trace_events_hist.c
@@ -669,7 +669,7 @@ static notrace void trace_event_raw_event_synth(void *__data,
char *str_val = (char *)(long)var_ref_vals[var_ref_idx + i];
char *str_field = (char *)&entry->fields[n_u64];
- strncpy(str_field, str_val, STR_VAR_LEN_MAX);
+ strscpy(str_field, str_val, STR_VAR_LEN_MAX);
n_u64 += STR_VAR_LEN_MAX / sizeof(u64);
} else {
entry->fields[n_u64] = var_ref_vals[var_ref_idx + i];
@@ -3091,7 +3091,7 @@ static inline void __update_field_vars(struct tracing_map_elt *elt,
char *str = elt_data->field_var_str[j++];
char *val_str = (char *)(uintptr_t)var_val;
- strncpy(str, val_str, STR_VAR_LEN_MAX);
+ strscpy(str, val_str, STR_VAR_LEN_MAX);
var_val = (u64)(uintptr_t)str;
}
tracing_map_set_var(elt, var_idx, var_val);
--
2.15.1
^ permalink raw reply related [flat|nested] 19+ messages in thread* [for-next][PATCH 10/18] tracing: Uninitialized variable in create_tracing_map_fields()
2018-04-06 13:00 [for-next][PATCH 00/18] tracing: Last minute updates before pushing to Linus Steven Rostedt
` (8 preceding siblings ...)
2018-04-06 13:00 ` [for-next][PATCH 09/18] tracing: Make sure variable string fields are NULL-terminated Steven Rostedt
@ 2018-04-06 13:00 ` Steven Rostedt
2018-04-06 13:00 ` [for-next][PATCH 11/18] vsprintf: Do not preprocess non-dereferenced pointers for bprintf (%px and %pK) Steven Rostedt
` (7 subsequent siblings)
17 siblings, 0 replies; 19+ messages in thread
From: Steven Rostedt @ 2018-04-06 13:00 UTC (permalink / raw)
To: linux-kernel; +Cc: Ingo Molnar, Andrew Morton, Tom Zanussi, Dan Carpenter
[-- Attachment #1: 0010-tracing-Uninitialized-variable-in-create_tracing_map.patch --]
[-- Type: text/plain, Size: 1219 bytes --]
From: Dan Carpenter <dan.carpenter@oracle.com>
Smatch complains that idx can be used uninitialized when we check if
(idx < 0). It has to be the first iteration through the loop and the
HIST_FIELD_FL_STACKTRACE bit has to be clear and the HIST_FIELD_FL_VAR
bit has to be set to reach the bug.
Link: http://lkml.kernel.org/r/20180328114815.GC29050@mwanda
Fixes: 30350d65ac56 ("tracing: Add variable support to hist triggers")
Acked-by: Tom Zanussi <tom.zanussi@linux.intel.com>
Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
Signed-off-by: Steven Rostedt (VMware) <rostedt@goodmis.org>
---
kernel/trace/trace_events_hist.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/kernel/trace/trace_events_hist.c b/kernel/trace/trace_events_hist.c
index 15ea11c29a51..0d7b3ffbecc2 100644
--- a/kernel/trace/trace_events_hist.c
+++ b/kernel/trace/trace_events_hist.c
@@ -4458,7 +4458,7 @@ static int create_tracing_map_fields(struct hist_trigger_data *hist_data)
struct tracing_map *map = hist_data->map;
struct ftrace_event_field *field;
struct hist_field *hist_field;
- int i, idx;
+ int i, idx = 0;
for_each_hist_field(i, hist_data) {
hist_field = hist_data->fields[i];
--
2.15.1
^ permalink raw reply related [flat|nested] 19+ messages in thread* [for-next][PATCH 11/18] vsprintf: Do not preprocess non-dereferenced pointers for bprintf (%px and %pK)
2018-04-06 13:00 [for-next][PATCH 00/18] tracing: Last minute updates before pushing to Linus Steven Rostedt
` (9 preceding siblings ...)
2018-04-06 13:00 ` [for-next][PATCH 10/18] tracing: Uninitialized variable in create_tracing_map_fields() Steven Rostedt
@ 2018-04-06 13:00 ` Steven Rostedt
2018-04-06 13:00 ` [for-next][PATCH 12/18] lockdep: Add print_irqtrace_events() to __warn Steven Rostedt
` (6 subsequent siblings)
17 siblings, 0 replies; 19+ messages in thread
From: Steven Rostedt @ 2018-04-06 13:00 UTC (permalink / raw)
To: linux-kernel; +Cc: Ingo Molnar, Andrew Morton, stable
[-- Attachment #1: 0011-vsprintf-Do-not-preprocess-non-dereferenced-pointers.patch --]
[-- Type: text/plain, Size: 1287 bytes --]
From: "Steven Rostedt (VMware)" <rostedt@goodmis.org>
Commit 841a915d20c7b2 ("printf: Do not have bprintf dereference pointers")
would preprocess various pointers that are dereferenced in the bprintf()
because the recording and printing are done at two different times. Some
pointers stayed dereferenced in the ring buffer because user space could
handle them (namely "%pS" and friends). Pointers that are not dereferenced
should not be processed immediately but instead just saved directly.
Cc: stable@vger.kernel.org
Fixes: 841a915d20c7b2 ("printf: Do not have bprintf dereference pointers")
Signed-off-by: Steven Rostedt (VMware) <rostedt@goodmis.org>
---
lib/vsprintf.c | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/lib/vsprintf.c b/lib/vsprintf.c
index d7a708f82559..89f8a4a4b770 100644
--- a/lib/vsprintf.c
+++ b/lib/vsprintf.c
@@ -2591,6 +2591,8 @@ int vbin_printf(u32 *bin_buf, size_t size, const char *fmt, va_list args)
case 's':
case 'F':
case 'f':
+ case 'x':
+ case 'K':
save_arg(void *);
break;
default:
@@ -2765,6 +2767,8 @@ int bstr_printf(char *buf, size_t size, const char *fmt, const u32 *bin_buf)
case 's':
case 'F':
case 'f':
+ case 'x':
+ case 'K':
process = true;
break;
default:
--
2.15.1
^ permalink raw reply related [flat|nested] 19+ messages in thread* [for-next][PATCH 12/18] lockdep: Add print_irqtrace_events() to __warn
2018-04-06 13:00 [for-next][PATCH 00/18] tracing: Last minute updates before pushing to Linus Steven Rostedt
` (10 preceding siblings ...)
2018-04-06 13:00 ` [for-next][PATCH 11/18] vsprintf: Do not preprocess non-dereferenced pointers for bprintf (%px and %pK) Steven Rostedt
@ 2018-04-06 13:00 ` Steven Rostedt
2018-04-06 13:00 ` [for-next][PATCH 13/18] ring-buffer: Check if memory is available before allocation Steven Rostedt
` (5 subsequent siblings)
17 siblings, 0 replies; 19+ messages in thread
From: Steven Rostedt @ 2018-04-06 13:00 UTC (permalink / raw)
To: linux-kernel; +Cc: Ingo Molnar, Andrew Morton
[-- Attachment #1: 0012-lockdep-Add-print_irqtrace_events-to-__warn.patch --]
[-- Type: text/plain, Size: 917 bytes --]
From: "Steven Rostedt (VMware)" <rostedt@goodmis.org>
Running a test on a x86_32 kernel I triggered a bug that an interrupt
disable/enable isn't being catched by lockdep. At least knowing where the
last one was found would be helpful, but the warnings that are produced do
not show this information. Even without debugging lockdep, having the WARN()
display the last place hard and soft irqs were enabled or disabled is
valuable.
Signed-off-by: Steven Rostedt (VMware) <rostedt@goodmis.org>
---
kernel/panic.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/kernel/panic.c b/kernel/panic.c
index 2cfef408fec9..fa8d4cc4956a 100644
--- a/kernel/panic.c
+++ b/kernel/panic.c
@@ -554,6 +554,8 @@ void __warn(const char *file, int line, void *caller, unsigned taint,
else
dump_stack();
+ print_irqtrace_events(current);
+
print_oops_end_marker();
/* Just a warning, don't kill lockdep. */
--
2.15.1
^ permalink raw reply related [flat|nested] 19+ messages in thread* [for-next][PATCH 13/18] ring-buffer: Check if memory is available before allocation
2018-04-06 13:00 [for-next][PATCH 00/18] tracing: Last minute updates before pushing to Linus Steven Rostedt
` (11 preceding siblings ...)
2018-04-06 13:00 ` [for-next][PATCH 12/18] lockdep: Add print_irqtrace_events() to __warn Steven Rostedt
@ 2018-04-06 13:00 ` Steven Rostedt
2018-04-06 13:00 ` [for-next][PATCH 14/18] ring-buffer: Add set/clear_current_oom_origin() during allocations Steven Rostedt
` (4 subsequent siblings)
17 siblings, 0 replies; 19+ messages in thread
From: Steven Rostedt @ 2018-04-06 13:00 UTC (permalink / raw)
To: linux-kernel
Cc: Ingo Molnar, Andrew Morton, stable, linux-mm, Zhaoyang Huang,
Joel Fernandes
[-- Attachment #1: 0013-ring-buffer-Check-if-memory-is-available-before-allo.patch --]
[-- Type: text/plain, Size: 3020 bytes --]
From: "Steven Rostedt (VMware)" <rostedt@goodmis.org>
The ring buffer is made up of a link list of pages. When making the ring
buffer bigger, it will allocate all the pages it needs before adding to the
ring buffer, and if it fails, it frees them and returns an error. This makes
increasing the ring buffer size an all or nothing action. When this was
first created, the pages were allocated with "NORETRY". This was to not
cause any Out-Of-Memory (OOM) actions from allocating the ring buffer. But
NORETRY was too strict, as the ring buffer would fail to expand even when
there's memory available, but was taken up in the page cache.
Commit 848618857d253 ("tracing/ring_buffer: Try harder to allocate") changed
the allocating from NORETRY to RETRY_MAYFAIL. The RETRY_MAYFAIL would
allocate from the page cache, but if there was no memory available, it would
simple fail the allocation and not trigger an OOM.
This worked fine, but had one problem. As the ring buffer would allocate one
page at a time, it could take up all memory in the system before it failed
to allocate and free that memory. If the allocation is happening and the
ring buffer allocates all memory and then tries to take more than available,
its allocation will not trigger an OOM, but if there's any allocation that
happens someplace else, that could trigger an OOM, even though once the ring
buffer's allocation fails, it would free up all the previous memory it tried
to allocate, and allow other memory allocations to succeed.
Commit d02bd27bd33dd ("mm/page_alloc.c: calculate 'available' memory in a
separate function") separated out si_mem_availble() as a separate function
that could be used to see how much memory is available in the system. Using
this function to make sure that the ring buffer could be allocated before it
tries to allocate pages we can avoid allocating all memory in the system and
making it vulnerable to OOMs if other allocations are taking place.
Link: http://lkml.kernel.org/r/1522320104-6573-1-git-send-email-zhaoyang.huang@spreadtrum.com
CC: stable@vger.kernel.org
Cc: linux-mm@kvack.org
Fixes: 848618857d253 ("tracing/ring_buffer: Try harder to allocate")
Requires: d02bd27bd33dd ("mm/page_alloc.c: calculate 'available' memory in a separate function")
Reported-by: Zhaoyang Huang <huangzhaoyang@gmail.com>
Tested-by: Joel Fernandes <joelaf@google.com>
Signed-off-by: Steven Rostedt (VMware) <rostedt@goodmis.org>
---
kernel/trace/ring_buffer.c | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/kernel/trace/ring_buffer.c b/kernel/trace/ring_buffer.c
index 515be03e3009..966128f02121 100644
--- a/kernel/trace/ring_buffer.c
+++ b/kernel/trace/ring_buffer.c
@@ -1164,6 +1164,11 @@ static int __rb_allocate_pages(long nr_pages, struct list_head *pages, int cpu)
struct buffer_page *bpage, *tmp;
long i;
+ /* Check if the available memory is there first */
+ i = si_mem_available();
+ if (i < nr_pages)
+ return -ENOMEM;
+
for (i = 0; i < nr_pages; i++) {
struct page *page;
/*
--
2.15.1
^ permalink raw reply related [flat|nested] 19+ messages in thread* [for-next][PATCH 14/18] ring-buffer: Add set/clear_current_oom_origin() during allocations
2018-04-06 13:00 [for-next][PATCH 00/18] tracing: Last minute updates before pushing to Linus Steven Rostedt
` (12 preceding siblings ...)
2018-04-06 13:00 ` [for-next][PATCH 13/18] ring-buffer: Check if memory is available before allocation Steven Rostedt
@ 2018-04-06 13:00 ` Steven Rostedt
2018-04-06 13:00 ` [for-next][PATCH 15/18] tracing: Hide global trace clock from lockdep Steven Rostedt
` (3 subsequent siblings)
17 siblings, 0 replies; 19+ messages in thread
From: Steven Rostedt @ 2018-04-06 13:00 UTC (permalink / raw)
To: linux-kernel; +Cc: Ingo Molnar, Andrew Morton, Michal Hocko
[-- Attachment #1: 0014-ring-buffer-Add-set-clear_current_oom_origin-during-.patch --]
[-- Type: text/plain, Size: 3761 bytes --]
From: "Steven Rostedt (VMware)" <rostedt@goodmis.org>
As si_mem_available() can say there is enough memory even though the memory
available is not useable by the ring buffer, it is best to not kill innocent
applications because the ring buffer is taking up all the memory while it is
trying to allocate a great deal of memory.
If the allocator is user space (because kernel threads can also increase the
size of the kernel ring buffer on boot up), then after si_mem_available()
says there is enough memory, set the OOM killer to kill the current task if
an OOM triggers during the allocation.
Link: http://lkml.kernel.org/r/20180404062340.GD6312@dhcp22.suse.cz
Suggested-by: Michal Hocko <mhocko@kernel.org>
Signed-off-by: Steven Rostedt (VMware) <rostedt@goodmis.org>
---
kernel/trace/ring_buffer.c | 48 ++++++++++++++++++++++++++++++++++++----------
1 file changed, 38 insertions(+), 10 deletions(-)
diff --git a/kernel/trace/ring_buffer.c b/kernel/trace/ring_buffer.c
index 966128f02121..c9cb9767d49b 100644
--- a/kernel/trace/ring_buffer.c
+++ b/kernel/trace/ring_buffer.c
@@ -22,6 +22,7 @@
#include <linux/hash.h>
#include <linux/list.h>
#include <linux/cpu.h>
+#include <linux/oom.h>
#include <asm/local.h>
@@ -1162,35 +1163,60 @@ static int rb_check_pages(struct ring_buffer_per_cpu *cpu_buffer)
static int __rb_allocate_pages(long nr_pages, struct list_head *pages, int cpu)
{
struct buffer_page *bpage, *tmp;
+ bool user_thread = current->mm != NULL;
+ gfp_t mflags;
long i;
- /* Check if the available memory is there first */
+ /*
+ * Check if the available memory is there first.
+ * Note, si_mem_available() only gives us a rough estimate of available
+ * memory. It may not be accurate. But we don't care, we just want
+ * to prevent doing any allocation when it is obvious that it is
+ * not going to succeed.
+ */
i = si_mem_available();
if (i < nr_pages)
return -ENOMEM;
+ /*
+ * __GFP_RETRY_MAYFAIL flag makes sure that the allocation fails
+ * gracefully without invoking oom-killer and the system is not
+ * destabilized.
+ */
+ mflags = GFP_KERNEL | __GFP_RETRY_MAYFAIL;
+
+ /*
+ * If a user thread allocates too much, and si_mem_available()
+ * reports there's enough memory, even though there is not.
+ * Make sure the OOM killer kills this thread. This can happen
+ * even with RETRY_MAYFAIL because another task may be doing
+ * an allocation after this task has taken all memory.
+ * This is the task the OOM killer needs to take out during this
+ * loop, even if it was triggered by an allocation somewhere else.
+ */
+ if (user_thread)
+ set_current_oom_origin();
for (i = 0; i < nr_pages; i++) {
struct page *page;
- /*
- * __GFP_RETRY_MAYFAIL flag makes sure that the allocation fails
- * gracefully without invoking oom-killer and the system is not
- * destabilized.
- */
+
bpage = kzalloc_node(ALIGN(sizeof(*bpage), cache_line_size()),
- GFP_KERNEL | __GFP_RETRY_MAYFAIL,
- cpu_to_node(cpu));
+ mflags, cpu_to_node(cpu));
if (!bpage)
goto free_pages;
list_add(&bpage->list, pages);
- page = alloc_pages_node(cpu_to_node(cpu),
- GFP_KERNEL | __GFP_RETRY_MAYFAIL, 0);
+ page = alloc_pages_node(cpu_to_node(cpu), mflags, 0);
if (!page)
goto free_pages;
bpage->page = page_address(page);
rb_init_page(bpage->page);
+
+ if (user_thread && fatal_signal_pending(current))
+ goto free_pages;
}
+ if (user_thread)
+ clear_current_oom_origin();
return 0;
@@ -1199,6 +1225,8 @@ static int __rb_allocate_pages(long nr_pages, struct list_head *pages, int cpu)
list_del_init(&bpage->list);
free_buffer_page(bpage);
}
+ if (user_thread)
+ clear_current_oom_origin();
return -ENOMEM;
}
--
2.15.1
^ permalink raw reply related [flat|nested] 19+ messages in thread* [for-next][PATCH 15/18] tracing: Hide global trace clock from lockdep
2018-04-06 13:00 [for-next][PATCH 00/18] tracing: Last minute updates before pushing to Linus Steven Rostedt
` (13 preceding siblings ...)
2018-04-06 13:00 ` [for-next][PATCH 14/18] ring-buffer: Add set/clear_current_oom_origin() during allocations Steven Rostedt
@ 2018-04-06 13:00 ` Steven Rostedt
2018-04-06 13:00 ` [for-next][PATCH 16/18] tracing: Fixup logic inversion on setting trace_global_clock defaults Steven Rostedt
` (2 subsequent siblings)
17 siblings, 0 replies; 19+ messages in thread
From: Steven Rostedt @ 2018-04-06 13:00 UTC (permalink / raw)
To: linux-kernel; +Cc: Ingo Molnar, Andrew Morton
[-- Attachment #1: 0015-tracing-Hide-global-trace-clock-from-lockdep.patch --]
[-- Type: text/plain, Size: 1362 bytes --]
From: "Steven Rostedt (VMware)" <rostedt@goodmis.org>
Function tracing can trace in NMIs and such. If the TSC is determined
to be unstable, the tracing clock will switch to the global clock on
boot up, unless "trace_clock" is specified on the kernel command line.
The global clock disables interrupts to access sched_clock_cpu(), and in
doing so can be done within lockdep internals (because of function
tracing and NMIs). This can trigger false lockdep splats.
The trace_clock_global() is special, best not to trace the irq logic
within it.
Link: http://lkml.kernel.org/r/20180404145015.77bde42d@gandalf.local.home
Signed-off-by: Steven Rostedt (VMware) <rostedt@goodmis.org>
---
kernel/trace/trace_clock.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/kernel/trace/trace_clock.c b/kernel/trace/trace_clock.c
index 5fdc779f411d..d8a188e0418a 100644
--- a/kernel/trace/trace_clock.c
+++ b/kernel/trace/trace_clock.c
@@ -96,7 +96,7 @@ u64 notrace trace_clock_global(void)
int this_cpu;
u64 now;
- local_irq_save(flags);
+ raw_local_irq_save(flags);
this_cpu = raw_smp_processor_id();
now = sched_clock_cpu(this_cpu);
@@ -122,7 +122,7 @@ u64 notrace trace_clock_global(void)
arch_spin_unlock(&trace_clock_struct.lock);
out:
- local_irq_restore(flags);
+ raw_local_irq_restore(flags);
return now;
}
--
2.15.1
^ permalink raw reply related [flat|nested] 19+ messages in thread* [for-next][PATCH 16/18] tracing: Fixup logic inversion on setting trace_global_clock defaults
2018-04-06 13:00 [for-next][PATCH 00/18] tracing: Last minute updates before pushing to Linus Steven Rostedt
` (14 preceding siblings ...)
2018-04-06 13:00 ` [for-next][PATCH 15/18] tracing: Hide global trace clock from lockdep Steven Rostedt
@ 2018-04-06 13:00 ` Steven Rostedt
2018-04-06 13:00 ` [for-next][PATCH 17/18] tracing: Add rcu dereference annotation for filter->prog Steven Rostedt
2018-04-06 13:00 ` [for-next][PATCH 18/18] tracing: Add rcu dereference annotation for test func that touches filter->prog Steven Rostedt
17 siblings, 0 replies; 19+ messages in thread
From: Steven Rostedt @ 2018-04-06 13:00 UTC (permalink / raw)
To: linux-kernel; +Cc: Ingo Molnar, Andrew Morton, Chris Wilson
[-- Attachment #1: 0016-tracing-Fixup-logic-inversion-on-setting-trace_globa.patch --]
[-- Type: text/plain, Size: 1422 bytes --]
From: Chris Wilson <chris@chris-wilson.co.uk>
In commit 932066a15335 ("tracing: Default to using trace_global_clock if
sched_clock is unstable"), the logic for deciding to override the
default clock if unstable was reversed from the earlier posting. I was
trying to reduce the width of the message by using an early return
rather than a if-block, but reverted back to using the if-block and
accidentally left the predicate inverted.
Link: http://lkml.kernel.org/r/20180404212450.26646-1-chris@chris-wilson.co.uk
Fixes: 932066a15335 ("tracing: Default to using trace_global_clock if sched_clock is unstable")
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Signed-off-by: Steven Rostedt (VMware) <rostedt@goodmis.org>
---
kernel/trace/trace.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/kernel/trace/trace.c b/kernel/trace/trace.c
index 0f47e653ffd8..e18e69183c9a 100644
--- a/kernel/trace/trace.c
+++ b/kernel/trace/trace.c
@@ -8602,7 +8602,7 @@ late_initcall_sync(clear_boot_tracer);
__init static int tracing_set_default_clock(void)
{
/* sched_clock_stable() is determined in late_initcall */
- if (trace_boot_clock || sched_clock_stable()) {
+ if (!trace_boot_clock && !sched_clock_stable()) {
printk(KERN_WARNING
"Unstable clock detected, switching default tracing clock to \"global\"\n"
"If you want to keep using the local clock, then add:\n"
--
2.15.1
^ permalink raw reply related [flat|nested] 19+ messages in thread* [for-next][PATCH 17/18] tracing: Add rcu dereference annotation for filter->prog
2018-04-06 13:00 [for-next][PATCH 00/18] tracing: Last minute updates before pushing to Linus Steven Rostedt
` (15 preceding siblings ...)
2018-04-06 13:00 ` [for-next][PATCH 16/18] tracing: Fixup logic inversion on setting trace_global_clock defaults Steven Rostedt
@ 2018-04-06 13:00 ` Steven Rostedt
2018-04-06 13:00 ` [for-next][PATCH 18/18] tracing: Add rcu dereference annotation for test func that touches filter->prog Steven Rostedt
17 siblings, 0 replies; 19+ messages in thread
From: Steven Rostedt @ 2018-04-06 13:00 UTC (permalink / raw)
To: linux-kernel; +Cc: Ingo Molnar, Andrew Morton, kbuild test robot
[-- Attachment #1: 0017-tracing-Add-rcu-dereference-annotation-for-filter-pr.patch --]
[-- Type: text/plain, Size: 1135 bytes --]
From: "Steven Rostedt (VMware)" <rostedt@goodmis.org>
ftrace_function_set_filter() referenences filter->prog without annotation
and sparse complains about it. It needs a rcu_dereference_protected()
wrapper.
Reported-by: kbuild test robot <fengguang.wu@intel.com>
Fixes: 80765597bc587 ("tracing: Rewrite filter logic to be simpler and faster")
Signed-off-by: Steven Rostedt (VMware) <rostedt@goodmis.org>
---
kernel/trace/trace_events_filter.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/kernel/trace/trace_events_filter.c b/kernel/trace/trace_events_filter.c
index 703a416aa5c2..cf8460caa95c 100644
--- a/kernel/trace/trace_events_filter.c
+++ b/kernel/trace/trace_events_filter.c
@@ -1992,7 +1992,8 @@ static bool is_or(struct prog_entry *prog, int i)
static int ftrace_function_set_filter(struct perf_event *event,
struct event_filter *filter)
{
- struct prog_entry *prog = filter->prog;
+ struct prog_entry *prog = rcu_dereference_protected(filter->prog,
+ lockdep_is_held(&event_mutex));
struct function_filter_data data = {
.first_filter = 1,
.first_notrace = 1,
--
2.15.1
^ permalink raw reply related [flat|nested] 19+ messages in thread* [for-next][PATCH 18/18] tracing: Add rcu dereference annotation for test func that touches filter->prog
2018-04-06 13:00 [for-next][PATCH 00/18] tracing: Last minute updates before pushing to Linus Steven Rostedt
` (16 preceding siblings ...)
2018-04-06 13:00 ` [for-next][PATCH 17/18] tracing: Add rcu dereference annotation for filter->prog Steven Rostedt
@ 2018-04-06 13:00 ` Steven Rostedt
17 siblings, 0 replies; 19+ messages in thread
From: Steven Rostedt @ 2018-04-06 13:00 UTC (permalink / raw)
To: linux-kernel; +Cc: Ingo Molnar, Andrew Morton, kbuild test robot
[-- Attachment #1: 0018-tracing-Add-rcu-dereference-annotation-for-test-func.patch --]
[-- Type: text/plain, Size: 1775 bytes --]
From: "Steven Rostedt (VMware)" <rostedt@goodmis.org>
A boot up test function update_pred_fn() dereferences filter->prog without
the proper rcu annotation.
To do this, we must also take the event_mutex first. Normally, this isn't
needed because this test function can not race with other use cases that
touch the event filters (it is disabled if any events are enabled).
Reported-by: kbuild test robot <fengguang.wu@intel.com>
Fixes: 80765597bc587 ("tracing: Rewrite filter logic to be simpler and faster")
Signed-off-by: Steven Rostedt (VMware) <rostedt@goodmis.org>
---
kernel/trace/trace_events_filter.c | 7 ++++++-
1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/kernel/trace/trace_events_filter.c b/kernel/trace/trace_events_filter.c
index cf8460caa95c..1bda4ec95e18 100644
--- a/kernel/trace/trace_events_filter.c
+++ b/kernel/trace/trace_events_filter.c
@@ -2155,7 +2155,8 @@ static int test_pred_visited_fn(struct filter_pred *pred, void *event)
static void update_pred_fn(struct event_filter *filter, char *fields)
{
- struct prog_entry *prog = filter->prog;
+ struct prog_entry *prog = rcu_dereference_protected(filter->prog,
+ lockdep_is_held(&event_mutex));
int i;
for (i = 0; prog[i].pred; i++) {
@@ -2197,6 +2198,8 @@ static __init int ftrace_test_event_filter(void)
break;
}
+ /* Needed to dereference filter->prog */
+ mutex_lock(&event_mutex);
/*
* The preemption disabling is not really needed for self
* tests, but the rcu dereference will complain without it.
@@ -2209,6 +2212,8 @@ static __init int ftrace_test_event_filter(void)
err = filter_match_preds(filter, &d->rec);
preempt_enable();
+ mutex_unlock(&event_mutex);
+
__free_filter(filter);
if (test_pred_visited) {
--
2.15.1
^ permalink raw reply related [flat|nested] 19+ messages in thread