Re: [PATCH v1 06/17] perf hist: In init, ensure mem_info is put on error paths

[Ian Rogers <irogers@google.com>]

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
> >