Message-ID: <CABWYdi1+E7MQD8mC2xQfSP0m9_WFdx9mbLkw-36tJ8EtLaw2Jg@mail.gmail.com>
Date:   Thu, 5 Dec 2019 18:17:23 -0800
From:   Ivan Babrou <ivan@...udflare.com>
To:     Arnaldo Carvalho de Melo <arnaldo.melo@...il.com>
Cc:     linux-kernel <linux-kernel@...r.kernel.org>,
        kernel-team <kernel-team@...udflare.com>,
        Jiri Olsa <jolsa@...hat.com>,
        Peter Zijlstra <peterz@...radead.org>,
        Ingo Molnar <mingo@...hat.com>,
        Alexander Shishkin <alexander.shishkin@...ux.intel.com>,
        Namhyung Kim <namhyung@...nel.org>, sashal@...nel.org,
        Kenton Varda <kenton@...udflare.com>
Subject: Re: perf not picking up symbols for namespaced processes

I'm not very good at this, but the following works for me. If you this
is in general vicinity of what you expected, I can email patch
properly.

Initially I hoped that setting dso->nsinfo->need_setns to false in
dso_open would do the trick, but it did not work.

$ cat 0001-perf-fallback-to-opening-dso-from-outside-of-mount-n.patch
| sed 's/\t/        /g'
>From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
From: Ivan Babrou <ivan@...udflare.com>
Date: Thu, 5 Dec 2019 16:27:48 -0800
Subject: [PATCH] perf: fallback to opening dso from outside of mount namespace

Some tasks enter mount namespace for isolation and this fallback
allows perf to read symbols from binaries that live outside of
mount namespace of the running task.

Signed-off-by: Ivan Babrou <ivan@...udflare.com>
---
 tools/perf/util/dso.c    |  7 +++++++
 tools/perf/util/symbol.c | 20 +++++++++++++++-----
 2 files changed, 22 insertions(+), 5 deletions(-)

diff --git a/tools/perf/util/dso.c b/tools/perf/util/dso.c
index e11ddf86f2b3..dac6bf42e43e 100644
--- a/tools/perf/util/dso.c
+++ b/tools/perf/util/dso.c
@@ -527,6 +527,13 @@ static int open_dso(struct dso *dso, struct
machine *machine)
         fd = __open_dso(dso, machine);
         if (dso->binary_type != DSO_BINARY_TYPE__BUILD_ID_CACHE)
                 nsinfo__mountns_exit(&nsc);
+
+        if (fd < 0) {
+                fd = __open_dso(dso, machine);
+                if (fd >= 0) {
+                        pr_warning("Using debug info for %s from
outside of its active mount namespace.\n", dso->long_name);
+                }
+        }

         if (fd >= 0) {
                 dso__list_add(dso);
diff --git a/tools/perf/util/symbol.c b/tools/perf/util/symbol.c
index a8f80e427674..e85d57dfcc14 100644
--- a/tools/perf/util/symbol.c
+++ b/tools/perf/util/symbol.c
@@ -1679,11 +1679,21 @@ int dso__load(struct dso *dso, struct map *map)
          * Read the build id if possible. This is required for
          * DSO_BINARY_TYPE__BUILDID_DEBUGINFO to work
          */
-        if (!dso->has_build_id &&
-            is_regular_file(dso->long_name)) {
-            __symbol__join_symfs(name, PATH_MAX, dso->long_name);
-            if (filename__read_build_id(name, build_id, BUILD_ID_SIZE) > 0)
-                dso__set_build_id(dso, build_id);
+        if (!dso->has_build_id) {
+            bool is_reg = is_regular_file(dso->long_name);
+            if (!is_reg) {
+                nsinfo__mountns_exit(&nsc);
+                is_reg = is_regular_file(dso->long_name);
+                if (!is_reg) {
+                    nsinfo__mountns_enter(dso->nsinfo, &nsc);
+                }
+            }
+
+            if (is_reg) {
+                __symbol__join_symfs(name, PATH_MAX, dso->long_name);
+                if (filename__read_build_id(name, build_id, BUILD_ID_SIZE) > 0)
+                    dso__set_build_id(dso, build_id);
+            }
         }

         /*
--
2.24.0

  /*

--
2.24.0

On Thu, Dec 5, 2019 at 4:33 AM Arnaldo Carvalho de Melo
<arnaldo.melo@...il.com> wrote:
>
> Em Wed, Dec 04, 2019 at 07:46:10PM -0800, Ivan Babrou escreveu:
> > We have a service that forks a child process in a namespace-based
> > sandbox where the mount namespace is intentionally designed to reflect
> > a totally empty filesystem. Our use case is very similar to Chrome's
> > sandbox, for example, but on a server. Within the sandbox, not even
> > the service's own binary is present in the mount namespace.
> >
> > Process tree looks like this:
> >
> > $ sudo pstree -psc 63989
> > edgeworker(63989)─┬─edgeworker/sbox(255716)─┬─edgeworker/zygt(255718)
> >                    │                         ├─{edgeworker/sbox}(255719)
> >                    │                         ├─{edgeworker/sbox}(255720)
> >                    │                         ├─{edgeworker/sbox}(255721)
> >                    ├─edgeworker/stry(5803)
> >                    ├─edgeworker/stry(63990)
> >                    ├─edgeworker/stry(106218)
> >                    ├─edgeworker/stry(191905)
> >                    ├─edgeworker/stry(255695)
> >                    ├─edgeworker/supr(255717)
> >
> > Here sbox processes do actual work living in an empty mount namespaces
> > and stry is a helper process for error reporting. All tasks come from
> > the same binary that lives in the root mount namespace, launched by
> > systemd.
> >
> > During "perf script" run on a trace obtained from the system there are
> > these possible outcomes:
> >
> > 1. The first pid to be processed is a non-namespaced helper and
> > symbols are present.
> > 2. The first pid is not found and symbols are present.
> > 3. The first pid is a sandboxed task and symbols are missing.
> >
> > Symbols are missing, because "perf script" tries to jump into an empty
> > sandbox and find a binary there, when in fact it lives outside:
> >
> > getcwd("/state/home/ivan", 4096)        = 17
> > open("/proc/self/ns/mnt", O_RDONLY)     = 5
> > open("/proc/255719/ns/mnt", O_RDONLY)   = 6
> > setns(6, CLONE_NEWNS)                   = 0
> > stat("/usr/local/bin/edgeworker", 0x7ffedb9b3ca0) = -1 ENOENT (No such
> > file or directory)
> >
> > In the second outcome we don't have a PID to figure out the namespace
> > to jump into, so this doesn't happen. It's a good fallback, but it was
> > a bit confusing during debugging.
> >
> > It's not entirely clear to me why sometimes a helper PID is picked,
> > even though it's not the first sample in the recorded trace (at least
> > not in the output). This happens deterministically, or at least
> > appears so. In my process tree it's 255695.
> >
> > I think perf should try to fallback to the default namespace to look
> > up symbols if they are not found inside to cover our case. Relevant
> > piece of logic is here:
>
> That should work for your use case, as you're sure that looking up by
> pathname only will find, outside the namespace, the binary you want.
>
> Even with pathname based looukups being fragile, it works for your
> usecase, so please consider providing a patch for such fallback,
> together with a pr_debug() or even pr_warning() if this don't get too
> noisy, to warn the user.
>
> - Arnaldo
>
> > * https://elixir.free-electrons.com/linux/v5.4.1/source/tools/perf/util/dso.c#L520
>
> --
>
> - Arnaldo

Powered by blists - more mailing lists