* [for-next][PATCH 01/13] tracepoints: Fix documentation of RCU lockdep checks
2015-11-04 15:29 [for-next][PATCH 00/13] tracing: Some more last minute cleanups and fixes for 4.4 Steven Rostedt
@ 2015-11-04 15:29 ` Steven Rostedt
2015-11-04 15:29 ` [for-next][PATCH 02/13] recordmcount: Fix endianness handling bug for nop_mcount Steven Rostedt
` (11 subsequent siblings)
12 siblings, 0 replies; 14+ messages in thread
From: Steven Rostedt @ 2015-11-04 15:29 UTC (permalink / raw)
To: linux-kernel; +Cc: Ingo Molnar, Andrew Morton, Dave Hansen, Mathieu Desnoyers
[-- Attachment #1: 0001-tracepoints-Fix-documentation-of-RCU-lockdep-checks.patch --]
[-- Type: text/plain, Size: 1771 bytes --]
From: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
The documentation on top of __DECLARE_TRACE() does not match its
implementation since the condition check has been added to the
RCU lockdep checks. Update the documentation to match its
implementation.
Link: http://lkml.kernel.org/r/1446504164-21563-1-git-send-email-mathieu.desnoyers@efficios.com
CC: Dave Hansen <dave@sr71.net>
Fixes: a05d59a56733 "tracing: Add condition check to RCU lockdep checks"
Signed-off-by: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
Signed-off-by: Steven Rostedt <rostedt@goodmis.org>
---
include/linux/tracepoint.h | 9 +++++----
1 file changed, 5 insertions(+), 4 deletions(-)
diff --git a/include/linux/tracepoint.h b/include/linux/tracepoint.h
index 6b79537a42b1..696a339c592c 100644
--- a/include/linux/tracepoint.h
+++ b/include/linux/tracepoint.h
@@ -184,10 +184,11 @@ extern void syscall_unregfunc(void);
* structure. Force alignment to the same alignment as the section start.
*
* When lockdep is enabled, we make sure to always do the RCU portions of
- * the tracepoint code, regardless of whether tracing is on or we match the
- * condition. This lets us find RCU issues triggered with tracepoints even
- * when this tracepoint is off. This code has no purpose other than poking
- * RCU a bit.
+ * the tracepoint code, regardless of whether tracing is on. However,
+ * don't check if the condition is false, due to interaction with idle
+ * instrumentation. This lets us find RCU issues triggered with tracepoints
+ * even when this tracepoint is off. This code has no purpose other than
+ * poking RCU a bit.
*/
#define __DECLARE_TRACE(name, proto, args, cond, data_proto, data_args) \
extern struct tracepoint __tracepoint_##name; \
--
2.6.1
^ permalink raw reply related [flat|nested] 14+ messages in thread* [for-next][PATCH 02/13] recordmcount: Fix endianness handling bug for nop_mcount
2015-11-04 15:29 [for-next][PATCH 00/13] tracing: Some more last minute cleanups and fixes for 4.4 Steven Rostedt
2015-11-04 15:29 ` [for-next][PATCH 01/13] tracepoints: Fix documentation of RCU lockdep checks Steven Rostedt
@ 2015-11-04 15:29 ` Steven Rostedt
2015-11-04 15:29 ` [for-next][PATCH 03/13] recordmcount: arm64: Replace the ignored mcount call into nop Steven Rostedt
` (10 subsequent siblings)
12 siblings, 0 replies; 14+ messages in thread
From: Steven Rostedt @ 2015-11-04 15:29 UTC (permalink / raw)
To: linux-kernel; +Cc: Ingo Molnar, Andrew Morton, stable, Li Bin
[-- Attachment #1: 0002-recordmcount-Fix-endianness-handling-bug-for-nop_mco.patch --]
[-- Type: text/plain, Size: 1110 bytes --]
From: libin <huawei.libin@huawei.com>
In nop_mcount, shdr->sh_offset and welp->r_offset should handle
endianness properly, otherwise it will trigger Segmentation fault
if the recordmcount main and file.o have different endianness.
Link: http://lkml.kernel.org/r/563806C7.7070606@huawei.com
Cc: <stable@vger.kernel.org> # 3.0+
Signed-off-by: Li Bin <huawei.libin@huawei.com>
Signed-off-by: Steven Rostedt <rostedt@goodmis.org>
---
scripts/recordmcount.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/scripts/recordmcount.h b/scripts/recordmcount.h
index 49b582a225b0..b9897e2be404 100644
--- a/scripts/recordmcount.h
+++ b/scripts/recordmcount.h
@@ -377,7 +377,7 @@ static void nop_mcount(Elf_Shdr const *const relhdr,
if (mcountsym == Elf_r_sym(relp) && !is_fake_mcount(relp)) {
if (make_nop)
- ret = make_nop((void *)ehdr, shdr->sh_offset + relp->r_offset);
+ ret = make_nop((void *)ehdr, _w(shdr->sh_offset) + _w(relp->r_offset));
if (warn_on_notrace_sect && !once) {
printf("Section %s has mcount callers being ignored\n",
txtname);
--
2.6.1
^ permalink raw reply related [flat|nested] 14+ messages in thread* [for-next][PATCH 03/13] recordmcount: arm64: Replace the ignored mcount call into nop
2015-11-04 15:29 [for-next][PATCH 00/13] tracing: Some more last minute cleanups and fixes for 4.4 Steven Rostedt
2015-11-04 15:29 ` [for-next][PATCH 01/13] tracepoints: Fix documentation of RCU lockdep checks Steven Rostedt
2015-11-04 15:29 ` [for-next][PATCH 02/13] recordmcount: Fix endianness handling bug for nop_mcount Steven Rostedt
@ 2015-11-04 15:29 ` Steven Rostedt
2015-11-04 15:29 ` [for-next][PATCH 04/13] tracing: Allow arch-specific stack tracer Steven Rostedt
` (9 subsequent siblings)
12 siblings, 0 replies; 14+ messages in thread
From: Steven Rostedt @ 2015-11-04 15:29 UTC (permalink / raw)
To: linux-kernel
Cc: Ingo Molnar, Andrew Morton, lkp, catalin.marinas,
takahiro.akashi, stable, Will Deacon, Li Bin
[-- Attachment #1: 0003-recordmcount-arm64-Replace-the-ignored-mcount-call-i.patch --]
[-- Type: text/plain, Size: 2427 bytes --]
From: Li Bin <huawei.libin@huawei.com>
By now, the recordmcount only records the function that in
following sections:
.text/.ref.text/.sched.text/.spinlock.text/.irqentry.text/
.kprobes.text/.text.unlikely
For the function that not in these sections, the call mcount
will be in place and not be replaced when kernel boot up. And
it will bring performance overhead, such as do_mem_abort (in
.exception.text section). This patch make the call mcount to
nop for this case in recordmcount.
Link: http://lkml.kernel.org/r/1446019445-14421-1-git-send-email-huawei.libin@huawei.com
Link: http://lkml.kernel.org/r/1446193864-24593-4-git-send-email-huawei.libin@huawei.com
Cc: <lkp@intel.com>
Cc: <catalin.marinas@arm.com>
Cc: <takahiro.akashi@linaro.org>
Cc: <stable@vger.kernel.org> # 3.18+
Acked-by: Will Deacon <will.deacon@arm.com>
Signed-off-by: Li Bin <huawei.libin@huawei.com>
Signed-off-by: Steven Rostedt <rostedt@goodmis.org>
---
scripts/recordmcount.c | 24 +++++++++++++++++++++++-
1 file changed, 23 insertions(+), 1 deletion(-)
diff --git a/scripts/recordmcount.c b/scripts/recordmcount.c
index 8cc020bbe859..698768bdc581 100644
--- a/scripts/recordmcount.c
+++ b/scripts/recordmcount.c
@@ -42,6 +42,7 @@
#ifndef EM_AARCH64
#define EM_AARCH64 183
+#define R_AARCH64_NONE 0
#define R_AARCH64_ABS64 257
#endif
@@ -160,6 +161,22 @@ static int make_nop_x86(void *map, size_t const offset)
return 0;
}
+static unsigned char ideal_nop4_arm64[4] = {0x1f, 0x20, 0x03, 0xd5};
+static int make_nop_arm64(void *map, size_t const offset)
+{
+ uint32_t *ptr;
+
+ ptr = map + offset;
+ /* bl <_mcount> is 0x94000000 before relocation */
+ if (*ptr != 0x94000000)
+ return -1;
+
+ /* Convert to nop */
+ ulseek(fd_map, offset, SEEK_SET);
+ uwrite(fd_map, ideal_nop, 4);
+ return 0;
+}
+
/*
* Get the whole file as a programming convenience in order to avoid
* malloc+lseek+read+free of many pieces. If successful, then mmap
@@ -354,7 +371,12 @@ do_file(char const *const fname)
altmcount = "__gnu_mcount_nc";
break;
case EM_AARCH64:
- reltype = R_AARCH64_ABS64; gpfx = '_'; break;
+ reltype = R_AARCH64_ABS64;
+ make_nop = make_nop_arm64;
+ rel_type_nop = R_AARCH64_NONE;
+ ideal_nop = ideal_nop4_arm64;
+ gpfx = '_';
+ break;
case EM_IA_64: reltype = R_IA64_IMM64; gpfx = '_'; break;
case EM_METAG: reltype = R_METAG_ADDR32;
altmcount = "_mcount_wrapper";
--
2.6.1
^ permalink raw reply related [flat|nested] 14+ messages in thread* [for-next][PATCH 04/13] tracing: Allow arch-specific stack tracer
2015-11-04 15:29 [for-next][PATCH 00/13] tracing: Some more last minute cleanups and fixes for 4.4 Steven Rostedt
` (2 preceding siblings ...)
2015-11-04 15:29 ` [for-next][PATCH 03/13] recordmcount: arm64: Replace the ignored mcount call into nop Steven Rostedt
@ 2015-11-04 15:29 ` Steven Rostedt
2015-11-04 15:29 ` [for-next][PATCH 05/13] tracing: Rename max_stack_lock to stack_trace_max_lock Steven Rostedt
` (8 subsequent siblings)
12 siblings, 0 replies; 14+ messages in thread
From: Steven Rostedt @ 2015-11-04 15:29 UTC (permalink / raw)
To: linux-kernel; +Cc: Ingo Molnar, Andrew Morton, AKASHI Takahiro
[-- Attachment #1: 0004-tracing-Allow-arch-specific-stack-tracer.patch --]
[-- Type: text/plain, Size: 8254 bytes --]
From: AKASHI Takahiro <takahiro.akashi@linaro.org>
A stack frame may be used in a different way depending on cpu architecture.
Thus it is not always appropriate to slurp the stack contents, as current
check_stack() does, in order to calcurate a stack index (height) at a given
function call. At least not on arm64.
In addition, there is a possibility that we will mistakenly detect a stale
stack frame which has not been overwritten.
This patch makes check_stack() a weak function so as to later implement
arch-specific version.
Link: http://lkml.kernel.org/r/1446182741-31019-5-git-send-email-takahiro.akashi@linaro.org
Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org>
Signed-off-by: Steven Rostedt <rostedt@goodmis.org>
---
include/linux/ftrace.h | 11 +++++++
kernel/trace/trace_stack.c | 80 +++++++++++++++++++++++++---------------------
2 files changed, 54 insertions(+), 37 deletions(-)
diff --git a/include/linux/ftrace.h b/include/linux/ftrace.h
index 6cd8c0ee4b6f..b4c92ab9e08b 100644
--- a/include/linux/ftrace.h
+++ b/include/linux/ftrace.h
@@ -263,7 +263,18 @@ static inline void ftrace_kill(void) { }
#endif /* CONFIG_FUNCTION_TRACER */
#ifdef CONFIG_STACK_TRACER
+
+#define STACK_TRACE_ENTRIES 500
+
+struct stack_trace;
+
+extern unsigned stack_trace_index[];
+extern struct stack_trace stack_trace_max;
+extern unsigned long stack_trace_max_size;
+extern arch_spinlock_t max_stack_lock;
+
extern int stack_tracer_enabled;
+void stack_trace_print(void);
int
stack_trace_sysctl(struct ctl_table *table, int write,
void __user *buffer, size_t *lenp,
diff --git a/kernel/trace/trace_stack.c b/kernel/trace/trace_stack.c
index b746399ab59c..50945a7939f4 100644
--- a/kernel/trace/trace_stack.c
+++ b/kernel/trace/trace_stack.c
@@ -16,24 +16,22 @@
#include "trace.h"
-#define STACK_TRACE_ENTRIES 500
-
static unsigned long stack_dump_trace[STACK_TRACE_ENTRIES+1] =
{ [0 ... (STACK_TRACE_ENTRIES)] = ULONG_MAX };
-static unsigned stack_dump_index[STACK_TRACE_ENTRIES];
+unsigned stack_trace_index[STACK_TRACE_ENTRIES];
/*
* Reserve one entry for the passed in ip. This will allow
* us to remove most or all of the stack size overhead
* added by the stack tracer itself.
*/
-static struct stack_trace max_stack_trace = {
+struct stack_trace stack_trace_max = {
.max_entries = STACK_TRACE_ENTRIES - 1,
.entries = &stack_dump_trace[0],
};
-static unsigned long max_stack_size;
-static arch_spinlock_t max_stack_lock =
+unsigned long stack_trace_max_size;
+arch_spinlock_t max_stack_lock =
(arch_spinlock_t)__ARCH_SPIN_LOCK_UNLOCKED;
static DEFINE_PER_CPU(int, trace_active);
@@ -42,30 +40,38 @@ static DEFINE_MUTEX(stack_sysctl_mutex);
int stack_tracer_enabled;
static int last_stack_tracer_enabled;
-static inline void print_max_stack(void)
+void stack_trace_print(void)
{
long i;
int size;
pr_emerg(" Depth Size Location (%d entries)\n"
" ----- ---- --------\n",
- max_stack_trace.nr_entries);
+ stack_trace_max.nr_entries);
- for (i = 0; i < max_stack_trace.nr_entries; i++) {
+ for (i = 0; i < stack_trace_max.nr_entries; i++) {
if (stack_dump_trace[i] == ULONG_MAX)
break;
- if (i+1 == max_stack_trace.nr_entries ||
+ if (i+1 == stack_trace_max.nr_entries ||
stack_dump_trace[i+1] == ULONG_MAX)
- size = stack_dump_index[i];
+ size = stack_trace_index[i];
else
- size = stack_dump_index[i] - stack_dump_index[i+1];
+ size = stack_trace_index[i] - stack_trace_index[i+1];
- pr_emerg("%3ld) %8d %5d %pS\n", i, stack_dump_index[i],
+ pr_emerg("%3ld) %8d %5d %pS\n", i, stack_trace_index[i],
size, (void *)stack_dump_trace[i]);
}
}
-static inline void
+/*
+ * When arch-specific code overides this function, the following
+ * data should be filled up, assuming max_stack_lock is held to
+ * prevent concurrent updates.
+ * stack_trace_index[]
+ * stack_trace_max
+ * stack_trace_max_size
+ */
+void __weak
check_stack(unsigned long ip, unsigned long *stack)
{
unsigned long this_size, flags; unsigned long *p, *top, *start;
@@ -78,7 +84,7 @@ check_stack(unsigned long ip, unsigned long *stack)
/* Remove the frame of the tracer */
this_size -= frame_size;
- if (this_size <= max_stack_size)
+ if (this_size <= stack_trace_max_size)
return;
/* we do not handle interrupt stacks yet */
@@ -93,18 +99,18 @@ check_stack(unsigned long ip, unsigned long *stack)
this_size -= tracer_frame;
/* a race could have already updated it */
- if (this_size <= max_stack_size)
+ if (this_size <= stack_trace_max_size)
goto out;
- max_stack_size = this_size;
+ stack_trace_max_size = this_size;
- max_stack_trace.nr_entries = 0;
- max_stack_trace.skip = 3;
+ stack_trace_max.nr_entries = 0;
+ stack_trace_max.skip = 3;
- save_stack_trace(&max_stack_trace);
+ save_stack_trace(&stack_trace_max);
/* Skip over the overhead of the stack tracer itself */
- for (i = 0; i < max_stack_trace.nr_entries; i++) {
+ for (i = 0; i < stack_trace_max.nr_entries; i++) {
if (stack_dump_trace[i] == ip)
break;
}
@@ -124,18 +130,18 @@ check_stack(unsigned long ip, unsigned long *stack)
* loop will only happen once. This code only takes place
* on a new max, so it is far from a fast path.
*/
- while (i < max_stack_trace.nr_entries) {
+ while (i < stack_trace_max.nr_entries) {
int found = 0;
- stack_dump_index[x] = this_size;
+ stack_trace_index[x] = this_size;
p = start;
- for (; p < top && i < max_stack_trace.nr_entries; p++) {
+ for (; p < top && i < stack_trace_max.nr_entries; p++) {
if (stack_dump_trace[i] == ULONG_MAX)
break;
if (*p == stack_dump_trace[i]) {
stack_dump_trace[x] = stack_dump_trace[i++];
- this_size = stack_dump_index[x++] =
+ this_size = stack_trace_index[x++] =
(top - p) * sizeof(unsigned long);
found = 1;
/* Start the search from here */
@@ -150,7 +156,7 @@ check_stack(unsigned long ip, unsigned long *stack)
if (unlikely(!tracer_frame)) {
tracer_frame = (p - stack) *
sizeof(unsigned long);
- max_stack_size -= tracer_frame;
+ stack_trace_max_size -= tracer_frame;
}
}
}
@@ -159,12 +165,12 @@ check_stack(unsigned long ip, unsigned long *stack)
i++;
}
- max_stack_trace.nr_entries = x;
+ stack_trace_max.nr_entries = x;
for (; x < i; x++)
stack_dump_trace[x] = ULONG_MAX;
if (task_stack_end_corrupted(current)) {
- print_max_stack();
+ stack_trace_print();
BUG();
}
@@ -262,7 +268,7 @@ __next(struct seq_file *m, loff_t *pos)
{
long n = *pos - 1;
- if (n > max_stack_trace.nr_entries || stack_dump_trace[n] == ULONG_MAX)
+ if (n > stack_trace_max.nr_entries || stack_dump_trace[n] == ULONG_MAX)
return NULL;
m->private = (void *)n;
@@ -332,9 +338,9 @@ static int t_show(struct seq_file *m, void *v)
seq_printf(m, " Depth Size Location"
" (%d entries)\n"
" ----- ---- --------\n",
- max_stack_trace.nr_entries);
+ stack_trace_max.nr_entries);
- if (!stack_tracer_enabled && !max_stack_size)
+ if (!stack_tracer_enabled && !stack_trace_max_size)
print_disabled(m);
return 0;
@@ -342,17 +348,17 @@ static int t_show(struct seq_file *m, void *v)
i = *(long *)v;
- if (i >= max_stack_trace.nr_entries ||
+ if (i >= stack_trace_max.nr_entries ||
stack_dump_trace[i] == ULONG_MAX)
return 0;
- if (i+1 == max_stack_trace.nr_entries ||
+ if (i+1 == stack_trace_max.nr_entries ||
stack_dump_trace[i+1] == ULONG_MAX)
- size = stack_dump_index[i];
+ size = stack_trace_index[i];
else
- size = stack_dump_index[i] - stack_dump_index[i+1];
+ size = stack_trace_index[i] - stack_trace_index[i+1];
- seq_printf(m, "%3ld) %8d %5d ", i, stack_dump_index[i], size);
+ seq_printf(m, "%3ld) %8d %5d ", i, stack_trace_index[i], size);
trace_lookup_stack(m, i);
@@ -442,7 +448,7 @@ static __init int stack_trace_init(void)
return 0;
trace_create_file("stack_max_size", 0644, d_tracer,
- &max_stack_size, &stack_max_size_fops);
+ &stack_trace_max_size, &stack_max_size_fops);
trace_create_file("stack_trace", 0444, d_tracer,
NULL, &stack_trace_fops);
--
2.6.1
^ permalink raw reply related [flat|nested] 14+ messages in thread* [for-next][PATCH 05/13] tracing: Rename max_stack_lock to stack_trace_max_lock
2015-11-04 15:29 [for-next][PATCH 00/13] tracing: Some more last minute cleanups and fixes for 4.4 Steven Rostedt
` (3 preceding siblings ...)
2015-11-04 15:29 ` [for-next][PATCH 04/13] tracing: Allow arch-specific stack tracer Steven Rostedt
@ 2015-11-04 15:29 ` Steven Rostedt
2015-11-04 15:29 ` [for-next][PATCH 06/13] tracing: Remove redundant TP_ARGS redefining Steven Rostedt
` (7 subsequent siblings)
12 siblings, 0 replies; 14+ messages in thread
From: Steven Rostedt @ 2015-11-04 15:29 UTC (permalink / raw)
To: linux-kernel; +Cc: Ingo Molnar, Andrew Morton
[-- Attachment #1: 0005-tracing-Rename-max_stack_lock-to-stack_trace_max_loc.patch --]
[-- Type: text/plain, Size: 3040 bytes --]
From: "Steven Rostedt (Red Hat)" <rostedt@goodmis.org>
Now that max_stack_lock is a global variable, it requires a naming
convention that is unlikely to collide. Rename it to the same naming
convention that the other stack_trace variables have.
Signed-off-by: Steven Rostedt <rostedt@goodmis.org>
---
include/linux/ftrace.h | 2 +-
kernel/trace/trace_stack.c | 16 ++++++++--------
2 files changed, 9 insertions(+), 9 deletions(-)
diff --git a/include/linux/ftrace.h b/include/linux/ftrace.h
index b4c92ab9e08b..eae6548efbf0 100644
--- a/include/linux/ftrace.h
+++ b/include/linux/ftrace.h
@@ -271,7 +271,7 @@ struct stack_trace;
extern unsigned stack_trace_index[];
extern struct stack_trace stack_trace_max;
extern unsigned long stack_trace_max_size;
-extern arch_spinlock_t max_stack_lock;
+extern arch_spinlock_t stack_trace_max_lock;
extern int stack_tracer_enabled;
void stack_trace_print(void);
diff --git a/kernel/trace/trace_stack.c b/kernel/trace/trace_stack.c
index 50945a7939f4..0bd212af406c 100644
--- a/kernel/trace/trace_stack.c
+++ b/kernel/trace/trace_stack.c
@@ -31,7 +31,7 @@ struct stack_trace stack_trace_max = {
};
unsigned long stack_trace_max_size;
-arch_spinlock_t max_stack_lock =
+arch_spinlock_t stack_trace_max_lock =
(arch_spinlock_t)__ARCH_SPIN_LOCK_UNLOCKED;
static DEFINE_PER_CPU(int, trace_active);
@@ -65,7 +65,7 @@ void stack_trace_print(void)
/*
* When arch-specific code overides this function, the following
- * data should be filled up, assuming max_stack_lock is held to
+ * data should be filled up, assuming stack_trace_max_lock is held to
* prevent concurrent updates.
* stack_trace_index[]
* stack_trace_max
@@ -92,7 +92,7 @@ check_stack(unsigned long ip, unsigned long *stack)
return;
local_irq_save(flags);
- arch_spin_lock(&max_stack_lock);
+ arch_spin_lock(&stack_trace_max_lock);
/* In case another CPU set the tracer_frame on us */
if (unlikely(!frame_size))
@@ -175,7 +175,7 @@ check_stack(unsigned long ip, unsigned long *stack)
}
out:
- arch_spin_unlock(&max_stack_lock);
+ arch_spin_unlock(&stack_trace_max_lock);
local_irq_restore(flags);
}
@@ -246,9 +246,9 @@ stack_max_size_write(struct file *filp, const char __user *ubuf,
cpu = smp_processor_id();
per_cpu(trace_active, cpu)++;
- arch_spin_lock(&max_stack_lock);
+ arch_spin_lock(&stack_trace_max_lock);
*ptr = val;
- arch_spin_unlock(&max_stack_lock);
+ arch_spin_unlock(&stack_trace_max_lock);
per_cpu(trace_active, cpu)--;
local_irq_restore(flags);
@@ -291,7 +291,7 @@ static void *t_start(struct seq_file *m, loff_t *pos)
cpu = smp_processor_id();
per_cpu(trace_active, cpu)++;
- arch_spin_lock(&max_stack_lock);
+ arch_spin_lock(&stack_trace_max_lock);
if (*pos == 0)
return SEQ_START_TOKEN;
@@ -303,7 +303,7 @@ static void t_stop(struct seq_file *m, void *p)
{
int cpu;
- arch_spin_unlock(&max_stack_lock);
+ arch_spin_unlock(&stack_trace_max_lock);
cpu = smp_processor_id();
per_cpu(trace_active, cpu)--;
--
2.6.1
^ permalink raw reply related [flat|nested] 14+ messages in thread* [for-next][PATCH 06/13] tracing: Remove redundant TP_ARGS redefining
2015-11-04 15:29 [for-next][PATCH 00/13] tracing: Some more last minute cleanups and fixes for 4.4 Steven Rostedt
` (4 preceding siblings ...)
2015-11-04 15:29 ` [for-next][PATCH 05/13] tracing: Rename max_stack_lock to stack_trace_max_lock Steven Rostedt
@ 2015-11-04 15:29 ` Steven Rostedt
2015-11-04 15:29 ` [for-next][PATCH 07/13] ring_buffer: Do no not complete benchmark reader too early Steven Rostedt
` (6 subsequent siblings)
12 siblings, 0 replies; 14+ messages in thread
From: Steven Rostedt @ 2015-11-04 15:29 UTC (permalink / raw)
To: linux-kernel; +Cc: Ingo Molnar, Andrew Morton, Dmitry Safonov
[-- Attachment #1: 0006-tracing-Remove-redundant-TP_ARGS-redefining.patch --]
[-- Type: text/plain, Size: 806 bytes --]
From: Dmitry Safonov <0x7f454c46@gmail.com>
TP_ARGS is not used anywhere in trace.h nor trace_entries.h
Firstly, I left just #undef TP_ARGS and had no errors - remove it.
Link: http://lkml.kernel.org/r/1446576560-14085-1-git-send-email-0x7f454c46@gmail.com
Signed-off-by: Dmitry Safonov <0x7f454c46@gmail.com>
Signed-off-by: Steven Rostedt <rostedt@goodmis.org>
---
kernel/trace/trace.h | 3 ---
1 file changed, 3 deletions(-)
diff --git a/kernel/trace/trace.h b/kernel/trace/trace.h
index be2126f5215d..dd7620802e72 100644
--- a/kernel/trace/trace.h
+++ b/kernel/trace/trace.h
@@ -71,9 +71,6 @@ enum trace_type {
tstruct \
}
-#undef TP_ARGS
-#define TP_ARGS(args...) args
-
#undef FTRACE_ENTRY_DUP
#define FTRACE_ENTRY_DUP(name, name_struct, id, tstruct, printk, filter)
--
2.6.1
^ permalink raw reply related [flat|nested] 14+ messages in thread* [for-next][PATCH 07/13] ring_buffer: Do no not complete benchmark reader too early
2015-11-04 15:29 [for-next][PATCH 00/13] tracing: Some more last minute cleanups and fixes for 4.4 Steven Rostedt
` (5 preceding siblings ...)
2015-11-04 15:29 ` [for-next][PATCH 06/13] tracing: Remove redundant TP_ARGS redefining Steven Rostedt
@ 2015-11-04 15:29 ` Steven Rostedt
2015-11-04 15:29 ` [for-next][PATCH 08/13] ring_buffer: Fix more races when terminating the producer in the benchmark Steven Rostedt
` (5 subsequent siblings)
12 siblings, 0 replies; 14+ messages in thread
From: Steven Rostedt @ 2015-11-04 15:29 UTC (permalink / raw)
To: linux-kernel; +Cc: Ingo Molnar, Andrew Morton, Petr Mladek
[-- Attachment #1: 0007-ring_buffer-Do-no-not-complete-benchmark-reader-too-.patch --]
[-- Type: text/plain, Size: 5711 bytes --]
From: Petr Mladek <pmladek@suse.com>
It seems that complete(&read_done) might be called too early
in some situations.
1st scenario:
-------------
CPU0 CPU1
ring_buffer_producer_thread()
wake_up_process(consumer);
wait_for_completion(&read_start);
ring_buffer_consumer_thread()
complete(&read_start);
ring_buffer_producer()
# producing data in
# the do-while cycle
ring_buffer_consumer();
# reading data
# got error
# set kill_test = 1;
set_current_state(
TASK_INTERRUPTIBLE);
if (reader_finish) # false
schedule();
# producer still in the middle of
# do-while cycle
if (consumer && !(cnt % wakeup_interval))
wake_up_process(consumer);
# spurious wakeup
while (!reader_finish &&
!kill_test)
# leaving because
# kill_test == 1
reader_finish = 0;
complete(&read_done);
1st BANG: We might access uninitialized "read_done" if this is the
the first round.
# producer finally leaving
# the do-while cycle because kill_test == 1;
if (consumer) {
reader_finish = 1;
wake_up_process(consumer);
wait_for_completion(&read_done);
2nd BANG: This will never complete because consumer already did
the completion.
2nd scenario:
-------------
CPU0 CPU1
ring_buffer_producer_thread()
wake_up_process(consumer);
wait_for_completion(&read_start);
ring_buffer_consumer_thread()
complete(&read_start);
ring_buffer_producer()
# CPU3 removes the module <--- difference from
# and stops producer <--- the 1st scenario
if (kthread_should_stop())
kill_test = 1;
ring_buffer_consumer();
while (!reader_finish &&
!kill_test)
# kill_test == 1 => we never go
# into the top level while()
reader_finish = 0;
complete(&read_done);
# producer still in the middle of
# do-while cycle
if (consumer && !(cnt % wakeup_interval))
wake_up_process(consumer);
# spurious wakeup
while (!reader_finish &&
!kill_test)
# leaving because kill_test == 1
reader_finish = 0;
complete(&read_done);
BANG: We are in the same "bang" situations as in the 1st scenario.
Root of the problem:
--------------------
ring_buffer_consumer() must complete "read_done" only when "reader_finish"
variable is set. It must not be skipped due to other conditions.
Note that we still must keep the check for "reader_finish" in a loop
because there might be spurious wakeups as described in the
above scenarios.
Solution:
----------
The top level cycle in ring_buffer_consumer() will finish only when
"reader_finish" is set. The data will be read in "while-do" cycle
so that they are not read after an error (kill_test == 1)
or a spurious wake up.
In addition, "reader_finish" is manipulated by the producer thread.
Therefore we add READ_ONCE() to make sure that the fresh value is
read in each cycle. Also we add the corresponding barrier
to synchronize the sleep check.
Next we set the state back to TASK_RUNNING for the situation where we
did not sleep.
Just from paranoid reasons, we initialize both completions statically.
This is safer, in case there are other races that we are unaware of.
As a side effect we could remove the memory barrier from
ring_buffer_producer_thread(). IMHO, this was the reason for
the barrier. ring_buffer_reset() uses spin locks that should
provide the needed memory barrier for using the buffer.
Link: http://lkml.kernel.org/r/1441629518-32712-2-git-send-email-pmladek@suse.com
Signed-off-by: Petr Mladek <pmladek@suse.com>
Signed-off-by: Steven Rostedt <rostedt@goodmis.org>
---
kernel/trace/ring_buffer_benchmark.c | 25 ++++++++++++++++---------
1 file changed, 16 insertions(+), 9 deletions(-)
diff --git a/kernel/trace/ring_buffer_benchmark.c b/kernel/trace/ring_buffer_benchmark.c
index a1503a027ee2..9ea7949366b3 100644
--- a/kernel/trace/ring_buffer_benchmark.c
+++ b/kernel/trace/ring_buffer_benchmark.c
@@ -24,8 +24,8 @@ struct rb_page {
static int wakeup_interval = 100;
static int reader_finish;
-static struct completion read_start;
-static struct completion read_done;
+static DECLARE_COMPLETION(read_start);
+static DECLARE_COMPLETION(read_done);
static struct ring_buffer *buffer;
static struct task_struct *producer;
@@ -178,10 +178,14 @@ static void ring_buffer_consumer(void)
read_events ^= 1;
read = 0;
- while (!reader_finish && !kill_test) {
- int found;
+ /*
+ * Continue running until the producer specifically asks to stop
+ * and is ready for the completion.
+ */
+ while (!READ_ONCE(reader_finish)) {
+ int found = 1;
- do {
+ while (found && !kill_test) {
int cpu;
found = 0;
@@ -195,17 +199,23 @@ static void ring_buffer_consumer(void)
if (kill_test)
break;
+
if (stat == EVENT_FOUND)
found = 1;
+
}
- } while (found && !kill_test);
+ }
+ /* Wait till the producer wakes us up when there is more data
+ * available or when the producer wants us to finish reading.
+ */
set_current_state(TASK_INTERRUPTIBLE);
if (reader_finish)
break;
schedule();
}
+ __set_current_state(TASK_RUNNING);
reader_finish = 0;
complete(&read_done);
}
@@ -389,13 +399,10 @@ static int ring_buffer_consumer_thread(void *arg)
static int ring_buffer_producer_thread(void *arg)
{
- init_completion(&read_start);
-
while (!kthread_should_stop() && !kill_test) {
ring_buffer_reset(buffer);
if (consumer) {
- smp_wmb();
wake_up_process(consumer);
wait_for_completion(&read_start);
}
--
2.6.1
^ permalink raw reply related [flat|nested] 14+ messages in thread* [for-next][PATCH 08/13] ring_buffer: Fix more races when terminating the producer in the benchmark
2015-11-04 15:29 [for-next][PATCH 00/13] tracing: Some more last minute cleanups and fixes for 4.4 Steven Rostedt
` (6 preceding siblings ...)
2015-11-04 15:29 ` [for-next][PATCH 07/13] ring_buffer: Do no not complete benchmark reader too early Steven Rostedt
@ 2015-11-04 15:29 ` Steven Rostedt
2015-11-04 15:29 ` [for-next][PATCH 09/13] tracing: Allow dumping traces without tracking trace started cpus Steven Rostedt
` (4 subsequent siblings)
12 siblings, 0 replies; 14+ messages in thread
From: Steven Rostedt @ 2015-11-04 15:29 UTC (permalink / raw)
To: linux-kernel; +Cc: Ingo Molnar, Andrew Morton, Petr Mladek
[-- Attachment #1: 0008-ring_buffer-Fix-more-races-when-terminating-the-prod.patch --]
[-- Type: text/plain, Size: 6042 bytes --]
From: Petr Mladek <pmladek@suse.com>
The commit b44754d8262d3aab8 ("ring_buffer: Allow to exit the ring
buffer benchmark immediately") added a hack into ring_buffer_producer()
that set @kill_test when kthread_should_stop() returned true. It improved
the situation a lot. It stopped the kthread in most cases because
the producer spent most of the time in the patched while cycle.
But there are still few possible races when kthread_should_stop()
is set outside of the cycle. Then we do not set @kill_test and
some other checks pass.
This patch adds a better fix. It renames @test_kill/TEST_KILL() into
a better descriptive @test_error/TEST_ERROR(). Also it introduces
break_test() function that checks for both @test_error and
kthread_should_stop().
The new function is used in the producer when the check for @test_error
is not enough. It is not used in the consumer because its state
is manipulated by the producer via the "reader_finish" variable.
Also we add a missing check into ring_buffer_producer_thread()
between setting TASK_INTERRUPTIBLE and calling schedule_timeout().
Otherwise, we might miss a wakeup from kthread_stop().
Link: http://lkml.kernel.org/r/1441629518-32712-3-git-send-email-pmladek@suse.com
Signed-off-by: Petr Mladek <pmladek@suse.com>
Signed-off-by: Steven Rostedt <rostedt@goodmis.org>
---
kernel/trace/ring_buffer_benchmark.c | 54 +++++++++++++++++++-----------------
1 file changed, 29 insertions(+), 25 deletions(-)
diff --git a/kernel/trace/ring_buffer_benchmark.c b/kernel/trace/ring_buffer_benchmark.c
index 9ea7949366b3..9e00fd178226 100644
--- a/kernel/trace/ring_buffer_benchmark.c
+++ b/kernel/trace/ring_buffer_benchmark.c
@@ -60,12 +60,12 @@ MODULE_PARM_DESC(consumer_fifo, "fifo prio for consumer");
static int read_events;
-static int kill_test;
+static int test_error;
-#define KILL_TEST() \
+#define TEST_ERROR() \
do { \
- if (!kill_test) { \
- kill_test = 1; \
+ if (!test_error) { \
+ test_error = 1; \
WARN_ON(1); \
} \
} while (0)
@@ -75,6 +75,11 @@ enum event_status {
EVENT_DROPPED,
};
+static bool break_test(void)
+{
+ return test_error || kthread_should_stop();
+}
+
static enum event_status read_event(int cpu)
{
struct ring_buffer_event *event;
@@ -87,7 +92,7 @@ static enum event_status read_event(int cpu)
entry = ring_buffer_event_data(event);
if (*entry != cpu) {
- KILL_TEST();
+ TEST_ERROR();
return EVENT_DROPPED;
}
@@ -115,10 +120,10 @@ static enum event_status read_page(int cpu)
rpage = bpage;
/* The commit may have missed event flags set, clear them */
commit = local_read(&rpage->commit) & 0xfffff;
- for (i = 0; i < commit && !kill_test; i += inc) {
+ for (i = 0; i < commit && !test_error ; i += inc) {
if (i >= (PAGE_SIZE - offsetof(struct rb_page, data))) {
- KILL_TEST();
+ TEST_ERROR();
break;
}
@@ -128,7 +133,7 @@ static enum event_status read_page(int cpu)
case RINGBUF_TYPE_PADDING:
/* failed writes may be discarded events */
if (!event->time_delta)
- KILL_TEST();
+ TEST_ERROR();
inc = event->array[0] + 4;
break;
case RINGBUF_TYPE_TIME_EXTEND:
@@ -137,12 +142,12 @@ static enum event_status read_page(int cpu)
case 0:
entry = ring_buffer_event_data(event);
if (*entry != cpu) {
- KILL_TEST();
+ TEST_ERROR();
break;
}
read++;
if (!event->array[0]) {
- KILL_TEST();
+ TEST_ERROR();
break;
}
inc = event->array[0] + 4;
@@ -150,17 +155,17 @@ static enum event_status read_page(int cpu)
default:
entry = ring_buffer_event_data(event);
if (*entry != cpu) {
- KILL_TEST();
+ TEST_ERROR();
break;
}
read++;
inc = ((event->type_len + 1) * 4);
}
- if (kill_test)
+ if (test_error)
break;
if (inc <= 0) {
- KILL_TEST();
+ TEST_ERROR();
break;
}
}
@@ -185,7 +190,7 @@ static void ring_buffer_consumer(void)
while (!READ_ONCE(reader_finish)) {
int found = 1;
- while (found && !kill_test) {
+ while (found && !test_error) {
int cpu;
found = 0;
@@ -197,7 +202,7 @@ static void ring_buffer_consumer(void)
else
stat = read_page(cpu);
- if (kill_test)
+ if (test_error)
break;
if (stat == EVENT_FOUND)
@@ -273,10 +278,7 @@ static void ring_buffer_producer(void)
if (cnt % wakeup_interval)
cond_resched();
#endif
- if (kthread_should_stop())
- kill_test = 1;
-
- } while (ktime_before(end_time, timeout) && !kill_test);
+ } while (ktime_before(end_time, timeout) && !break_test());
trace_printk("End ring buffer hammer\n");
if (consumer) {
@@ -297,7 +299,7 @@ static void ring_buffer_producer(void)
entries = ring_buffer_entries(buffer);
overruns = ring_buffer_overruns(buffer);
- if (kill_test && !kthread_should_stop())
+ if (test_error)
trace_printk("ERROR!\n");
if (!disable_reader) {
@@ -378,15 +380,14 @@ static void wait_to_die(void)
static int ring_buffer_consumer_thread(void *arg)
{
- while (!kthread_should_stop() && !kill_test) {
+ while (!break_test()) {
complete(&read_start);
ring_buffer_consumer();
set_current_state(TASK_INTERRUPTIBLE);
- if (kthread_should_stop() || kill_test)
+ if (break_test())
break;
-
schedule();
}
__set_current_state(TASK_RUNNING);
@@ -399,7 +400,7 @@ static int ring_buffer_consumer_thread(void *arg)
static int ring_buffer_producer_thread(void *arg)
{
- while (!kthread_should_stop() && !kill_test) {
+ while (!break_test()) {
ring_buffer_reset(buffer);
if (consumer) {
@@ -408,15 +409,18 @@ static int ring_buffer_producer_thread(void *arg)
}
ring_buffer_producer();
- if (kill_test)
+ if (break_test())
goto out_kill;
trace_printk("Sleeping for 10 secs\n");
set_current_state(TASK_INTERRUPTIBLE);
+ if (break_test())
+ goto out_kill;
schedule_timeout(HZ * SLEEP_TIME);
}
out_kill:
+ __set_current_state(TASK_RUNNING);
if (!kthread_should_stop())
wait_to_die();
--
2.6.1
^ permalink raw reply related [flat|nested] 14+ messages in thread* [for-next][PATCH 09/13] tracing: Allow dumping traces without tracking trace started cpus
2015-11-04 15:29 [for-next][PATCH 00/13] tracing: Some more last minute cleanups and fixes for 4.4 Steven Rostedt
` (7 preceding siblings ...)
2015-11-04 15:29 ` [for-next][PATCH 08/13] ring_buffer: Fix more races when terminating the producer in the benchmark Steven Rostedt
@ 2015-11-04 15:29 ` Steven Rostedt
2015-11-04 15:29 ` [for-next][PATCH 10/13] ring_buffer: Remove unneeded smp_wmb() before wakeup of reader benchmark Steven Rostedt
` (3 subsequent siblings)
12 siblings, 0 replies; 14+ messages in thread
From: Steven Rostedt @ 2015-11-04 15:29 UTC (permalink / raw)
To: linux-kernel; +Cc: Ingo Molnar, Andrew Morton, Sasha Levin
[-- Attachment #1: 0009-tracing-Allow-dumping-traces-without-tracking-trace-.patch --]
[-- Type: text/plain, Size: 1241 bytes --]
From: Sasha Levin <sasha.levin@oracle.com>
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.
Link: http://lkml.kernel.org/r/1441385156-27279-1-git-send-email-sasha.levin@oracle.com
Signed-off-by: Sasha Levin <sasha.levin@oracle.com>
Signed-off-by: Steven Rostedt <rostedt@goodmis.org>
---
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 67873c67665e..6459e141aac3 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)
--
2.6.1
^ permalink raw reply related [flat|nested] 14+ messages in thread* [for-next][PATCH 10/13] ring_buffer: Remove unneeded smp_wmb() before wakeup of reader benchmark
2015-11-04 15:29 [for-next][PATCH 00/13] tracing: Some more last minute cleanups and fixes for 4.4 Steven Rostedt
` (8 preceding siblings ...)
2015-11-04 15:29 ` [for-next][PATCH 09/13] tracing: Allow dumping traces without tracking trace started cpus Steven Rostedt
@ 2015-11-04 15:29 ` Steven Rostedt
2015-11-04 15:29 ` [for-next][PATCH 11/13] tracing: Add some documentation about set_event_pid Steven Rostedt
` (2 subsequent siblings)
12 siblings, 0 replies; 14+ messages in thread
From: Steven Rostedt @ 2015-11-04 15:29 UTC (permalink / raw)
To: linux-kernel; +Cc: Ingo Molnar, Andrew Morton
[-- Attachment #1: 0010-ring_buffer-Remove-unneeded-smp_wmb-before-wakeup-of.patch --]
[-- Type: text/plain, Size: 841 bytes --]
From: "Steven Rostedt (Red Hat)" <rostedt@goodmis.org>
wake_up_process() has a memory barrier before doing anything, thus adding a
memory barrier before calling it is redundant.
Signed-off-by: Steven Rostedt <rostedt@goodmis.org>
---
kernel/trace/ring_buffer_benchmark.c | 2 --
1 file changed, 2 deletions(-)
diff --git a/kernel/trace/ring_buffer_benchmark.c b/kernel/trace/ring_buffer_benchmark.c
index 9e00fd178226..6df9a83e20d7 100644
--- a/kernel/trace/ring_buffer_benchmark.c
+++ b/kernel/trace/ring_buffer_benchmark.c
@@ -288,8 +288,6 @@ static void ring_buffer_producer(void)
/* the completions must be visible before the finish var */
smp_wmb();
reader_finish = 1;
- /* finish var visible before waking up the consumer */
- smp_wmb();
wake_up_process(consumer);
wait_for_completion(&read_done);
}
--
2.6.1
^ permalink raw reply related [flat|nested] 14+ messages in thread* [for-next][PATCH 11/13] tracing: Add some documentation about set_event_pid
2015-11-04 15:29 [for-next][PATCH 00/13] tracing: Some more last minute cleanups and fixes for 4.4 Steven Rostedt
` (9 preceding siblings ...)
2015-11-04 15:29 ` [for-next][PATCH 10/13] ring_buffer: Remove unneeded smp_wmb() before wakeup of reader benchmark Steven Rostedt
@ 2015-11-04 15:29 ` Steven Rostedt
2015-11-04 15:29 ` [for-next][PATCH 12/13] tracing: Apply tracer specific options from kernel command line Steven Rostedt
2015-11-04 15:29 ` [for-next][PATCH 13/13] tracing: Put back comma for empty fields in boot string parsing Steven Rostedt
12 siblings, 0 replies; 14+ messages in thread
From: Steven Rostedt @ 2015-11-04 15:29 UTC (permalink / raw)
To: linux-kernel; +Cc: Ingo Molnar, Andrew Morton
[-- Attachment #1: 0011-tracing-Add-some-documentation-about-set_event_pid.patch --]
[-- Type: text/plain, Size: 1707 bytes --]
From: "Steven Rostedt (Red Hat)" <rostedt@goodmis.org>
Update Documentation to include some comments about how to use
set_event_pid.
Signed-off-by: Steven Rostedt <rostedt@goodmis.org>
---
Documentation/trace/events.txt | 18 ++++++++++++++++++
Documentation/trace/ftrace.txt | 6 ++++++
2 files changed, 24 insertions(+)
diff --git a/Documentation/trace/events.txt b/Documentation/trace/events.txt
index 75d25a1d6e42..c010be8c85d7 100644
--- a/Documentation/trace/events.txt
+++ b/Documentation/trace/events.txt
@@ -288,6 +288,24 @@ prev_pid == 0
# cat sched_wakeup/filter
common_pid == 0
+5.4 PID filtering
+-----------------
+
+The set_event_pid file in the same directory as the top events directory
+exists, will filter all events from tracing any task that does not have the
+PID listed in the set_event_pid file.
+
+# cd /sys/kernel/debug/tracing
+# echo $$ > set_event_pid
+# echo 1 > events/enabled
+
+Will only trace events for the current task.
+
+To add more PIDs without losing the PIDs already included, use '>>'.
+
+# echo 123 244 1 >> set_event_pid
+
+
6. Event triggers
=================
diff --git a/Documentation/trace/ftrace.txt b/Documentation/trace/ftrace.txt
index db18362c14f3..f52f297cb406 100644
--- a/Documentation/trace/ftrace.txt
+++ b/Documentation/trace/ftrace.txt
@@ -204,6 +204,12 @@ of ftrace. Here is a list of some of the key files:
Have the function tracer only trace a single thread.
+ set_event_pid:
+
+ Have the events only trace a task with a PID listed in this file.
+ Note, sched_switch and sched_wake_up will also trace events
+ listed in this file.
+
set_graph_function:
Set a "trigger" function where tracing should start
--
2.6.1
^ permalink raw reply related [flat|nested] 14+ messages in thread* [for-next][PATCH 12/13] tracing: Apply tracer specific options from kernel command line.
2015-11-04 15:29 [for-next][PATCH 00/13] tracing: Some more last minute cleanups and fixes for 4.4 Steven Rostedt
` (10 preceding siblings ...)
2015-11-04 15:29 ` [for-next][PATCH 11/13] tracing: Add some documentation about set_event_pid Steven Rostedt
@ 2015-11-04 15:29 ` Steven Rostedt
2015-11-04 15:29 ` [for-next][PATCH 13/13] tracing: Put back comma for empty fields in boot string parsing Steven Rostedt
12 siblings, 0 replies; 14+ messages in thread
From: Steven Rostedt @ 2015-11-04 15:29 UTC (permalink / raw)
To: linux-kernel; +Cc: Ingo Molnar, Andrew Morton, Jiaxing Wang
[-- Attachment #1: 0012-tracing-Apply-tracer-specific-options-from-kernel-co.patch --]
[-- Type: text/plain, Size: 4030 bytes --]
From: Jiaxing Wang <hello.wjx@gmail.com>
Currently, the trace_options parameter is only applied in
tracer_alloc_buffers() when global_trace.current_trace is nop_trace,
so a tracer specific option will not be applied even when the specific
tracer is also enabled from kernel command line. For example, the
'func_stack_trace' option can't be enabled with the following kernel
parameter:
ftrace=function ftrace_filter=kfree trace_options=func_stack_trace
We can enable tracer specific options by simply apply the options again
if the specific tracer is also supplied from command line and started
in register_tracer().
To make trace_boot_options_buf can be parsed again, a comma and a space
is put back if they were replaced by strsep and strstrip respectively.
Also make register_tracer() be __init to access the __init data, and
in fact register_tracer is only called from __init code.
Link: http://lkml.kernel.org/r/1446599669-9294-1-git-send-email-hello.wjx@gmail.com
Signed-off-by: Jiaxing Wang <hello.wjx@gmail.com>
Signed-off-by: Steven Rostedt <rostedt@goodmis.org>
---
kernel/trace/trace.c | 45 ++++++++++++++++++++++++++++++++++++---------
1 file changed, 36 insertions(+), 9 deletions(-)
diff --git a/kernel/trace/trace.c b/kernel/trace/trace.c
index 6459e141aac3..7fe7cc987dab 100644
--- a/kernel/trace/trace.c
+++ b/kernel/trace/trace.c
@@ -214,12 +214,10 @@ __setup("alloc_snapshot", boot_alloc_snapshot);
static char trace_boot_options_buf[MAX_TRACER_SIZE] __initdata;
-static char *trace_boot_options __initdata;
static int __init set_trace_boot_options(char *str)
{
strlcpy(trace_boot_options_buf, str, MAX_TRACER_SIZE);
- trace_boot_options = trace_boot_options_buf;
return 0;
}
__setup("trace_options=", set_trace_boot_options);
@@ -1223,13 +1221,15 @@ static inline int run_tracer_selftest(struct tracer *type)
static void add_tracer_options(struct trace_array *tr, struct tracer *t);
+static void __init apply_trace_boot_options(void);
+
/**
* register_tracer - register a tracer with the ftrace system.
* @type - the plugin for the tracer
*
* Register a new plugin tracer.
*/
-int register_tracer(struct tracer *type)
+int __init register_tracer(struct tracer *type)
{
struct tracer *t;
int ret = 0;
@@ -1288,6 +1288,9 @@ int register_tracer(struct tracer *type)
/* Do we want this tracer to start on bootup? */
tracing_set_tracer(&global_trace, type->name);
default_bootup_tracer = NULL;
+
+ apply_trace_boot_options();
+
/* disable other selftests, since this will break it. */
tracing_selftest_disabled = true;
#ifdef CONFIG_FTRACE_STARTUP_TEST
@@ -3589,6 +3592,7 @@ static int trace_set_options(struct trace_array *tr, char *option)
int neg = 0;
int ret = -ENODEV;
int i;
+ size_t orig_len = strlen(option);
cmp = strstrip(option);
@@ -3612,9 +3616,37 @@ static int trace_set_options(struct trace_array *tr, char *option)
mutex_unlock(&trace_types_lock);
+ /*
+ * If the first trailing whitespace is replaced with '\0' by strstrip,
+ * turn it back into a space.
+ */
+ if (orig_len > strlen(option))
+ option[strlen(option)] = ' ';
+
return ret;
}
+static void __init apply_trace_boot_options(void)
+{
+ char *buf = trace_boot_options_buf;
+ char *option;
+
+ while (true) {
+ option = strsep(&buf, ",");
+
+ if (!option)
+ break;
+ if (!*option)
+ continue;
+
+ trace_set_options(&global_trace, option);
+
+ /* Put back the comma to allow this to be called again */
+ if (buf)
+ *(buf - 1) = ',';
+ }
+}
+
static ssize_t
tracing_trace_options_write(struct file *filp, const char __user *ubuf,
size_t cnt, loff_t *ppos)
@@ -7248,12 +7280,7 @@ __init static int tracer_alloc_buffers(void)
INIT_LIST_HEAD(&global_trace.events);
list_add(&global_trace.list, &ftrace_trace_arrays);
- while (trace_boot_options) {
- char *option;
-
- option = strsep(&trace_boot_options, ",");
- trace_set_options(&global_trace, option);
- }
+ apply_trace_boot_options();
register_snapshot_cmd();
--
2.6.1
^ permalink raw reply related [flat|nested] 14+ messages in thread* [for-next][PATCH 13/13] tracing: Put back comma for empty fields in boot string parsing
2015-11-04 15:29 [for-next][PATCH 00/13] tracing: Some more last minute cleanups and fixes for 4.4 Steven Rostedt
` (11 preceding siblings ...)
2015-11-04 15:29 ` [for-next][PATCH 12/13] tracing: Apply tracer specific options from kernel command line Steven Rostedt
@ 2015-11-04 15:29 ` Steven Rostedt
12 siblings, 0 replies; 14+ messages in thread
From: Steven Rostedt @ 2015-11-04 15:29 UTC (permalink / raw)
To: linux-kernel; +Cc: Ingo Molnar, Andrew Morton
[-- Attachment #1: 0013-tracing-Put-back-comma-for-empty-fields-in-boot-stri.patch --]
[-- Type: text/plain, Size: 2366 bytes --]
From: "Steven Rostedt (Red Hat)" <rostedt@goodmis.org>
Both early_enable_events() and apply_trace_boot_options() parse a boot
string that may get parsed later on. They both use strsep() which converts a
comma into a nul character. To still allow the boot string to be parsed
again the same way, the nul character gets converted back to a comma after
the token is processed.
The problem is that these two functions check for an empty parameter (two
commas in a row ",,"), and continue the loop if the parameter is empty, but
fails to place the comma back. In this case, the second parsing will end at
this blank field, and not process fields afterward.
In most cases, users should not have an empty field, but if its going to be
checked, the code might as well be correct.
Signed-off-by: Steven Rostedt <rostedt@goodmis.org>
---
kernel/trace/trace.c | 5 ++---
kernel/trace/trace_events.c | 16 ++++++++--------
2 files changed, 10 insertions(+), 11 deletions(-)
diff --git a/kernel/trace/trace.c b/kernel/trace/trace.c
index 7fe7cc987dab..2198a630ef58 100644
--- a/kernel/trace/trace.c
+++ b/kernel/trace/trace.c
@@ -3636,10 +3636,9 @@ static void __init apply_trace_boot_options(void)
if (!option)
break;
- if (!*option)
- continue;
- trace_set_options(&global_trace, option);
+ if (*option)
+ trace_set_options(&global_trace, option);
/* Put back the comma to allow this to be called again */
if (buf)
diff --git a/kernel/trace/trace_events.c b/kernel/trace/trace_events.c
index 292bccf3e011..bee1e1530052 100644
--- a/kernel/trace/trace_events.c
+++ b/kernel/trace/trace_events.c
@@ -3042,16 +3042,16 @@ early_enable_events(struct trace_array *tr, bool disable_first)
if (!token)
break;
- if (!*token)
- continue;
- /* Restarting syscalls requires that we stop them first */
- if (disable_first)
- ftrace_set_clr_event(tr, token, 0);
+ if (*token) {
+ /* Restarting syscalls requires that we stop them first */
+ if (disable_first)
+ ftrace_set_clr_event(tr, token, 0);
- ret = ftrace_set_clr_event(tr, token, 1);
- if (ret)
- pr_warn("Failed to enable trace event: %s\n", token);
+ ret = ftrace_set_clr_event(tr, token, 1);
+ if (ret)
+ pr_warn("Failed to enable trace event: %s\n", token);
+ }
/* Put back the comma to allow this to be called again */
if (buf)
--
2.6.1
^ permalink raw reply related [flat|nested] 14+ messages in thread