[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20170703184434.GB27350@kernel.org>
Date: Mon, 3 Jul 2017 15:44:34 -0300
From: Arnaldo Carvalho de Melo <acme@...nel.org>
To: Krister Johansen <kjlx@...pleofstupid.com>
Cc: Peter Zijlstra <peterz@...radead.org>,
Ingo Molnar <mingo@...hat.com>,
Alexander Shishkin <alexander.shishkin@...ux.intel.com>,
linux-kernel@...r.kernel.org
Subject: Re: [PATCH tip/perf/core 2/7] perf maps: lookup maps in both
intitial mountns and inner mountns.
Em Fri, Jun 30, 2017 at 07:18:54PM -0700, Krister Johansen escreveu:
> If a process is in a mountns and has symbols in /tmp/perf-<pid>.map,
> look first in the namespace using the tgid for the pidns that the
> process might be in. If the map isn't found there, try looking in
> the mountns where perf is running, and use the tgid that's appropriate
> for perf's pid namespace. If all else fails, use the original pid.
>
> This allows us to locate a symbol map file in the mount namespace, if
> it was generated there. However, we also try the tool's /tmp in case
> it's there instead.
>
> Signed-off-by: Krister Johansen <kjlx@...pleofstupid.com>
> ---
> tools/perf/util/machine.c | 19 ++++------
> tools/perf/util/map.c | 29 ++++++++++++----
> tools/perf/util/map.h | 8 ++---
> tools/perf/util/namespaces.c | 83 ++++++++++++++++++++++++++++++++++++++++----
> tools/perf/util/namespaces.h | 5 ++-
> tools/perf/util/symbol.c | 71 ++++++++++++++++++++++++++++++-------
> 6 files changed, 172 insertions(+), 43 deletions(-)
>
> diff --git a/tools/perf/util/machine.c b/tools/perf/util/machine.c
> index d7f31cb..ed98881 100644
> --- a/tools/perf/util/machine.c
> +++ b/tools/perf/util/machine.c
> @@ -1388,13 +1388,10 @@ int machine__process_mmap2_event(struct machine *machine,
> else
> type = MAP__FUNCTION;
>
> - map = map__new(machine, event->mmap2.start,
> - event->mmap2.len, event->mmap2.pgoff,
> - event->mmap2.pid, event->mmap2.maj,
> - event->mmap2.min, event->mmap2.ino,
> - event->mmap2.ino_generation,
> - event->mmap2.prot,
> - event->mmap2.flags,
> + map = map__new(machine, event->mmap2.start, event->mmap2.len,
> + event->mmap2.pgoff, event->mmap2.maj, event->mmap2.min,
> + event->mmap2.ino, event->mmap2.ino_generation,
> + event->mmap2.prot, event->mmap2.flags,
> event->mmap2.filename, type, thread);
Try not to reflow like that, it makes it harder to spot _just what
changed_, so you just removed the .pid parameter, ok, it should be:
map = map__new(machine, event->mmap2.start,
event->mmap2.len, event->mmap2.pgoff,
- event->mmap2.pid, event->mmap2.maj,
+ event->mmap2.maj,
event->mmap2.min, event->mmap2.ino,
event->mmap2.ino_generation,
event->mmap2.prot,
event->mmap2.flags,
event->mmap2.filename, type, thread);
>
> if (map == NULL)
> @@ -1446,11 +1443,9 @@ int machine__process_mmap_event(struct machine *machine, union perf_event *event
> else
> type = MAP__FUNCTION;
>
> - map = map__new(machine, event->mmap.start,
> - event->mmap.len, event->mmap.pgoff,
> - event->mmap.pid, 0, 0, 0, 0, 0, 0,
> - event->mmap.filename,
> - type, thread);
> + map = map__new(machine, event->mmap.start, event->mmap.len,
> + event->mmap.pgoff, 0, 0, 0, 0, 0, 0,
> + event->mmap.filename, type, thread);
Ditto.
>
> if (map == NULL)
> goto out_problem_map;
> diff --git a/tools/perf/util/map.c b/tools/perf/util/map.c
> index 5dc60ca..2bd20cb 100644
> --- a/tools/perf/util/map.c
> +++ b/tools/perf/util/map.c
> @@ -145,12 +145,14 @@ void map__init(struct map *map, enum map_type type,
> refcount_set(&map->refcnt, 1);
> }
>
> -struct map *map__new(struct machine *machine, u64 start, u64 len,
> - u64 pgoff, u32 pid, u32 d_maj, u32 d_min, u64 ino,
> - u64 ino_gen, u32 prot, u32 flags, char *filename,
> - enum map_type type, struct thread *thread)
> +struct map *map__new(struct machine *machine, u64 start, u64 len, u64 pgoff,
> + u32 d_maj, u32 d_min, u64 ino, u64 ino_gen, u32 prot,
> + u32 flags, char *filename, enum map_type type,
> + struct thread *thread)
Ditto
> {
> struct map *map = malloc(sizeof(*map));
> + struct nsinfo *nsi = NULL;
> + struct nsinfo *nnsi;
>
> if (map != NULL) {
> char newfilename[PATH_MAX];
> @@ -168,9 +170,11 @@ struct map *map__new(struct machine *machine, u64 start, u64 len,
> map->ino_generation = ino_gen;
> map->prot = prot;
> map->flags = flags;
> + nsi = nsinfo__get(thread->nsinfo);
>
> - if ((anon || no_dso) && type == MAP__FUNCTION) {
> - snprintf(newfilename, sizeof(newfilename), "/tmp/perf-%d.map", pid);
> + if ((anon || no_dso) && nsi && type == MAP__FUNCTION) {
> + snprintf(newfilename, sizeof(newfilename),
> + "/tmp/perf-%d.map", nsi->pid);
> filename = newfilename;
> }
>
> @@ -180,6 +184,16 @@ struct map *map__new(struct machine *machine, u64 start, u64 len,
> }
>
> if (vdso) {
> + /* The vdso maps are always on the host and not the
> + * container. Ensure that we don't use setns to look
> + * them up.
> + */
> + nnsi = nsinfo__copy(nsi);
> + if (nnsi) {
> + nsinfo__put(nsi);
> + nnsi->need_setns = false;
> + nsi = nnsi;
> + }
> pgoff = 0;
> dso = machine__findnew_vdso(machine, thread);
> } else
> @@ -201,11 +215,12 @@ struct map *map__new(struct machine *machine, u64 start, u64 len,
> if (type != MAP__FUNCTION)
> dso__set_loaded(dso, map->type);
> }
> - dso->nsinfo = nsinfo__get(thread->nsinfo);
> + dso->nsinfo = nsi;
> dso__put(dso);
> }
> return map;
> out_delete:
> + nsinfo__put(nsi);
> free(map);
> return NULL;
> }
> diff --git a/tools/perf/util/map.h b/tools/perf/util/map.h
> index f9e8ac8..bf015a93 100644
> --- a/tools/perf/util/map.h
> +++ b/tools/perf/util/map.h
> @@ -140,10 +140,10 @@ struct thread;
>
> void map__init(struct map *map, enum map_type type,
> u64 start, u64 end, u64 pgoff, struct dso *dso);
> -struct map *map__new(struct machine *machine, u64 start, u64 len,
> - u64 pgoff, u32 pid, u32 d_maj, u32 d_min, u64 ino,
> - u64 ino_gen, u32 prot, u32 flags,
> - char *filename, enum map_type type, struct thread *thread);
> +struct map *map__new(struct machine *machine, u64 start, u64 len, u64 pgoff,
> + u32 d_maj, u32 d_min, u64 ino, u64 ino_gen, u32 prot,
> + u32 flags, char *filename, enum map_type type,
> + struct thread *thread);
Ditto
> struct map *map__new2(u64 start, struct dso *dso, enum map_type type);
> void map__delete(struct map *map);
> struct map *map__clone(struct map *map);
> diff --git a/tools/perf/util/namespaces.c b/tools/perf/util/namespaces.c
> index d7f31e0..cad7d8b 100644
> --- a/tools/perf/util/namespaces.c
> +++ b/tools/perf/util/namespaces.c
> @@ -40,18 +40,23 @@ void namespaces__free(struct namespaces *namespaces)
> free(namespaces);
> }
>
> -void nsinfo__init(struct nsinfo *nsi)
> +int nsinfo__init(struct nsinfo *nsi)
> {
> char oldns[PATH_MAX];
> + char spath[PATH_MAX];
> char *newns = NULL;
> + char *statln = NULL;
> struct stat old_stat;
> struct stat new_stat;
> + FILE *f = NULL;
> + size_t linesz = 0;
> + int rv = -1;
>
> if (snprintf(oldns, PATH_MAX, "/proc/self/ns/mnt") >= PATH_MAX)
> - return;
> + return rv;
>
> if (asprintf(&newns, "/proc/%d/ns/mnt", nsi->pid) == -1)
> - return;
> + return rv;
>
> if (stat(oldns, &old_stat) < 0)
> goto out;
> @@ -68,24 +73,89 @@ void nsinfo__init(struct nsinfo *nsi)
> newns = NULL;
> }
>
> + /* If we're dealing with a process that is in a different PID namespace,
> + * attempt to work out the innermost tgid for the process.
> + */
> + if (snprintf(spath, PATH_MAX, "/proc/%d/status", nsi->pid) >= PATH_MAX)
> + goto out;
> +
> + f = fopen(spath, "r");
> + if (f == NULL)
> + goto out;
> +
> + while (getline(&statln, &linesz, f) != -1) {
> + /* Use tgid if CONFIG_PID_NS is not defined. */
> + if (strstr(statln, "Tgid:") != NULL) {
> + nsi->tgid = (pid_t)strtol(strrchr(statln, '\t'),
> + NULL, 10);
> + nsi->nstgid = nsi->tgid;
> + }
> +
> + if (strstr(statln, "NStgid:") != NULL) {
> + nsi->nstgid = (pid_t)strtol(strrchr(statln, '\t'),
> + NULL, 10);
> + break;
> + }
> + }
> + rv = 0;
> +
> out:
> + if (f != NULL)
> + (void) fclose(f);
> + free(statln);
> free(newns);
> + return rv;
> }
>
> struct nsinfo *nsinfo__new(pid_t pid)
> {
> - struct nsinfo *nsi = calloc(1, sizeof(*nsi));
> + struct nsinfo *nsi;
>
> + if (pid == 0)
> + return NULL;
> +
> + nsi = calloc(1, sizeof(*nsi));
> if (nsi != NULL) {
> nsi->pid = pid;
> + nsi->tgid = pid;
> + nsi->nstgid = pid;
> nsi->need_setns = false;
> - nsinfo__init(nsi);
> + /* 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.
> + */
> + if (nsinfo__init(nsi) == -1)
> + nsi->need_setns = false;
> +
> refcount_set(&nsi->refcnt, 1);
> }
>
> return nsi;
> }
>
> +struct nsinfo *nsinfo__copy(struct nsinfo *nsi)
> +{
> + struct nsinfo *nnsi;
> +
> + nnsi = calloc(1, sizeof(*nnsi));
> + if (nnsi != NULL) {
> + nnsi->pid = nsi->pid;
> + nnsi->tgid = nsi->tgid;
> + nnsi->nstgid = nsi->nstgid;
> + nnsi->need_setns = nsi->need_setns;
> + if (nsi->mntns_path) {
> + nnsi->mntns_path = strdup(nsi->mntns_path);
> + if (!nnsi->mntns_path) {
> + free(nnsi);
> + return NULL;
> + }
> + }
> + refcount_set(&nnsi->refcnt, 1);
> + }
> +
> + return nnsi;
> +}
> +
> void nsinfo__delete(struct nsinfo *nsi)
> {
> free(nsi->mntns_path);
> @@ -105,7 +175,8 @@ void nsinfo__put(struct nsinfo *nsi)
> nsinfo__delete(nsi);
> }
>
> -void nsinfo__mountns_enter(struct nsinfo *nsi, struct nscookie *nc)
> +void nsinfo__mountns_enter(struct nsinfo *nsi,
> + struct nscookie *nc)
> {
> char curpath[PATH_MAX];
> int oldns = -1;
> diff --git a/tools/perf/util/namespaces.h b/tools/perf/util/namespaces.h
> index b20f6ea..f19aa41 100644
> --- a/tools/perf/util/namespaces.h
> +++ b/tools/perf/util/namespaces.h
> @@ -26,6 +26,8 @@ void namespaces__free(struct namespaces *namespaces);
>
> struct nsinfo {
> pid_t pid;
> + pid_t tgid;
> + pid_t nstgid;
> bool need_setns;
> char *mntns_path;
> refcount_t refcnt;
> @@ -36,8 +38,9 @@ struct nscookie {
> int newns;
> };
>
> -void nsinfo__init(struct nsinfo *nsi);
> +int nsinfo__init(struct nsinfo *nsi);
> struct nsinfo *nsinfo__new(pid_t pid);
> +struct nsinfo *nsinfo__copy(struct nsinfo *nsi);
> void nsinfo__delete(struct nsinfo *nsi);
>
> struct nsinfo *nsinfo__get(struct nsinfo *nsi);
> diff --git a/tools/perf/util/symbol.c b/tools/perf/util/symbol.c
> index 60a9eaa..97e454f 100644
> --- a/tools/perf/util/symbol.c
> +++ b/tools/perf/util/symbol.c
> @@ -19,7 +19,6 @@
> #include "strlist.h"
> #include "intlist.h"
> #include "namespaces.h"
> -#include "vdso.h"
> #include "header.h"
> #include "path.h"
> #include "sane_ctype.h"
> @@ -1327,14 +1326,15 @@ int dso__load_kallsyms(struct dso *dso, const char *filename,
> return __dso__load_kallsyms(dso, filename, map, false);
> }
>
> -static int dso__load_perf_map(struct dso *dso, struct map *map)
> +static int dso__load_perf_map(const char *map_path, struct dso *dso,
> + struct map *map)
> {
> char *line = NULL;
> size_t n;
> FILE *file;
> int nr_syms = 0;
>
> - file = fopen(dso->long_name, "r");
> + file = fopen(map_path, "r");
> if (file == NULL)
> goto out_failure;
>
> @@ -1426,6 +1426,45 @@ static bool dso__is_compatible_symtab_type(struct dso *dso, bool kmod,
> }
> }
>
> +/* Checks for the existence of the perf-<pid>.map file in two different
> + * locations. First, if the process is a separate mount namespace, check in
> + * that namespace using the pid of the innermost pid namespace. If's not in a
> + * namespace, or the file can't be found there, try in the mount namespace of
> + * the tracing process using our view of its pid.
> + */
> +static int dso__find_perf_map(char *filebuf, size_t bufsz,
> + struct nsinfo **nsip)
> +{
> + struct nscookie nsc;
> + struct nsinfo *nsi;
> + struct nsinfo *nnsi;
> + int rc = -1;
> +
> + nsi = *nsip;
> +
> + if (nsi->need_setns) {
> + snprintf(filebuf, bufsz, "/tmp/perf-%d.map", nsi->nstgid);
> + nsinfo__mountns_enter(nsi, &nsc);
> + rc = access(filebuf, R_OK);
> + nsinfo__mountns_exit(&nsc);
> + if (rc == 0)
> + return rc;
> + }
> +
> + nnsi = nsinfo__copy(nsi);
> + if (nnsi) {
> + nsinfo__put(nsi);
> +
> + nnsi->need_setns = false;
> + snprintf(filebuf, bufsz, "/tmp/perf-%d.map", nnsi->tgid);
> + *nsip = nnsi;
> + rc = 0;
> + }
> +
> + return rc;
> +}
> +
> +
> int dso__load(struct dso *dso, struct map *map)
> {
> char *name;
> @@ -1437,18 +1476,23 @@ int dso__load(struct dso *dso, struct map *map)
> struct symsrc ss_[2];
> struct symsrc *syms_ss = NULL, *runtime_ss = NULL;
> bool kmod;
> + bool perfmap;
> unsigned char build_id[BUILD_ID_SIZE];
> struct nscookie nsc;
> + char newmapname[PATH_MAX];
> + const char *map_path = dso->long_name;
> +
> + perfmap = strncmp(dso->name, "/tmp/perf-", 10) == 0;
> + if (perfmap) {
> + if (dso->nsinfo && (dso__find_perf_map(newmapname,
> + sizeof(newmapname), &dso->nsinfo) == 0)) {
> + map_path = newmapname;
> + }
> + }
>
> nsinfo__mountns_enter(dso->nsinfo, &nsc);
> pthread_mutex_lock(&dso->lock);
>
> - /* The vdso files always live in the host container, so don't go looking
> - * for them in the container's mount namespace.
> - */
> - if (dso__is_vdso(dso))
> - nsinfo__mountns_exit(&nsc);
> -
> /* check again under the dso->lock */
> if (dso__loaded(dso, map->type)) {
> ret = 1;
> @@ -1471,19 +1515,20 @@ int dso__load(struct dso *dso, struct map *map)
>
> dso->adjust_symbols = 0;
>
> - if (strncmp(dso->name, "/tmp/perf-", 10) == 0) {
> + if (perfmap) {
> struct stat st;
>
> - if (lstat(dso->name, &st) < 0)
> + if (lstat(map_path, &st) < 0)
> goto out;
>
> if (!symbol_conf.force && st.st_uid && (st.st_uid != geteuid())) {
> pr_warning("File %s not owned by current user or root, "
> - "ignoring it (use -f to override).\n", dso->name);
> + "ignoring it (use -f to override).\n",
> + map_path);
Keep on the same line, that way we see straight away that you're
replacing dso->name with map_path, as they will be aligned.
> goto out;
> }
>
> - ret = dso__load_perf_map(dso, map);
> + ret = dso__load_perf_map(map_path, dso, map);
> dso->symtab_type = ret > 0 ? DSO_BINARY_TYPE__JAVA_JIT :
> DSO_BINARY_TYPE__NOT_FOUND;
> goto out;
> --
> 2.7.4
Powered by blists - more mailing lists