* [PATCH 1/4] perf annotate: Set error stream of objdump process for TUI
2022-02-02 7:08 [PATCH 0/4] Handle chroot tasks properly (v2) Namhyung Kim
@ 2022-02-02 7:08 ` Namhyung Kim
2022-02-02 7:08 ` [PATCH 2/4] perf tools: Try chroot'ed filename when opening dso/symbol Namhyung Kim
` (3 subsequent siblings)
4 siblings, 0 replies; 7+ messages in thread
From: Namhyung Kim @ 2022-02-02 7:08 UTC (permalink / raw)
To: Arnaldo Carvalho de Melo, Jiri Olsa
Cc: Ingo Molnar, Peter Zijlstra, LKML, Andi Kleen, Ian Rogers
The stderr should be set to a pipe when using TUI. Otherwise it'd
print to stdout and break TUI windows with an error message.
Signed-off-by: Namhyung Kim <namhyung@kernel.org>
---
tools/perf/util/annotate.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/tools/perf/util/annotate.c b/tools/perf/util/annotate.c
index 01900689dc00..8190a124b99d 100644
--- a/tools/perf/util/annotate.c
+++ b/tools/perf/util/annotate.c
@@ -2036,6 +2036,7 @@ static int symbol__disassemble(struct symbol *sym, struct annotate_args *args)
memset(&objdump_process, 0, sizeof(objdump_process));
objdump_process.argv = objdump_argv;
objdump_process.out = -1;
+ objdump_process.err = -1;
if (start_command(&objdump_process)) {
pr_err("Failure starting to run %s\n", command);
err = -1;
--
2.35.0.rc2.247.g8bbb082509-goog
^ permalink raw reply related [flat|nested] 7+ messages in thread* [PATCH 2/4] perf tools: Try chroot'ed filename when opening dso/symbol
2022-02-02 7:08 [PATCH 0/4] Handle chroot tasks properly (v2) Namhyung Kim
2022-02-02 7:08 ` [PATCH 1/4] perf annotate: Set error stream of objdump process for TUI Namhyung Kim
@ 2022-02-02 7:08 ` Namhyung Kim
2022-02-02 7:08 ` [PATCH 3/4] perf inject: Try chroot directory when reading build-id Namhyung Kim
` (2 subsequent siblings)
4 siblings, 0 replies; 7+ messages in thread
From: Namhyung Kim @ 2022-02-02 7:08 UTC (permalink / raw)
To: Arnaldo Carvalho de Melo, Jiri Olsa
Cc: Ingo Molnar, Peter Zijlstra, LKML, Andi Kleen, Ian Rogers, Jiri Olsa
Currently it doesn't handle tasks in chroot properly. As filenames in
MMAP records base on their root directory, it's different than what
perf tool can see from outside.
Add filename_with_chroot() helper to deal with those cases. The
function returns a new filename only if it's in a different root
directory. Since it needs to access /proc for the process, it only
works until the task exits.
With this change, I can see symbols in my program like below.
# perf record -o- chroot myroot myprog 3 | perf report -i-
...
#
# Overhead Command Shared Object Symbol
# ........ ....... ................. .............................
#
99.83% myprog myprog [.] loop
0.04% chroot [kernel.kallsyms] [k] fxregs_fixup
0.04% chroot [kernel.kallsyms] [k] rsm_load_seg_32
...
Acked-by: Jiri Olsa <jolsa@kernel.org>
Signed-off-by: Namhyung Kim <namhyung@kernel.org>
---
tools/perf/util/dso.c | 15 +++++++++++++--
tools/perf/util/dsos.c | 13 +++++++++++++
tools/perf/util/symbol.c | 10 ++++++++++
tools/perf/util/util.c | 31 +++++++++++++++++++++++++++++++
tools/perf/util/util.h | 2 ++
5 files changed, 69 insertions(+), 2 deletions(-)
diff --git a/tools/perf/util/dso.c b/tools/perf/util/dso.c
index 9cc8a1772b4b..5ac13958d1bd 100644
--- a/tools/perf/util/dso.c
+++ b/tools/perf/util/dso.c
@@ -508,8 +508,19 @@ static int __open_dso(struct dso *dso, struct machine *machine)
root_dir, name, PATH_MAX))
goto out;
- if (!is_regular_file(name))
- goto out;
+ if (!is_regular_file(name)) {
+ char *new_name;
+
+ if (errno != ENOENT || dso->nsinfo == NULL)
+ goto out;
+
+ new_name = filename_with_chroot(dso->nsinfo->pid, name);
+ if (!new_name)
+ goto out;
+
+ free(name);
+ name = new_name;
+ }
if (dso__needs_decompress(dso)) {
char newpath[KMOD_DECOMP_LEN];
diff --git a/tools/perf/util/dsos.c b/tools/perf/util/dsos.c
index 183a81d5b2f9..b97366f77bbf 100644
--- a/tools/perf/util/dsos.c
+++ b/tools/perf/util/dsos.c
@@ -2,12 +2,15 @@
#include "debug.h"
#include "dsos.h"
#include "dso.h"
+#include "util.h"
#include "vdso.h"
#include "namespaces.h"
+#include <errno.h>
#include <libgen.h>
#include <stdlib.h>
#include <string.h>
#include <symbol.h> // filename__read_build_id
+#include <unistd.h>
static int __dso_id__cmp(struct dso_id *a, struct dso_id *b)
{
@@ -76,6 +79,16 @@ bool __dsos__read_build_ids(struct list_head *head, bool with_hits)
if (filename__read_build_id(pos->long_name, &pos->bid) > 0) {
have_build_id = true;
pos->has_build_id = true;
+ } else if (errno == ENOENT && pos->nsinfo) {
+ char *new_name = filename_with_chroot(pos->nsinfo->pid,
+ pos->long_name);
+
+ if (new_name && filename__read_build_id(new_name,
+ &pos->bid) > 0) {
+ have_build_id = true;
+ pos->has_build_id = true;
+ }
+ free(new_name);
}
nsinfo__mountns_exit(&nsc);
}
diff --git a/tools/perf/util/symbol.c b/tools/perf/util/symbol.c
index b2ed3140a1fa..34302c61d36b 100644
--- a/tools/perf/util/symbol.c
+++ b/tools/perf/util/symbol.c
@@ -1864,6 +1864,16 @@ int dso__load(struct dso *dso, struct map *map)
nsinfo__mountns_exit(&nsc);
is_reg = is_regular_file(name);
+ if (!is_reg && errno == ENOENT && dso->nsinfo) {
+ char *new_name = filename_with_chroot(dso->nsinfo->pid,
+ name);
+ if (new_name) {
+ is_reg = is_regular_file(new_name);
+ strlcpy(name, new_name, PATH_MAX);
+ free(new_name);
+ }
+ }
+
#ifdef HAVE_LIBBFD_SUPPORT
if (is_reg)
bfdrc = dso__load_bfd_symbols(dso, name);
diff --git a/tools/perf/util/util.c b/tools/perf/util/util.c
index fb4f6616b5fa..f8571a66d063 100644
--- a/tools/perf/util/util.c
+++ b/tools/perf/util/util.c
@@ -431,3 +431,34 @@ void perf_debuginfod_setup(struct perf_debuginfod *di)
pr_debug("DEBUGINFOD_URLS=%s\n", getenv("DEBUGINFOD_URLS"));
}
+
+/*
+ * Return a new filename prepended with task's root directory if it's in
+ * a chroot. Callers should free the returned string.
+ */
+char *filename_with_chroot(int pid, const char *filename)
+{
+ char buf[PATH_MAX];
+ char proc_root[32];
+ char *new_name = NULL;
+ int ret;
+
+ scnprintf(proc_root, sizeof(proc_root), "/proc/%d/root", pid);
+ ret = readlink(proc_root, buf, sizeof(buf) - 1);
+ if (ret <= 0)
+ return NULL;
+
+ /* readlink(2) does not append a null byte to buf */
+ buf[ret] = '\0';
+
+ if (!strcmp(buf, "/"))
+ return NULL;
+
+ if (strstr(buf, "(deleted)"))
+ return NULL;
+
+ if (asprintf(&new_name, "%s/%s", buf, filename) < 0)
+ return NULL;
+
+ return new_name;
+}
diff --git a/tools/perf/util/util.h b/tools/perf/util/util.h
index 7b625cbd2dd8..0f78f1e7782d 100644
--- a/tools/perf/util/util.h
+++ b/tools/perf/util/util.h
@@ -77,4 +77,6 @@ struct perf_debuginfod {
bool set;
};
void perf_debuginfod_setup(struct perf_debuginfod *di);
+
+char *filename_with_chroot(int pid, const char *filename);
#endif /* GIT_COMPAT_UTIL_H */
--
2.35.0.rc2.247.g8bbb082509-goog
^ permalink raw reply related [flat|nested] 7+ messages in thread* [PATCH 3/4] perf inject: Try chroot directory when reading build-id
2022-02-02 7:08 [PATCH 0/4] Handle chroot tasks properly (v2) Namhyung Kim
2022-02-02 7:08 ` [PATCH 1/4] perf annotate: Set error stream of objdump process for TUI Namhyung Kim
2022-02-02 7:08 ` [PATCH 2/4] perf tools: Try chroot'ed filename when opening dso/symbol Namhyung Kim
@ 2022-02-02 7:08 ` Namhyung Kim
2022-02-02 7:08 ` [PATCH 4/4] perf annotate: Try chroot filename for objdump Namhyung Kim
2022-02-10 18:34 ` [PATCH 0/4] Handle chroot tasks properly (v2) Arnaldo Carvalho de Melo
4 siblings, 0 replies; 7+ messages in thread
From: Namhyung Kim @ 2022-02-02 7:08 UTC (permalink / raw)
To: Arnaldo Carvalho de Melo, Jiri Olsa
Cc: Ingo Molnar, Peter Zijlstra, LKML, Andi Kleen, Ian Rogers, Jiri Olsa
When reading build-id from a DSO, it should consider if it's from a
chroot task. In that case, the path is different so it needs to
prepend the root directory to access the file correctly.
Acked-by: Jiri Olsa <jolsa@kernel.org>
Signed-off-by: Namhyung Kim <namhyung@kernel.org>
---
tools/perf/builtin-inject.c | 10 ++++++++++
1 file changed, 10 insertions(+)
diff --git a/tools/perf/builtin-inject.c b/tools/perf/builtin-inject.c
index fbf43a454cba..3be581abbdb6 100644
--- a/tools/perf/builtin-inject.c
+++ b/tools/perf/builtin-inject.c
@@ -25,6 +25,7 @@
#include "util/synthetic-events.h"
#include "util/thread.h"
#include "util/namespaces.h"
+#include "util/util.h"
#include <linux/err.h>
#include <subcmd/parse-options.h>
@@ -550,6 +551,15 @@ static int dso__read_build_id(struct dso *dso)
nsinfo__mountns_enter(dso->nsinfo, &nsc);
if (filename__read_build_id(dso->long_name, &dso->bid) > 0)
dso->has_build_id = true;
+ else if (dso->nsinfo) {
+ char *new_name;
+
+ new_name = filename_with_chroot(dso->nsinfo->pid,
+ dso->long_name);
+ if (new_name && filename__read_build_id(new_name, &dso->bid) > 0)
+ dso->has_build_id = true;
+ free(new_name);
+ }
nsinfo__mountns_exit(&nsc);
return dso->has_build_id ? 0 : -1;
--
2.35.0.rc2.247.g8bbb082509-goog
^ permalink raw reply related [flat|nested] 7+ messages in thread* [PATCH 4/4] perf annotate: Try chroot filename for objdump
2022-02-02 7:08 [PATCH 0/4] Handle chroot tasks properly (v2) Namhyung Kim
` (2 preceding siblings ...)
2022-02-02 7:08 ` [PATCH 3/4] perf inject: Try chroot directory when reading build-id Namhyung Kim
@ 2022-02-02 7:08 ` Namhyung Kim
2022-02-10 18:34 ` [PATCH 0/4] Handle chroot tasks properly (v2) Arnaldo Carvalho de Melo
4 siblings, 0 replies; 7+ messages in thread
From: Namhyung Kim @ 2022-02-02 7:08 UTC (permalink / raw)
To: Arnaldo Carvalho de Melo, Jiri Olsa
Cc: Ingo Molnar, Peter Zijlstra, LKML, Andi Kleen, Ian Rogers, Jiri Olsa
Likewise, it should use a proper name in case the task runs under
chroot.
Acked-by: Jiri Olsa <jolsa@kernel.org>
Signed-off-by: Namhyung Kim <namhyung@kernel.org>
---
tools/perf/util/annotate.c | 10 ++++++++++
1 file changed, 10 insertions(+)
diff --git a/tools/perf/util/annotate.c b/tools/perf/util/annotate.c
index 8190a124b99d..e4c641b240df 100644
--- a/tools/perf/util/annotate.c
+++ b/tools/perf/util/annotate.c
@@ -33,6 +33,7 @@
#include "string2.h"
#include "util/event.h"
#include "arch/common.h"
+#include "namespaces.h"
#include <regex.h>
#include <pthread.h>
#include <linux/bitops.h>
@@ -1696,6 +1697,15 @@ static int dso__disassemble_filename(struct dso *dso, char *filename, size_t fil
* DSO is the same as when 'perf record' ran.
*/
__symbol__join_symfs(filename, filename_size, dso->long_name);
+
+ if (access(filename, R_OK) && errno == ENOENT && dso->nsinfo) {
+ char *new_name = filename_with_chroot(dso->nsinfo->pid,
+ filename);
+ if (new_name) {
+ strlcpy(filename, new_name, filename_size);
+ free(new_name);
+ }
+ }
}
free(build_id_path);
--
2.35.0.rc2.247.g8bbb082509-goog
^ permalink raw reply related [flat|nested] 7+ messages in thread* Re: [PATCH 0/4] Handle chroot tasks properly (v2)
2022-02-02 7:08 [PATCH 0/4] Handle chroot tasks properly (v2) Namhyung Kim
` (3 preceding siblings ...)
2022-02-02 7:08 ` [PATCH 4/4] perf annotate: Try chroot filename for objdump Namhyung Kim
@ 2022-02-10 18:34 ` Arnaldo Carvalho de Melo
2022-02-11 6:16 ` Namhyung Kim
4 siblings, 1 reply; 7+ messages in thread
From: Arnaldo Carvalho de Melo @ 2022-02-10 18:34 UTC (permalink / raw)
To: Namhyung Kim
Cc: Jiri Olsa, Ingo Molnar, Peter Zijlstra, LKML, Andi Kleen, Ian Rogers
Em Tue, Feb 01, 2022 at 11:08:24PM -0800, Namhyung Kim escreveu:
> Hello,
>
> I found that perf tools don't work well with tasks in a chroot. The
> filenames in MMAP record are from the root directory of the task so
> it's different than what it sees from outside.
>
> * changes in v2)
> - add Jiri's Acked-by
> - split stderr setup for objdump
Applied 1/4 to perf/urgent, its already upstream.
The others are bit big for this time, so I've added it to perf/core.
Thanks,
- Arnaldo
> I've tested it with a simple program (myprog - statically built) which
> just consumes cpu cycles in a loop for a while (default 1 sec, can by
> overridden by a command-line argument).
>
> # cd $HOME
> # mkdir -p myroot/bin
> # cp myprog myroot/bin
>
> # perf record chroot myroot myprog
> # perf report -D | grep MMAP | grep myprog
> 2084916774977911 0xa2e4 [0x70]: PERF_RECORD_MMAP2 3363818/3363818: \
> [0x401000(0x80000) @ 0x1000 fe:01 4346398 2543719070]: r-xp /bin/myprog
>
> So it's reported as /bin/myprog and then it's unable to symbolize the
> samples. It seems hard to fix it for the above use case as the record
> ended after the task exited. It cannot know the root directory of the
> task anymore. But we can fix it for real-time use cases like perf top
> or pipe-mode at least.
>
> I tested it again with the following command.
>
> # perf record -o- chroot myroot myprog | perf report -i-
> ...
> #
> # Overhead Command Shared Object Symbol
> # ........ ....... ................. ................................
> #
> 46.02% myprog myprog [.] 0x000000000000178a
> 36.71% myprog myprog [.] 0x0000000000001792
> 17.12% myprog myprog [.] 0x000000000000178e
> 0.05% myprog myprog [.] 0x0000000000001796
> 0.05% chroot ld-2.33.so [.] intel_check_word.constprop.0
>
> The symbols are not resolved because it failed to load the symbol
> table as it didn't find the file in the given path.
>
> So I modified the code to try a new name prepended with the task's
> root directory when it's not "/". With this change, I can see the
> symbols of myprog. In fact, it depends on timing if perf report hits
> the file before the task is gone. Increasing the loop time to 3 sec
> helped it to get symbols reliably.
>
> # perf record -o- chroot myroot myprog 3 | perf report -i-
> ...
> #
> # Overhead Command Shared Object Symbol
> # ........ ....... ................. .............................
> #
> 99.83% myprog myprog [.] loop
> 0.04% chroot [kernel.kallsyms] [k] fxregs_fixup
> 0.04% chroot [kernel.kallsyms] [k] rsm_load_seg_32
> 0.03% chroot [kernel.kallsyms] [k] show_cpuinfo_cur_freq
> 0.01% myprog [kernel.kallsyms] [k] alarmtimer_fired
>
>
> I also found that perf inject and perf annotate need the similar changes.
>
> You can get it from 'perf/dso-chroot-v2' branch at
>
> git://git.kernel.org/pub/scm/linux/kernel/git/namhyung/linux-perf.git
>
>
> Thanks,
> Namhyung
>
>
> Namhyung Kim (4):
> perf annotate: Set error stream of objdump process for TUI
> perf tools: Try chroot'ed filename when opening dso/symbol
> perf inject: Try chroot directory when reading build-id
> perf annotate: Try chroot filename for objdump
>
> tools/perf/builtin-inject.c | 10 ++++++++++
> tools/perf/util/annotate.c | 11 +++++++++++
> tools/perf/util/dso.c | 15 +++++++++++++--
> tools/perf/util/dsos.c | 13 +++++++++++++
> tools/perf/util/symbol.c | 10 ++++++++++
> tools/perf/util/util.c | 31 +++++++++++++++++++++++++++++++
> tools/perf/util/util.h | 2 ++
> 7 files changed, 90 insertions(+), 2 deletions(-)
>
>
> base-commit: e783362eb54cd99b2cac8b3a9aeac942e6f6ac07
> --
> 2.35.0.rc2.247.g8bbb082509-goog
--
- Arnaldo
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: [PATCH 0/4] Handle chroot tasks properly (v2)
2022-02-10 18:34 ` [PATCH 0/4] Handle chroot tasks properly (v2) Arnaldo Carvalho de Melo
@ 2022-02-11 6:16 ` Namhyung Kim
0 siblings, 0 replies; 7+ messages in thread
From: Namhyung Kim @ 2022-02-11 6:16 UTC (permalink / raw)
To: Arnaldo Carvalho de Melo
Cc: Jiri Olsa, Ingo Molnar, Peter Zijlstra, LKML, Andi Kleen, Ian Rogers
Hi Arnaldo,
On Thu, Feb 10, 2022 at 10:35 AM Arnaldo Carvalho de Melo
<acme@kernel.org> wrote:
>
> Em Tue, Feb 01, 2022 at 11:08:24PM -0800, Namhyung Kim escreveu:
> > Hello,
> >
> > I found that perf tools don't work well with tasks in a chroot. The
> > filenames in MMAP record are from the root directory of the task so
> > it's different than what it sees from outside.
> >
> > * changes in v2)
> > - add Jiri's Acked-by
> > - split stderr setup for objdump
>
> Applied 1/4 to perf/urgent, its already upstream.
>
> The others are bit big for this time, so I've added it to perf/core.
Perfect, thanks for your work!
Namhyung
^ permalink raw reply [flat|nested] 7+ messages in thread