Re: PATCH? trace_remove_event_call() should fail if call is active

[Steven Rostedt]

On Tue, 2013-07-02 at 17:04 -0400, Steven Rostedt wrote:

> But what if we simply add a new event file flag called
> FTRACE_EVENT_FL_SHUTDOWN. Which will not allow the event to be enabled
> anymore. What do you think about this patch?

Would be helpful to try to at least compile the patch ;-)

This one has been complied *and* tested.

-- Steve

Index: linux-trace.git/include/linux/ftrace_event.h
===================================================================
--- linux-trace.git.orig/include/linux/ftrace_event.h
+++ linux-trace.git/include/linux/ftrace_event.h
@@ -253,6 +253,7 @@ enum {
 	FTRACE_EVENT_FL_RECORDED_CMD_BIT,
 	FTRACE_EVENT_FL_SOFT_MODE_BIT,
 	FTRACE_EVENT_FL_SOFT_DISABLED_BIT,
+	FTRACE_EVENT_FL_SHUTDOWN_BIT,
 };
 
 /*
@@ -262,12 +263,15 @@ enum {
  *  SOFT_MODE     - The event is enabled/disabled by SOFT_DISABLED
  *  SOFT_DISABLED - When set, do not trace the event (even though its
  *                   tracepoint may be enabled)
+ *  SHUTDOWN      - The event is going away, don't allow it to be enabled
+ *                  anymore.
  */
 enum {
 	FTRACE_EVENT_FL_ENABLED		= (1 << FTRACE_EVENT_FL_ENABLED_BIT),
 	FTRACE_EVENT_FL_RECORDED_CMD	= (1 << FTRACE_EVENT_FL_RECORDED_CMD_BIT),
 	FTRACE_EVENT_FL_SOFT_MODE	= (1 << FTRACE_EVENT_FL_SOFT_MODE_BIT),
 	FTRACE_EVENT_FL_SOFT_DISABLED	= (1 << FTRACE_EVENT_FL_SOFT_DISABLED_BIT),
+	FTRACE_EVENT_FL_SHUTDOWN	= (1 << FTRACE_EVENT_FL_SHUTDOWN_BIT),
 };
 
 struct ftrace_event_file {
Index: linux-trace.git/kernel/trace/trace.h
===================================================================
--- linux-trace.git.orig/kernel/trace/trace.h
+++ linux-trace.git/kernel/trace/trace.h
@@ -229,6 +229,9 @@ extern struct mutex trace_types_lock;
 extern int trace_array_get(struct trace_array *tr);
 extern void trace_array_put(struct trace_array *tr);
 
+extern void ftrace_event_shutdown(struct ftrace_event_call *call);
+extern void ftrace_event_restart(struct ftrace_event_call *call);
+
 /*
  * The global tracer (top) should be the first trace array added,
  * but we check the flag anyway.
Index: linux-trace.git/kernel/trace/trace_events.c
===================================================================
--- linux-trace.git.orig/kernel/trace/trace_events.c
+++ linux-trace.git/kernel/trace/trace_events.c
@@ -299,6 +299,13 @@ static int __ftrace_event_enable_disable
 		break;
 	case 1:
 		/*
+		 * If the event is set to go away, do not allow anyone to
+		 * enable it anymore.
+		 */
+		if (file->flags & FTRACE_EVENT_FL_SHUTDOWN)
+			return -EBUSY;
+
+		/*
 		 * When soft_disable is set and enable is set, we want to
 		 * register the tracepoint for the event, but leave the event
 		 * as is. That means, if the event was already enabled, we do
@@ -342,6 +349,56 @@ static int __ftrace_event_enable_disable
 	return ret;
 }
 
+void ftrace_event_shutdown(struct ftrace_event_call *call)
+{
+	struct trace_array *tr;
+	struct ftrace_event_file *file;
+
+	mutex_lock(&trace_types_lock);
+	mutex_lock(&event_mutex);
+
+	do_for_each_event_file(tr, file) {
+		if (file->event_call != call)
+			continue;
+		set_bit(FTRACE_EVENT_FL_SHUTDOWN, &file->flags);
+		/*
+		 * The do_for_each_event_file() is
+		 * a double loop. After finding the call for this
+ 		 * trace_array, we use break to jump to the next
+		 * trace_array.
+		 */
+		break;
+	} while_for_each_event_file();
+
+	mutex_unlock(&event_mutex);
+	mutex_unlock(&trace_types_lock);
+}
+
+void ftrace_event_restart(struct ftrace_event_call *call)
+{
+	struct trace_array *tr;
+	struct ftrace_event_file *file;
+
+	mutex_lock(&trace_types_lock);
+	mutex_lock(&event_mutex);
+
+	do_for_each_event_file(tr, file) {
+		if (file->event_call != call)
+			continue;
+		clear_bit(FTRACE_EVENT_FL_SHUTDOWN, &file->flags);
+		/*
+		 * The do_for_each_event_file() is
+		 * a double loop. After finding the call for this
+ 		 * trace_array, we use break to jump to the next
+		 * trace_array.
+		 */
+		break;
+	} while_for_each_event_file();
+
+	mutex_unlock(&event_mutex);
+	mutex_unlock(&trace_types_lock);
+}
+
 static int ftrace_event_enable_disable(struct ftrace_event_file *file,
 				       int enable)
 {
Index: linux-trace.git/kernel/trace/trace_kprobe.c
===================================================================
--- linux-trace.git.orig/kernel/trace/trace_kprobe.c
+++ linux-trace.git/kernel/trace/trace_kprobe.c
@@ -336,9 +336,14 @@ static void __unregister_trace_probe(str
 /* Unregister a trace_probe and probe_event: call with locking probe_lock */
 static int unregister_trace_probe(struct trace_probe *tp)
 {
+	/* Prevent the event from being enabled */
+	ftrace_event_shutdown(&tp->call);
+
 	/* Enabled event can not be unregistered */
-	if (trace_probe_is_enabled(tp))
+	if (trace_probe_is_enabled(tp)) {
+		ftrace_event_restart(&tp->call);
 		return -EBUSY;
+	}
 
 	__unregister_trace_probe(tp);
 	list_del(&tp->list);


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/