* [PATCH 1/7] tracing: Add trace_array_find() to find instance trace arrays
2019-12-18 15:27 [PATCH 0/7] tracing: Add support for in-kernel synthetic event API Tom Zanussi
@ 2019-12-18 15:27 ` Tom Zanussi
2019-12-18 15:27 ` [PATCH 2/7] tracing: Add get/put_event_file() Tom Zanussi
` (6 subsequent siblings)
7 siblings, 0 replies; 12+ messages in thread
From: Tom Zanussi @ 2019-12-18 15:27 UTC (permalink / raw)
To: rostedt; +Cc: artem.bityutskiy, mhiramat, linux-kernel, linux-rt-users
Add a new trace_array_find() function that can be used to find a trace
array given the instance name, and replace existing code that does the
same thing with it.
Also make it available for use outside of trace.c.
Signed-off-by: Tom Zanussi <zanussi@kernel.org>
---
kernel/trace/trace.c | 30 ++++++++++++++++++++----------
kernel/trace/trace.h | 1 +
2 files changed, 21 insertions(+), 10 deletions(-)
diff --git a/kernel/trace/trace.c b/kernel/trace/trace.c
index 23459d53d576..5483b50a11ff 100644
--- a/kernel/trace/trace.c
+++ b/kernel/trace/trace.c
@@ -8455,6 +8455,21 @@ static void update_tracer_options(struct trace_array *tr)
mutex_unlock(&trace_types_lock);
}
+/* Must have event_mutex and trace_types_lock held */
+struct trace_array *trace_array_find(const char *name)
+{
+ struct trace_array *tr, *found = NULL;
+
+ list_for_each_entry(tr, &ftrace_trace_arrays, list) {
+ if (tr->name && strcmp(tr->name, name) == 0) {
+ found = tr;
+ break;
+ }
+ }
+
+ return found;
+}
+
static struct trace_array *trace_array_create(const char *name)
{
struct trace_array *tr;
@@ -8531,10 +8546,8 @@ static int instance_mkdir(const char *name)
mutex_lock(&trace_types_lock);
ret = -EEXIST;
- list_for_each_entry(tr, &ftrace_trace_arrays, list) {
- if (tr->name && strcmp(tr->name, name) == 0)
- goto out_unlock;
- }
+ if (trace_array_find(name))
+ goto out_unlock;
tr = trace_array_create(name);
@@ -8658,12 +8671,9 @@ static int instance_rmdir(const char *name)
mutex_lock(&trace_types_lock);
ret = -ENODEV;
- list_for_each_entry(tr, &ftrace_trace_arrays, list) {
- if (tr->name && strcmp(tr->name, name) == 0) {
- ret = __remove_instance(tr);
- break;
- }
- }
+ tr = trace_array_find(name);
+ if (tr)
+ ret = __remove_instance(tr);
mutex_unlock(&trace_types_lock);
mutex_unlock(&event_mutex);
diff --git a/kernel/trace/trace.h b/kernel/trace/trace.h
index 63bf60f79398..f37ef6524821 100644
--- a/kernel/trace/trace.h
+++ b/kernel/trace/trace.h
@@ -346,6 +346,7 @@ extern struct mutex trace_types_lock;
extern int trace_array_get(struct trace_array *tr);
extern int tracing_check_open_get_tr(struct trace_array *tr);
+extern struct trace_array *trace_array_find(const char *name);
extern int tracing_set_time_stamp_abs(struct trace_array *tr, bool abs);
extern int tracing_set_clock(struct trace_array *tr, const char *clockstr);
--
2.14.1
^ permalink raw reply related [flat|nested] 12+ messages in thread* [PATCH 2/7] tracing: Add get/put_event_file()
2019-12-18 15:27 [PATCH 0/7] tracing: Add support for in-kernel synthetic event API Tom Zanussi
2019-12-18 15:27 ` [PATCH 1/7] tracing: Add trace_array_find() to find instance trace arrays Tom Zanussi
@ 2019-12-18 15:27 ` Tom Zanussi
2019-12-18 15:27 ` [PATCH 3/7] tracing: Add delete_synth_event() Tom Zanussi
` (5 subsequent siblings)
7 siblings, 0 replies; 12+ messages in thread
From: Tom Zanussi @ 2019-12-18 15:27 UTC (permalink / raw)
To: rostedt; +Cc: artem.bityutskiy, mhiramat, linux-kernel, linux-rt-users
Add a function to get an event file and prevent it from going away on
module or instance removal.
get_event_file() will find an event file in a given instance (if
instance is NULL, it assumes the top trace array) and return it,
pinning the instance's trace array as well as the event's module, if
applicable, so they won't go away while in use.
put_event_file() does the matching release.
Also included are _nolock() versions, which can be used if event_mutex
is already held.
Signed-off-by: Tom Zanussi <zanussi@kernel.org>
---
include/linux/trace_events.h | 10 ++++
kernel/trace/trace_events.c | 130 +++++++++++++++++++++++++++++++++++++++++++
2 files changed, 140 insertions(+)
diff --git a/include/linux/trace_events.h b/include/linux/trace_events.h
index 4c6e15605766..cf982c7d6636 100644
--- a/include/linux/trace_events.h
+++ b/include/linux/trace_events.h
@@ -348,6 +348,16 @@ enum {
EVENT_FILE_FL_WAS_ENABLED_BIT,
};
+extern struct trace_event_file *get_event_file(const char *instance,
+ const char *system,
+ const char *event);
+extern struct trace_event_file *get_event_file_nolock(const char *instance,
+ const char *system,
+ const char *event);
+
+extern void put_event_file(struct trace_event_file *file);
+extern void put_event_file_nolock(struct trace_event_file *file);
+
/*
* Event file flags:
* ENABLED - The event is enabled
diff --git a/kernel/trace/trace_events.c b/kernel/trace/trace_events.c
index c6de3cebc127..2ee417d003eb 100644
--- a/kernel/trace/trace_events.c
+++ b/kernel/trace/trace_events.c
@@ -2535,6 +2535,136 @@ find_event_file(struct trace_array *tr, const char *system, const char *event)
return file;
}
+static struct trace_event_file *__get_event_file(const char *instance,
+ const char *system,
+ const char *event,
+ bool lock)
+{
+ struct trace_array *tr = top_trace_array();
+ struct trace_event_file *file = NULL;
+ int ret = -EINVAL;
+
+ if (instance) {
+ tr = trace_array_find(instance);
+ if (!tr)
+ return ERR_PTR(ret);
+ }
+
+ ret = trace_array_get(tr);
+ if (ret)
+ return ERR_PTR(ret);
+
+ if (lock)
+ mutex_lock(&event_mutex);
+
+ file = find_event_file(tr, system, event);
+ if (!file) {
+ trace_array_put(tr);
+ ret = -EINVAL;
+ goto out;
+ }
+
+ /* Don't let event modules unload while in use */
+ ret = try_module_get(file->event_call->mod);
+ if (!ret) {
+ trace_array_put(tr);
+ ret = -EBUSY;
+ goto out;
+ }
+
+ ret = 0;
+ out:
+ if (lock)
+ mutex_unlock(&event_mutex);
+
+ if (ret)
+ file = ERR_PTR(ret);
+
+ return file;
+}
+
+/**
+ * get_event_file - Find and return a trace event file
+ * @instance: The name of the trace instance containing the event
+ * @system: The name of the system containing the event
+ * @event: The name of the event
+ *
+ * Return a trace event file given the trace instance name, trace
+ * system, and trace event name. If the instance name is NULL, it
+ * refers to the top-level trace array.
+ *
+ * This function will look it up and return it if found, after calling
+ * trace_array_get() to prevent the instance from going away, and
+ * increment the event's module refcount to prevent it from being
+ * removed.
+ *
+ * To release the file, call put_event_file(), which will call
+ * trace_array_put() and decrement the event's module refcount.
+ *
+ * Return: The trace event on success, ERR_PTR otherwise.
+ */
+struct trace_event_file *get_event_file(const char *instance,
+ const char *system,
+ const char *event)
+{
+ return __get_event_file(instance, system, event, true);
+}
+EXPORT_SYMBOL_GPL(get_event_file);
+
+/**
+ * get_event_file_nolock - non-locking version of get_event_file
+ *
+ * Same as get_event_file() but doesn't take event_mutex. See
+ * get_event_file() for details.
+ */
+struct trace_event_file *get_event_file_nolock(const char *instance,
+ const char *system,
+ const char *event)
+{
+ return __get_event_file(instance, system, event, false);
+}
+EXPORT_SYMBOL_GPL(get_event_file_nolock);
+
+/**
+ * put_event_file - Release a file from get_event_file()
+ * @file: The trace event file
+ *
+ * If a file was retrieved using get_event_file(), this should be
+ * called when it's no longer needed. It will cancel the previous
+ * trace_array_get() called by that function, and decrement the
+ * event's module refcount.
+ */
+void __put_event_file(struct trace_event_file *file, bool lock)
+{
+ trace_array_put(file->tr);
+
+ if (lock)
+ mutex_lock(&event_mutex);
+
+ module_put(file->event_call->mod);
+
+ if (lock)
+ mutex_unlock(&event_mutex);
+}
+
+void put_event_file(struct trace_event_file *file)
+{
+ return __put_event_file(file, true);
+}
+EXPORT_SYMBOL_GPL(put_event_file);
+
+/**
+ * put_event_file_nolock - non-locking version of put_event_file
+ *
+ * Same as put_event_file() but doesn't take event_mutex. See
+ * put_event_file() for details.
+ */
+void put_event_file_nolock(struct trace_event_file *file)
+{
+ return __put_event_file(file, false);
+}
+EXPORT_SYMBOL_GPL(put_event_file_nolock);
+
#ifdef CONFIG_DYNAMIC_FTRACE
/* Avoid typos */
--
2.14.1
^ permalink raw reply related [flat|nested] 12+ messages in thread* [PATCH 3/7] tracing: Add delete_synth_event()
2019-12-18 15:27 [PATCH 0/7] tracing: Add support for in-kernel synthetic event API Tom Zanussi
2019-12-18 15:27 ` [PATCH 1/7] tracing: Add trace_array_find() to find instance trace arrays Tom Zanussi
2019-12-18 15:27 ` [PATCH 2/7] tracing: Add get/put_event_file() Tom Zanussi
@ 2019-12-18 15:27 ` Tom Zanussi
2019-12-18 15:27 ` [PATCH 4/7] tracing: Add create_synth_event() Tom Zanussi
` (4 subsequent siblings)
7 siblings, 0 replies; 12+ messages in thread
From: Tom Zanussi @ 2019-12-18 15:27 UTC (permalink / raw)
To: rostedt; +Cc: artem.bityutskiy, mhiramat, linux-kernel, linux-rt-users
create_or_delete_synth_event() contains code to delete a synthetic
event, which would be useful on its own - specifically, it would be
useful to allow event-creating modules to call it separately.
Separate out the delete code from that function and create an exported
function named delete_synth_event().
Signed-off-by: Tom Zanussi <zanussi@kernel.org>
---
include/linux/trace_events.h | 2 ++
kernel/trace/trace_events_hist.c | 57 +++++++++++++++++++++++++++++-----------
2 files changed, 43 insertions(+), 16 deletions(-)
diff --git a/include/linux/trace_events.h b/include/linux/trace_events.h
index cf982c7d6636..0c36a58cea43 100644
--- a/include/linux/trace_events.h
+++ b/include/linux/trace_events.h
@@ -358,6 +358,8 @@ extern struct trace_event_file *get_event_file_nolock(const char *instance,
extern void put_event_file(struct trace_event_file *file);
extern void put_event_file_nolock(struct trace_event_file *file);
+extern int delete_synth_event(const char *name);
+
/*
* Event file flags:
* ENABLED - The event is enabled
diff --git a/kernel/trace/trace_events_hist.c b/kernel/trace/trace_events_hist.c
index f49d1a36d3ae..8c9894681100 100644
--- a/kernel/trace/trace_events_hist.c
+++ b/kernel/trace/trace_events_hist.c
@@ -1335,29 +1335,54 @@ static int __create_synth_event(int argc, const char *name, const char **argv)
goto out;
}
+static int destroy_synth_event(struct synth_event *se)
+{
+ int ret;
+
+ if (se->ref)
+ ret = -EBUSY;
+ else {
+ ret = unregister_synth_event(se);
+ if (!ret) {
+ dyn_event_remove(&se->devent);
+ free_synth_event(se);
+ }
+ }
+
+ return ret;
+}
+
+/**
+ * delete_synth_event - Delete a synthetic event
+ * @event_name: The name of the new sythetic event
+ *
+ * Delete a synthetic event that was created with create_synth_event().
+ *
+ * Return: 0 if successful, error otherwise.
+ */
+int delete_synth_event(const char *event_name)
+{
+ struct synth_event *se = NULL;
+ int ret = -ENOENT;
+
+ mutex_lock(&event_mutex);
+ se = find_synth_event(event_name);
+ if (se)
+ ret = destroy_synth_event(se);
+ mutex_unlock(&event_mutex);
+
+ return ret;
+}
+EXPORT_SYMBOL_GPL(delete_synth_event);
+
static int create_or_delete_synth_event(int argc, char **argv)
{
const char *name = argv[0];
- struct synth_event *event = NULL;
int ret;
/* trace_run_command() ensures argc != 0 */
if (name[0] == '!') {
- mutex_lock(&event_mutex);
- event = find_synth_event(name + 1);
- if (event) {
- if (event->ref)
- ret = -EBUSY;
- else {
- ret = unregister_synth_event(event);
- if (!ret) {
- dyn_event_remove(&event->devent);
- free_synth_event(event);
- }
- }
- } else
- ret = -ENOENT;
- mutex_unlock(&event_mutex);
+ ret = delete_synth_event(name + 1);
return ret;
}
--
2.14.1
^ permalink raw reply related [flat|nested] 12+ messages in thread* [PATCH 4/7] tracing: Add create_synth_event()
2019-12-18 15:27 [PATCH 0/7] tracing: Add support for in-kernel synthetic event API Tom Zanussi
` (2 preceding siblings ...)
2019-12-18 15:27 ` [PATCH 3/7] tracing: Add delete_synth_event() Tom Zanussi
@ 2019-12-18 15:27 ` Tom Zanussi
2019-12-18 15:27 ` [PATCH 5/7] tracing: Add generate_synth_event() and related functions Tom Zanussi
` (3 subsequent siblings)
7 siblings, 0 replies; 12+ messages in thread
From: Tom Zanussi @ 2019-12-18 15:27 UTC (permalink / raw)
To: rostedt; +Cc: artem.bityutskiy, mhiramat, linux-kernel, linux-rt-users
Add an exported function named create_synth_event(), allowing modules
or other kernel code to create a synthetic event.
create_synth_event() is actually a higher-level function composed of
the subfunctions create_empty_synth_event(), finalize_synth_event(),
and add_synth_field(). These functions and the related
add_synth_fields() are also exported so that users who want to
dynamically create an event can do so.
If the event passed to delete_synth_event() is associated with a
module, it also resets the trace buffer as similar functionality that
removes trace events does elsewhere.
Signed-off-by: Tom Zanussi <zanussi@kernel.org>
---
include/linux/trace_events.h | 21 +++
kernel/trace/trace_events_hist.c | 297 ++++++++++++++++++++++++++++++++++++++-
2 files changed, 316 insertions(+), 2 deletions(-)
diff --git a/include/linux/trace_events.h b/include/linux/trace_events.h
index 0c36a58cea43..8b385778b2db 100644
--- a/include/linux/trace_events.h
+++ b/include/linux/trace_events.h
@@ -358,8 +358,29 @@ extern struct trace_event_file *get_event_file_nolock(const char *instance,
extern void put_event_file(struct trace_event_file *file);
extern void put_event_file_nolock(struct trace_event_file *file);
+struct synth_field_desc {
+ const char *type;
+ const char *name;
+};
+
+struct synth_event;
+
+extern int create_synth_event(const char *name,
+ struct synth_field_desc *fields,
+ unsigned int n_fields,
+ struct module *mod);
+extern void free_synth_event(struct synth_event *event);
extern int delete_synth_event(const char *name);
+extern struct synth_event *create_empty_synth_event(const char *name,
+ struct module *mod);
+extern int add_synth_field(struct synth_event *event, const char *field_type,
+ const char *field_name);
+extern int add_synth_fields(struct synth_event *event,
+ struct synth_field_desc *fields,
+ unsigned int n_fields);
+extern int finalize_synth_event(struct synth_event *event);
+
/*
* Event file flags:
* ENABLED - The event is enabled
diff --git a/kernel/trace/trace_events_hist.c b/kernel/trace/trace_events_hist.c
index 8c9894681100..4b8d7a4bac2d 100644
--- a/kernel/trace/trace_events_hist.c
+++ b/kernel/trace/trace_events_hist.c
@@ -407,6 +407,7 @@ struct synth_event {
struct trace_event_class class;
struct trace_event_call call;
struct tracepoint *tp;
+ struct module *mod;
};
static bool is_synth_event(struct dyn_event *ev)
@@ -1198,7 +1199,7 @@ static int unregister_synth_event(struct synth_event *event)
return ret;
}
-static void free_synth_event(struct synth_event *event)
+void free_synth_event(struct synth_event *event)
{
unsigned int i;
@@ -1215,6 +1216,7 @@ static void free_synth_event(struct synth_event *event)
free_synth_event_print_fmt(&event->call);
kfree(event);
}
+EXPORT_SYMBOL(free_synth_event);
static struct synth_event *alloc_synth_event(const char *name, int n_fields,
struct synth_field **fields)
@@ -1267,6 +1269,278 @@ struct hist_var_data {
struct hist_trigger_data *hist_data;
};
+/**
+ * finalize_synth_event - Finalize and register a new synth event
+ * @event: A pointer to the synth_event struct representing the new event
+ *
+ * Register a new synth event only if an event with the same name
+ * doesn't already exist.
+ *
+ * Return: 0 on success, ERR otherwise.
+ */
+int finalize_synth_event(struct synth_event *event)
+{
+ int ret;
+
+ mutex_lock(&event_mutex);
+
+ if (find_synth_event(event->name)) {
+ ret = -EEXIST;
+ goto out;
+ }
+
+ ret = register_synth_event(event);
+ if (!ret)
+ ret = dyn_event_add(&event->devent);
+ out:
+ mutex_unlock(&event_mutex);
+
+ return ret;
+}
+EXPORT_SYMBOL_GPL(finalize_synth_event);
+
+static int update_fields(struct synth_event *event, struct synth_field *field)
+{
+ struct synth_field **old_fields;
+ unsigned int i, n_fields;
+
+ old_fields = event->fields;
+
+ n_fields = event->n_fields + 1;
+
+ event->fields = kcalloc(n_fields, sizeof(*event->fields), GFP_KERNEL);
+ if (!event->fields)
+ return -ENOMEM;
+
+ /* if field_name contains [n] it's an array */
+ for (i = 0; i < n_fields - 1; i++)
+ event->fields[i] = old_fields[i];
+
+ event->fields[n_fields - 1] = field;
+
+ event->n_fields = n_fields;
+
+ return 0;
+}
+
+/**
+ * add_synth_field - Add a new field to a synthetic event
+ * @event: A pointer to the synth_event struct representing the new event
+ * @field_type: The type of the new field to add
+ * @field_name: The name of the new field to add
+ *
+ * Add a new field to a synthetic event object. Field ordering is in
+ * the same order the fields are added.
+ *
+ * See synth_field_size() for available types. If field_name contains
+ * [n] the field is considered to be an array.
+ *
+ * Return: 0 if successful, error otherwise.
+ */
+int add_synth_field(struct synth_event *event, const char *field_type,
+ const char *field_name)
+{
+ struct synth_field *field;
+ const char *array;
+ int len, ret = 0;
+
+ field = kzalloc(sizeof(*field), GFP_KERNEL);
+ if (!field)
+ return -ENOMEM;
+
+ len = strlen(field_name);
+ array = strchr(field_name, '[');
+ if (array)
+ len -= strlen(array);
+
+ field->name = kmemdup_nul(field_name, len, GFP_KERNEL);
+ if (!field->name) {
+ ret = -ENOMEM;
+ goto free;
+ }
+
+ len = strlen(field_type) + 1;
+ if (array)
+ len += strlen(array);
+
+ field->type = kzalloc(len, GFP_KERNEL);
+ if (!field->type) {
+ ret = -ENOMEM;
+ goto free;
+ }
+
+ strcat(field->type, field_type);
+ if (array)
+ strcat(field->type, array);
+
+ field->size = synth_field_size(field->type);
+ if (!field->size) {
+ ret = -EINVAL;
+ goto free;
+ }
+
+ if (synth_field_is_string(field->type))
+ field->is_string = true;
+
+ field->is_signed = synth_field_signed(field->type);
+
+ ret = update_fields(event, field);
+ out:
+ return ret;
+ free:
+ free_synth_field(field);
+ goto out;
+}
+EXPORT_SYMBOL_GPL(add_synth_field);
+
+/**
+ * add_synth_fields - Add a new field to a synthetic event
+ * @event: A pointer to the synth_event struct representing the new event
+ * @fields: An array of type/name field descriptions
+ * @n_fields: The number of field descriptions contained in the fields array
+ *
+ * Add a new set of fields to a synthetic event object. The event
+ * fields that will be defined for the event should be passed in as an
+ * array of struct synth_field_desc, and the number of elements in the
+ * array passed in as n_fields. Field ordering will retain the
+ * ordering given in the fields array.
+ *
+ * See synth_field_size() for available types. If field_name contains
+ * [n] the field is considered to be an array.
+ *
+ * Return: 0 if successful, error otherwise.
+ */
+int add_synth_fields(struct synth_event *event,
+ struct synth_field_desc *fields, unsigned int n_fields)
+{
+ unsigned int i;
+ int ret = 0;
+
+ for (i = 0; i < n_fields; i++) {
+ if (fields[i].type == NULL || fields[i].name == NULL) {
+ ret = -EINVAL;
+ break;
+ }
+
+ ret = add_synth_field(event, fields[i].type, fields[i].name);
+ if (ret)
+ break;
+ }
+
+ return ret;
+}
+EXPORT_SYMBOL_GPL(add_synth_fields);
+
+/**
+ * create_empty_synth_event - Create a synth event to be populated with fields
+ * @name: The name of the synthetic event to create
+ * @mod: The module creating the event, NULL if not created from a module
+ *
+ * Allocate and initialize a new synth_event struct for a new
+ * synthetic event.
+ *
+ * The new synthetic event should be populated with fields using one
+ * or more calls to add_synth_field() or add_synth_fields() and then
+ * finalized and registered using finalize_synth_event().
+ *
+ * If the new synthetic event is being created from a module, the mod
+ * param must be non-NULL. This will ensure that the trace buffer
+ * won't contain unreadable events.
+ *
+ * The new synth event should be deleted using delete_synth_event() if
+ * registration was successful using finalize_synth_event(). If not,
+ * free_synth_event() should be used.
+ *
+ * Return: A pointer to the synth_event struct representing the new
+ * synth event, ERR_PTR otherwise.
+ */
+struct synth_event *create_empty_synth_event(const char *name,
+ struct module *mod)
+{
+ struct synth_event *event;
+
+ event = kzalloc(sizeof(*event), GFP_KERNEL);
+ if (!event) {
+ event = ERR_PTR(-ENOMEM);
+ goto out;
+ }
+
+ event->name = kstrdup(name, GFP_KERNEL);
+ if (!event->name) {
+ kfree(event);
+ event = ERR_PTR(-ENOMEM);
+ goto out;
+ }
+
+ event->mod = mod;
+
+ dyn_event_init(&event->devent, &synth_event_ops);
+ out:
+ return event;
+}
+EXPORT_SYMBOL_GPL(create_empty_synth_event);
+
+/**
+ * create_synth_event - Create a new synthetic event
+ * @name: The name of the new sythetic event
+ * @fields: An array of type/name field descriptions
+ * @n_fields: The number of field descriptions contained in the fields array
+ * @mod: The module creating the event, NULL if not created from a module
+ *
+ * Create a new synthetic event with the given name under the
+ * trace/events/synthetic/ directory. The event fields that will be
+ * defined for the event should be passed in as an array of struct
+ * synth_field_desc, and the number elements in the array passed in as
+ * n_fields. Field ordering will retain the ordering given in the
+ * fields array.
+ *
+ * If the new synthetic event is being created from a module, the mod
+ * param must be non-NULL. This will ensure that the trace buffer
+ * won't contain unreadable events.
+ *
+ * The new synth event should be deleted using delete_synth_event()
+ * function. The new synthetic event can be generated from modules or
+ * other kernel code using generate_synth_event().
+ *
+ * Return: 0 if successful, error otherwise.
+ */
+int create_synth_event(const char *name, struct synth_field_desc *fields,
+ unsigned int n_fields, struct module *mod)
+{
+ struct synth_event *se;
+ int ret = -EINVAL;
+ unsigned int i;
+
+ if (n_fields > SYNTH_FIELDS_MAX)
+ return ret;
+
+ se = create_empty_synth_event(name, mod);
+ if (IS_ERR(se))
+ return PTR_ERR(se);
+
+ for (i = 0; i < n_fields; i++) {
+ if (fields[i].type == NULL || fields[i].name == NULL) {
+ ret = -EINVAL;
+ goto free;
+ }
+
+ ret = add_synth_field(se, fields[i].type, fields[i].name);
+ if (ret)
+ goto free;
+ }
+
+ ret = finalize_synth_event(se);
+ if (ret)
+ goto free;
+ out:
+ return ret;
+ free:
+ free_synth_event(se);
+
+ goto out;
+}
+EXPORT_SYMBOL_GPL(create_synth_event);
+
static int __create_synth_event(int argc, const char *name, const char **argv)
{
struct synth_field *field, *fields[SYNTH_FIELDS_MAX];
@@ -1363,14 +1637,33 @@ static int destroy_synth_event(struct synth_event *se)
int delete_synth_event(const char *event_name)
{
struct synth_event *se = NULL;
+ struct module *mod = NULL;
int ret = -ENOENT;
mutex_lock(&event_mutex);
se = find_synth_event(event_name);
- if (se)
+ if (se) {
+ mod = se->mod;
ret = destroy_synth_event(se);
+ }
mutex_unlock(&event_mutex);
+ if (mod) {
+ mutex_lock(&trace_types_lock);
+ /*
+ * It is safest to reset the ring buffer if the module
+ * being unloaded registered any events that were
+ * used. The only worry is if a new module gets
+ * loaded, and takes on the same id as the events of
+ * this module. When printing out the buffer, traced
+ * events left over from this module may be passed to
+ * the new module events and unexpected results may
+ * occur.
+ */
+ tracing_reset_all_online_cpus();
+ mutex_unlock(&trace_types_lock);
+ }
+
return ret;
}
EXPORT_SYMBOL_GPL(delete_synth_event);
--
2.14.1
^ permalink raw reply related [flat|nested] 12+ messages in thread* [PATCH 5/7] tracing: Add generate_synth_event() and related functions
2019-12-18 15:27 [PATCH 0/7] tracing: Add support for in-kernel synthetic event API Tom Zanussi
` (3 preceding siblings ...)
2019-12-18 15:27 ` [PATCH 4/7] tracing: Add create_synth_event() Tom Zanussi
@ 2019-12-18 15:27 ` Tom Zanussi
2019-12-18 15:27 ` [PATCH 6/7] tracing: Add synth event generation test module Tom Zanussi
` (2 subsequent siblings)
7 siblings, 0 replies; 12+ messages in thread
From: Tom Zanussi @ 2019-12-18 15:27 UTC (permalink / raw)
To: rostedt; +Cc: artem.bityutskiy, mhiramat, linux-kernel, linux-rt-users
Add an exported function named generate_synth_event(), allowing
modules or other kernel code to generate synthetic events.
Also added are several functions that allow the same functionality to
be broken out in a piecewise fashion, which are useful in situations
where generating an event from a full array of values would be
cumbersome. Those functions are generate_synth_event_start/end() and
add_(next)_synth_val().
Signed-off-by: Tom Zanussi <zanussi@kernel.org>
---
include/linux/trace_events.h | 20 +++
kernel/trace/trace_events_hist.c | 370 +++++++++++++++++++++++++++++++++++++++
2 files changed, 390 insertions(+)
diff --git a/include/linux/trace_events.h b/include/linux/trace_events.h
index 8b385778b2db..8a413b9cff40 100644
--- a/include/linux/trace_events.h
+++ b/include/linux/trace_events.h
@@ -365,6 +365,18 @@ struct synth_field_desc {
struct synth_event;
+struct synth_gen_state {
+ struct trace_event_buffer fbuffer;
+ struct synth_trace_event *entry;
+ struct ring_buffer *buffer;
+ struct synth_event *event;
+ unsigned int cur_field;
+ unsigned int n_u64;
+ bool enabled;
+ bool add_next;
+ bool add_name;
+};
+
extern int create_synth_event(const char *name,
struct synth_field_desc *fields,
unsigned int n_fields,
@@ -380,6 +392,14 @@ extern int add_synth_fields(struct synth_event *event,
struct synth_field_desc *fields,
unsigned int n_fields);
extern int finalize_synth_event(struct synth_event *event);
+extern int generate_synth_event(struct trace_event_file *file, u64 *vals,
+ unsigned int n_vals);
+extern int generate_synth_event_start(struct trace_event_file *file,
+ struct synth_gen_state *gen_state);
+extern int add_next_synth_val(u64 val, struct synth_gen_state *gen_state);
+extern int add_synth_val(const char *field_name, u64 val,
+ struct synth_gen_state *gen_state);
+extern int generate_synth_event_end(struct synth_gen_state *gen_state);
/*
* Event file flags:
diff --git a/kernel/trace/trace_events_hist.c b/kernel/trace/trace_events_hist.c
index 4b8d7a4bac2d..426c385f941f 100644
--- a/kernel/trace/trace_events_hist.c
+++ b/kernel/trace/trace_events_hist.c
@@ -393,6 +393,7 @@ struct synth_field {
char *type;
char *name;
size_t size;
+ unsigned int offset;
bool is_signed;
bool is_string;
};
@@ -662,6 +663,8 @@ static int synth_event_define_fields(struct trace_event_call *call)
if (ret)
break;
+ event->fields[i]->offset = n_u64;
+
if (event->fields[i]->is_string) {
offset += STR_VAR_LEN_MAX;
n_u64 += STR_VAR_LEN_MAX / sizeof(u64);
@@ -1269,6 +1272,90 @@ struct hist_var_data {
struct hist_trigger_data *hist_data;
};
+/**
+ * generate_synth_event - Generate a synthetic event
+ * @file: The trace_event_file representing the synthetic event
+ * @vals: Array of values
+ * @n_vals: The number of values in vals
+ *
+ * Generate a synthetic event using the values passed in as 'vals'.
+ *
+ * The 'vals' array is just an array of 'n_vals' u64. The number of
+ * vals must match the number of field in the synthetic event, and
+ * must be in the same order as the synthetic event fields.
+ *
+ * All vals should be cast to u64, and string vals are just pointers
+ * to strings, cast to u64. Strings will be copied into space
+ * reserved in the event for the string, using these pointers.
+ *
+ * Return: 0 on success, err otherwise.
+ */
+int generate_synth_event(struct trace_event_file *file, u64 *vals,
+ unsigned int n_vals)
+{
+ struct trace_event_buffer fbuffer;
+ struct synth_trace_event *entry;
+ struct ring_buffer *buffer;
+ struct synth_event *event;
+ unsigned int i, n_u64;
+ int fields_size = 0;
+ int ret = 0;
+
+ /*
+ * Normal event generation doesn't get called at all unless
+ * the ENABLED bit is set (which attaches the probe thus
+ * allowing this code to be called, etc). Because this is
+ * called directly by the user, we don't have that but we
+ * still need to honor not logging when disabled.
+ */
+ if (!(file->flags & EVENT_FILE_FL_ENABLED))
+ return 0;
+
+ event = file->event_call->data;
+
+ if (n_vals != event->n_fields)
+ return -EINVAL;
+
+ if (trace_trigger_soft_disabled(file))
+ return -EINVAL;
+
+ fields_size = event->n_u64 * sizeof(u64);
+
+ /*
+ * Avoid ring buffer recursion detection, as this event
+ * is being performed within another event.
+ */
+ buffer = file->tr->trace_buffer.buffer;
+ ring_buffer_nest_start(buffer);
+
+ entry = trace_event_buffer_reserve(&fbuffer, file,
+ sizeof(*entry) + fields_size);
+ if (!entry) {
+ ret = -EINVAL;
+ goto out;
+ }
+
+ for (i = 0, n_u64 = 0; i < event->n_fields; i++) {
+ if (event->fields[i]->is_string) {
+ char *str_val = (char *)(long)vals[i];
+ char *str_field = (char *)&entry->fields[n_u64];
+
+ strscpy(str_field, str_val, STR_VAR_LEN_MAX);
+ n_u64 += STR_VAR_LEN_MAX / sizeof(u64);
+ } else {
+ entry->fields[n_u64] = vals[i];
+ n_u64++;
+ }
+ }
+
+ trace_event_buffer_commit(&fbuffer);
+out:
+ ring_buffer_nest_end(buffer);
+
+ return ret;
+}
+EXPORT_SYMBOL_GPL(generate_synth_event);
+
/**
* finalize_synth_event - Finalize and register a new synth event
* @event: A pointer to the synth_event struct representing the new event
@@ -1299,6 +1386,289 @@ int finalize_synth_event(struct synth_event *event)
}
EXPORT_SYMBOL_GPL(finalize_synth_event);
+/**
+ * generate_synth_event_start - Start piecewise synthetic event generation
+ * @file: The trace_event_file representing the synthetic event
+ * @gen_state: A pointer to object tracking the piecewise generation state
+ *
+ * Start the generation of a synthetic event field-by-field rather
+ * than all at once.
+ *
+ * This function 'opens' an event generation, which means space is
+ * reserved for the event in the trace buffer, after which the event's
+ * individual field values can be set through either
+ * add_next_synth_val() or add_synth_val().
+ *
+ * A pointer to a gen_state object is passed in, which will keep track
+ * of the current event generation state until the event generation is
+ * closed (and the event finally generated) using
+ * generate_synth_event_end().
+ *
+ * Note that generate_synth_event_end() must be called after all
+ * values have been added for each event generation, regardless of
+ * whether adding all field values succeeded or not.
+ *
+ * Note also that for a given event generation, all fields must be
+ * added using either add_next_synth_val() or add_synth_val() but not
+ * both together or interleaved.
+ *
+ * Return: 0 on success, err otherwise.
+ */
+int generate_synth_event_start(struct trace_event_file *file,
+ struct synth_gen_state *gen_state)
+{
+ struct synth_trace_event *entry;
+ int fields_size = 0;
+ int ret = 0;
+
+ if (!gen_state) {
+ ret = -EINVAL;
+ goto out;
+ }
+
+ memset(gen_state, '\0', sizeof(*gen_state));
+
+ /*
+ * Normal event generation doesn't get called at all unless
+ * the ENABLED bit is set (which attaches the probe thus
+ * allowing this code to be called, etc). Because this is
+ * called directly by the user, we don't have that but we
+ * still need to honor not logging when disabled. For the the
+ * iterated gen case, we save the enabed state upon start and
+ * just ignore the following data calls.
+ */
+ if (!(file->flags & EVENT_FILE_FL_ENABLED)) {
+ gen_state->enabled = false;
+ goto out;
+ }
+
+ gen_state->enabled = true;
+
+ gen_state->event = file->event_call->data;
+
+ if (trace_trigger_soft_disabled(file)) {
+ ret = -EINVAL;
+ goto out;
+ }
+
+ fields_size = gen_state->event->n_u64 * sizeof(u64);
+
+ /*
+ * Avoid ring buffer recursion detection, as this event
+ * is being performed within another event.
+ */
+ gen_state->buffer = file->tr->trace_buffer.buffer;
+ ring_buffer_nest_start(gen_state->buffer);
+
+ entry = trace_event_buffer_reserve(&gen_state->fbuffer, file,
+ sizeof(*entry) + fields_size);
+ if (!entry) {
+ ret = -EINVAL;
+ goto out;
+ }
+
+ gen_state->entry = entry;
+out:
+ return ret;
+}
+EXPORT_SYMBOL_GPL(generate_synth_event_start);
+
+static int save_synth_val(struct synth_field *field, u64 val,
+ struct synth_gen_state *gen_state)
+{
+ struct synth_trace_event *entry = gen_state->entry;
+
+ if (field->is_string) {
+ char *str_val = (char *)(long)val;
+ char *str_field;
+
+ if (!str_val)
+ return -EINVAL;
+
+ str_field = (char *)&entry->fields[field->offset];
+ strscpy(str_field, str_val, STR_VAR_LEN_MAX);
+ } else
+ entry->fields[field->offset] = val;
+
+ return 0;
+}
+
+/**
+ * add_next_synth_val - Add the next field's value to an open synth generation
+ * @val: The value to set the next field to
+ * @gen_state: A pointer to object tracking the piecewise generation state
+ *
+ * Set the value of the next field in an event that's been opened by
+ * generate_synth_event_start().
+ *
+ * The val param should be the value cast to u64. If the value points
+ * to a string, the val param should be a char * cast to u64.
+ *
+ * This function assumes all the fields in an event are to be set one
+ * after another - successive calls to this function are made, one for
+ * each field, in the order of the fields in the event, until all
+ * fields have been set. If you'd rather set each field individually
+ * without regard to ordering, add_synth_val() can be used instead.
+ *
+ * Note however that add_next_synth_val() and add_synth_val() can't be
+ * intermixed for a given event generation - one or the other but not
+ * both can be used at the same time.
+ *
+ * Note also that generate_synth_event_end() must be called after all
+ * values have been added for each event generation, regardless of
+ * whether adding all field values succeeded or not.
+ *
+ * Return: 0 on success, err otherwise.
+ */
+int add_next_synth_val(u64 val, struct synth_gen_state *gen_state)
+{
+ struct synth_field *field;
+ struct synth_event *event;
+ int ret = 0;
+
+ if (!gen_state) {
+ ret = -EINVAL;
+ goto out;
+ }
+
+ /* can't mix add_next_synth_val() with add_synth_val() */
+ if (gen_state->add_name) {
+ ret = -EINVAL;
+ goto out;
+ }
+ gen_state->add_next = true;
+
+ if (!gen_state->enabled)
+ goto out;
+
+ event = gen_state->event;
+
+ if (gen_state->cur_field >= event->n_fields) {
+ ret = -EINVAL;
+ goto out;
+ }
+
+ field = event->fields[gen_state->cur_field++];
+ ret = save_synth_val(field, val, gen_state);
+ out:
+ return ret;
+}
+EXPORT_SYMBOL_GPL(add_next_synth_val);
+
+static struct synth_field *find_synth_field(struct synth_event *event,
+ const char *field_name)
+{
+ struct synth_field *field = NULL;
+ unsigned int i;
+
+ for (i = 0; i < event->n_fields; i++) {
+ field = event->fields[i];
+ if (strcmp(field->name, field_name) == 0)
+ return field;
+ }
+
+ return NULL;
+}
+
+/**
+ * add_synth_val - Add a named field's value to an open synth generation
+ * @field_name: The name of the synthetic event field value to set
+ * @val: The value to set the next field to
+ * @gen_state: A pointer to object tracking the piecewise generation state
+ *
+ * Set the value of the named field in an event that's been opened by
+ * generate_synth_event_start().
+ *
+ * The val param should be the value cast to u64. If the value points
+ * to a string, the val param should be a char * cast to u64.
+ *
+ * This function looks up the field name, and if found, sets the field
+ * to the specified value. This lookup makes this function more
+ * expensive than add_next_synth_val(), so use that or the
+ * none-piecewise generate_synth_event() instead if efficiency is more
+ * important.
+ *
+ * Note however that add_next_synth_val() and add_synth_val() can't be
+ * intermixed for a given event generation - one or the other but not
+ * both can be used at the same time.
+ *
+ * Note also that generate_synth_event_end() must be called after all
+ * values have been added for each event generation, regardless of
+ * whether adding all field values succeeded or not.
+ *
+ * Return: 0 on success, err otherwise.
+ */
+int add_synth_val(const char *field_name, u64 val,
+ struct synth_gen_state *gen_state)
+{
+ struct synth_trace_event *entry;
+ struct synth_event *event;
+ struct synth_field *field;
+ int ret = 0;
+
+ if (!gen_state) {
+ ret = -EINVAL;
+ goto out;
+ }
+
+ /* can't mix add_next_synth_val() with add_synth_val() */
+ if (gen_state->add_next) {
+ ret = -EINVAL;
+ goto out;
+ }
+ gen_state->add_name = true;
+
+ if (!gen_state->enabled)
+ goto out;
+
+ event = gen_state->event;
+ entry = gen_state->entry;
+
+ field = find_synth_field(event, field_name);
+ if (!field) {
+ ret = -EINVAL;
+ goto out;
+ }
+
+ ret = save_synth_val(field, val, gen_state);
+ out:
+ return ret;
+}
+EXPORT_SYMBOL_GPL(add_synth_val);
+
+/**
+ * generate_synth_event_end - End piecewise synthetic event generation
+ * @gen_state: A pointer to object tracking the piecewise generation state
+ *
+ * End the generation of a synthetic event opened by
+ * generate_synth_event_start().
+ *
+ * This function 'closes' an event generation, which basically means
+ * that it commits the reserved event and cleans up other loose ends.
+ *
+ * A pointer to a gen_state object is passed in, which will keep track
+ * of the current event generation state opened with
+ * generate_synth_event_start().
+ *
+ * Note that this function must be called after all values have been
+ * added for each event generation, regardless of whether adding all
+ * field values succeeded or not.
+ *
+ * Return: 0 on success, err otherwise.
+ */
+int generate_synth_event_end(struct synth_gen_state *gen_state)
+{
+ if (!gen_state)
+ return -EINVAL;
+
+ trace_event_buffer_commit(&gen_state->fbuffer);
+
+ ring_buffer_nest_end(gen_state->buffer);
+
+ return 0;
+}
+EXPORT_SYMBOL_GPL(generate_synth_event_end);
+
static int update_fields(struct synth_event *event, struct synth_field *field)
{
struct synth_field **old_fields;
--
2.14.1
^ permalink raw reply related [flat|nested] 12+ messages in thread* [PATCH 6/7] tracing: Add synth event generation test module
2019-12-18 15:27 [PATCH 0/7] tracing: Add support for in-kernel synthetic event API Tom Zanussi
` (4 preceding siblings ...)
2019-12-18 15:27 ` [PATCH 5/7] tracing: Add generate_synth_event() and related functions Tom Zanussi
@ 2019-12-18 15:27 ` Tom Zanussi
2019-12-18 15:27 ` [PATCH 7/7] tracing: Documentation for in-kernel synthetic event API Tom Zanussi
2019-12-19 14:45 ` [PATCH 0/7] tracing: Add support " Masami Hiramatsu
7 siblings, 0 replies; 12+ messages in thread
From: Tom Zanussi @ 2019-12-18 15:27 UTC (permalink / raw)
To: rostedt; +Cc: artem.bityutskiy, mhiramat, linux-kernel, linux-rt-users
Add a test module that checks the basic functionality of the in-kernel
synthetic event generation API by creating and generating synthetic
events from a module.
Signed-off-by: Tom Zanussi <zanussi@kernel.org>
---
kernel/trace/Kconfig | 13 ++
kernel/trace/Makefile | 1 +
kernel/trace/synth_event_gen_test.c | 330 ++++++++++++++++++++++++++++++++++++
3 files changed, 344 insertions(+)
create mode 100644 kernel/trace/synth_event_gen_test.c
diff --git a/kernel/trace/Kconfig b/kernel/trace/Kconfig
index 29a9c5058b62..0597d60158b4 100644
--- a/kernel/trace/Kconfig
+++ b/kernel/trace/Kconfig
@@ -775,6 +775,19 @@ config PREEMPTIRQ_DELAY_TEST
If unsure, say N
+config SYNTH_EVENT_GEN_TEST
+ tristate "Test module for in-kernel synthetic event generation"
+ depends on HIST_TRIGGERS
+ help
+ This option creates a test module to check the base
+ functionality of in-kernel synthetic event definition and
+ generation.
+
+ To test, insert the module, and then check the trace buffer
+ for the generated sample events.
+
+ If unsure, say N.
+
config TRACE_EVAL_MAP_FILE
bool "Show eval mappings for trace events"
depends on TRACING
diff --git a/kernel/trace/Makefile b/kernel/trace/Makefile
index 0e63db62225f..99de699c6062 100644
--- a/kernel/trace/Makefile
+++ b/kernel/trace/Makefile
@@ -44,6 +44,7 @@ obj-$(CONFIG_TRACING) += trace_stat.o
obj-$(CONFIG_TRACING) += trace_printk.o
obj-$(CONFIG_TRACING_MAP) += tracing_map.o
obj-$(CONFIG_PREEMPTIRQ_DELAY_TEST) += preemptirq_delay_test.o
+obj-$(CONFIG_SYNTH_EVENT_GEN_TEST) += synth_event_gen_test.o
obj-$(CONFIG_CONTEXT_SWITCH_TRACER) += trace_sched_switch.o
obj-$(CONFIG_FUNCTION_TRACER) += trace_functions.o
obj-$(CONFIG_PREEMPTIRQ_TRACEPOINTS) += trace_preemptirq.o
diff --git a/kernel/trace/synth_event_gen_test.c b/kernel/trace/synth_event_gen_test.c
new file mode 100644
index 000000000000..f6a008cad489
--- /dev/null
+++ b/kernel/trace/synth_event_gen_test.c
@@ -0,0 +1,330 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Test module for in-kernel sythetic event creation and generation.
+ *
+ * Copyright (C) 2019 Tom Zanussi <zanussi@kernel.org>
+ */
+
+#include <linux/module.h>
+#include <linux/trace_events.h>
+#include "trace.h"
+
+/*
+ * This module is a simple test of basic functionality for in-kernel
+ * synthetic event creation and generation, the first test using
+ * create_synth_event() with a static field array and a version that
+ * builds the same thing dynamically using create_empty_synth_event(),
+ * add_synth_field(s), and finalize_synth_event().
+ *
+ * To test, select CONFIG_SYNTH_EVENT_GEN_TEST and build the module.
+ * Then:
+ *
+ * # insmod kernel/trace/synth_event_gen_test.ko
+ * # cat /sys/kernel/debug/tracing/trace
+ *
+ * You should see two events in the trace buffer - "synthtest" and
+ * "dyn_synthtest".
+ *
+ * To remove the events, remove the module:
+ *
+ * # rmmod synth_event_gen_test
+ *
+ */
+
+static struct synth_field_desc synthtest_fields[] = {
+ { .type = "pid_t", .name = "next_pid_field" },
+ { .type = "char[16]", .name = "next_comm_field" },
+ { .type = "u64", .name = "ts_ns" },
+ { .type = "u64", .name = "ts_ms" },
+ { .type = "unsigned int", .name = "cpu" },
+ { .type = "char[64]", .name = "my_string_field" },
+ { .type = "int", .name = "my_int_field" },
+};
+
+static struct trace_event_file *synthtest_event_file;
+static struct trace_event_file *dyn_synthtest_event_file;
+
+static int __init test_synth_event(void)
+{
+ u64 vals[7];
+ int ret;
+
+ /* Create the synthtest synthetic event with the fields above */
+ ret = create_synth_event("synthtest", synthtest_fields,
+ ARRAY_SIZE(synthtest_fields), THIS_MODULE);
+ if (ret)
+ goto out;
+
+ /*
+ * Now get the synthtest event file. We need to prevent the
+ * instance and event from disappearing from underneath us,
+ * which get_event_file() does (though in this case we're
+ * using the top-level instance which never goes away).
+ */
+ synthtest_event_file = get_event_file(NULL, "synthetic", "synthtest");
+ if (IS_ERR(synthtest_event_file)) {
+ ret = PTR_ERR(synthtest_event_file);
+ goto delete;
+ }
+
+ /* Enable the event or you won't see anything */
+ ret = trace_array_set_clr_event(synthtest_event_file->tr,
+ "synthetic", "synthtest", true);
+ if (ret) {
+ put_event_file(synthtest_event_file);
+ goto delete;
+ }
+
+ /* Create some bogus values just for testing */
+
+ vals[0] = 777; /* next_pid_field */
+ vals[1] = (u64)"tiddlywinks"; /* next_comm_field */
+ vals[2] = 1000000; /* ts_ns */
+ vals[3] = 1000; /* ts_ms */
+ vals[4] = smp_processor_id(); /* cpu */
+ vals[5] = (u64)"thneed"; /* my_string_field */
+ vals[6] = 398; /* my_int_field */
+
+ /* Now generate the event */
+ ret = generate_synth_event(synthtest_event_file, vals,
+ ARRAY_SIZE(vals));
+ out:
+ return ret;
+ delete:
+ ret = delete_synth_event("synthtest");
+
+ goto out;
+}
+
+static int __init test_dynamic_synth_event(void)
+{
+ struct synth_event *se = NULL;
+ u64 vals[7];
+ int ret;
+
+ /* Create the empty synthtest synthetic event */
+ se = create_empty_synth_event("dyn_synthtest", THIS_MODULE);
+ if (IS_ERR(se)) {
+ ret = PTR_ERR(se);
+ goto free;
+ }
+
+ /* Use add_synth_fields to add the first 4 synthtest fields */
+ ret = add_synth_fields(se, synthtest_fields, 4);
+ if (ret)
+ goto out;
+
+ /* Use add_synth_field to add the rest of the fields */
+
+ ret = add_synth_field(se, "unsigned int", "cpu");
+ if (ret)
+ goto free;
+
+ ret = add_synth_field(se, "char[64]", "my_string_field");
+ if (ret)
+ goto free;
+
+ ret = add_synth_field(se, "int", "my_int_field");
+ if (ret)
+ goto free;
+
+ /* All fields have been added, close and register the synth event */
+ ret = finalize_synth_event(se);
+ if (ret)
+ goto free;
+
+ /*
+ * Now get the dyn_synthtest event file. We need to prevent
+ * the instance and event from disappearing from underneath
+ * us, which get_event_file() does (though in this case we're
+ * using the top-level instance which never goes away).
+ */
+ dyn_synthtest_event_file = get_event_file(NULL, "synthetic",
+ "dyn_synthtest");
+ if (IS_ERR(dyn_synthtest_event_file)) {
+ ret = PTR_ERR(dyn_synthtest_event_file);
+ goto delete;
+ }
+
+ /* Enable the event or you won't see anything */
+ ret = trace_array_set_clr_event(dyn_synthtest_event_file->tr,
+ "synthetic", "dyn_synthtest", true);
+ if (ret) {
+ put_event_file(dyn_synthtest_event_file);
+ goto delete;
+ }
+
+ /* Create some bogus values just for testing */
+
+ vals[0] = 777; /* next_pid_field */
+ vals[1] = (u64)"tiddlywinks"; /* next_comm_field */
+ vals[2] = 1000000; /* ts_ns */
+ vals[3] = 1000; /* ts_ms */
+ vals[4] = smp_processor_id(); /* cpu */
+ vals[5] = (u64)"thneed_2.0"; /* my_string_field */
+ vals[6] = 399; /* my_int_field */
+
+ /* Now generate the event */
+ ret = generate_synth_event(dyn_synthtest_event_file, vals,
+ ARRAY_SIZE(vals));
+ out:
+ return ret;
+ free: /* If not finalized, just free */
+ free_synth_event(se);
+ goto out;
+ delete: /* Event was finalized, delete */
+ delete_synth_event("dyn_synthtest");
+ goto out;
+}
+
+static int __init test_add_next_synth_val(void)
+{
+ struct synth_gen_state gen_state;
+ int ret;
+
+ ret = generate_synth_event_start(synthtest_event_file, &gen_state);
+ if (ret)
+ return ret;
+
+ /* Add some bogus values just for testing */
+
+ /* next_pid_field */
+ ret = add_next_synth_val(777, &gen_state);
+ if (ret)
+ goto out;
+
+ /* next_comm_field */
+ ret = add_next_synth_val((u64)"slinky", &gen_state);
+ if (ret)
+ goto out;
+
+ /* ts_ns */
+ ret = add_next_synth_val(1000000, &gen_state);
+ if (ret)
+ goto out;
+
+ /* ts_ms */
+ ret = add_next_synth_val(1000, &gen_state);
+ if (ret)
+ goto out;
+
+ /* cpu */
+ ret = add_next_synth_val(smp_processor_id(), &gen_state);
+ if (ret)
+ goto out;
+
+ /* my_string_field */
+ ret = add_next_synth_val((u64)"thneed_2.01", &gen_state);
+ if (ret)
+ goto out;
+
+ /* my_int_field */
+ ret = add_next_synth_val(395, &gen_state);
+ out:
+ /* Now generate or at least finalize the event */
+ ret = generate_synth_event_end(&gen_state);
+
+ return ret;
+}
+
+static int __init test_add_synth_val(void)
+{
+ struct synth_gen_state gen_state;
+ int ret;
+
+ ret = generate_synth_event_start(synthtest_event_file, &gen_state);
+ if (ret)
+ return ret;
+
+ /* Add some bogus values just for testing */
+
+ ret = add_synth_val("next_pid_field", 777, &gen_state);
+ if (ret)
+ goto out;
+
+ ret = add_synth_val("next_comm_field", (u64)"silly putty", &gen_state);
+ if (ret)
+ goto out;
+
+ ret = add_synth_val("ts_ns", 1000000, &gen_state);
+ if (ret)
+ goto out;
+
+ ret = add_synth_val("ts_ms", 1000, &gen_state);
+ if (ret)
+ goto out;
+
+ ret = add_synth_val("cpu", smp_processor_id(), &gen_state);
+ if (ret)
+ goto out;
+
+ ret = add_synth_val("my_string_field", (u64)"thneed_9", &gen_state);
+ if (ret)
+ goto out;
+
+ ret = add_synth_val("my_int_field", 3999, &gen_state);
+ out:
+ /* Now generate or at least finalize the event */
+ ret = generate_synth_event_end(&gen_state);
+
+ return ret;
+}
+
+static int __init synth_event_gen_test_init(void)
+{
+ int ret;
+
+ ret = test_synth_event();
+ if (ret)
+ return ret;
+
+ ret = test_dynamic_synth_event();
+ if (ret) {
+ WARN_ON(trace_array_set_clr_event(synthtest_event_file->tr,
+ "synthetic",
+ "synthtest", false));
+ put_event_file(synthtest_event_file);
+ WARN_ON(delete_synth_event("synthtest"));
+ goto out;
+ }
+
+ ret = test_add_next_synth_val();
+ WARN_ON(ret);
+
+ ret = test_add_synth_val();
+ WARN_ON(ret);
+ out:
+ return ret;
+}
+
+static void __exit synth_event_gen_test_exit(void)
+{
+ /* Disable the event or you can't remove it */
+ WARN_ON(trace_array_set_clr_event(dyn_synthtest_event_file->tr,
+ "synthetic",
+ "dyn_synthtest", false));
+
+ /* Now give the file and instance back */
+ put_event_file(dyn_synthtest_event_file);
+
+ /* Now unregister and free the synthetic event */
+ WARN_ON(delete_synth_event("dyn_synthtest"));
+
+ /* Disable the event or you can't remove it */
+ WARN_ON(trace_array_set_clr_event(synthtest_event_file->tr,
+ "synthetic",
+ "synthtest", false));
+
+ /* Now give the file and instance back */
+ put_event_file(synthtest_event_file);
+
+ /* Now unregister and free the synthetic event */
+ WARN_ON(delete_synth_event("synthtest"));
+}
+
+module_init(synth_event_gen_test_init)
+module_exit(synth_event_gen_test_exit)
+
+MODULE_AUTHOR("Tom Zanussi");
+MODULE_DESCRIPTION("synthetic event generation test");
+MODULE_LICENSE("GPL v2");
--
2.14.1
^ permalink raw reply related [flat|nested] 12+ messages in thread* [PATCH 7/7] tracing: Documentation for in-kernel synthetic event API
2019-12-18 15:27 [PATCH 0/7] tracing: Add support for in-kernel synthetic event API Tom Zanussi
` (5 preceding siblings ...)
2019-12-18 15:27 ` [PATCH 6/7] tracing: Add synth event generation test module Tom Zanussi
@ 2019-12-18 15:27 ` Tom Zanussi
2019-12-19 14:45 ` [PATCH 0/7] tracing: Add support " Masami Hiramatsu
7 siblings, 0 replies; 12+ messages in thread
From: Tom Zanussi @ 2019-12-18 15:27 UTC (permalink / raw)
To: rostedt; +Cc: artem.bityutskiy, mhiramat, linux-kernel, linux-rt-users
Add Documentation for creating and generating synthetic events from
modules.
Signed-off-by: Tom Zanussi <zanussi@kernel.org>
---
Documentation/trace/events.rst | 268 +++++++++++++++++++++++++++++++++++++++++
1 file changed, 268 insertions(+)
diff --git a/Documentation/trace/events.rst b/Documentation/trace/events.rst
index f7e1fcc0953c..084d983df3a2 100644
--- a/Documentation/trace/events.rst
+++ b/Documentation/trace/events.rst
@@ -525,3 +525,271 @@ The following commands are supported:
event counts (hitcount).
See Documentation/trace/histogram.rst for details and examples.
+
+6.3 In-kernel trace event API
+-----------------------------
+
+In most cases, the command-line interface to trace events is more than
+sufficient. Sometimes, however, applications might find the need for
+more complex relationships than can be expressed through a simple
+series of linked command-line expressions, or putting together sets of
+commands may be simply too cumbersome. An example might be an
+application that needs to 'listen' to the trace stream in order to
+maintain an in-kernel state machine detecting, for instance, when an
+illegal kernel state occurs in the scheduler.
+
+The trace event subsystem provides an in-kernel API allowing modules
+or other kernel code to generate user-defined 'synthetic' events at
+will, which can be used to either augment the existing trace stream
+and/or signal that a particular important state has occurred.
+
+The API provided for these purposes is describe below and allows the
+following:
+
+ - dynamically creating synthetic event definitions
+ - generating synthetic events from in-kernel code
+
+6.3.1 Dyamically creating synthetic event definitions
+-----------------------------------------------------
+
+There are a couple ways to create a new synthetic event from a kernel
+module or other kernel code.
+
+The first creates the event in one step, using create_synth_event().
+In this method, the name of the event to create and an array defining
+the fields is supplied to create_synth_event(). If successful, a
+synthetic event with that name and fields will exist following that
+call. For example, to create a new "schedtest" synthetic event:
+
+ ret = create_synth_event("schedtest", sched_fields,
+ ARRAY_SIZE(sched_fields), THIS_MODULE);
+
+The sched_fields param in this example points to an array of struct
+synth_field_desc, each of which describes an event field by type and
+name:
+
+ static struct synth_field_desc sched_fields[] = {
+ { .type = "pid_t", .name = "next_pid_field" },
+ { .type = "char[16]", .name = "next_comm_field" },
+ { .type = "u64", .name = "ts_ns" },
+ { .type = "u64", .name = "ts_ms" },
+ { .type = "unsigned int", .name = "cpu" },
+ { .type = "char[64]", .name = "my_string_field" },
+ { .type = "int", .name = "my_int_field" },
+ };
+
+See synth_field_size() for available types. If field_name contains [n]
+the field is considered to be an array.
+
+If the event is created from within a module, a pointer to the module
+must be passed to create_synth_event(). This will ensure that the
+trace buffer won't contain unreadable events when the module is
+removed.
+
+At this point, the event object is ready to be used for generating new
+events.
+
+In the second method, the event is created in several steps. This
+allows events to be created dynamically and without the need to create
+and populate an array of fields beforehand.
+
+To use this method, an empty synthetic event should first be created
+using create_empty_synth_event(). The name of the event should be
+supplied to this function. For example, to create a new "schedtest"
+synthetic event:
+
+ struct synth_event *se = create_empty_synth_event("schedtest", THIS_MODULE);
+
+Once the synthetic event object has been created, it can then be
+populated with fields. Fields are added one by one using
+add_synth_field(), supplying the new synthetic event object, a field
+type, and a field name. For example, to add a new int field named
+"intfield", the following call should be made:
+
+ ret = add_synth_field(se, "int", "intfield");
+
+See synth_field_size() for available types. If field_name contains [n]
+the field is considered to be an array.
+
+A group of fields can also be added all at once using an array of
+synth_field_desc with add_synth_fields(). For example, this would add
+just the first four sched_fields:
+
+ ret = add_synth_fields(se, sched_fields, 4);
+
+Once all the fields have been added, the event should be finalized and
+registered by calling the finalize_synth_event() function:
+
+ ret = finalize_synth_event(se);
+
+At this point, the event object is ready to be used for generating new
+events.
+
+6.3.3 Generating synthetic events from in-kernel code
+-----------------------------------------------------
+
+To generate a synthetic event, there are a couple of options. The
+first option is to generate the event in one call, using
+generate_synth_event() with an array of values to be set. A second
+option can be used to avoid the need for a pre-formed array of values,
+using generate_synth_event_start() and generate_synth_event_end()
+along with add_next_synth_val() or add_synth_val() to add the values
+piecewise.
+
+6.3.3.1 Generating a synthetic event all at once
+------------------------------------------------
+
+To generate a synthetic event all at once, the generate_synth_event()
+function is used. It's passed the trace_event_file representing the
+synthetic event (which can be retrieved using get_event_file() using
+the synthetic event name, "synthetic" as the system name, and the
+trace instance name (NULL if using the global trace array)), along
+with an array of u64, one for each synthetic event field.
+
+So, to generate an event corresponding to the synthetic event
+definition above, code like the following could be used:
+
+ u64 vals[7];
+
+ vals[0] = 777; /* next_pid_field */
+ vals[1] = (u64)"tiddlywinks"; /* next_comm_field */
+ vals[2] = 1000000; /* ts_ns */
+ vals[3] = 1000; /* ts_ms */
+ vals[4] = smp_processor_id(); /* cpu */
+ vals[5] = (u64)"thneed"; /* my_string_field */
+ vals[6] = 398; /* my_int_field */
+
+The 'vals' array is just an array of u64, the number of which must
+match the number of field in the synthetic event, and which must be in
+the same order as the synthetic event fields.
+
+All vals should be cast to u64, and string vals are just pointers to
+strings, cast to u64. Strings will be copied into space reserved in
+the event for the string, using these pointers.
+
+In order to generate a synthetic event, a pointer to the trace event
+file is needed. The get_event_file() function can be used to get it -
+it will find the file in the given trace instance (in this case NULL
+since the top trace array is being used) while at the same time
+preventing the instance containing it from going away:
+
+ schedtest_event_file = get_event_file(NULL, "synthetic",
+ "schedtest");
+
+Before generating the event, it should be enabled in some way,
+otherwise the synthetic event won't actually show up in the trace
+buffer.
+
+To enable a synthetic event from the kernel, trace_array_set_clr_event()
+can be used (which is not specific to synthetic events, so does need
+the "synthetic" system name to be specified explicitly).
+
+To enable the event, pass 'true' to it:
+
+ trace_array_set_clr_event(schedtest_event_file->tr,
+ "synthetic", "schedtest", true);
+
+To disable it pass false:
+
+ trace_array_set_clr_event(schedtest_event_file->tr,
+ "synthetic", "schedtest", false);
+
+Finally, generate_synth_event() can be used to actually generate the
+event, which should be visible in the trace buffer afterwards:
+
+ ret = generate_synth_event(schedtest_event_file, vals,
+ ARRAY_SIZE(vals));
+
+To remove the synthetic event, the event should be disabled, and the
+trace instance should be 'put' back using put_event_file():
+
+ trace_array_set_clr_event(schedtest_event_file->tr,
+ "synthetic", "schedtest", false);
+ put_event_file(schedtest_event_file);
+
+If those have been successful, delete_synth_event() can be called to
+remove the event:
+
+ ret = delete_synth_event("schedtest");
+
+6.3.3.1 Generating a synthetic event piecewise
+----------------------------------------------
+
+To generate a synthetic using the piecewise method described above,
+the generate_synth_event_start() function is used to 'open' the
+synthetic event generation:
+
+ struct synth_gen_state gen_state;
+
+ ret = generate_synth_event_start(schedtest_event_file, &gen_state);
+
+It's passed the trace_event_file representing the synthetic event
+using the same methods as described above, along with a pointer to a
+struct synth_gen_state object, which will be zeroed before use and
+used to maintain state between this and following calls.
+
+Once the event has been opened, which means space for it has been
+reserved in the trace buffer, the individual fields can be set. There
+are two ways to do that, either one after another for each field in
+the event, which requires no lookups, or by name, which does. The
+tradeoff is flexibility in doing the assignments vs the cost of a
+lookup per field.
+
+To assign the values one after the other without lookups,
+add_next_synth_val() should be used. Each call is passed the same
+synth_gen_state object used in the generate_synth_event_start(), along
+with the value to set the next field in the event. After each field
+is set, the 'cursor' points to the next field, which will be set by
+the subsequent call, continuing until all the fields have been set in
+order. The same sequence of calls as in the above examples using this
+method would be (without error-handling code):
+
+ /* next_pid_field */
+ ret = add_next_synth_val(777, &gen_state);
+
+ /* next_comm_field */
+ ret = add_next_synth_val((u64)"slinky", &gen_state);
+
+ /* ts_ns */
+ ret = add_next_synth_val(1000000, &gen_state);
+
+ /* ts_ms */
+ ret = add_next_synth_val(1000, &gen_state);
+
+ /* cpu */
+ ret = add_next_synth_val(smp_processor_id(), &gen_state);
+
+ /* my_string_field */
+ ret = add_next_synth_val((u64)"thneed_2.01", &gen_state);
+
+ /* my_int_field */
+ ret = add_next_synth_val(395, &gen_state);
+
+To assign the values in any order, add_synth_val() should be used.
+Each call is passed the same synth_gen_state object used in the
+generate_synth_event_start(), along with the field name of the field
+to set and the value to set it to. The same sequence of calls as in
+the above examples using this method would be (without error-handling
+code):
+
+ ret = add_synth_val("next_pid_field", 777, &gen_state);
+ ret = add_synth_val("next_comm_field", (u64)"silly putty", &gen_state);
+ ret = add_synth_val("ts_ns", 1000000, &gen_state);
+ ret = add_synth_val("ts_ms", 1000, &gen_state);
+ ret = add_synth_val("cpu", smp_processor_id(), &gen_state);
+ ret = add_synth_val("my_string_field", (u64)"thneed_9", &gen_state);
+ ret = add_synth_val("my_int_field", 3999, &gen_state);
+
+Note that add_next_synth_val() and add_synth_val() are incompatible if
+used within the same generation of an event - either one can be used
+but not both at the same time.
+
+Finally, the event won't be actually generated until it's 'closed',
+which is done using generate_synth_event_end(), which takes only the
+struct synth_gen_state object used in the previous calls:
+
+ ret = generate_synth_event_end(&gen_state);
+
+Note that generate_synth_event_end() must be called at the end
+regardless of whether any of the add calls failed (say due to a bad
+field name being passed in).
--
2.14.1
^ permalink raw reply related [flat|nested] 12+ messages in thread* Re: [PATCH 0/7] tracing: Add support for in-kernel synthetic event API
2019-12-18 15:27 [PATCH 0/7] tracing: Add support for in-kernel synthetic event API Tom Zanussi
` (6 preceding siblings ...)
2019-12-18 15:27 ` [PATCH 7/7] tracing: Documentation for in-kernel synthetic event API Tom Zanussi
@ 2019-12-19 14:45 ` Masami Hiramatsu
2019-12-19 16:24 ` Tom Zanussi
7 siblings, 1 reply; 12+ messages in thread
From: Masami Hiramatsu @ 2019-12-19 14:45 UTC (permalink / raw)
To: Tom Zanussi
Cc: rostedt, artem.bityutskiy, mhiramat, linux-kernel, linux-rt-users
Hello Tom,
On Wed, 18 Dec 2019 09:27:36 -0600
Tom Zanussi <zanussi@kernel.org> wrote:
> Hi,
>
> I've recently had several requests and suggestions from users to add
> support for the creation and generation of synthetic events from
> kernel code such as modules, and not just from the available command
> line commands.
This reminds me my recent series of patches.
Could you use synth_event_run_command() for this purpose as I did
in boot-time tracing series?
https://lkml.kernel.org/r/157528179955.22451.16317363776831311868.stgit@devnote2
Above example uses a command string as same as command string, but I think
we can introduce some macros to construct the command string for easier
definition.
Or maybe it is possible to pass the const char *args[] array to that API,
instead of single char *cmd. (for dynamic event definiton)
Maybe we should consider more generic APIs for modules to create/delete
dynamic-event including synthetic and probes, and those might reuse
existing command parsers.
> This patchset adds support for that. The first three patches add some
> minor preliminary setup, followed by the two main patches that add the
> ability to create and generate synthetic events from the kernel. The
> next patch adds a test module that demonstrates actual use of the API
> and verifies that it works as intended, followed by Documentation.
Could you also check the locks are correctly acquired? It seems that
your APIs doesn't lock it.
Thank you,
--
Masami Hiramatsu <mhiramat@kernel.org>
^ permalink raw reply [flat|nested] 12+ messages in thread* Re: [PATCH 0/7] tracing: Add support for in-kernel synthetic event API
2019-12-19 14:45 ` [PATCH 0/7] tracing: Add support " Masami Hiramatsu
@ 2019-12-19 16:24 ` Tom Zanussi
2019-12-20 8:41 ` Masami Hiramatsu
0 siblings, 1 reply; 12+ messages in thread
From: Tom Zanussi @ 2019-12-19 16:24 UTC (permalink / raw)
To: Masami Hiramatsu; +Cc: rostedt, artem.bityutskiy, linux-kernel, linux-rt-users
Hi Masami,
On Thu, 2019-12-19 at 23:45 +0900, Masami Hiramatsu wrote:
> Hello Tom,
>
> On Wed, 18 Dec 2019 09:27:36 -0600
> Tom Zanussi <zanussi@kernel.org> wrote:
>
> > Hi,
> >
> > I've recently had several requests and suggestions from users to
> > add
> > support for the creation and generation of synthetic events from
> > kernel code such as modules, and not just from the available
> > command
> > line commands.
>
> This reminds me my recent series of patches.
>
> Could you use synth_event_run_command() for this purpose as I did
> in boot-time tracing series?
>
> https://lkml.kernel.org/r/157528179955.22451.16317363776831311868.stg
> it@devnote2
>
> Above example uses a command string as same as command string, but I
> think
> we can introduce some macros to construct the command string for
> easier
> definition.
>
> Or maybe it is possible to pass the const char *args[] array to that
> API,
> instead of single char *cmd. (for dynamic event definiton)
>
> Maybe we should consider more generic APIs for modules to
> create/delete
> dynamic-event including synthetic and probes, and those might reuse
> existing command parsers.
>
I'll have to look at your patches more closely, but I think it should
be possible to generate the command string synth_event_run_command()
needs, or the equivalent const char *args[] array you mentioned, from
the higher-level event definition in my patches.
So the two ways of defining an event in my patches is 1) from a static
array known at build-time defined like this:
static struct synth_field_desc synthtest_fields[] = {
{ .type = "pid_t", .name = "next_pid_field" },
{ .type = "char[16]", .name = "next_comm_field" },
{ .type = "u64", .name = "ts_ns" },
{ .type = "u64", .name = "ts_ms" },
{ .type = "unsigned int", .name = "cpu" },
{ .type = "char[64]", .name = "my_string_field" },
{ .type = "int", .name = "my_int_field" },
};
which is then passed to create_synth_event(&synthtest_fields)
or 2) at run-time by adding fields individually as they become known:
add_synth_field("type", "name")
which requires some sort of start/end("event name").
I think that should all be possible using your patches, and maybe
generalizable to not just synth events by removing _synth_ from
everything? Let me know what you think. If that's correct, I can go
and rewrite the event creation part on top of your patches.
> > This patchset adds support for that. The first three patches add
> > some
> > minor preliminary setup, followed by the two main patches that add
> > the
> > ability to create and generate synthetic events from the
> > kernel. The
> > next patch adds a test module that demonstrates actual use of the
> > API
> > and verifies that it works as intended, followed by Documentation.
>
> Could you also check the locks are correctly acquired? It seems that
> your APIs doesn't lock it.
>
I did notice that I said that trace_types_lock and event_mutex should
be held for trace_array_find() but it should only be trace_types_lock,
and then I missed doing that in get_event_file() in one place. And I
also don't really need the nolock versions, so will simplify and remove
them. I think elsewhere event_mutex is taken where needed. But if
talking about something else, please let me know.
Thanks,
Tom
>
> Thank you,
>
>
^ permalink raw reply [flat|nested] 12+ messages in thread* Re: [PATCH 0/7] tracing: Add support for in-kernel synthetic event API
2019-12-19 16:24 ` Tom Zanussi
@ 2019-12-20 8:41 ` Masami Hiramatsu
2019-12-20 16:24 ` Tom Zanussi
0 siblings, 1 reply; 12+ messages in thread
From: Masami Hiramatsu @ 2019-12-20 8:41 UTC (permalink / raw)
To: Tom Zanussi; +Cc: rostedt, artem.bityutskiy, linux-kernel, linux-rt-users
Hi Tom,
On Thu, 19 Dec 2019 10:24:27 -0600
Tom Zanussi <zanussi@kernel.org> wrote:
> Hi Masami,
>
> On Thu, 2019-12-19 at 23:45 +0900, Masami Hiramatsu wrote:
> > Hello Tom,
> >
> > On Wed, 18 Dec 2019 09:27:36 -0600
> > Tom Zanussi <zanussi@kernel.org> wrote:
> >
> > > Hi,
> > >
> > > I've recently had several requests and suggestions from users to
> > > add
> > > support for the creation and generation of synthetic events from
> > > kernel code such as modules, and not just from the available
> > > command
> > > line commands.
> >
> > This reminds me my recent series of patches.
> >
> > Could you use synth_event_run_command() for this purpose as I did
> > in boot-time tracing series?
> >
> > https://lkml.kernel.org/r/157528179955.22451.16317363776831311868.stg
> > it@devnote2
> >
> > Above example uses a command string as same as command string, but I
> > think
> > we can introduce some macros to construct the command string for
> > easier
> > definition.
> >
> > Or maybe it is possible to pass the const char *args[] array to that
> > API,
> > instead of single char *cmd. (for dynamic event definiton)
> >
> > Maybe we should consider more generic APIs for modules to
> > create/delete
> > dynamic-event including synthetic and probes, and those might reuse
> > existing command parsers.
> >
>
> I'll have to look at your patches more closely, but I think it should
> be possible to generate the command string synth_event_run_command()
> needs, or the equivalent const char *args[] array you mentioned, from
> the higher-level event definition in my patches.
Agreed.
>
> So the two ways of defining an event in my patches is 1) from a static
> array known at build-time defined like this:
>
> static struct synth_field_desc synthtest_fields[] = {
> { .type = "pid_t", .name = "next_pid_field" },
> { .type = "char[16]", .name = "next_comm_field" },
> { .type = "u64", .name = "ts_ns" },
> { .type = "u64", .name = "ts_ms" },
> { .type = "unsigned int", .name = "cpu" },
> { .type = "char[64]", .name = "my_string_field" },
> { .type = "int", .name = "my_int_field" },
> };
>
> which is then passed to create_synth_event(&synthtest_fields)
>
> or 2) at run-time by adding fields individually as they become known:
>
> add_synth_field("type", "name")
>
> which requires some sort of start/end("event name").
I think the 1) API seems a bit redundant IF we can expose the
single comamnd string interface.
> I think that should all be possible using your patches, and maybe
> generalizable to not just synth events by removing _synth_ from
> everything?
If the implementation is enough generic, I think it is better to keep
"synth" for better usability.
For example, if the API is just generating a command string,
it would be easy to be reused by probe-events too.
----
struct dynevent_command {
char *cmd_buf;
enum dynevent_type type; /* Set by gen_*_cmd and checked on each API */
};
int gen_synth_cmd(struct dynevent_command *, const char *name, ...);
int add_synth_field(struct dynevent_command *, const char *type, const char *var);
int gen_kprobe_cmd(struct dynevent_command *, const char *name, const char *loc, ....);
int gen_kretprobe_cmd(struct dynevent_command *, const char *name, const char *loc, ....);
int add_probe_fields(struct dynevent_command *, const char *field, ...);
int create_dynevent(struct dynevent_command *);
struct dynevent_command cmd;
gen_synth_cmd(&cmd, "synthtest", "pid_t", "next_pid_field");
add_synth_field(&cmd, "char[16]", "next_comm_field");
...
create_dynevent(&cmd);
gen_kprobe_cmd(&cmd, "myprobe", "vfs_read+5", "%ax");
add_probe_fields(&cmd, "%bx", "%dx");
create_dynevent(&cmd);
gen_kretprobe_cmd(&cmd, "myretprobe", "vfs_write", "$retval");
create_dynevent(&cmd);
----
Actually, these are just wrappers of type verifier & strcat() :P
And it can provide similar user-experience and generic interface
with simplar implementation.
> Let me know what you think. If that's correct, I can go
> and rewrite the event creation part on top of your patches.
No need to move onto my series. Mine focuses on tracing
boot process, but your's are providing APIs for modules.
> > > This patchset adds support for that. The first three patches add
> > > some
> > > minor preliminary setup, followed by the two main patches that add
> > > the
> > > ability to create and generate synthetic events from the
> > > kernel. The
> > > next patch adds a test module that demonstrates actual use of the
> > > API
> > > and verifies that it works as intended, followed by Documentation.
> >
> > Could you also check the locks are correctly acquired? It seems that
> > your APIs doesn't lock it.
> >
>
> I did notice that I said that trace_types_lock and event_mutex should
> be held for trace_array_find() but it should only be trace_types_lock,
> and then I missed doing that in get_event_file() in one place. And I
> also don't really need the nolock versions, so will simplify and remove
> them. I think elsewhere event_mutex is taken where needed. But if
> talking about something else, please let me know.
Yes, that is what I found.
Thank you,
--
Masami Hiramatsu <mhiramat@kernel.org>
^ permalink raw reply [flat|nested] 12+ messages in thread* Re: [PATCH 0/7] tracing: Add support for in-kernel synthetic event API
2019-12-20 8:41 ` Masami Hiramatsu
@ 2019-12-20 16:24 ` Tom Zanussi
0 siblings, 0 replies; 12+ messages in thread
From: Tom Zanussi @ 2019-12-20 16:24 UTC (permalink / raw)
To: Masami Hiramatsu; +Cc: rostedt, artem.bityutskiy, linux-kernel, linux-rt-users
Hi Masami,
On Fri, 2019-12-20 at 17:41 +0900, Masami Hiramatsu wrote:
> Hi Tom,
>
> On Thu, 19 Dec 2019 10:24:27 -0600
> Tom Zanussi <zanussi@kernel.org> wrote:
>
> > Hi Masami,
> >
> > On Thu, 2019-12-19 at 23:45 +0900, Masami Hiramatsu wrote:
> > > Hello Tom,
> > >
> > > On Wed, 18 Dec 2019 09:27:36 -0600
> > > Tom Zanussi <zanussi@kernel.org> wrote:
> > >
> > > > Hi,
> > > >
> > > > I've recently had several requests and suggestions from users
> > > > to
> > > > add
> > > > support for the creation and generation of synthetic events
> > > > from
> > > > kernel code such as modules, and not just from the available
> > > > command
> > > > line commands.
> > >
> > > This reminds me my recent series of patches.
> > >
> > > Could you use synth_event_run_command() for this purpose as I did
> > > in boot-time tracing series?
> > >
> > > https://lkml.kernel.org/r/157528179955.22451.16317363776831311868
> > > .stg
> > > it@devnote2
> > >
> > > Above example uses a command string as same as command string,
> > > but I
> > > think
> > > we can introduce some macros to construct the command string for
> > > easier
> > > definition.
> > >
> > > Or maybe it is possible to pass the const char *args[] array to
> > > that
> > > API,
> > > instead of single char *cmd. (for dynamic event definiton)
> > >
> > > Maybe we should consider more generic APIs for modules to
> > > create/delete
> > > dynamic-event including synthetic and probes, and those might
> > > reuse
> > > existing command parsers.
> > >
> >
> > I'll have to look at your patches more closely, but I think it
> > should
> > be possible to generate the command string
> > synth_event_run_command()
> > needs, or the equivalent const char *args[] array you mentioned,
> > from
> > the higher-level event definition in my patches.
>
> Agreed.
>
> >
> > So the two ways of defining an event in my patches is 1) from a
> > static
> > array known at build-time defined like this:
> >
> > static struct synth_field_desc synthtest_fields[] = {
> > { .type = "pid_t", .name = "next_pid_field" },
> > { .type = "char[16]", .name = "next_comm_field" },
> > { .type = "u64", .name = "ts_ns" },
> > { .type = "u64", .name = "ts_ms" },
> > { .type = "unsigned int", .name = "cpu" },
> > { .type = "char[64]", .name = "my_string_field" },
> > { .type = "int", .name = "my_int_field" },
> > };
> >
> > which is then passed to create_synth_event(&synthtest_fields)
> >
> > or 2) at run-time by adding fields individually as they become
> > known:
> >
> > add_synth_field("type", "name")
> >
> > which requires some sort of start/end("event name").
>
> I think the 1) API seems a bit redundant IF we can expose the
> single comamnd string interface.
>
It may be redundant, but I think it might be a preferred interface for
some users. In any case, supporting 1) would just a simple matter of
providing a wrapper interface around your string interface.
> > I think that should all be possible using your patches, and maybe
> > generalizable to not just synth events by removing _synth_ from
> > everything?
>
> If the implementation is enough generic, I think it is better to keep
> "synth" for better usability.
>
> For example, if the API is just generating a command string,
> it would be easy to be reused by probe-events too.
>
> ----
> struct dynevent_command {
> char *cmd_buf;
> enum dynevent_type type; /* Set by gen_*_cmd and checked on each
> API */
> };
>
> int gen_synth_cmd(struct dynevent_command *, const char *name, ...);
> int add_synth_field(struct dynevent_command *, const char *type,
> const char *var);
> int gen_kprobe_cmd(struct dynevent_command *, const char *name, const
> char *loc, ....);
> int gen_kretprobe_cmd(struct dynevent_command *, const char *name,
> const char *loc, ....);
> int add_probe_fields(struct dynevent_command *, const char *field,
> ...);
> int create_dynevent(struct dynevent_command *);
>
> struct dynevent_command cmd;
>
> gen_synth_cmd(&cmd, "synthtest", "pid_t", "next_pid_field");
> add_synth_field(&cmd, "char[16]", "next_comm_field");
> ...
> create_dynevent(&cmd);
>
> gen_kprobe_cmd(&cmd, "myprobe", "vfs_read+5", "%ax");
> add_probe_fields(&cmd, "%bx", "%dx");
> create_dynevent(&cmd);
>
> gen_kretprobe_cmd(&cmd, "myretprobe", "vfs_write", "$retval");
> create_dynevent(&cmd);
> ----
>
> Actually, these are just wrappers of type verifier & strcat() :P
> And it can provide similar user-experience and generic interface
> with simplar implementation.
>
Good suggestions - I'll start implementing something like this and
rebase my patches on top of this.
> > Let me know what you think. If that's correct, I can go
> > and rewrite the event creation part on top of your patches.
>
> No need to move onto my series. Mine focuses on tracing
> boot process, but your's are providing APIs for modules.
>
OK, yeah I guess synth_event_run_command() et al are very simple.
Thanks,
Tom
^ permalink raw reply [flat|nested] 12+ messages in thread