On Tue, 23 Jan 2024 11:07:54 +0000
Vincent Donnefort <vdonnef...@google.com> wrote:
[...]
> @@ -6592,8 +6641,11 @@ int tracing_set_tracer(struct trace_array *tr, const
> char *buf)
>
> if (t->init) {
> ret = tracer_init(t, tr);
> - if (ret)
> + if (ret) {
> + if (t->use_max_tr)
> + tracing_disarm_snapshot_locked(tr);
This part is out of CONFIG_TRACER_MAX_TRACE, so it may cause a compile error
if CONFIG_TRACER_MAX_TRACE is not set.
> goto out;
> + }
> }
>
> tr->current_trace = t;
[...]
> diff --git a/kernel/trace/trace_events_trigger.c
> b/kernel/trace/trace_events_trigger.c
> index 46439e3bcec4..d41bf64741e2 100644
> --- a/kernel/trace/trace_events_trigger.c
> +++ b/kernel/trace/trace_events_trigger.c
> @@ -597,20 +597,9 @@ static int register_trigger(char *glob,
> return ret;
> }
>
> -/**
> - * unregister_trigger - Generic event_command @unreg implementation
> - * @glob: The raw string used to register the trigger
> - * @test: Trigger-specific data used to find the trigger to remove
> - * @file: The trace_event_file associated with the event
> - *
> - * Common implementation for event trigger unregistration.
> - *
> - * Usually used directly as the @unreg method in event command
> - * implementations.
> - */
> -static void unregister_trigger(char *glob,
> - struct event_trigger_data *test,
> - struct trace_event_file *file)
OK, so __unregister_trigger returns true if data exists, but
unregister_trigger() ignores results. (I want some comment here)
> +static bool __unregister_trigger(char *glob,
> + struct event_trigger_data *test,
> + struct trace_event_file *file)
> {
> struct event_trigger_data *data = NULL, *iter;
>
> @@ -626,8 +615,32 @@ static void unregister_trigger(char *glob,
> }
> }
>
> - if (data && data->ops->free)
> - data->ops->free(data);
> + if (data) {
> + if (data->ops->free)
> + data->ops->free(data);
> +
> + return true;
> + }
> +
> + return false;
> +}
> +
> +/**
> + * unregister_trigger - Generic event_command @unreg implementation
> + * @glob: The raw string used to register the trigger
> + * @test: Trigger-specific data used to find the trigger to remove
> + * @file: The trace_event_file associated with the event
> + *
> + * Common implementation for event trigger unregistration.
> + *
> + * Usually used directly as the @unreg method in event command
> + * implementations.
> + */
> +static void unregister_trigger(char *glob,
> + struct event_trigger_data *test,
> + struct trace_event_file *file)
> +{
> + __unregister_trigger(glob, test, file);
> }
>
> /*
> @@ -1470,12 +1483,20 @@ register_snapshot_trigger(char *glob,
> struct event_trigger_data *data,
> struct trace_event_file *file)
> {
> - if (tracing_alloc_snapshot_instance(file->tr) != 0)
> + if (tracing_arm_snapshot(file->tr))
> return 0;
BTW, is this return value correct? It seems that the register_*_trigger()
will return error code when it fails.
Thanks,
>
> return register_trigger(glob, data, file);
> }
>
> +static void unregister_snapshot_trigger(char *glob,
> + struct event_trigger_data *data,
> + struct trace_event_file *file)
> +{
> + if (__unregister_trigger(glob, data, file))
> + tracing_disarm_snapshot(file->tr);
> +}
> +
> static int
> snapshot_trigger_print(struct seq_file *m, struct event_trigger_data *data)
> {
> @@ -1508,7 +1529,7 @@ static struct event_command trigger_snapshot_cmd = {
> .trigger_type = ETT_SNAPSHOT,
> .parse = event_trigger_parse,
> .reg = register_snapshot_trigger,
> - .unreg = unregister_trigger,
> + .unreg = unregister_snapshot_trigger,
> .get_trigger_ops = snapshot_get_trigger_ops,
> .set_filter = set_trigger_filter,
> };
> --
> 2.43.0.429.g432eaa2c6b-goog
>
--
Masami Hiramatsu (Google) <mhira...@kernel.org>
