Messages in this thread Patch in this message | | | Date | Fri, 15 Jun 2012 00:47:38 +0200 (CEST) | From | Thomas Gleixner <> | Subject | [PATCH] tracing: Fix the outmost stupidity of tracing_off() |
| |
I lost another day of waiting for a complex bug to trigger and turn off tracing, which did not happen due to:
commit 499e54705(tracing/ring-buffer: Only have tracing_on disable tracing buffers)
The subject line itself is supsicious itself:
Only have tracing_on disable tracing buffers
tracing_on as far as I'm concerned is supposed to start tracing. So disabling trace buffers on tracing_on is counterproductive.
The patch description is not really a better source of enlightment:
As the ring-buffer code is being used by other facilities in the kernel, having tracing_on file disable *all* buffers is not a desired affect. It should only disable the ftrace buffers that are being used.
Move the code into the trace.c file and use the buffer disabling for tracing_on() and tracing_off(). This way only the ftrace buffers will be affected by them and other kernel utilities will not be confused to why their output suddenly stopped.
How should the casual reader of this commit message figure out that tracing_on is a debugfs file which accepts 0 resp. 1 to it, where 0 causes the buffers to be disabled?
Yes, it's obvious to the patch author, but ....
While I account that to the usual inability to write proper changelogs, I'm amazed about the following parts of the patch
-void tracing_off(void) -{ - clear_bit(RB_BUFFERS_ON_BIT, &ring_buffer_flags); -}
...
+void tracing_off(void) +{ + if (global_trace.buffer) + ring_buffer_record_on(global_trace.buffer);
This is so amazingly stupid, that it's not even funny anymore.
Is it really too much to expect that a patch of the size
4 files changed, 171 insertions(+), 97 deletions(-)
which touches fundamental functionality of a subsystem is at least tested to maintain the previous functionality?
Signed-off-by: Thomas [royally annoyed] Gleixner <tglx@linutronix.de>
--- kernel/trace/trace.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
Index: linux-2.6/kernel/trace/trace.c =================================================================== --- linux-2.6.orig/kernel/trace/trace.c +++ linux-2.6/kernel/trace/trace.c @@ -371,7 +371,7 @@ EXPORT_SYMBOL_GPL(tracing_on); void tracing_off(void) { if (global_trace.buffer) - ring_buffer_record_on(global_trace.buffer); + ring_buffer_record_off(global_trace.buffer); /* * This flag is only looked at when buffers haven't been * allocated yet. We don't really care about the race
| |