public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: Arnaldo Carvalho de Melo <acme@kernel.org>
To: Krister Johansen <kjlx@templeofstupid.com>
Cc: Brendan Gregg <brendan.d.gregg@gmail.com>,
	Peter Zijlstra <peterz@infradead.org>,
	Ingo Molnar <mingo@redhat.com>,
	Alexander Shishkin <alexander.shishkin@linux.intel.com>,
	linux-kernel@vger.kernel.org,
	Thomas-Mich Richter <tmricht@linux.vnet.ibm.com>
Subject: Re: [PATCH v2 tip/perf/core 1/6] perf symbols: find symbols in different mount namespace
Date: Thu, 6 Jul 2017 16:41:30 -0300	[thread overview]
Message-ID: <20170706194130.GM27350@kernel.org> (raw)
In-Reply-To: <1499305693-1599-2-git-send-email-kjlx@templeofstupid.com>

Em Wed, Jul 05, 2017 at 06:48:08PM -0700, Krister Johansen escreveu:
> Teach perf how to resolve symbols from binaries that are in a different
> mount namespace from the tool.  This allows perf to generate meaningful
> stack traces even if the binary resides in a different mount namespace
> from the tool.

I was trying to find a way to test after applying each of the patches in
this series, when it ocurred to me that if a process that appears on a
perf.data file has exit, how can we access /proc/%ITS_PID/something?

This can be used for synthesizing records at 'perf record' start, but
how to get that on a different machine, different arch, say?

We would use what is in PERF_RECORD_NAMESPACES? How?

What we have there is:

        /*
         * struct {
         *      struct perf_event_header        header;
         *      u32                             pid;
         *      u32                             tid;
         *      u64                             nr_namespaces;
         *      { u64                           dev, inode; } [nr_namespaces];
         *      struct sample_id                sample_id;
         * };
         */
        PERF_RECORD_NAMESPACES                  = 16,

So how would we use setns with that?

I must be missing something...

- Arnaldo
 
> Signed-off-by: Krister Johansen <kjlx@templeofstupid.com>
> ---
>  tools/perf/util/dso.c        |   1 +
>  tools/perf/util/dso.h        |   2 +
>  tools/perf/util/map.c        |   2 +
>  tools/perf/util/namespaces.c | 127 +++++++++++++++++++++++++++++++++++++++++++
>  tools/perf/util/namespaces.h |  33 +++++++++++
>  tools/perf/util/symbol.c     |  11 ++++
>  tools/perf/util/thread.c     |   3 +
>  tools/perf/util/thread.h     |   1 +
>  8 files changed, 180 insertions(+)
> 
> diff --git a/tools/perf/util/dso.c b/tools/perf/util/dso.c
> index 4e7ab61..beda40e 100644
> --- a/tools/perf/util/dso.c
> +++ b/tools/perf/util/dso.c
> @@ -1236,6 +1236,7 @@ void dso__delete(struct dso *dso)
>  	dso_cache__free(dso);
>  	dso__free_a2l(dso);
>  	zfree(&dso->symsrc_filename);
> +	nsinfo__zput(dso->nsinfo);
>  	pthread_mutex_destroy(&dso->lock);
>  	free(dso);
>  }
> diff --git a/tools/perf/util/dso.h b/tools/perf/util/dso.h
> index bd061ba..78ec637 100644
> --- a/tools/perf/util/dso.h
> +++ b/tools/perf/util/dso.h
> @@ -10,6 +10,7 @@
>  #include <linux/types.h>
>  #include <linux/bitops.h>
>  #include "map.h"
> +#include "namespaces.h"
>  #include "build-id.h"
>  
>  enum dso_binary_type {
> @@ -187,6 +188,7 @@ struct dso {
>  		void	 *priv;
>  		u64	 db_id;
>  	};
> +	struct nsinfo	*nsinfo;
>  	refcount_t	 refcnt;
>  	char		 name[0];
>  };
> diff --git a/tools/perf/util/map.c b/tools/perf/util/map.c
> index 2179b2d..5dc60ca 100644
> --- a/tools/perf/util/map.c
> +++ b/tools/perf/util/map.c
> @@ -16,6 +16,7 @@
>  #include "machine.h"
>  #include <linux/string.h>
>  #include "srcline.h"
> +#include "namespaces.h"
>  #include "unwind.h"
>  
>  static void __maps__insert(struct maps *maps, struct map *map);
> @@ -200,6 +201,7 @@ 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__put(dso);
>  	}
>  	return map;
> diff --git a/tools/perf/util/namespaces.c b/tools/perf/util/namespaces.c
> index 67dcbcc..bcc6bb1 100644
> --- a/tools/perf/util/namespaces.c
> +++ b/tools/perf/util/namespaces.c
> @@ -9,9 +9,13 @@
>  #include "namespaces.h"
>  #include "util.h"
>  #include "event.h"
> +#include <sys/types.h>
> +#include <sys/stat.h>
> +#include <sched.h>
>  #include <stdlib.h>
>  #include <stdio.h>
>  #include <string.h>
> +#include <unistd.h>
>  
>  struct namespaces *namespaces__new(struct namespaces_event *event)
>  {
> @@ -35,3 +39,126 @@ void namespaces__free(struct namespaces *namespaces)
>  {
>  	free(namespaces);
>  }
> +
> +void nsinfo__init(struct nsinfo *nsi)
> +{
> +	char oldns[PATH_MAX];
> +	char *newns = NULL;
> +	struct stat old_stat;
> +	struct stat new_stat;
> +
> +	if (snprintf(oldns, PATH_MAX, "/proc/self/ns/mnt") >= PATH_MAX)
> +		return;
> +
> +	if (asprintf(&newns, "/proc/%d/ns/mnt", nsi->pid) == -1)
> +		return;
> +
> +	if (stat(oldns, &old_stat) < 0)
> +		goto out;
> +
> +	if (stat(newns, &new_stat) < 0)
> +		goto out;
> +
> +	/* Check if the mount namespaces differ, if so then indicate that we
> +	 * want to switch as part of looking up dso/map data.
> +	 */
> +	if (old_stat.st_ino != new_stat.st_ino) {
> +		nsi->need_setns = true;
> +		nsi->mntns_path = newns;
> +		newns = NULL;
> +	}
> +
> +out:
> +	free(newns);
> +}
> +
> +struct nsinfo *nsinfo__new(pid_t pid)
> +{
> +	struct nsinfo *nsi = calloc(1, sizeof(*nsi));
> +
> +	if (nsi != NULL) {
> +		nsi->pid = pid;
> +		nsi->need_setns = false;
> +		nsinfo__init(nsi);
> +		refcount_set(&nsi->refcnt, 1);
> +	}
> +
> +	return nsi;
> +}
> +
> +void nsinfo__delete(struct nsinfo *nsi)
> +{
> +	zfree(&nsi->mntns_path);
> +	free(nsi);
> +}
> +
> +struct nsinfo *nsinfo__get(struct nsinfo *nsi)
> +{
> +	if (nsi)
> +		refcount_inc(&nsi->refcnt);
> +	return nsi;
> +}
> +
> +void nsinfo__put(struct nsinfo *nsi)
> +{
> +	if (nsi && refcount_dec_and_test(&nsi->refcnt))
> +		nsinfo__delete(nsi);
> +}
> +
> +void nsinfo__mountns_enter(struct nsinfo *nsi, struct nscookie *nc)
> +{
> +	char curpath[PATH_MAX];
> +	int oldns = -1;
> +	int newns = -1;
> +
> +	if (nc == NULL)
> +		return;
> +
> +	nc->oldns = -1;
> +	nc->newns = -1;
> +
> +	if (!nsi || !nsi->need_setns)
> +		return;
> +
> +	if (snprintf(curpath, PATH_MAX, "/proc/self/ns/mnt") >= PATH_MAX)
> +		return;
> +
> +	oldns = open(curpath, O_RDONLY);
> +	if (oldns < 0)
> +		return;
> +
> +	newns = open(nsi->mntns_path, O_RDONLY);
> +	if (newns < 0)
> +		goto errout;
> +
> +	if (setns(newns, CLONE_NEWNS) < 0)
> +		goto errout;
> +
> +	nc->oldns = oldns;
> +	nc->newns = newns;
> +	return;
> +
> +errout:
> +	if (oldns > -1)
> +		close(oldns);
> +	if (newns > -1)
> +		close(newns);
> +}
> +
> +void nsinfo__mountns_exit(struct nscookie *nc)
> +{
> +	if (nc == NULL || nc->oldns == -1 || nc->newns == -1)
> +		return;
> +
> +	setns(nc->oldns, CLONE_NEWNS);
> +
> +	if (nc->oldns > -1) {
> +		close(nc->oldns);
> +		nc->oldns = -1;
> +	}
> +
> +	if (nc->newns > -1) {
> +		close(nc->newns);
> +		nc->newns = -1;
> +	}
> +}
> diff --git a/tools/perf/util/namespaces.h b/tools/perf/util/namespaces.h
> index 468f1e9..b20f6ea 100644
> --- a/tools/perf/util/namespaces.h
> +++ b/tools/perf/util/namespaces.h
> @@ -11,6 +11,7 @@
>  
>  #include "../perf.h"
>  #include <linux/list.h>
> +#include <linux/refcount.h>
>  
>  struct namespaces_event;
>  
> @@ -23,4 +24,36 @@ struct namespaces {
>  struct namespaces *namespaces__new(struct namespaces_event *event);
>  void namespaces__free(struct namespaces *namespaces);
>  
> +struct nsinfo {
> +	pid_t			pid;
> +	bool			need_setns;
> +	char			*mntns_path;
> +	refcount_t		refcnt;
> +};
> +
> +struct nscookie {
> +	int			oldns;
> +	int			newns;
> +};
> +
> +void nsinfo__init(struct nsinfo *nsi);
> +struct nsinfo *nsinfo__new(pid_t pid);
> +void nsinfo__delete(struct nsinfo *nsi);
> +
> +struct nsinfo *nsinfo__get(struct nsinfo *nsi);
> +void nsinfo__put(struct nsinfo *nsi);
> +
> +void nsinfo__mountns_enter(struct nsinfo *nsi, struct nscookie *nc);
> +void nsinfo__mountns_exit(struct nscookie *nc);
> +
> +static inline void __nsinfo__zput(struct nsinfo **nsip)
> +{
> +	if (nsip) {
> +		nsinfo__put(*nsip);
> +		*nsip = NULL;
> +	}
> +}
> +
> +#define nsinfo__zput(nsi) __nsinfo__zput(&nsi)
> +
>  #endif  /* __PERF_NAMESPACES_H */
> diff --git a/tools/perf/util/symbol.c b/tools/perf/util/symbol.c
> index e7a98db..60a9eaa 100644
> --- a/tools/perf/util/symbol.c
> +++ b/tools/perf/util/symbol.c
> @@ -18,6 +18,8 @@
>  #include "symbol.h"
>  #include "strlist.h"
>  #include "intlist.h"
> +#include "namespaces.h"
> +#include "vdso.h"
>  #include "header.h"
>  #include "path.h"
>  #include "sane_ctype.h"
> @@ -1436,9 +1438,17 @@ int dso__load(struct dso *dso, struct map *map)
>  	struct symsrc *syms_ss = NULL, *runtime_ss = NULL;
>  	bool kmod;
>  	unsigned char build_id[BUILD_ID_SIZE];
> +	struct nscookie nsc;
>  
> +	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;
> @@ -1584,6 +1594,7 @@ int dso__load(struct dso *dso, struct map *map)
>  out:
>  	dso__set_loaded(dso, map->type);
>  	pthread_mutex_unlock(&dso->lock);
> +	nsinfo__mountns_exit(&nsc);
>  
>  	return ret;
>  }
> diff --git a/tools/perf/util/thread.c b/tools/perf/util/thread.c
> index 378c418..aee9a42 100644
> --- a/tools/perf/util/thread.c
> +++ b/tools/perf/util/thread.c
> @@ -59,6 +59,8 @@ struct thread *thread__new(pid_t pid, pid_t tid)
>  		list_add(&comm->list, &thread->comm_list);
>  		refcount_set(&thread->refcnt, 1);
>  		RB_CLEAR_NODE(&thread->rb_node);
> +		/* Thread holds first ref to nsdata. */
> +		thread->nsinfo = nsinfo__new(pid);
>  	}
>  
>  	return thread;
> @@ -91,6 +93,7 @@ void thread__delete(struct thread *thread)
>  		comm__free(comm);
>  	}
>  	unwind__finish_access(thread);
> +	nsinfo__zput(thread->nsinfo);
>  
>  	free(thread);
>  }
> diff --git a/tools/perf/util/thread.h b/tools/perf/util/thread.h
> index 4eb849e..cb1a5dd 100644
> --- a/tools/perf/util/thread.h
> +++ b/tools/perf/util/thread.h
> @@ -34,6 +34,7 @@ struct thread {
>  
>  	void			*priv;
>  	struct thread_stack	*ts;
> +	struct nsinfo		*nsinfo;
>  #ifdef HAVE_LIBUNWIND_SUPPORT
>  	void				*addr_space;
>  	struct unwind_libunwind_ops	*unwind_libunwind_ops;
> -- 
> 2.7.4

  reply	other threads:[~2017-07-06 19:41 UTC|newest]

Thread overview: 42+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-07-01  2:18 [PATCH tip/perf/core 0/7] namespace tracing improvements Krister Johansen
2017-07-01  2:18 ` [PATCH tip/perf/core 1/7] perf symbols: find symbols in different mount namespace Krister Johansen
2017-07-03 18:38   ` Arnaldo Carvalho de Melo
2017-07-05 20:44     ` Krister Johansen
2017-07-01  2:18 ` [PATCH tip/perf/core 2/7] perf maps: lookup maps in both intitial mountns and inner mountns Krister Johansen
2017-07-03 18:44   ` Arnaldo Carvalho de Melo
2017-07-01  2:18 ` [PATCH tip/perf/core 3/7] perf probe: allow placing uprobes in alternate namespaces Krister Johansen
2017-07-03 18:46   ` Arnaldo Carvalho de Melo
2017-07-05 20:45     ` Krister Johansen
2017-07-01  2:18 ` [PATCH tip/perf/core 4/7] perf buildid-cache: support binary objects from other namespaces Krister Johansen
2017-07-01  2:18 ` [PATCH tip/perf/core 5/7] perf top: support lookup of symbols in other mount namespaces Krister Johansen
2017-07-01  2:18 ` [PATCH tip/perf/core 6/7] perf documentation: updates for target-ns Krister Johansen
2017-07-03 18:48   ` Arnaldo Carvalho de Melo
2017-07-05 20:45     ` Krister Johansen
2017-07-06  1:48       ` [PATCH v2 tip/perf/core 0/6] namespace tracing improvements Krister Johansen
2017-07-06  1:48         ` [PATCH v2 tip/perf/core 1/6] perf symbols: find symbols in different mount namespace Krister Johansen
2017-07-06 19:41           ` Arnaldo Carvalho de Melo [this message]
2017-07-07 19:36             ` Krister Johansen
2017-07-10  6:17               ` Thomas-Mich Richter
2017-07-10 22:39                 ` Krister Johansen
2017-07-10 22:52                   ` Arnaldo Carvalho de Melo
2017-07-10 23:29                     ` Krister Johansen
2017-07-11 12:51                       ` Arnaldo Carvalho de Melo
2017-07-11 17:15                         ` Krister Johansen
2017-07-20  8:48           ` [tip:perf/core] perf symbols: Find " tip-bot for Krister Johansen
2017-07-06  1:48         ` [PATCH v2 tip/perf/core 2/6] perf maps: lookup maps in both intitial mountns and inner mountns Krister Johansen
2017-07-20  8:48           ` [tip:perf/core] perf maps: Lookup " tip-bot for Krister Johansen
2017-07-06  1:48         ` [PATCH v2 tip/perf/core 3/6] perf probe: allow placing uprobes in alternate namespaces Krister Johansen
2017-07-17 10:32           ` [PATCH] perf probe: Fix build failure for get_target_map() Ravi Bangoria
2017-07-17 13:11             ` Arnaldo Carvalho de Melo
2017-07-18  9:19               ` Michael Ellerman
2017-07-18 15:45                 ` Arnaldo Carvalho de Melo
2017-07-19  5:58                   ` Michael Ellerman
2017-07-20  8:48           ` [tip:perf/core] perf probe: Allow placing uprobes in alternate namespaces tip-bot for Krister Johansen
2017-07-06  1:48         ` [PATCH v2 tip/perf/core 4/6] perf buildid-cache: support binary objects from other namespaces Krister Johansen
2017-07-20  8:49           ` [tip:perf/core] perf buildid-cache: Support " tip-bot for Krister Johansen
2017-07-06  1:48         ` [PATCH v2 tip/perf/core 5/6] perf top: support lookup of symbols in other mount namespaces Krister Johansen
2017-07-21 17:10           ` Arnaldo Carvalho de Melo
2017-07-26 17:23           ` [tip:perf/core] perf top: Support " tip-bot for Krister Johansen
2017-07-06  1:48         ` [PATCH v2 tip/perf/core 6/6] perf buildid-cache: cache debuginfo Krister Johansen
2017-07-20  8:49           ` [tip:perf/core] perf buildid-cache: Cache debuginfo tip-bot for Krister Johansen
2017-07-01  2:18 ` [PATCH tip/perf/core 7/7] perf buildid-cache: cache debuginfo Krister Johansen

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20170706194130.GM27350@kernel.org \
    --to=acme@kernel.org \
    --cc=alexander.shishkin@linux.intel.com \
    --cc=brendan.d.gregg@gmail.com \
    --cc=kjlx@templeofstupid.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mingo@redhat.com \
    --cc=peterz@infradead.org \
    --cc=tmricht@linux.vnet.ibm.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox