From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-14.0 required=3.0 tests=BAYES_00,INCLUDES_CR_TRAILER, INCLUDES_PATCH,MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 59AC2C433DB for ; Tue, 19 Jan 2021 06:41:39 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 1194923120 for ; Tue, 19 Jan 2021 06:41:39 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1729382AbhASGkw (ORCPT ); Tue, 19 Jan 2021 01:40:52 -0500 Received: from mail-lf1-f51.google.com ([209.85.167.51]:47097 "EHLO mail-lf1-f51.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S2387629AbhASFih (ORCPT ); Tue, 19 Jan 2021 00:38:37 -0500 Received: by mail-lf1-f51.google.com with SMTP id o10so27323759lfl.13 for ; Mon, 18 Jan 2021 21:38:20 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=URBaQWvo21Po7lHS1Q5G/RCwEYa2F0a4Wp9IVeoO+Eg=; b=Fa1Xm6n/Uh4zVtknc7VhiEzPtAhy74Dw+P3yIPAU3LLhU/ywmR383CUVibhF31adB6 E/9hC/5LwAvtWTpFGtmPbqpTJNu33zKR7d+V4tcRXiJOt8rVLmv/8yR3zbsciQVfFCMo 9u4eWG7D2NzNAmZ8vq2gcaCQtYOCXO6n4shgp1u232sMKHFqR0Mvq8gMyy3X5woxxvL3 kNTw9AyIABI3ei18knj4IpyfFWqfUIhS2BSev10YjuuLCYMMGPyqiS0SKEuZ0xFeztw+ oQvEmZhp/wVgAgRmhIVuXuGUb9atsq2A8PovVy6W/Ssg2/TW4oqm+JQGJgLwB68VyY8e rY/A== X-Gm-Message-State: AOAM533vnG+remunjWtXReZmbEmqhNqBrDJpkVwxgivmFDlzZ1jtF0+o mM/d680huenRUbUhJx4IqIzZeRTiJo0bjLUjRYc= X-Google-Smtp-Source: ABdhPJw2LQNDH0X0sbKs4rmIbQT2cKK8YH8WGcvQPnrdC84rmwGjyVLNIfJul9d/uYNe41R55yNvnesWxwJ/nbqp0PQ= X-Received: by 2002:ac2:50d0:: with SMTP id h16mr1100639lfm.300.1611034674236; Mon, 18 Jan 2021 21:37:54 -0800 (PST) MIME-Version: 1.0 References: <20210102220441.794923-1-jolsa@kernel.org> <20210102220441.794923-13-jolsa@kernel.org> In-Reply-To: <20210102220441.794923-13-jolsa@kernel.org> From: Namhyung Kim Date: Tue, 19 Jan 2021 14:37:42 +0900 Message-ID: Subject: Re: [PATCH 12/22] perf daemon: Allow only one daemon over base directory To: Jiri Olsa Cc: Arnaldo Carvalho de Melo , lkml , Peter Zijlstra , Ingo Molnar , Mark Rutland , Alexander Shishkin , Michael Petlan , Ian Rogers , Stephane Eranian , Alexei Budankov Content-Type: text/plain; charset="UTF-8" Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Sun, Jan 3, 2021 at 7:05 AM Jiri Olsa wrote: > > Add 'lock' file under daemon base and flock it, so only one > perf daemon can run on top of it. > > Example: > > # cat ~/.perfconfig > [daemon] > base=/opt/perfdata > > [session-cycles] > run = -m 10M -e cycles --overwrite --switch-output -a > > [session-sched] > run = -m 20M -e sched:* --overwrite --switch-output -a > > Starting the daemon: > > # perf daemon start > > And try once more: > > # perf daemon start > failed: another perf daemon (pid 775594) owns /opt/perfdata > > will end up with an error, because there's already one running > on top of /opt/perfdata. > > Signed-off-by: Jiri Olsa > --- > tools/perf/builtin-daemon.c | 49 +++++++++++++++++++++++++++++++++++++ > 1 file changed, 49 insertions(+) > > diff --git a/tools/perf/builtin-daemon.c b/tools/perf/builtin-daemon.c > index 45748bb471ec..1982eedd3f3f 100644 > --- a/tools/perf/builtin-daemon.c > +++ b/tools/perf/builtin-daemon.c > @@ -8,6 +8,7 @@ > #include > #include > #include > +#include > #include > #include > #include > @@ -562,12 +563,18 @@ static int cmd_session_list(struct daemon *daemon, union cmd *cmd, FILE *out) > /* output */ > csv_sep, daemon->base, SESSION_OUTPUT); > > + fprintf(out, "%c%s/%s", > + /* lock */ > + csv_sep, daemon->base, "lock"); > + > fprintf(out, "\n"); > } else { > fprintf(out, "[%d:daemon] base: %s\n", getpid(), daemon->base); > if (cmd->list.verbose) { > fprintf(out, " output: %s/%s\n", > daemon->base, SESSION_OUTPUT); > + fprintf(out, " lock: %s/lock\n", > + daemon->base); > } > } > > @@ -761,6 +768,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|O_CLOEXEC, 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; > + } So the fd is (a kind of) leaked and the lock is released only when the daemon is going to die, right? Thanks, Namhyung > + > + 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; > @@ -775,6 +818,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)) { > @@ -861,6 +907,9 @@ static int __cmd_start(struct daemon *daemon, struct option parent_options[], > if (setup_server_config(daemon)) > return -1; > > + if (foreground && check_lock(daemon)) > + return -1; > + > if (!foreground && go_background(daemon)) > return -1; > > -- > 2.26.2 >