On Mon, Dec 1, 2025 at 9:14 AM Arnaldo Carvalho de Melo <acme@kernel.org> wrote:
>
> On Sat, Nov 22, 2025 at 12:19:18AM -0800, Ian Rogers wrote:
> > Rather than exit the internal map_symbols directly, put the mem-info
> > that does this and also lowers the reference count on the mem-info
> > itself otherwise the mem-info is being leaked.
>
> Shouldn't this have a:
>
> Fixes: 56e144fe98260a0f ("perf mem_info: Add and use map_symbol__exit and addr_map_symbol__exit")
I don't mind the fixes tag. The change in that patch was:
```
if (he->mem_info) {
- map__put(he->mem_info->iaddr.ms.map);
- map__put(he->mem_info->daddr.ms.map);
+ map_symbol__exit(&he->mem_info->iaddr.ms);
+ map_symbol__exit(&he->mem_info->daddr.ms);
}
```
and so technically the memory leak predated that change.
Thanks,
Ian
> Other than that:
>
> Reviewed-by: Arnaldo Carvalho de Melo <acme@redhat.com>
>
> - Arnaldo
>
> > Signed-off-by: Ian Rogers <irogers@google.com>
> > ---
> > tools/perf/util/hist.c | 6 ++----
> > 1 file changed, 2 insertions(+), 4 deletions(-)
> >
> > diff --git a/tools/perf/util/hist.c b/tools/perf/util/hist.c
> > index 64ff427040c3..ef4b569f7df4 100644
> > --- a/tools/perf/util/hist.c
> > +++ b/tools/perf/util/hist.c
> > @@ -608,10 +608,8 @@ static int hist_entry__init(struct hist_entry *he,
> > map_symbol__exit(&he->branch_info->to.ms);
> > zfree(&he->branch_info);
> > }
> > - if (he->mem_info) {
> > - map_symbol__exit(&mem_info__iaddr(he->mem_info)->ms);
> > - map_symbol__exit(&mem_info__daddr(he->mem_info)->ms);
> > - }
> > + if (he->mem_info)
> > + mem_info__zput(he->mem_info);
> > err:
> > map_symbol__exit(&he->ms);
> > zfree(&he->stat_acc);
> > --
> > 2.52.0.rc2.455.g230fcf2819-goog
> >