* [PATCH 1/5] perf jitdump: Fix PID namespace detection
2025-11-05 19:10 [PATCH 0/5] perf jitdump: Fix PID namespace detection Ilya Leoshkevich
@ 2025-11-05 19:10 ` Ilya Leoshkevich
2025-11-07 2:16 ` Namhyung Kim
2025-11-05 19:10 ` [PATCH 2/5] perf test java symbol: Get rid of shellcheck warnings Ilya Leoshkevich
` (3 subsequent siblings)
4 siblings, 1 reply; 15+ messages in thread
From: Ilya Leoshkevich @ 2025-11-05 19:10 UTC (permalink / raw)
To: Arnaldo Carvalho de Melo
Cc: linux-kernel, linux-perf-users, Heiko Carstens, Vasily Gorbik,
Alexander Gordeev, Ilya Leoshkevich
perf inject fails to detect jitdump file produced by a process
running in a different PID namespace if this process has not exited
yet.
The PID namespace heuristic in jit_detect() compares two PIDs:
* pid: outermost NStgid of mmap(jitdump) caller from perf's PoV.
* nsinfo__nstgid(nsi): innermost NStgid of mmap(jitdump) caller from
perf's PoV.
The semantics of the in_pidns variable can be seen in, e.g.,
nsinfo__get_nspid(): it's true if and only if perf and the profiled
process are in different PID namespaces.
The current logic is clearly inverted: if pid and nsinfo__nstgid(nsi)
are different, then the profiled process must be in a different PID
namespace. This, of course, ignores that fact that they may end up
being equal by accident, but that's not the point here.
Fix by flipping the comparison.
Changing just that, however, breaks the case when the process has
exited. Add explicit support for that by adding "synthesized" field to
nsinfo, which tracks whether NStgid was obtained from a running
process (ignoring considerations of PID reuse or running inject on
a different machine). When the namespace information is synthesized,
assume the process ran in a different PID namespace.
Signed-off-by: Ilya Leoshkevich <iii@linux.ibm.com>
---
tools/perf/util/jitdump.c | 27 +++++++++++++++++++++------
tools/perf/util/namespaces.c | 9 +++++++++
tools/perf/util/namespaces.h | 2 ++
3 files changed, 32 insertions(+), 6 deletions(-)
diff --git a/tools/perf/util/jitdump.c b/tools/perf/util/jitdump.c
index b062b1f234b62..19e4bc139935b 100644
--- a/tools/perf/util/jitdump.c
+++ b/tools/perf/util/jitdump.c
@@ -788,15 +788,30 @@ jit_detect(const char *mmap_name, pid_t pid, struct nsinfo *nsi, bool *in_pidns)
if (!end)
return -1;
- *in_pidns = pid == nsinfo__nstgid(nsi);
/*
- * pid does not match mmap pid
- * pid==0 in system-wide mode (synthesized)
+ * Determine whether the process ran inside a container, and whether it
+ * mapped jit.dump for profiling purposes or by accident. Record this
+ * for further use in jit_inject(). The kernel does not provide PID
+ * namespace information, so we have to resort to guesswork.
*
- * 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 the process exited and perf had to synthesize the namespace
+ * information, then it's not possible to have any certainty; be
+ * aggressive and assume that the process ran inside a container. This
+ * allows the user to proceed with injection at the cost of a small
+ * probability of injecting irrelevant data.
+ *
+ * If the process' NStgid as observed by perf is different from its
+ * innermost NStgid, then it must have run inside a container. There is
+ * a very small probability that NStgids randomly happenned to be the
+ * same; ignore it.
+ *
+ * pid == 0 means system-wide mode, just proceed.
+ *
+ * Finally, the most straightforward case: if the PID in the file name
+ * is equal to the process' NStgid as observed by perf, then it must be
+ * a match.
*/
+ *in_pidns = nsinfo__synthesized(nsi) || pid != nsinfo__nstgid(nsi);
if (pid && !(pid2 == pid || *in_pidns))
return -1;
/*
diff --git a/tools/perf/util/namespaces.c b/tools/perf/util/namespaces.c
index 01502570b32d0..7de5d62e271c4 100644
--- a/tools/perf/util/namespaces.c
+++ b/tools/perf/util/namespaces.c
@@ -132,6 +132,8 @@ int nsinfo__init(struct nsinfo *nsi)
rv = nsinfo__get_nspid(&RC_CHK_ACCESS(nsi)->tgid, &RC_CHK_ACCESS(nsi)->nstgid,
&RC_CHK_ACCESS(nsi)->in_pidns, spath);
+ if (rv == 0)
+ RC_CHK_ACCESS(nsi)->synthesized = false;
out:
free(newns);
@@ -166,6 +168,7 @@ struct nsinfo *nsinfo__new(pid_t pid)
RC_CHK_ACCESS(nsi)->nstgid = pid;
nsinfo__clear_need_setns(nsi);
RC_CHK_ACCESS(nsi)->in_pidns = false;
+ RC_CHK_ACCESS(nsi)->synthesized = true;
/* Init may fail if the process exits while we're trying to look at its
* proc information. In that case, save the pid but don't try to enter
* the namespace.
@@ -197,6 +200,7 @@ struct nsinfo *nsinfo__copy(const struct nsinfo *nsi)
RC_CHK_ACCESS(nnsi)->nstgid = nsinfo__nstgid(nsi);
RC_CHK_ACCESS(nnsi)->need_setns = nsinfo__need_setns(nsi);
RC_CHK_ACCESS(nnsi)->in_pidns = nsinfo__in_pidns(nsi);
+ RC_CHK_ACCESS(nnsi)->synthesized = nsinfo__synthesized(nsi);
if (nsinfo__mntns_path(nsi)) {
RC_CHK_ACCESS(nnsi)->mntns_path = strdup(nsinfo__mntns_path(nsi));
if (!RC_CHK_ACCESS(nnsi)->mntns_path) {
@@ -275,6 +279,11 @@ void nsinfo__set_in_pidns(struct nsinfo *nsi)
RC_CHK_ACCESS(nsi)->in_pidns = true;
}
+bool nsinfo__synthesized(const struct nsinfo *nsi)
+{
+ return RC_CHK_ACCESS(nsi)->synthesized;
+}
+
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 e95c79b80e27c..41ba2ea8137e5 100644
--- a/tools/perf/util/namespaces.h
+++ b/tools/perf/util/namespaces.h
@@ -38,6 +38,7 @@ DECLARE_RC_STRUCT(nsinfo) {
bool in_pidns;
char *mntns_path;
refcount_t refcnt;
+ bool synthesized;
};
struct nscookie {
@@ -60,6 +61,7 @@ pid_t nsinfo__nstgid(const struct nsinfo *nsi);
pid_t nsinfo__pid(const struct nsinfo *nsi);
bool nsinfo__in_pidns(const struct nsinfo *nsi);
void nsinfo__set_in_pidns(struct nsinfo *nsi);
+bool nsinfo__synthesized(const struct nsinfo *nsi);
void nsinfo__mountns_enter(struct nsinfo *nsi, struct nscookie *nc);
void nsinfo__mountns_exit(struct nscookie *nc);
--
2.51.1
^ permalink raw reply related [flat|nested] 15+ messages in thread* Re: [PATCH 1/5] perf jitdump: Fix PID namespace detection
2025-11-05 19:10 ` [PATCH 1/5] " Ilya Leoshkevich
@ 2025-11-07 2:16 ` Namhyung Kim
2025-11-07 8:19 ` Ilya Leoshkevich
0 siblings, 1 reply; 15+ messages in thread
From: Namhyung Kim @ 2025-11-07 2:16 UTC (permalink / raw)
To: Ilya Leoshkevich
Cc: Arnaldo Carvalho de Melo, linux-kernel, linux-perf-users,
Heiko Carstens, Vasily Gorbik, Alexander Gordeev
On Wed, Nov 05, 2025 at 08:10:24PM +0100, Ilya Leoshkevich wrote:
> perf inject fails to detect jitdump file produced by a process
> running in a different PID namespace if this process has not exited
> yet.
>
> The PID namespace heuristic in jit_detect() compares two PIDs:
>
> * pid: outermost NStgid of mmap(jitdump) caller from perf's PoV.
> * nsinfo__nstgid(nsi): innermost NStgid of mmap(jitdump) caller from
> perf's PoV.
>
> The semantics of the in_pidns variable can be seen in, e.g.,
> nsinfo__get_nspid(): it's true if and only if perf and the profiled
> process are in different PID namespaces.
>
> The current logic is clearly inverted: if pid and nsinfo__nstgid(nsi)
> are different, then the profiled process must be in a different PID
> namespace. This, of course, ignores that fact that they may end up
> being equal by accident, but that's not the point here.
>
> Fix by flipping the comparison.
>
> Changing just that, however, breaks the case when the process has
> exited. Add explicit support for that by adding "synthesized" field to
> nsinfo, which tracks whether NStgid was obtained from a running
> process (ignoring considerations of PID reuse or running inject on
> a different machine). When the namespace information is synthesized,
> assume the process ran in a different PID namespace.
I'm not sure I'm following. It'd be great if anyone understand the
topic well could review.
>
> Signed-off-by: Ilya Leoshkevich <iii@linux.ibm.com>
> ---
> tools/perf/util/jitdump.c | 27 +++++++++++++++++++++------
> tools/perf/util/namespaces.c | 9 +++++++++
> tools/perf/util/namespaces.h | 2 ++
> 3 files changed, 32 insertions(+), 6 deletions(-)
>
> diff --git a/tools/perf/util/jitdump.c b/tools/perf/util/jitdump.c
> index b062b1f234b62..19e4bc139935b 100644
> --- a/tools/perf/util/jitdump.c
> +++ b/tools/perf/util/jitdump.c
> @@ -788,15 +788,30 @@ jit_detect(const char *mmap_name, pid_t pid, struct nsinfo *nsi, bool *in_pidns)
> if (!end)
> return -1;
>
> - *in_pidns = pid == nsinfo__nstgid(nsi);
> /*
> - * pid does not match mmap pid
> - * pid==0 in system-wide mode (synthesized)
> + * Determine whether the process ran inside a container, and whether it
> + * mapped jit.dump for profiling purposes or by accident. Record this
> + * for further use in jit_inject(). The kernel does not provide PID
> + * namespace information, so we have to resort to guesswork.
> *
> - * 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 the process exited and perf had to synthesize the namespace
> + * information, then it's not possible to have any certainty; be
> + * aggressive and assume that the process ran inside a container. This
> + * allows the user to proceed with injection at the cost of a small
> + * probability of injecting irrelevant data.
> + *
> + * If the process' NStgid as observed by perf is different from its
> + * innermost NStgid, then it must have run inside a container. There is
> + * a very small probability that NStgids randomly happenned to be the
> + * same; ignore it.
> + *
> + * pid == 0 means system-wide mode, just proceed.
> + *
> + * Finally, the most straightforward case: if the PID in the file name
> + * is equal to the process' NStgid as observed by perf, then it must be
> + * a match.
> */
> + *in_pidns = nsinfo__synthesized(nsi) || pid != nsinfo__nstgid(nsi);
> if (pid && !(pid2 == pid || *in_pidns))
> return -1;
> /*
> diff --git a/tools/perf/util/namespaces.c b/tools/perf/util/namespaces.c
> index 01502570b32d0..7de5d62e271c4 100644
> --- a/tools/perf/util/namespaces.c
> +++ b/tools/perf/util/namespaces.c
> @@ -132,6 +132,8 @@ int nsinfo__init(struct nsinfo *nsi)
>
> rv = nsinfo__get_nspid(&RC_CHK_ACCESS(nsi)->tgid, &RC_CHK_ACCESS(nsi)->nstgid,
> &RC_CHK_ACCESS(nsi)->in_pidns, spath);
> + if (rv == 0)
> + RC_CHK_ACCESS(nsi)->synthesized = false;
>
> out:
> free(newns);
> @@ -166,6 +168,7 @@ struct nsinfo *nsinfo__new(pid_t pid)
> RC_CHK_ACCESS(nsi)->nstgid = pid;
> nsinfo__clear_need_setns(nsi);
> RC_CHK_ACCESS(nsi)->in_pidns = false;
> + RC_CHK_ACCESS(nsi)->synthesized = true;
> /* Init may fail if the process exits while we're trying to look at its
> * proc information. In that case, save the pid but don't try to enter
> * the namespace.
> @@ -197,6 +200,7 @@ struct nsinfo *nsinfo__copy(const struct nsinfo *nsi)
> RC_CHK_ACCESS(nnsi)->nstgid = nsinfo__nstgid(nsi);
> RC_CHK_ACCESS(nnsi)->need_setns = nsinfo__need_setns(nsi);
> RC_CHK_ACCESS(nnsi)->in_pidns = nsinfo__in_pidns(nsi);
> + RC_CHK_ACCESS(nnsi)->synthesized = nsinfo__synthesized(nsi);
> if (nsinfo__mntns_path(nsi)) {
> RC_CHK_ACCESS(nnsi)->mntns_path = strdup(nsinfo__mntns_path(nsi));
> if (!RC_CHK_ACCESS(nnsi)->mntns_path) {
> @@ -275,6 +279,11 @@ void nsinfo__set_in_pidns(struct nsinfo *nsi)
> RC_CHK_ACCESS(nsi)->in_pidns = true;
> }
>
> +bool nsinfo__synthesized(const struct nsinfo *nsi)
> +{
> + return RC_CHK_ACCESS(nsi)->synthesized;
> +}
> +
> 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 e95c79b80e27c..41ba2ea8137e5 100644
> --- a/tools/perf/util/namespaces.h
> +++ b/tools/perf/util/namespaces.h
> @@ -38,6 +38,7 @@ DECLARE_RC_STRUCT(nsinfo) {
> bool in_pidns;
> char *mntns_path;
> refcount_t refcnt;
> + bool synthesized;
It'd be nice if you can put this along with other bool fields.
Thanks,
Namhyung
> };
>
> struct nscookie {
> @@ -60,6 +61,7 @@ pid_t nsinfo__nstgid(const struct nsinfo *nsi);
> pid_t nsinfo__pid(const struct nsinfo *nsi);
> bool nsinfo__in_pidns(const struct nsinfo *nsi);
> void nsinfo__set_in_pidns(struct nsinfo *nsi);
> +bool nsinfo__synthesized(const struct nsinfo *nsi);
>
> void nsinfo__mountns_enter(struct nsinfo *nsi, struct nscookie *nc);
> void nsinfo__mountns_exit(struct nscookie *nc);
> --
> 2.51.1
>
^ permalink raw reply [flat|nested] 15+ messages in thread* Re: [PATCH 1/5] perf jitdump: Fix PID namespace detection
2025-11-07 2:16 ` Namhyung Kim
@ 2025-11-07 8:19 ` Ilya Leoshkevich
2025-11-14 8:07 ` Namhyung Kim
0 siblings, 1 reply; 15+ messages in thread
From: Ilya Leoshkevich @ 2025-11-07 8:19 UTC (permalink / raw)
To: Namhyung Kim
Cc: Arnaldo Carvalho de Melo, linux-kernel, linux-perf-users,
Heiko Carstens, Vasily Gorbik, Alexander Gordeev
On Thu, 2025-11-06 at 18:16 -0800, Namhyung Kim wrote:
> On Wed, Nov 05, 2025 at 08:10:24PM +0100, Ilya Leoshkevich wrote:
> > perf inject fails to detect jitdump file produced by a process
> > running in a different PID namespace if this process has not exited
> > yet.
> >
> > The PID namespace heuristic in jit_detect() compares two PIDs:
> >
> > * pid: outermost NStgid of mmap(jitdump) caller from perf's PoV.
> > * nsinfo__nstgid(nsi): innermost NStgid of mmap(jitdump) caller
> > from
> > perf's PoV.
> >
> > The semantics of the in_pidns variable can be seen in, e.g.,
> > nsinfo__get_nspid(): it's true if and only if perf and the profiled
> > process are in different PID namespaces.
> >
> > The current logic is clearly inverted: if pid and
> > nsinfo__nstgid(nsi)
> > are different, then the profiled process must be in a different PID
> > namespace. This, of course, ignores that fact that they may end up
> > being equal by accident, but that's not the point here.
> >
> > Fix by flipping the comparison.
> >
> > Changing just that, however, breaks the case when the process has
> > exited. Add explicit support for that by adding "synthesized" field
> > to
> > nsinfo, which tracks whether NStgid was obtained from a running
> > process (ignoring considerations of PID reuse or running inject on
> > a different machine). When the namespace information is
> > synthesized,
> > assume the process ran in a different PID namespace.
>
> I'm not sure I'm following. It'd be great if anyone understand the
> topic well could review.
Perhaps some data from the testcase from [5/5] can make it more clear.
Here are the PIDs that exist in reality:
unshare[a] perf-record unshare[b] jshell
Host PIDNS: 1000 1001 1002 1003
PIDNS[a]: - 1 2 3
PIDNS[b]: - - - 1
In jit_detect() we deal with 2 of them.
- pid is jshell@PIDNS[a].
It is taken from the MMAP2 event, this is how perf sees jshell.
- pid2 is jshell@PIDNS[b].
It is taken from "jit-1.dump", this is how jshell sees itself.
- nsinfo__nstgid(nsi) ideally should be jshell@PIDNS[b].
This is jshell's innermost NStgid.
But perf can see it differently. This is the core of the problem this
series deals with.
Why does nsinfo__nstgid(nsi) vary? Because the kernel does not record
it, and perf has to guess it. I have a WIP patch to fix that [1], but
it needs a lot more work at this point.
How does perf guess it? It looks into /proc/$PID/status. This is quite
unreliable, but this is the best perf can do under circumstances. As a
result we have 3 possibilities:
- The original process is still around. This is the buggy case. In this
case nsinfo__nstgid(nsi) == jshell@PIDNS[b]. IMHO this is a very
clear indication of namespacing, and that's why the condition should
be flipped.
- The original process has exited and PID was not reused. I believe
this is the use case the current code has been extensively tested
with. In this case perf assumes there was no namespacing and
nsinfo__nstgid(nsi) == pid. That's why I need the "synthesized"
field: to indicate that NStgid is just an assumption and didn't come
from any real data source.
- The original process has exited ans PID was reused. I am not trying
to deal with this here, because this is rare and absolutely anything
is possible. The only concern is running perf inject on a different
machine, but I'm also not sure whether we should care about this.
[1]
https://github.com/iii-i/linux/commit/4b519b774eed850abe0757df39be13a704d2748e
[...]
> > diff --git a/tools/perf/util/namespaces.h
> > b/tools/perf/util/namespaces.h
> > index e95c79b80e27c..41ba2ea8137e5 100644
> > --- a/tools/perf/util/namespaces.h
> > +++ b/tools/perf/util/namespaces.h
> > @@ -38,6 +38,7 @@ DECLARE_RC_STRUCT(nsinfo) {
> > bool in_pidns;
> > char *mntns_path;
> > refcount_t refcnt;
> > + bool synthesized;
>
> It'd be nice if you can put this along with other bool fields.
Sure, will do.
[...]
>
^ permalink raw reply [flat|nested] 15+ messages in thread* Re: [PATCH 1/5] perf jitdump: Fix PID namespace detection
2025-11-07 8:19 ` Ilya Leoshkevich
@ 2025-11-14 8:07 ` Namhyung Kim
2025-11-14 12:44 ` Ilya Leoshkevich
0 siblings, 1 reply; 15+ messages in thread
From: Namhyung Kim @ 2025-11-14 8:07 UTC (permalink / raw)
To: Ilya Leoshkevich
Cc: Arnaldo Carvalho de Melo, linux-kernel, linux-perf-users,
Heiko Carstens, Vasily Gorbik, Alexander Gordeev
Hello,
Sorry for the delay.
On Fri, Nov 07, 2025 at 09:19:47AM +0100, Ilya Leoshkevich wrote:
> On Thu, 2025-11-06 at 18:16 -0800, Namhyung Kim wrote:
> > On Wed, Nov 05, 2025 at 08:10:24PM +0100, Ilya Leoshkevich wrote:
> > > perf inject fails to detect jitdump file produced by a process
> > > running in a different PID namespace if this process has not exited
> > > yet.
> > >
> > > The PID namespace heuristic in jit_detect() compares two PIDs:
> > >
> > > * pid: outermost NStgid of mmap(jitdump) caller from perf's PoV.
> > > * nsinfo__nstgid(nsi): innermost NStgid of mmap(jitdump) caller
> > > from
> > > perf's PoV.
> > >
> > > The semantics of the in_pidns variable can be seen in, e.g.,
> > > nsinfo__get_nspid(): it's true if and only if perf and the profiled
> > > process are in different PID namespaces.
> > >
> > > The current logic is clearly inverted: if pid and
> > > nsinfo__nstgid(nsi)
> > > are different, then the profiled process must be in a different PID
> > > namespace. This, of course, ignores that fact that they may end up
> > > being equal by accident, but that's not the point here.
> > >
> > > Fix by flipping the comparison.
> > >
> > > Changing just that, however, breaks the case when the process has
> > > exited. Add explicit support for that by adding "synthesized" field
> > > to
> > > nsinfo, which tracks whether NStgid was obtained from a running
> > > process (ignoring considerations of PID reuse or running inject on
> > > a different machine). When the namespace information is
> > > synthesized,
> > > assume the process ran in a different PID namespace.
> >
> > I'm not sure I'm following. It'd be great if anyone understand the
> > topic well could review.
>
> Perhaps some data from the testcase from [5/5] can make it more clear.
> Here are the PIDs that exist in reality:
>
> unshare[a] perf-record unshare[b] jshell
> Host PIDNS: 1000 1001 1002 1003
> PIDNS[a]: - 1 2 3
> PIDNS[b]: - - - 1
>
> In jit_detect() we deal with 2 of them.
>
> - pid is jshell@PIDNS[a].
> It is taken from the MMAP2 event, this is how perf sees jshell.
>
> - pid2 is jshell@PIDNS[b].
> It is taken from "jit-1.dump", this is how jshell sees itself.
>
> - nsinfo__nstgid(nsi) ideally should be jshell@PIDNS[b].
> This is jshell's innermost NStgid.
> But perf can see it differently. This is the core of the problem this
> series deals with.
Thanks a lot for the example and explanation! I'm trying to understand.
:)
>
> Why does nsinfo__nstgid(nsi) vary? Because the kernel does not record
> it, and perf has to guess it. I have a WIP patch to fix that [1], but
> it needs a lot more work at this point.
>
> How does perf guess it? It looks into /proc/$PID/status. This is quite
> unreliable, but this is the best perf can do under circumstances. As a
> result we have 3 possibilities:
>
> - The original process is still around. This is the buggy case. In this
> case nsinfo__nstgid(nsi) == jshell@PIDNS[b]. IMHO this is a very
> clear indication of namespacing, and that's why the condition should
> be flipped.
So perf would look at /proc/3/status and the file would have the below
NStgid: 1003 3 1
and *in_pidns should be true, right?
>
> - The original process has exited and PID was not reused. I believe
> this is the use case the current code has been extensively tested
> with. In this case perf assumes there was no namespacing and
> nsinfo__nstgid(nsi) == pid. That's why I need the "synthesized"
> field: to indicate that NStgid is just an assumption and didn't come
> from any real data source.
Ok, can you please put short comments on each boolean field so that we
can see the meaning of them clearly?
>
> - The original process has exited ans PID was reused. I am not trying
> to deal with this here, because this is rare and absolutely anything
> is possible. The only concern is running perf inject on a different
> machine, but I'm also not sure whether we should care about this.
Fair enough.
Thanks,
Namhyung
>
> [1]
> https://github.com/iii-i/linux/commit/4b519b774eed850abe0757df39be13a704d2748e
>
> [...]
>
> > > diff --git a/tools/perf/util/namespaces.h
> > > b/tools/perf/util/namespaces.h
> > > index e95c79b80e27c..41ba2ea8137e5 100644
> > > --- a/tools/perf/util/namespaces.h
> > > +++ b/tools/perf/util/namespaces.h
> > > @@ -38,6 +38,7 @@ DECLARE_RC_STRUCT(nsinfo) {
> > > bool in_pidns;
> > > char *mntns_path;
> > > refcount_t refcnt;
> > > + bool synthesized;
> >
> > It'd be nice if you can put this along with other bool fields.
>
> Sure, will do.
>
> [...]
> >
^ permalink raw reply [flat|nested] 15+ messages in thread* Re: [PATCH 1/5] perf jitdump: Fix PID namespace detection
2025-11-14 8:07 ` Namhyung Kim
@ 2025-11-14 12:44 ` Ilya Leoshkevich
2025-11-14 18:44 ` Namhyung Kim
0 siblings, 1 reply; 15+ messages in thread
From: Ilya Leoshkevich @ 2025-11-14 12:44 UTC (permalink / raw)
To: Namhyung Kim
Cc: Arnaldo Carvalho de Melo, linux-kernel, linux-perf-users,
Heiko Carstens, Vasily Gorbik, Alexander Gordeev
On Fri, 2025-11-14 at 00:07 -0800, Namhyung Kim wrote:
> Hello,
>
> Sorry for the delay.
>
> On Fri, Nov 07, 2025 at 09:19:47AM +0100, Ilya Leoshkevich wrote:
> > On Thu, 2025-11-06 at 18:16 -0800, Namhyung Kim wrote:
> > > On Wed, Nov 05, 2025 at 08:10:24PM +0100, Ilya Leoshkevich wrote:
> > > > perf inject fails to detect jitdump file produced by a process
> > > > running in a different PID namespace if this process has not
> > > > exited
> > > > yet.
> > > >
> > > > The PID namespace heuristic in jit_detect() compares two PIDs:
> > > >
> > > > * pid: outermost NStgid of mmap(jitdump) caller from perf's
> > > > PoV.
> > > > * nsinfo__nstgid(nsi): innermost NStgid of mmap(jitdump) caller
> > > > from
> > > > perf's PoV.
> > > >
> > > > The semantics of the in_pidns variable can be seen in, e.g.,
> > > > nsinfo__get_nspid(): it's true if and only if perf and the
> > > > profiled
> > > > process are in different PID namespaces.
> > > >
> > > > The current logic is clearly inverted: if pid and
> > > > nsinfo__nstgid(nsi)
> > > > are different, then the profiled process must be in a different
> > > > PID
> > > > namespace. This, of course, ignores that fact that they may end
> > > > up
> > > > being equal by accident, but that's not the point here.
> > > >
> > > > Fix by flipping the comparison.
> > > >
> > > > Changing just that, however, breaks the case when the process
> > > > has
> > > > exited. Add explicit support for that by adding "synthesized"
> > > > field
> > > > to
> > > > nsinfo, which tracks whether NStgid was obtained from a running
> > > > process (ignoring considerations of PID reuse or running inject
> > > > on
> > > > a different machine). When the namespace information is
> > > > synthesized,
> > > > assume the process ran in a different PID namespace.
> > >
> > > I'm not sure I'm following. It'd be great if anyone understand
> > > the
> > > topic well could review.
> >
> > Perhaps some data from the testcase from [5/5] can make it more
> > clear.
> > Here are the PIDs that exist in reality:
> >
> > unshare[a] perf-record unshare[b] jshell
> > Host PIDNS: 1000 1001 1002 1003
> > PIDNS[a]: - 1 2 3
> > PIDNS[b]: - - - 1
> >
> > In jit_detect() we deal with 2 of them.
> >
> > - pid is jshell@PIDNS[a].
> > It is taken from the MMAP2 event, this is how perf sees jshell.
> >
> > - pid2 is jshell@PIDNS[b].
> > It is taken from "jit-1.dump", this is how jshell sees itself.
> >
> > - nsinfo__nstgid(nsi) ideally should be jshell@PIDNS[b].
> > This is jshell's innermost NStgid.
> > But perf can see it differently. This is the core of the problem
> > this
> > series deals with.
>
> Thanks a lot for the example and explanation! I'm trying to
> understand.
> :)
Thank you for the patience!
> > Why does nsinfo__nstgid(nsi) vary? Because the kernel does not
> > record
> > it, and perf has to guess it. I have a WIP patch to fix that [1],
> > but
> > it needs a lot more work at this point.
> >
> > How does perf guess it? It looks into /proc/$PID/status. This is
> > quite
> > unreliable, but this is the best perf can do under circumstances.
> > As a
> > result we have 3 possibilities:
> >
> > - The original process is still around. This is the buggy case. In
> > this
> > case nsinfo__nstgid(nsi) == jshell@PIDNS[b]. IMHO this is a very
> > clear indication of namespacing, and that's why the condition
> > should
> > be flipped.
>
> So perf would look at /proc/3/status and the file would have the
> below
>
> NStgid: 1003 3 1
>
> and *in_pidns should be true, right?
It should be
NStgid: 3 1
because perf itself is namespaced too - in this testcase nobody sees
the root PID namespace. I wrote it this way to make sure there are
no accidental leaks from the root PID namespace, but it's not very
important and perhaps obscures things somewhat unnecessarily.
But regardless, I believe that in this case setting *in_pidns to true
is the right thing.
> > - The original process has exited and PID was not reused. I believe
> > this is the use case the current code has been extensively tested
> > with. In this case perf assumes there was no namespacing and
> > nsinfo__nstgid(nsi) == pid. That's why I need the "synthesized"
> > field: to indicate that NStgid is just an assumption and didn't
> > come
> > from any real data source.
>
> Ok, can you please put short comments on each boolean field so that
> we
> can see the meaning of them clearly?
Sure, I can add this in v2.
[...]
^ permalink raw reply [flat|nested] 15+ messages in thread* Re: [PATCH 1/5] perf jitdump: Fix PID namespace detection
2025-11-14 12:44 ` Ilya Leoshkevich
@ 2025-11-14 18:44 ` Namhyung Kim
0 siblings, 0 replies; 15+ messages in thread
From: Namhyung Kim @ 2025-11-14 18:44 UTC (permalink / raw)
To: Ilya Leoshkevich
Cc: Arnaldo Carvalho de Melo, linux-kernel, linux-perf-users,
Heiko Carstens, Vasily Gorbik, Alexander Gordeev
On Fri, Nov 14, 2025 at 01:44:30PM +0100, Ilya Leoshkevich wrote:
> On Fri, 2025-11-14 at 00:07 -0800, Namhyung Kim wrote:
> > Hello,
> >
> > Sorry for the delay.
> >
> > On Fri, Nov 07, 2025 at 09:19:47AM +0100, Ilya Leoshkevich wrote:
> > > On Thu, 2025-11-06 at 18:16 -0800, Namhyung Kim wrote:
> > > > On Wed, Nov 05, 2025 at 08:10:24PM +0100, Ilya Leoshkevich wrote:
> > > > > perf inject fails to detect jitdump file produced by a process
> > > > > running in a different PID namespace if this process has not
> > > > > exited
> > > > > yet.
> > > > >
> > > > > The PID namespace heuristic in jit_detect() compares two PIDs:
> > > > >
> > > > > * pid: outermost NStgid of mmap(jitdump) caller from perf's
> > > > > PoV.
> > > > > * nsinfo__nstgid(nsi): innermost NStgid of mmap(jitdump) caller
> > > > > from
> > > > > perf's PoV.
> > > > >
> > > > > The semantics of the in_pidns variable can be seen in, e.g.,
> > > > > nsinfo__get_nspid(): it's true if and only if perf and the
> > > > > profiled
> > > > > process are in different PID namespaces.
> > > > >
> > > > > The current logic is clearly inverted: if pid and
> > > > > nsinfo__nstgid(nsi)
> > > > > are different, then the profiled process must be in a different
> > > > > PID
> > > > > namespace. This, of course, ignores that fact that they may end
> > > > > up
> > > > > being equal by accident, but that's not the point here.
> > > > >
> > > > > Fix by flipping the comparison.
> > > > >
> > > > > Changing just that, however, breaks the case when the process
> > > > > has
> > > > > exited. Add explicit support for that by adding "synthesized"
> > > > > field
> > > > > to
> > > > > nsinfo, which tracks whether NStgid was obtained from a running
> > > > > process (ignoring considerations of PID reuse or running inject
> > > > > on
> > > > > a different machine). When the namespace information is
> > > > > synthesized,
> > > > > assume the process ran in a different PID namespace.
> > > >
> > > > I'm not sure I'm following. It'd be great if anyone understand
> > > > the
> > > > topic well could review.
> > >
> > > Perhaps some data from the testcase from [5/5] can make it more
> > > clear.
> > > Here are the PIDs that exist in reality:
> > >
> > > unshare[a] perf-record unshare[b] jshell
> > > Host PIDNS: 1000 1001 1002 1003
> > > PIDNS[a]: - 1 2 3
> > > PIDNS[b]: - - - 1
> > >
> > > In jit_detect() we deal with 2 of them.
> > >
> > > - pid is jshell@PIDNS[a].
> > > It is taken from the MMAP2 event, this is how perf sees jshell.
> > >
> > > - pid2 is jshell@PIDNS[b].
> > > It is taken from "jit-1.dump", this is how jshell sees itself.
> > >
> > > - nsinfo__nstgid(nsi) ideally should be jshell@PIDNS[b].
> > > This is jshell's innermost NStgid.
> > > But perf can see it differently. This is the core of the problem
> > > this
> > > series deals with.
> >
> > Thanks a lot for the example and explanation! I'm trying to
> > understand.
> > :)
>
> Thank you for the patience!
>
> > > Why does nsinfo__nstgid(nsi) vary? Because the kernel does not
> > > record
> > > it, and perf has to guess it. I have a WIP patch to fix that [1],
> > > but
> > > it needs a lot more work at this point.
> > >
> > > How does perf guess it? It looks into /proc/$PID/status. This is
> > > quite
> > > unreliable, but this is the best perf can do under circumstances.
> > > As a
> > > result we have 3 possibilities:
> > >
> > > - The original process is still around. This is the buggy case. In
> > > this
> > > case nsinfo__nstgid(nsi) == jshell@PIDNS[b]. IMHO this is a very
> > > clear indication of namespacing, and that's why the condition
> > > should
> > > be flipped.
> >
> > So perf would look at /proc/3/status and the file would have the
> > below
> >
> > NStgid: 1003 3 1
> >
> > and *in_pidns should be true, right?
>
> It should be
>
> NStgid: 3 1
>
> because perf itself is namespaced too - in this testcase nobody sees
> the root PID namespace. I wrote it this way to make sure there are
> no accidental leaks from the root PID namespace, but it's not very
> important and perhaps obscures things somewhat unnecessarily.
Ah, ok. Thanks for the correction.
>
> But regardless, I believe that in this case setting *in_pidns to true
> is the right thing.
Yep, I agree.
>
> > > - The original process has exited and PID was not reused. I believe
> > > this is the use case the current code has been extensively tested
> > > with. In this case perf assumes there was no namespacing and
> > > nsinfo__nstgid(nsi) == pid. That's why I need the "synthesized"
> > > field: to indicate that NStgid is just an assumption and didn't
> > > come
> > > from any real data source.
> >
> > Ok, can you please put short comments on each boolean field so that
> > we
> > can see the meaning of them clearly?
>
> Sure, I can add this in v2.
Perhaps it's better to split the part adding "synthesize" field to
nsinfo in a separate commit and this one focuses on setting in_pidns.
Thanks,
Namhyung
^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH 2/5] perf test java symbol: Get rid of shellcheck warnings
2025-11-05 19:10 [PATCH 0/5] perf jitdump: Fix PID namespace detection Ilya Leoshkevich
2025-11-05 19:10 ` [PATCH 1/5] " Ilya Leoshkevich
@ 2025-11-05 19:10 ` Ilya Leoshkevich
2025-11-07 2:07 ` Namhyung Kim
2025-11-05 19:10 ` [PATCH 3/5] perf test java symbol: Extract LIBJVMTI detection Ilya Leoshkevich
` (2 subsequent siblings)
4 siblings, 1 reply; 15+ messages in thread
From: Ilya Leoshkevich @ 2025-11-05 19:10 UTC (permalink / raw)
To: Arnaldo Carvalho de Melo
Cc: linux-kernel, linux-perf-users, Heiko Carstens, Vasily Gorbik,
Alexander Gordeev, Ilya Leoshkevich
Add missing quotes and suppress the $? warnings.
Signed-off-by: Ilya Leoshkevich <iii@linux.ibm.com>
---
tools/perf/tests/shell/test_java_symbol.sh | 20 ++++++++++++--------
1 file changed, 12 insertions(+), 8 deletions(-)
diff --git a/tools/perf/tests/shell/test_java_symbol.sh b/tools/perf/tests/shell/test_java_symbol.sh
index 499539d1c4794..b1d7cd43af01a 100755
--- a/tools/perf/tests/shell/test_java_symbol.sh
+++ b/tools/perf/tests/shell/test_java_symbol.sh
@@ -4,6 +4,9 @@
# SPDX-License-Identifier: GPL-2.0
# Leo Yan <leo.yan@linaro.org>, 2022
+# Allow [ $? -ne 0 ], because long commands look ugly in if statements.
+# shellcheck disable=SC2181
+
# skip if there's no jshell
if ! [ -x "$(command -v jshell)" ]; then
echo "skip: no jshell, install JDK"
@@ -13,11 +16,12 @@ fi
PERF_DATA=$(mktemp /tmp/__perf_test.perf.data.XXXXX)
PERF_INJ_DATA=$(mktemp /tmp/__perf_test.perf.data.inj.XXXXX)
-cleanup_files()
-{
+# Shellcheck does not understand that this function is used by a trap.
+# shellcheck disable=SC2317
+cleanup_files() {
echo "Cleaning up files..."
- rm -f ${PERF_DATA}
- rm -f ${PERF_INJ_DATA}
+ rm -f "$PERF_DATA"
+ rm -f "$PERF_INJ_DATA"
}
trap cleanup_files exit term int
@@ -38,7 +42,7 @@ else
exit 2
fi
-cat <<EOF | perf record -k 1 -o $PERF_DATA jshell -s -J-agentpath:$LIBJVMTI
+cat <<EOF | perf record -k 1 -o "$PERF_DATA" jshell -s -J"-agentpath:$LIBJVMTI"
int fib(int x) {
return x > 1 ? fib(x - 2) + fib(x - 1) : 1;
}
@@ -56,7 +60,7 @@ if [ $? -ne 0 ]; then
exit 1
fi
-if ! DEBUGINFOD_URLS='' perf inject -i $PERF_DATA -o $PERF_INJ_DATA -j; then
+if ! DEBUGINFOD_URLS='' perf inject -i "$PERF_DATA" -o "$PERF_INJ_DATA" -j; then
echo "Fail to inject samples"
exit 1
fi
@@ -64,8 +68,8 @@ fi
# Below is an example of the instruction samples reporting:
# 8.18% jshell jitted-50116-29.so [.] Interpreter
# 0.75% Thread-1 jitted-83602-1670.so [.] jdk.internal.jimage.BasicImageReader.getString(int)
-perf report --stdio -i ${PERF_INJ_DATA} 2>&1 | \
- grep -E " +[0-9]+\.[0-9]+% .* (Interpreter|jdk\.internal).*" > /dev/null 2>&1
+perf report --stdio -i "$PERF_INJ_DATA" 2>&1 |
+ grep -E " +[0-9]+\.[0-9]+% .* (Interpreter|jdk\.internal).*" >/dev/null 2>&1
if [ $? -ne 0 ]; then
echo "Fail to find java symbols"
--
2.51.1
^ permalink raw reply related [flat|nested] 15+ messages in thread* Re: [PATCH 2/5] perf test java symbol: Get rid of shellcheck warnings
2025-11-05 19:10 ` [PATCH 2/5] perf test java symbol: Get rid of shellcheck warnings Ilya Leoshkevich
@ 2025-11-07 2:07 ` Namhyung Kim
2025-11-07 7:57 ` Ilya Leoshkevich
0 siblings, 1 reply; 15+ messages in thread
From: Namhyung Kim @ 2025-11-07 2:07 UTC (permalink / raw)
To: Ilya Leoshkevich
Cc: Arnaldo Carvalho de Melo, linux-kernel, linux-perf-users,
Heiko Carstens, Vasily Gorbik, Alexander Gordeev
Hello,
On Wed, Nov 05, 2025 at 08:10:25PM +0100, Ilya Leoshkevich wrote:
> Add missing quotes and suppress the $? warnings.
Can you please share the actual shellcheck warnings you see?
>
> Signed-off-by: Ilya Leoshkevich <iii@linux.ibm.com>
> ---
> tools/perf/tests/shell/test_java_symbol.sh | 20 ++++++++++++--------
> 1 file changed, 12 insertions(+), 8 deletions(-)
>
> diff --git a/tools/perf/tests/shell/test_java_symbol.sh b/tools/perf/tests/shell/test_java_symbol.sh
> index 499539d1c4794..b1d7cd43af01a 100755
> --- a/tools/perf/tests/shell/test_java_symbol.sh
> +++ b/tools/perf/tests/shell/test_java_symbol.sh
> @@ -4,6 +4,9 @@
> # SPDX-License-Identifier: GPL-2.0
> # Leo Yan <leo.yan@linaro.org>, 2022
>
> +# Allow [ $? -ne 0 ], because long commands look ugly in if statements.
> +# shellcheck disable=SC2181
> +
> # skip if there's no jshell
> if ! [ -x "$(command -v jshell)" ]; then
> echo "skip: no jshell, install JDK"
> @@ -13,11 +16,12 @@ fi
> PERF_DATA=$(mktemp /tmp/__perf_test.perf.data.XXXXX)
> PERF_INJ_DATA=$(mktemp /tmp/__perf_test.perf.data.inj.XXXXX)
>
> -cleanup_files()
> -{
> +# Shellcheck does not understand that this function is used by a trap.
> +# shellcheck disable=SC2317
> +cleanup_files() {
Please minimize unnecessary changes. It seems you don't need to change
the function declaration.
Thanks,
Namhyung
> echo "Cleaning up files..."
> - rm -f ${PERF_DATA}
> - rm -f ${PERF_INJ_DATA}
> + rm -f "$PERF_DATA"
> + rm -f "$PERF_INJ_DATA"
> }
>
> trap cleanup_files exit term int
> @@ -38,7 +42,7 @@ else
> exit 2
> fi
>
> -cat <<EOF | perf record -k 1 -o $PERF_DATA jshell -s -J-agentpath:$LIBJVMTI
> +cat <<EOF | perf record -k 1 -o "$PERF_DATA" jshell -s -J"-agentpath:$LIBJVMTI"
> int fib(int x) {
> return x > 1 ? fib(x - 2) + fib(x - 1) : 1;
> }
> @@ -56,7 +60,7 @@ if [ $? -ne 0 ]; then
> exit 1
> fi
>
> -if ! DEBUGINFOD_URLS='' perf inject -i $PERF_DATA -o $PERF_INJ_DATA -j; then
> +if ! DEBUGINFOD_URLS='' perf inject -i "$PERF_DATA" -o "$PERF_INJ_DATA" -j; then
> echo "Fail to inject samples"
> exit 1
> fi
> @@ -64,8 +68,8 @@ fi
> # Below is an example of the instruction samples reporting:
> # 8.18% jshell jitted-50116-29.so [.] Interpreter
> # 0.75% Thread-1 jitted-83602-1670.so [.] jdk.internal.jimage.BasicImageReader.getString(int)
> -perf report --stdio -i ${PERF_INJ_DATA} 2>&1 | \
> - grep -E " +[0-9]+\.[0-9]+% .* (Interpreter|jdk\.internal).*" > /dev/null 2>&1
> +perf report --stdio -i "$PERF_INJ_DATA" 2>&1 |
> + grep -E " +[0-9]+\.[0-9]+% .* (Interpreter|jdk\.internal).*" >/dev/null 2>&1
>
> if [ $? -ne 0 ]; then
> echo "Fail to find java symbols"
> --
> 2.51.1
>
^ permalink raw reply [flat|nested] 15+ messages in thread* Re: [PATCH 2/5] perf test java symbol: Get rid of shellcheck warnings
2025-11-07 2:07 ` Namhyung Kim
@ 2025-11-07 7:57 ` Ilya Leoshkevich
0 siblings, 0 replies; 15+ messages in thread
From: Ilya Leoshkevich @ 2025-11-07 7:57 UTC (permalink / raw)
To: Namhyung Kim
Cc: Arnaldo Carvalho de Melo, linux-kernel, linux-perf-users,
Heiko Carstens, Vasily Gorbik, Alexander Gordeev
On Thu, 2025-11-06 at 18:07 -0800, Namhyung Kim wrote:
> Hello,
>
> On Wed, Nov 05, 2025 at 08:10:25PM +0100, Ilya Leoshkevich wrote:
> > Add missing quotes and suppress the $? warnings.
>
> Can you please share the actual shellcheck warnings you see?
There are a lot of them, I have cut similar ones from the output:
linux/tools/perf/tests/shell$ shellcheck -x test_java_symbol.sh
[...]
In test_java_symbol.sh line 19:
rm -f ${PERF_DATA}
^----------------^ SC2317 (info): Command appears to be
unreachable. Check usage (or ignore if invoked indirectly).
^----------^ SC2086 (info): Double quote to prevent
globbing and word splitting.
[...]
In test_java_symbol.sh line 54:
if [ $? -ne 0 ]; then
^-- SC2181 (style): Check exit code directly with e.g. 'if !
mycmd;', not indirectly with $?.
[...]
> > Signed-off-by: Ilya Leoshkevich <iii@linux.ibm.com>
> > ---
> > tools/perf/tests/shell/test_java_symbol.sh | 20 ++++++++++++------
> > --
> > 1 file changed, 12 insertions(+), 8 deletions(-)
> >
> > diff --git a/tools/perf/tests/shell/test_java_symbol.sh
> > b/tools/perf/tests/shell/test_java_symbol.sh
> > index 499539d1c4794..b1d7cd43af01a 100755
> > --- a/tools/perf/tests/shell/test_java_symbol.sh
> > +++ b/tools/perf/tests/shell/test_java_symbol.sh
> > @@ -4,6 +4,9 @@
> > # SPDX-License-Identifier: GPL-2.0
> > # Leo Yan <leo.yan@linaro.org>, 2022
> >
> > +# Allow [ $? -ne 0 ], because long commands look ugly in if
> > statements.
> > +# shellcheck disable=SC2181
> > +
> > # skip if there's no jshell
> > if ! [ -x "$(command -v jshell)" ]; then
> > echo "skip: no jshell, install JDK"
> > @@ -13,11 +16,12 @@ fi
> > PERF_DATA=$(mktemp /tmp/__perf_test.perf.data.XXXXX)
> > PERF_INJ_DATA=$(mktemp /tmp/__perf_test.perf.data.inj.XXXXX)
> >
> > -cleanup_files()
> > -{
> > +# Shellcheck does not understand that this function is used by a
> > trap.
> > +# shellcheck disable=SC2317
> > +cleanup_files() {
>
> Please minimize unnecessary changes. It seems you don't need to
> change
> the function declaration.
My bad, I ran shfmt as well and forgot about this. Interestingly
enough, there appears to be no consensus regarding style, but { on
the same line is a bit more popular:
linux/tools/perf/tests/shell$ grep -R '^[a-zA-Z0-9_]*()$' | wc -l
128
linux/tools/perf/tests/shell$ grep -R '^[a-zA-Z0-9_]*() {$' | wc -l
185
[...]
>
^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH 3/5] perf test java symbol: Extract LIBJVMTI detection
2025-11-05 19:10 [PATCH 0/5] perf jitdump: Fix PID namespace detection Ilya Leoshkevich
2025-11-05 19:10 ` [PATCH 1/5] " Ilya Leoshkevich
2025-11-05 19:10 ` [PATCH 2/5] perf test java symbol: Get rid of shellcheck warnings Ilya Leoshkevich
@ 2025-11-05 19:10 ` Ilya Leoshkevich
2025-11-05 19:10 ` [PATCH 4/5] perf test java symbol: Fix a false negative in symbol regex Ilya Leoshkevich
2025-11-05 19:10 ` [PATCH 5/5] perf test java symbol: Add PID namespace variant Ilya Leoshkevich
4 siblings, 0 replies; 15+ messages in thread
From: Ilya Leoshkevich @ 2025-11-05 19:10 UTC (permalink / raw)
To: Arnaldo Carvalho de Melo
Cc: linux-kernel, linux-perf-users, Heiko Carstens, Vasily Gorbik,
Alexander Gordeev, Ilya Leoshkevich
Extract LIBJVMTI into lib/setup_libjvmti.sh for reuse by future tests.
Signed-off-by: Ilya Leoshkevich <iii@linux.ibm.com>
---
tools/perf/tests/shell/lib/setup_libjvmti.sh | 18 ++++++++++++++++++
tools/perf/tests/shell/test_java_symbol.sh | 17 ++---------------
2 files changed, 20 insertions(+), 15 deletions(-)
create mode 100644 tools/perf/tests/shell/lib/setup_libjvmti.sh
diff --git a/tools/perf/tests/shell/lib/setup_libjvmti.sh b/tools/perf/tests/shell/lib/setup_libjvmti.sh
new file mode 100644
index 0000000000000..fab4a44f8ecb2
--- /dev/null
+++ b/tools/perf/tests/shell/lib/setup_libjvmti.sh
@@ -0,0 +1,18 @@
+#!/bin/bash
+# SPDX-License-Identifier: GPL-2.0
+
+if [ -e "$PWD/tools/perf/libperf-jvmti.so" ]; then
+ LIBJVMTI=$PWD/tools/perf/libperf-jvmti.so
+elif [ -e "$PWD/libperf-jvmti.so" ]; then
+ LIBJVMTI=$PWD/libperf-jvmti.so
+elif [ -e "$PREFIX/lib64/libperf-jvmti.so" ]; then
+ LIBJVMTI=$PREFIX/lib64/libperf-jvmti.so
+elif [ -e "$PREFIX/lib/libperf-jvmti.so" ]; then
+ LIBJVMTI=$PREFIX/lib/libperf-jvmti.so
+elif [ -e "/usr/lib/linux-tools-$(uname -a | awk '{ print $3 }' | sed -r 's/-generic//')/libperf-jvmti.so" ]; then
+ LIBJVMTI=/usr/lib/linux-tools-$(uname -a | awk '{ print $3 }' | sed -r 's/-generic//')/libperf-jvmti.so
+else
+ echo "Fail to find libperf-jvmti.so"
+ # JVMTI is a build option, skip the test if fail to find lib
+ exit 2
+fi
diff --git a/tools/perf/tests/shell/test_java_symbol.sh b/tools/perf/tests/shell/test_java_symbol.sh
index b1d7cd43af01a..f36c9321568c5 100755
--- a/tools/perf/tests/shell/test_java_symbol.sh
+++ b/tools/perf/tests/shell/test_java_symbol.sh
@@ -26,21 +26,8 @@ cleanup_files() {
trap cleanup_files exit term int
-if [ -e "$PWD/tools/perf/libperf-jvmti.so" ]; then
- LIBJVMTI=$PWD/tools/perf/libperf-jvmti.so
-elif [ -e "$PWD/libperf-jvmti.so" ]; then
- LIBJVMTI=$PWD/libperf-jvmti.so
-elif [ -e "$PREFIX/lib64/libperf-jvmti.so" ]; then
- LIBJVMTI=$PREFIX/lib64/libperf-jvmti.so
-elif [ -e "$PREFIX/lib/libperf-jvmti.so" ]; then
- LIBJVMTI=$PREFIX/lib/libperf-jvmti.so
-elif [ -e "/usr/lib/linux-tools-$(uname -a | awk '{ print $3 }' | sed -r 's/-generic//')/libperf-jvmti.so" ]; then
- LIBJVMTI=/usr/lib/linux-tools-$(uname -a | awk '{ print $3 }' | sed -r 's/-generic//')/libperf-jvmti.so
-else
- echo "Fail to find libperf-jvmti.so"
- # JVMTI is a build option, skip the test if fail to find lib
- exit 2
-fi
+# shellcheck source=lib/setup_libjvmti.sh
+. "$(dirname "$0")/lib/setup_libjvmti.sh"
cat <<EOF | perf record -k 1 -o "$PERF_DATA" jshell -s -J"-agentpath:$LIBJVMTI"
int fib(int x) {
--
2.51.1
^ permalink raw reply related [flat|nested] 15+ messages in thread* [PATCH 4/5] perf test java symbol: Fix a false negative in symbol regex
2025-11-05 19:10 [PATCH 0/5] perf jitdump: Fix PID namespace detection Ilya Leoshkevich
` (2 preceding siblings ...)
2025-11-05 19:10 ` [PATCH 3/5] perf test java symbol: Extract LIBJVMTI detection Ilya Leoshkevich
@ 2025-11-05 19:10 ` Ilya Leoshkevich
2025-11-07 2:08 ` Namhyung Kim
2025-11-05 19:10 ` [PATCH 5/5] perf test java symbol: Add PID namespace variant Ilya Leoshkevich
4 siblings, 1 reply; 15+ messages in thread
From: Ilya Leoshkevich @ 2025-11-05 19:10 UTC (permalink / raw)
To: Arnaldo Carvalho de Melo
Cc: linux-kernel, linux-perf-users, Heiko Carstens, Vasily Gorbik,
Alexander Gordeev, Ilya Leoshkevich
There are a lot of symbols like InterpreterRuntime::resolve_get_put()
in the perf report output, so the existing regex unfortunately always
matches something. Replace it with a more precise one.
Signed-off-by: Ilya Leoshkevich <iii@linux.ibm.com>
---
tools/perf/tests/shell/test_java_symbol.sh | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/tools/perf/tests/shell/test_java_symbol.sh b/tools/perf/tests/shell/test_java_symbol.sh
index f36c9321568c5..4c6bc57b87181 100755
--- a/tools/perf/tests/shell/test_java_symbol.sh
+++ b/tools/perf/tests/shell/test_java_symbol.sh
@@ -55,8 +55,10 @@ fi
# Below is an example of the instruction samples reporting:
# 8.18% jshell jitted-50116-29.so [.] Interpreter
# 0.75% Thread-1 jitted-83602-1670.so [.] jdk.internal.jimage.BasicImageReader.getString(int)
+# Look for them, while avoiding false positives from lines like this:
+# 0.03% jshell libjvm.so [.] InterpreterRuntime::resolve_get_put(JavaThread*, Bytecodes::Code)
perf report --stdio -i "$PERF_INJ_DATA" 2>&1 |
- grep -E " +[0-9]+\.[0-9]+% .* (Interpreter|jdk\.internal).*" >/dev/null 2>&1
+ grep ' jshell .* jitted-.*\.so .* \(Interpreter$\|jdk\.internal\)' &>/dev/null
if [ $? -ne 0 ]; then
echo "Fail to find java symbols"
--
2.51.1
^ permalink raw reply related [flat|nested] 15+ messages in thread* Re: [PATCH 4/5] perf test java symbol: Fix a false negative in symbol regex
2025-11-05 19:10 ` [PATCH 4/5] perf test java symbol: Fix a false negative in symbol regex Ilya Leoshkevich
@ 2025-11-07 2:08 ` Namhyung Kim
2025-11-07 7:59 ` Ilya Leoshkevich
0 siblings, 1 reply; 15+ messages in thread
From: Namhyung Kim @ 2025-11-07 2:08 UTC (permalink / raw)
To: Ilya Leoshkevich
Cc: Arnaldo Carvalho de Melo, linux-kernel, linux-perf-users,
Heiko Carstens, Vasily Gorbik, Alexander Gordeev
On Wed, Nov 05, 2025 at 08:10:27PM +0100, Ilya Leoshkevich wrote:
> There are a lot of symbols like InterpreterRuntime::resolve_get_put()
> in the perf report output, so the existing regex unfortunately always
> matches something. Replace it with a more precise one.
>
> Signed-off-by: Ilya Leoshkevich <iii@linux.ibm.com>
> ---
> tools/perf/tests/shell/test_java_symbol.sh | 4 +++-
> 1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/tools/perf/tests/shell/test_java_symbol.sh b/tools/perf/tests/shell/test_java_symbol.sh
> index f36c9321568c5..4c6bc57b87181 100755
> --- a/tools/perf/tests/shell/test_java_symbol.sh
> +++ b/tools/perf/tests/shell/test_java_symbol.sh
> @@ -55,8 +55,10 @@ fi
> # Below is an example of the instruction samples reporting:
> # 8.18% jshell jitted-50116-29.so [.] Interpreter
> # 0.75% Thread-1 jitted-83602-1670.so [.] jdk.internal.jimage.BasicImageReader.getString(int)
> +# Look for them, while avoiding false positives from lines like this:
> +# 0.03% jshell libjvm.so [.] InterpreterRuntime::resolve_get_put(JavaThread*, Bytecodes::Code)
> perf report --stdio -i "$PERF_INJ_DATA" 2>&1 |
> - grep -E " +[0-9]+\.[0-9]+% .* (Interpreter|jdk\.internal).*" >/dev/null 2>&1
> + grep ' jshell .* jitted-.*\.so .* \(Interpreter$\|jdk\.internal\)' &>/dev/null
Maybe 'jshell' part can go away as well.. but it's up to you. :)
Thanks,
Namhyung
>
> if [ $? -ne 0 ]; then
> echo "Fail to find java symbols"
> --
> 2.51.1
>
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 4/5] perf test java symbol: Fix a false negative in symbol regex
2025-11-07 2:08 ` Namhyung Kim
@ 2025-11-07 7:59 ` Ilya Leoshkevich
0 siblings, 0 replies; 15+ messages in thread
From: Ilya Leoshkevich @ 2025-11-07 7:59 UTC (permalink / raw)
To: Namhyung Kim
Cc: Arnaldo Carvalho de Melo, linux-kernel, linux-perf-users,
Heiko Carstens, Vasily Gorbik, Alexander Gordeev
On Thu, 2025-11-06 at 18:08 -0800, Namhyung Kim wrote:
> On Wed, Nov 05, 2025 at 08:10:27PM +0100, Ilya Leoshkevich wrote:
> > There are a lot of symbols like
> > InterpreterRuntime::resolve_get_put()
> > in the perf report output, so the existing regex unfortunately
> > always
> > matches something. Replace it with a more precise one.
> >
> > Signed-off-by: Ilya Leoshkevich <iii@linux.ibm.com>
> > ---
> > tools/perf/tests/shell/test_java_symbol.sh | 4 +++-
> > 1 file changed, 3 insertions(+), 1 deletion(-)
> >
> > diff --git a/tools/perf/tests/shell/test_java_symbol.sh
> > b/tools/perf/tests/shell/test_java_symbol.sh
> > index f36c9321568c5..4c6bc57b87181 100755
> > --- a/tools/perf/tests/shell/test_java_symbol.sh
> > +++ b/tools/perf/tests/shell/test_java_symbol.sh
> > @@ -55,8 +55,10 @@ fi
> > # Below is an example of the instruction samples reporting:
> > # 8.18% jshell jitted-50116-29.so [.] Interpreter
> > # 0.75% Thread-1 jitted-83602-1670.so [.]
> > jdk.internal.jimage.BasicImageReader.getString(int)
> > +# Look for them, while avoiding false positives from lines like
> > this:
> > +# 0.03% jshell libjvm.so [.]
> > InterpreterRuntime::resolve_get_put(JavaThread*, Bytecodes::Code)
> > perf report --stdio -i "$PERF_INJ_DATA" 2>&1 |
> > - grep -E " +[0-9]+\.[0-9]+% .*
> > (Interpreter|jdk\.internal).*" >/dev/null 2>&1
> > + grep ' jshell .* jitted-.*\.so .*
> > \(Interpreter$\|jdk\.internal\)' &>/dev/null
>
> Maybe 'jshell' part can go away as well.. but it's up to you. :)
>
> Thanks,
> Namhyung
You are right, jdk.internal stuff may appear in Thread-1, as one can
see in the existing example. I will drop jshell from the regex.
[...]
>
^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH 5/5] perf test java symbol: Add PID namespace variant
2025-11-05 19:10 [PATCH 0/5] perf jitdump: Fix PID namespace detection Ilya Leoshkevich
` (3 preceding siblings ...)
2025-11-05 19:10 ` [PATCH 4/5] perf test java symbol: Fix a false negative in symbol regex Ilya Leoshkevich
@ 2025-11-05 19:10 ` Ilya Leoshkevich
4 siblings, 0 replies; 15+ messages in thread
From: Ilya Leoshkevich @ 2025-11-05 19:10 UTC (permalink / raw)
To: Arnaldo Carvalho de Melo
Cc: linux-kernel, linux-perf-users, Heiko Carstens, Vasily Gorbik,
Alexander Gordeev, Ilya Leoshkevich
Add a PID namespace variant of test_java_symbol.sh. Compared to the
existing test, we must jump through two extra hoops here.
First, we need to check whether unshare is there an is usable, which
may not be the case either due to kernel config or permissions.
Second, we need to keep jshell running after perf record exits in order
to reproduce a real issue with this setup. This means we have to attach
perf after starting jshell, use ctl-fifo and ack-fifo, and work around
perf's inability to attach to existing children.
Signed-off-by: Ilya Leoshkevich <iii@linux.ibm.com>
---
.../tests/shell/test_java_symbol_pidns.sh | 135 ++++++++++++++++++
1 file changed, 135 insertions(+)
create mode 100755 tools/perf/tests/shell/test_java_symbol_pidns.sh
diff --git a/tools/perf/tests/shell/test_java_symbol_pidns.sh b/tools/perf/tests/shell/test_java_symbol_pidns.sh
new file mode 100755
index 0000000000000..afee6d055ad90
--- /dev/null
+++ b/tools/perf/tests/shell/test_java_symbol_pidns.sh
@@ -0,0 +1,135 @@
+#!/bin/bash
+# Test symbol resolution for java running inside a PID namespace
+# SPDX-License-Identifier: GPL-2.0
+
+set -u -o pipefail
+
+# shellcheck source=lib/setup_libjvmti.sh
+. "$(dirname "$0")/lib/setup_libjvmti.sh"
+
+JSHELL=(jshell -s -J"-agentpath:$LIBJVMTI")
+if ! command -v jshell &>/dev/null; then
+ echo "skip: no jshell, install JDK"
+ exit 2
+fi
+
+UNSHARE=(
+ unshare
+ --user --map-user="$(id -u)" --map-group="$(id -g)"
+ --fork --pid --mount-proc
+)
+if ! "${UNSHARE[@]}" true; then
+ echo "skip: unshare not functional"
+ exit 2
+fi
+
+if [ "$#" -eq 0 ]; then
+ # Verify isolation by running the test inside a separate PID namespace.
+ exec "${UNSHARE[@]}" "$0" "$@" stage2
+fi
+if [ "$1" != stage2 ]; then
+ echo "$0 must be started without arguments"
+ exit 1
+fi
+shift
+
+WORKDIR=$(mktemp -d /tmp/__perf_test_java_symbol_pidns.XXXXX)
+trap 'rm -r "$WORKDIR"' EXIT
+
+test() {
+ TEST_ID=jshell-exit-$JSHELL_EXIT
+
+ # Verify that perf inject can deal with JIT dump produced by jshell
+ # running in PID namespace that is different from its own one.
+ # Make sure jshell is still around when perf inject runs, so attach and
+ # detach perf manually.
+ # jshell is a shell script that runs java as a child process, and perf
+ # cannot attach to existing children. Therefore create a stopped
+ # process, which will exec jshell after it's resumed.
+ JSHELL_STDIN=$WORKDIR/jshell-stdin-$TEST_ID
+ JSHELL_STDOUT=$WORKDIR/jshell-stdout-$TEST_ID
+ mkfifo "$JSHELL_STDIN"
+ mkfifo "$JSHELL_STDOUT"
+ exec {JSHELL_STDIN_FD}<>"$JSHELL_STDIN"
+ exec {JSHELL_STDOUT_FD}<>"$JSHELL_STDOUT"
+ JITDUMPDIR="$WORKDIR" bash -c 'kill -STOP "$$" && exec "$0" "$@"' \
+ "${UNSHARE[@]}" "${JSHELL[@]}" \
+ <&"$JSHELL_STDIN_FD" >&"$JSHELL_STDOUT_FD" &
+ JSHELL_PID=$!
+
+ # Start perf record and wait until it attaches to jshell.
+ PERF_CTL=$WORKDIR/perf-ctl-$TEST_ID
+ PERF_ACK=$WORKDIR/perf-ack-$TEST_ID
+ mkfifo "$PERF_CTL"
+ mkfifo "$PERF_ACK"
+ exec {PERF_CTL_FD}<>"$PERF_CTL"
+ exec {PERF_ACK_FD}<>"$PERF_ACK"
+ PERF_DATA=$WORKDIR/perf-$TEST_ID.data
+ perf record --clockid=1 --delay=-1 \
+ --control="fd:$PERF_CTL_FD,$PERF_ACK_FD" \
+ --output="$PERF_DATA" --pid="$JSHELL_PID" &
+ PERF_PID=$!
+ echo "enable" >&"$PERF_CTL_FD"
+ while IFS= read -r LINE <&"$PERF_ACK_FD"; do
+ [ "$LINE" != "ack" ] || break
+ done
+
+ # Spawn jshell and ask it to run some CPU-intensive code.
+ kill -CONT "$JSHELL_PID"
+ cat >&"$JSHELL_STDIN_FD" <<EOF
+int fib(int x) { return x > 1 ? fib(x - 2) + fib(x - 1) : 1; }
+System.out.println(fib(44));
+EOF
+ while IFS= read -r LINE <&"$JSHELL_STDOUT_FD"; do
+ [ "$LINE" != 1134903170 ] || break
+ done
+
+ # Terminate perf record.
+ echo "stop" >"$PERF_CTL"
+ if ! wait "$PERF_PID"; then
+ echo "Fail to record for java program"
+ exit 1
+ fi
+
+ # Terminate jshell before perf inject.
+ if [ "$JSHELL_EXIT" = "early" ]; then
+ echo "/exit" >&"$JSHELL_STDIN_FD"
+ if ! wait "$JSHELL_PID"; then
+ echo "Fail to run java program"
+ exit 1
+ fi
+ fi
+
+ # Inject the JIT data.
+ PERF_INJ_DATA=$WORKDIR/perf-$TEST_ID.data.inj
+ if ! DEBUGINFOD_URLS="" perf inject --input="$PERF_DATA" \
+ --output="$PERF_INJ_DATA" --jit; then
+ echo "Fail to inject samples"
+ exit 1
+ fi
+
+ # Below is an example of the instruction samples reporting:
+ # 8.18% jshell jitted-50116-29.so [.] Interpreter
+ # 0.75% Thread-1 jitted-83602-1670.so [.] jdk.internal.jimage.BasicImageReader.getString(int)
+ # Look for them, while avoiding false positives from lines like this:
+ # 0.03% jshell libjvm.so [.] InterpreterRuntime::resolve_get_put(JavaThread*, Bytecodes::Code)
+ if ! perf report --input="$PERF_INJ_DATA" --stdio |
+ grep ' jshell .* jitted-.*\.so .* \(Interpreter$\|jdk\.internal\)' \
+ &>/dev/null; then
+ echo "Fail to find java symbols"
+ exit 1
+ fi
+
+ # Terminate jshell after perf inject.
+ if [ "$JSHELL_EXIT" = "late" ]; then
+ echo "/exit" >&"$JSHELL_STDIN_FD"
+ if ! wait "$JSHELL_PID"; then
+ echo "Fail to run java program"
+ exit 1
+ fi
+ fi
+}
+
+for JSHELL_EXIT in early late; do
+ test
+done
--
2.51.1
^ permalink raw reply related [flat|nested] 15+ messages in thread