Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Date:   Tue, 15 Dec 2020 20:16:29 +0100
From:   Jiri Olsa <jolsa@redhat.com>
To:     Arnaldo Carvalho de Melo <acme@kernel.org>
Cc:     Jiri Olsa <jolsa@kernel.org>, lkml <linux-kernel@vger.kernel.org>,
        Peter Zijlstra <a.p.zijlstra@chello.nl>,
        Ingo Molnar <mingo@kernel.org>,
        Mark Rutland <mark.rutland@arm.com>,
        Namhyung Kim <namhyung@kernel.org>,
        Alexander Shishkin <alexander.shishkin@linux.intel.com>,
        Michael Petlan <mpetlan@redhat.com>,
        Ian Rogers <irogers@google.com>,
        Stephane Eranian <eranian@google.com>,
        Alexei Budankov <abudankov@huawei.com>
Subject: Re: [PATCH 7/8] perf daemon: Allow only one daemon over base
 directory
Message-ID: <20201215191629.GF698181@krava>
References: <20201212104358.412065-1-jolsa@kernel.org>
 <20201212104358.412065-8-jolsa@kernel.org>
 <20201215154627.GH258566@kernel.org>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20201215154627.GH258566@kernel.org>
Precedence: bulk

On Tue, Dec 15, 2020 at 12:46:27PM -0300, Arnaldo Carvalho de Melo wrote:
> Em Sat, Dec 12, 2020 at 11:43:57AM +0100, Jiri Olsa escreveu:
> > Add 'lock' file under daemon base and flock it, so only one
> > perf daemon can run on top of it.
> > 
> >   # perf daemon --config ./config.daemon
> >   # perf daemon --config ./config.daemon
> >   failed: another perf daemon (pid 369675) owns /opt/perfdata
> 
> So the way to disambiguate is the "daemon base"?

yes, the 'base' is path where perf daemon will create
all control files and 'session' directories and does
chdir for 'perf record' session on top of that

jirka

> 
> - Arnaldo
>  
> > Signed-off-by: Jiri Olsa <jolsa@kernel.org>
> > ---
> >  tools/perf/builtin-daemon.c | 43 +++++++++++++++++++++++++++++++++++++
> >  1 file changed, 43 insertions(+)
> > 
> > diff --git a/tools/perf/builtin-daemon.c b/tools/perf/builtin-daemon.c
> > index 855fed2fe364..1bd5432a57a3 100644
> > --- a/tools/perf/builtin-daemon.c
> > +++ b/tools/perf/builtin-daemon.c
> > @@ -8,6 +8,7 @@
> >  #include <string.h>
> >  #include <sys/types.h>
> >  #include <sys/wait.h>
> > +#include <sys/file.h>
> >  #include <signal.h>
> >  #include <stdlib.h>
> >  #include <time.h>
> > @@ -639,6 +640,42 @@ static int handle_config_changes(struct daemon *daemon, int conf_fd,
> >  	return 0;
> >  }
> >  
> > +static int check_lock(struct daemon *daemon)
> > +{
> > +	char path[PATH_MAX];
> > +	char buf[20];
> > +	int fd, pid;
> > +	ssize_t len;
> > +
> > +	scnprintf(path, sizeof(path), "%s/lock", daemon->base);
> > +
> > +	fd = open(path, O_RDWR|O_CREAT, 0640);
> > +	if (fd < 0)
> > +		return -1;
> > +
> > +	if (lockf(fd, F_TLOCK, 0) < 0) {
> > +		filename__read_int(path, &pid);
> > +		fprintf(stderr, "failed: another perf daemon (pid %d) owns %s\n",
> > +			pid, daemon->base);
> > +		return -1;
> > +	}
> > +
> > +	scnprintf(buf, sizeof(buf), "%d", getpid());
> > +	len = strlen(buf);
> > +
> > +	if (write(fd, buf, len) != len) {
> > +		perror("write failed");
> > +		return -1;
> > +	}
> > +
> > +	if (ftruncate(fd, len)) {
> > +		perror("ftruncate failed");
> > +		return -1;
> > +	}
> > +
> > +	return 0;
> > +}
> > +
> >  static int go_background(struct daemon *daemon)
> >  {
> >  	int pid, fd;
> > @@ -653,6 +690,9 @@ static int go_background(struct daemon *daemon)
> >  	if (setsid() < 0)
> >  		return -1;
> >  
> > +	if (check_lock(daemon))
> > +		return -1;
> > +
> >  	umask(0);
> >  
> >  	if (chdir(daemon->base)) {
> > @@ -704,6 +744,9 @@ static int __cmd_daemon(struct daemon *daemon, bool foreground, const char *conf
> >  	if (setup_server_config(daemon))
> >  		return -1;
> >  
> > +	if (foreground && check_lock(daemon))
> > +		return -1;
> > +
> >  	if (!foreground && go_background(daemon))
> >  		return -1;
> >  
> > -- 
> > 2.26.2
> > 
> 
> -- 
> 
> - Arnaldo
>