Skip to contentSkip to conversation

tracing: Unifying dynamic event interface

PATCH In review Masami Hiramatsu 3 participants 22 messages Nov 5, 2018 – Dec 7, 2018

Subsystems (3)
Documentation Kselftest Tracing
lore
Share & tools
lore

Masami Hiramatsu

author maintainer
#0 lore raw

v2 cover tracing: Unifying dynamic event interface

Hi,

This is v2 series of unifying dynamic event interface on ftrace. Currently ftrace has 3 dynamic event interfaces, kprobes, uprobes and synthetic. This series unifies those dynamic event interfaces to "dynamic_events" so that we can add other dynamic events easily on same interface, e.g. function events.
The older interfaces are left on the tracefs for backward compatibility.

dynamic_events syntax has no difference from kprobe_events and uprobe_events. You can use same syntax for dynamic_events interface. For synthetic events, similar to the probe events, dynamic_events adds "s:[GROUP/]" prefix, where the "GROUP/" must be "synthetic/".

s:[synthetic/]<event-name> type arg [type arg]...

E.g.

 $ echo 'wakeup_latency u64 lat pid_t pid char' > synthetic_events

is same as

 $ echo 's:wakeup_latency u64 lat pid_t pid char' > dynamic_events

Or

 $ echo 's:synthetic/wakeup_latency u64 lat pid_t pid char' > dynamic_events

This series modifies synthetic event interface behavior a bit, reorder lock dependency and related cleanups so that we can integrate the synthetic event to dynamic_events interface.

In this version, I changed the generic '!' erase command, which now supports entire line style like other interfaces. So you can delete events via dynamic_events as below

 $ cat dynamic_events | while read line; \
   do echo "!$line" >> dynamic_events; done

Also, the big change will be removing dyn_event_mutex and synth_event_mutex because all those parts are protected by event_mutex.

Changes from v2 are here;

New patches:
- Reorder event_mutex and synth_event_mutex to solve
AB-BA deadlock correctly. ([2/12])
- Simplify creation and deletion of synthetic event. ([3/12])
- Retern -ENOENT if there is no synthetic event when deleting ([4/12])
- Integrate similar probe argument parsers ([5/12])
- Use dyn_event framework for synthetic events ([9/12])
- Remove synth_event_mutex ([10/12])
- Remove unused APIs ([11/12])

Modified patches:
[6/12] - [8/12]
- Generalize delete event and export as dyn_event_release_all().
- Add match operation for find deleting event.
- Reorder event_mutex and dyn_event_mutex to solve lock dependency issue.
- Pass const char **argv for create operation and use -ECANCELED to signal for trying next dyn_event_operations.
- Remove dyn_event_mutex.

[12/12]
- Accept entire line, but instead of checking the given entire line strictly, simply checking the event and group name.

Tom, thanks for your Ack for v1 series. Since I changed many things from v1 (not only minor change), I decided to not add your Ack for this version. Anyway, what I've added in this version are related to synthetic events. I need your review for those. (especially removing synth_event_mutex)

You can try it from my git tree.

https://github.com/mhiramat/linux/tree/unify-dynamic-events-v2

Thank you,

---

Masami Hiramatsu (12):
tracing/uprobes: Add busy check when cleanup all uprobes
tracing: Lock event_mutex before synth_event_mutex
tracing: Simplify creation and deletion of synthetic event
tracing: Integrate similar probe argument parsers
tracing: Add unified dynamic event framework tracing/kprobes: Use dyn_event framework for kprobe events tracing/uprobes: Use dyn_event framework for uprobe events
tracing: Use dyn_event framework for synthetic events
tracing: Remove unneeded synth_event_mutex
tracing: Remove orphaned trace_add/remove_event_call functions
tracing: Add generic event-name based remove event method selftests/ftrace: Add testcases for dynamic event

 Documentation/trace/kprobetrace.rst                |    3 
 Documentation/trace/uprobetracer.rst               |    4 
 include/linux/trace_events.h                       |    4 
 kernel/trace/Kconfig                               |    6 
 kernel/trace/Makefile                              |    1 
 kernel/trace/trace.c                               |   12 +
 kernel/trace/trace_dynevent.c                      |  217 ++++++++++++
 kernel/trace/trace_dynevent.h                      |  119 +++++++
 kernel/trace/trace_events.c                        |   12 -
 kernel/trace/trace_events_hist.c                   |  322 ++++++++++--------
 kernel/trace/trace_kprobe.c                        |  357 ++++++++++----------
 kernel/trace/trace_probe.c                         |   74 ++++
 kernel/trace/trace_probe.h                         |    9 -
 kernel/trace/trace_uprobe.c                        |  305 ++++++++---------
 .../ftrace/test.d/dynevent/add_remove_kprobe.tc    |   30 ++
 .../ftrace/test.d/dynevent/add_remove_synth.tc     |   27 ++
 .../ftrace/test.d/dynevent/clear_select_events.tc  |   50 +++
 .../ftrace/test.d/dynevent/generic_clear_event.tc  |   49 +++
 18 files changed, 1094 insertions(+), 507 deletions(-)
 create mode 100644 kernel/trace/trace_dynevent.c
 create mode 100644 kernel/trace/trace_dynevent.h
 create mode 100644 tools/testing/selftests/ftrace/test.d/dynevent/add_remove_kprobe.tc
 create mode 100644 tools/testing/selftests/ftrace/test.d/dynevent/add_remove_synth.tc
 create mode 100644 tools/testing/selftests/ftrace/test.d/dynevent/clear_select_events.tc
 create mode 100644 tools/testing/selftests/ftrace/test.d/dynevent/generic_clear_event.tc

--
Masami Hiramatsu (Linaro) <[email protected]>

author maintainer
#1 lore raw

PATCH v2 1/12 tracing/uprobes: Add busy check when cleanup all uprobes

Add a busy check loop in cleanup_all_probes() before trying to remove all events in uprobe_events as same as kprobe_events does.

Without this change, writing null to uprobe_events will try to remove events but if one of them is enabled, it stopped there but some of events are already cleared.

With this change, writing null to uprobe_events make sure all events are not enabled before removing events. So, it clears all events, or return an error (-EBUSY) with keeping all events.

Signed-off-by Masami Hiramatsu <[email protected]>
---
 kernel/trace/trace_uprobe.c |    7 +++++++
 1 file changed, 7 insertions(+)
kernel/trace/trace_uprobe.c +7 -0
index 31ea48eceda1..b708e4ff7ea7 100644
@@ -587,12 +587,19 @@ static int cleanup_all_probes(void)
 	int ret = 0;
 
 	mutex_lock(&uprobe_lock);
+	/* Ensure no probe is in use. */
+	list_for_each_entry(tu, &uprobe_list, list)
+		if (trace_probe_is_enabled(&tu->tp)) {
+			ret = -EBUSY;
+			goto end;
+		}
 	while (!list_empty(&uprobe_list)) {
 		tu = list_entry(uprobe_list.next, struct trace_uprobe, list);
 		ret = unregister_trace_uprobe(tu);
 		if (ret)
 			break;
 	}
+end:
 	mutex_unlock(&uprobe_lock);
 	return ret;
 }
maintainer
#2 lore raw

Re: [PATCH v2 01/12] tracing/uprobes: Add busy check when cleanup all uprobes

Quoted 4 lines · On Mon, 5 Nov 2018 18:00:15 +0900 Masami Hiramatsu <[email protected]> wrote:

Add a busy check loop in cleanup_all_probes() before trying to remove all events in uprobe_events as same as kprobe_events does.

Quoted 9 lines

Without this change, writing null to uprobe_events will try to remove events but if one of them is enabled, it stopped there but some of events are already cleared.

With this change, writing null to uprobe_events make sure all events are not enabled before removing events. So, it clears all events, or return an error (-EBUSY) with keeping all events.

Hmm, should this patch be marked as stable?

-- Steve

Quoted 29 lines
Signed-off-by Masami Hiramatsu <[email protected]>
---
 kernel/trace/trace_uprobe.c |    7 +++++++
 1 file changed, 7 insertions(+)
kernel/trace/trace_uprobe.c +7 -0
index 31ea48eceda1..b708e4ff7ea7 100644
@@ -587,12 +587,19 @@ static int cleanup_all_probes(void)
 	int ret = 0;
 
 	mutex_lock(&uprobe_lock);
+	/* Ensure no probe is in use. */
+	list_for_each_entry(tu, &uprobe_list, list)
+		if (trace_probe_is_enabled(&tu->tp)) {
+			ret = -EBUSY;
+			goto end;
+		}
 	while (!list_empty(&uprobe_list)) {
 		tu = list_entry(uprobe_list.next, struct trace_uprobe, list);
 		ret = unregister_trace_uprobe(tu);
 		if (ret)
 			break;
 	}
+end:
 	mutex_unlock(&uprobe_lock);
 	return ret;
 }
author maintainer
#3 lore raw

Re: [PATCH v2 01/12] tracing/uprobes: Add busy check when cleanup all uprobes

Quoted 11 lines · On Tue, 4 Dec 2018 12:43:33 -0500 Steven Rostedt <[email protected]> wrote:
Quoted 8 lines · On Mon, 5 Nov 2018 18:00:15 +0900 Masami Hiramatsu <[email protected]> wrote:

Add a busy check loop in cleanup_all_probes() before trying to remove all events in uprobe_events as same as kprobe_events does.

Without this change, writing null to uprobe_events will try to remove events but if one of them is enabled, it stopped there but some of events are already cleared.

Quoted 7 lines
Quoted 5 lines

With this change, writing null to uprobe_events make sure all events are not enabled before removing events. So, it clears all events, or return an error (-EBUSY) with keeping all events.

Hmm, should this patch be marked as stable?

Hmm, OK, let this go to stable. Since anyway, this will cause a wired result on uprobe_events from user point of view.

Thank you!

Quoted 33 lines

-- Steve

Quoted 29 lines
Signed-off-by Masami Hiramatsu <[email protected]>
---
 kernel/trace/trace_uprobe.c |    7 +++++++
 1 file changed, 7 insertions(+)
kernel/trace/trace_uprobe.c +7 -0
index 31ea48eceda1..b708e4ff7ea7 100644
@@ -587,12 +587,19 @@ static int cleanup_all_probes(void)
 	int ret = 0;
 
 	mutex_lock(&uprobe_lock);
+	/* Ensure no probe is in use. */
+	list_for_each_entry(tu, &uprobe_list, list)
+		if (trace_probe_is_enabled(&tu->tp)) {
+			ret = -EBUSY;
+			goto end;
+		}
 	while (!list_empty(&uprobe_list)) {
 		tu = list_entry(uprobe_list.next, struct trace_uprobe, list);
 		ret = unregister_trace_uprobe(tu);
 		if (ret)
 			break;
 	}
+end:
 	mutex_unlock(&uprobe_lock);
 	return ret;
 }
Signature · 1 line
Masami Hiramatsu <[email protected]>
author maintainer
#4 lore raw

PATCH v2 2/12 tracing: Lock event_mutex before synth_event_mutex

In reply to Masami Hiramatsu

synthetic event is using synth_event_mutex for protecting synth_event_list, and event_trigger_write() path acquires locks as below order.

event_trigger_write(event_mutex)
->trigger_process_regex(trigger_cmd_mutex)

    ->event_hist_trigger_func(synth_event_mutex)

On the other hand, synthetic event creation and deletion paths finally calls trace_add_event_call() and trace_remove_event_call() which acquires event_mutex. In that case, if we keep the synth_event_mutex locked while registering/unregistering synthetic events, its dependency will be inversed.

To avoid this issue, current synthetic event is using 2 phases process to create/delete events. For example, it searches existing event under synth_event_mutex to checking event-name conflict, and unlock synth_event_mutex, then register new event under event_mutex locked. Finally, it locks synth_event_mutex and tries to add the new event to the list. But it can introduce complexity and a chance of name conflict.

To solve this simpler, this introduces trace_add_event_call_nolock() and trace_remove_event_call_nolock() which don't acquires event_mutex inside. synthetic event can lock event_mutex before synth_event_mutex for solving lock dependency issue simpler.

Signed-off-by Masami Hiramatsu <[email protected]>
---
 include/linux/trace_events.h     |    2 ++
 kernel/trace/trace_events.c      |   34 ++++++++++++++++++++++++++++------
 kernel/trace/trace_events_hist.c |   24 ++++++++++--------------
 3 files changed, 40 insertions(+), 20 deletions(-)
include/linux/trace_events.h +2 -0
index 4130a5497d40..3aa05593a53f 100644
@@ -529,6 +529,8 @@ extern int trace_event_raw_init(struct trace_event_call *call);
 extern int trace_define_field(struct trace_event_call *call, const char *type,
 			      const char *name, int offset, int size,
 			      int is_signed, int filter_type);
+extern int trace_add_event_call_nolock(struct trace_event_call *call);
+extern int trace_remove_event_call_nolock(struct trace_event_call *call);
 extern int trace_add_event_call(struct trace_event_call *call);
 extern int trace_remove_event_call(struct trace_event_call *call);
 extern int trace_event_get_offsets(struct trace_event_call *call);
kernel/trace/trace_events.c +28 -6
index f94be0c2827b..a3b157f689ee 100644
@@ -2305,11 +2305,11 @@ __trace_early_add_new_event(struct trace_event_call *call,
 struct ftrace_module_file_ops;
 static void __add_event_to_tracers(struct trace_event_call *call);
 
-/* Add an additional event_call dynamically */
-int trace_add_event_call(struct trace_event_call *call)
+int trace_add_event_call_nolock(struct trace_event_call *call)
 {
 	int ret;
-	mutex_lock(&event_mutex);
+	lockdep_assert_held(&event_mutex);
+
 	mutex_lock(&trace_types_lock);
 
 	ret = __register_event(call, NULL);

@@ -2317,6 +2317,16 @@ int trace_add_event_call(struct trace_event_call *call)
 		__add_event_to_tracers(call);
 
 	mutex_unlock(&trace_types_lock);
+	return ret;
+}
+
+/* Add an additional event_call dynamically */
+int trace_add_event_call(struct trace_event_call *call)
+{
+	int ret;
+
+	mutex_lock(&event_mutex);
+	ret = trace_add_event_call_nolock(call);
 	mutex_unlock(&event_mutex);
 	return ret;
 }

@@ -2366,17 +2376,29 @@ static int probe_remove_event_call(struct trace_event_call *call)
 	return 0;
 }
 
-/* Remove an event_call */
-int trace_remove_event_call(struct trace_event_call *call)
+/* no event_mutex version */
+int trace_remove_event_call_nolock(struct trace_event_call *call)
 {
 	int ret;
 
-	mutex_lock(&event_mutex);
+	lockdep_assert_held(&event_mutex);
+
 	mutex_lock(&trace_types_lock);
 	down_write(&trace_event_sem);
 	ret = probe_remove_event_call(call);
 	up_write(&trace_event_sem);
 	mutex_unlock(&trace_types_lock);
+
+	return ret;
+}
+
+/* Remove an event_call */
+int trace_remove_event_call(struct trace_event_call *call)
+{
+	int ret;
+
+	mutex_lock(&event_mutex);
+	ret = trace_remove_event_call_nolock(call);
 	mutex_unlock(&event_mutex);
 
 	return ret;
kernel/trace/trace_events_hist.c +10 -14
index eb908ef2ecec..1670c65389fe 100644
@@ -912,7 +912,7 @@ static int register_synth_event(struct synth_event *event)
 	call->data = event;
 	call->tp = event->tp;
 
-	ret = trace_add_event_call(call);
+	ret = trace_add_event_call_nolock(call);
 	if (ret) {
 		pr_warn("Failed to register synthetic event: %s\n",
 			trace_event_name(call));

@@ -936,7 +936,7 @@ static int unregister_synth_event(struct synth_event *event)
 	struct trace_event_call *call = &event->call;
 	int ret;
 
-	ret = trace_remove_event_call(call);
+	ret = trace_remove_event_call_nolock(call);
 
 	return ret;
 }

@@ -1013,12 +1013,10 @@ static void add_or_delete_synth_event(struct synth_event *event, int delete)
 	if (delete)
 		free_synth_event(event);
 	else {
-		mutex_lock(&synth_event_mutex);
 		if (!find_synth_event(event->name))
 			list_add(&event->list, &synth_event_list);
 		else
 			free_synth_event(event);
-		mutex_unlock(&synth_event_mutex);
 	}
 }
 

@@ -1030,6 +1028,7 @@ static int create_synth_event(int argc, char **argv)
 	int i, consumed = 0, n_fields = 0, ret = 0;
 	char *name;
 
+	mutex_lock(&event_mutex);
 	mutex_lock(&synth_event_mutex);
 
 	/*

@@ -1102,8 +1101,6 @@ static int create_synth_event(int argc, char **argv)
 		goto err;
 	}
  out:
-	mutex_unlock(&synth_event_mutex);
-
 	if (event) {
 		if (delete_event) {
 			ret = unregister_synth_event(event);

@@ -1113,10 +1110,13 @@ static int create_synth_event(int argc, char **argv)
 			add_or_delete_synth_event(event, ret);
 		}
 	}
+	mutex_unlock(&synth_event_mutex);
+	mutex_unlock(&event_mutex);
 
 	return ret;
  err:
 	mutex_unlock(&synth_event_mutex);
+	mutex_unlock(&event_mutex);
 
 	for (i = 0; i < n_fields; i++)
 		free_synth_field(fields[i]);

@@ -1127,12 +1127,10 @@ static int create_synth_event(int argc, char **argv)
 
 static int release_all_synth_events(void)
 {
-	struct list_head release_events;
 	struct synth_event *event, *e;
 	int ret = 0;
 
-	INIT_LIST_HEAD(&release_events);
-
+	mutex_lock(&event_mutex);
 	mutex_lock(&synth_event_mutex);
 
 	list_for_each_entry(event, &synth_event_list, list) {

@@ -1142,16 +1140,14 @@ static int release_all_synth_events(void)
 		}
 	}
 
-	list_splice_init(&event->list, &release_events);
-
-	mutex_unlock(&synth_event_mutex);
-
-	list_for_each_entry_safe(event, e, &release_events, list) {
+	list_for_each_entry_safe(event, e, &synth_event_list, list) {
 		list_del(&event->list);
 
 		ret = unregister_synth_event(event);
 		add_or_delete_synth_event(event, !ret);
 	}
+	mutex_unlock(&synth_event_mutex);
+	mutex_unlock(&event_mutex);
 
 	return ret;
 }
author maintainer
#5 lore raw

PATCH v2 3/12 tracing: Simplify creation and deletion of synthetic event

In reply to Masami Hiramatsu

Simplify creation and deletion code of synthetic event.

Since the event_mutex and synth_event_mutex ordering issue is gone, we can skip existing event check when adding or deleting event, and some redundant code in error path.

This changes release_all_synth_events() to abort the process when it hits any error and returns the error code. It succeeds only if it has no error.

Signed-off-by Masami Hiramatsu <[email protected]>
---
 kernel/trace/trace_events_hist.c |   53 +++++++++++++-------------------------
 1 file changed, 18 insertions(+), 35 deletions(-)
kernel/trace/trace_events_hist.c +18 -35
index 1670c65389fe..0feb7f460123 100644
@@ -1008,18 +1008,6 @@ struct hist_var_data {
 	struct hist_trigger_data *hist_data;
 };
 
-static void add_or_delete_synth_event(struct synth_event *event, int delete)
-{
-	if (delete)
-		free_synth_event(event);
-	else {
-		if (!find_synth_event(event->name))
-			list_add(&event->list, &synth_event_list);
-		else
-			free_synth_event(event);
-	}
-}
-
 static int create_synth_event(int argc, char **argv)
 {
 	struct synth_field *field, *fields[SYNTH_FIELDS_MAX];

@@ -1052,15 +1040,16 @@ static int create_synth_event(int argc, char **argv)
 	if (event) {
 		if (delete_event) {
 			if (event->ref) {
-				event = NULL;
 				ret = -EBUSY;
 				goto out;
 			}
-			list_del(&event->list);
-			goto out;
-		}
-		event = NULL;
-		ret = -EEXIST;
+			ret = unregister_synth_event(event);
+			if (!ret) {
+				list_del(&event->list);
+				free_synth_event(event);
+			}
+		} else
+			ret = -EEXIST;
 		goto out;
 	} else if (delete_event) {
 		ret = -ENOENT;

@@ -1100,29 +1089,21 @@ static int create_synth_event(int argc, char **argv)
 		event = NULL;
 		goto err;
 	}
+	ret = register_synth_event(event);
+	if (!ret)
+		list_add(&event->list, &synth_event_list);
+	else
+		free_synth_event(event);
  out:
-	if (event) {
-		if (delete_event) {
-			ret = unregister_synth_event(event);
-			add_or_delete_synth_event(event, !ret);
-		} else {
-			ret = register_synth_event(event);
-			add_or_delete_synth_event(event, ret);
-		}
-	}
 	mutex_unlock(&synth_event_mutex);
 	mutex_unlock(&event_mutex);
 
 	return ret;
  err:
-	mutex_unlock(&synth_event_mutex);
-	mutex_unlock(&event_mutex);
-
 	for (i = 0; i < n_fields; i++)
 		free_synth_field(fields[i]);
-	free_synth_event(event);
 
-	return ret;
+	goto out;
 }
 
 static int release_all_synth_events(void)

@@ -1141,10 +1122,12 @@ static int release_all_synth_events(void)
 	}
 
 	list_for_each_entry_safe(event, e, &synth_event_list, list) {
-		list_del(&event->list);
-
 		ret = unregister_synth_event(event);
-		add_or_delete_synth_event(event, !ret);
+		if (!ret) {
+			list_del(&event->list);
+			free_synth_event(event);
+		} else
+			break;
 	}
 	mutex_unlock(&synth_event_mutex);
 	mutex_unlock(&event_mutex);
author maintainer
#6 lore raw

PATCH v2 4/12 tracing: Integrate similar probe argument parsers

In reply to Masami Hiramatsu

Integrate similar argument parsers for kprobes and uprobes events into traceprobe_parse_probe_arg().

Signed-off-by Masami Hiramatsu <[email protected]>
---
 kernel/trace/trace_kprobe.c |   48 ++-----------------------------------------
 kernel/trace/trace_probe.c  |   47 +++++++++++++++++++++++++++++++++++++++---
 kernel/trace/trace_probe.h  |    7 ++----
 kernel/trace/trace_uprobe.c |   44 ++-------------------------------------
 4 files changed, 50 insertions(+), 96 deletions(-)
kernel/trace/trace_kprobe.c +2 -46
index fec67188c4d2..d313bcc259dc 100644
@@ -548,7 +548,6 @@ static int create_trace_kprobe(int argc, char **argv)
 	bool is_return = false, is_delete = false;
 	char *symbol = NULL, *event = NULL, *group = NULL;
 	int maxactive = 0;
-	char *arg;
 	long offset = 0;
 	void *addr = NULL;
 	char buf[MAX_EVENT_NAME_LEN];

@@ -676,53 +675,10 @@ static int create_trace_kprobe(int argc, char **argv)
 	}
 
 	/* parse arguments */
-	ret = 0;
 	for (i = 0; i < argc && i < MAX_TRACE_ARGS; i++) {
-		struct probe_arg *parg = &tk->tp.args[i];
-
-		/* Increment count for freeing args in error case */
-		tk->tp.nr_args++;
-
-		/* Parse argument name */
-		arg = strchr(argv[i], '=');
-		if (arg) {
-			*arg++ = '\0';
-			parg->name = kstrdup(argv[i], GFP_KERNEL);
-		} else {
-			arg = argv[i];
-			/* If argument name is omitted, set "argN" */
-			snprintf(buf, MAX_EVENT_NAME_LEN, "arg%d", i + 1);
-			parg->name = kstrdup(buf, GFP_KERNEL);
-		}
-
-		if (!parg->name) {
-			pr_info("Failed to allocate argument[%d] name.\n", i);
-			ret = -ENOMEM;
-			goto error;
-		}
-
-		if (!is_good_name(parg->name)) {
-			pr_info("Invalid argument[%d] name: %s\n",
-				i, parg->name);
-			ret = -EINVAL;
-			goto error;
-		}
-
-		if (traceprobe_conflict_field_name(parg->name,
-							tk->tp.args, i)) {
-			pr_info("Argument[%d] name '%s' conflicts with "
-				"another field.\n", i, argv[i]);
-			ret = -EINVAL;
-			goto error;
-		}
-
-		/* Parse fetch argument */
-		ret = traceprobe_parse_probe_arg(arg, &tk->tp.size, parg,
-						 flags);
-		if (ret) {
-			pr_info("Parse error at argument[%d]. (%d)\n", i, ret);
+		ret = traceprobe_parse_probe_arg(&tk->tp, i, argv[i], flags);
+		if (ret)
 			goto error;
-		}
 	}
 
 	ret = register_trace_kprobe(tk);
kernel/trace/trace_probe.c +44 -3

65 lines not on this page · raw message

kernel/trace/trace_probe.h +2 -5
index 974afc1a3e73..feeec261b356 100644
@@ -272,11 +272,8 @@ find_event_file_link(struct trace_probe *tp, struct trace_event_file *file)
 #define TPARG_FL_FENTRY BIT(2)
 #define TPARG_FL_MASK	GENMASK(2, 0)
 
-extern int traceprobe_parse_probe_arg(char *arg, ssize_t *size,
-		   struct probe_arg *parg, unsigned int flags);
-
-extern int traceprobe_conflict_field_name(const char *name,
-			       struct probe_arg *args, int narg);
+extern int traceprobe_parse_probe_arg(struct trace_probe *tp, int i,
+				char *arg, unsigned int flags);
 
 extern int traceprobe_update_arg(struct probe_arg *arg);
 extern void traceprobe_free_probe_arg(struct probe_arg *arg);
kernel/trace/trace_uprobe.c +2 -42

53 lines not on this page · raw message

author maintainer
#7 lore raw

PATCH v2 5/12 tracing: Add unified dynamic event framework

In reply to Masami Hiramatsu

Add unified dynamic event framework for ftrace kprobes, uprobes and synthetic events. Those dynamic events can be co-exist on same file because those syntax doesn't over-wrapped.

This introduces a framework part which provides a unified tracefs interface and operations.

Signed-off-by Masami Hiramatsu <[email protected]>

---
Changes in v2:
- Fix to lock dyn_event_mutex when freeing all events.
- Generalize and export dyn_event_release_all().
- To fix dependency issue, lock event_mutex before dyn_event_mutex.
- Separate create and delete operation.
- If create operation returns -ECANCELED, try next operation.
- Pass const argv to create operation for safety.
- Add .match operation and generic delete API.
- Add precise description for data structures.
- Remove dyn_event_mutex, use event_mutex instead.

---
 kernel/trace/Kconfig          |    3 +
 kernel/trace/Makefile         |    1 
 kernel/trace/trace.c          |    4 +
 kernel/trace/trace_dynevent.c |  210 +++++++++++++++++++++++++++++++++++++++++
 kernel/trace/trace_dynevent.h |  119 +++++++++++++++++++++++
 5 files changed, 337 insertions(+)
 create mode 100644 kernel/trace/trace_dynevent.c
 create mode 100644 kernel/trace/trace_dynevent.h
kernel/trace/Kconfig +3 -0
index 5e3de28c7677..bf2e8a5a91f1 100644
@@ -518,6 +518,9 @@ config BPF_EVENTS
 	help
 	  This allows the user to attach BPF programs to kprobe events.
 
+config DYNAMIC_EVENTS
+	def_bool n
+
 config PROBE_EVENTS
 	def_bool n
kernel/trace/Makefile +1 -0
index f81dadbc7c4a..9ff3c4fa91b6 100644
@@ -78,6 +78,7 @@ endif
 ifeq ($(CONFIG_TRACING),y)
 obj-$(CONFIG_KGDB_KDB) += trace_kdb.o
 endif
+obj-$(CONFIG_DYNAMIC_EVENTS) += trace_dynevent.o
 obj-$(CONFIG_PROBE_EVENTS) += trace_probe.o
 obj-$(CONFIG_UPROBE_EVENTS) += trace_uprobe.o
kernel/trace/trace.c +4 -0
index ff1c4b20cd0a..2886e92e8eab 100644
@@ -4604,6 +4604,10 @@ static const char readme_msg[] =
 	"\t\t\t  traces\n"
 #endif
 #endif /* CONFIG_STACK_TRACER */
+#ifdef CONFIG_DYNAMIC_EVENTS
+	"  dynamic_events\t\t- Add/remove/show the generic dynamic events\n"
+	"\t\t\t  Write into this file to define/undefine new trace events.\n"
+#endif
 #ifdef CONFIG_KPROBE_EVENTS
 	"  kprobe_events\t\t- Add/remove/show the kernel dynamic events\n"
 	"\t\t\t  Write into this file to define/undefine new trace events.\n"
kernel/trace/trace_dynevent.c new file +210 -0

210 lines not on this page · raw message

kernel/trace/trace_dynevent.h new file +119 -0

119 lines not on this page · raw message

author maintainer
#8 lore raw

PATCH v2 6/12 tracing/kprobes: Use dyn_event framework for kprobe events

In reply to Masami Hiramatsu

Use dyn_event framework for kprobe events. This shows kprobe events on "tracing/dynamic_events" file.

User can also define new events via tracing/dynamic_events.

Signed-off-by Masami Hiramatsu <[email protected]>

---
Changes in v2:
- Use dyn_events_release_all() for clearing events.
- Use nolock event_call register/unregister and lock event_mutex before dyn_event_mutex to avoid lock dependency error.
- Check probe cleanup result in self-check.
- Rewrite trace_kprobe_create not to destroy arguments.
- Add match operation and new APIs for deleting event.
- Return -ECANCEL if given definition is not for kprobes.
- Remove unused for_each_trace_kprobe_safe.

---
 Documentation/trace/kprobetrace.rst |    3 
 kernel/trace/Kconfig                |    1 
 kernel/trace/trace_kprobe.c         |  319 +++++++++++++++++++----------------
 kernel/trace/trace_probe.c          |   27 +++
 kernel/trace/trace_probe.h          |    2 
 5 files changed, 207 insertions(+), 145 deletions(-)
Documentation/trace/kprobetrace.rst +3 -0
index 47e765c2f2c3..235ce2ab131a 100644
@@ -20,6 +20,9 @@ current_tracer. Instead of that, add probe points via
 /sys/kernel/debug/tracing/kprobe_events, and enable it via
 /sys/kernel/debug/tracing/events/kprobes/<EVENT>/enable.
 
+You can also use /sys/kernel/debug/tracing/dynamic_events instead of
+kprobe_events. That interface will provide unified access to other
+dynamic events too.
 
 Synopsis of kprobe_events
 -------------------------
kernel/trace/Kconfig +1 -0
index bf2e8a5a91f1..c0f6b0105609 100644
@@ -461,6 +461,7 @@ config KPROBE_EVENTS
 	bool "Enable kprobes-based dynamic events"
 	select TRACING
 	select PROBE_EVENTS
+	select DYNAMIC_EVENTS
 	default y
 	help
 	  This allows the user to add tracing events (similar to tracepoints)
kernel/trace/trace_kprobe.c +174 -145

591 lines not on this page · raw message

kernel/trace/trace_probe.c +27 -0
index 449150c6a87f..ff86417c0149 100644
@@ -154,6 +154,33 @@ int traceprobe_split_symbol_offset(char *symbol, long *offset)
 	return 0;
 }
 
+/* @buf must has MAX_EVENT_NAME_LEN size */
+int traceprobe_parse_event_name(const char **pevent, const char **pgroup,
+				char *buf)
+{
+	const char *slash, *event = *pevent;
+
+	slash = strchr(event, '/');
+	if (slash) {
+		if (slash == event) {
+			pr_info("Group name is not specified\n");
+			return -EINVAL;
+		}
+		if (slash - event + 1 > MAX_EVENT_NAME_LEN) {
+			pr_info("Group name is too long\n");
+			return -E2BIG;
+		}
+		strlcpy(buf, event, slash - event + 1);
+		*pgroup = buf;
+		*pevent = slash + 1;
+	}
+	if (strlen(event) == 0) {
+		pr_info("Event name is not specified\n");
+		return -EINVAL;
+	}
+	return 0;
+}
+
 #define PARAM_MAX_STACK (THREAD_SIZE / sizeof(unsigned long))
 
 static int parse_probe_vars(char *arg, const struct fetch_type *t,
kernel/trace/trace_probe.h +2 -0
index feeec261b356..8a63f8bc01bc 100644
@@ -279,6 +279,8 @@ extern int traceprobe_update_arg(struct probe_arg *arg);
 extern void traceprobe_free_probe_arg(struct probe_arg *arg);
 
 extern int traceprobe_split_symbol_offset(char *symbol, long *offset);
+extern int traceprobe_parse_event_name(const char **pevent,
+				       const char **pgroup, char *buf);
 
 extern int traceprobe_set_print_fmt(struct trace_probe *tp, bool is_return);
author maintainer
#9 lore raw

PATCH v2 7/12 tracing/uprobes: Use dyn_event framework for uprobe events

In reply to Masami Hiramatsu

Use dyn_event framework for uprobe events. This shows uprobe events on "dynamic_events" file.
User can also define new uprobe events via dynamic_events.

Signed-off-by Masami Hiramatsu <[email protected]>

---
Changes in v2:
- Use dyn_events_release_all() for clearing events.
- Use nolock event_call register/unregister and lock event_mutex before dyn_event_mutex to avoid lock dependency error.
- Select CONFIG_DYNAMIC_EVENTS in Kconfig
- Add match operation and use new API for event deletion
- Return -ECANCELED if given event is not for uprobe.
- Remove unused for_each_trace_uprobe_safe.

---
 Documentation/trace/uprobetracer.rst |    4 
 kernel/trace/Kconfig                 |    1 
 kernel/trace/trace_uprobe.c          |  278 ++++++++++++++++++----------------
 3 files changed, 153 insertions(+), 130 deletions(-)
Documentation/trace/uprobetracer.rst +4 -0
index d0822811527a..4c3bfde2ba47 100644
@@ -18,6 +18,10 @@ current_tracer. Instead of that, add probe points via
 However unlike kprobe-event tracer, the uprobe event interface expects the
 user to calculate the offset of the probepoint in the object.
 
+You can also use /sys/kernel/debug/tracing/dynamic_events instead of
+uprobe_events. That interface will provide unified access to other
+dynamic events too.
+
 Synopsis of uprobe_tracer
 -------------------------
 ::
kernel/trace/Kconfig +1 -0
index c0f6b0105609..2cab3c5dfe2c 100644
@@ -501,6 +501,7 @@ config UPROBE_EVENTS
 	depends on PERF_EVENTS
 	select UPROBES
 	select PROBE_EVENTS
+	select DYNAMIC_EVENTS
 	select TRACING
 	default y
 	help
kernel/trace/trace_uprobe.c +148 -130

498 lines not on this page · raw message

author maintainer
#10 lore raw

PATCH v2 8/12 tracing: Use dyn_event framework for synthetic events

In reply to Masami Hiramatsu

Use dyn_event framework for synthetic events. This shows synthetic events on "tracing/dynamic_events" file in addition to tracing/synthetic_events interface.

User can also define new events via tracing/dynamic_events with "s:" prefix. So, the new syntax is below;

s:[synthetic/]EVENT_NAME TYPE ARG; [TYPE ARG;]...

To remove events via tracing/dynamic_events, you can use "-:" prefix as same as other events.

Signed-off-by Masami Hiramatsu <[email protected]>
---
 kernel/trace/Kconfig             |    1 
 kernel/trace/trace.c             |    8 +
 kernel/trace/trace_events_hist.c |  265 ++++++++++++++++++++++++--------------
 3 files changed, 176 insertions(+), 98 deletions(-)
kernel/trace/Kconfig +1 -0
index 2cab3c5dfe2c..fa8b1fe824f3 100644
@@ -635,6 +635,7 @@ config HIST_TRIGGERS
 	depends on ARCH_HAVE_NMI_SAFE_CMPXCHG
 	select TRACING_MAP
 	select TRACING
+	select DYNAMIC_EVENTS
 	default n
 	help
 	  Hist triggers allow one or more arbitrary trace event fields
kernel/trace/trace.c +8 -0
index 2886e92e8eab..5e71d0c3373c 100644
@@ -4620,6 +4620,9 @@ static const char readme_msg[] =
 	"\t  accepts: event-definitions (one definition per line)\n"
 	"\t   Format: p[:[<group>/]<event>] <place> [<args>]\n"
 	"\t           r[maxactive][:[<group>/]<event>] <place> [<args>]\n"
+#ifdef CONFIG_HIST_TRIGGERS
+	"\t           s:[synthetic/]<event> <field> [<field>]\n"
+#endif
 	"\t           -:[<group>/]<event>\n"
 #ifdef CONFIG_KPROBE_EVENTS
 	"\t    place: [<module>:]<symbol>[+<offset>]|<memaddr>\n"

@@ -4638,6 +4641,11 @@ static const char readme_msg[] =
 	"\t     type: s8/16/32/64, u8/16/32/64, x8/16/32/64, string, symbol,\n"
 	"\t           b<bit-width>@<bit-offset>/<container-size>,\n"
 	"\t           <type>\\[<array-size>\\]\n"
+#ifdef CONFIG_HIST_TRIGGERS
+	"\t    field: <stype> <name>;\n"
+	"\t    stype: u8/u16/u32/u64, s8/s16/s32/s64, pid_t,\n"
+	"\t           [unsigned] char/int/long\n"
+#endif
 #endif
 	"  events/\t\t- Directory containing all trace event subsystems:\n"
 	"      enable\t\t- Write 0/1 to enable/disable tracing of all events\n"
kernel/trace/trace_events_hist.c +167 -98

441 lines not on this page · raw message

author maintainer
#11 lore raw

PATCH v2 9/12 tracing: Remove unneeded synth_event_mutex

In reply to Masami Hiramatsu

Rmove unneeded synth_event_mutex. This mutex protects the reference count in synth_event, however, those operational points are already protected by event_mutex.

1. In __create_synth_event() and create_or_delete_synth_event(), those synth_event_mutex clearly obtained right after event_mutex.

2. event_hist_trigger_func() is trigger_hist_cmd.func() which is called by trigger_process_regex(), which is a part of event_trigger_regex_write() and this function takes event_mutex.

3. hist_unreg_all() is trigger_hist_cmd.unreg_all() which is called by event_trigger_regex_open() and it takes event_mutex.

4. onmatch_destroy() and onmatch_create() have long call tree, but both are finally invoked from event_trigger_regex_write() and event_trace_del_tracer(), former takes event_mutex, and latter ensures called under event_mutex locked.

Finally, I ensured there is no resource conflict. For safety, I added lockdep_assert_held(&event_mutex) for each function.

Signed-off-by Masami Hiramatsu <[email protected]>
---
 kernel/trace/trace_events_hist.c |   30 +++++++-----------------------
 1 file changed, 7 insertions(+), 23 deletions(-)
kernel/trace/trace_events_hist.c +7 -23
index 414aabd67d1f..21e4954375a1 100644
@@ -444,8 +444,6 @@ static bool have_hist_err(void)
 	return false;
 }
 
-static DEFINE_MUTEX(synth_event_mutex);
-
 struct synth_trace_event {
 	struct trace_entry	ent;
 	u64			fields[];

@@ -1077,7 +1075,6 @@ static int __create_synth_event(int argc, const char *name, const char **argv)
 		return -EINVAL;
 
 	mutex_lock(&event_mutex);
-	mutex_lock(&synth_event_mutex);
 
 	event = find_synth_event(name);
 	if (event) {

@@ -1119,7 +1116,6 @@ static int __create_synth_event(int argc, const char *name, const char **argv)
 	else
 		free_synth_event(event);
  out:
-	mutex_unlock(&synth_event_mutex);
 	mutex_unlock(&event_mutex);
 
 	return ret;

@@ -1139,7 +1135,6 @@ static int create_or_delete_synth_event(int argc, char **argv)
 	/* trace_run_command() ensures argc != 0 */
 	if (name[0] == '!') {
 		mutex_lock(&event_mutex);
-		mutex_lock(&synth_event_mutex);
 		event = find_synth_event(name + 1);
 		if (event) {
 			if (event->ref)

@@ -1153,7 +1148,6 @@ static int create_or_delete_synth_event(int argc, char **argv)
 			}
 		} else
 			ret = -ENOENT;
-		mutex_unlock(&synth_event_mutex);
 		mutex_unlock(&event_mutex);
 		return ret;
 	}

@@ -3535,7 +3529,7 @@ static void onmatch_destroy(struct action_data *data)
 {
 	unsigned int i;
 
-	mutex_lock(&synth_event_mutex);
+	lockdep_assert_held(&event_mutex);
 
 	kfree(data->onmatch.match_event);
 	kfree(data->onmatch.match_event_system);

@@ -3548,8 +3542,6 @@ static void onmatch_destroy(struct action_data *data)
 		data->onmatch.synth_event->ref--;
 
 	kfree(data);
-
-	mutex_unlock(&synth_event_mutex);
 }
 
 static void destroy_field_var(struct field_var *field_var)

@@ -3700,15 +3692,14 @@ static int onmatch_create(struct hist_trigger_data *hist_data,
 	struct synth_event *event;
 	int ret = 0;
 
-	mutex_lock(&synth_event_mutex);
+	lockdep_assert_held(&event_mutex);
+
 	event = find_synth_event(data->onmatch.synth_event_name);
 	if (!event) {
 		hist_err("onmatch: Couldn't find synthetic event: ", data->onmatch.synth_event_name);
-		mutex_unlock(&synth_event_mutex);
 		return -EINVAL;
 	}
 	event->ref++;
-	mutex_unlock(&synth_event_mutex);
 
 	var_ref_idx = hist_data->n_var_refs;
 

@@ -3782,9 +3773,7 @@ static int onmatch_create(struct hist_trigger_data *hist_data,
  out:
 	return ret;
  err:
-	mutex_lock(&synth_event_mutex);
 	event->ref--;
-	mutex_unlock(&synth_event_mutex);
 
 	goto out;
 }

@@ -5492,6 +5481,8 @@ static void hist_unreg_all(struct trace_event_file *file)
 	struct synth_event *se;
 	const char *se_name;
 
+	lockdep_assert_held(&event_mutex);
+
 	if (hist_file_check_refs(file))
 		return;
 

@@ -5501,12 +5492,10 @@ static void hist_unreg_all(struct trace_event_file *file)
 			list_del_rcu(&test->list);
 			trace_event_trigger_enable_disable(file, 0);
 
-			mutex_lock(&synth_event_mutex);
 			se_name = trace_event_name(file->event_call);
 			se = find_synth_event(se_name);
 			if (se)
 				se->ref--;
-			mutex_unlock(&synth_event_mutex);
 
 			update_cond_flag(file);
 			if (hist_data->enable_timestamps)

@@ -5532,6 +5521,8 @@ static int event_hist_trigger_func(struct event_command *cmd_ops,
 	char *trigger, *p;
 	int ret = 0;
 
+	lockdep_assert_held(&event_mutex);
+
 	if (glob && strlen(glob)) {
 		last_cmd_set(param);
 		hist_err_clear();

@@ -5622,14 +5613,10 @@ static int event_hist_trigger_func(struct event_command *cmd_ops,
 		}
 
 		cmd_ops->unreg(glob+1, trigger_ops, trigger_data, file);
-
-		mutex_lock(&synth_event_mutex);
 		se_name = trace_event_name(file->event_call);
 		se = find_synth_event(se_name);
 		if (se)
 			se->ref--;
-		mutex_unlock(&synth_event_mutex);
-
 		ret = 0;
 		goto out_free;
 	}

@@ -5665,13 +5652,10 @@ static int event_hist_trigger_func(struct event_command *cmd_ops,
 	if (ret)
 		goto out_unreg;
 
-	mutex_lock(&synth_event_mutex);
 	se_name = trace_event_name(file->event_call);
 	se = find_synth_event(se_name);
 	if (se)
 		se->ref++;
-	mutex_unlock(&synth_event_mutex);
-
 	/* Just return zero, not the number of registered triggers */
 	ret = 0;
  out:
author maintainer
#12 lore raw

PATCH v2 10/12 tracing: Remove orphaned trace_add/remove_event_call functions

In reply to Masami Hiramatsu

Remove trace_add_event_call() and trace_remove_event_call() functions since those are not used anymore.

Signed-off-by Masami Hiramatsu <[email protected]>
---
 include/linux/trace_events.h |    2 --
 kernel/trace/trace_events.c  |   26 ++------------------------
 2 files changed, 2 insertions(+), 26 deletions(-)
include/linux/trace_events.h +0 -2
index 3aa05593a53f..7a3147ead6bf 100644
@@ -531,8 +531,6 @@ extern int trace_define_field(struct trace_event_call *call, const char *type,
 			      int is_signed, int filter_type);
 extern int trace_add_event_call_nolock(struct trace_event_call *call);
 extern int trace_remove_event_call_nolock(struct trace_event_call *call);
-extern int trace_add_event_call(struct trace_event_call *call);
-extern int trace_remove_event_call(struct trace_event_call *call);
 extern int trace_event_get_offsets(struct trace_event_call *call);
 
 #define is_signed_type(type)	(((type)(-1)) < (type)1)
kernel/trace/trace_events.c +2 -24
index a3b157f689ee..16cff7c0fd40 100644
@@ -2305,6 +2305,7 @@ __trace_early_add_new_event(struct trace_event_call *call,
 struct ftrace_module_file_ops;
 static void __add_event_to_tracers(struct trace_event_call *call);
 
+/* Add an additional event_call dynamically */
 int trace_add_event_call_nolock(struct trace_event_call *call)
 {
 	int ret;

@@ -2320,17 +2321,6 @@ int trace_add_event_call_nolock(struct trace_event_call *call)
 	return ret;
 }
 
-/* Add an additional event_call dynamically */
-int trace_add_event_call(struct trace_event_call *call)
-{
-	int ret;
-
-	mutex_lock(&event_mutex);
-	ret = trace_add_event_call_nolock(call);
-	mutex_unlock(&event_mutex);
-	return ret;
-}
-
 /*
  * Must be called under locking of trace_types_lock, event_mutex and
  * trace_event_sem.

@@ -2376,7 +2366,7 @@ static int probe_remove_event_call(struct trace_event_call *call)
 	return 0;
 }
 
-/* no event_mutex version */
+/* Remove an event_call */
 int trace_remove_event_call_nolock(struct trace_event_call *call)
 {
 	int ret;

@@ -2392,18 +2382,6 @@ int trace_remove_event_call_nolock(struct trace_event_call *call)
 	return ret;
 }
 
-/* Remove an event_call */
-int trace_remove_event_call(struct trace_event_call *call)
-{
-	int ret;
-
-	mutex_lock(&event_mutex);
-	ret = trace_remove_event_call_nolock(call);
-	mutex_unlock(&event_mutex);
-
-	return ret;
-}
-
 #define for_each_event(event, start, end)			\
 	for (event = start;					\
 	     (unsigned long)event < (unsigned long)end;		\
maintainer
#13 lore raw

Re: [PATCH v2 10/12] tracing: Remove orphaned trace_add/remove_event_call functions

Quoted 4 lines · On Mon, 5 Nov 2018 18:04:29 +0900 Masami Hiramatsu <[email protected]> wrote:

Remove trace_add_event_call() and trace_remove_event_call() functions since those are not used anymore.

Signed-off-by Masami Hiramatsu <[email protected]>

Hi Masami,

I've applied the series locally (need to test it) except for this patch. Honestly, I hate the "_nolock" name, and it makes no sense when

1) they still grab locks
2) there's no version without "_nolock"

I added this patch in its place:

-- Steve

From: "Steven Rostedt (VMware)" <[email protected]>
Date: Tue, 4 Dec 2018 13:35:45 -0500
Subject: [PATCH] tracing: Consolidate trace_add/remove_event_call back to the
nolock functions

The trace_add/remove_event_call_nolock() functions were added to allow the tace_add/remove_event_call() code be called when the event_mutex lock was already taken. Now that all callers are done within the event_mutex, there's no reason to have two different interfaces.

Remove the current wrapper trace_add/remove_event_call()s and rename the _nolock versions back to the original names.

Link http://lkml.kernel.org/r/154140866955.17322.2081425494660638846.stgit@devbox open here
Signed-off-by Steven Rostedt (VMware) <[email protected]>
---
 include/linux/trace_events.h     |  2 --
 kernel/trace/trace_events.c      | 30 ++++--------------------------
 kernel/trace/trace_events_hist.c |  6 +++---
 kernel/trace/trace_kprobe.c      |  4 ++--
 kernel/trace/trace_uprobe.c      |  4 ++--
 5 files changed, 11 insertions(+), 35 deletions(-)
include/linux/trace_events.h +0 -2
index 3aa05593a53f..4130a5497d40 100644
@@ -529,8 +529,6 @@ extern int trace_event_raw_init(struct trace_event_call *call);
 extern int trace_define_field(struct trace_event_call *call, const char *type,
 			      const char *name, int offset, int size,
 			      int is_signed, int filter_type);
-extern int trace_add_event_call_nolock(struct trace_event_call *call);
-extern int trace_remove_event_call_nolock(struct trace_event_call *call);
 extern int trace_add_event_call(struct trace_event_call *call);
 extern int trace_remove_event_call(struct trace_event_call *call);
 extern int trace_event_get_offsets(struct trace_event_call *call);
kernel/trace/trace_events.c +4 -26
index a3b157f689ee..bd0162c0467c 100644
@@ -2305,7 +2305,8 @@ __trace_early_add_new_event(struct trace_event_call *call,
 struct ftrace_module_file_ops;
 static void __add_event_to_tracers(struct trace_event_call *call);
 
-int trace_add_event_call_nolock(struct trace_event_call *call)
+/* Add an additional event_call dynamically */
+int trace_add_event_call(struct trace_event_call *call)
 {
 	int ret;
 	lockdep_assert_held(&event_mutex);

@@ -2320,17 +2321,6 @@ int trace_add_event_call_nolock(struct trace_event_call *call)
 	return ret;
 }
 
-/* Add an additional event_call dynamically */
-int trace_add_event_call(struct trace_event_call *call)
-{
-	int ret;
-
-	mutex_lock(&event_mutex);
-	ret = trace_add_event_call_nolock(call);
-	mutex_unlock(&event_mutex);
-	return ret;
-}
-
 /*
  * Must be called under locking of trace_types_lock, event_mutex and
  * trace_event_sem.

@@ -2376,8 +2366,8 @@ static int probe_remove_event_call(struct trace_event_call *call)
 	return 0;
 }
 
-/* no event_mutex version */
-int trace_remove_event_call_nolock(struct trace_event_call *call)
+/* Remove an event_call */
+int trace_remove_event_call(struct trace_event_call *call)
 {
 	int ret;
 

@@ -2392,18 +2382,6 @@ int trace_remove_event_call_nolock(struct trace_event_call *call)
 	return ret;
 }
 
-/* Remove an event_call */
-int trace_remove_event_call(struct trace_event_call *call)
-{
-	int ret;
-
-	mutex_lock(&event_mutex);
-	ret = trace_remove_event_call_nolock(call);
-	mutex_unlock(&event_mutex);
-
-	return ret;
-}
-
 #define for_each_event(event, start, end)			\
 	for (event = start;					\
 	     (unsigned long)event < (unsigned long)end;		\
kernel/trace/trace_events_hist.c +3 -3
index 21e4954375a1..82e72c48a5a9 100644
@@ -960,7 +960,7 @@ static int register_synth_event(struct synth_event *event)
 	call->data = event;
 	call->tp = event->tp;
 
-	ret = trace_add_event_call_nolock(call);
+	ret = trace_add_event_call(call);
 	if (ret) {
 		pr_warn("Failed to register synthetic event: %s\n",
 			trace_event_name(call));

@@ -969,7 +969,7 @@ static int register_synth_event(struct synth_event *event)
 
 	ret = set_synth_event_print_fmt(call);
 	if (ret < 0) {
-		trace_remove_event_call_nolock(call);
+		trace_remove_event_call(call);
 		goto err;
 	}
  out:

@@ -984,7 +984,7 @@ static int unregister_synth_event(struct synth_event *event)
 	struct trace_event_call *call = &event->call;
 	int ret;
 
-	ret = trace_remove_event_call_nolock(call);
+	ret = trace_remove_event_call(call);
 
 	return ret;
 }
kernel/trace/trace_kprobe.c +2 -2
index bdf8c2ad5152..0e0f7b8024fb 100644
@@ -1353,7 +1353,7 @@ static int register_kprobe_event(struct trace_kprobe *tk)
 		kfree(call->print_fmt);
 		return -ENODEV;
 	}
-	ret = trace_add_event_call_nolock(call);
+	ret = trace_add_event_call(call);
 	if (ret) {
 		pr_info("Failed to register kprobe event: %s\n",
 			trace_event_name(call));

@@ -1368,7 +1368,7 @@ static int unregister_kprobe_event(struct trace_kprobe *tk)
 	int ret;
 
 	/* tp->event is unregistered in trace_remove_event_call() */
-	ret = trace_remove_event_call_nolock(&tk->tp.call);
+	ret = trace_remove_event_call(&tk->tp.call);
 	if (!ret)
 		kfree(tk->tp.call.print_fmt);
 	return ret;
kernel/trace/trace_uprobe.c +2 -2
index 4a7b21c891f3..e335576b9411 100644
@@ -1320,7 +1320,7 @@ static int register_uprobe_event(struct trace_uprobe *tu)
 		return -ENODEV;
 	}
 
-	ret = trace_add_event_call_nolock(call);
+	ret = trace_add_event_call(call);
 
 	if (ret) {
 		pr_info("Failed to register uprobe event: %s\n",

@@ -1337,7 +1337,7 @@ static int unregister_uprobe_event(struct trace_uprobe *tu)
 	int ret;
 
 	/* tu->event is unregistered in trace_remove_event_call() */
-	ret = trace_remove_event_call_nolock(&tu->tp.call);
+	ret = trace_remove_event_call(&tu->tp.call);
 	if (ret)
 		return ret;
 	kfree(tu->tp.call.print_fmt);
Signature · 1 line
2.19.2
author maintainer Acked-by
#14 lore raw

Re: [PATCH v2 10/12] tracing: Remove orphaned trace_add/remove_event_call functions

Quoted 8 lines · On Tue, 4 Dec 2018 13:51:20 -0500 Steven Rostedt <[email protected]> wrote:
Quoted 4 lines · On Mon, 5 Nov 2018 18:04:29 +0900 Masami Hiramatsu <[email protected]> wrote:

Remove trace_add_event_call() and trace_remove_event_call() functions since those are not used anymore.

Signed-off-by Masami Hiramatsu <[email protected]>
Quoted 7 lines

Hi Masami,

I've applied the series locally (need to test it) except for this patch. Honestly, I hate the "_nolock" name, and it makes no sense when

1) they still grab locks
2) there's no version without "_nolock"

Agreed.

Quoted 2 lines

I added this patch in its place:

That looks good to me too. I thought to add similar patch, but just waited for the comment.

Acked-by Masami Hiramatsu <[email protected]>

Thank you!

Quoted 180 lines

-- Steve

From: "Steven Rostedt (VMware)" <[email protected]>
Date: Tue, 4 Dec 2018 13:35:45 -0500
Subject: [PATCH] tracing: Consolidate trace_add/remove_event_call back to the
nolock functions

The trace_add/remove_event_call_nolock() functions were added to allow the tace_add/remove_event_call() code be called when the event_mutex lock was already taken. Now that all callers are done within the event_mutex, there's no reason to have two different interfaces.

Remove the current wrapper trace_add/remove_event_call()s and rename the _nolock versions back to the original names.

Link http://lkml.kernel.org/r/154140866955.17322.2081425494660638846.stgit@devbox open here
Signed-off-by Steven Rostedt (VMware) <[email protected]>
---
 include/linux/trace_events.h     |  2 --
 kernel/trace/trace_events.c      | 30 ++++--------------------------
 kernel/trace/trace_events_hist.c |  6 +++---
 kernel/trace/trace_kprobe.c      |  4 ++--
 kernel/trace/trace_uprobe.c      |  4 ++--
 5 files changed, 11 insertions(+), 35 deletions(-)
include/linux/trace_events.h +0 -2
index 3aa05593a53f..4130a5497d40 100644
@@ -529,8 +529,6 @@ extern int trace_event_raw_init(struct trace_event_call *call);
 extern int trace_define_field(struct trace_event_call *call, const char *type,
 			      const char *name, int offset, int size,
 			      int is_signed, int filter_type);
-extern int trace_add_event_call_nolock(struct trace_event_call *call);
-extern int trace_remove_event_call_nolock(struct trace_event_call *call);
 extern int trace_add_event_call(struct trace_event_call *call);
 extern int trace_remove_event_call(struct trace_event_call *call);
 extern int trace_event_get_offsets(struct trace_event_call *call);
kernel/trace/trace_events.c +4 -26
index a3b157f689ee..bd0162c0467c 100644
@@ -2305,7 +2305,8 @@ __trace_early_add_new_event(struct trace_event_call *call,
 struct ftrace_module_file_ops;
 static void __add_event_to_tracers(struct trace_event_call *call);
 
-int trace_add_event_call_nolock(struct trace_event_call *call)
+/* Add an additional event_call dynamically */
+int trace_add_event_call(struct trace_event_call *call)
 {
 	int ret;
 	lockdep_assert_held(&event_mutex);

@@ -2320,17 +2321,6 @@ int trace_add_event_call_nolock(struct trace_event_call *call)
 	return ret;
 }
 
-/* Add an additional event_call dynamically */
-int trace_add_event_call(struct trace_event_call *call)
-{
-	int ret;
-
-	mutex_lock(&event_mutex);
-	ret = trace_add_event_call_nolock(call);
-	mutex_unlock(&event_mutex);
-	return ret;
-}
-
 /*
  * Must be called under locking of trace_types_lock, event_mutex and
  * trace_event_sem.

@@ -2376,8 +2366,8 @@ static int probe_remove_event_call(struct trace_event_call *call)
 	return 0;
 }
 
-/* no event_mutex version */
-int trace_remove_event_call_nolock(struct trace_event_call *call)
+/* Remove an event_call */
+int trace_remove_event_call(struct trace_event_call *call)
 {
 	int ret;
 

@@ -2392,18 +2382,6 @@ int trace_remove_event_call_nolock(struct trace_event_call *call)
 	return ret;
 }
 
-/* Remove an event_call */
-int trace_remove_event_call(struct trace_event_call *call)
-{
-	int ret;
-
-	mutex_lock(&event_mutex);
-	ret = trace_remove_event_call_nolock(call);
-	mutex_unlock(&event_mutex);
-
-	return ret;
-}
-
 #define for_each_event(event, start, end)			\
 	for (event = start;					\
 	     (unsigned long)event < (unsigned long)end;		\
kernel/trace/trace_events_hist.c +3 -3
index 21e4954375a1..82e72c48a5a9 100644
@@ -960,7 +960,7 @@ static int register_synth_event(struct synth_event *event)
 	call->data = event;
 	call->tp = event->tp;
 
-	ret = trace_add_event_call_nolock(call);
+	ret = trace_add_event_call(call);
 	if (ret) {
 		pr_warn("Failed to register synthetic event: %s\n",
 			trace_event_name(call));

@@ -969,7 +969,7 @@ static int register_synth_event(struct synth_event *event)
 
 	ret = set_synth_event_print_fmt(call);
 	if (ret < 0) {
-		trace_remove_event_call_nolock(call);
+		trace_remove_event_call(call);
 		goto err;
 	}
  out:

@@ -984,7 +984,7 @@ static int unregister_synth_event(struct synth_event *event)
 	struct trace_event_call *call = &event->call;
 	int ret;
 
-	ret = trace_remove_event_call_nolock(call);
+	ret = trace_remove_event_call(call);
 
 	return ret;
 }
kernel/trace/trace_kprobe.c +2 -2
index bdf8c2ad5152..0e0f7b8024fb 100644
@@ -1353,7 +1353,7 @@ static int register_kprobe_event(struct trace_kprobe *tk)
 		kfree(call->print_fmt);
 		return -ENODEV;
 	}
-	ret = trace_add_event_call_nolock(call);
+	ret = trace_add_event_call(call);
 	if (ret) {
 		pr_info("Failed to register kprobe event: %s\n",
 			trace_event_name(call));

@@ -1368,7 +1368,7 @@ static int unregister_kprobe_event(struct trace_kprobe *tk)
 	int ret;
 
 	/* tp->event is unregistered in trace_remove_event_call() */
-	ret = trace_remove_event_call_nolock(&tk->tp.call);
+	ret = trace_remove_event_call(&tk->tp.call);
 	if (!ret)
 		kfree(tk->tp.call.print_fmt);
 	return ret;
kernel/trace/trace_uprobe.c +2 -2
index 4a7b21c891f3..e335576b9411 100644
@@ -1320,7 +1320,7 @@ static int register_uprobe_event(struct trace_uprobe *tu)
 		return -ENODEV;
 	}
 
-	ret = trace_add_event_call_nolock(call);
+	ret = trace_add_event_call(call);
 
 	if (ret) {
 		pr_info("Failed to register uprobe event: %s\n",

@@ -1337,7 +1337,7 @@ static int unregister_uprobe_event(struct trace_uprobe *tu)
 	int ret;
 
 	/* tu->event is unregistered in trace_remove_event_call() */
-	ret = trace_remove_event_call_nolock(&tu->tp.call);
+	ret = trace_remove_event_call(&tu->tp.call);
 	if (ret)
 		return ret;
 	kfree(tu->tp.call.print_fmt);
Signature · 1 line
2.19.2
Signature · 1 line
Masami Hiramatsu <[email protected]>
author maintainer
#15 lore raw

PATCH v2 11/12 tracing: Add generic event-name based remove event method

In reply to Masami Hiramatsu

Add a generic method to remove event from dynamic event list. This is same as other system under ftrace. You just need to pass the event name with '!', e.g.

  # echo p:new_grp/new_event _do_fork > dynamic_events

This creates an event, and

  # echo '!p:new_grp/new_event _do_fork' > dynamic_events

Or,

  # echo '!p:new_grp/new_event' > dynamic_events

will remove new_grp/new_event event.

Note that this doesn't check the event prefix (e.g. "p:") strictly, because the "group/event" name must be unique.

Signed-off-by Masami Hiramatsu <[email protected]>

---
Changes in v2:
- Instead of checking the given entire line strictly, simply checking the event and group name.

---
 kernel/trace/trace_dynevent.c |   15 +++++++++++----
 1 file changed, 11 insertions(+), 4 deletions(-)
kernel/trace/trace_dynevent.c +11 -4
index f17a887abb66..dd1f43588d70 100644
@@ -37,10 +37,17 @@ int dyn_event_release(int argc, char **argv, struct dyn_event_operations *type)
 	char *system = NULL, *event, *p;
 	int ret = -ENOENT;
 
-	if (argv[0][1] != ':')
-		return -EINVAL;
+	if (argv[0][0] == '-') {
+		if (argv[0][1] != ':')
+			return -EINVAL;
+		event = &argv[0][2];
+	} else {
+		event = strchr(argv[0], ':');
+		if (!event)
+			return -EINVAL;
+		event++;
+	}
 
-	event = &argv[0][2];
 	p = strchr(event, '/');
 	if (p) {
 		system = event;

@@ -69,7 +76,7 @@ static int create_dyn_event(int argc, char **argv)
 	struct dyn_event_operations *ops;
 	int ret;
 
-	if (argv[0][0] == '-')
+	if (argv[0][0] == '-' || argv[0][0] == '!')
 		return dyn_event_release(argc, argv, NULL);
 
 	mutex_lock(&dyn_event_ops_mutex);
author maintainer
#16 lore raw

PATCH v2 12/12 selftests/ftrace: Add testcases for dynamic event

In reply to Masami Hiramatsu

Add common testcases for dynamic_events interface.
- Add/remove kprobe events via dynamic_events
- Add/remove synthetic events via dynamic_events
- Selective clear events (clear events other interfaces)
- Genelic clear events ("!LINE" syntax)

Signed-off-by Masami Hiramatsu <[email protected]>
---
 .../ftrace/test.d/dynevent/add_remove_kprobe.tc    |   30 ++++++++++++
 .../ftrace/test.d/dynevent/add_remove_synth.tc     |   27 +++++++++++
 .../ftrace/test.d/dynevent/clear_select_events.tc  |   50 ++++++++++++++++++++
 .../ftrace/test.d/dynevent/generic_clear_event.tc  |   49 ++++++++++++++++++++
 4 files changed, 156 insertions(+)
 create mode 100644 tools/testing/selftests/ftrace/test.d/dynevent/add_remove_kprobe.tc
 create mode 100644 tools/testing/selftests/ftrace/test.d/dynevent/add_remove_synth.tc
 create mode 100644 tools/testing/selftests/ftrace/test.d/dynevent/clear_select_events.tc
 create mode 100644 tools/testing/selftests/ftrace/test.d/dynevent/generic_clear_event.tc
tools/testing/selftests/ftrace/test.d/dynevent/add_remove_kprobe.tc new file +30 -0
new file mode 100644
index 000000000000..c6d8387dbbb8
@@ -0,0 +1,30 @@
+#!/bin/sh
+# SPDX-License-Identifier: GPL-2.0
+# description: Generic dynamic event - add/remove kprobe events
+
+[ -f dynamic_events ] || exit_unsupported
+
+grep -q "place: \[<module>:\]<symbol>" README || exit_unsupported
+grep -q "place (kretprobe): \[<module>:\]<symbol>" README || exit_unsupported
+
+echo 0 > events/enable
+echo > dynamic_events
+
+PLACE=_do_fork
+
+echo "p:myevent1 $PLACE" >> dynamic_events
+echo "r:myevent2 $PLACE" >> dynamic_events
+
+grep -q myevent1 dynamic_events
+grep -q myevent2 dynamic_events
+test -d events/kprobes/myevent1
+test -d events/kprobes/myevent2
+
+echo "-:myevent2" >> dynamic_events
+
+grep -q myevent1 dynamic_events
+! grep -q myevent2 dynamic_events
+
+echo > dynamic_events
+
+clear_trace
tools/testing/selftests/ftrace/test.d/dynevent/add_remove_synth.tc new file +27 -0
new file mode 100644
index 000000000000..62b77b5941d0
@@ -0,0 +1,27 @@
+#!/bin/sh
+# SPDX-License-Identifier: GPL-2.0
+# description: Generic dynamic event - add/remove synthetic events
+
+[ -f dynamic_events ] || exit_unsupported
+
+grep -q "s:\[synthetic/\]" README || exit_unsupported
+
+echo 0 > events/enable
+echo > dynamic_events
+
+echo "s:latency1 u64 lat; pid_t pid;" >> dynamic_events
+echo "s:latency2 u64 lat; pid_t pid;" >> dynamic_events
+
+grep -q latency1 dynamic_events
+grep -q latency2 dynamic_events
+test -d events/synthetic/latency1
+test -d events/synthetic/latency2
+
+echo "-:synthetic/latency2" >> dynamic_events
+
+grep -q latency1 dynamic_events
+! grep -q latency2 dynamic_events
+
+echo > dynamic_events
+
+clear_trace
tools/testing/selftests/ftrace/test.d/dynevent/clear_select_events.tc new file +50 -0

50 lines not on this page · raw message

tools/testing/selftests/ftrace/test.d/dynevent/generic_clear_event.tc new file +49 -0

49 lines not on this page · raw message

author maintainer
#17 lore raw
In reply to Masami Hiramatsu

Ping?

Hi Tom,

This series, especially [09/12] tracing: Remove unneeded synth_event_mutex will effect your current working series. Please tell me your opinion.

Thank you,

Quoted 127 lines · On Mon, 5 Nov 2018 17:59:46 +0900 Masami Hiramatsu <[email protected]> wrote:

Hi,

This is v2 series of unifying dynamic event interface on ftrace. Currently ftrace has 3 dynamic event interfaces, kprobes, uprobes and synthetic. This series unifies those dynamic event interfaces to "dynamic_events" so that we can add other dynamic events easily on same interface, e.g. function events.
The older interfaces are left on the tracefs for backward compatibility.

dynamic_events syntax has no difference from kprobe_events and uprobe_events. You can use same syntax for dynamic_events interface. For synthetic events, similar to the probe events, dynamic_events adds "s:[GROUP/]" prefix, where the "GROUP/" must be "synthetic/".

s:[synthetic/]<event-name> type arg [type arg]...

E.g.

 $ echo 'wakeup_latency u64 lat pid_t pid char' > synthetic_events

is same as

 $ echo 's:wakeup_latency u64 lat pid_t pid char' > dynamic_events

Or

 $ echo 's:synthetic/wakeup_latency u64 lat pid_t pid char' > dynamic_events

This series modifies synthetic event interface behavior a bit, reorder lock dependency and related cleanups so that we can integrate the synthetic event to dynamic_events interface.

In this version, I changed the generic '!' erase command, which now supports entire line style like other interfaces. So you can delete events via dynamic_events as below

 $ cat dynamic_events | while read line; \
   do echo "!$line" >> dynamic_events; done

Also, the big change will be removing dyn_event_mutex and synth_event_mutex because all those parts are protected by event_mutex.

Changes from v2 are here;

New patches:
- Reorder event_mutex and synth_event_mutex to solve
AB-BA deadlock correctly. ([2/12])
- Simplify creation and deletion of synthetic event. ([3/12])
- Retern -ENOENT if there is no synthetic event when deleting ([4/12])
- Integrate similar probe argument parsers ([5/12])
- Use dyn_event framework for synthetic events ([9/12])
- Remove synth_event_mutex ([10/12])
- Remove unused APIs ([11/12])

Modified patches:
[6/12] - [8/12]
- Generalize delete event and export as dyn_event_release_all().
- Add match operation for find deleting event.
- Reorder event_mutex and dyn_event_mutex to solve lock dependency issue.
- Pass const char **argv for create operation and use -ECANCELED to signal for trying next dyn_event_operations.
- Remove dyn_event_mutex.

[12/12]
- Accept entire line, but instead of checking the given entire line strictly, simply checking the event and group name.

Tom, thanks for your Ack for v1 series. Since I changed many things from v1 (not only minor change), I decided to not add your Ack for this version. Anyway, what I've added in this version are related to synthetic events. I need your review for those. (especially removing synth_event_mutex)

You can try it from my git tree.

https://github.com/mhiramat/linux/tree/unify-dynamic-events-v2

Thank you,

---

Masami Hiramatsu (12):
tracing/uprobes: Add busy check when cleanup all uprobes
tracing: Lock event_mutex before synth_event_mutex
tracing: Simplify creation and deletion of synthetic event
tracing: Integrate similar probe argument parsers
tracing: Add unified dynamic event framework tracing/kprobes: Use dyn_event framework for kprobe events tracing/uprobes: Use dyn_event framework for uprobe events
tracing: Use dyn_event framework for synthetic events
tracing: Remove unneeded synth_event_mutex
tracing: Remove orphaned trace_add/remove_event_call functions
tracing: Add generic event-name based remove event method selftests/ftrace: Add testcases for dynamic event

 Documentation/trace/kprobetrace.rst                |    3 
 Documentation/trace/uprobetracer.rst               |    4 
 include/linux/trace_events.h                       |    4 
 kernel/trace/Kconfig                               |    6 
 kernel/trace/Makefile                              |    1 
 kernel/trace/trace.c                               |   12 +
 kernel/trace/trace_dynevent.c                      |  217 ++++++++++++
 kernel/trace/trace_dynevent.h                      |  119 +++++++
 kernel/trace/trace_events.c                        |   12 -
 kernel/trace/trace_events_hist.c                   |  322 ++++++++++--------
 kernel/trace/trace_kprobe.c                        |  357 ++++++++++----------
 kernel/trace/trace_probe.c                         |   74 ++++
 kernel/trace/trace_probe.h                         |    9 -
 kernel/trace/trace_uprobe.c                        |  305 ++++++++---------
 .../ftrace/test.d/dynevent/add_remove_kprobe.tc    |   30 ++
 .../ftrace/test.d/dynevent/add_remove_synth.tc     |   27 ++
 .../ftrace/test.d/dynevent/clear_select_events.tc  |   50 +++
 .../ftrace/test.d/dynevent/generic_clear_event.tc  |   49 +++
 18 files changed, 1094 insertions(+), 507 deletions(-)
 create mode 100644 kernel/trace/trace_dynevent.c
 create mode 100644 kernel/trace/trace_dynevent.h
 create mode 100644 tools/testing/selftests/ftrace/test.d/dynevent/add_remove_kprobe.tc
 create mode 100644 tools/testing/selftests/ftrace/test.d/dynevent/add_remove_synth.tc
 create mode 100644 tools/testing/selftests/ftrace/test.d/dynevent/clear_select_events.tc
 create mode 100644 tools/testing/selftests/ftrace/test.d/dynevent/generic_clear_event.tc

--
Masami Hiramatsu (Linaro) <[email protected]>

Signature · 1 line
Masami Hiramatsu <[email protected]>
Reviewed-by Tested-by
#18 lore raw

Hi Masami,

Quoted 8 lines · Masami Hiramatsu

Ping?

Hi Tom,

This series, especially [09/12] tracing: Remove unneeded synth_event_mutex
will effect your current working series. Please tell me your opinion.

Sorry for the delay in reviewing this - I completely forgot about it in my inbox.

It's all very nice, and the mutex updates along with the dyn_event management make that part of the code much cleaner, thanks!

In any case, you can have my

Reviewed-by Tom Zanussi <[email protected]>
Tested-by Tom Zanussi <[email protected]>

Thanks,

Tom

Quoted 152 lines

Thank you,

Quoted 145 lines · On Mon, 5 Nov 2018 17:59:46 +0900 Masami Hiramatsu <[email protected]> wrote:

Hi,

This is v2 series of unifying dynamic event interface on ftrace. Currently ftrace has 3 dynamic event interfaces, kprobes, uprobes and synthetic. This series unifies those dynamic event interfaces to "dynamic_events" so that we can add other dynamic events easily on same interface, e.g. function events.
The older interfaces are left on the tracefs for backward compatibility.

dynamic_events syntax has no difference from kprobe_events and uprobe_events. You can use same syntax for dynamic_events interface.
For synthetic events, similar to the probe events, dynamic_events adds "s:[GROUP/]" prefix, where the "GROUP/" must be "synthetic/".

s:[synthetic/]<event-name> type arg [type arg]...

E.g.

 $ echo 'wakeup_latency u64 lat pid_t pid char' > synthetic_events

is same as

 $ echo 's:wakeup_latency u64 lat pid_t pid char' > dynamic_events

Or

 $ echo 's:synthetic/wakeup_latency u64 lat pid_t pid char' >
dynamic_events

This series modifies synthetic event interface behavior a bit, reorder lock dependency and related cleanups so that we can integrate
the synthetic event to dynamic_events interface.

In this version, I changed the generic '!' erase command, which now supports entire line style like other interfaces. So you can delete events via dynamic_events as below

 $ cat dynamic_events | while read line; \
   do echo "!$line" >> dynamic_events; done

Also, the big change will be removing dyn_event_mutex and synth_event_mutex because all those parts are protected by event_mutex.

Changes from v2 are here;

New patches:
- Reorder event_mutex and synth_event_mutex to solve AB-BA deadlock correctly. ([2/12])
- Simplify creation and deletion of synthetic event. ([3/12])
- Retern -ENOENT if there is no synthetic event when deleting
([4/12])
- Integrate similar probe argument parsers ([5/12])
- Use dyn_event framework for synthetic events ([9/12])
- Remove synth_event_mutex ([10/12])
- Remove unused APIs ([11/12])

Modified patches:
[6/12] - [8/12]
- Generalize delete event and export as dyn_event_release_all().
- Add match operation for find deleting event.
- Reorder event_mutex and dyn_event_mutex to solve lock dependency issue.
- Pass const char **argv for create operation and use -ECANCELED
to
signal for trying next dyn_event_operations.
- Remove dyn_event_mutex.

[12/12]
- Accept entire line, but instead of checking the given entire
line
strictly, simply checking the event and group name.

Tom, thanks for your Ack for v1 series. Since I changed many things from v1 (not only minor change), I decided to not add your Ack for this version. Anyway, what I've added in this version are related to
synthetic events. I need your review for those. (especially removing synth_event_mutex)

You can try it from my git tree.

https://github.com/mhiramat/linux/tree/unify-dynamic-events-v2

Thank you,

---

Masami Hiramatsu (12):
tracing/uprobes: Add busy check when cleanup all uprobes
tracing: Lock event_mutex before synth_event_mutex
tracing: Simplify creation and deletion of synthetic event
tracing: Integrate similar probe argument parsers
tracing: Add unified dynamic event framework tracing/kprobes: Use dyn_event framework for kprobe events tracing/uprobes: Use dyn_event framework for uprobe events
tracing: Use dyn_event framework for synthetic events
tracing: Remove unneeded synth_event_mutex
tracing: Remove orphaned trace_add/remove_event_call
functions
tracing: Add generic event-name based remove event method selftests/ftrace: Add testcases for dynamic event

 Documentation/trace/kprobetrace.rst                |    3 
 Documentation/trace/uprobetracer.rst               |    4 
 include/linux/trace_events.h                       |    4 
 kernel/trace/Kconfig                               |    6 
 kernel/trace/Makefile                              |    1 
 kernel/trace/trace.c                               |   12 +
 kernel/trace/trace_dynevent.c                      |  217
++++++++++++
 kernel/trace/trace_dynevent.h                      |  119 +++++++
 kernel/trace/trace_events.c                        |   12 -
 kernel/trace/trace_events_hist.c                   |  322
++++++++++--------
 kernel/trace/trace_kprobe.c                        |  357
++++++++++----------
 kernel/trace/trace_probe.c                         |   74 ++++
 kernel/trace/trace_probe.h                         |    9 -
 kernel/trace/trace_uprobe.c                        |  305
++++++++---------
 .../ftrace/test.d/dynevent/add_remove_kprobe.tc    |   30 ++
 .../ftrace/test.d/dynevent/add_remove_synth.tc     |   27 ++
 .../ftrace/test.d/dynevent/clear_select_events.tc  |   50 +++
 .../ftrace/test.d/dynevent/generic_clear_event.tc  |   49 +++
 18 files changed, 1094 insertions(+), 507 deletions(-)
 create mode 100644 kernel/trace/trace_dynevent.c
 create mode 100644 kernel/trace/trace_dynevent.h
 create mode 100644
tools/testing/selftests/ftrace/test.d/dynevent/add_remove_kprobe.tc
 create mode 100644
tools/testing/selftests/ftrace/test.d/dynevent/add_remove_synth.tc
 create mode 100644
tools/testing/selftests/ftrace/test.d/dynevent/clear_select_events.
tc
 create mode 100644

tools/testing/selftests/ftrace/test.d/dynevent/generic_clear_event. tc

--
Masami Hiramatsu (Linaro) <[email protected]>

maintainer
#19 lore raw
Quoted 15 lines · On Wed, 28 Nov 2018 17:42:22 -0600 Tom Zanussi <[email protected]>…

Hi Masami,

Quoted 8 lines · Masami Hiramatsu

Ping?

Hi Tom,

This series, especially [09/12] tracing: Remove unneeded synth_event_mutex
will effect your current working series. Please tell me your opinion.

Sorry for the delay in reviewing this - I completely forgot about it in my inbox.

Quoted 9 lines

It's all very nice, and the mutex updates along with the dyn_event management make that part of the code much cleaner, thanks!

In any case, you can have my

Reviewed-by Tom Zanussi <[email protected]>
Tested-by Tom Zanussi <[email protected]>

Thanks Tom! I'll take a look at this tomorrow.

-- Steve

author maintainer
#20 lore raw
In reply to Tom Zanussi

Hi Tom,

Quoted 15 lines · On Wed, 28 Nov 2018 17:42:22 -0600 Tom Zanussi <[email protected]>…

Hi Masami,

Quoted 8 lines · Masami Hiramatsu

Ping?

Hi Tom,

This series, especially [09/12] tracing: Remove unneeded synth_event_mutex
will effect your current working series. Please tell me your opinion.

Sorry for the delay in reviewing this - I completely forgot about it in my inbox.

Quoted 7 lines

It's all very nice, and the mutex updates along with the dyn_event management make that part of the code much cleaner, thanks!

In any case, you can have my

Reviewed-by Tom Zanussi <[email protected]>
Tested-by Tom Zanussi <[email protected]>

Thank you!
And as I pointed, synth_event_mutex is removed by this series, it may affect your hist trigger series too.

Thanks,

Signature · 1 line
Masami Hiramatsu <[email protected]>
#21 lore raw

Hi Masami,

Quoted 25 lines · Masami Hiramatsu

Hi Tom,

Quoted 20 lines · On Wed, 28 Nov 2018 17:42:22 -0600 Tom Zanussi <[email protected]>…

Hi Masami,

Quoted 9 lines · Masami Hiramatsu

Ping?

Hi Tom,

This series, especially [09/12] tracing: Remove unneeded synth_event_mutex
will effect your current working series. Please tell me your opinion.

Sorry for the delay in reviewing this - I completely forgot about it in
my inbox.

It's all very nice, and the mutex updates along with the dyn_event management make that part of the code much cleaner, thanks!

Quoted 9 lines
Quoted 4 lines

In any case, you can have my

Reviewed-by Tom Zanussi <[email protected]>
Tested-by Tom Zanussi <[email protected]>

Thank you!
And as I pointed, synth_event_mutex is removed by this series, it may affect your hist trigger series too.

Yes, thanks for pointing that out - I'll rebase mine on top of this.

Tom