The perf jvmti agent doesn't create program headers, in this case
fallback on section headers as happened previously.
Fixes: 882528d2e776 ("perf symbol: Skip symbols if SHF_ALLOC flag is not set")
Signed-off-by: Ian Rogers <[email protected]>
---
tools/perf/util/symbol-elf.c | 27 ++++++++++++++++++++-------
1 file changed, 20 insertions(+), 7 deletions(-)
diff --git a/tools/perf/util/symbol-elf.c b/tools/perf/util/symbol-elf.c
index b3be5b1d9dbb..75bec32d4f57 100644
--- a/tools/perf/util/symbol-elf.c
+++ b/tools/perf/util/symbol-elf.c
@@ -1305,16 +1305,29 @@ dso__load_sym_internal(struct dso *dso, struct map *map, struct symsrc *syms_ss,
if (elf_read_program_header(syms_ss->elf,
(u64)sym.st_value, &phdr)) {
- pr_warning("%s: failed to find program header for "
+ pr_debug4("%s: failed to find program header for "
"symbol: %s st_value: %#" PRIx64 "\n",
__func__, elf_name, (u64)sym.st_value);
- continue;
+ pr_debug4("%s: adjusting symbol: st_value: %#" PRIx64 " "
+ "sh_addr: %#" PRIx64 " sh_offset: %#" PRIx64 "\n",
+ __func__, (u64)sym.st_value, (u64)shdr.sh_addr,
+ (u64)shdr.sh_offset);
+ /*
+ * Fail to find program header, let's rollback
+ * to use shdr.sh_addr and shdr.sh_offset to
+ * calibrate symbol's file address, though this
+ * is not necessary for normal C ELF file, we
+ * still need to handle java JIT symbols in this
+ * case.
+ */
+ sym.st_value -= shdr.sh_addr - shdr.sh_offset;
+ } else {
+ pr_debug4("%s: adjusting symbol: st_value: %#" PRIx64 " "
+ "p_vaddr: %#" PRIx64 " p_offset: %#" PRIx64 "\n",
+ __func__, (u64)sym.st_value, (u64)phdr.p_vaddr,
+ (u64)phdr.p_offset);
+ sym.st_value -= phdr.p_vaddr - phdr.p_offset;
}
- pr_debug4("%s: adjusting symbol: st_value: %#" PRIx64 " "
- "p_vaddr: %#" PRIx64 " p_offset: %#" PRIx64 "\n",
- __func__, (u64)sym.st_value, (u64)phdr.p_vaddr,
- (u64)phdr.p_offset);
- sym.st_value -= phdr.p_vaddr - phdr.p_offset;
}
demangled = demangle_sym(dso, kmodule, elf_name);
--
2.37.1.455.g008518b4e5-goog
On Sun, Jul 31, 2022 at 09:49:23AM -0700, Ian Rogers wrote:
> The perf jvmti agent doesn't create program headers, in this case
> fallback on section headers as happened previously.
>
> Fixes: 882528d2e776 ("perf symbol: Skip symbols if SHF_ALLOC flag is not set")
It's good to change fix tag as:
Fixes: 2d86612aacb7 ("perf symbol: Correct address for bss symbols")
I saw stable kernel maintainers have back ported patch "perf symbol:
Correct address for bss symbols" for stable kernel branches, the
suggested fix tag would allow this patch to be landed on stable kernels
as well.
I think I need to manually send the patch "perf symbol: Skip symbols
if SHF_ALLOC flag is not set" to stable kernel mailing list, this
patch missed fix tag.
> Signed-off-by: Ian Rogers <[email protected]>
With updating fix tag:
Reviewed-by: Leo Yan <[email protected]>
Tested-by: Leo Yan <[email protected]>
Em Sun, Jul 31, 2022 at 11:19:15PM -0700, Ian Rogers escreveu:
> On Sun, Jul 31, 2022, 6:53 PM Leo Yan <[email protected]> wrote:
>
> > On Sun, Jul 31, 2022 at 09:49:23AM -0700, Ian Rogers wrote:
> > > The perf jvmti agent doesn't create program headers, in this case
> > > fallback on section headers as happened previously.
> > >
> > > Fixes: 882528d2e776 ("perf symbol: Skip symbols if SHF_ALLOC flag is not
> > set")
> >
> > It's good to change fix tag as:
> > Fixes: 2d86612aacb7 ("perf symbol: Correct address for bss symbols")
> >
>
> Doh! I was rushing this morning. Thanks for catching and reviewing!
I made the adjustments and added a note with the repro, to help in the
future when trying to test this area.
I also think we could have something like a 'perf test' mode where, when
asked to, it would enable tests that involve downloading such files to
perform tests, such as this dacapo benchmark, and then would test if the
output matches expectations.
- Arnaldo
commit 6d518ac7be6223811ab947897273b1bbef846180
Author: Ian Rogers <[email protected]>
Date: Sun Jul 31 09:49:23 2022 -0700
perf symbol: Fail to read phdr workaround
The perf jvmti agent doesn't create program headers, in this case
fallback on section headers as happened previously.
Committer notes:
To test this, from a public post by Ian:
1) download a Java workload dacapo-9.12-MR1-bach.jar from
https://sourceforge.net/projects/dacapobench/
2) build perf such as "make -C tools/perf O=/tmp/perf NO_LIBBFD=1" it
should detect Java and create /tmp/perf/libperf-jvmti.so
3) run perf with the jvmti agent:
perf record -k 1 java -agentpath:/tmp/perf/libperf-jvmti.so -jar dacapo-9.12-MR1-bach.jar -n 10 fop
4) run perf inject:
perf inject -i perf.data -o perf-injected.data -j
5) run perf report
perf report -i perf-injected.data | grep org.apache.fop
With this patch reverted I see lots of symbols like:
0.00% java jitted-388040-4656.so [.] org.apache.fop.fo.FObj.bind(org.apache.fop.fo.PropertyList)
With the patch (2d86612aacb7805f ("perf symbol: Correct address for bss
symbols")) I see lots of:
dso__load_sym_internal: failed to find program header for symbol:
Lorg/apache/fop/fo/FObj;bind(Lorg/apache/fop/fo/PropertyList;)V
st_value: 0x40
Fixes: 2d86612aacb7805f ("perf symbol: Correct address for bss symbols")
Reviewed-by: Leo Yan <[email protected]>
Signed-off-by: Ian Rogers <[email protected]>
Tested-by: Leo Yan <[email protected]>
Cc: Alexander Shishkin <[email protected]>
Cc: Jiri Olsa <[email protected]>
Cc: Leo Yan <[email protected]>
Cc: Mark Rutland <[email protected]>
Cc: Namhyung Kim <[email protected]>
Cc: Peter Zijlstra <[email protected]>
Cc: Stephane Eranian <[email protected]>
Link: http://lore.kernel.org/lkml/[email protected]
Signed-off-by: Arnaldo Carvalho de Melo <[email protected]>
diff --git a/tools/perf/util/symbol-elf.c b/tools/perf/util/symbol-elf.c
index b3be5b1d9dbb00bc..75bec32d4f571319 100644
--- a/tools/perf/util/symbol-elf.c
+++ b/tools/perf/util/symbol-elf.c
@@ -1305,16 +1305,29 @@ dso__load_sym_internal(struct dso *dso, struct map *map, struct symsrc *syms_ss,
if (elf_read_program_header(syms_ss->elf,
(u64)sym.st_value, &phdr)) {
- pr_warning("%s: failed to find program header for "
+ pr_debug4("%s: failed to find program header for "
"symbol: %s st_value: %#" PRIx64 "\n",
__func__, elf_name, (u64)sym.st_value);
- continue;
+ pr_debug4("%s: adjusting symbol: st_value: %#" PRIx64 " "
+ "sh_addr: %#" PRIx64 " sh_offset: %#" PRIx64 "\n",
+ __func__, (u64)sym.st_value, (u64)shdr.sh_addr,
+ (u64)shdr.sh_offset);
+ /*
+ * Fail to find program header, let's rollback
+ * to use shdr.sh_addr and shdr.sh_offset to
+ * calibrate symbol's file address, though this
+ * is not necessary for normal C ELF file, we
+ * still need to handle java JIT symbols in this
+ * case.
+ */
+ sym.st_value -= shdr.sh_addr - shdr.sh_offset;
+ } else {
+ pr_debug4("%s: adjusting symbol: st_value: %#" PRIx64 " "
+ "p_vaddr: %#" PRIx64 " p_offset: %#" PRIx64 "\n",
+ __func__, (u64)sym.st_value, (u64)phdr.p_vaddr,
+ (u64)phdr.p_offset);
+ sym.st_value -= phdr.p_vaddr - phdr.p_offset;
}
- pr_debug4("%s: adjusting symbol: st_value: %#" PRIx64 " "
- "p_vaddr: %#" PRIx64 " p_offset: %#" PRIx64 "\n",
- __func__, (u64)sym.st_value, (u64)phdr.p_vaddr,
- (u64)phdr.p_offset);
- sym.st_value -= phdr.p_vaddr - phdr.p_offset;
}
demangled = demangle_sym(dso, kmodule, elf_name);
On Mon, Aug 01, 2022 at 09:38:23AM -0300, Arnaldo Carvalho de Melo wrote:
> Em Sun, Jul 31, 2022 at 11:19:15PM -0700, Ian Rogers escreveu:
> > On Sun, Jul 31, 2022, 6:53 PM Leo Yan <[email protected]> wrote:
> >
> > > On Sun, Jul 31, 2022 at 09:49:23AM -0700, Ian Rogers wrote:
> > > > The perf jvmti agent doesn't create program headers, in this case
> > > > fallback on section headers as happened previously.
> > > >
> > > > Fixes: 882528d2e776 ("perf symbol: Skip symbols if SHF_ALLOC flag is not
> > > set")
> > >
> > > It's good to change fix tag as:
> > > Fixes: 2d86612aacb7 ("perf symbol: Correct address for bss symbols")
> > >
> >
> > Doh! I was rushing this morning. Thanks for catching and reviewing!
>
> I made the adjustments and added a note with the repro, to help in the
> future when trying to test this area.
Thanks, Arnaldo.
> I also think we could have something like a 'perf test' mode where, when
> asked to, it would enable tests that involve downloading such files to
> perform tests, such as this dacapo benchmark, and then would test if the
> output matches expectations.
I will add a testing based on the steps, alongside with the discussed
testing for data symbols. Will share out after get ready.
Thanks,
Leo
Em Mon, Aug 01, 2022 at 09:25:11PM +0800, Leo Yan escreveu:
> On Mon, Aug 01, 2022 at 09:38:23AM -0300, Arnaldo Carvalho de Melo wrote:
> > Em Sun, Jul 31, 2022 at 11:19:15PM -0700, Ian Rogers escreveu:
> > > On Sun, Jul 31, 2022, 6:53 PM Leo Yan <[email protected]> wrote:
> > >
> > > > On Sun, Jul 31, 2022 at 09:49:23AM -0700, Ian Rogers wrote:
> > > > > The perf jvmti agent doesn't create program headers, in this case
> > > > > fallback on section headers as happened previously.
> > > > >
> > > > > Fixes: 882528d2e776 ("perf symbol: Skip symbols if SHF_ALLOC flag is not
> > > > set")
> > > >
> > > > It's good to change fix tag as:
> > > > Fixes: 2d86612aacb7 ("perf symbol: Correct address for bss symbols")
> > > >
> > >
> > > Doh! I was rushing this morning. Thanks for catching and reviewing!
> >
> > I made the adjustments and added a note with the repro, to help in the
> > future when trying to test this area.
>
> Thanks, Arnaldo.
>
> > I also think we could have something like a 'perf test' mode where, when
> > asked to, it would enable tests that involve downloading such files to
> > perform tests, such as this dacapo benchmark, and then would test if the
> > output matches expectations.
>
> I will add a testing based on the steps, alongside with the discussed
> testing for data symbols. Will share out after get ready.
Great! Thanks in advance,
- Arnaldo
Hi Arnaldo, Ian,
On Mon, Aug 01, 2022 at 09:38:23AM -0300, Arnaldo Carvalho de Melo wrote:
[...]
> I also think we could have something like a 'perf test' mode where, when
> asked to, it would enable tests that involve downloading such files to
> perform tests, such as this dacapo benchmark, and then would test if the
> output matches expectations.
I am working on testing script for java symbols, one of the steps shared by
Ian is:
# /tmp/perf/perf record -k 1 java -agentpath:/tmp/perf/libperf-jvmti.so \
-jar dacapo-9.12-MR1-bach.jar -n 10 fop
The question is how we can specify the path for the lib
libperf-jvmti.so in the testing script?
If we can run the test case from the root folder of Linux kernel
source code, the lib libperf-jvmti.so can be found in the folder
$linux/tools/perf, but for the integration testing the lib should be
placed in an installed folder. Any suggestion if we have exited
way to specify the path for libperf-jvmti.so, or need to introduce a
new shell envorinment variable for the lib path?
Thanks,
Leo
On Wed, Aug 3, 2022 at 8:25 AM Leo Yan <[email protected]> wrote:
>
> Hi Arnaldo, Ian,
>
> On Mon, Aug 01, 2022 at 09:38:23AM -0300, Arnaldo Carvalho de Melo wrote:
>
> [...]
>
> > I also think we could have something like a 'perf test' mode where, when
> > asked to, it would enable tests that involve downloading such files to
> > perform tests, such as this dacapo benchmark, and then would test if the
> > output matches expectations.
>
> I am working on testing script for java symbols, one of the steps shared by
> Ian is:
>
> # /tmp/perf/perf record -k 1 java -agentpath:/tmp/perf/libperf-jvmti.so \
> -jar dacapo-9.12-MR1-bach.jar -n 10 fop
>
> The question is how we can specify the path for the lib
> libperf-jvmti.so in the testing script?
>
> If we can run the test case from the root folder of Linux kernel
> source code, the lib libperf-jvmti.so can be found in the folder
> $linux/tools/perf, but for the integration testing the lib should be
> placed in an installed folder. Any suggestion if we have exited
> way to specify the path for libperf-jvmti.so, or need to introduce a
> new shell envorinment variable for the lib path?
There is a hack in 'perf test' where we assume a few paths to tests:
https://git.kernel.org/pub/scm/linux/kernel/git/acme/linux.git/tree/tools/perf/tests/builtin-test.c?h=perf/core#n308
so in this case we could look in the same directory. There is also a
#define for PERF_EXEC_PATH.
I'd prefer it if the test could be self contained for example:
echo "int fib(int x) { return x > 1 ? fib(x - 2) + fib(x - 1) : 1; }
int q = 0; for(int i=0; i < 10; i++) q += fib(i);
System.out.println(q);" | /tmp/perf/perf record -k 1 jshell
-J-agentpath:/tmp/perf/libperf-jvmti.so
where jshell runs on the JVM and so we should get some jitted execution time.
Thanks,
Ian
> Thanks,
> Leo
On Wed, Aug 03, 2022 at 05:26:39PM -0700, Ian Rogers wrote:
[...]
> > The question is how we can specify the path for the lib
> > libperf-jvmti.so in the testing script?
> >
> > If we can run the test case from the root folder of Linux kernel
> > source code, the lib libperf-jvmti.so can be found in the folder
> > $linux/tools/perf, but for the integration testing the lib should be
> > placed in an installed folder. Any suggestion if we have exited
> > way to specify the path for libperf-jvmti.so, or need to introduce a
> > new shell envorinment variable for the lib path?
>
> There is a hack in 'perf test' where we assume a few paths to tests:
> https://git.kernel.org/pub/scm/linux/kernel/git/acme/linux.git/tree/tools/perf/tests/builtin-test.c?h=perf/core#n308
> so in this case we could look in the same directory. There is also a
> #define for PERF_EXEC_PATH.
Thanks for the info. I will try to search paths like the buildin-test.c
file, and the lib libperf-jvmti.so is installed in the folder
"$HOME/lib64/", I will check if can reuse PERF_EXEC_PATH for this
case.
> I'd prefer it if the test could be self contained for example:
>
> echo "int fib(int x) { return x > 1 ? fib(x - 2) + fib(x - 1) : 1; }
> int q = 0; for(int i=0; i < 10; i++) q += fib(i);
> System.out.println(q);" | /tmp/perf/perf record -k 1 jshell
> -J-agentpath:/tmp/perf/libperf-jvmti.so
>
> where jshell runs on the JVM and so we should get some jitted execution time.
Will do. Appreciate for sharing this!
Leo