linux-perf-users.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/5 v1] Processing java JIT dumps from containers
@ 2024-12-06 20:48 Arnaldo Carvalho de Melo
  2024-12-06 20:48 ` [PATCH 1/5] perf config: Fix trival typo 'an' -> 'can' Arnaldo Carvalho de Melo
                   ` (5 more replies)
  0 siblings, 6 replies; 8+ messages in thread
From: Arnaldo Carvalho de Melo @ 2024-12-06 20:48 UTC (permalink / raw)
  To: Stephane Eranian, Yonatan Goldschmidt
  Cc: Francesco Nigro, Ilan Green, Namhyung Kim, Ingo Molnar,
	Thomas Gleixner, James Clark, Jiri Olsa, Ian Rogers,
	Adrian Hunter, Kan Liang, Clark Williams, linux-kernel,
	linux-perf-users, Arnaldo Carvalho de Melo

From: Arnaldo Carvalho de Melo <acme@redhat.com>

Hi,

	This is trying to move the needle on supporting jitdump done
from a container while perf is running from the outside, that is not
working right now.

	I tried to collect as much details on what is being done in the
commit logs to document further work that needs to be done to support
such a scenario in a streamlined way, but what in this patchkit at least
helped in a real case, allowing us to get 'perf annotate' to work and
confirm suspicions.

- Arnaldo

Arnaldo Carvalho de Melo (5):
  perf config: Fix trival typo 'an' -> 'can'
  perf jitdump: Accept jitdump mmaps emitted from inside containers
  perf namespaces: Introduce nsinfo__set_in_pidns()
  perf jitdump: Fixup in_pidns member when java agent and 'perf record' are not in the same pidns
  perf namespaces: Fixup the nsinfo__in_pidns() return type, its bool

 tools/perf/Documentation/perf-config.txt |  2 +-
 tools/perf/util/jitdump.c                | 15 ++++++++++++---
 tools/perf/util/namespaces.c             |  7 ++++++-
 tools/perf/util/namespaces.h             |  3 ++-
 4 files changed, 21 insertions(+), 6 deletions(-)

-- 
2.47.0

^ permalink raw reply	[flat|nested] 8+ messages in thread

* [PATCH 1/5] perf config: Fix trival typo 'an' -> 'can'
  2024-12-06 20:48 [PATCH 0/5 v1] Processing java JIT dumps from containers Arnaldo Carvalho de Melo
@ 2024-12-06 20:48 ` Arnaldo Carvalho de Melo
  2024-12-06 20:48 ` [PATCH 2/5] perf jitdump: Accept jitdump mmaps emitted from inside containers Arnaldo Carvalho de Melo
                   ` (4 subsequent siblings)
  5 siblings, 0 replies; 8+ messages in thread
From: Arnaldo Carvalho de Melo @ 2024-12-06 20:48 UTC (permalink / raw)
  To: Stephane Eranian, Yonatan Goldschmidt
  Cc: Francesco Nigro, Ilan Green, Namhyung Kim, Ingo Molnar,
	Thomas Gleixner, James Clark, Jiri Olsa, Ian Rogers,
	Adrian Hunter, Kan Liang, Clark Williams, linux-kernel,
	linux-perf-users, Arnaldo Carvalho de Melo

From: Arnaldo Carvalho de Melo <acme@redhat.com>

Just a trivial typo, should be 'can', did a spell check on the rest of
the file just in case, nothing more stood out.

Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/Documentation/perf-config.txt | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/perf/Documentation/perf-config.txt b/tools/perf/Documentation/perf-config.txt
index 1f668d4724e3749a..36ebebc875ea2d09 100644
--- a/tools/perf/Documentation/perf-config.txt
+++ b/tools/perf/Documentation/perf-config.txt
@@ -40,7 +40,7 @@ The '$HOME/.perfconfig' file is used to store a per-user configuration.
 The file '$(sysconfdir)/perfconfig' can be used to
 store a system-wide default configuration.
 
-One an disable reading config files by setting the PERF_CONFIG environment
+One can disable reading config files by setting the PERF_CONFIG environment
 variable to /dev/null, or provide an alternate config file by setting that
 variable.
 
-- 
2.47.0


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [PATCH 2/5] perf jitdump: Accept jitdump mmaps emitted from inside containers
  2024-12-06 20:48 [PATCH 0/5 v1] Processing java JIT dumps from containers Arnaldo Carvalho de Melo
  2024-12-06 20:48 ` [PATCH 1/5] perf config: Fix trival typo 'an' -> 'can' Arnaldo Carvalho de Melo
@ 2024-12-06 20:48 ` Arnaldo Carvalho de Melo
  2024-12-06 20:48 ` [PATCH 3/5] perf namespaces: Introduce nsinfo__set_in_pidns() Arnaldo Carvalho de Melo
                   ` (3 subsequent siblings)
  5 siblings, 0 replies; 8+ messages in thread
From: Arnaldo Carvalho de Melo @ 2024-12-06 20:48 UTC (permalink / raw)
  To: Stephane Eranian, Yonatan Goldschmidt
  Cc: Francesco Nigro, Ilan Green, Namhyung Kim, Ingo Molnar,
	Thomas Gleixner, James Clark, Jiri Olsa, Ian Rogers,
	Adrian Hunter, Kan Liang, Clark Williams, linux-kernel,
	linux-perf-users, Arnaldo Carvalho de Melo

From: Arnaldo Carvalho de Melo <acme@redhat.com>

When the java agent is running inside a container it will emit mmaps
with the format:

  ⬢ [acme@toolbox a]$ perf report -D | grep PERF_RECORD_MMAP | grep \.dump
  0 0x15c400 [0x90]: PERF_RECORD_MMAP2 3308868/3308868: [0x7fb8de6cb000(0x1000) @ 0 08:14 3222905945 0]: r-xp /tmp/.debug/jit/java-jit-20241126.XXTxEIOn/jit-1.dump
  ⬢ [acme@toolbox a]$

Since perf is running from outside the container it sees the pid 3308868
in PERF_RECORD_MMAP2, while the agent saw the pid of the profiled app
inside the container, 1.

The previous validation was:

  if (pid && pid2 != nsinfo__nstgid(nsi))

pid2 at this point is '1' (/jit-1.dump), so it considers this as a
malformed jitdump mmap and refuses to process it.

The test ends up as:

  if (3308868 && 1 != 3308868)

which is true and the jitdump is not processed.

Since 1 in the container namespace is really 3308868 in the namespace
that perf is running, consider this a valid mmap.

We need to make perf realize this and behave accordingly, for now
checking instead:

  if (pid && pid2 && pid != nsinfo__nstgid(nsi))

Translating to:

  if (3308868 && 1 && 3308868 != 3308868)

Will make the jitdump mmap to be considered valid and processed.

The jitdump is described in:

  https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/tools/perf/Documentation/jitdump-specification.txt

Now we end up with the expected flurry of MMAPs, one per jitted function
transformed into a little ELF file that should then be processable by
the other perf features, like code annotation:

  [acme@toolbox a]$ echo $JITDUMPDIR
  /tmp/.debug/jit
  [acme@toolbox a]$

First use 'perf inject':

  ⬢ [acme@toolbox a]$ time perf inject -i perf.data -o acme-perf-injected.data -j

Then look at the PERF_RECORD_MMAP events in the result file, that went
thru the JIT map file:

  ⬢ [acme@toolbox a]$ ls -la /tmp/*.map
  -rw-r--r--. 1 acme acme 2989559 Nov 27 16:11 /tmp/perf-3308868.map
  [acme@toolbox a]$

It is a symbol table:

  ⬢ [acme@toolbox a]$ head /tmp/*.map
  0x00007fb8bda5c1a0 0x00000000000000d0 java.lang.String java.lang.module.ModuleDescriptor.name()
  0x00007fb8bda5c4a0 0x0000000000000178 int java.lang.StringLatin1.hashCode(byte[])
  0x00007fb8bda5c9a0 0x00000000000000d0 java.lang.String org.springframework.boot.context.config.ConfigDataLocation.getValue()
  0x00007fb8bda5cca0 0x00000000000000d0 java.lang.module.ModuleDescriptor java.lang.module.ModuleReference.descriptor()
  0x00007fb8bda5cfa0 0x00000000000000d0 java.lang.Object java.util.KeyValueHolder.getKey()
  0x00007fb8bda5d2a0 0x00000000000000d0 java.lang.Object java.util.KeyValueHolder.getValue()
  0x00007fb8bda5d5a0 0x0000000000000218 boolean jdk.internal.misc.Unsafe.compareAndSetReference(java.lang.Object, long, java.lang.Object, java.lang.Object)
  0x00007fb8bda5d9a0 0x00000000000001f0 boolean jdk.internal.misc.Unsafe.compareAndSetLong(java.lang.Object, long, long, long)
  0x00007fb8bda5dda0 0x00000000000001f8 void java.lang.System.arraycopy(java.lang.Object, int, java.lang.Object, int, int)
  0x00007fb8bda5e1a0 0x00000000000001e8 int java.lang.Object.hashCode()
  ⬢ [acme@toolbox a]$

As specified in:

  https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/tools/perf/Documentation/jit-interface.txt

This was collected from inside the container, so came as
/tmp/perf-1.map.

To make perf, running outside the container to use it we need to copy it
to /tmp/perf-3308868.map.

This is another logic that has to be added to perf to work on this
scenario of running outside the container but processing things created
by the hava agent running inside the container.

With all this in place we get to:

  ⬢ [acme@toolbox a]$ perf report -D -i acme-perf-injected.data | \
  			grep PERF_RECORD_MMAP > /tmp/acme-perf-injected.data.mmaps ; \
  			wc -l /tmp/acme-perf-injected.data.mmaps
  44182 /tmp/acme-perf-injected.data.mmaps
  ⬢ [acme@toolbox a]$ tail /tmp/acme-perf-injected.data.mmaps
  1030266786574466 0x7bc9e0 [0x98]: PERF_RECORD_MMAP2 1/78: [0x7fb8c0ceb1c0(0x8d0) @ 0x80 00:2c 238715 1]: --xs /tmp/.debug/jit/java-jit-20241126.XXTxEIOn/jitted-1-43989.so
  1030266795288774 0x7bca78 [0x98]: PERF_RECORD_MMAP2 1/78: [0x7fb8c0cecc00(0x7e8) @ 0x80 00:2c 238716 1]: --xs /tmp/.debug/jit/java-jit-20241126.XXTxEIOn/jitted-1-43990.so
  1030266895967339 0x7bcb10 [0x98]: PERF_RECORD_MMAP2 1/78: [0x7fb8c0cee500(0x3328) @ 0x80 00:2c 238717 1]: --xs /tmp/.debug/jit/java-jit-20241126.XXTxEIOn/jitted-1-43991.so
  1030266915748306 0x7bcba8 [0x98]: PERF_RECORD_MMAP2 1/78: [0x7fb8c0aae0a0(0x138) @ 0x80 00:2c 238718 1]: --xs /tmp/.debug/jit/java-jit-20241126.XXTxEIOn/jitted-1-43992.so
  1030267185851220 0x7bcc40 [0x98]: PERF_RECORD_MMAP2 1/78: [0x7fb8c0cf61e0(0x3b50) @ 0x80 00:2c 238719 1]: --xs /tmp/.debug/jit/java-jit-20241126.XXTxEIOn/jitted-1-43993.so
  1030267231364524 0x7bccd8 [0x98]: PERF_RECORD_MMAP2 1/78: [0x7fb8c0cfea80(0x14a0) @ 0x80 00:2c 238720 1]: --xs /tmp/.debug/jit/java-jit-20241126.XXTxEIOn/jitted-1-43994.so
  1030267425498831 0x7bcd70 [0x98]: PERF_RECORD_MMAP2 1/78: [0x7fb8c054b4a0(0x338) @ 0x80 00:2c 238721 1]: --xs /tmp/.debug/jit/java-jit-20241126.XXTxEIOn/jitted-1-43995.so
  1030267506147888 0x7bce08 [0x98]: PERF_RECORD_MMAP2 1/78: [0x7fb8c0a995c0(0x1e8) @ 0x80 00:2c 238722 1]: --xs /tmp/.debug/jit/java-jit-20241126.XXTxEIOn/jitted-1-43996.so
  1030268112586116 0x7bcea0 [0x98]: PERF_RECORD_MMAP2 1/78: [0x7fb8c0d02520(0x258) @ 0x80 00:2c 238723 1]: --xs /tmp/.debug/jit/java-jit-20241126.XXTxEIOn/jitted-1-43997.so
  1030269435398150 0x7bcf38 [0x98]: PERF_RECORD_MMAP2 1/78: [0x7fb8c0d02dc0(0x278) @ 0x80 00:2c 238724 1]: --xs /tmp/.debug/jit/java-jit-20241126.XXTxEIOn/jitted-1-43998.so
  ⬢ [acme@toolbox a]$

And if we look at those tiny ELF files generated by the jitdump code
used by 'perf inject' we see:

  ⬢ [acme@toolbox a]$ file /tmp/.debug/jit/java-jit-20241126.XXTxEIOn/jitted-1-43989.so
  /tmp/.debug/jit/java-jit-20241126.XXTxEIOn/jitted-1-43989.so: ELF 64-bit LSB shared object, x86-64, version 1 (SYSV), statically linked, BuildID[sha1]=790591db95a77d644657dfe5058658b200000000, with debug_info, not stripped
  ⬢ [acme@toolbox a]$ file /tmp/.debug/jit/java-jit-20241126.XXTxEIOn/jitted-1-43990.so
  /tmp/.debug/jit/java-jit-20241126.XXTxEIOn/jitted-1-43990.so: ELF 64-bit LSB shared object, x86-64, version 1 (SYSV), statically linked, BuildID[sha1]=762f932acbee53a22638bf4c2b86780200000000, with debug_info, not stripped
  ⬢ [acme@toolbox a]$

  ⬢ [acme@toolbox a]$ ls -la /tmp/.debug/jit/java-jit-20241126.XXTxEIOn/jitted-1-43989.so /tmp/.debug/jit/java-jit-20241126.XXTxEIOn/jitted-1-43990.so
  -rw-r--r--. 1 acme acme 9432 Nov 29 10:56 /tmp/.debug/jit/java-jit-20241126.XXTxEIOn/jitted-1-43989.so
  -rw-r--r--. 1 acme acme 7504 Nov 29 10:56 /tmp/.debug/jit/java-jit-20241126.XXTxEIOn/jitted-1-43990.so
  ⬢ [acme@toolbox a]$

And:

  ⬢ [acme@toolbox a]$ objdump -dS /tmp/.debug/jit/java-jit-20241126.XXTxEIOn/jitted-1-43990.so | head -20

  /tmp/.debug/jit/java-jit-20241126.XXTxEIOn/jitted-1-43990.so:     file format elf64-x86-64

  Disassembly of section .text:

  0000000000000080 <Lredacted/REDACTED/REDACTED/logging/RedactedRedacted;Redacted(Lredacted/REDACTED/REDACTED/redactedRedacted/Redacted;)V>:
    80:	44 8b 56 08          	mov    0x8(%rsi),%r10d
    84:	49 c1 e2 03          	shl    $0x3,%r10
    88:	49 3b c2             	cmp    %r10,%rax
    8b:	0f 85 6f 15 83 fc    	jne    fffffffffc831600 <Lredacted/REDACTED/REDACTED/redacted/RedactedRedactedRedacted;Redacted(Lredacted/Redacted/Redacted/redactedRedacted/Redacted;)V+0xfffffffffc831580>
    91:	66 66 90             	data16 xchg %ax,%ax
    94:	0f 1f 84 00 00 00 00 	nopl   0x0(%rax,%rax,1)
    9b:	00
    9c:	66 66 66 90          	data16 data16 xchg %ax,%ax
    a0:	89 84 24 00 c0 fe ff 	mov    %eax,-0x14000(%rsp)
    a7:	55                   	push   %rbp
    a8:	48 8b ec             	mov    %rsp,%rbp
    ab:	48 83 ec 40          	sub    $0x40,%rsp
    af:	48 89 34 24          	mov    %rsi,(%rsp)
  ⬢ [acme@toolbox a]$

The thing now being investigated is why we can't annotate anything here,
maybe that JITDUMPDIR is getting in the way:

  ⬢ [acme@toolbox a]$ perf annotate --stdio2 -i acme-perf-injected.data 'java.lang.String com.fasterxml.jackson.core.sym.CharsToNameCanonicalizer.findSymbol(char[], int, int, int)'
  Error:
  Couldn't annotate java.lang.String com.fasterxml.jackson.core.sym.CharsToNameCanonicalizer.findSymbol(char[], int, int, int):
  Internal error: Invalid -1 error code
  ⬢ [acme@toolbox a]$

In the tests I performed while merging this patch:

  https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=6d518ac7be6223811ab947897273b1bbef846180

It works, but then there was no JITDUMPDIR involved:

  /home/acme/.debug/jit/java-jit-20241127.XXF1SRgN/jitted-3912413-4191.so

⬢ [acme@toolbox perf-tools-next]$ perf report --call-graph none --no-child -i perf-injected.data | grep jitted- | head
     1.36%  java             jitted-3912413-54.so    [.] Interpreter
     0.30%  C1 CompilerThre  jitted-3912413-1.so     [.] flush_icache_stub
     0.18%  java             jitted-3912413-4184.so  [.] org.apache.fop.fo.properties.PropertyMaker.get(int, org.apache.fop.fo.PropertyList, boolean, boolean)
     0.18%  java             jitted-3912413-4177.so  [.] org.apache.fop.layoutmgr.inline.TextLayoutManager.getNextKnuthElements(org.apache.fop.layoutmgr.LayoutContext, int)
     0.13%  java             jitted-3912413-3845.so  [.] java.text.DecimalFormat.subformatNumber(java.lang.StringBuffer, java.text.Format$FieldDelegate, boolean, boolean, int, int, int, int)
     0.11%  java             jitted-3912413-4191.so  [.] org.apache.fop.fo.FObj.addChildNode(org.apache.fop.fo.FONode)
     0.09%  java             jitted-3912413-2418.so  [.] org.apache.fop.fo.XMLWhiteSpaceHandler.handleWhiteSpace()
     0.08%  Reference Handl  jitted-3912413-54.so    [.] Interpreter
     0.08%  java             jitted-3912413-3326.so  [.] org.apache.xmlgraphics.fonts.Glyphs.stringToGlyph(java.lang.String)
     0.08%  java             jitted-3912413-3953.so  [.] org.apache.fop.layoutmgr.BreakingAlgorithm.considerLegalBreak(org.apache.fop.layoutmgr.KnuthElement, int)
⬢ [acme@toolbox perf-tools-next]$

And then:

  ⬢ [acme@toolbox perf-tools-next]$ perf annotate --stdio2 -i perf-injected.data 'org.apache.fop.layoutmgr.inline.TextLayoutManager.getNextKnuthElements(org.apache.fop.layoutmgr.LayoutContext, int)' | head -20
  Samples: 8  of event 'cpu_atom/cycles/Pu', 4000 Hz, Event count (approx.): 8112794, [percent: local period]
  org.apache.fop.layoutmgr.inline.TextLayoutManager.getNextKnuthElements(org.apache.fop.layoutmgr.LayoutContext, int)() /home/acme/.debug/jit/java-jit-20241127.XXF1SRgN/jitted-3912413-4177.so
  Percent        0x80 <org.apache.fop.layoutmgr.inline.TextLayoutManager.getNextKnuthElements(org.apache.fop.layoutmgr.LayoutContext, int)>:
                   nop
                   movl       0x8(%rsi),%r10d
                   cmpl       0x8(%rax),%r10d
                 → jne        0
                   movl       %eax,-0x14000(%rsp)
                   pushq      %rbp
                   subq       $0xb0,%rsp
                   nop
                   cmpl       $0x3,0x20(%r15)
                 ↓ jne        7037
             2e:   movl       %ecx,0x28(%rsp)
                   movq       %rdx,%rbp
                   movl       0x64(%rdx),%ebx
                   cmpb       $0x0,0x38(%r15)
                 ↓ jne        3a44
                   movq       %rsi,0x30(%rsp)
             48:   movq       0x30(%rsp),%r10
  ⬢ [acme@toolbox perf-tools-next]$

No source code nor line numbers, that I saw in another build of perf for
RHEL9, for the same workload described in the cset above (a publicly
available java benchmark), so something to investigate on perf upstream
running on fedora, maybe some quirk with the jdk used when building perf
for RHEL 9 and for Fedora 40.

A related patch that should have make this all work is:

  "perf inject jit: Add namespaces support"
  https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=67dec926931448d688efb5fe34f7b5a22470fc0a

But we still need to polish this some more, maybe there are differences
in the agent used in NodeJS with --perf-prof and the jvmti one we're
using.

Hopefully describing all the steps while we investigate this case will
help us improve perf support for profiling JITed environments running in
containers while profiling from inside and outside it.

Reported-by: Francesco Nigro <fnigro@redhat.com>
Reported-by: Ilan Green <igreen@redhat.com>
Cc: Adrian Hunter <adrian.hunter@intel.com>
Cc: Ian Rogers <irogers@google.com>
Cc: James Clark <james.clark@linaro.org>
Cc: Jiri Olsa <jolsa@kernel.org>
Cc: Kan Liang <kan.liang@linux.intel.com>
Cc: Namhyung Kim <namhyung@kernel.org>
Cc: Stephane Eranian <eranian@google.com>
Cc: Yonatan Goldschmidt <yonatan.goldschmidt@granulate.io>
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/util/jitdump.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/perf/util/jitdump.c b/tools/perf/util/jitdump.c
index 346513e5e9b7a956..d53c6ac4095b663c 100644
--- a/tools/perf/util/jitdump.c
+++ b/tools/perf/util/jitdump.c
@@ -777,7 +777,7 @@ jit_detect(const char *mmap_name, pid_t pid, struct nsinfo *nsi)
 	 * pid does not match mmap pid
 	 * pid==0 in system-wide mode (synthesized)
 	 */
-	if (pid && pid2 != nsinfo__nstgid(nsi))
+	if (pid && pid2 && pid != nsinfo__nstgid(nsi))
 		return -1;
 	/*
 	 * validate suffix
-- 
2.47.0


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [PATCH 3/5] perf namespaces: Introduce nsinfo__set_in_pidns()
  2024-12-06 20:48 [PATCH 0/5 v1] Processing java JIT dumps from containers Arnaldo Carvalho de Melo
  2024-12-06 20:48 ` [PATCH 1/5] perf config: Fix trival typo 'an' -> 'can' Arnaldo Carvalho de Melo
  2024-12-06 20:48 ` [PATCH 2/5] perf jitdump: Accept jitdump mmaps emitted from inside containers Arnaldo Carvalho de Melo
@ 2024-12-06 20:48 ` Arnaldo Carvalho de Melo
  2024-12-06 20:48 ` [PATCH 4/5] perf jitdump: Fixup in_pidns member when java agent and 'perf record' are not in the same pidns Arnaldo Carvalho de Melo
                   ` (2 subsequent siblings)
  5 siblings, 0 replies; 8+ messages in thread
From: Arnaldo Carvalho de Melo @ 2024-12-06 20:48 UTC (permalink / raw)
  To: Stephane Eranian, Yonatan Goldschmidt
  Cc: Francesco Nigro, Ilan Green, Namhyung Kim, Ingo Molnar,
	Thomas Gleixner, James Clark, Jiri Olsa, Ian Rogers,
	Adrian Hunter, Kan Liang, Clark Williams, linux-kernel,
	linux-perf-users, Arnaldo Carvalho de Melo

From: Arnaldo Carvalho de Melo <acme@redhat.com>

When we're processing a perf.data file we will, for every thread in that
file do a machine__findnew_thread(machine, pid, tid) that when that pid
is seen for the first time will create a 'struct thread' representing
it.

That in turn will call nsinfo__new() -> nsinfo__init() and there it will
assume we're running live, which is wrong and will need to be addressed
in a followup patch.

The nsinfo__new() assumes that if we can't access that thread it has
already finished and will ignore the -1 return from nsinfo__init(), just
taking notes to avoid trying to enter in that namespace, since it isn't
there anymore, a race.

When doing this from 'perf inject', tho, we can fill in parts of that
nsinfo from what we get from the PERF_RECORD_MMAP2 (pid, tid) and in the
jitdump file name, that has the form of jit-<PID>.dump.

So if the pid in the jitdump file name is not the one in the
PERF_RECORD_MMAP2, we can assume that its the pid of the process
_inside_ the namespace, and that perf was runing outside that namespace.

This will be done in the following patch.

Reported-by: Francesco Nigro <fnigro@redhat.com>
Reported-by: Ilan Green <igreen@redhat.com>
Cc: Adrian Hunter <adrian.hunter@intel.com>
Cc: Ian Rogers <irogers@google.com>
Cc: James Clark <james.clark@linaro.org>
Cc: Jiri Olsa <jolsa@kernel.org>
Cc: Kan Liang <kan.liang@linux.intel.com>
Cc: Namhyung Kim <namhyung@kernel.org>
Cc: Stephane Eranian <eranian@google.com>
Cc: Yonatan Goldschmidt <yonatan.goldschmidt@granulate.io>
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/util/namespaces.c | 5 +++++
 tools/perf/util/namespaces.h | 1 +
 2 files changed, 6 insertions(+)

diff --git a/tools/perf/util/namespaces.c b/tools/perf/util/namespaces.c
index cb185c5659d6b323..36047184d76e2f80 100644
--- a/tools/perf/util/namespaces.c
+++ b/tools/perf/util/namespaces.c
@@ -271,6 +271,11 @@ pid_t nsinfo__in_pidns(const struct nsinfo  *nsi)
 	return RC_CHK_ACCESS(nsi)->in_pidns;
 }
 
+void nsinfo__set_in_pidns(struct nsinfo *nsi)
+{
+	RC_CHK_ACCESS(nsi)->in_pidns = true;
+}
+
 void nsinfo__mountns_enter(struct nsinfo *nsi,
 				  struct nscookie *nc)
 {
diff --git a/tools/perf/util/namespaces.h b/tools/perf/util/namespaces.h
index 8c0731c6cbb7ee01..e014becb9cd8eb3a 100644
--- a/tools/perf/util/namespaces.h
+++ b/tools/perf/util/namespaces.h
@@ -59,6 +59,7 @@ pid_t nsinfo__tgid(const struct nsinfo  *nsi);
 pid_t nsinfo__nstgid(const struct nsinfo  *nsi);
 pid_t nsinfo__pid(const struct nsinfo  *nsi);
 pid_t nsinfo__in_pidns(const struct nsinfo  *nsi);
+void nsinfo__set_in_pidns(struct nsinfo *nsi);
 
 void nsinfo__mountns_enter(struct nsinfo *nsi, struct nscookie *nc);
 void nsinfo__mountns_exit(struct nscookie *nc);
-- 
2.47.0


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [PATCH 4/5] perf jitdump: Fixup in_pidns member when java agent and 'perf record' are not in the same pidns
  2024-12-06 20:48 [PATCH 0/5 v1] Processing java JIT dumps from containers Arnaldo Carvalho de Melo
                   ` (2 preceding siblings ...)
  2024-12-06 20:48 ` [PATCH 3/5] perf namespaces: Introduce nsinfo__set_in_pidns() Arnaldo Carvalho de Melo
@ 2024-12-06 20:48 ` Arnaldo Carvalho de Melo
  2024-12-06 20:48 ` [PATCH 5/5] perf namespaces: Fixup the nsinfo__in_pidns() return type, its bool Arnaldo Carvalho de Melo
  2024-12-23 19:51 ` [PATCH 0/5 v1] Processing java JIT dumps from containers Arnaldo Carvalho de Melo
  5 siblings, 0 replies; 8+ messages in thread
From: Arnaldo Carvalho de Melo @ 2024-12-06 20:48 UTC (permalink / raw)
  To: Stephane Eranian, Yonatan Goldschmidt
  Cc: Francesco Nigro, Ilan Green, Namhyung Kim, Ingo Molnar,
	Thomas Gleixner, James Clark, Jiri Olsa, Ian Rogers,
	Adrian Hunter, Kan Liang, Clark Williams, linux-kernel,
	linux-perf-users, Arnaldo Carvalho de Melo

From: Arnaldo Carvalho de Melo <acme@redhat.com>

When running 'perf record' outside a container and the java agent inside
a container the jit_repipe_code_load() and friends will emit
PERF_RECORD_MMAP2 entries for the jitdump records and will check if we
need to fixup the pid/tid:

        nspid = jr->load.pid;
        pid   = jr_entry_pid(jd, jr);
        tid   = jr_entry_tid(jd, jr);

The jr_entry_pid() function looks if we're in the same pidns:

  static pid_t jr_entry_pid(struct jit_buf_desc *jd, union jr_entry *jr)
  {
          if (jd->nsi && nsinfo__in_pidns(jd->nsi))
                  return nsinfo__tgid(jd->nsi);
          return jr->load.pid;
  }

But since the thread, populated from perf.data records, try to figure
out if in the same pidns by actually trying, on the system where 'perf
inject' is running to open a procfs file (a bug that remains to be
fixed), assuming that if it is not possible that is because that thread
terminated and thus we can't get its namespace info and tolerates
nsinfo__init() failing, noting only that that namespace can't be
entered, so don't even try.

But we can kinda get at least that info (thread->nsinfo->in_pidns) from
the data in the perf.data file, namely the pid and tid in the
PERF_RECORD_MMAP2 for the jit-<PID>.dump file generated from the java
agent, if the PERF_RECORD_MMAP2->pid is the same as what is in the
jitdump file, then we're in the same namespace, otherwise we need to use
the PERF_RECORD_MMAP2->pid.

This all has to be revamped for this jitdump + running perf from
outside, as the meaning of in_pidns is being abused, the initialization
of nsinfo->pid with the value coming from the PERF_RECORD_MMAP2 data is
wrong as it is the pid _outside_ the container since perf was running
there.

The hack in this patch at least produces the expected result in this
scenario by following the assumptions in the current codebase for
finding maps and for generating the PERF_RECORD_MMAP2 for the ELF files
synthesized from the jitdump records in jit_repipe_code_load(), etc.s

Reported-by: Francesco Nigro <fnigro@redhat.com>
Reported-by: Ilan Green <igreen@redhat.com>
Cc: Adrian Hunter <adrian.hunter@intel.com>
Cc: Ian Rogers <irogers@google.com>
Cc: James Clark <james.clark@linaro.org>
Cc: Jiri Olsa <jolsa@kernel.org>
Cc: Kan Liang <kan.liang@linux.intel.com>
Cc: Namhyung Kim <namhyung@kernel.org>
Cc: Stephane Eranian <eranian@google.com>
Cc: Yonatan Goldschmidt <yonatan.goldschmidt@granulate.io>
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/util/jitdump.c | 15 ++++++++++++---
 1 file changed, 12 insertions(+), 3 deletions(-)

diff --git a/tools/perf/util/jitdump.c b/tools/perf/util/jitdump.c
index d53c6ac4095b663c..f23e21502bf8381a 100644
--- a/tools/perf/util/jitdump.c
+++ b/tools/perf/util/jitdump.c
@@ -737,7 +737,7 @@ jit_inject(struct jit_buf_desc *jd, const char *path)
  * as captured in the RECORD_MMAP record
  */
 static int
-jit_detect(const char *mmap_name, pid_t pid, struct nsinfo *nsi)
+jit_detect(const char *mmap_name, pid_t pid, struct nsinfo *nsi, bool *in_pidns)
  {
 	char *p;
 	char *end = NULL;
@@ -773,11 +773,16 @@ jit_detect(const char *mmap_name, pid_t pid, struct nsinfo *nsi)
 	if (!end)
 		return -1;
 
+	*in_pidns = pid == nsinfo__nstgid(nsi);
 	/*
 	 * pid does not match mmap pid
 	 * pid==0 in system-wide mode (synthesized)
+	 *
+	 * If the pid in the file name is equal to the nstgid, then
+	 * the agent ran inside a container and perf outside the
+	 * container, so record it for further use in jit_inject().
 	 */
-	if (pid && pid2 && pid != nsinfo__nstgid(nsi))
+	if (pid && !(pid2 == pid || *in_pidns))
 		return -1;
 	/*
 	 * validate suffix
@@ -830,6 +835,7 @@ jit_process(struct perf_session *session,
 	struct nsinfo *nsi;
 	struct evsel *first;
 	struct jit_buf_desc jd;
+	bool in_pidns = false;
 	int ret;
 
 	thread = machine__findnew_thread(machine, pid, tid);
@@ -844,7 +850,7 @@ jit_process(struct perf_session *session,
 	/*
 	 * first, detect marker mmap (i.e., the jitdump mmap)
 	 */
-	if (jit_detect(filename, pid, nsi)) {
+	if (jit_detect(filename, pid, nsi, &in_pidns)) {
 		nsinfo__put(nsi);
 
 		/*
@@ -866,6 +872,9 @@ jit_process(struct perf_session *session,
 	jd.machine = machine;
 	jd.nsi = nsi;
 
+	if (in_pidns)
+		nsinfo__set_in_pidns(nsi);
+
 	/*
 	 * track sample_type to compute id_all layout
 	 * perf sets the same sample type to all events as of now
-- 
2.47.0


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [PATCH 5/5] perf namespaces: Fixup the nsinfo__in_pidns() return type, its bool
  2024-12-06 20:48 [PATCH 0/5 v1] Processing java JIT dumps from containers Arnaldo Carvalho de Melo
                   ` (3 preceding siblings ...)
  2024-12-06 20:48 ` [PATCH 4/5] perf jitdump: Fixup in_pidns member when java agent and 'perf record' are not in the same pidns Arnaldo Carvalho de Melo
@ 2024-12-06 20:48 ` Arnaldo Carvalho de Melo
  2024-12-23 19:51 ` [PATCH 0/5 v1] Processing java JIT dumps from containers Arnaldo Carvalho de Melo
  5 siblings, 0 replies; 8+ messages in thread
From: Arnaldo Carvalho de Melo @ 2024-12-06 20:48 UTC (permalink / raw)
  To: Stephane Eranian, Yonatan Goldschmidt
  Cc: Francesco Nigro, Ilan Green, Namhyung Kim, Ingo Molnar,
	Thomas Gleixner, James Clark, Jiri Olsa, Ian Rogers,
	Adrian Hunter, Kan Liang, Clark Williams, linux-kernel,
	linux-perf-users, Arnaldo Carvalho de Melo

From: Arnaldo Carvalho de Melo <acme@redhat.com>

When adding support for refconunt checking a cut'n'paste made this
function, that is just an accessor to a bool member of 'struct nsinfo',
return a pid_t, when that member is a boolean, fix it.

Fixes: bcaf0a97858de7ab ("perf namespaces: Add functions to access nsinfo")
Reported-by: Francesco Nigro <fnigro@redhat.com>
Reported-by: Ilan Green <igreen@redhat.com>
Cc: Adrian Hunter <adrian.hunter@intel.com>
Cc: Ian Rogers <irogers@google.com>
Cc: James Clark <james.clark@linaro.org>
Cc: Jiri Olsa <jolsa@kernel.org>
Cc: Kan Liang <kan.liang@linux.intel.com>
Cc: Namhyung Kim <namhyung@kernel.org>
Cc: Stephane Eranian <eranian@google.com>
Cc: Yonatan Goldschmidt <yonatan.goldschmidt@granulate.io>
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/util/namespaces.c | 2 +-
 tools/perf/util/namespaces.h | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/tools/perf/util/namespaces.c b/tools/perf/util/namespaces.c
index 36047184d76e2f80..68f5de2d79c72c2c 100644
--- a/tools/perf/util/namespaces.c
+++ b/tools/perf/util/namespaces.c
@@ -266,7 +266,7 @@ pid_t nsinfo__pid(const struct nsinfo  *nsi)
 	return RC_CHK_ACCESS(nsi)->pid;
 }
 
-pid_t nsinfo__in_pidns(const struct nsinfo  *nsi)
+bool nsinfo__in_pidns(const struct nsinfo *nsi)
 {
 	return RC_CHK_ACCESS(nsi)->in_pidns;
 }
diff --git a/tools/perf/util/namespaces.h b/tools/perf/util/namespaces.h
index e014becb9cd8eb3a..e95c79b80e27c8fc 100644
--- a/tools/perf/util/namespaces.h
+++ b/tools/perf/util/namespaces.h
@@ -58,7 +58,7 @@ void nsinfo__clear_need_setns(struct nsinfo *nsi);
 pid_t nsinfo__tgid(const struct nsinfo  *nsi);
 pid_t nsinfo__nstgid(const struct nsinfo  *nsi);
 pid_t nsinfo__pid(const struct nsinfo  *nsi);
-pid_t nsinfo__in_pidns(const struct nsinfo  *nsi);
+bool nsinfo__in_pidns(const struct nsinfo  *nsi);
 void nsinfo__set_in_pidns(struct nsinfo *nsi);
 
 void nsinfo__mountns_enter(struct nsinfo *nsi, struct nscookie *nc);
-- 
2.47.0


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* Re: [PATCH 0/5 v1] Processing java JIT dumps from containers
  2024-12-06 20:48 [PATCH 0/5 v1] Processing java JIT dumps from containers Arnaldo Carvalho de Melo
                   ` (4 preceding siblings ...)
  2024-12-06 20:48 ` [PATCH 5/5] perf namespaces: Fixup the nsinfo__in_pidns() return type, its bool Arnaldo Carvalho de Melo
@ 2024-12-23 19:51 ` Arnaldo Carvalho de Melo
  2025-01-08 20:31   ` Arnaldo Carvalho de Melo
  5 siblings, 1 reply; 8+ messages in thread
From: Arnaldo Carvalho de Melo @ 2024-12-23 19:51 UTC (permalink / raw)
  To: Stephane Eranian, Yonatan Goldschmidt
  Cc: Francesco Nigro, Ilan Green, Namhyung Kim, Ingo Molnar,
	Thomas Gleixner, James Clark, Jiri Olsa, Ian Rogers,
	Adrian Hunter, Kan Liang, Clark Williams, linux-kernel,
	linux-perf-users, Arnaldo Carvalho de Melo

On Fri, Dec 06, 2024 at 05:48:23PM -0300, Arnaldo Carvalho de Melo wrote:
> From: Arnaldo Carvalho de Melo <acme@redhat.com>
> 
> Hi,
> 
> 	This is trying to move the needle on supporting jitdump done
> from a container while perf is running from the outside, that is not
> working right now.
> 
> 	I tried to collect as much details on what is being done in the
> commit logs to document further work that needs to be done to support
> such a scenario in a streamlined way, but what in this patchkit at least
> helped in a real case, allowing us to get 'perf annotate' to work and
> confirm suspicions.

I'm going to apply this to perf-tools-next, please holler if you
disagree.

- Arnaldo
 
> - Arnaldo
> 
> Arnaldo Carvalho de Melo (5):
>   perf config: Fix trival typo 'an' -> 'can'
>   perf jitdump: Accept jitdump mmaps emitted from inside containers
>   perf namespaces: Introduce nsinfo__set_in_pidns()
>   perf jitdump: Fixup in_pidns member when java agent and 'perf record' are not in the same pidns
>   perf namespaces: Fixup the nsinfo__in_pidns() return type, its bool
> 
>  tools/perf/Documentation/perf-config.txt |  2 +-
>  tools/perf/util/jitdump.c                | 15 ++++++++++++---
>  tools/perf/util/namespaces.c             |  7 ++++++-
>  tools/perf/util/namespaces.h             |  3 ++-
>  4 files changed, 21 insertions(+), 6 deletions(-)
> 
> -- 
> 2.47.0

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH 0/5 v1] Processing java JIT dumps from containers
  2024-12-23 19:51 ` [PATCH 0/5 v1] Processing java JIT dumps from containers Arnaldo Carvalho de Melo
@ 2025-01-08 20:31   ` Arnaldo Carvalho de Melo
  0 siblings, 0 replies; 8+ messages in thread
From: Arnaldo Carvalho de Melo @ 2025-01-08 20:31 UTC (permalink / raw)
  To: Stephane Eranian, Yonatan Goldschmidt
  Cc: Francesco Nigro, Ilan Green, Namhyung Kim, Ingo Molnar,
	Thomas Gleixner, James Clark, Jiri Olsa, Ian Rogers,
	Adrian Hunter, Kan Liang, Clark Williams, linux-kernel,
	linux-perf-users, Arnaldo Carvalho de Melo

On Mon, Dec 23, 2024 at 04:51:49PM -0300, Arnaldo Carvalho de Melo wrote:
> On Fri, Dec 06, 2024 at 05:48:23PM -0300, Arnaldo Carvalho de Melo wrote:
> > From: Arnaldo Carvalho de Melo <acme@redhat.com>
> > 
 > Hi,
> > 
> > 	This is trying to move the needle on supporting jitdump done
> > from a container while perf is running from the outside, that is not
> > working right now.
> > 
> > 	I tried to collect as much details on what is being done in the
> > commit logs to document further work that needs to be done to support
> > such a scenario in a streamlined way, but what in this patchkit at least
> > helped in a real case, allowing us to get 'perf annotate' to work and
> > confirm suspicions.
> 
> I'm going to apply this to perf-tools-next, please holler if you
> disagree.

Applied.

- Arnaldo

^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2025-01-08 20:31 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-12-06 20:48 [PATCH 0/5 v1] Processing java JIT dumps from containers Arnaldo Carvalho de Melo
2024-12-06 20:48 ` [PATCH 1/5] perf config: Fix trival typo 'an' -> 'can' Arnaldo Carvalho de Melo
2024-12-06 20:48 ` [PATCH 2/5] perf jitdump: Accept jitdump mmaps emitted from inside containers Arnaldo Carvalho de Melo
2024-12-06 20:48 ` [PATCH 3/5] perf namespaces: Introduce nsinfo__set_in_pidns() Arnaldo Carvalho de Melo
2024-12-06 20:48 ` [PATCH 4/5] perf jitdump: Fixup in_pidns member when java agent and 'perf record' are not in the same pidns Arnaldo Carvalho de Melo
2024-12-06 20:48 ` [PATCH 5/5] perf namespaces: Fixup the nsinfo__in_pidns() return type, its bool Arnaldo Carvalho de Melo
2024-12-23 19:51 ` [PATCH 0/5 v1] Processing java JIT dumps from containers Arnaldo Carvalho de Melo
2025-01-08 20:31   ` Arnaldo Carvalho de Melo

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