On Tue, 10 Mar 2026 00:47:15 -0600
Wesley Atwell <[email protected]> wrote:
> Boot-time trigger registration can fail before the trigger-data cleanup
> kthread exists. Deferring those frees until late init is fine, but the
> post-boot fallback must still drain the deferred list if kthread
> creation never succeeds.
>
> Otherwise, boot-deferred nodes can accumulate on
> trigger_data_free_list, later frees fall back to synchronously freeing
> only the current object, and the older queued entries are leaked
> forever.
>
> Keep the deferred boot-time behavior, but when kthread creation fails,
> drain the whole queued list synchronously. Do the same in the late-init
> drain path so queued entries are not stranded there either.
>
> Fixes: 61d445af0a7c ("tracing: Add bulk garbage collection of freeing
> event_trigger_data")
> Signed-off-by: Wesley Atwell <[email protected]>
> ---
> kernel/trace/trace_events_trigger.c | 79 ++++++++++++++++++++++++-----
> 1 file changed, 66 insertions(+), 13 deletions(-)
>
> diff --git a/kernel/trace/trace_events_trigger.c
> b/kernel/trace/trace_events_trigger.c
> index d5230b759a2d..428b46272ac8 100644
> --- a/kernel/trace/trace_events_trigger.c
> +++ b/kernel/trace/trace_events_trigger.c
> @@ -22,6 +22,39 @@ static struct task_struct *trigger_kthread;
> static struct llist_head trigger_data_free_list;
> static DEFINE_MUTEX(trigger_data_kthread_mutex);
>
> +static int trigger_kthread_fn(void *ignore);
> +
> +static void trigger_start_kthread_locked(void)
> +{
> + lockdep_assert_held(&trigger_data_kthread_mutex);
> +
> + if (!trigger_kthread) {
> + struct task_struct *kthread;
> +
> + kthread = kthread_create(trigger_kthread_fn, NULL,
> + "trigger_data_free");
This only creates the thread and doesn't start it. The function name is
confusing. Please change it to:
trigger_create_kthread_locked()
> + if (!IS_ERR(kthread))
> + WRITE_ONCE(trigger_kthread, kthread);
> + }
> +}
> +
> +static void trigger_data_free_queued_locked(void)
> +{
> + struct event_trigger_data *data, *tmp;
> + struct llist_node *llnodes;
> +
> + lockdep_assert_held(&trigger_data_kthread_mutex);
> +
> + llnodes = llist_del_all(&trigger_data_free_list);
> + if (!llnodes)
> + return;
> +
> + tracepoint_synchronize_unregister();
> +
> + llist_for_each_entry_safe(data, tmp, llnodes, llist)
> + kfree(data);
> +}
> +
> /* Bulk garbage collection of event_trigger_data elements */
> static int trigger_kthread_fn(void *ignore)
> {
> @@ -56,30 +89,50 @@ void trigger_data_free(struct event_trigger_data *data)
> if (data->cmd_ops->set_filter)
> data->cmd_ops->set_filter(NULL, data, NULL);
>
> + /*
> + * Boot-time trigger registration can fail before kthread creation
> + * works. Keep the deferred-free semantics during boot and let late
> + * init start the kthread to drain the list.
> + */
> + if (system_state == SYSTEM_BOOTING && !trigger_kthread) {
> + llist_add(&data->llist, &trigger_data_free_list);
> + return;
> + }
> +
> if (unlikely(!trigger_kthread)) {
> guard(mutex)(&trigger_data_kthread_mutex);
> +
> + trigger_start_kthread_locked();
> /* Check again after taking mutex */
> if (!trigger_kthread) {
> - struct task_struct *kthread;
> -
> - kthread = kthread_create(trigger_kthread_fn, NULL,
> - "trigger_data_free");
> - if (!IS_ERR(kthread))
> - WRITE_ONCE(trigger_kthread, kthread);
> + llist_add(&data->llist, &trigger_data_free_list);
> + /* Drain the queued frees synchronously if startup
> failed. */
s/startup/creation/
> + trigger_data_free_queued_locked();
> + return;
> }
> }
-- Steve
>
> - if (!trigger_kthread) {
> - /* Do it the slow way */
> - tracepoint_synchronize_unregister();
> - kfree(data);
> - return;
> - }
> -
> llist_add(&data->llist, &trigger_data_free_list);
> wake_up_process(trigger_kthread);
> }
>
> +static int __init trigger_data_free_init(void)
> +{
> + guard(mutex)(&trigger_data_kthread_mutex);
> +
> + if (llist_empty(&trigger_data_free_list))
> + return 0;
> +
> + trigger_start_kthread_locked();
> + if (trigger_kthread)
> + wake_up_process(trigger_kthread);
> + else
> + trigger_data_free_queued_locked();
> +
> + return 0;
> +}
> +late_initcall(trigger_data_free_init);
> +
> static inline void data_ops_trigger(struct event_trigger_data *data,
> struct trace_buffer *buffer, void *rec,
> struct ring_buffer_event *event)
