lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <CAP-5=fU+nJreGEc1HVsYOL1Ba=YFE4G7OQqtQ=iwH=6+Crxssw@mail.gmail.com>
Date: Wed, 2 Jul 2025 20:08:35 -0700
From: Ian Rogers <irogers@...gle.com>
To: Namhyung Kim <namhyung@...nel.org>
Cc: Arnaldo Carvalho de Melo <acme@...nel.org>, Kan Liang <kan.liang@...ux.intel.com>, 
	Jiri Olsa <jolsa@...nel.org>, Adrian Hunter <adrian.hunter@...el.com>, 
	Peter Zijlstra <peterz@...radead.org>, Ingo Molnar <mingo@...nel.org>, 
	LKML <linux-kernel@...r.kernel.org>, linux-perf-users@...r.kernel.org
Subject: Re: [PATCH 4/8] perf sched: Fix thread leaks in 'perf sched timehist'

On Wed, Jul 2, 2025 at 6:49 PM Namhyung Kim <namhyung@...nel.org> wrote:
>
> Add missing thread__put() after machine__findnew_thread() or
> timehist_get_thread().  Also idle threads' last_thread should be
> refcounted properly.
>
> Signed-off-by: Namhyung Kim <namhyung@...nel.org>

Reviewed-by: Ian Rogers <irogers@...gle.com>

Thanks,
Ian

> ---
>  tools/perf/builtin-sched.c | 48 +++++++++++++++++++++++++++++---------
>  1 file changed, 37 insertions(+), 11 deletions(-)
>
> diff --git a/tools/perf/builtin-sched.c b/tools/perf/builtin-sched.c
> index b73989fb6acef8d6..83b5a85a91b7ffbd 100644
> --- a/tools/perf/builtin-sched.c
> +++ b/tools/perf/builtin-sched.c
> @@ -2313,8 +2313,10 @@ static void save_task_callchain(struct perf_sched *sched,
>                 return;
>         }
>
> -       if (!sched->show_callchain || sample->callchain == NULL)
> +       if (!sched->show_callchain || sample->callchain == NULL) {
> +               thread__put(thread);
>                 return;
> +       }
>
>         cursor = get_tls_callchain_cursor();
>
> @@ -2323,10 +2325,12 @@ static void save_task_callchain(struct perf_sched *sched,
>                 if (verbose > 0)
>                         pr_err("Failed to resolve callchain. Skipping\n");
>
> +               thread__put(thread);
>                 return;
>         }
>
>         callchain_cursor_commit(cursor);
> +       thread__put(thread);
>
>         while (true) {
>                 struct callchain_cursor_node *node;
> @@ -2403,8 +2407,17 @@ static void free_idle_threads(void)
>                 return;
>
>         for (i = 0; i < idle_max_cpu; ++i) {
> -               if ((idle_threads[i]))
> -                       thread__delete(idle_threads[i]);
> +               struct thread *idle = idle_threads[i];
> +
> +               if (idle) {
> +                       struct idle_thread_runtime *itr;
> +
> +                       itr = thread__priv(idle);
> +                       if (itr)
> +                               thread__put(itr->last_thread);
> +
> +                       thread__delete(idle);
> +               }
>         }
>
>         free(idle_threads);
> @@ -2441,7 +2454,7 @@ static struct thread *get_idle_thread(int cpu)
>                 }
>         }
>
> -       return idle_threads[cpu];
> +       return thread__get(idle_threads[cpu]);
>  }
>
>  static void save_idle_callchain(struct perf_sched *sched,
> @@ -2496,7 +2509,8 @@ static struct thread *timehist_get_thread(struct perf_sched *sched,
>                         if (itr == NULL)
>                                 return NULL;
>
> -                       itr->last_thread = thread;
> +                       thread__put(itr->last_thread);
> +                       itr->last_thread = thread__get(thread);
>
>                         /* copy task callchain when entering to idle */
>                         if (evsel__intval(evsel, sample, "next_pid") == 0)
> @@ -2567,6 +2581,7 @@ static void timehist_print_wakeup_event(struct perf_sched *sched,
>         /* show wakeup unless both awakee and awaker are filtered */
>         if (timehist_skip_sample(sched, thread, evsel, sample) &&
>             timehist_skip_sample(sched, awakened, evsel, sample)) {
> +               thread__put(thread);
>                 return;
>         }
>
> @@ -2583,6 +2598,8 @@ static void timehist_print_wakeup_event(struct perf_sched *sched,
>         printf("awakened: %s", timehist_get_commstr(awakened));
>
>         printf("\n");
> +
> +       thread__put(thread);
>  }
>
>  static int timehist_sched_wakeup_ignore(const struct perf_tool *tool __maybe_unused,
> @@ -2611,8 +2628,10 @@ static int timehist_sched_wakeup_event(const struct perf_tool *tool,
>                 return -1;
>
>         tr = thread__get_runtime(thread);
> -       if (tr == NULL)
> +       if (tr == NULL) {
> +               thread__put(thread);
>                 return -1;
> +       }
>
>         if (tr->ready_to_run == 0)
>                 tr->ready_to_run = sample->time;
> @@ -2622,6 +2641,7 @@ static int timehist_sched_wakeup_event(const struct perf_tool *tool,
>             !perf_time__skip_sample(&sched->ptime, sample->time))
>                 timehist_print_wakeup_event(sched, evsel, sample, machine, thread);
>
> +       thread__put(thread);
>         return 0;
>  }
>
> @@ -2649,6 +2669,7 @@ static void timehist_print_migration_event(struct perf_sched *sched,
>
>         if (timehist_skip_sample(sched, thread, evsel, sample) &&
>             timehist_skip_sample(sched, migrated, evsel, sample)) {
> +               thread__put(thread);
>                 return;
>         }
>
> @@ -2676,6 +2697,7 @@ static void timehist_print_migration_event(struct perf_sched *sched,
>         printf(" cpu %d => %d", ocpu, dcpu);
>
>         printf("\n");
> +       thread__put(thread);
>  }
>
>  static int timehist_migrate_task_event(const struct perf_tool *tool,
> @@ -2695,8 +2717,10 @@ static int timehist_migrate_task_event(const struct perf_tool *tool,
>                 return -1;
>
>         tr = thread__get_runtime(thread);
> -       if (tr == NULL)
> +       if (tr == NULL) {
> +               thread__put(thread);
>                 return -1;
> +       }
>
>         tr->migrations++;
>         tr->migrated = sample->time;
> @@ -2706,6 +2730,7 @@ static int timehist_migrate_task_event(const struct perf_tool *tool,
>                 timehist_print_migration_event(sched, evsel, sample,
>                                                         machine, thread);
>         }
> +       thread__put(thread);
>
>         return 0;
>  }
> @@ -2728,10 +2753,10 @@ static void timehist_update_task_prio(struct evsel *evsel,
>                 return;
>
>         tr = thread__get_runtime(thread);
> -       if (tr == NULL)
> -               return;
> +       if (tr != NULL)
> +               tr->prio = next_prio;
>
> -       tr->prio = next_prio;
> +       thread__put(thread);
>  }
>
>  static int timehist_sched_change_event(const struct perf_tool *tool,
> @@ -2743,7 +2768,7 @@ static int timehist_sched_change_event(const struct perf_tool *tool,
>         struct perf_sched *sched = container_of(tool, struct perf_sched, tool);
>         struct perf_time_interval *ptime = &sched->ptime;
>         struct addr_location al;
> -       struct thread *thread;
> +       struct thread *thread = NULL;
>         struct thread_runtime *tr = NULL;
>         u64 tprev, t = sample->time;
>         int rc = 0;
> @@ -2867,6 +2892,7 @@ static int timehist_sched_change_event(const struct perf_tool *tool,
>
>         evsel__save_time(evsel, sample->time, sample->cpu);
>
> +       thread__put(thread);
>         addr_location__exit(&al);
>         return rc;
>  }
> --
> 2.50.0.727.gbf7dc18ff4-goog
>

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ