Skip to contentSkip to conversation

ring-buffer: disable tracing switch

PATCH In review Steven Rostedt 5 participants 9 messages Nov 11, 2008 – Nov 12, 2008

Subsystems (4)
lore
Share & tools
lore

Steven Rostedt

author maintainer
#0 lore raw

cover ring-buffer: disable tracing switch

Stephen and Ingo,

I've been pressured into making this patch for mainline. It is a low risk patch, but I would like to get it into linux-next.

This patch adds a way for a kernel developer to stop all tracers from within the kernel and restart it from userspace. The previous APIs that allowed for kernel stopping the tracer did not allow for userspace to restart. This was mainly because the stopping could be for areas that are dangerous to be tracing, and allowing userspace to start them could risk the system stability.

This switch is made to let userspace interact with the kernel space in starting and stopping tracers.

The following patches are in:

git://git.kernel.org/pub/scm/linux/kernel/git/rostedt/linux-2.6-trace.git

branch: devel

Steven Rostedt (1):
ring-buffer: buffer record on/off switch

----

 include/linux/ring_buffer.h |    3 +
 kernel/trace/ftrace.c       |    8 +---
 kernel/trace/ring_buffer.c  |  101 +++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 106 insertions(+), 6 deletions(-)
author maintainer
#1 lore raw

PATCH 1/1 ring-buffer: buffer record on/off switch

Impact: enable/disable ring buffer recording API added

Several kernel developers have requested that there be a way to stop recording into the ring buffers with a simple switch that can also be enabled from userspace. This patch addes a new kernel API to the ring buffers called:

tracing_on()
tracing_off()

When tracing_off() is called, all ring buffers will not be able to record into their buffers.

tracing_on() will enable the ring buffers again.

These two act like an on/off switch. That is, there is no counting of the number of times tracing_off or tracing_on has been called.

A new file is added to the debugfs/tracing directory called

tracing_on

This allows for userspace applications to also flip the switch.

echo 0 > debugfs/tracing/tracing_on

disables the tracing.

echo 1 > /debugfs/tracing/tracing_on

enables it.

Note, this does not disable or enable any tracers. It only sets or clears a flag that needs to be set in order for the ring buffers to write to their buffers. It is a global flag, and affects all ring buffers.

The buffers start out with tracing_on enabled.

There are now three flags that control recording into the buffers:

tracing_on: which affects all ring buffer tracers.

buffer->record_disabled: which affects an allocated buffer, which may be set
if an anomaly is detected, and tracing is disabled.

cpu_buffer->record_disabled: which is set by tracing_stop() or if an
anomaly is detected. tracing_start can not reenable this if an anomaly occurred.

The userspace debugfs/tracing/tracing_enabled is implemented with tracing_stop() but the user space code can not enable it if the kernel called tracing_stop().

Userspace can enable the tracing_on even if the kernel disabled it. It is just a switch used to stop tracing if a condition was hit. tracing_on is not for protecting critical areas in the kernel nor is it for stopping tracing if an anomaly occurred. This is because userspace can reenable it at any time.

Side effect: With this patch, I discovered a dead variable in ftrace.c
called tracing_on. This patch removes it.

---
 include/linux/ring_buffer.h |    3 +
 kernel/trace/ftrace.c       |    8 +---
 kernel/trace/ring_buffer.c  |  101 +++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 106 insertions(+), 6 deletions(-)
include/linux/ring_buffer.h +3 -0
index 536b0ca..e097c2e 100644
@@ -120,6 +120,9 @@ unsigned long ring_buffer_overruns(struct ring_buffer *buffer);
 u64 ring_buffer_time_stamp(int cpu);
 void ring_buffer_normalize_time_stamp(int cpu, u64 *ts);
 
+void tracing_on(void);
+void tracing_off(void);
+
 enum ring_buffer_flags {
 	RB_FL_OVERWRITE		= 1 << 0,
 };
kernel/trace/ftrace.c +2 -6
index 4a39d24..14fa522 100644
@@ -185,7 +185,6 @@ enum {
 };
 
 static int ftrace_filtered;
-static int tracing_on;
 
 static LIST_HEAD(ftrace_new_addrs);
 
@@ -506,13 +505,10 @@ static int __ftrace_modify_code(void *data)
 {
 	int *command = data;
 
-	if (*command & FTRACE_ENABLE_CALLS) {
+	if (*command & FTRACE_ENABLE_CALLS)
 		ftrace_replace_code(1);
-		tracing_on = 1;
-	} else if (*command & FTRACE_DISABLE_CALLS) {
+	else if (*command & FTRACE_DISABLE_CALLS)
 		ftrace_replace_code(0);
-		tracing_on = 0;
-	}
 
 	if (*command & FTRACE_UPDATE_TRACE_FUNC)
 		ftrace_update_ftrace_func(ftrace_trace_function);
kernel/trace/ring_buffer.c +101 -0

122 lines not on this page · raw message

Signature · 3 lines
1.5.6.5

-- 
#2 lore raw

Re: [PATCH 1/1] ring-buffer: buffer record on/off switch

Quoted 1 line · On Tue, 11 Nov 2008 15:06:02 -0500 Steven Rostedt <[email protected]> wrote:

+static struct file_operations rb_simple_fops = {

should be const...

Probably it isn't worth fixing. Someone(tm) should do period sweeps - 10% of our file_operationses aren't const.

author maintainer
#3 lore raw

Re: [PATCH 1/1] ring-buffer: buffer record on/off switch

Quoted 9 lines · Andrew Morton
Quoted 1 line · On Tue, 11 Nov 2008 15:06:02 -0500 Steven Rostedt <[email protected]> wrote:

+static struct file_operations rb_simple_fops = {

should be const...

Probably it isn't worth fixing. Someone(tm) should do period sweeps - 10% of our file_operationses aren't const.

Thanks, I never really thought about it.

I'll add to my todo list to send out a patch that cleans all of this up. I think I'm guilty at doing this in more than one place.

-- Steve

#4 lore raw

Re: [PATCH 1/1] ring-buffer: buffer record on/off switch

Quoted 10 lines · Steven Rostedt
Quoted 7 lines · Andrew Morton
Quoted 1 line · On Tue, 11 Nov 2008 15:06:02 -0500 Steven Rostedt <[email protected]> wrote:

+static struct file_operations rb_simple_fops = {

should be const...

Quoted 7 lines
Quoted 2 lines

Probably it isn't worth fixing. Someone(tm) should do period sweeps - 10% of our file_operationses aren't const.

Thanks, I never really thought about it.

I'll add to my todo list to send out a patch that cleans all of this up. I think I'm guilty at doing this in more than one place.

Perhaps checkpatch.pl could remind us about it? a new file_operations struct definition should be const in 99.9% of the cases.

Ingo

#5 lore raw

Re: [PATCH 1/1] ring-buffer: buffer record on/off switch

Quoted 16 lines · Ingo Molnar
Quoted 13 lines · Steven Rostedt
Quoted 9 lines · Andrew Morton
Quoted 1 line · On Tue, 11 Nov 2008 15:06:02 -0500 Steven Rostedt <[email protected]> wrote:

+static struct file_operations rb_simple_fops = {

should be const...

Probably it isn't worth fixing. Someone(tm) should do period sweeps - 10% of our file_operationses aren't const.

Quoted 7 lines
Quoted 4 lines

Thanks, I never really thought about it.

I'll add to my todo list to send out a patch that cleans all of this up. I think I'm guilty at doing this in more than one place.

Perhaps checkpatch.pl could remind us about it? a new file_operations struct definition should be const in 99.9% of the cases.

Added to my TODO list.

-apw

author maintainer
#6 lore raw

Re: [PATCH 1/1] ring-buffer: buffer record on/off switch

In reply to Andrew Morton

[ Added Andy on Cc ]

Quoted 9 lines · Andrew Morton
Quoted 1 line · On Tue, 11 Nov 2008 15:06:02 -0500 Steven Rostedt <[email protected]> wrote:

+static struct file_operations rb_simple_fops = {

should be const...

Probably it isn't worth fixing. Someone(tm) should do period sweeps - 10% of our file_operationses aren't const.

I wonder if this should be added to checkpatch?

-- Steve

#7 lore raw

Re: [PATCH 1/1] ring-buffer: buffer record on/off switch

In reply to Andrew Morton
Quoted 9 lines · Andrew Morton
Quoted 1 line · On Tue, 11 Nov 2008 15:06:02 -0500 Steven Rostedt <[email protected]> wrote:

+static struct file_operations rb_simple_fops = {

should be const...

Probably it isn't worth fixing. Someone(tm) should do period sweeps - 10% of our file_operationses aren't const.

checkptch rule? struct file_operations .* = { -> needs const?

Signature · 2 lines
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
#8 lore raw
In reply to Steven Rostedt
Quoted 4 lines · Steven Rostedt

Stephen and Ingo,

I've been pressured into making this patch for mainline. It is a low risk patch, but I would like to get it into linux-next.

pulled, and pushed out into tip/auto-ftrace-next, thanks Steve.

Ingo