Re: [PATCH 5/9] perf sched: Clean up idle_threads entry on init failure

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:

Pre-existing issues:
- [High] The perf sched command fails to register the `.feature` callback in its `perf_tool` initialization, which prevents `perf_env` from being populated in pipe mode.
- [Medium] free_idle_threads() leaks the dynamically allocated nodes of the callchain root attached to the idle thread's private data.
--

commit 22f7015aaa31f156b54bc85f9ec298a36781e9ba
Author: Arnaldo Carvalho de Melo <acme@redhat.com>

perf sched: Clean up idle_threads entry on init failure

get_idle_thread() allocates a thread via thread__new() and stores it in
idle_threads[cpu], then calls init_idle_thread() to set up the private
data.  If init_idle_thread() fails (e.g. OOM for the idle_thread_runtime
struct), the function returns NULL but leaves the partially initialized
thread in idle_threads[cpu].

> diff --git a/tools/perf/builtin-sched.c b/tools/perf/builtin-sched.c
> index 21fb820b625b4..e4378cc9ab3ed 100644
> --- a/tools/perf/builtin-sched.c
> +++ b/tools/perf/builtin-sched.c
> @@ -2514,8 +2514,11 @@ static struct thread *get_idle_thread(int cpu)
>  		idle_threads[cpu] = thread__new(0, 0);

[Severity: Medium]
This is a pre-existing issue, but does free_idle_threads() leak the
dynamically allocated nodes of the callchain root attached to the idle
thread's private data?

During tool execution, callchain_append() dynamically allocates struct
callchain_node objects and attaches them to itr->callchain (inside struct
idle_thread_runtime).

When free_idle_threads() runs at exit:

tools/perf/builtin-sched.c:free_idle_threads() {
    ...
            itr = thread__priv(idle);
            if (itr)
                thread__put(itr->last_thread);

            thread__put(idle);
    ...
}

Because cmd_sched() registers the generic free() function as the private
data destructor (thread__set_priv_destructor(free)), thread__put(idle)
performs a shallow free on itr.

The function free_callchain(&itr->callchain) is never called, resulting in
a leak of all appended callchain nodes at tool exit.

>  
>  		if (idle_threads[cpu]) {
> -			if (init_idle_thread(idle_threads[cpu]) < 0)
> +			if (init_idle_thread(idle_threads[cpu]) < 0) {
> +				/* clean up so next call doesn't find a half-initialized thread */
> +				thread__zput(idle_threads[cpu]);
>  				return NULL;
> +			}
>  		}
>  	}
>  

[Severity: High]
This is a pre-existing issue, but does the perf sched command fail to
set up the .feature callback in its perf_tool initialization, preventing
perf_env from being populated in pipe mode?

In pipe mode, perf_env is populated via synthesized feature events from the
stream.  Because perf_sched__timehist (via cmd_sched) assigns callbacks like
.mmap, but completely omits .feature:

tools/perf/builtin-sched.c:perf_sched__timehist() {
    ...
        sched->tool.sample       = perf_timehist__process_sample;
        sched->tool.mmap         = perf_event__process_mmap;
        /* missing sched->tool.feature = perf_event__process_feature; */
    ...
}

Are feature events routed to a default stub and dropped?  This would prevent
proper initialization of tool structures (e.g., env->nr_cpus_online defaults
to 4 rather than the actual topology), breaking analysis functionality.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260605233837.1773732-1-acme@kernel.org?part=5