ftrace updates for 2.6.28
Subsystems (3)
Steven Rostedt
maintainercover ftrace updates for 2.6.28
Stephen and Ingo,
The following patches are needed for 2.6.28. The first one solves a bug where the resizing of the buffer array was not protected against modifications by ftrace.
The second patch solves an issue tha Thomas Gleixner was seeing where the tracer would detect the infinite recursion too easily by tracing the timer code. This patch prevents the loop from happening and still does not lose any trace entries.
The changes caused by these patches are low in risk, but I would still like to put them through linux-next before passing them off to Linus.
Note, these changes are based against Linus's mainline and not tip. Also note that the branch in this tree is "devel" and not "tip/devel".
The following patches are in:
git://git.kernel.org/pub/scm/linux/kernel/git/rostedt/linux-2.6-trace.git
branch: devel
Steven Rostedt (2):
ftrace: disable tracing on resize
ring-buffer: prevent infinite looping on time stamping
----
kernel/trace/ring_buffer.c | 2 +- kernel/trace/trace.c | 17 ++++++++++++++++- 2 files changed, 17 insertions(+), 2 deletions(-)
Steven Rostedt
maintainerPATCH 1/2 ftrace: disable tracing on resize
Impact: fix for bug on resize
This patch addresses the bug found here:
http://bugzilla.kernel.org/show_bug.cgi?id=11996
When ftrace converted to the new unified trace buffer, the resizing of the buffer was not protected as much as it was originally. If tracing is performed while the resize occurs, then the buffer can be corrupted.
This patch disables all ftrace buffer modifications before a resize takes place.
--- kernel/trace/trace.c | 17 ++++++++++++++++- 1 files changed, 16 insertions(+), 1 deletions(-)
kernel/trace/trace.c +16 -1
@@ -2676,7 +2676,7 @@ tracing_entries_write(struct file *filp, const char __user *ubuf, {unsigned long val;char buf[64 ]; -int ret; +int ret, cpu;struct trace_array *tr = filp->private_data;if (cnt >=sizeof (buf))
@@ -2704,6 +2704,14 @@ tracing_entries_write(struct file *filp, const char __user *ubuf, goto out; } +/* disable all cpu buffers */ +for_each_tracing_cpu (cpu) { +if (global_trace.data[cpu]) +atomic_inc (&global_trace.data[cpu]->disabled); +if (max_tr.data[cpu]) +atomic_inc (&max_tr.data[cpu]->disabled); + } +if (val != global_trace.entries) { ret =ring_buffer_resize (global_trace.buffer, val);if (ret <0 ) {
@@ -2735,6 +2743,13 @@ tracing_entries_write(struct file *filp, const char __user *ubuf,if (tracing_disabled) cnt = -ENOMEM ; out: +for_each_tracing_cpu (cpu) { +if (global_trace.data[cpu]) +atomic_dec (&global_trace.data[cpu]->disabled); +if (max_tr.data[cpu]) +atomic_dec (&max_tr.data[cpu]->disabled); + } + max_tr.entries = global_trace.entries;mutex_unlock (&trace_types_lock);
Signature · 3 lines
1.5.6.5 --
Steven Rostedt
maintainerRe: [PATCH 1/2] ftrace: disable tracing on resize
Pekka,
I forgot to add you to the Cc list. Could you test this patch to see if it fixes the bug you reported. It fixed a bug I was able to reproduce, but I was never able to reproduce the exact same bug you have seen.
-- Steve
Quoted 66 lines · Steven Rostedt
Impact: fix for bug on resize
This patch addresses the bug found here:
http://bugzilla.kernel.org/show_bug.cgi?id=11996
When ftrace converted to the new unified trace buffer, the resizing of the buffer was not protected as much as it was originally. If tracing is performed while the resize occurs, then the buffer can be corrupted.
This patch disables all ftrace buffer modifications before a resize takes place.
Signed-off-by Steven Rostedt <[email protected]>--- kernel/trace/trace.c | 17 ++++++++++++++++- 1 files changed, 16 insertions(+), 1 deletions(-)kernel/trace/trace.c +16
-1@@ -2676,7 +2676,7 @@ tracing_entries_write(struct file *filp, const char __user *ubuf, {unsigned long val;char buf[64 ]; -int ret; +int ret, cpu;struct trace_array *tr = filp->private_data;if (cnt >=sizeof (buf))@@ -2704,6 +2704,14 @@ tracing_entries_write(struct file *filp, const char __user *ubuf, goto out; } +/* disable all cpu buffers */ +for_each_tracing_cpu (cpu) { +if (global_trace.data[cpu]) +atomic_inc (&global_trace.data[cpu]->disabled); +if (max_tr.data[cpu]) +atomic_inc (&max_tr.data[cpu]->disabled); + } +if (val != global_trace.entries) { ret =ring_buffer_resize (global_trace.buffer, val);if (ret <0 ) {@@ -2735,6 +2743,13 @@ tracing_entries_write(struct file *filp, const char __user *ubuf,if (tracing_disabled) cnt = -ENOMEM ; out: +for_each_tracing_cpu (cpu) { +if (global_trace.data[cpu]) +atomic_dec (&global_trace.data[cpu]->disabled); +if (max_tr.data[cpu]) +atomic_dec (&max_tr.data[cpu]->disabled); + } + max_tr.entries = global_trace.entries;mutex_unlock (&trace_types_lock);Signature · 3 lines
1.5.6.5 --
Re: [PATCH 1/2] ftrace: disable tracing on resize
Quoted 6 lines · On Mon, 10 Nov 2008 22:18:19 -0500 (EST) Steven Rostedt <[email protected]>…
Pekka,
I forgot to add you to the Cc list. Could you test this patch to see if it fixes the bug you reported. It fixed a bug I was able to reproduce, but I was never able to reproduce the exact same bug you have seen.
Thanks, Steve.
I didn't have time to try it myself yet, but a kind soul tried it for me. He used 2.6.28-rc4 and was immediately able to trigger the bug by just:
cd /debug/tracing
echo 0 > tracing_enabled
echo 200 > trace_entries
Then he applied the patch you referred to, and the bug is still present. He has a 32-bit x86 amd system, and I'm on x86_64.
I don't know why you couldn't reproduce it, maybe you have all the tracers enabled in the kernel config, when we only have MMIOTRACE.
I can't promise to look into this deeper in the near future :-/
Quoted 64 lines · Steven Rostedt
Quoted 63 lines
Impact: fix for bug on resize
This patch addresses the bug found here:
http://bugzilla.kernel.org/show_bug.cgi?id=11996
When ftrace converted to the new unified trace buffer, the resizing of the buffer was not protected as much as it was originally. If tracing is performed while the resize occurs, then the buffer can be corrupted.
This patch disables all ftrace buffer modifications before a resize takes place.
Signed-off-by Steven Rostedt <[email protected]>--- kernel/trace/trace.c | 17 ++++++++++++++++- 1 files changed, 16 insertions(+), 1 deletions(-)kernel/trace/trace.c +16
-1@@ -2676,7 +2676,7 @@ tracing_entries_write(struct file *filp, const char __user *ubuf, {unsigned long val;char buf[64 ]; -int ret; +int ret, cpu;struct trace_array *tr = filp->private_data;if (cnt >=sizeof (buf))@@ -2704,6 +2704,14 @@ tracing_entries_write(struct file *filp, const char __user *ubuf, goto out; } +/* disable all cpu buffers */ +for_each_tracing_cpu (cpu) { +if (global_trace.data[cpu]) +atomic_inc (&global_trace.data[cpu]->disabled); +if (max_tr.data[cpu]) +atomic_inc (&max_tr.data[cpu]->disabled); + } +if (val != global_trace.entries) { ret =ring_buffer_resize (global_trace.buffer, val);if (ret <0 ) {@@ -2735,6 +2743,13 @@ tracing_entries_write(struct file *filp, const char __user *ubuf,if (tracing_disabled) cnt = -ENOMEM ; out: +for_each_tracing_cpu (cpu) { +if (global_trace.data[cpu]) +atomic_dec (&global_trace.data[cpu]->disabled); +if (max_tr.data[cpu]) +atomic_dec (&max_tr.data[cpu]->disabled); + } + max_tr.entries = global_trace.entries;mutex_unlock (&trace_types_lock);Signature · 1 line
1.5.6.5
Signature · 2 lines
Pekka Paalanen http://www.iki.fi/pq/
Steven Rostedt
maintainerRe: [PATCH 1/2] ftrace: disable tracing on resize
Quoted 18 lines · Pekka Paalanen
Quoted 6 lines · On Mon, 10 Nov 2008 22:18:19 -0500 (EST) Steven Rostedt <[email protected]>…
Pekka,
I forgot to add you to the Cc list. Could you test this patch to see if it fixes the bug you reported. It fixed a bug I was able to reproduce, but I was never able to reproduce the exact same bug you have seen.
Thanks, Steve.
I didn't have time to try it myself yet, but a kind soul tried it for me. He used 2.6.28-rc4 and was immediately able to trigger the bug by just:
cd /debug/tracing
echo 0 >tracing_enabled
echo 200 >trace_entries
Quoted 8 lines
Then he applied the patch you referred to, and the bug is still present. He has a 32-bit x86 amd system, and I'm on
x86_64.I don't know why you couldn't reproduce it, maybe you have all the tracers enabled in the kernel config, when we only have MMIOTRACE.
I can't promise to look into this deeper in the near future :-/
OK, I'll disable everything but MMIOTRACE and see if I can reproduce it.
Thanks,
-- Steve
Steven Rostedt
maintainerRe: [PATCH 1/2] ftrace: disable tracing on resize
In reply to Pekka PaalanenQuoted 18 lines · Pekka Paalanen
Quoted 6 lines · On Mon, 10 Nov 2008 22:18:19 -0500 (EST) Steven Rostedt <[email protected]>…
Pekka,
I forgot to add you to the Cc list. Could you test this patch to see if it fixes the bug you reported. It fixed a bug I was able to reproduce, but I was never able to reproduce the exact same bug you have seen.
Thanks, Steve.
I didn't have time to try it myself yet, but a kind soul tried it for me. He used 2.6.28-rc4 and was immediately able to trigger the bug by just:
cd /debug/tracing
echo 0 >tracing_enabled
echo 200 >trace_entries
Quoted 5 lines
Then he applied the patch you referred to, and the bug is still present. He has a 32-bit x86 amd system, and I'm on
x86_64.I don't know why you couldn't reproduce it, maybe you have all the tracers enabled in the kernel config, when we only have MMIOTRACE.
After disabling all other tracers, you are right. This sequence crashes.
I'll look into it, and I should have a patch sometime tonight.
-- Steve
Steven Rostedt
maintainerPATCH 2/2 ring-buffer: prevent infinite looping on time stamping
In reply to Steven RostedtImpact: removal of unnecessary looping
The lockless part of the ring buffer allows for reentry into the code from interrupts. A timestamp is taken, a test is preformed and if it detects that an interrupt occurred that did tracing, it tries again.
The problem arises if the timestamp code itself causes a trace. The detection will detect this and loop again. The difference between this and an interrupt doing tracing, is that this will fail every time, and cause an infinite loop.
Currently, we test if the loop happens 1000 times, and if so, it will produce a warning and disable the ring buffer.
The problem with this approach is that it makes it difficult to perform some types of tracing (tracing the timestamp code itself).
Each trace entry has a delta timestamp from the previous entry. If a trace entry is reserved but and interrupt occurs and traces before the previous entry is commited, the delta timestamp for that entry will be zero. This actually makes sense in terms of tracing, because the interrupt entry happened before the preempted entry was commited, so one may consider the two happening at the same time. The order is still preserved in the buffer.
With this idea, instead of trying to get a new timestamp if an interrupt made it in between the timestamp and the test, the entry could simply make the delta zero and continue. This will prevent interrupts or tracers in the timer code from causing the above loop.
--- kernel/trace/ring_buffer.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-)
kernel/trace/ring_buffer.c +1 -1
@@ -1060,7 +1060,7 @@ rb_reserve_next_event(struct ring_buffer_per_cpu *cpu_buffer,/* Did the write stamp get updated already? */ if (unlikely (ts < cpu_buffer->write_stamp)) - goto again; + delta =0 ;if (test_time_stamp (delta)) {
Signature · 3 lines
1.5.6.5 --
Steven Rostedt
maintainerQuoted 12 lines · Steven Rostedt
The following patches are in:
git://git.kernel.org/pub/scm/linux/kernel/git/rostedt/linux-2.6-trace.gitbranch: devel
Steven Rostedt (2):
ftrace: disable tracing on resize
ring-buffer: prevent infinite looping on time stamping
Ingo,
I just pushed these two patches to tip/devel after solving a conflict with the first one.
-- Steve
Quoted 6 lines · Steven Rostedt
The following patches are in:
git://git.kernel.org/pub/scm/linux/kernel/git/rostedt/linux-2.6-trace.gitbranch: devel
Quoted 8 lines
Steven Rostedt (2):
ftrace: disable tracing on resize
ring-buffer: prevent infinite looping on time stamping----
kernel/trace/ring_buffer.c | 2 +- kernel/trace/trace.c | 17 ++++++++++++++++- 2 files changed, 17 insertions(+), 2 deletions(-)
pulled into tip/tracing/urgent, thanks Steve!
Quoted 9 lines
The following patches are needed for 2.6.28. The first one solves a bug where the resizing of the buffer array was not protected against modifications by ftrace.
The second patch solves an issue tha Thomas Gleixner was seeing where the tracer would detect the infinite recursion too easily by tracing the timer code. This patch prevents the loop from happening and still does not lose any trace entries.
Quoted 7 lines
The changes caused by these patches are low in risk, but I would still like to put them through linux-next before passing them off to Linus.
Note, these changes are based against Linus's mainline and not tip. Also note that the branch in this tree is "devel" and not "tip/devel".
that's OK. I resolved the (trivial) conflict with pending changes in tip/tracing/ftrace and pushed out a new ftrace integration branch towards linux-next.
Ingo