All of lore.kernel.org
 help / color / mirror / Atom feed
From: sashiko-bot@kernel.org
To: "Arnaldo Carvalho de Melo" <acme@kernel.org>
Cc: linux-perf-users@vger.kernel.org
Subject: Re: [PATCH 7/8] perf sched: Use thread__put() in free_idle_threads()
Date: Fri, 05 Jun 2026 20:49:27 +0000	[thread overview]
Message-ID: <20260605204928.0BD8B1F00893@smtp.kernel.org> (raw)
In-Reply-To: <20260605203316.1758661-8-acme@kernel.org>

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Replacing thread__delete() with thread__put() fails to free idle threads when --idle-hist is enabled.
--

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

perf sched: Use thread__put() in free_idle_threads()

This commit changes the cleanup of idle threads from thread__delete() to
thread__put() to correctly balance reference counts. It aims to avoid
leaking pointer handles in REFCNT_CHECKING builds by routing the object
destruction through the reference count wrapper.

> diff --git a/tools/perf/builtin-sched.c b/tools/perf/builtin-sched.c
> index e2df69a624765..b5a7a944f5c8c 100644
> --- a/tools/perf/builtin-sched.c
> +++ b/tools/perf/builtin-sched.c
> @@ -2442,7 +2442,7 @@ static void free_idle_threads(void)
>  			if (itr)
>  				thread__put(itr->last_thread);
>  
> -			thread__delete(idle);
> +			thread__put(idle);

[Severity: High]
Does this replace an intentional forceful deletion with a reference decrement
that causes a leak when the idle_hist option is enabled?

In timehist_get_thread(), an additional reference to the idle thread is
acquired if sched->idle_hist is enabled:

tools/perf/builtin-sched.c:timehist_get_thread() {
    ...
		if (sched->idle_hist) {
			struct thread *idle;
			struct idle_thread_runtime *itr;

			idle = get_idle_thread(sample->cpu);
			if (idle == NULL) {
    ...
			thread__put(itr->last_thread);
			itr->last_thread = thread__get(thread);
    ...
}

The reference acquired by get_idle_thread() is never released with a matching
thread__put() before the function returns.

Previously, thread__delete() forcefully destroyed the thread object
regardless of this inflated reference count.

Now that the cleanup relies on thread__put() and the reference count reaching
zero, the leaked reference from timehist_get_thread() prevents the thread
object from being freed.

Does this result in permanently leaking the idle thread objects, their
private idle_thread_runtime data, and accumulated callchains?

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

  reply	other threads:[~2026-06-05 20:49 UTC|newest]

Thread overview: 15+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-06-05 20:33 [PATCHES v1 0/8] perf tools: Fix OOB writes, refcount bugs, and BUG_ON in mmap/stat/c2c/sched Arnaldo Carvalho de Melo
2026-06-05 20:33 ` [PATCH 1/8] perf mmap: Guard cpu__get_node() return in aio_bind() Arnaldo Carvalho de Melo
2026-06-05 20:48   ` sashiko-bot
2026-06-05 20:33 ` [PATCH 2/8] perf stat: Bounds-check CPU index in topology aggregation callbacks Arnaldo Carvalho de Melo
2026-06-05 20:33 ` [PATCH 3/8] perf c2c: Bounds-check CPU and node IDs before bitmap and array access Arnaldo Carvalho de Melo
2026-06-05 20:46   ` sashiko-bot
2026-06-05 20:33 ` [PATCH 4/8] perf c2c: Bounds-check CPU IDs in setup_nodes() topology loop Arnaldo Carvalho de Melo
2026-06-05 20:33 ` [PATCH 5/8] perf sched: Clean up idle_threads entry on init failure Arnaldo Carvalho de Melo
2026-06-05 20:53   ` sashiko-bot
2026-06-05 20:33 ` [PATCH 6/8] perf sched: Fix thread reference leak in idle hist processing Arnaldo Carvalho de Melo
2026-06-05 20:46   ` sashiko-bot
2026-06-05 20:33 ` [PATCH 7/8] perf sched: Use thread__put() in free_idle_threads() Arnaldo Carvalho de Melo
2026-06-05 20:49   ` sashiko-bot [this message]
2026-06-05 20:33 ` [PATCH 8/8] perf sched: Replace BUG_ON and add NULL checks in replay event helpers Arnaldo Carvalho de Melo
2026-06-05 20:46   ` sashiko-bot

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20260605204928.0BD8B1F00893@smtp.kernel.org \
    --to=sashiko-bot@kernel.org \
    --cc=acme@kernel.org \
    --cc=linux-perf-users@vger.kernel.org \
    --cc=sashiko-reviews@lists.linux.dev \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.