From: Arnaldo Carvalho de Melo <acme@kernel.org>
To: Krister Johansen <kjlx@templeofstupid.com>
Cc: Riccardo Mancini <rickyman7@gmail.com>,
Ian Rogers <irogers@google.com>,
Namhyung Kim <namhyung@kernel.org>,
Peter Zijlstra <peterz@infradead.org>,
Ingo Molnar <mingo@redhat.com>,
Mark Rutland <mark.rutland@arm.com>, Jiri Olsa <jolsa@redhat.com>,
linux-kernel@vger.kernel.org, linux-perf-users@vger.kernel.org
Subject: Re: [PATCH 01/20] perf nsinfo: fix refcounting
Date: Thu, 6 Jan 2022 17:27:43 -0300 [thread overview]
Message-ID: <YddQv1O057Mw5Whv@kernel.org> (raw)
In-Reply-To: <20220105061942.GA20464@templeofstupid.com>
Em Tue, Jan 04, 2022 at 10:19:42PM -0800, Krister Johansen escreveu:
> On Thu, Jul 15, 2021 at 04:20:24PM -0300, Arnaldo Carvalho de Melo wrote:
> > Em Thu, Jul 15, 2021 at 04:02:16PM -0300, Arnaldo Carvalho de Melo escreveu:
> > > Em Thu, Jul 15, 2021 at 06:07:06PM +0200, Riccardo Mancini escreveu:
> > > > ASan reports a memory leak of nsinfo during the execution of the perf
> > > > test "31: Lookup mmap thread".
> > > > The leak is caused by a refcounted variable being replaced without
> > > > dropping the refcount.
> >
> > > > This patch makes sure that the refcnt of nsinfo is decreased whenever
> > > > a refcounted variable is replaced with a new value.
> >
> > > So, there are multiple fixes in just one patch, I'll split it into
> > > three, no need to resend.
> >
> > > I'll try and check if finding Fixes: for the three is easy, that way
> > > stable@vger.kernel.org will figure out which of the supported releases
> > > need each of them.
> >
> > The second is at the end of this message, and looking at it I think
> > there is a problem with where this nsinfo reference is stored, it should
> > be in the 'struct map', that is per-thread, not in the 'struct dso',
> > that may be shared by multiple 'struct map's in different cgroups, so we
> > can't set dso->nsinfo in map__new(), right? Kristen?
>
> Apologies for the long delayed reply, and for the hassle that this has
> caused.
>
> You're right, Arnaldo, that namespace mapping could differ on a per
> thread basis. If the dso objects are intended to be read by multiple
> threads with different views of the target process's namespaces. With
> containers, though, there might be many different versions of the same
> object with the same filename, and the only ways we can disambiguate are
> through the build-ids or by ensuring that the dso refers to a single
> pid and mount namespace. I may have erred in assuming that all of
> perf's threads would be able to access the target namespace via the same
> identifiers. Is that what's going on here?
yeah, I think using build-ids, when available is the way to go, and
since we can almost take it for granted (haven't checked) that it will
be present, as I think gcc/clang adds them by default, seems the way to
go.
That or somehow have the DSO list to be perf namespace.
- Arnaldo
> -K
>
>
> > commit 280f3933661f9ca9563ed04ce82313fdbcbb8f75
> > Author: Riccardo Mancini <rickyman7@gmail.com>
> > Date: Thu Jul 15 18:07:06 2021 +0200
> >
> > perf map: Fix dso->nsinfo refcounting
> >
> > ASan reports a memory leak of nsinfo during the execution of
> >
> > # perf test "31: Lookup mmap thread"
> >
> > The leak is caused by a refcounted variable being replaced without
> > dropping the refcount.
> >
> > This patch makes sure that the refcnt of nsinfo is decreased whenever a
> > refcounted variable is replaced with a new value.
> >
> > Signed-off-by: Riccardo Mancini <rickyman7@gmail.com>
> > Fixes: bf2e710b3cb8445c ("perf maps: Lookup maps in both intitial mountns and inner mountns.")
> > Cc: Ian Rogers <irogers@google.com>
> > Cc: Jiri Olsa <jolsa@redhat.com>
> > Cc: Krister Johansen <kjlx@templeofstupid.com>
> > Cc: Mark Rutland <mark.rutland@arm.com>
> > Cc: Namhyung Kim <namhyung@kernel.org>
> > Cc: Peter Zijlstra <peterz@infradead.org>
> > Link: http://lore.kernel.org/lkml/55223bc8821b34ccb01f92ef1401c02b6a32e61f.1626343282.git.rickyman7@gmail.com
> > Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
> >
> > diff --git a/tools/perf/util/map.c b/tools/perf/util/map.c
> > index 8af693d9678cefe0..72e7f3616157ead4 100644
> > --- a/tools/perf/util/map.c
> > +++ b/tools/perf/util/map.c
> > @@ -192,6 +192,8 @@ struct map *map__new(struct machine *machine, u64 start, u64 len,
> > if (!(prot & PROT_EXEC))
> > dso__set_loaded(dso);
> > }
> > +
> > + nsinfo__put(dso->nsinfo);
> > dso->nsinfo = nsi;
> >
> > if (build_id__is_defined(bid))
> >
--
- Arnaldo
next prev parent reply other threads:[~2022-01-06 20:27 UTC|newest]
Thread overview: 50+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-07-15 16:07 [PATCH 00/20] perf: fix several memory leaks reported by ASan on perf-test Riccardo Mancini
2021-07-15 16:07 ` [PATCH 01/20] perf nsinfo: fix refcounting Riccardo Mancini
2021-07-15 19:02 ` Arnaldo Carvalho de Melo
2021-07-15 19:07 ` Arnaldo Carvalho de Melo
2021-07-15 19:20 ` Arnaldo Carvalho de Melo
2022-01-05 6:19 ` Krister Johansen
2022-01-06 20:27 ` Arnaldo Carvalho de Melo [this message]
2022-01-06 20:34 ` Arnaldo Carvalho de Melo
2021-07-15 19:44 ` Arnaldo Carvalho de Melo
2021-07-15 16:07 ` [PATCH 02/20] perf env: fix sibling_dies memory leak Riccardo Mancini
2021-07-15 19:49 ` Arnaldo Carvalho de Melo
2021-07-16 13:13 ` Riccardo Mancini
2021-07-15 16:07 ` [PATCH 03/20] perf test: session_topology: delete session->evlist Riccardo Mancini
2021-07-15 19:51 ` Arnaldo Carvalho de Melo
2021-07-15 16:07 ` [PATCH 04/20] perf test: event_update: fix memory leak of evlist Riccardo Mancini
2021-07-15 19:54 ` Arnaldo Carvalho de Melo
2021-07-15 16:07 ` [PATCH 05/20] perf test: event_update: fix memory leak of unit Riccardo Mancini
2021-07-15 19:58 ` Arnaldo Carvalho de Melo
2021-07-15 16:07 ` [PATCH 06/20] perf dso: fix memory leak in dso__new_map Riccardo Mancini
2021-07-15 20:01 ` Arnaldo Carvalho de Melo
2021-07-15 20:07 ` Arnaldo Carvalho de Melo
2021-07-15 16:07 ` [PATCH 07/20] perf test: maps__merge_in: fix memory leak of maps Riccardo Mancini
2021-07-15 20:10 ` Arnaldo Carvalho de Melo
2021-07-15 16:07 ` [PATCH 08/20] perf env: fix memory leak of cpu_pmu_caps Riccardo Mancini
2021-07-15 20:12 ` Arnaldo Carvalho de Melo
2021-07-15 16:07 ` [PATCH 09/20] perf report: free generated help strings for sort option Riccardo Mancini
2021-07-15 20:15 ` Arnaldo Carvalho de Melo
2021-07-15 16:07 ` [PATCH 10/20] perf inject: close inject.output Riccardo Mancini
2021-07-15 20:17 ` Arnaldo Carvalho de Melo
2021-07-15 16:07 ` [PATCH 11/20] perf session: cleanup trace_event Riccardo Mancini
2021-07-15 20:21 ` Arnaldo Carvalho de Melo
2021-07-15 16:07 ` [PATCH 12/20] perf script: release zstd data Riccardo Mancini
2021-07-15 20:23 ` Arnaldo Carvalho de Melo
2021-07-15 16:07 ` [PATCH 13/20] perf script: fix memory leaks in perf_script Riccardo Mancini
2021-07-15 20:25 ` Arnaldo Carvalho de Melo
2021-07-15 16:07 ` [PATCH 14/20] perf util/lzma: close lzma stream Riccardo Mancini
2021-07-15 20:31 ` Arnaldo Carvalho de Melo
2021-07-15 16:07 ` [PATCH 15/20] perf trace: free malloc'd trace fields on exit Riccardo Mancini
2021-07-15 20:32 ` Arnaldo Carvalho de Melo
2021-07-15 16:07 ` [PATCH 16/20] perf trace: free syscall->arg_fmt Riccardo Mancini
2021-07-15 20:35 ` Arnaldo Carvalho de Melo
2021-07-15 16:07 ` [PATCH 17/20] perf trace: free syscall tp fields in evsel->priv Riccardo Mancini
2021-07-15 20:35 ` Arnaldo Carvalho de Melo
2021-07-15 16:07 ` [PATCH 18/20] perf trace: free strings in trace__parse_events_option Riccardo Mancini
2021-07-15 20:36 ` Arnaldo Carvalho de Melo
2021-07-15 16:07 ` [PATCH 19/20] perf test: bpf: free obj_buf Riccardo Mancini
2021-07-15 20:39 ` Arnaldo Carvalho de Melo
2021-07-16 16:50 ` Arnaldo Carvalho de Melo
2021-07-15 16:07 ` [PATCH 20/20] perf util/probe-file: delete namelist on error in del_events Riccardo Mancini
2021-07-15 20:41 ` Arnaldo Carvalho de Melo
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=YddQv1O057Mw5Whv@kernel.org \
--to=acme@kernel.org \
--cc=irogers@google.com \
--cc=jolsa@redhat.com \
--cc=kjlx@templeofstupid.com \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-perf-users@vger.kernel.org \
--cc=mark.rutland@arm.com \
--cc=mingo@redhat.com \
--cc=namhyung@kernel.org \
--cc=peterz@infradead.org \
--cc=rickyman7@gmail.com \
/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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).