All of lore.kernel.org
 help / color / mirror / Atom feed
From: Arnaldo Carvalho de Melo <acme@kernel.org>
To: Maciej Debski <maciejd@google.com>
Cc: Stephane Eranian <eranian@google.com>,
	linux-kernel@vger.kernel.org, jolsa@redhat.com,
	peterz@infradead.org, Ingo Molnar <mingo@kernel.org>,
	anton@ozlabs.org, namhyung@kernel.org
Subject: Re: [PATCH 2/9] perf/jit: enable jitdump support without dwarf
Date: Thu, 13 Oct 2016 15:37:42 -0300	[thread overview]
Message-ID: <20161013183742.GH23873@kernel.org> (raw)
In-Reply-To: <20161013181643.GG23873@kernel.org>

Em Thu, Oct 13, 2016 at 03:16:43PM -0300, Arnaldo Carvalho de Melo escreveu:
> Em Thu, Oct 13, 2016 at 03:59:36AM -0700, Stephane Eranian escreveu:
> > From: Maciej Debski <maciejd@google.com>
> > 
> > This patch modifies the build dependencies on the jitdump
> > support in perf. As it stands jitdump was wrongfully made dependent
> > 100% on using DWARF. However, the dwarf dependency, only exist if
> > generating the source line table in genelf_debug.c. The rest of the
> > support does not need DWARF.
> > 
> > This patch removes the dependency on DWARF for the entire jitdump
> > support. It keeps it only for the genelf_debug.c support.
> 
> Please add a:
> 
> Fixes: e12b202f8fb9 ("perf jitdump: Build only on supported archs")
> 
> Next time, so that scripts can pick patches for stable kernels, etc.
> 
> Adding it this time.

But, this introduces a bug, breaking the perf's static build:

[acme@jouet linux]$ git bisect bad
Bisecting: 0 revisions left to test after this (roughly 0 steps)
[f5722b101fb37339001e9524262d3eb08b7b1bfe] perf jit: Enable jitdump support without dwarf
[acme@jouet linux]$ rm -rf /tmp/build/perf/ ; mkdir -p /tmp/build/perf ; make -C tools/perf LDFLAGS=-static O=/tmp/build/perf install-bin

<big SNIP>

/tmp/build/perf/perf-in.o: In function `perf_event__jit_repipe_mmap2':
/home/acme/git/linux/tools/perf/builtin-inject.c:298: undefined reference to `jit_process'
/tmp/build/perf/perf-in.o: In function `perf_event__jit_repipe_mmap':
/home/acme/git/linux/tools/perf/builtin-inject.c:260: undefined reference to `jit_process'
collect2: error: ld returned 1 exit status
Makefile.perf:470: recipe for target '/tmp/build/perf/perf' failed
make[1]: *** [/tmp/build/perf/perf] Error 1
Makefile:108: recipe for target 'install-bin' failed
make: *** [install-bin] Error 2
make: Leaving directory '/home/acme/git/linux/tools/perf'
[acme@jouet linux]$ git bisect bad
f5722b101fb37339001e9524262d3eb08b7b1bfe is the first bad commit
commit f5722b101fb37339001e9524262d3eb08b7b1bfe
Author: Maciej Debski <maciejd@google.com>
Date:   Thu Oct 13 03:59:36 2016 -0700

    perf jit: Enable jitdump support without dwarf
    
    This patch modifies the build dependencies on the jitdump support in
    perf. As it stands jitdump was wrongfully made dependent 100% on using
    DWARF. However, the dwarf dependency, only exist if generating the
    source line table in genelf_debug.c. The rest of the support does not
    need DWARF.
    
    This patch removes the dependency on DWARF for the entire jitdump
    support. It keeps it only for the genelf_debug.c support.
    
    Signed-off-by: Maciej Debski <maciejd@google.com>
    Reviewed-by: Stephane Eranian <eranian@google.com>
    Cc: Anton Blanchard <anton@ozlabs.org>
    Cc: Jiri Olsa <jolsa@redhat.com>
    Cc: Namhyung Kim <namhyung@kernel.org>
    Cc: Peter Zijlstra <peterz@infradead.org>
    Link: http://lkml.kernel.org/r/1476356383-30100-3-git-send-email-eranian@google.com
    Fixes: e12b202f8fb9 ("perf jitdump: Build only on supported archs")
    Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>

:040000 040000 281764609ccfd745f984caae6a331b5b9ebd15b0 4f1ebed718481cacdd901f28a05b347a135e46cf M	tools
[acme@jouet linux]$

Please use:

  make -C tools/perf build-test

That will do may build tests, including this one, the static build.

I'm trying to figure this out...

- Arnaldo
 
> Thanks,
> 
> - Arnaldo
> 
>  
> > Signed-off-by: Maciej Debski <maciejd@google.com>
> > Reviewed-by: Stephane Eranian <eranian@google.com>
> > ---
> >  tools/perf/Makefile.config | 2 --
> >  tools/perf/util/Build      | 2 +-
> >  tools/perf/util/genelf.c   | 9 +++++++--
> >  tools/perf/util/genelf.h   | 2 ++
> >  4 files changed, 10 insertions(+), 5 deletions(-)
> > 
> > diff --git a/tools/perf/Makefile.config b/tools/perf/Makefile.config
> > index 72edf83d76b7..8cfc310d4358 100644
> > --- a/tools/perf/Makefile.config
> > +++ b/tools/perf/Makefile.config
> > @@ -366,10 +366,8 @@ ifndef NO_SDT
> >  endif
> >  
> >  ifdef PERF_HAVE_JITDUMP
> > -  ifndef NO_DWARF
> >      $(call detected,CONFIG_JITDUMP)
> >      CFLAGS += -DHAVE_JITDUMP
> > -  endif
> >  endif
> >  
> >  ifeq ($(ARCH),powerpc)
> > diff --git a/tools/perf/util/Build b/tools/perf/util/Build
> > index eb60e613d795..1dc67efad634 100644
> > --- a/tools/perf/util/Build
> > +++ b/tools/perf/util/Build
> > @@ -120,7 +120,7 @@ libperf-y += demangle-rust.o
> >  ifdef CONFIG_JITDUMP
> >  libperf-$(CONFIG_LIBELF) += jitdump.o
> >  libperf-$(CONFIG_LIBELF) += genelf.o
> > -libperf-$(CONFIG_LIBELF) += genelf_debug.o
> > +libperf-$(CONFIG_DWARF) += genelf_debug.o
> >  endif
> >  
> >  CFLAGS_config.o   += -DETC_PERFCONFIG="BUILD_STR($(ETC_PERFCONFIG_SQ))"
> > diff --git a/tools/perf/util/genelf.c b/tools/perf/util/genelf.c
> > index c1ef805c6a8f..14a73acc549c 100644
> > --- a/tools/perf/util/genelf.c
> > +++ b/tools/perf/util/genelf.c
> > @@ -19,7 +19,9 @@
> >  #include <limits.h>
> >  #include <fcntl.h>
> >  #include <err.h>
> > +#ifdef HAVE_DWARF_SUPPORT
> >  #include <dwarf.h>
> > +#endif
> >  
> >  #include "perf.h"
> >  #include "genelf.h"
> > @@ -157,7 +159,7 @@ gen_build_id(struct buildid_note *note, unsigned long load_addr, const void *cod
> >  int
> >  jit_write_elf(int fd, uint64_t load_addr, const char *sym,
> >  	      const void *code, int csize,
> > -	      void *debug, int nr_debug_entries)
> > +	      void *debug __maybe_unused, int nr_debug_entries __maybe_unused)
> >  {
> >  	Elf *e;
> >  	Elf_Data *d;
> > @@ -386,11 +388,14 @@ jit_write_elf(int fd, uint64_t load_addr, const char *sym,
> >  	shdr->sh_size = sizeof(bnote);
> >  	shdr->sh_entsize = 0;
> >  
> > +#ifdef HAVE_DWARF_SUPPORT
> >  	if (debug && nr_debug_entries) {
> >  		retval = jit_add_debug_info(e, load_addr, debug, nr_debug_entries);
> >  		if (retval)
> >  			goto error;
> > -	} else {
> > +	} else
> > +#endif
> > +	{
> >  		if (elf_update(e, ELF_C_WRITE) < 0) {
> >  			warnx("elf_update 4 failed");
> >  			goto error;
> > diff --git a/tools/perf/util/genelf.h b/tools/perf/util/genelf.h
> > index 2fbeb59c4bdd..5c933ac71451 100644
> > --- a/tools/perf/util/genelf.h
> > +++ b/tools/perf/util/genelf.h
> > @@ -4,8 +4,10 @@
> >  /* genelf.c */
> >  int jit_write_elf(int fd, uint64_t code_addr, const char *sym,
> >  		  const void *code, int csize, void *debug, int nr_debug_entries);
> > +#ifdef HAVE_DWARF_SUPPORT
> >  /* genelf_debug.c */
> >  int jit_add_debug_info(Elf *e, uint64_t code_addr, void *debug, int nr_debug_entries);
> > +#endif
> >  
> >  #if   defined(__arm__)
> >  #define GEN_ELF_ARCH	EM_ARM
> > -- 
> > 1.9.1

  reply	other threads:[~2016-10-13 19:31 UTC|newest]

Thread overview: 29+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-10-13 10:59 [PATCH 0/9] perf/jit: various improvements Stephane Eranian
2016-10-13 10:59 ` [PATCH 1/9] perf/jit: improve error messages from JVMTI Stephane Eranian
2016-10-13 20:05   ` Nilay Vaish
2016-10-14 11:13     ` Arnaldo Carvalho de Melo
2016-10-14 12:57       ` Stephane Eranian
2016-10-14 15:20         ` Arnaldo Carvalho de Melo
2016-10-17 13:52           ` Stephane Eranian
2016-10-24 18:58   ` [tip:perf/core] perf jit: Improve " tip-bot for Stephane Eranian
2016-10-13 10:59 ` [PATCH 2/9] perf/jit: enable jitdump support without dwarf Stephane Eranian
2016-10-13 18:16   ` Arnaldo Carvalho de Melo
2016-10-13 18:37     ` Arnaldo Carvalho de Melo [this message]
2016-10-13 18:51       ` Arnaldo Carvalho de Melo
2016-10-13 19:03         ` Arnaldo Carvalho de Melo
2016-10-13 19:44           ` Arnaldo Carvalho de Melo
2016-10-24 18:58   ` [tip:perf/core] perf jit: Enable " tip-bot for Maciej Debski
2016-10-13 10:59 ` [PATCH 3/9] perf/jit: remove unecessary padding in jitdump file Stephane Eranian
2016-10-24 18:59   ` [tip:perf/core] perf jit: Remove " tip-bot for Stephane Eranian
2016-10-13 10:59 ` [PATCH 4/9] perf/jit: make perf skip unknown records Stephane Eranian
2016-10-24 18:59   ` [tip:perf/core] perf jit: Make " tip-bot for Stefano Sanfilippo
2016-10-13 10:59 ` [PATCH 5/9] perf/jit: do not assume pgoff is zero Stephane Eranian
2016-10-24 19:00   ` [tip:perf/core] perf jit: Do " tip-bot for Stefano Sanfilippo
2016-10-13 10:59 ` [PATCH 6/9] perf/jit: add unwinding support Stephane Eranian
2016-10-24 19:00   ` [tip:perf/core] perf jit: Add " tip-bot for Stefano Sanfilippo
2016-10-13 10:59 ` [PATCH 7/9] perf/jit: generate .eh_frame/.eh_frame_hdr in DSO Stephane Eranian
2016-10-24 19:01   ` [tip:perf/core] perf jit: Generate " tip-bot for Stefano Sanfilippo
2016-10-13 10:59 ` [PATCH 8/9] perf/jit: Check JITHEADER_VERSION Stephane Eranian
2016-10-24 19:01   ` [tip:perf/core] perf jit: " tip-bot for Stefano Sanfilippo
2016-10-13 10:59 ` [PATCH 9/9] perf/jit: add jitdump format specification document Stephane Eranian
2016-10-24 19:02   ` [tip:perf/core] perf jit: Add " tip-bot for Stephane Eranian

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=20161013183742.GH23873@kernel.org \
    --to=acme@kernel.org \
    --cc=anton@ozlabs.org \
    --cc=eranian@google.com \
    --cc=jolsa@redhat.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=maciejd@google.com \
    --cc=mingo@kernel.org \
    --cc=namhyung@kernel.org \
    --cc=peterz@infradead.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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.