public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: Arnaldo Carvalho de Melo <acme@kernel.org>
To: Jiri Olsa <jolsa@redhat.com>
Cc: Namhyung Kim <namhyung@kernel.org>,
	Ingo Molnar <mingo@kernel.org>,
	Peter Zijlstra <a.p.zijlstra@chello.nl>,
	Mark Rutland <mark.rutland@arm.com>,
	Alexander Shishkin <alexander.shishkin@linux.intel.com>,
	LKML <linux-kernel@vger.kernel.org>,
	Stephane Eranian <eranian@google.com>,
	Ian Rogers <irogers@google.com>
Subject: Re: [PATCH 4/7] perf inject: Do not load map/dso when injecting build-id
Date: Mon, 28 Sep 2020 09:31:20 -0300	[thread overview]
Message-ID: <20200928123120.GE3087422@kernel.org> (raw)
In-Reply-To: <20200925142619.GD3273770@krava>

Em Fri, Sep 25, 2020 at 04:26:19PM +0200, Jiri Olsa escreveu:
> On Thu, Sep 24, 2020 at 11:46:32PM +0900, Namhyung Kim wrote:
> > On Thu, Sep 24, 2020 at 03:44:44PM +0200, Jiri Olsa wrote:
> > > On Thu, Sep 24, 2020 at 10:20:51PM +0900, Namhyung Kim wrote:
> > > > On Thu, Sep 24, 2020 at 10:09 PM Jiri Olsa <jolsa@redhat.com> wrote:
> > > > >
> > > > > On Wed, Sep 23, 2020 at 05:05:34PM +0900, Namhyung Kim wrote:
> > > > >
> > > > > SNIP
> > > > >
> > > > > > -static inline int is_no_dso_memory(const char *filename)
> > > > > > -{
> > > > > > -     return !strncmp(filename, "[stack", 6) ||
> > > > > > -            !strncmp(filename, "/SYSV",5)   ||
> > > > > > -            !strcmp(filename, "[heap]");
> > > > > > -}
> > > > > > -
> > > > > >  static inline int is_android_lib(const char *filename)
> > > > > >  {
> > > > > >       return strstarts(filename, "/data/app-lib/") ||
> > > > > > @@ -158,7 +143,7 @@ struct map *map__new(struct machine *machine, u64 start, u64 len,
> > > > > >               int anon, no_dso, vdso, android;
> > > > > >
> > > > > >               android = is_android_lib(filename);
> > > > > > -             anon = is_anon_memory(filename, flags);
> > > > > > +             anon = is_anon_memory(filename) || flags & MAP_HUGETLB;
> > > > >
> > > > > what's the reason to take 'flags & MAP_HUGETLB' out of is_anon_memory?
> > > > 
> > > > The MAP_HUGETLB is defined in uapi/linux/mman.h and I had trouble
> > > > when including the header in the map.h file.
> > > 
> > > could you share the error? it might be corner case, but it
> > > could bite us in future
> > 
> > Sure.
> > 
> >   CC       util/session.o
> > In file included from /home/namhyung/project/linux/tools/include/uapi/asm-generic/mman-common-tools.h:5,
> >                  from /home/namhyung/project/linux/tools/include/uapi/asm-generic/mman.h:5,
> >                  from /home/namhyung/project/linux/tools/arch/x86/include/uapi/asm/mman.h:5,
> >                  from /home/namhyung/project/linux/tools/include/uapi/linux/mman.h:5,
> >                  from util/map.h:13,
> >                  from util/session.c:21:
> > /home/namhyung/project/linux/tools/include/uapi/asm-generic/mman-common.h:26: error: "MAP_POPULATE" redefined [-Werror]
> >    26 | #define MAP_POPULATE  0x008000 /* populate (prefault) pagetables */
> >       | 
> > In file included from /usr/include/x86_64-linux-gnu/bits/mman.h:31,
> >                  from /usr/include/x86_64-linux-gnu/sys/mman.h:41,
> >                  from util/session.c:12:
> > /usr/include/x86_64-linux-gnu/bits/mman-map-flags-generic.h:34: note: this is the location of the previous definition
> >    34 | # define MAP_POPULATE 0x08000  /* Populate (prefault) pagetables.  */
> >       | 
> > 
> > This is repeated for each macro definitions..
> 
> hm, some black magic happened in the past and it looks like now we
> can't have <sys/mman.h> and <linux/mman.h> includes together
> 
> it looks related to this commit:
>   be709d48329a tools headers uapi: Sync asm-generic/mman-common.h and linux/mman.h
> 
> I'm not sure I understand the purpose of asm-generic/mman-common-tools.h file
> 
> Arnaldo,
> any chance you might have some quick solution before I dive in?
> you can reproduce the issue with change below

So, some of the headers in tools/include/ are for
tools/perf/trace/beauty/ to auto-generate tables, like described in the
be709d48329a cset.

This causes problems like the one you're describing, that is why I'm now
using tools/perf/trace/beauty/include/ for these purposes, and that is
not used in building source code, just in autogenerating such tables and
helping us to notice when changes were made in the kernel that may break
those autogeneration scripts, all under that tools/perf/check_patch.sh
build checks.

I'll look into this mmap case and try to get this resolved and then
build it on all the containers to make sure everything continues to work
as expected.

- Arnaldo

  reply	other threads:[~2020-09-28 12:31 UTC|newest]

Thread overview: 23+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-09-23  8:05 [PATCHSET v2 0/7] perf inject: Speed build-id injection Namhyung Kim
2020-09-23  8:05 ` [PATCH 1/7] perf bench: Add build-id injection benchmark Namhyung Kim
2020-09-23 22:13   ` Ian Rogers
2020-09-24  6:23     ` Namhyung Kim
2020-09-23  8:05 ` [PATCH 2/7] perf inject: Add missing callbacks in perf_tool Namhyung Kim
2020-09-23  8:05 ` [PATCH 3/7] perf inject: Enter namespace when reading build-id Namhyung Kim
2020-09-23  8:05 ` [PATCH 4/7] perf inject: Do not load map/dso when injecting build-id Namhyung Kim
2020-09-24 13:09   ` Jiri Olsa
2020-09-24 13:20     ` Namhyung Kim
2020-09-24 13:44       ` Jiri Olsa
2020-09-24 14:46         ` Namhyung Kim
2020-09-25 14:26           ` Jiri Olsa
2020-09-28 12:31             ` Arnaldo Carvalho de Melo [this message]
2020-09-23  8:05 ` [PATCH 5/7] perf inject: Add --buildid-all option Namhyung Kim
2020-09-23 22:16   ` Ian Rogers
2020-09-23  8:05 ` [PATCH 6/7] perf bench: Run inject-build-id with --buildid-all option too Namhyung Kim
2020-09-23 22:17   ` Ian Rogers
2020-09-23  8:05 ` [PATCH 7/7] perf inject: Remove stale build-id processing Namhyung Kim
2020-09-23 14:36   ` Adrian Hunter
2020-09-24  3:51     ` Namhyung Kim
2020-09-24 13:33   ` Jiri Olsa
2020-09-24 14:23     ` Namhyung Kim
2020-09-24 13:35 ` [PATCHSET v2 0/7] perf inject: Speed build-id injection Jiri Olsa

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=20200928123120.GE3087422@kernel.org \
    --to=acme@kernel.org \
    --cc=a.p.zijlstra@chello.nl \
    --cc=alexander.shishkin@linux.intel.com \
    --cc=eranian@google.com \
    --cc=irogers@google.com \
    --cc=jolsa@redhat.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mark.rutland@arm.com \
    --cc=mingo@kernel.org \
    --cc=namhyung@kernel.org \
    /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