lkml.org 
[lkml]   [2022]   [Jan]   [5]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: [PATCH 01/20] perf nsinfo: fix refcounting
    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))
    >

    \
     
     \ /
      Last update: 2022-01-05 07:20    [W:2.176 / U:0.300 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site