* Re: [PATCH v2] tracing: Show the tracer options in boot-time created instance
From: Masami Hiramatsu @ 2025-11-19 4:39 UTC (permalink / raw)
To: Masami Hiramatsu (Google)
Cc: Steven Rostedt, Mark Rutland, Mathieu Desnoyers, Andrew Morton,
linux-kernel, linux-trace-kernel
In-Reply-To: <176351169375.1795430.7653862896005389480.stgit@mhiramat.tok.corp.google.com>
On Wed, 19 Nov 2025 09:21:34 +0900
"Masami Hiramatsu (Google)" <mhiramat@kernel.org> wrote:
> From: Masami Hiramatsu (Google) <mhiramat@kernel.org>
>
> Since tracer_init_tracefs_work_func() only updates the tracer options
> for the global_trace, the instances created by the kernel cmdline
> do not have those options.
>
> Fix to update tracer options for those boot-time created instances
> to show those options.
>
Wait, this seems not updating the global instance. Let me fix it.
(ftracetest found it.)
Thanks,
> Fixes: 428add559b69 ("tracing: Have tracer option be instance specific")
> Signed-off-by: Masami Hiramatsu (Google) <mhiramat@kernel.org>
> ---
> Change in v2:
> - Fix update_tracer_options() to update all instances.
> - Update tracer_options_updated after update all instances.
> ---
> kernel/trace/trace.c | 11 ++++++++---
> 1 file changed, 8 insertions(+), 3 deletions(-)
>
> diff --git a/kernel/trace/trace.c b/kernel/trace/trace.c
> index 8ae95800592d..4a6784057855 100644
> --- a/kernel/trace/trace.c
> +++ b/kernel/trace/trace.c
> @@ -10231,11 +10231,16 @@ static __init int __update_tracer_options(struct trace_array *tr)
> return ret;
> }
>
> -static __init void update_tracer_options(struct trace_array *tr)
> +static __init void update_tracer_options(void)
> {
> + struct trace_array *tr;
> +
> guard(mutex)(&trace_types_lock);
> +
> + list_for_each_entry(tr, &ftrace_trace_arrays, list)
> + __update_tracer_options(tr);
> +
> tracer_options_updated = true;
> - __update_tracer_options(tr);
> }
>
> /* Must have trace_types_lock held */
> @@ -10937,7 +10942,7 @@ static __init void tracer_init_tracefs_work_func(struct work_struct *work)
>
> create_trace_instances(NULL);
>
> - update_tracer_options(&global_trace);
> + update_tracer_options();
> }
>
> static __init int tracer_init_tracefs(void)
>
--
Masami Hiramatsu (Google) <mhiramat@kernel.org>
^ permalink raw reply
* [PATCH 2/2] tracing: Merge struct event_trigger_ops into struct event_command
From: Steven Rostedt @ 2025-11-19 3:10 UTC (permalink / raw)
To: linux-kernel, linux-trace-kernel
Cc: Masami Hiramatsu, Mark Rutland, Mathieu Desnoyers, Andrew Morton,
Tom Zanussi
In-Reply-To: <20251119031042.328864818@kernel.org>
From: Steven Rostedt <rostedt@goodmis.org>
Now that there's pretty much a one to one mapping between the struct
event_trigger_ops and struct event_command, there's no reason to have two
different structures. Merge the function pointers of event_trigger_ops
into event_command.
There's one exception in trace_events_hist.c for the
event_hist_trigger_named_ops. This has special logic for the init and free
function pointers for "named histograms". In this case, allocate the
cmd_ops of the event_trigger_data and set it to the proper init and free
functions, which are used to initialize and free the event_trigger_data
respectively. Have the free function and the init function (on failure)
free the cmd_ops of the data element.
Signed-off-by: Steven Rostedt (Google) <rostedt@goodmis.org>
---
kernel/trace/trace.h | 126 +++++++++++-----------------
kernel/trace/trace_eprobe.c | 13 +--
kernel/trace/trace_events_hist.c | 93 ++++++++++----------
kernel/trace/trace_events_trigger.c | 121 +++++++++++---------------
4 files changed, 152 insertions(+), 201 deletions(-)
diff --git a/kernel/trace/trace.h b/kernel/trace/trace.h
index 036019ffc407..5863800b1ab3 100644
--- a/kernel/trace/trace.h
+++ b/kernel/trace/trace.h
@@ -1798,7 +1798,6 @@ struct event_trigger_data {
unsigned long count;
int ref;
int flags;
- const struct event_trigger_ops *ops;
struct event_command *cmd_ops;
struct event_filter __rcu *filter;
char *filter_str;
@@ -1889,73 +1888,6 @@ extern void event_trigger_unregister(struct event_command *cmd_ops,
extern void event_file_get(struct trace_event_file *file);
extern void event_file_put(struct trace_event_file *file);
-/**
- * struct event_trigger_ops - callbacks for trace event triggers
- *
- * The methods in this structure provide per-event trigger hooks for
- * various trigger operations.
- *
- * The @init and @free methods are used during trigger setup and
- * teardown, typically called from an event_command's @parse()
- * function implementation.
- *
- * The @print method is used to print the trigger spec.
- *
- * The @trigger method is the function that actually implements the
- * trigger and is called in the context of the triggering event
- * whenever that event occurs.
- *
- * All the methods below, except for @init() and @free(), must be
- * implemented.
- *
- * @trigger: The trigger 'probe' function called when the triggering
- * event occurs. The data passed into this callback is the data
- * that was supplied to the event_command @reg() function that
- * registered the trigger (see struct event_command) along with
- * the trace record, rec.
- *
- * @count_func: If defined and a numeric parameter is passed to the
- * trigger, then this function will be called before @trigger
- * is called. If this function returns false, then @trigger is not
- * executed.
- *
- * @init: An optional initialization function called for the trigger
- * when the trigger is registered (via the event_command reg()
- * function). This can be used to perform per-trigger
- * initialization such as incrementing a per-trigger reference
- * count, for instance. This is usually implemented by the
- * generic utility function @event_trigger_init() (see
- * trace_event_triggers.c).
- *
- * @free: An optional de-initialization function called for the
- * trigger when the trigger is unregistered (via the
- * event_command @reg() function). This can be used to perform
- * per-trigger de-initialization such as decrementing a
- * per-trigger reference count and freeing corresponding trigger
- * data, for instance. This is usually implemented by the
- * generic utility function @event_trigger_free() (see
- * trace_event_triggers.c).
- *
- * @print: The callback function invoked to have the trigger print
- * itself. This is usually implemented by a wrapper function
- * that calls the generic utility function @event_trigger_print()
- * (see trace_event_triggers.c).
- */
-struct event_trigger_ops {
- void (*trigger)(struct event_trigger_data *data,
- struct trace_buffer *buffer,
- void *rec,
- struct ring_buffer_event *rbe);
- bool (*count_func)(struct event_trigger_data *data,
- struct trace_buffer *buffer,
- void *rec,
- struct ring_buffer_event *rbe);
- int (*init)(struct event_trigger_data *data);
- void (*free)(struct event_trigger_data *data);
- int (*print)(struct seq_file *m,
- struct event_trigger_data *data);
-};
-
/**
* struct event_command - callbacks and data members for event commands
*
@@ -1976,9 +1908,6 @@ struct event_trigger_ops {
* @name: The unique name that identifies the event command. This is
* the name used when setting triggers via trigger files.
*
- * @trigger_ops: The event_trigger_ops implementation associated with
- * the command.
- *
* @trigger_type: A unique id that identifies the event command
* 'type'. This value has two purposes, the first to ensure that
* only one trigger of the same type can be set at a given time
@@ -2008,7 +1937,7 @@ struct event_trigger_ops {
*
* @reg: Adds the trigger to the list of triggers associated with the
* event, and enables the event trigger itself, after
- * initializing it (via the event_trigger_ops @init() function).
+ * initializing it (via the event_command @init() function).
* This is also where commands can use the @trigger_type value to
* make the decision as to whether or not multiple instances of
* the trigger should be allowed. This is usually implemented by
@@ -2017,7 +1946,7 @@ struct event_trigger_ops {
*
* @unreg: Removes the trigger from the list of triggers associated
* with the event, and disables the event trigger itself, after
- * initializing it (via the event_trigger_ops @free() function).
+ * initializing it (via the event_command @free() function).
* This is usually implemented by the generic utility function
* @unregister_trigger() (see trace_event_triggers.c).
*
@@ -2030,11 +1959,46 @@ struct event_trigger_ops {
* event command, filters set by the user for the command will be
* ignored. This is usually implemented by the generic utility
* function @set_trigger_filter() (see trace_event_triggers.c).
+ *
+ * All the methods below, except for @init() and @free(), must be
+ * implemented.
+ *
+ * @trigger: The trigger 'probe' function called when the triggering
+ * event occurs. The data passed into this callback is the data
+ * that was supplied to the event_command @reg() function that
+ * registered the trigger (see struct event_command) along with
+ * the trace record, rec.
+ *
+ * @count_func: If defined and a numeric parameter is passed to the
+ * trigger, then this function will be called before @trigger
+ * is called. If this function returns false, then @trigger is not
+ * executed.
+ *
+ * @init: An optional initialization function called for the trigger
+ * when the trigger is registered (via the event_command reg()
+ * function). This can be used to perform per-trigger
+ * initialization such as incrementing a per-trigger reference
+ * count, for instance. This is usually implemented by the
+ * generic utility function @event_trigger_init() (see
+ * trace_event_triggers.c).
+ *
+ * @free: An optional de-initialization function called for the
+ * trigger when the trigger is unregistered (via the
+ * event_command @reg() function). This can be used to perform
+ * per-trigger de-initialization such as decrementing a
+ * per-trigger reference count and freeing corresponding trigger
+ * data, for instance. This is usually implemented by the
+ * generic utility function @event_trigger_free() (see
+ * trace_event_triggers.c).
+ *
+ * @print: The callback function invoked to have the trigger print
+ * itself. This is usually implemented by a wrapper function
+ * that calls the generic utility function @event_trigger_print()
+ * (see trace_event_triggers.c).
*/
struct event_command {
struct list_head list;
char *name;
- const struct event_trigger_ops *trigger_ops;
enum event_trigger_type trigger_type;
int flags;
int (*parse)(struct event_command *cmd_ops,
@@ -2051,6 +2015,18 @@ struct event_command {
int (*set_filter)(char *filter_str,
struct event_trigger_data *data,
struct trace_event_file *file);
+ void (*trigger)(struct event_trigger_data *data,
+ struct trace_buffer *buffer,
+ void *rec,
+ struct ring_buffer_event *rbe);
+ bool (*count_func)(struct event_trigger_data *data,
+ struct trace_buffer *buffer,
+ void *rec,
+ struct ring_buffer_event *rbe);
+ int (*init)(struct event_trigger_data *data);
+ void (*free)(struct event_trigger_data *data);
+ int (*print)(struct seq_file *m,
+ struct event_trigger_data *data);
};
/**
@@ -2071,7 +2047,7 @@ struct event_command {
* either committed or discarded. At that point, if any commands
* have deferred their triggers, those commands are finally
* invoked following the close of the current event. In other
- * words, if the event_trigger_ops @func() probe implementation
+ * words, if the event_command @func() probe implementation
* itself logs to the trace buffer, this flag should be set,
* otherwise it can be left unspecified.
*
diff --git a/kernel/trace/trace_eprobe.c b/kernel/trace/trace_eprobe.c
index 14ae896dbe75..f3e0442c3b96 100644
--- a/kernel/trace/trace_eprobe.c
+++ b/kernel/trace/trace_eprobe.c
@@ -484,13 +484,6 @@ static void eprobe_trigger_func(struct event_trigger_data *data,
__eprobe_trace_func(edata, rec);
}
-static const struct event_trigger_ops eprobe_trigger_ops = {
- .trigger = eprobe_trigger_func,
- .print = eprobe_trigger_print,
- .init = eprobe_trigger_init,
- .free = eprobe_trigger_free,
-};
-
static int eprobe_trigger_cmd_parse(struct event_command *cmd_ops,
struct trace_event_file *file,
char *glob, char *cmd,
@@ -517,12 +510,15 @@ static struct event_command event_trigger_cmd = {
.name = "eprobe",
.trigger_type = ETT_EVENT_EPROBE,
.flags = EVENT_CMD_FL_NEEDS_REC,
- .trigger_ops = &eprobe_trigger_ops,
.parse = eprobe_trigger_cmd_parse,
.reg = eprobe_trigger_reg_func,
.unreg = eprobe_trigger_unreg_func,
.unreg_all = NULL,
.set_filter = NULL,
+ .trigger = eprobe_trigger_func,
+ .print = eprobe_trigger_print,
+ .init = eprobe_trigger_init,
+ .free = eprobe_trigger_free,
};
static struct event_trigger_data *
@@ -542,7 +538,6 @@ new_eprobe_trigger(struct trace_eprobe *ep, struct trace_event_file *file)
trigger->flags = EVENT_TRIGGER_FL_PROBE;
trigger->count = -1;
- trigger->ops = &eprobe_trigger_ops;
/*
* EVENT PROBE triggers are not registered as commands with
diff --git a/kernel/trace/trace_events_hist.c b/kernel/trace/trace_events_hist.c
index f9cc8d6a215b..1e03398b9e91 100644
--- a/kernel/trace/trace_events_hist.c
+++ b/kernel/trace/trace_events_hist.c
@@ -5694,7 +5694,7 @@ static void hist_trigger_show(struct seq_file *m,
seq_puts(m, "\n\n");
seq_puts(m, "# event histogram\n#\n# trigger info: ");
- data->ops->print(m, data);
+ data->cmd_ops->print(m, data);
seq_puts(m, "#\n\n");
hist_data = data->private_data;
@@ -6016,7 +6016,7 @@ static void hist_trigger_debug_show(struct seq_file *m,
seq_puts(m, "\n\n");
seq_puts(m, "# event histogram\n#\n# trigger info: ");
- data->ops->print(m, data);
+ data->cmd_ops->print(m, data);
seq_puts(m, "#\n\n");
hist_data = data->private_data;
@@ -6326,20 +6326,23 @@ static void event_hist_trigger_free(struct event_trigger_data *data)
free_hist_pad();
}
-static const struct event_trigger_ops event_hist_trigger_ops = {
- .trigger = event_hist_trigger,
- .print = event_hist_trigger_print,
- .init = event_hist_trigger_init,
- .free = event_hist_trigger_free,
-};
+static struct event_command trigger_hist_cmd;
static int event_hist_trigger_named_init(struct event_trigger_data *data)
{
+ int ret;
+
data->ref++;
save_named_trigger(data->named_data->name, data);
- return event_hist_trigger_init(data->named_data);
+ ret = event_hist_trigger_init(data->named_data);
+ if (ret < 0) {
+ kfree(data->cmd_ops);
+ data->cmd_ops = &trigger_hist_cmd;
+ }
+
+ return ret;
}
static void event_hist_trigger_named_free(struct event_trigger_data *data)
@@ -6351,18 +6354,14 @@ static void event_hist_trigger_named_free(struct event_trigger_data *data)
data->ref--;
if (!data->ref) {
+ struct event_command *cmd_ops = data->cmd_ops;
+
del_named_trigger(data);
trigger_data_free(data);
+ kfree(cmd_ops);
}
}
-static const struct event_trigger_ops event_hist_trigger_named_ops = {
- .trigger = event_hist_trigger,
- .print = event_hist_trigger_print,
- .init = event_hist_trigger_named_init,
- .free = event_hist_trigger_named_free,
-};
-
static void hist_clear(struct event_trigger_data *data)
{
struct hist_trigger_data *hist_data = data->private_data;
@@ -6556,13 +6555,24 @@ static int hist_register_trigger(char *glob,
data->paused = true;
if (named_data) {
+ struct event_command *cmd_ops;
+
data->private_data = named_data->private_data;
set_named_trigger_data(data, named_data);
- data->ops = &event_hist_trigger_named_ops;
+ /* Copy the command ops and update some of the functions */
+ cmd_ops = kmalloc(sizeof(*cmd_ops), GFP_KERNEL);
+ if (!cmd_ops) {
+ ret = -ENOMEM;
+ goto out;
+ }
+ *cmd_ops = *data->cmd_ops;
+ cmd_ops->init = event_hist_trigger_named_init;
+ cmd_ops->free = event_hist_trigger_named_free;
+ data->cmd_ops = cmd_ops;
}
- if (data->ops->init) {
- ret = data->ops->init(data);
+ if (data->cmd_ops->init) {
+ ret = data->cmd_ops->init(data);
if (ret < 0)
goto out;
}
@@ -6676,8 +6686,8 @@ static void hist_unregister_trigger(char *glob,
}
}
- if (test && test->ops->free)
- test->ops->free(test);
+ if (test && test->cmd_ops->free)
+ test->cmd_ops->free(test);
if (hist_data->enable_timestamps) {
if (!hist_data->remove || test)
@@ -6729,8 +6739,8 @@ static void hist_unreg_all(struct trace_event_file *file)
update_cond_flag(file);
if (hist_data->enable_timestamps)
tracing_set_filter_buffering(file->tr, false);
- if (test->ops->free)
- test->ops->free(test);
+ if (test->cmd_ops->free)
+ test->cmd_ops->free(test);
}
}
}
@@ -6902,12 +6912,15 @@ static struct event_command trigger_hist_cmd = {
.name = "hist",
.trigger_type = ETT_EVENT_HIST,
.flags = EVENT_CMD_FL_NEEDS_REC,
- .trigger_ops = &event_hist_trigger_ops,
.parse = event_hist_trigger_parse,
.reg = hist_register_trigger,
.unreg = hist_unregister_trigger,
.unreg_all = hist_unreg_all,
.set_filter = set_trigger_filter,
+ .trigger = event_hist_trigger,
+ .print = event_hist_trigger_print,
+ .init = event_hist_trigger_init,
+ .free = event_hist_trigger_free,
};
__init int register_trigger_hist_cmd(void)
@@ -6939,22 +6952,6 @@ hist_enable_trigger(struct event_trigger_data *data,
}
}
-static const struct event_trigger_ops hist_enable_trigger_ops = {
- .trigger = hist_enable_trigger,
- .count_func = event_trigger_count,
- .print = event_enable_trigger_print,
- .init = event_trigger_init,
- .free = event_enable_trigger_free,
-};
-
-static const struct event_trigger_ops hist_disable_trigger_ops = {
- .trigger = hist_enable_trigger,
- .count_func = event_trigger_count,
- .print = event_enable_trigger_print,
- .init = event_trigger_init,
- .free = event_enable_trigger_free,
-};
-
static void hist_enable_unreg_all(struct trace_event_file *file)
{
struct event_trigger_data *test, *n;
@@ -6964,8 +6961,8 @@ static void hist_enable_unreg_all(struct trace_event_file *file)
list_del_rcu(&test->list);
update_cond_flag(file);
trace_event_trigger_enable_disable(file, 0);
- if (test->ops->free)
- test->ops->free(test);
+ if (test->cmd_ops->free)
+ test->cmd_ops->free(test);
}
}
}
@@ -6973,23 +6970,31 @@ static void hist_enable_unreg_all(struct trace_event_file *file)
static struct event_command trigger_hist_enable_cmd = {
.name = ENABLE_HIST_STR,
.trigger_type = ETT_HIST_ENABLE,
- .trigger_ops = &hist_enable_trigger_ops,
.parse = event_enable_trigger_parse,
.reg = event_enable_register_trigger,
.unreg = event_enable_unregister_trigger,
.unreg_all = hist_enable_unreg_all,
.set_filter = set_trigger_filter,
+ .trigger = hist_enable_trigger,
+ .count_func = event_trigger_count,
+ .print = event_enable_trigger_print,
+ .init = event_trigger_init,
+ .free = event_enable_trigger_free,
};
static struct event_command trigger_hist_disable_cmd = {
.name = DISABLE_HIST_STR,
.trigger_type = ETT_HIST_ENABLE,
- .trigger_ops = &hist_disable_trigger_ops,
.parse = event_enable_trigger_parse,
.reg = event_enable_register_trigger,
.unreg = event_enable_unregister_trigger,
.unreg_all = hist_enable_unreg_all,
.set_filter = set_trigger_filter,
+ .trigger = hist_enable_trigger,
+ .count_func = event_trigger_count,
+ .print = event_enable_trigger_print,
+ .init = event_trigger_init,
+ .free = event_enable_trigger_free,
};
static __init void unregister_trigger_hist_enable_disable_cmds(void)
diff --git a/kernel/trace/trace_events_trigger.c b/kernel/trace/trace_events_trigger.c
index 576bad18bcdb..7795af600466 100644
--- a/kernel/trace/trace_events_trigger.c
+++ b/kernel/trace/trace_events_trigger.c
@@ -32,14 +32,14 @@ static inline void data_ops_trigger(struct event_trigger_data *data,
struct trace_buffer *buffer, void *rec,
struct ring_buffer_event *event)
{
- const struct event_trigger_ops *ops = data->ops;
+ const struct event_command *cmd_ops = data->cmd_ops;
if (data->flags & EVENT_TRIGGER_FL_COUNT) {
- if (!ops->count_func(data, buffer, rec, event))
+ if (!cmd_ops->count_func(data, buffer, rec, event))
return;
}
- ops->trigger(data, buffer, rec, event);
+ cmd_ops->trigger(data, buffer, rec, event);
}
/**
@@ -205,7 +205,7 @@ static int trigger_show(struct seq_file *m, void *v)
}
data = list_entry(v, struct event_trigger_data, list);
- data->ops->print(m, data);
+ data->cmd_ops->print(m, data);
return 0;
}
@@ -422,7 +422,7 @@ bool event_trigger_count(struct event_trigger_data *data,
}
/**
- * event_trigger_print - Generic event_trigger_ops @print implementation
+ * event_trigger_print - Generic event_command @print implementation
* @name: The name of the event trigger
* @m: The seq_file being printed to
* @data: Trigger-specific data
@@ -457,7 +457,7 @@ event_trigger_print(const char *name, struct seq_file *m,
}
/**
- * event_trigger_init - Generic event_trigger_ops @init implementation
+ * event_trigger_init - Generic event_command @init implementation
* @data: Trigger-specific data
*
* Common implementation of event trigger initialization.
@@ -474,7 +474,7 @@ int event_trigger_init(struct event_trigger_data *data)
}
/**
- * event_trigger_free - Generic event_trigger_ops @free implementation
+ * event_trigger_free - Generic event_command @free implementation
* @data: Trigger-specific data
*
* Common implementation of event trigger de-initialization.
@@ -536,8 +536,8 @@ clear_event_triggers(struct trace_array *tr)
list_for_each_entry_safe(data, n, &file->triggers, list) {
trace_event_trigger_enable_disable(file, 0);
list_del_rcu(&data->list);
- if (data->ops->free)
- data->ops->free(data);
+ if (data->cmd_ops->free)
+ data->cmd_ops->free(data);
}
}
}
@@ -600,8 +600,8 @@ static int register_trigger(char *glob,
return -EEXIST;
}
- if (data->ops->init) {
- ret = data->ops->init(data);
+ if (data->cmd_ops->init) {
+ ret = data->cmd_ops->init(data);
if (ret < 0)
return ret;
}
@@ -639,8 +639,8 @@ static bool try_unregister_trigger(char *glob,
}
if (data) {
- if (data->ops->free)
- data->ops->free(data);
+ if (data->cmd_ops->free)
+ data->cmd_ops->free(data);
return true;
}
@@ -875,10 +875,9 @@ struct event_trigger_data *trigger_data_alloc(struct event_command *cmd_ops,
return NULL;
trigger_data->count = -1;
- trigger_data->ops = cmd_ops->trigger_ops;
trigger_data->cmd_ops = cmd_ops;
trigger_data->private_data = private_data;
- if (param && cmd_ops->trigger_ops->count_func)
+ if (param && cmd_ops->count_func)
trigger_data->flags |= EVENT_TRIGGER_FL_COUNT;
INIT_LIST_HEAD(&trigger_data->list);
@@ -1401,41 +1400,33 @@ traceoff_trigger_print(struct seq_file *m, struct event_trigger_data *data)
data->filter_str);
}
-static const struct event_trigger_ops traceon_trigger_ops = {
- .trigger = traceon_trigger,
- .count_func = traceon_count_func,
- .print = traceon_trigger_print,
- .init = event_trigger_init,
- .free = event_trigger_free,
-};
-
-static const struct event_trigger_ops traceoff_trigger_ops = {
- .trigger = traceoff_trigger,
- .count_func = traceoff_count_func,
- .print = traceoff_trigger_print,
- .init = event_trigger_init,
- .free = event_trigger_free,
-};
-
static struct event_command trigger_traceon_cmd = {
.name = "traceon",
.trigger_type = ETT_TRACE_ONOFF,
- .trigger_ops = &traceon_trigger_ops,
.parse = event_trigger_parse,
.reg = register_trigger,
.unreg = unregister_trigger,
.set_filter = set_trigger_filter,
+ .trigger = traceon_trigger,
+ .count_func = traceon_count_func,
+ .print = traceon_trigger_print,
+ .init = event_trigger_init,
+ .free = event_trigger_free,
};
static struct event_command trigger_traceoff_cmd = {
.name = "traceoff",
.trigger_type = ETT_TRACE_ONOFF,
.flags = EVENT_CMD_FL_POST_TRIGGER,
- .trigger_ops = &traceoff_trigger_ops,
.parse = event_trigger_parse,
.reg = register_trigger,
.unreg = unregister_trigger,
.set_filter = set_trigger_filter,
+ .trigger = traceoff_trigger,
+ .count_func = traceoff_count_func,
+ .print = traceoff_trigger_print,
+ .init = event_trigger_init,
+ .free = event_trigger_free,
};
#ifdef CONFIG_TRACER_SNAPSHOT
@@ -1483,22 +1474,18 @@ snapshot_trigger_print(struct seq_file *m, struct event_trigger_data *data)
data->filter_str);
}
-static const struct event_trigger_ops snapshot_trigger_ops = {
- .trigger = snapshot_trigger,
- .count_func = event_trigger_count,
- .print = snapshot_trigger_print,
- .init = event_trigger_init,
- .free = event_trigger_free,
-};
-
static struct event_command trigger_snapshot_cmd = {
.name = "snapshot",
.trigger_type = ETT_SNAPSHOT,
- .trigger_ops = &snapshot_trigger_ops,
.parse = event_trigger_parse,
.reg = register_snapshot_trigger,
.unreg = unregister_snapshot_trigger,
.set_filter = set_trigger_filter,
+ .trigger = snapshot_trigger,
+ .count_func = event_trigger_count,
+ .print = snapshot_trigger_print,
+ .init = event_trigger_init,
+ .free = event_trigger_free,
};
static __init int register_trigger_snapshot_cmd(void)
@@ -1552,23 +1539,19 @@ stacktrace_trigger_print(struct seq_file *m, struct event_trigger_data *data)
data->filter_str);
}
-static const struct event_trigger_ops stacktrace_trigger_ops = {
- .trigger = stacktrace_trigger,
- .count_func = event_trigger_count,
- .print = stacktrace_trigger_print,
- .init = event_trigger_init,
- .free = event_trigger_free,
-};
-
static struct event_command trigger_stacktrace_cmd = {
.name = "stacktrace",
.trigger_type = ETT_STACKTRACE,
- .trigger_ops = &stacktrace_trigger_ops,
.flags = EVENT_CMD_FL_POST_TRIGGER,
.parse = event_trigger_parse,
.reg = register_trigger,
.unreg = unregister_trigger,
.set_filter = set_trigger_filter,
+ .trigger = stacktrace_trigger,
+ .count_func = event_trigger_count,
+ .print = stacktrace_trigger_print,
+ .init = event_trigger_init,
+ .free = event_trigger_free,
};
static __init int register_trigger_stacktrace_cmd(void)
@@ -1665,22 +1648,6 @@ void event_enable_trigger_free(struct event_trigger_data *data)
}
}
-static const struct event_trigger_ops event_enable_trigger_ops = {
- .trigger = event_enable_trigger,
- .count_func = event_enable_count_func,
- .print = event_enable_trigger_print,
- .init = event_trigger_init,
- .free = event_enable_trigger_free,
-};
-
-static const struct event_trigger_ops event_disable_trigger_ops = {
- .trigger = event_enable_trigger,
- .count_func = event_enable_count_func,
- .print = event_enable_trigger_print,
- .init = event_trigger_init,
- .free = event_enable_trigger_free,
-};
-
int event_enable_trigger_parse(struct event_command *cmd_ops,
struct trace_event_file *file,
char *glob, char *cmd, char *param_and_filter)
@@ -1810,8 +1777,8 @@ int event_enable_register_trigger(char *glob,
}
}
- if (data->ops->init) {
- ret = data->ops->init(data);
+ if (data->cmd_ops->init) {
+ ret = data->cmd_ops->init(data);
if (ret < 0)
return ret;
}
@@ -1851,28 +1818,36 @@ void event_enable_unregister_trigger(char *glob,
}
}
- if (data && data->ops->free)
- data->ops->free(data);
+ if (data && data->cmd_ops->free)
+ data->cmd_ops->free(data);
}
static struct event_command trigger_enable_cmd = {
.name = ENABLE_EVENT_STR,
.trigger_type = ETT_EVENT_ENABLE,
- .trigger_ops = &event_enable_trigger_ops,
.parse = event_enable_trigger_parse,
.reg = event_enable_register_trigger,
.unreg = event_enable_unregister_trigger,
.set_filter = set_trigger_filter,
+ .trigger = event_enable_trigger,
+ .count_func = event_enable_count_func,
+ .print = event_enable_trigger_print,
+ .init = event_trigger_init,
+ .free = event_enable_trigger_free,
};
static struct event_command trigger_disable_cmd = {
.name = DISABLE_EVENT_STR,
.trigger_type = ETT_EVENT_ENABLE,
- .trigger_ops = &event_disable_trigger_ops,
.parse = event_enable_trigger_parse,
.reg = event_enable_register_trigger,
.unreg = event_enable_unregister_trigger,
.set_filter = set_trigger_filter,
+ .trigger = event_enable_trigger,
+ .count_func = event_enable_count_func,
+ .print = event_enable_trigger_print,
+ .init = event_trigger_init,
+ .free = event_enable_trigger_free,
};
static __init void unregister_trigger_enable_disable_cmds(void)
--
2.51.0
^ permalink raw reply related
* [PATCH 1/2] tracing: Remove get_trigger_ops() and add count_func() from trigger ops
From: Steven Rostedt @ 2025-11-19 3:10 UTC (permalink / raw)
To: linux-kernel, linux-trace-kernel
Cc: Masami Hiramatsu, Mark Rutland, Mathieu Desnoyers, Andrew Morton,
Tom Zanussi
In-Reply-To: <20251119031042.328864818@kernel.org>
From: Steven Rostedt <rostedt@goodmis.org>
The struct event_command has a callback function called get_trigger_ops().
This callback returns the "trigger_ops" to use for the trigger. These ops
define the trigger function, how to init the trigger, how to print the
trigger and how to free it.
The only reason there's a callback function to get these ops is because
some triggers have two types of operations. One is an "always on"
operation, and the other is a "count down" operation. If a user passes in
a parameter to say how many times the trigger should execute. For example:
echo stacktrace:5 > events/kmem/kmem_cache_alloc/trigger
It will trigger the stacktrace for the first 5 times the kmem_cache_alloc
event is hit.
Instead of having two different trigger_ops since the only difference
between them is the tigger itself (the print, init and free functions are
all the same), just use a single ops that the event_command points to and
add a function field to the trigger_ops to have a count_func.
When a trigger is added to an event, if there's a count attached to it and
the trigger ops has the count_func field, the data allocated to represent
this trigger will have a new flag set called COUNT.
Then when the trigger executes, it will check if the COUNT data flag is
set, and if so, it will call the ops count_func(). If that returns false,
it returns without executing the trigger.
This removes the need for duplicate event_trigger_ops structures.
Signed-off-by: Steven Rostedt (Google) <rostedt@goodmis.org>
---
kernel/trace/trace.h | 26 ++-
kernel/trace/trace_eprobe.c | 8 +-
kernel/trace/trace_events_hist.c | 60 +------
kernel/trace/trace_events_trigger.c | 257 ++++++++++------------------
4 files changed, 116 insertions(+), 235 deletions(-)
diff --git a/kernel/trace/trace.h b/kernel/trace/trace.h
index 58be6d741d72..036019ffc407 100644
--- a/kernel/trace/trace.h
+++ b/kernel/trace/trace.h
@@ -1791,6 +1791,7 @@ extern void clear_event_triggers(struct trace_array *tr);
enum {
EVENT_TRIGGER_FL_PROBE = BIT(0),
+ EVENT_TRIGGER_FL_COUNT = BIT(1),
};
struct event_trigger_data {
@@ -1822,6 +1823,10 @@ struct enable_trigger_data {
bool hist;
};
+bool event_trigger_count(struct event_trigger_data *data,
+ struct trace_buffer *buffer, void *rec,
+ struct ring_buffer_event *event);
+
extern int event_enable_trigger_print(struct seq_file *m,
struct event_trigger_data *data);
extern void event_enable_trigger_free(struct event_trigger_data *data);
@@ -1909,6 +1914,11 @@ extern void event_file_put(struct trace_event_file *file);
* registered the trigger (see struct event_command) along with
* the trace record, rec.
*
+ * @count_func: If defined and a numeric parameter is passed to the
+ * trigger, then this function will be called before @trigger
+ * is called. If this function returns false, then @trigger is not
+ * executed.
+ *
* @init: An optional initialization function called for the trigger
* when the trigger is registered (via the event_command reg()
* function). This can be used to perform per-trigger
@@ -1936,6 +1946,10 @@ struct event_trigger_ops {
struct trace_buffer *buffer,
void *rec,
struct ring_buffer_event *rbe);
+ bool (*count_func)(struct event_trigger_data *data,
+ struct trace_buffer *buffer,
+ void *rec,
+ struct ring_buffer_event *rbe);
int (*init)(struct event_trigger_data *data);
void (*free)(struct event_trigger_data *data);
int (*print)(struct seq_file *m,
@@ -1962,6 +1976,9 @@ struct event_trigger_ops {
* @name: The unique name that identifies the event command. This is
* the name used when setting triggers via trigger files.
*
+ * @trigger_ops: The event_trigger_ops implementation associated with
+ * the command.
+ *
* @trigger_type: A unique id that identifies the event command
* 'type'. This value has two purposes, the first to ensure that
* only one trigger of the same type can be set at a given time
@@ -2013,17 +2030,11 @@ struct event_trigger_ops {
* event command, filters set by the user for the command will be
* ignored. This is usually implemented by the generic utility
* function @set_trigger_filter() (see trace_event_triggers.c).
- *
- * @get_trigger_ops: The callback function invoked to retrieve the
- * event_trigger_ops implementation associated with the command.
- * This callback function allows a single event_command to
- * support multiple trigger implementations via different sets of
- * event_trigger_ops, depending on the value of the @param
- * string.
*/
struct event_command {
struct list_head list;
char *name;
+ const struct event_trigger_ops *trigger_ops;
enum event_trigger_type trigger_type;
int flags;
int (*parse)(struct event_command *cmd_ops,
@@ -2040,7 +2051,6 @@ struct event_command {
int (*set_filter)(char *filter_str,
struct event_trigger_data *data,
struct trace_event_file *file);
- const struct event_trigger_ops *(*get_trigger_ops)(char *cmd, char *param);
};
/**
diff --git a/kernel/trace/trace_eprobe.c b/kernel/trace/trace_eprobe.c
index a1d402124836..14ae896dbe75 100644
--- a/kernel/trace/trace_eprobe.c
+++ b/kernel/trace/trace_eprobe.c
@@ -513,21 +513,15 @@ static void eprobe_trigger_unreg_func(char *glob,
}
-static const struct event_trigger_ops *eprobe_trigger_get_ops(char *cmd,
- char *param)
-{
- return &eprobe_trigger_ops;
-}
-
static struct event_command event_trigger_cmd = {
.name = "eprobe",
.trigger_type = ETT_EVENT_EPROBE,
.flags = EVENT_CMD_FL_NEEDS_REC,
+ .trigger_ops = &eprobe_trigger_ops,
.parse = eprobe_trigger_cmd_parse,
.reg = eprobe_trigger_reg_func,
.unreg = eprobe_trigger_unreg_func,
.unreg_all = NULL,
- .get_trigger_ops = eprobe_trigger_get_ops,
.set_filter = NULL,
};
diff --git a/kernel/trace/trace_events_hist.c b/kernel/trace/trace_events_hist.c
index 1d536219b624..f9cc8d6a215b 100644
--- a/kernel/trace/trace_events_hist.c
+++ b/kernel/trace/trace_events_hist.c
@@ -6363,12 +6363,6 @@ static const struct event_trigger_ops event_hist_trigger_named_ops = {
.free = event_hist_trigger_named_free,
};
-static const struct event_trigger_ops *event_hist_get_trigger_ops(char *cmd,
- char *param)
-{
- return &event_hist_trigger_ops;
-}
-
static void hist_clear(struct event_trigger_data *data)
{
struct hist_trigger_data *hist_data = data->private_data;
@@ -6908,11 +6902,11 @@ static struct event_command trigger_hist_cmd = {
.name = "hist",
.trigger_type = ETT_EVENT_HIST,
.flags = EVENT_CMD_FL_NEEDS_REC,
+ .trigger_ops = &event_hist_trigger_ops,
.parse = event_hist_trigger_parse,
.reg = hist_register_trigger,
.unreg = hist_unregister_trigger,
.unreg_all = hist_unreg_all,
- .get_trigger_ops = event_hist_get_trigger_ops,
.set_filter = set_trigger_filter,
};
@@ -6945,29 +6939,9 @@ hist_enable_trigger(struct event_trigger_data *data,
}
}
-static void
-hist_enable_count_trigger(struct event_trigger_data *data,
- struct trace_buffer *buffer, void *rec,
- struct ring_buffer_event *event)
-{
- if (!data->count)
- return;
-
- if (data->count != -1)
- (data->count)--;
-
- hist_enable_trigger(data, buffer, rec, event);
-}
-
static const struct event_trigger_ops hist_enable_trigger_ops = {
.trigger = hist_enable_trigger,
- .print = event_enable_trigger_print,
- .init = event_trigger_init,
- .free = event_enable_trigger_free,
-};
-
-static const struct event_trigger_ops hist_enable_count_trigger_ops = {
- .trigger = hist_enable_count_trigger,
+ .count_func = event_trigger_count,
.print = event_enable_trigger_print,
.init = event_trigger_init,
.free = event_enable_trigger_free,
@@ -6975,36 +6949,12 @@ static const struct event_trigger_ops hist_enable_count_trigger_ops = {
static const struct event_trigger_ops hist_disable_trigger_ops = {
.trigger = hist_enable_trigger,
+ .count_func = event_trigger_count,
.print = event_enable_trigger_print,
.init = event_trigger_init,
.free = event_enable_trigger_free,
};
-static const struct event_trigger_ops hist_disable_count_trigger_ops = {
- .trigger = hist_enable_count_trigger,
- .print = event_enable_trigger_print,
- .init = event_trigger_init,
- .free = event_enable_trigger_free,
-};
-
-static const struct event_trigger_ops *
-hist_enable_get_trigger_ops(char *cmd, char *param)
-{
- const struct event_trigger_ops *ops;
- bool enable;
-
- enable = (strcmp(cmd, ENABLE_HIST_STR) == 0);
-
- if (enable)
- ops = param ? &hist_enable_count_trigger_ops :
- &hist_enable_trigger_ops;
- else
- ops = param ? &hist_disable_count_trigger_ops :
- &hist_disable_trigger_ops;
-
- return ops;
-}
-
static void hist_enable_unreg_all(struct trace_event_file *file)
{
struct event_trigger_data *test, *n;
@@ -7023,22 +6973,22 @@ static void hist_enable_unreg_all(struct trace_event_file *file)
static struct event_command trigger_hist_enable_cmd = {
.name = ENABLE_HIST_STR,
.trigger_type = ETT_HIST_ENABLE,
+ .trigger_ops = &hist_enable_trigger_ops,
.parse = event_enable_trigger_parse,
.reg = event_enable_register_trigger,
.unreg = event_enable_unregister_trigger,
.unreg_all = hist_enable_unreg_all,
- .get_trigger_ops = hist_enable_get_trigger_ops,
.set_filter = set_trigger_filter,
};
static struct event_command trigger_hist_disable_cmd = {
.name = DISABLE_HIST_STR,
.trigger_type = ETT_HIST_ENABLE,
+ .trigger_ops = &hist_disable_trigger_ops,
.parse = event_enable_trigger_parse,
.reg = event_enable_register_trigger,
.unreg = event_enable_unregister_trigger,
.unreg_all = hist_enable_unreg_all,
- .get_trigger_ops = hist_enable_get_trigger_ops,
.set_filter = set_trigger_filter,
};
diff --git a/kernel/trace/trace_events_trigger.c b/kernel/trace/trace_events_trigger.c
index cbfc306c0159..576bad18bcdb 100644
--- a/kernel/trace/trace_events_trigger.c
+++ b/kernel/trace/trace_events_trigger.c
@@ -28,6 +28,20 @@ void trigger_data_free(struct event_trigger_data *data)
kfree(data);
}
+static inline void data_ops_trigger(struct event_trigger_data *data,
+ struct trace_buffer *buffer, void *rec,
+ struct ring_buffer_event *event)
+{
+ const struct event_trigger_ops *ops = data->ops;
+
+ if (data->flags & EVENT_TRIGGER_FL_COUNT) {
+ if (!ops->count_func(data, buffer, rec, event))
+ return;
+ }
+
+ ops->trigger(data, buffer, rec, event);
+}
+
/**
* event_triggers_call - Call triggers associated with a trace event
* @file: The trace_event_file associated with the event
@@ -70,7 +84,7 @@ event_triggers_call(struct trace_event_file *file,
if (data->paused)
continue;
if (!rec) {
- data->ops->trigger(data, buffer, rec, event);
+ data_ops_trigger(data, buffer, rec, event);
continue;
}
filter = rcu_dereference_sched(data->filter);
@@ -80,7 +94,7 @@ event_triggers_call(struct trace_event_file *file,
tt |= data->cmd_ops->trigger_type;
continue;
}
- data->ops->trigger(data, buffer, rec, event);
+ data_ops_trigger(data, buffer, rec, event);
}
return tt;
}
@@ -122,7 +136,7 @@ event_triggers_post_call(struct trace_event_file *file,
if (data->paused)
continue;
if (data->cmd_ops->trigger_type & tt)
- data->ops->trigger(data, NULL, NULL, NULL);
+ data_ops_trigger(data, NULL, NULL, NULL);
}
}
EXPORT_SYMBOL_GPL(event_triggers_post_call);
@@ -377,6 +391,36 @@ __init int unregister_event_command(struct event_command *cmd)
return -ENODEV;
}
+/**
+ * event_trigger_count - Optional count function for event triggers
+ * @data: Trigger-specific data
+ * @buffer: The ring buffer that the event is being written to
+ * @rec: The trace entry for the event, NULL for unconditional invocation
+ * @event: The event meta data in the ring buffer
+ *
+ * For triggers that can take a count parameter that doesn't do anything
+ * special, they can use this function to assign to their .count_func
+ * field.
+ *
+ * This simply does a count down of the @data->count field.
+ *
+ * If the @data->count is greater than zero, it will decrement it.
+ *
+ * Returns false if @data->count is zero, otherwise true.
+ */
+bool event_trigger_count(struct event_trigger_data *data,
+ struct trace_buffer *buffer, void *rec,
+ struct ring_buffer_event *event)
+{
+ if (!data->count)
+ return false;
+
+ if (data->count != -1)
+ (data->count)--;
+
+ return true;
+}
+
/**
* event_trigger_print - Generic event_trigger_ops @print implementation
* @name: The name of the event trigger
@@ -807,9 +851,13 @@ int event_trigger_separate_filter(char *param_and_filter, char **param,
* @private_data: User data to associate with the event trigger
*
* Allocate an event_trigger_data instance and initialize it. The
- * @cmd_ops are used along with the @cmd and @param to get the
- * trigger_ops to assign to the event_trigger_data. @private_data can
- * also be passed in and associated with the event_trigger_data.
+ * @cmd_ops defines how the trigger will operate. If @param is set,
+ * and @cmd_ops->trigger_ops->count_func is non NULL, then the
+ * data->count is set to @param and before the trigger is executed, the
+ * @cmd_ops->trigger_ops->count_func() is called. If that function returns
+ * false, the @cmd_ops->trigger_ops->trigger() function will not be called.
+ * @private_data can also be passed in and associated with the
+ * event_trigger_data.
*
* Use trigger_data_free() to free an event_trigger_data object.
*
@@ -821,18 +869,17 @@ struct event_trigger_data *trigger_data_alloc(struct event_command *cmd_ops,
void *private_data)
{
struct event_trigger_data *trigger_data;
- const struct event_trigger_ops *trigger_ops;
-
- trigger_ops = cmd_ops->get_trigger_ops(cmd, param);
trigger_data = kzalloc(sizeof(*trigger_data), GFP_KERNEL);
if (!trigger_data)
return NULL;
trigger_data->count = -1;
- trigger_data->ops = trigger_ops;
+ trigger_data->ops = cmd_ops->trigger_ops;
trigger_data->cmd_ops = cmd_ops;
trigger_data->private_data = private_data;
+ if (param && cmd_ops->trigger_ops->count_func)
+ trigger_data->flags |= EVENT_TRIGGER_FL_COUNT;
INIT_LIST_HEAD(&trigger_data->list);
INIT_LIST_HEAD(&trigger_data->named_list);
@@ -1271,31 +1318,28 @@ traceon_trigger(struct event_trigger_data *data,
tracing_on();
}
-static void
-traceon_count_trigger(struct event_trigger_data *data,
- struct trace_buffer *buffer, void *rec,
- struct ring_buffer_event *event)
+static bool
+traceon_count_func(struct event_trigger_data *data,
+ struct trace_buffer *buffer, void *rec,
+ struct ring_buffer_event *event)
{
struct trace_event_file *file = data->private_data;
if (file) {
if (tracer_tracing_is_on(file->tr))
- return;
+ return false;
} else {
if (tracing_is_on())
- return;
+ return false;
}
if (!data->count)
- return;
+ return false;
if (data->count != -1)
(data->count)--;
- if (file)
- tracer_tracing_on(file->tr);
- else
- tracing_on();
+ return true;
}
static void
@@ -1319,31 +1363,28 @@ traceoff_trigger(struct event_trigger_data *data,
tracing_off();
}
-static void
-traceoff_count_trigger(struct event_trigger_data *data,
- struct trace_buffer *buffer, void *rec,
- struct ring_buffer_event *event)
+static bool
+traceoff_count_func(struct event_trigger_data *data,
+ struct trace_buffer *buffer, void *rec,
+ struct ring_buffer_event *event)
{
struct trace_event_file *file = data->private_data;
if (file) {
if (!tracer_tracing_is_on(file->tr))
- return;
+ return false;
} else {
if (!tracing_is_on())
- return;
+ return false;
}
if (!data->count)
- return;
+ return false;
if (data->count != -1)
(data->count)--;
- if (file)
- tracer_tracing_off(file->tr);
- else
- tracing_off();
+ return true;
}
static int
@@ -1362,13 +1403,7 @@ traceoff_trigger_print(struct seq_file *m, struct event_trigger_data *data)
static const struct event_trigger_ops traceon_trigger_ops = {
.trigger = traceon_trigger,
- .print = traceon_trigger_print,
- .init = event_trigger_init,
- .free = event_trigger_free,
-};
-
-static const struct event_trigger_ops traceon_count_trigger_ops = {
- .trigger = traceon_count_trigger,
+ .count_func = traceon_count_func,
.print = traceon_trigger_print,
.init = event_trigger_init,
.free = event_trigger_free,
@@ -1376,41 +1411,19 @@ static const struct event_trigger_ops traceon_count_trigger_ops = {
static const struct event_trigger_ops traceoff_trigger_ops = {
.trigger = traceoff_trigger,
+ .count_func = traceoff_count_func,
.print = traceoff_trigger_print,
.init = event_trigger_init,
.free = event_trigger_free,
};
-static const struct event_trigger_ops traceoff_count_trigger_ops = {
- .trigger = traceoff_count_trigger,
- .print = traceoff_trigger_print,
- .init = event_trigger_init,
- .free = event_trigger_free,
-};
-
-static const struct event_trigger_ops *
-onoff_get_trigger_ops(char *cmd, char *param)
-{
- const struct event_trigger_ops *ops;
-
- /* we register both traceon and traceoff to this callback */
- if (strcmp(cmd, "traceon") == 0)
- ops = param ? &traceon_count_trigger_ops :
- &traceon_trigger_ops;
- else
- ops = param ? &traceoff_count_trigger_ops :
- &traceoff_trigger_ops;
-
- return ops;
-}
-
static struct event_command trigger_traceon_cmd = {
.name = "traceon",
.trigger_type = ETT_TRACE_ONOFF,
+ .trigger_ops = &traceon_trigger_ops,
.parse = event_trigger_parse,
.reg = register_trigger,
.unreg = unregister_trigger,
- .get_trigger_ops = onoff_get_trigger_ops,
.set_filter = set_trigger_filter,
};
@@ -1418,10 +1431,10 @@ static struct event_command trigger_traceoff_cmd = {
.name = "traceoff",
.trigger_type = ETT_TRACE_ONOFF,
.flags = EVENT_CMD_FL_POST_TRIGGER,
+ .trigger_ops = &traceoff_trigger_ops,
.parse = event_trigger_parse,
.reg = register_trigger,
.unreg = unregister_trigger,
- .get_trigger_ops = onoff_get_trigger_ops,
.set_filter = set_trigger_filter,
};
@@ -1439,20 +1452,6 @@ snapshot_trigger(struct event_trigger_data *data,
tracing_snapshot();
}
-static void
-snapshot_count_trigger(struct event_trigger_data *data,
- struct trace_buffer *buffer, void *rec,
- struct ring_buffer_event *event)
-{
- if (!data->count)
- return;
-
- if (data->count != -1)
- (data->count)--;
-
- snapshot_trigger(data, buffer, rec, event);
-}
-
static int
register_snapshot_trigger(char *glob,
struct event_trigger_data *data,
@@ -1486,31 +1485,19 @@ snapshot_trigger_print(struct seq_file *m, struct event_trigger_data *data)
static const struct event_trigger_ops snapshot_trigger_ops = {
.trigger = snapshot_trigger,
+ .count_func = event_trigger_count,
.print = snapshot_trigger_print,
.init = event_trigger_init,
.free = event_trigger_free,
};
-static const struct event_trigger_ops snapshot_count_trigger_ops = {
- .trigger = snapshot_count_trigger,
- .print = snapshot_trigger_print,
- .init = event_trigger_init,
- .free = event_trigger_free,
-};
-
-static const struct event_trigger_ops *
-snapshot_get_trigger_ops(char *cmd, char *param)
-{
- return param ? &snapshot_count_trigger_ops : &snapshot_trigger_ops;
-}
-
static struct event_command trigger_snapshot_cmd = {
.name = "snapshot",
.trigger_type = ETT_SNAPSHOT,
+ .trigger_ops = &snapshot_trigger_ops,
.parse = event_trigger_parse,
.reg = register_snapshot_trigger,
.unreg = unregister_snapshot_trigger,
- .get_trigger_ops = snapshot_get_trigger_ops,
.set_filter = set_trigger_filter,
};
@@ -1558,20 +1545,6 @@ stacktrace_trigger(struct event_trigger_data *data,
trace_dump_stack(STACK_SKIP);
}
-static void
-stacktrace_count_trigger(struct event_trigger_data *data,
- struct trace_buffer *buffer, void *rec,
- struct ring_buffer_event *event)
-{
- if (!data->count)
- return;
-
- if (data->count != -1)
- (data->count)--;
-
- stacktrace_trigger(data, buffer, rec, event);
-}
-
static int
stacktrace_trigger_print(struct seq_file *m, struct event_trigger_data *data)
{
@@ -1581,32 +1554,20 @@ stacktrace_trigger_print(struct seq_file *m, struct event_trigger_data *data)
static const struct event_trigger_ops stacktrace_trigger_ops = {
.trigger = stacktrace_trigger,
+ .count_func = event_trigger_count,
.print = stacktrace_trigger_print,
.init = event_trigger_init,
.free = event_trigger_free,
};
-static const struct event_trigger_ops stacktrace_count_trigger_ops = {
- .trigger = stacktrace_count_trigger,
- .print = stacktrace_trigger_print,
- .init = event_trigger_init,
- .free = event_trigger_free,
-};
-
-static const struct event_trigger_ops *
-stacktrace_get_trigger_ops(char *cmd, char *param)
-{
- return param ? &stacktrace_count_trigger_ops : &stacktrace_trigger_ops;
-}
-
static struct event_command trigger_stacktrace_cmd = {
.name = "stacktrace",
.trigger_type = ETT_STACKTRACE,
+ .trigger_ops = &stacktrace_trigger_ops,
.flags = EVENT_CMD_FL_POST_TRIGGER,
.parse = event_trigger_parse,
.reg = register_trigger,
.unreg = unregister_trigger,
- .get_trigger_ops = stacktrace_get_trigger_ops,
.set_filter = set_trigger_filter,
};
@@ -1642,24 +1603,24 @@ event_enable_trigger(struct event_trigger_data *data,
set_bit(EVENT_FILE_FL_SOFT_DISABLED_BIT, &enable_data->file->flags);
}
-static void
-event_enable_count_trigger(struct event_trigger_data *data,
- struct trace_buffer *buffer, void *rec,
- struct ring_buffer_event *event)
+static bool
+event_enable_count_func(struct event_trigger_data *data,
+ struct trace_buffer *buffer, void *rec,
+ struct ring_buffer_event *event)
{
struct enable_trigger_data *enable_data = data->private_data;
if (!data->count)
- return;
+ return false;
/* Skip if the event is in a state we want to switch to */
if (enable_data->enable == !(enable_data->file->flags & EVENT_FILE_FL_SOFT_DISABLED))
- return;
+ return false;
if (data->count != -1)
(data->count)--;
- event_enable_trigger(data, buffer, rec, event);
+ return true;
}
int event_enable_trigger_print(struct seq_file *m,
@@ -1706,13 +1667,7 @@ void event_enable_trigger_free(struct event_trigger_data *data)
static const struct event_trigger_ops event_enable_trigger_ops = {
.trigger = event_enable_trigger,
- .print = event_enable_trigger_print,
- .init = event_trigger_init,
- .free = event_enable_trigger_free,
-};
-
-static const struct event_trigger_ops event_enable_count_trigger_ops = {
- .trigger = event_enable_count_trigger,
+ .count_func = event_enable_count_func,
.print = event_enable_trigger_print,
.init = event_trigger_init,
.free = event_enable_trigger_free,
@@ -1720,13 +1675,7 @@ static const struct event_trigger_ops event_enable_count_trigger_ops = {
static const struct event_trigger_ops event_disable_trigger_ops = {
.trigger = event_enable_trigger,
- .print = event_enable_trigger_print,
- .init = event_trigger_init,
- .free = event_enable_trigger_free,
-};
-
-static const struct event_trigger_ops event_disable_count_trigger_ops = {
- .trigger = event_enable_count_trigger,
+ .count_func = event_enable_count_func,
.print = event_enable_trigger_print,
.init = event_trigger_init,
.free = event_enable_trigger_free,
@@ -1906,45 +1855,23 @@ void event_enable_unregister_trigger(char *glob,
data->ops->free(data);
}
-static const struct event_trigger_ops *
-event_enable_get_trigger_ops(char *cmd, char *param)
-{
- const struct event_trigger_ops *ops;
- bool enable;
-
-#ifdef CONFIG_HIST_TRIGGERS
- enable = ((strcmp(cmd, ENABLE_EVENT_STR) == 0) ||
- (strcmp(cmd, ENABLE_HIST_STR) == 0));
-#else
- enable = strcmp(cmd, ENABLE_EVENT_STR) == 0;
-#endif
- if (enable)
- ops = param ? &event_enable_count_trigger_ops :
- &event_enable_trigger_ops;
- else
- ops = param ? &event_disable_count_trigger_ops :
- &event_disable_trigger_ops;
-
- return ops;
-}
-
static struct event_command trigger_enable_cmd = {
.name = ENABLE_EVENT_STR,
.trigger_type = ETT_EVENT_ENABLE,
+ .trigger_ops = &event_enable_trigger_ops,
.parse = event_enable_trigger_parse,
.reg = event_enable_register_trigger,
.unreg = event_enable_unregister_trigger,
- .get_trigger_ops = event_enable_get_trigger_ops,
.set_filter = set_trigger_filter,
};
static struct event_command trigger_disable_cmd = {
.name = DISABLE_EVENT_STR,
.trigger_type = ETT_EVENT_ENABLE,
+ .trigger_ops = &event_disable_trigger_ops,
.parse = event_enable_trigger_parse,
.reg = event_enable_register_trigger,
.unreg = event_enable_unregister_trigger,
- .get_trigger_ops = event_enable_get_trigger_ops,
.set_filter = set_trigger_filter,
};
--
2.51.0
^ permalink raw reply related
* [PATCH 0/2] tracing: Clean up trigger code my merging structures
From: Steven Rostedt @ 2025-11-19 3:10 UTC (permalink / raw)
To: linux-kernel, linux-trace-kernel
Cc: Masami Hiramatsu, Mark Rutland, Mathieu Desnoyers, Andrew Morton,
Tom Zanussi
The struct event_command has a callback function called get_trigger_ops().
This callback returns the "trigger_ops" to use for the trigger. These ops
define the trigger function, how to init the trigger, how to print the
trigger and how to free it.
The only reason there's a callback function to get these ops is because
some triggers have two types of operations. One is an "always on"
operation, and the other is a "count down" operation. If a user passes in
a parameter to say how many times the trigger should execute. For example:
echo stacktrace:5 > events/kmem/kmem_cache_alloc/trigger
It will trigger the stacktrace for the first 5 times the kmem_cache_alloc
event is hit.
Instead of having two different trigger_ops since the only difference
between them is the tigger itself (the print, init and free functions are
all the same), just use a single ops that the event_command points to and
add a function field to the trigger_ops to have a count_func.
When a trigger is added to an event, if there's a count attached to it and
the trigger ops has the count_func field, the data allocated to represent
this trigger will have a new flag set called COUNT.
Then when the trigger executes, it will check if the COUNT data flag is
set, and if so, it will call the ops count_func(). If that returns false,
it returns without executing the trigger.
After making the struct event_trigger_ops be mapped one to one with the
struct event_command, merge the former into the latter.
Steven Rostedt (2):
tracing: Remove get_trigger_ops() and add count_func() from trigger ops
tracing: Merge struct event_trigger_ops into struct event_command
----
kernel/trace/trace.h | 124 ++++++-------
kernel/trace/trace_eprobe.c | 19 +-
kernel/trace/trace_events_hist.c | 143 +++++----------
kernel/trace/trace_events_trigger.c | 344 +++++++++++++-----------------------
4 files changed, 231 insertions(+), 399 deletions(-)
^ permalink raw reply
* Re: [PATCH bpf-next v3 0/6] bpf trampoline support "jmp" mode
From: Leon Hwang @ 2025-11-19 2:55 UTC (permalink / raw)
To: Menglong Dong, Menglong Dong, Alexei Starovoitov, Leon Hwang
Cc: Alexei Starovoitov, Steven Rostedt, Daniel Borkmann,
John Fastabend, Andrii Nakryiko, Martin KaFai Lau, Eduard,
Song Liu, Yonghong Song, KP Singh, Stanislav Fomichev, Hao Luo,
Jiri Olsa, Masami Hiramatsu, Mark Rutland, Mathieu Desnoyers,
jiang.biao, bpf, LKML, linux-trace-kernel
In-Reply-To: <8606158.T7Z3S40VBb@7950hx>
On 19/11/25 10:47, Menglong Dong wrote:
> On 2025/11/19 08:28, Alexei Starovoitov wrote:
>> On Tue, Nov 18, 2025 at 4:36 AM Menglong Dong <menglong8.dong@gmail.com> wrote:
>>>
>>> As we can see above, the performance of fexit increase from 80.544M/s to
>>> 136.540M/s, and the "fmodret" increase from 78.301M/s to 159.248M/s.
>>
>> Nice! Now we're talking.
>>
>> I think arm64 CPUs have a similar RSB-like return address predictor.
>> Do we need to do something similar there?
>> The question is not targeted to you, Menglong,
>> just wondering.
>
> I did some research before, and I find that most arch
> have such RSB-like stuff. I'll have a look at the loongarch
> later(maybe after the LPC, as I'm forcing on the English practice),
> and Leon is following the arm64.
Yep, happy to take this on.
I'm reviewing the arm64 JIT code now and will experiment with possible
approaches to handle this as well.
Thanks,
Leon
>
> For the other arch, we don't have the machine, and I think
> it needs some else help.
>
> Thanks!
> Menglong Dong
^ permalink raw reply
* Re: [PATCH bpf-next v3 0/6] bpf trampoline support "jmp" mode
From: Menglong Dong @ 2025-11-19 2:47 UTC (permalink / raw)
To: Menglong Dong, Alexei Starovoitov, Leon Hwang
Cc: Alexei Starovoitov, Steven Rostedt, Daniel Borkmann,
John Fastabend, Andrii Nakryiko, Martin KaFai Lau, Eduard,
Song Liu, Yonghong Song, KP Singh, Stanislav Fomichev, Hao Luo,
Jiri Olsa, Masami Hiramatsu, Mark Rutland, Mathieu Desnoyers,
jiang.biao, bpf, LKML, linux-trace-kernel
In-Reply-To: <CAADnVQJF5qkT8J=VJW00pPX7=hVdwn2545BzZPEi=mPwFouThw@mail.gmail.com>
On 2025/11/19 08:28, Alexei Starovoitov wrote:
> On Tue, Nov 18, 2025 at 4:36 AM Menglong Dong <menglong8.dong@gmail.com> wrote:
> >
> > As we can see above, the performance of fexit increase from 80.544M/s to
> > 136.540M/s, and the "fmodret" increase from 78.301M/s to 159.248M/s.
>
> Nice! Now we're talking.
>
> I think arm64 CPUs have a similar RSB-like return address predictor.
> Do we need to do something similar there?
> The question is not targeted to you, Menglong,
> just wondering.
I did some research before, and I find that most arch
have such RSB-like stuff. I'll have a look at the loongarch
later(maybe after the LPC, as I'm forcing on the English practice),
and Leon is following the arm64.
For the other arch, we don't have the machine, and I think
it needs some else help.
Thanks!
Menglong Dong
>
>
^ permalink raw reply
* Re: [PATCH bpf-next v3 2/6] x86/ftrace: implement DYNAMIC_FTRACE_WITH_JMP
From: Menglong Dong @ 2025-11-19 1:05 UTC (permalink / raw)
To: Menglong Dong, Jiri Olsa
Cc: ast, rostedt, daniel, john.fastabend, andrii, martin.lau, eddyz87,
song, yonghong.song, kpsingh, sdf, haoluo, mhiramat, mark.rutland,
mathieu.desnoyers, jiang.biao, bpf, linux-kernel,
linux-trace-kernel
In-Reply-To: <aRzs1GGLCm5svW5_@krava>
On 2025/11/19 06:01, Jiri Olsa wrote:
> On Tue, Nov 18, 2025 at 08:36:30PM +0800, Menglong Dong wrote:
> > Implement the DYNAMIC_FTRACE_WITH_JMP for x86_64. In ftrace_call_replace,
> > we will use JMP32_INSN_OPCODE instead of CALL_INSN_OPCODE if the address
> > should use "jmp".
> >
> > Meanwhile, adjust the direct call in the ftrace_regs_caller. The RSB is
> > balanced in the "jmp" mode. Take the function "foo" for example:
> >
> > original_caller:
> > call foo -> foo:
> > call fentry -> fentry:
> > [do ftrace callbacks ]
> > move tramp_addr to stack
> > RET -> tramp_addr
> > tramp_addr:
> > [..]
> > call foo_body -> foo_body:
> > [..]
> > RET -> back to tramp_addr
> > [..]
> > RET -> back to original_caller
> >
> > Signed-off-by: Menglong Dong <dongml2@chinatelecom.cn>
> > ---
> > arch/x86/Kconfig | 1 +
> > arch/x86/kernel/ftrace.c | 7 ++++++-
> > arch/x86/kernel/ftrace_64.S | 12 +++++++++++-
> > 3 files changed, 18 insertions(+), 2 deletions(-)
> >
> > diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
> > index fa3b616af03a..462250a20311 100644
> > --- a/arch/x86/Kconfig
> > +++ b/arch/x86/Kconfig
> > @@ -230,6 +230,7 @@ config X86
> > select HAVE_DYNAMIC_FTRACE_WITH_ARGS if X86_64
> > select HAVE_FTRACE_REGS_HAVING_PT_REGS if X86_64
> > select HAVE_DYNAMIC_FTRACE_WITH_DIRECT_CALLS
> > + select HAVE_DYNAMIC_FTRACE_WITH_JMP if X86_64
> > select HAVE_SAMPLE_FTRACE_DIRECT if X86_64
> > select HAVE_SAMPLE_FTRACE_DIRECT_MULTI if X86_64
> > select HAVE_EBPF_JIT
> > diff --git a/arch/x86/kernel/ftrace.c b/arch/x86/kernel/ftrace.c
> > index 4450acec9390..0543b57f54ee 100644
> > --- a/arch/x86/kernel/ftrace.c
> > +++ b/arch/x86/kernel/ftrace.c
> > @@ -74,7 +74,12 @@ static const char *ftrace_call_replace(unsigned long ip, unsigned long addr)
> > * No need to translate into a callthunk. The trampoline does
> > * the depth accounting itself.
> > */
> > - return text_gen_insn(CALL_INSN_OPCODE, (void *)ip, (void *)addr);
> > + if (ftrace_is_jmp(addr)) {
> > + addr = ftrace_jmp_get(addr);
> > + return text_gen_insn(JMP32_INSN_OPCODE, (void *)ip, (void *)addr);
> > + } else {
> > + return text_gen_insn(CALL_INSN_OPCODE, (void *)ip, (void *)addr);
> > + }
> > }
> >
> > static int ftrace_verify_code(unsigned long ip, const char *old_code)
> > diff --git a/arch/x86/kernel/ftrace_64.S b/arch/x86/kernel/ftrace_64.S
> > index 823dbdd0eb41..a132608265f6 100644
> > --- a/arch/x86/kernel/ftrace_64.S
> > +++ b/arch/x86/kernel/ftrace_64.S
> > @@ -285,8 +285,18 @@ SYM_INNER_LABEL(ftrace_regs_caller_end, SYM_L_GLOBAL)
> > ANNOTATE_NOENDBR
> > RET
> >
> > +1:
> > + testb $1, %al
> > + jz 2f
> > + andq $0xfffffffffffffffe, %rax
> > + movq %rax, MCOUNT_REG_SIZE+8(%rsp)
> > + restore_mcount_regs
> > + /* Restore flags */
> > + popfq
> > + RET
>
> is this hunk the reason for the 0x1 jmp-bit you set in the address?
Exactly!
>
> I wonder if we introduced new flag in dyn_ftrace::flags for this,
> then we'd need to have extra ftrace trampoline for jmp ftrace_ops
We don't introduce new dyn_ftrace::flags. I tried to introduce
FTRACE_FL_JMP and FTRACE_FL_JMP_EN for this propose before I
added the jmp-bit to the address. It's hard to do it this way.
First, we need to introduce a ftrace_regs_jmp_caller, which will
be used for the "jmp" mode. However, it's difficult when we need
to change a call_address to jmp_address in __modify_ftrace_direct(),
as it will change the "entry->direct" directly. And maybe we need
reconstruct the direct call to implement it this way.
I were almost giving up before I thought the jmp-bit, which allow
us the update the address from call mode to jmp mode atomically.
Thanks!
Menglong Dong
>
> jirka
>
>
^ permalink raw reply
* Re: [PATCH bpf-next v3 6/6] bpf: implement "jmp" mode for trampoline
From: Steven Rostedt @ 2025-11-19 1:03 UTC (permalink / raw)
To: Alexei Starovoitov
Cc: Menglong Dong, Alexei Starovoitov, Daniel Borkmann,
John Fastabend, Andrii Nakryiko, Martin KaFai Lau, Eduard,
Song Liu, Yonghong Song, KP Singh, Stanislav Fomichev, Hao Luo,
Jiri Olsa, Masami Hiramatsu, Mark Rutland, Mathieu Desnoyers,
jiang.biao, bpf, LKML, linux-trace-kernel
In-Reply-To: <CAADnVQ+mHb0AZe=J+yswjMiXLToG3-_3cfMxnNJJM-KAukbxBw@mail.gmail.com>
On Tue, 18 Nov 2025 16:59:59 -0800
Alexei Starovoitov <alexei.starovoitov@gmail.com> wrote:
> Steven,
> are you happy with patch 1?
> Can you pls Ack?
Let me run patch 1 and 2 through my tests.
-- Steve
^ permalink raw reply
* Re: [PATCH bpf-next v3 6/6] bpf: implement "jmp" mode for trampoline
From: Alexei Starovoitov @ 2025-11-19 0:59 UTC (permalink / raw)
To: Menglong Dong
Cc: Alexei Starovoitov, Steven Rostedt, Daniel Borkmann,
John Fastabend, Andrii Nakryiko, Martin KaFai Lau, Eduard,
Song Liu, Yonghong Song, KP Singh, Stanislav Fomichev, Hao Luo,
Jiri Olsa, Masami Hiramatsu, Mark Rutland, Mathieu Desnoyers,
jiang.biao, bpf, LKML, linux-trace-kernel
In-Reply-To: <20251118123639.688444-7-dongml2@chinatelecom.cn>
On Tue, Nov 18, 2025 at 4:37 AM Menglong Dong <menglong8.dong@gmail.com> wrote:
>
> Implement the "jmp" mode for the bpf trampoline. For the ftrace_managed
> case, we need only to set the FTRACE_OPS_FL_JMP on the tr->fops if "jmp"
> is needed.
>
> For the bpf poke case, we will check the origin poke type with the
> "origin_flags", and current poke type with "tr->flags". The function
> bpf_trampoline_update_fentry() is introduced to do the job.
>
> The "jmp" mode will only be enabled with CONFIG_DYNAMIC_FTRACE_WITH_JMP
> enabled and BPF_TRAMP_F_SHARE_IPMODIFY is not set. With
> BPF_TRAMP_F_SHARE_IPMODIFY, we need to get the origin call ip from the
> stack, so we can't use the "jmp" mode.
>
> Signed-off-by: Menglong Dong <dongml2@chinatelecom.cn>
> ---
> v3:
> - wrap the write to tr->fops->flags with CONFIG_DYNAMIC_FTRACE_WITH_JMP
> - reset BPF_TRAMP_F_SKIP_FRAME when the second try of modify_fentry in
> bpf_trampoline_update()
All looks good to me.
Steven,
are you happy with patch 1?
Can you pls Ack?
^ permalink raw reply
* Re: [PATCH bpf-next v3 0/6] bpf trampoline support "jmp" mode
From: Alexei Starovoitov @ 2025-11-19 0:28 UTC (permalink / raw)
To: Menglong Dong
Cc: Alexei Starovoitov, Steven Rostedt, Daniel Borkmann,
John Fastabend, Andrii Nakryiko, Martin KaFai Lau, Eduard,
Song Liu, Yonghong Song, KP Singh, Stanislav Fomichev, Hao Luo,
Jiri Olsa, Masami Hiramatsu, Mark Rutland, Mathieu Desnoyers,
jiang.biao, bpf, LKML, linux-trace-kernel
In-Reply-To: <20251118123639.688444-1-dongml2@chinatelecom.cn>
On Tue, Nov 18, 2025 at 4:36 AM Menglong Dong <menglong8.dong@gmail.com> wrote:
>
> As we can see above, the performance of fexit increase from 80.544M/s to
> 136.540M/s, and the "fmodret" increase from 78.301M/s to 159.248M/s.
Nice! Now we're talking.
I think arm64 CPUs have a similar RSB-like return address predictor.
Do we need to do something similar there?
The question is not targeted to you, Menglong,
just wondering.
^ permalink raw reply
* [PATCH v2] tracing: Show the tracer options in boot-time created instance
From: Masami Hiramatsu (Google) @ 2025-11-19 0:21 UTC (permalink / raw)
To: Steven Rostedt
Cc: Masami Hiramatsu, Mark Rutland, Mathieu Desnoyers, Andrew Morton,
linux-kernel, linux-trace-kernel
From: Masami Hiramatsu (Google) <mhiramat@kernel.org>
Since tracer_init_tracefs_work_func() only updates the tracer options
for the global_trace, the instances created by the kernel cmdline
do not have those options.
Fix to update tracer options for those boot-time created instances
to show those options.
Fixes: 428add559b69 ("tracing: Have tracer option be instance specific")
Signed-off-by: Masami Hiramatsu (Google) <mhiramat@kernel.org>
---
Change in v2:
- Fix update_tracer_options() to update all instances.
- Update tracer_options_updated after update all instances.
---
kernel/trace/trace.c | 11 ++++++++---
1 file changed, 8 insertions(+), 3 deletions(-)
diff --git a/kernel/trace/trace.c b/kernel/trace/trace.c
index 8ae95800592d..4a6784057855 100644
--- a/kernel/trace/trace.c
+++ b/kernel/trace/trace.c
@@ -10231,11 +10231,16 @@ static __init int __update_tracer_options(struct trace_array *tr)
return ret;
}
-static __init void update_tracer_options(struct trace_array *tr)
+static __init void update_tracer_options(void)
{
+ struct trace_array *tr;
+
guard(mutex)(&trace_types_lock);
+
+ list_for_each_entry(tr, &ftrace_trace_arrays, list)
+ __update_tracer_options(tr);
+
tracer_options_updated = true;
- __update_tracer_options(tr);
}
/* Must have trace_types_lock held */
@@ -10937,7 +10942,7 @@ static __init void tracer_init_tracefs_work_func(struct work_struct *work)
create_trace_instances(NULL);
- update_tracer_options(&global_trace);
+ update_tracer_options();
}
static __init int tracer_init_tracefs(void)
^ permalink raw reply related
* Re: [PATCH] tracing: Show the tracer options in boot-time created instance
From: Masami Hiramatsu @ 2025-11-19 0:06 UTC (permalink / raw)
To: Steven Rostedt
Cc: Mark Rutland, Mathieu Desnoyers, Andrew Morton, linux-kernel,
linux-trace-kernel
In-Reply-To: <20251118133434.67def626@gandalf.local.home>
On Tue, 18 Nov 2025 13:34:34 -0500
Steven Rostedt <rostedt@goodmis.org> wrote:
> On Tue, 18 Nov 2025 16:49:50 +0900
> "Masami Hiramatsu (Google)" <mhiramat@kernel.org> wrote:
>
> > From: Masami Hiramatsu (Google) <mhiramat@kernel.org>
> >
> > Since tracer_init_tracefs_work_func() only updates the tracer options
> > for the global_trace, the instances created by the kernel cmdline
> > do not have those options.
> >
> > Fix to update tracer options for those boot-time created instances
> > to show those options.
> >
> > Signed-off-by: Masami Hiramatsu (Google) <mhiramat@kernel.org>
> > Fixes: f20a580627f43 ("ftrace: Allow instances to use function tracing")
>
> Actually, I think this should be:
>
> Fixes: 428add559b69 ("tracing: Have tracer option be instance specific")
>
> As it broke when we made function tracer options no longer global.
>
OK, let me change it.
> > ---
> > kernel/trace/trace.c | 4 +++-
> > 1 file changed, 3 insertions(+), 1 deletion(-)
> >
> > diff --git a/kernel/trace/trace.c b/kernel/trace/trace.c
> > index 8ae95800592d..2e87060e1d5a 100644
> > --- a/kernel/trace/trace.c
> > +++ b/kernel/trace/trace.c
> > @@ -10903,6 +10903,7 @@ static struct notifier_block trace_module_nb = {
> >
> > static __init void tracer_init_tracefs_work_func(struct work_struct *work)
> > {
> > + struct trace_array *tr;
> >
> > event_trace_init();
> >
> > @@ -10937,7 +10938,8 @@ static __init void tracer_init_tracefs_work_func(struct work_struct *work)
> >
> > create_trace_instances(NULL);
> >
> > - update_tracer_options(&global_trace);
> > + list_for_each_entry(tr, &ftrace_trace_arrays, list)
> > + update_tracer_options(tr);
> > }
> >
> > static __init int tracer_init_tracefs(void)
>
> Since update_trace_options() is only called by this function, instead of
> doing the loop here, change the function to be:
>
> static __init void update_tracer_options(void)
> {
> struct trace_array *tr;
>
> guard(mutex)(&trace_types_lock);
> tracer_options_updated = true;
> list_for_each_entry(tr, &ftrace_trace_arrays, list)
> __update_tracer_options(tr);
> }
OK. Let me send v2.
Thanks,
>
> Thanks,
>
> -- Steve
>
--
Masami Hiramatsu (Google) <mhiramat@kernel.org>
^ permalink raw reply
* Re: [RFC PATCH 2/8] mm: Add PG_atomic
From: Dave Chinner @ 2025-11-18 23:30 UTC (permalink / raw)
To: Ritesh Harjani
Cc: Matthew Wilcox, Ojaswin Mujoo, Christian Brauner, djwong,
john.g.garry, tytso, dchinner, hch, linux-xfs, linux-kernel,
linux-ext4, linux-fsdevel, linux-mm, jack, nilay, martin.petersen,
rostedt, axboe, linux-block, linux-trace-kernel
In-Reply-To: <878qg32u3d.ritesh.list@gmail.com>
On Tue, Nov 18, 2025 at 09:47:42PM +0530, Ritesh Harjani wrote:
> Matthew Wilcox <willy@infradead.org> writes:
>
> > On Fri, Nov 14, 2025 at 10:30:09AM +0530, Ritesh Harjani wrote:
> >> Matthew Wilcox <willy@infradead.org> writes:
> >>
> >> > On Wed, Nov 12, 2025 at 04:36:05PM +0530, Ojaswin Mujoo wrote:
> >> >> From: John Garry <john.g.garry@oracle.com>
> >> >>
> >> >> Add page flag PG_atomic, meaning that a folio needs to be written back
> >> >> atomically. This will be used by for handling RWF_ATOMIC buffered IO
> >> >> in upcoming patches.
> >> >
> >> > Page flags are a precious resource. I'm not thrilled about allocating one
> >> > to this rather niche usecase. Wouldn't this be more aptly a flag on the
> >> > address_space rather than the folio? ie if we're doing this kind of write
> >> > to a file, aren't most/all of the writes to the file going to be atomic?
> >>
> >> As of today the atomic writes functionality works on the per-write
> >> basis (given it's a per-write characteristic).
> >>
> >> So, we can have two types of dirty folios sitting in the page cache of
> >> an inode. Ones which were done using atomic buffered I/O flag
> >> (RWF_ATOMIC) and the other ones which were non-atomic writes. Hence a
> >> need of a folio flag to distinguish between the two writes.
> >
> > I know, but is this useful? AFAIK, the files where Postgres wants to
> > use this functionality are the log files, and all writes to the log
> > files will want to use the atomic functionality. What's the usecase
> > for "I want to mix atomic and non-atomic buffered writes to this file"?
>
> Actually this goes back to the design of how we added support of atomic
> writes during DIO. So during the initial design phase we decided that
> this need not be a per-inode attribute or an open flag, but this is a
> per write I/O characteristic.
>
> So as per the current design, we don't have any open flag or a
> persistent inode attribute which says kernel should permit _only_ atomic
> writes I/O to this file. Instead what we support today is DIO atomic
> writes using RWF_ATOMIC flag in pwritev2 syscall.
Which, if we can't do with REQ_ATOMIC IO, we fall back to the
filesystem COW IO path to provide RWF_ATOMIC semantics without
needing to involve the page cache.
IOWs, DIO REQ_ATOMIC writes are simply a fast path for the atomic
COW IO path inherent in COW-capable filesystems.
This is no different for buffered RWF_ATOMIC writes. We need to
ingest the data into the page cache as a COW operation, then at
writeback time we optimise away the COW operations if REQ_ATOMIC IO
can be performed instead.
Using COW for buffered RWF_ATOMIC writes means don't need to involve
the page caceh at all - this can all be implemented at the
filesystem extent mapping and iomap layers....
> Having said that there can be several policy decision that could still be
> discussed e.g. make sure any previous dirty data is flushed to disk when a
> buffered atomic write request is made to an inode.
We don't need to care about mixed dirty non-atomic/atomic data on the
same file if REQ_ATOMIC is used as an optimisation for COW-based
atomic IO. Filesystems like XFS naturally separate COW and non-COW
extents. If we combine non-atomic and atomic data into a single
atomic update at writeback(be it COW or REQ_ATOMIC IO), then we
have still honoured the requested atomic semantics required to
persist the data. It just doesn't matter.
IMO, trying to hack atomic physical IO semantics through the page
cache creates all sorts of issues that simply don't exist when we
use the atomic overwrite paths present in modern COW capable
filesystems....
-Dave.
--
Dave Chinner
david@fromorbit.com
^ permalink raw reply
* Re: [PATCH bpf-next v3 2/6] x86/ftrace: implement DYNAMIC_FTRACE_WITH_JMP
From: Jiri Olsa @ 2025-11-18 22:01 UTC (permalink / raw)
To: Menglong Dong
Cc: ast, rostedt, daniel, john.fastabend, andrii, martin.lau, eddyz87,
song, yonghong.song, kpsingh, sdf, haoluo, mhiramat, mark.rutland,
mathieu.desnoyers, jiang.biao, bpf, linux-kernel,
linux-trace-kernel
In-Reply-To: <20251118123639.688444-3-dongml2@chinatelecom.cn>
On Tue, Nov 18, 2025 at 08:36:30PM +0800, Menglong Dong wrote:
> Implement the DYNAMIC_FTRACE_WITH_JMP for x86_64. In ftrace_call_replace,
> we will use JMP32_INSN_OPCODE instead of CALL_INSN_OPCODE if the address
> should use "jmp".
>
> Meanwhile, adjust the direct call in the ftrace_regs_caller. The RSB is
> balanced in the "jmp" mode. Take the function "foo" for example:
>
> original_caller:
> call foo -> foo:
> call fentry -> fentry:
> [do ftrace callbacks ]
> move tramp_addr to stack
> RET -> tramp_addr
> tramp_addr:
> [..]
> call foo_body -> foo_body:
> [..]
> RET -> back to tramp_addr
> [..]
> RET -> back to original_caller
>
> Signed-off-by: Menglong Dong <dongml2@chinatelecom.cn>
> ---
> arch/x86/Kconfig | 1 +
> arch/x86/kernel/ftrace.c | 7 ++++++-
> arch/x86/kernel/ftrace_64.S | 12 +++++++++++-
> 3 files changed, 18 insertions(+), 2 deletions(-)
>
> diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
> index fa3b616af03a..462250a20311 100644
> --- a/arch/x86/Kconfig
> +++ b/arch/x86/Kconfig
> @@ -230,6 +230,7 @@ config X86
> select HAVE_DYNAMIC_FTRACE_WITH_ARGS if X86_64
> select HAVE_FTRACE_REGS_HAVING_PT_REGS if X86_64
> select HAVE_DYNAMIC_FTRACE_WITH_DIRECT_CALLS
> + select HAVE_DYNAMIC_FTRACE_WITH_JMP if X86_64
> select HAVE_SAMPLE_FTRACE_DIRECT if X86_64
> select HAVE_SAMPLE_FTRACE_DIRECT_MULTI if X86_64
> select HAVE_EBPF_JIT
> diff --git a/arch/x86/kernel/ftrace.c b/arch/x86/kernel/ftrace.c
> index 4450acec9390..0543b57f54ee 100644
> --- a/arch/x86/kernel/ftrace.c
> +++ b/arch/x86/kernel/ftrace.c
> @@ -74,7 +74,12 @@ static const char *ftrace_call_replace(unsigned long ip, unsigned long addr)
> * No need to translate into a callthunk. The trampoline does
> * the depth accounting itself.
> */
> - return text_gen_insn(CALL_INSN_OPCODE, (void *)ip, (void *)addr);
> + if (ftrace_is_jmp(addr)) {
> + addr = ftrace_jmp_get(addr);
> + return text_gen_insn(JMP32_INSN_OPCODE, (void *)ip, (void *)addr);
> + } else {
> + return text_gen_insn(CALL_INSN_OPCODE, (void *)ip, (void *)addr);
> + }
> }
>
> static int ftrace_verify_code(unsigned long ip, const char *old_code)
> diff --git a/arch/x86/kernel/ftrace_64.S b/arch/x86/kernel/ftrace_64.S
> index 823dbdd0eb41..a132608265f6 100644
> --- a/arch/x86/kernel/ftrace_64.S
> +++ b/arch/x86/kernel/ftrace_64.S
> @@ -285,8 +285,18 @@ SYM_INNER_LABEL(ftrace_regs_caller_end, SYM_L_GLOBAL)
> ANNOTATE_NOENDBR
> RET
>
> +1:
> + testb $1, %al
> + jz 2f
> + andq $0xfffffffffffffffe, %rax
> + movq %rax, MCOUNT_REG_SIZE+8(%rsp)
> + restore_mcount_regs
> + /* Restore flags */
> + popfq
> + RET
is this hunk the reason for the 0x1 jmp-bit you set in the address?
I wonder if we introduced new flag in dyn_ftrace::flags for this,
then we'd need to have extra ftrace trampoline for jmp ftrace_ops
jirka
^ permalink raw reply
* Re: [POC][RFC][PATCH 1/3] tracing: Add perf events
From: Steven Rostedt @ 2025-11-18 20:24 UTC (permalink / raw)
To: Peter Zijlstra
Cc: Steven Rostedt, linux-kernel, linux-trace-kernel,
Masami Hiramatsu, Mark Rutland, Mathieu Desnoyers, Andrew Morton,
Thomas Gleixner, Ian Rogers, Namhyung Kim,
Arnaldo Carvalho de Melo, Jiri Olsa, Douglas Raillard
In-Reply-To: <20251118084226.1e531c4b@gandalf.local.home>
On Tue, 18 Nov 2025 08:42:26 -0500
Steven Rostedt <rostedt@goodmis.org> wrote:
> >
> > What you wanted to use was perf_event_read_local().
>
> Great! I didn't know about that. Which is why I posted this as a
> PROOF-OF-CONCEPT and not even a normal RFC, so that I could learn about the
> proper way of doing this.
I folded in this change:
diff --git a/kernel/trace/trace_event_perf.c b/kernel/trace/trace_event_perf.c
index ff864d300251..34962f80dce1 100644
--- a/kernel/trace/trace_event_perf.c
+++ b/kernel/trace/trace_event_perf.c
@@ -478,6 +478,7 @@ u64 do_trace_perf_event(int type)
struct trace_perf_event __percpu **pevents;
struct trace_perf_event __percpu *events;
struct perf_event *e;
+ u64 val;
int *count;
int cpu;
@@ -499,8 +500,10 @@ u64 do_trace_perf_event(int type)
if (!e)
return 0;
- e->pmu->read(e);
- return local64_read(&e->count);
+ if (perf_event_read_local(e, &val, NULL, NULL) < 0)
+ return 0;
+
+ return val;
}
static void __free_trace_perf_events(struct trace_perf_event __percpu *events)
Thanks!
-- Steve
^ permalink raw reply related
* Re: [PATCH v11 03/15] unwind_user/sframe: Add support for reading .sframe headers
From: Steven Rostedt @ 2025-11-18 19:26 UTC (permalink / raw)
To: Jens Remus
Cc: linux-kernel, linux-trace-kernel, bpf, x86, linux-mm,
Steven Rostedt, Josh Poimboeuf, Masami Hiramatsu,
Mathieu Desnoyers, Peter Zijlstra, Ingo Molnar, Jiri Olsa,
Arnaldo Carvalho de Melo, Namhyung Kim, Thomas Gleixner,
Andrii Nakryiko, Indu Bhagat, Jose E. Marchesi, Beau Belgrave,
Linus Torvalds, Andrew Morton, Florian Weimer, Kees Cook,
Carlos O'Donell, Sam James, Borislav Petkov, Dave Hansen,
David Hildenbrand, H. Peter Anvin, Liam R. Howlett,
Lorenzo Stoakes, Michal Hocko, Mike Rapoport, Suren Baghdasaryan,
Vlastimil Babka, Heiko Carstens, Vasily Gorbik
In-Reply-To: <e5c5e17f-1efd-4f9e-be2d-c6f65003ba3d@linux.ibm.com>
On Tue, 18 Nov 2025 18:04:27 +0100
Jens Remus <jremus@linux.ibm.com> wrote:
> I wonder whether the series should be restructured as follows:
>
> unwind_user/sframe: Store .sframe section data in per-mm maple tree
> unwind_user/sframe: Detect .sframe sections in executables
> unwind_user/sframe: Add support for reading .sframe headers
> unwind_user/sframe: Add support for reading .sframe contents
> unwind_user/sframe: Wire up unwind_user to sframe
> x86/uaccess: Add unsafe_copy_from_user() implementation
> unwind_user/sframe/x86: Enable sframe unwinding on x86
> unwind_user: Stop when reaching an outermost frame
> unwind_user/sframe: Add support for outermost frame indication
> unwind_user/sframe: Remove .sframe section on detected corruption
> unwind_user/sframe: Show file name in debug output
> unwind_user/sframe: Add .sframe validation option
> unwind_user/sframe: Add prctl() interface for registering .sframe sections
>
> While moving sframe_add_section() and sframe_remove_section() from
> "unwind_user/sframe: Add support for reading .sframe headers" to
> "unwind_user/sframe: Store .sframe section data in per-mm maple tree" or
> into a new second patch, as they depend on the first and are required
> by the third.
>
> What are your thoughts? The reordering might be wasted effort.
If you feel it makes it better, sure, go ahead and do it.
-- Steve
^ permalink raw reply
* RE: [PATCH v3 19/21] scsi: fnic: Switch to use %ptSp
From: Karan Tilak Kumar (kartilak) @ 2025-11-18 18:53 UTC (permalink / raw)
To: Andy Shevchenko
Cc: Steven Rostedt, Petr Mladek, linux-doc@vger.kernel.org,
linux-kernel@vger.kernel.org,
openipmi-developer@lists.sourceforge.net,
linux-media@vger.kernel.org, dri-devel@lists.freedesktop.org,
linaro-mm-sig@lists.linaro.org, amd-gfx@lists.freedesktop.org,
linux-arm-msm@vger.kernel.org, freedreno@lists.freedesktop.org,
intel-xe@lists.freedesktop.org, linux-mmc@vger.kernel.org,
netdev@vger.kernel.org, intel-wired-lan@lists.osuosl.org,
linux-pci@vger.kernel.org, linux-s390@vger.kernel.org,
linux-scsi@vger.kernel.org, linux-staging@lists.linux.dev,
ceph-devel@vger.kernel.org, linux-trace-kernel@vger.kernel.org,
Rasmus Villemoes, Sergey Senozhatsky, Satish Kharat (satishkh),
Sesidhar Baddela (sebaddel), James E.J. Bottomley, Andrew Morton
In-Reply-To: <aRbreoKzashQcEne@smile.fi.intel.com>
On Friday, November 14, 2025 12:43 AM, Andy Shevchenko <andriy.shevchenko@linux.intel.com> wrote:
>
> On Thu, Nov 13, 2025 at 10:34:36PM +0000, Karan Tilak Kumar (kartilak) wrote:
> > On Thursday, November 13, 2025 6:33 AM, Andy Shevchenko <andriy.shevchenko@linux.intel.com> wrote:
>
> ...
>
> > Can you please advise how I can compile test this change?
>
> I have added the following to my x86_64_defconfig
>
> CONFIG_SCSI_FC_ATTRS=m
> CONFIG_LIBFC=m
> CONFIG_LIBFCOE=m
> CONFIG_FCOE_FNIC=m
>
> You can always add the just a one (last) line to a configuration stanza that
> can be merged to the .config with help of merge_config tool. It will take care
> of all needed dependencies.
>
> --
> With Best Regards,
> Andy Shevchenko
>
Thank you Andy.
Regards,
Karan
^ permalink raw reply
* Re: [PATCH v3 14/21] net: dsa: sja1105: Switch to use %ptSp
From: Vladimir Oltean @ 2025-11-18 18:50 UTC (permalink / raw)
To: Andy Shevchenko
Cc: Corey Minyard, Christian König, Dr. David Alan Gilbert,
Alex Deucher, Thomas Zimmermann, Dmitry Baryshkov, Rob Clark,
Matthew Brost, Ulf Hansson, Aleksandr Loktionov, Vitaly Lifshits,
Manivannan Sadhasivam, Niklas Cassel, Calvin Owens,
Vadim Fedorenko, Sagi Maimon, Martin K. Petersen,
Karan Tilak Kumar, Hans Verkuil, Casey Schaufler, Steven Rostedt,
Petr Mladek, Viacheslav Dubeyko, Max Kellermann, linux-doc,
linux-kernel, openipmi-developer, linux-media, dri-devel,
linaro-mm-sig, amd-gfx, linux-arm-msm, freedreno, intel-xe,
linux-mmc, netdev, intel-wired-lan, linux-pci, linux-s390,
linux-scsi, linux-staging, ceph-devel, linux-trace-kernel,
Rasmus Villemoes, Sergey Senozhatsky, Jonathan Corbet,
Sumit Semwal, Gustavo Padovan, David Airlie, Simona Vetter,
Maarten Lankhorst, Maxime Ripard, Dmitry Baryshkov, Abhinav Kumar,
Jessica Zhang, Sean Paul, Marijn Suijten, Konrad Dybcio,
Lucas De Marchi, Thomas Hellström, Rodrigo Vivi, Andrew Lunn,
David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
Tony Nguyen, Przemek Kitszel, Krzysztof Wilczyński,
Kishon Vijay Abraham I, Bjorn Helgaas, Rodolfo Giometti,
Jonathan Lemon, Richard Cochran, Stefan Haberland, Jan Hoeppner,
Heiko Carstens, Vasily Gorbik, Alexander Gordeev,
Christian Borntraeger, Sven Schnelle, Satish Kharat,
Sesidhar Baddela, James E.J. Bottomley, Mauro Carvalho Chehab,
Greg Kroah-Hartman, Xiubo Li, Ilya Dryomov, Masami Hiramatsu,
Mathieu Desnoyers, Andrew Morton
In-Reply-To: <20251113150217.3030010-15-andriy.shevchenko@linux.intel.com>
Hi Andy,
On Thu, Nov 13, 2025 at 03:32:28PM +0100, Andy Shevchenko wrote:
> Use %ptSp instead of open coded variants to print content of
> struct timespec64 in human readable format.
>
> Reviewed-by: Aleksandr Loktionov <aleksandr.loktionov@intel.com>
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> ---
Acked-by: Vladimir Oltean <olteanv@gmail.com>
Tested-by: Vladimir Oltean <olteanv@gmail.com>
Thanks!
This is a rarely modified portion of the SJA1105 driver, and it doesn't
conflict with other changes that I have planned, so from my PoV there is
no problem with the patch being picked up via other trees.
^ permalink raw reply
* Re: [PATCH] tracing: Show the tracer options in boot-time created instance
From: Steven Rostedt @ 2025-11-18 18:34 UTC (permalink / raw)
To: Masami Hiramatsu (Google)
Cc: Mark Rutland, Mathieu Desnoyers, Andrew Morton, linux-kernel,
linux-trace-kernel
In-Reply-To: <176345219055.1127138.4292126981726580037.stgit@mhiramat.tok.corp.google.com>
On Tue, 18 Nov 2025 16:49:50 +0900
"Masami Hiramatsu (Google)" <mhiramat@kernel.org> wrote:
> From: Masami Hiramatsu (Google) <mhiramat@kernel.org>
>
> Since tracer_init_tracefs_work_func() only updates the tracer options
> for the global_trace, the instances created by the kernel cmdline
> do not have those options.
>
> Fix to update tracer options for those boot-time created instances
> to show those options.
>
> Signed-off-by: Masami Hiramatsu (Google) <mhiramat@kernel.org>
> Fixes: f20a580627f43 ("ftrace: Allow instances to use function tracing")
Actually, I think this should be:
Fixes: 428add559b69 ("tracing: Have tracer option be instance specific")
As it broke when we made function tracer options no longer global.
> ---
> kernel/trace/trace.c | 4 +++-
> 1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/kernel/trace/trace.c b/kernel/trace/trace.c
> index 8ae95800592d..2e87060e1d5a 100644
> --- a/kernel/trace/trace.c
> +++ b/kernel/trace/trace.c
> @@ -10903,6 +10903,7 @@ static struct notifier_block trace_module_nb = {
>
> static __init void tracer_init_tracefs_work_func(struct work_struct *work)
> {
> + struct trace_array *tr;
>
> event_trace_init();
>
> @@ -10937,7 +10938,8 @@ static __init void tracer_init_tracefs_work_func(struct work_struct *work)
>
> create_trace_instances(NULL);
>
> - update_tracer_options(&global_trace);
> + list_for_each_entry(tr, &ftrace_trace_arrays, list)
> + update_tracer_options(tr);
> }
>
> static __init int tracer_init_tracefs(void)
Since update_trace_options() is only called by this function, instead of
doing the loop here, change the function to be:
static __init void update_tracer_options(void)
{
struct trace_array *tr;
guard(mutex)(&trace_types_lock);
tracer_options_updated = true;
list_for_each_entry(tr, &ftrace_trace_arrays, list)
__update_tracer_options(tr);
}
Thanks,
-- Steve
^ permalink raw reply
* Re: [PATCH v4 0/9] introduce VM_MAYBE_GUARD and make it sticky
From: Andrew Morton @ 2025-11-18 17:33 UTC (permalink / raw)
To: Lorenzo Stoakes
Cc: Jonathan Corbet, David Hildenbrand, Liam R . Howlett,
Vlastimil Babka, Mike Rapoport, Suren Baghdasaryan, Michal Hocko,
Steven Rostedt, Masami Hiramatsu, Mathieu Desnoyers, Jann Horn,
Pedro Falcato, Zi Yan, Baolin Wang, Nico Pache, Ryan Roberts,
Dev Jain, Barry Song, Lance Yang, linux-kernel, linux-fsdevel,
linux-doc, linux-mm, linux-trace-kernel, linux-kselftest,
Andrei Vagin
In-Reply-To: <cover.1763460113.git.lorenzo.stoakes@oracle.com>
On Tue, 18 Nov 2025 10:17:42 +0000 Lorenzo Stoakes <lorenzo.stoakes@oracle.com> wrote:
> Currently, guard regions are not visible to users except through
> /proc/$pid/pagemap, with no explicit visibility at the VMA level.
>
> This makes the feature less useful, as it isn't entirely apparent which
> VMAs may have these entries present, especially when performing actions
> which walk through memory regions such as those performed by CRIU.
>
> This series addresses this issue by introducing the VM_MAYBE_GUARD flag
> which fulfils this role, updating the smaps logic to display an entry for
> these.
I updated mm-unstable to this version.
^ permalink raw reply
* Re: [RFC PATCH 2/8] mm: Add PG_atomic
From: Ritesh Harjani @ 2025-11-18 16:17 UTC (permalink / raw)
To: Matthew Wilcox
Cc: Ojaswin Mujoo, Christian Brauner, djwong, john.g.garry, tytso,
dchinner, hch, linux-xfs, linux-kernel, linux-ext4, linux-fsdevel,
linux-mm, jack, nilay, martin.petersen, rostedt, axboe,
linux-block, linux-trace-kernel
In-Reply-To: <aRcrwgxV6cBu2_RH@casper.infradead.org>
Matthew Wilcox <willy@infradead.org> writes:
> On Fri, Nov 14, 2025 at 10:30:09AM +0530, Ritesh Harjani wrote:
>> Matthew Wilcox <willy@infradead.org> writes:
>>
>> > On Wed, Nov 12, 2025 at 04:36:05PM +0530, Ojaswin Mujoo wrote:
>> >> From: John Garry <john.g.garry@oracle.com>
>> >>
>> >> Add page flag PG_atomic, meaning that a folio needs to be written back
>> >> atomically. This will be used by for handling RWF_ATOMIC buffered IO
>> >> in upcoming patches.
>> >
>> > Page flags are a precious resource. I'm not thrilled about allocating one
>> > to this rather niche usecase. Wouldn't this be more aptly a flag on the
>> > address_space rather than the folio? ie if we're doing this kind of write
>> > to a file, aren't most/all of the writes to the file going to be atomic?
>>
>> As of today the atomic writes functionality works on the per-write
>> basis (given it's a per-write characteristic).
>>
>> So, we can have two types of dirty folios sitting in the page cache of
>> an inode. Ones which were done using atomic buffered I/O flag
>> (RWF_ATOMIC) and the other ones which were non-atomic writes. Hence a
>> need of a folio flag to distinguish between the two writes.
>
> I know, but is this useful? AFAIK, the files where Postgres wants to
> use this functionality are the log files, and all writes to the log
> files will want to use the atomic functionality. What's the usecase
> for "I want to mix atomic and non-atomic buffered writes to this file"?
Actually this goes back to the design of how we added support of atomic
writes during DIO. So during the initial design phase we decided that
this need not be a per-inode attribute or an open flag, but this is a
per write I/O characteristic.
So as per the current design, we don't have any open flag or a
persistent inode attribute which says kernel should permit _only_ atomic
writes I/O to this file. Instead what we support today is DIO atomic
writes using RWF_ATOMIC flag in pwritev2 syscall.
Having said that there can be several policy decision that could still be
discussed e.g. make sure any previous dirty data is flushed to disk when a
buffered atomic write request is made to an inode.
Maybe that would allow us to just keep a flag at the address space level
because we would never have a mix of atomic and non-atomic page cache
pages.
IMO, I agree that folio flag is a scarce resource, but I guess the
initial goal of this patch series is mainly to discuss the initial
design of the core feature i.e. how buffered atomic writes should look
in Linux kernel. I agree and point taken that we should be careful with
using folio flags, but let's see how the design shapes up maybe? - that
will help us understand whether a folio flag is really required or maybe
an address space flag would do.
-ritesh
^ permalink raw reply
* Re: [PATCH v11 03/15] unwind_user/sframe: Add support for reading .sframe headers
From: Jens Remus @ 2025-11-18 17:04 UTC (permalink / raw)
To: linux-kernel, linux-trace-kernel, bpf, x86, linux-mm,
Steven Rostedt, Josh Poimboeuf
Cc: Masami Hiramatsu, Mathieu Desnoyers, Peter Zijlstra, Ingo Molnar,
Jiri Olsa, Arnaldo Carvalho de Melo, Namhyung Kim,
Thomas Gleixner, Andrii Nakryiko, Indu Bhagat, Jose E. Marchesi,
Beau Belgrave, Linus Torvalds, Andrew Morton, Florian Weimer,
Kees Cook, Carlos O'Donell, Sam James, Borislav Petkov,
Dave Hansen, David Hildenbrand, H. Peter Anvin, Liam R. Howlett,
Lorenzo Stoakes, Michal Hocko, Mike Rapoport, Suren Baghdasaryan,
Vlastimil Babka, Heiko Carstens, Vasily Gorbik,
Steven Rostedt (Google)
In-Reply-To: <20251022144326.4082059-4-jremus@linux.ibm.com>
Hello Josh and Steven!
On 10/22/2025 4:43 PM, Jens Remus wrote:
> From: Josh Poimboeuf <jpoimboe@kernel.org>
>
> In preparation for unwinding user space stacks with sframe, add basic
> sframe compile infrastructure and support for reading the .sframe
> section header.
>
> sframe_add_section() reads the header and unconditionally returns an
> error, so it's not very useful yet. A subsequent patch will improve
> that.
>
> Link: https://lore.kernel.org/all/f27e8463783febfa0dabb0432a3dd6be8ad98412.1737511963.git.jpoimboe@kernel.org/
>
> [ Jens Remus: Add support for PC-relative FDE function start address. ]
I took a closer look and wondered whether some parts should better be
moved to subsequent patches.
> diff --git a/include/linux/sframe.h b/include/linux/sframe.h
> @@ -0,0 +1,40 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +#ifndef _LINUX_SFRAME_H
> +#define _LINUX_SFRAME_H
> +
> +#include <linux/mm_types.h>
Move to "[PATCH v11 04/15] unwind_user/sframe: Store sframe section data
in per-mm maple tree".
> +#include <linux/unwind_user_types.h>
Move to "[PATCH v11 06/15] unwind_user/sframe: Add support for reading
.sframe contents". find_sframe() needs the types.
> +
> +#ifdef CONFIG_HAVE_UNWIND_USER_SFRAME
> +
> +struct sframe_section {
> + unsigned long sframe_start;
> + unsigned long sframe_end;
> + unsigned long text_start;
> + unsigned long text_end;
> +
> + unsigned long fdes_start;
> + unsigned long fres_start;
> + unsigned long fres_end;
> + unsigned int num_fdes;
> +
> + signed char ra_off;
> + signed char fp_off;
> +};
> +
> +extern int sframe_add_section(unsigned long sframe_start, unsigned long sframe_end,
> + unsigned long text_start, unsigned long text_end);
> +extern int sframe_remove_section(unsigned long sframe_addr);
> +
> +#else /* !CONFIG_HAVE_UNWIND_USER_SFRAME */
> +
> +static inline int sframe_add_section(unsigned long sframe_start, unsigned long sframe_end,
> + unsigned long text_start, unsigned long text_end)
> +{
> + return -ENOSYS;
> +}
> +static inline int sframe_remove_section(unsigned long sframe_addr) { return -ENOSYS; }
> +
> +#endif /* CONFIG_HAVE_UNWIND_USER_SFRAME */
> +
> +#endif /* _LINUX_SFRAME_H */
> diff --git a/kernel/unwind/sframe.c b/kernel/unwind/sframe.c
> @@ -0,0 +1,137 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Userspace sframe access functions
> + */
> +
> +#define pr_fmt(fmt) "sframe: " fmt
> +
> +#include <linux/sched.h>
> +#include <linux/slab.h>
> +#include <linux/srcu.h>
> +#include <linux/uaccess.h>
> +#include <linux/mm.h>
Move to "[PATCH v11 04/15] unwind_user/sframe: Store sframe section data
in per-mm maple tree".
> +#include <linux/string_helpers.h>
> +#include <linux/sframe.h>
> +#include <linux/unwind_user_types.h>
> +
> +#include "sframe.h"
> +
> +#define dbg(fmt, ...) \
> + pr_debug("%s (%d): " fmt, current->comm, current->pid, ##__VA_ARGS__)
> +
> +static void free_section(struct sframe_section *sec)
> +{
> + kfree(sec);
> +}
> +
> +static int sframe_read_header(struct sframe_section *sec)
> +{
> + unsigned long header_end, fdes_start, fdes_end, fres_start, fres_end;
> + struct sframe_header shdr;
> + unsigned int num_fdes;
> +
> + if (copy_from_user(&shdr, (void __user *)sec->sframe_start, sizeof(shdr))) {
> + dbg("header usercopy failed\n");
> + return -EFAULT;
> + }
> +
> + if (shdr.preamble.magic != SFRAME_MAGIC ||
> + shdr.preamble.version != SFRAME_VERSION_2 ||
> + !(shdr.preamble.flags & SFRAME_F_FDE_SORTED) ||
> + !(shdr.preamble.flags & SFRAME_F_FDE_FUNC_START_PCREL) ||
> + shdr.auxhdr_len) {
> + dbg("bad/unsupported sframe header\n");
> + return -EINVAL;
> + }
> +
> + if (!shdr.num_fdes || !shdr.num_fres) {
> + dbg("no fde/fre entries\n");
> + return -EINVAL;
> + }
> +
> + header_end = sec->sframe_start + SFRAME_HEADER_SIZE(shdr);
> + if (header_end >= sec->sframe_end) {
> + dbg("header doesn't fit in section\n");
> + return -EINVAL;
> + }
> +
> + num_fdes = shdr.num_fdes;
> + fdes_start = header_end + shdr.fdes_off;
> + fdes_end = fdes_start + (num_fdes * sizeof(struct sframe_fde));
> +
> + fres_start = header_end + shdr.fres_off;
> + fres_end = fres_start + shdr.fre_len;
> +
> + if (fres_start < fdes_end || fres_end > sec->sframe_end) {
> + dbg("inconsistent fde/fre offsets\n");
> + return -EINVAL;
> + }
> +
> + sec->num_fdes = num_fdes;
> + sec->fdes_start = fdes_start;
> + sec->fres_start = fres_start;
> + sec->fres_end = fres_end;
> +
> + sec->ra_off = shdr.cfa_fixed_ra_offset;
> + sec->fp_off = shdr.cfa_fixed_fp_offset;
> +
> + return 0;
> +}
> +
> +int sframe_add_section(unsigned long sframe_start, unsigned long sframe_end,
> + unsigned long text_start, unsigned long text_end)
> +{
> + struct maple_tree *sframe_mt = ¤t->mm->sframe_mt;
Move to "[PATCH v11 04/15] unwind_user/sframe: Store sframe section data
in per-mm maple tree".
> + struct vm_area_struct *sframe_vma, *text_vma;
> + struct mm_struct *mm = current->mm;
> + struct sframe_section *sec;
> + int ret;
> +
> + if (!sframe_start || !sframe_end || !text_start || !text_end) {
> + dbg("zero-length sframe/text address\n");
> + return -EINVAL;
> + }
> +
> + scoped_guard(mmap_read_lock, mm) {
> + sframe_vma = vma_lookup(mm, sframe_start);
> + if (!sframe_vma || sframe_end > sframe_vma->vm_end) {
> + dbg("bad sframe address (0x%lx - 0x%lx)\n",
> + sframe_start, sframe_end);
> + return -EINVAL;
> + }
> +
> + text_vma = vma_lookup(mm, text_start);
> + if (!text_vma ||
> + !(text_vma->vm_flags & VM_EXEC) ||
> + text_end > text_vma->vm_end) {
> + dbg("bad text address (0x%lx - 0x%lx)\n",
> + text_start, text_end);
> + return -EINVAL;
> + }
> + }
> +
> + sec = kzalloc(sizeof(*sec), GFP_KERNEL);
> + if (!sec)
> + return -ENOMEM;
> +
> + sec->sframe_start = sframe_start;
> + sec->sframe_end = sframe_end;
> + sec->text_start = text_start;
> + sec->text_end = text_end;
> +
> + ret = sframe_read_header(sec);
> + if (ret)
> + goto err_free;
> +
> + /* TODO nowhere to store it yet - just free it and return an error */
An alternative would be to move sframe_add_section() to
"[PATCH v11 04/15] unwind_user/sframe: Store sframe section data in
per-mm maple tree" and reorder the patches as outlined below.
> + ret = -ENOSYS;
> +
> +err_free:
> + free_section(sec);
> + return ret;
> +}
> +
> +int sframe_remove_section(unsigned long sframe_start)
> +{
> + return -ENOSYS;
> +}
I wonder whether the series should be restructured as follows:
unwind_user/sframe: Store .sframe section data in per-mm maple tree
unwind_user/sframe: Detect .sframe sections in executables
unwind_user/sframe: Add support for reading .sframe headers
unwind_user/sframe: Add support for reading .sframe contents
unwind_user/sframe: Wire up unwind_user to sframe
x86/uaccess: Add unsafe_copy_from_user() implementation
unwind_user/sframe/x86: Enable sframe unwinding on x86
unwind_user: Stop when reaching an outermost frame
unwind_user/sframe: Add support for outermost frame indication
unwind_user/sframe: Remove .sframe section on detected corruption
unwind_user/sframe: Show file name in debug output
unwind_user/sframe: Add .sframe validation option
unwind_user/sframe: Add prctl() interface for registering .sframe sections
While moving sframe_add_section() and sframe_remove_section() from
"unwind_user/sframe: Add support for reading .sframe headers" to
"unwind_user/sframe: Store .sframe section data in per-mm maple tree" or
into a new second patch, as they depend on the first and are required
by the third.
What are your thoughts? The reordering might be wasted effort.
Thanks and regards,
Jens
--
Jens Remus
Linux on Z Development (D3303)
+49-7031-16-1128 Office
jremus@de.ibm.com
IBM
IBM Deutschland Research & Development GmbH; Vorsitzender des Aufsichtsrats: Wolfgang Wendt; Geschäftsführung: David Faller; Sitz der Gesellschaft: Böblingen; Registergericht: Amtsgericht Stuttgart, HRB 243294
IBM Data Privacy Statement: https://www.ibm.com/privacy/
^ permalink raw reply
* Re: [POC][RFC][PATCH 0/3] tracing: Add perf events to trace buffer
From: Steven Rostedt @ 2025-11-18 16:31 UTC (permalink / raw)
To: Masami Hiramatsu (Google)
Cc: Steven Rostedt, linux-kernel, linux-trace-kernel, Mark Rutland,
Mathieu Desnoyers, Andrew Morton, Peter Zijlstra, Thomas Gleixner,
Ian Rogers, Namhyung Kim, Arnaldo Carvalho de Melo, Jiri Olsa,
Douglas Raillard
In-Reply-To: <20251118171147.72d4f1e26407f8d54720e0f2@kernel.org>
On Tue, 18 Nov 2025 17:11:47 +0900
Masami Hiramatsu (Google) <mhiramat@kernel.org> wrote:
> > > If we need to set those counters for tracers and events separately,
> > > we can add `events/trigger` and `tracer-trigger` files.
> >
> > As I mentioned, the trigger for events should be in the events directory.
>
> Agreed.
>
> >
> > We could add a ftrace_trigger that can affect both function and
> > function graph tracer.
>
Actually, I should add "trigger" files in the ftrace events:
events/ftrace/function/trigger
events/ftrace/funcgraph_entry/tigger
events/ftrace/funcgraph_exit/tigger
Hmm,
-- Steve
^ permalink raw reply
* Re: [POC][RFC][PATCH 0/3] tracing: Add perf events to trace buffer
From: Steven Rostedt @ 2025-11-18 16:24 UTC (permalink / raw)
To: Namhyung Kim
Cc: Steven Rostedt, linux-kernel, linux-trace-kernel,
Masami Hiramatsu, Mark Rutland, Mathieu Desnoyers, Andrew Morton,
Peter Zijlstra, Thomas Gleixner, Ian Rogers,
Arnaldo Carvalho de Melo, Jiri Olsa, Douglas Raillard
In-Reply-To: <aRwfhIT4pJ0pbY2k@google.com>
On Mon, 17 Nov 2025 23:25:56 -0800
Namhyung Kim <namhyung@kernel.org> wrote:
> > As for the perf event that is triggered. It currently is a dynamic array of
> > 64 bit values. Each value is broken up into 8 bits for what type of perf
> > event it is, and 56 bits for the counter. It only writes a per CPU raw
> > counter and does not do any math. That would be needed to be done by any
> > post processing.
>
> If you want to keep the perf events per CPU, you may consider CPU
> migrations for the func-graph case. Otherwise userspace may not
> calculate the diff from the begining correctly.
That's easily solved by the user space too adding a sched_switch perf event
trigger. ;-)
>
> Just FYI, I did the similar thing (like fgraph case) in uftrace and I
> grouped two related events to produce a metric.
>
> $ uftrace -T a@read=pmu-cycle ~/tmp/abc
> # DURATION TID FUNCTION
> [ 521741] | main() {
> [ 521741] | a() {
> [ 521741] | /* read:pmu-cycle (cycles=482 instructions=38) */
> [ 521741] | b() {
> [ 521741] | c() {
> 0.659 us [ 521741] | getpid();
> 1.600 us [ 521741] | } /* c */
> 1.780 us [ 521741] | } /* b */
> [ 521741] | /* diff:pmu-cycle (cycles=+7361 instructions=+3955 IPC=0.54) */
> 24.485 us [ 521741] | } /* a */
> 34.797 us [ 521741] | } /* main */
>
> It reads cycles and instructions events (specified by 'pmu-cycle') at
> entry and exit of the given function ('a') and shows the diff with the
> metric IPC.
I originally tried to implement this, but then it became more complex than
I wanted in the kernel. As then I need to add a hook in the sched_switch
and record the perf event counter there, and keep track of it for every
task. That would require memory to be saved somewhere. I started adding it
to the function graph shadow stack and then just decided that it would be
so much easier to let user space figure it out.
By running function graph tracer and showing the start and end counters, as
well as the counters at the sched_switch trace event, user space could do
all the math and accounting, and the code in the kernel can remain simple.
-- Steve
^ permalink raw reply
* Re: [PATCH v3 18/21] s390/dasd: Switch to use %ptSp
From: Stefan Haberland @ 2025-11-18 15:23 UTC (permalink / raw)
To: Andy Shevchenko, Corey Minyard, Christian König,
Dr. David Alan Gilbert, Alex Deucher, Thomas Zimmermann,
Dmitry Baryshkov, Rob Clark, Matthew Brost, Ulf Hansson,
Aleksandr Loktionov, Vitaly Lifshits, Manivannan Sadhasivam,
Niklas Cassel, Calvin Owens, Vadim Fedorenko, Sagi Maimon,
Martin K. Petersen, Karan Tilak Kumar, Hans Verkuil,
Casey Schaufler, Steven Rostedt, Petr Mladek, Viacheslav Dubeyko,
Max Kellermann, linux-doc, linux-kernel, openipmi-developer,
linux-media, dri-devel, linaro-mm-sig, amd-gfx, linux-arm-msm,
freedreno, intel-xe, linux-mmc, netdev, intel-wired-lan,
linux-pci, linux-s390, linux-scsi, linux-staging, ceph-devel,
linux-trace-kernel
Cc: Rasmus Villemoes, Sergey Senozhatsky, Jonathan Corbet,
Sumit Semwal, Gustavo Padovan, David Airlie, Simona Vetter,
Maarten Lankhorst, Maxime Ripard, Dmitry Baryshkov, Abhinav Kumar,
Jessica Zhang, Sean Paul, Marijn Suijten, Konrad Dybcio,
Lucas De Marchi, Thomas Hellström, Rodrigo Vivi,
Vladimir Oltean, Andrew Lunn, David S. Miller, Eric Dumazet,
Jakub Kicinski, Paolo Abeni, Tony Nguyen, Przemek Kitszel,
Krzysztof Wilczyński, Kishon Vijay Abraham I, Bjorn Helgaas,
Rodolfo Giometti, Jonathan Lemon, Richard Cochran, Jan Hoeppner,
Heiko Carstens, Vasily Gorbik, Alexander Gordeev,
Christian Borntraeger, Sven Schnelle, Satish Kharat,
Sesidhar Baddela, James E.J. Bottomley, Mauro Carvalho Chehab,
Greg Kroah-Hartman, Xiubo Li, Ilya Dryomov, Masami Hiramatsu,
Mathieu Desnoyers, Andrew Morton
In-Reply-To: <20251113150217.3030010-19-andriy.shevchenko@linux.intel.com>
Am 13.11.25 um 15:32 schrieb Andy Shevchenko:
> Use %ptSp instead of open coded variants to print content of
> struct timespec64 in human readable format.
>
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> ---
Thanks, looks good to me.
Acked-by: Stefan Haberland <sth@linux.ibm.com>
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox