linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Krister Johansen <kjlx@templeofstupid.com>
To: Arnaldo Carvalho de Melo <acme@kernel.org>
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: Tue, 4 Jan 2022 22:19:42 -0800	[thread overview]
Message-ID: <20220105061942.GA20464@templeofstupid.com> (raw)
In-Reply-To: <YPCKeDvyLOQzKTln@kernel.org>

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?

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

  reply	other threads:[~2022-01-05  6:19 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 [this message]
2022-01-06 20:27         ` Arnaldo Carvalho de Melo
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=20220105061942.GA20464@templeofstupid.com \
    --to=kjlx@templeofstupid.com \
    --cc=acme@kernel.org \
    --cc=irogers@google.com \
    --cc=jolsa@redhat.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).