All of lore.kernel.org
 help / color / mirror / Atom feed
From: Jiri Olsa <jolsa@redhat.com>
To: Jiri Olsa <jolsa@kernel.org>
Cc: Arnaldo Carvalho de Melo <acme@kernel.org>,
	Krister Johansen <kjlx@templeofstupid.com>,
	lkml <linux-kernel@vger.kernel.org>,
	Ingo Molnar <mingo@kernel.org>,
	Namhyung Kim <namhyung@kernel.org>,
	Alexander Shishkin <alexander.shishkin@linux.intel.com>,
	Peter Zijlstra <a.p.zijlstra@chello.nl>
Subject: Re: [PATCH] perf tools: Restore proper cwd on return from mnt ns
Date: Fri, 16 Nov 2018 14:03:29 +0100	[thread overview]
Message-ID: <20181116130329.GB30583@krava> (raw)
In-Reply-To: <20181101170001.30019-1-jolsa@kernel.org>

ping

jirka

On Thu, Nov 01, 2018 at 06:00:01PM +0100, Jiri Olsa wrote:
> When reporting on 'record' server we try to retrieve/use
> the mnt namespace of the profiled tasks. We use following
> API with cookie to hold the return namespace, roughly:
> 
>   nsinfo__mountns_enter(struct nsinfo *nsi, struct nscookie *nc)
>     setns(newns, 0);
>   ...
>   new ns related open..
>   ...
>   nsinfo__mountns_exit(struct nscookie *nc)
>     setns(nc->oldns)
> 
> Once finished we setns to old namespace, which also sets the
> current working directory (cwd) to "/", trashing the cwd we had.
> 
> This is mostly fine, because we use absolute paths almost everywhere,
> but it screws up perf diff:
> 
>   # perf diff
>   failed to open perf.data: No such file or directory  (try 'perf record' first)
>   ...
> 
> Adding the current working directory to be part of the cookie
> and restoring it in the nsinfo__mountns_exit call.
> 
> Cc: Krister Johansen <kjlx@templeofstupid.com>
> Fixes: 843ff37bb59e ("perf symbols: Find symbols in different mount namespace")
> Link: http://lkml.kernel.org/n/tip-zg3vz7kjr86cco7lo91v8yhj@git.kernel.org
> Signed-off-by: Jiri Olsa <jolsa@kernel.org>
> ---
>  tools/perf/util/namespaces.c | 18 ++++++++++++++++--
>  tools/perf/util/namespaces.h |  1 +
>  2 files changed, 17 insertions(+), 2 deletions(-)
> 
> diff --git a/tools/perf/util/namespaces.c b/tools/perf/util/namespaces.c
> index cf8bd123cf73..fb0458b7e6aa 100644
> --- a/tools/perf/util/namespaces.c
> +++ b/tools/perf/util/namespaces.c
> @@ -18,6 +18,7 @@
>  #include <stdio.h>
>  #include <string.h>
>  #include <unistd.h>
> +#include <asm/bug.h>
>  
>  struct namespaces *namespaces__new(struct namespaces_event *event)
>  {
> @@ -186,6 +187,7 @@ void nsinfo__mountns_enter(struct nsinfo *nsi,
>  	char curpath[PATH_MAX];
>  	int oldns = -1;
>  	int newns = -1;
> +	char *oldcwd = NULL;
>  
>  	if (nc == NULL)
>  		return;
> @@ -199,9 +201,13 @@ void nsinfo__mountns_enter(struct nsinfo *nsi,
>  	if (snprintf(curpath, PATH_MAX, "/proc/self/ns/mnt") >= PATH_MAX)
>  		return;
>  
> +	oldcwd = get_current_dir_name();
> +	if (!oldcwd)
> +		return;
> +
>  	oldns = open(curpath, O_RDONLY);
>  	if (oldns < 0)
> -		return;
> +		goto errout;
>  
>  	newns = open(nsi->mntns_path, O_RDONLY);
>  	if (newns < 0)
> @@ -210,11 +216,14 @@ void nsinfo__mountns_enter(struct nsinfo *nsi,
>  	if (setns(newns, CLONE_NEWNS) < 0)
>  		goto errout;
>  
> +	nc->oldcwd = oldcwd;
>  	nc->oldns = oldns;
>  	nc->newns = newns;
>  	return;
>  
>  errout:
> +	if (oldcwd)
> +		free(oldcwd);
>  	if (oldns > -1)
>  		close(oldns);
>  	if (newns > -1)
> @@ -223,11 +232,16 @@ void nsinfo__mountns_enter(struct nsinfo *nsi,
>  
>  void nsinfo__mountns_exit(struct nscookie *nc)
>  {
> -	if (nc == NULL || nc->oldns == -1 || nc->newns == -1)
> +	if (nc == NULL || nc->oldns == -1 || nc->newns == -1 || !nc->oldcwd)
>  		return;
>  
>  	setns(nc->oldns, CLONE_NEWNS);
>  
> +	if (nc->oldcwd) {
> +		WARN_ON_ONCE(chdir(nc->oldcwd));
> +		free(nc->oldcwd);
> +	}
> +
>  	if (nc->oldns > -1) {
>  		close(nc->oldns);
>  		nc->oldns = -1;
> diff --git a/tools/perf/util/namespaces.h b/tools/perf/util/namespaces.h
> index cae1a9a39722..d5f46c09ea31 100644
> --- a/tools/perf/util/namespaces.h
> +++ b/tools/perf/util/namespaces.h
> @@ -38,6 +38,7 @@ struct nsinfo {
>  struct nscookie {
>  	int			oldns;
>  	int			newns;
> +	char			*oldcwd;
>  };
>  
>  int nsinfo__init(struct nsinfo *nsi);
> -- 
> 2.17.2
> 

  reply	other threads:[~2018-11-16 13:03 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-11-01 17:00 [PATCH] perf tools: Restore proper cwd on return from mnt ns Jiri Olsa
2018-11-16 13:03 ` Jiri Olsa [this message]
2018-11-16 18:31 ` Arnaldo Carvalho de Melo
2018-11-16 18:42   ` Arnaldo Carvalho de Melo
2018-11-17  9:00     ` Jiri Olsa
2018-11-19 16:10 ` Arnaldo Carvalho de Melo
2018-11-20 10:56   ` Jiri Olsa
2018-11-20 11:23     ` [PATCHv2] " Jiri Olsa
2018-11-20 15:56     ` [PATCH] " Arnaldo Carvalho de Melo
2018-11-20 17:42       ` Jiri Olsa
2018-11-21 15:02 ` [tip:perf/urgent] perf tools: Restore proper cwd on return from mnt namespace tip-bot for Jiri Olsa

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=20181116130329.GB30583@krava \
    --to=jolsa@redhat.com \
    --cc=a.p.zijlstra@chello.nl \
    --cc=acme@kernel.org \
    --cc=alexander.shishkin@linux.intel.com \
    --cc=jolsa@kernel.org \
    --cc=kjlx@templeofstupid.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mingo@kernel.org \
    --cc=namhyung@kernel.org \
    /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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.