2021-02-08 21:12:14

by Jiri Olsa

[permalink] [raw]
Subject: [PATCHv4 00/24] perf tools: Add daemon command

hi,
we were asked for possibility to be able run record
sessions on background.

This patchset adds support to configure and run record
sessions on background via new 'perf daemon' command.

Please check below the example on usage.

Available also here:
git://git.kernel.org/pub/scm/linux/kernel/git/jolsa/perf.git
perf/daemon

v4 changes:
- update man page gradually in each patch [Arnaldo]
- fixes in network code, add error handling [Arnaldo]
- use readn/writen functions where possible [Arnaldo]
- add more error checks for poll [Arnaldo]
- use strlcpy in socket setup code [Arnaldo]
- free/delete renames [Arnaldo]
- add more error checks in check_base [Namhyung]
- rename session -> daemon_session plus related
functions renames [Namhyung]
- man page updates

v3 changes:
- several patches merged
- add comments to daemon locking [Namhyung]
- split patch 1 to multiple patches [Namhyung]
- add missing allocation checks [Namhyung]
- add comments for session state transitions [Namhyung]
- add base directory check [Namhyung]
- use ',' as default for -x option [Namhyung]
- remove extra close before dup2 [Namhyung]
- add new reconfig test for empty config
- add --base option

v2 changes:
- switch options to sub-commands [Namhyung]
- use signalfd to track on sessions [Alexei]
- use stop command to stop sessions [Alexei]
- couple minor fixes [Alexei]
- more detailed changelogs [Arnaldo]
- added tests

thanks,
jirka


---
Jiri Olsa (24):
perf daemon: Add daemon command
perf daemon: Add config option
perf daemon: Add base option
perf daemon: Add server socket support
perf daemon: Add client socket support
perf daemon: Add config file support
perf daemon: Add config file change check
perf daemon: Add background support
perf daemon: Add signalfd support
perf daemon: Add list command
perf daemon: Add signal command
perf daemon: Add stop command
perf daemon: Allow only one daemon over base directory
perf daemon: Set control fifo for session
perf daemon: Add ping command
perf daemon: Use control to stop session
perf daemon: Add up time for daemon/session list
perf daemon: Add examples to man page
perf tests: Add daemon list command test
perf tests: Add daemon reconfig test
perf tests: Add daemon stop command test
perf tests: Add daemon signal command test
perf tests: Add daemon ping command test
perf tests: Add daemon lock test

tools/perf/Build | 1 +
tools/perf/Documentation/perf-config.txt | 14 ++
tools/perf/Documentation/perf-daemon.txt | 208 +++++++++++++++++++++++
tools/perf/builtin-daemon.c | 1506 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
tools/perf/builtin.h | 1 +
tools/perf/command-list.txt | 1 +
tools/perf/perf.c | 1 +
tools/perf/tests/shell/daemon.sh | 475 +++++++++++++++++++++++++++++++++++++++++++++++++++
8 files changed, 2207 insertions(+)
create mode 100644 tools/perf/Documentation/perf-daemon.txt
create mode 100644 tools/perf/builtin-daemon.c
create mode 100755 tools/perf/tests/shell/daemon.sh


---
Example with 2 record sessions:

# 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


Check sessions:

# perf daemon
[603349:daemon] base: /opt/perfdata
[603350:cycles] perf record -m 10M -e cycles --overwrite --switch-output -a
[603351:sched] perf record -m 20M -e sched:* --overwrite --switch-output -a

First line is daemon process info with configured daemon base.


Check sessions with more info:

# perf daemon -v
[603349:daemon] base: /opt/perfdata
output: /opt/perfdata/output
lock: /opt/perfdata/lock
up: 1 minutes
[603350:cycles] perf record -m 10M -e cycles --overwrite --switch-output -a
base: /opt/perfdata/session-cycles
output: /opt/perfdata/session-cycles/output
control: /opt/perfdata/session-cycles/control
ack: /opt/perfdata/session-cycles/ack
up: 1 minutes
[603351:sched] perf record -m 20M -e sched:* --overwrite --switch-output -a
base: /opt/perfdata/session-sched
output: /opt/perfdata/session-sched/output
control: /opt/perfdata/session-sched/control
ack: /opt/perfdata/session-sched/ack
up: 1 minutes

The 'base' path is daemon/session base.
The 'lock' file is daemon's lock file guarding that no other
daemon is running on top of the base.
The 'output' file is perf record output for specific session.
The 'control' and 'ack' files are perf control files.
The 'up' number shows minutes daemon/session is running.


Make sure control session is online:

# perf daemon ping
OK cycles
OK sched


Send USR2 signal to session 'cycles' to generate perf.data file:

# perf daemon signal --session cycles
signal 12 sent to session 'cycles [603452]'

# tail -2 /opt/perfdata/session-cycles/output
[ perf record: dump data: Woken up 1 times ]
[ perf record: Dump perf.data.2020123017013149 ]


Send USR2 signal to all sessions:

# perf daemon signal
signal 12 sent to session 'cycles [603452]'
signal 12 sent to session 'sched [603453]'

# tail -2 /opt/perfdata/session-cycles/output
[ perf record: dump data: Woken up 1 times ]
[ perf record: Dump perf.data.2020123017024689 ]
# tail -2 /opt/perfdata/session-sched/output
[ perf record: dump data: Woken up 1 times ]
[ perf record: Dump perf.data.2020123017024713 ]


Stop daemon:

# perf daemon stop


2021-02-08 21:13:12

by Jiri Olsa

[permalink] [raw]
Subject: [PATCH 05/24] perf daemon: Add client socket support

Adding support for client socket side that will be used
to send commands to daemon server socket.

This patch adds only the core support, all commands using
this functionality are coming in following patches.

Signed-off-by: Jiri Olsa <[email protected]>
---
tools/perf/builtin-daemon.c | 135 ++++++++++++++++++++++++++++++++++++
1 file changed, 135 insertions(+)

diff --git a/tools/perf/builtin-daemon.c b/tools/perf/builtin-daemon.c
index 495e4ff120ed..e0880c5ee39c 100644
--- a/tools/perf/builtin-daemon.c
+++ b/tools/perf/builtin-daemon.c
@@ -13,6 +13,7 @@
#include <sys/types.h>
#include <sys/socket.h>
#include <sys/un.h>
+#include <sys/stat.h>
#include <poll.h>
#include "builtin.h"
#include "perf.h"
@@ -44,6 +45,67 @@ static void sig_handler(int sig __maybe_unused)
done = true;
}

+static int client_config(const char *var, const char *value, void *cb)
+{
+ struct daemon *daemon = cb;
+
+ if (!strcmp(var, "daemon.base") && !daemon->base_user) {
+ daemon->base = strdup(value);
+ if (!daemon->base)
+ return -ENOMEM;
+ }
+
+ return 0;
+}
+
+static int check_base(struct daemon *daemon)
+{
+ struct stat st;
+
+ if (!daemon->base) {
+ pr_err("failed: base not defined\n");
+ return -EINVAL;
+ }
+
+ if (stat(daemon->base, &st)) {
+ switch (errno) {
+ case EACCES:
+ pr_err("failed: permission denied for '%s' base\n",
+ daemon->base);
+ return -EACCES;
+ case ENOENT:
+ pr_err("failed: base '%s' does not exists\n",
+ daemon->base);
+ return -EACCES;
+ default:
+ pr_err("failed: can't access base '%s': %s\n",
+ daemon->base, strerror(errno));
+ return -errno;
+ }
+ }
+
+ if ((st.st_mode & S_IFMT) != S_IFDIR) {
+ pr_err("failed: base '%s' is not directory\n",
+ daemon->base);
+ return -EINVAL;
+ }
+
+ return 0;
+}
+
+static int setup_client_config(struct daemon *daemon)
+{
+ struct perf_config_set *set = perf_config_set__load_file(daemon->config_real);
+ int err = -ENOMEM;
+
+ if (set) {
+ err = perf_config_set(set, client_config, daemon);
+ perf_config_set__delete(set);
+ }
+
+ return err ?: check_base(daemon);
+}
+
static int setup_server_socket(struct daemon *daemon)
{
struct sockaddr_un addr;
@@ -130,6 +192,38 @@ static int handle_server_socket(struct daemon *daemon __maybe_unused, int sock_f
return ret;
}

+static int setup_client_socket(struct daemon *daemon)
+{
+ struct sockaddr_un addr;
+ char path[PATH_MAX];
+ int fd = socket(AF_UNIX, SOCK_STREAM, 0);
+
+ if (fd == -1) {
+ perror("failed: socket");
+ return -1;
+ }
+
+ scnprintf(path, sizeof(path), "%s/control", daemon->base);
+
+ if (strlen(path) + 1 >= sizeof(addr.sun_path)) {
+ pr_err("failed: control path too long '%s'\n", path);
+ close(fd);
+ return -1;
+ }
+
+ memset(&addr, 0, sizeof(addr));
+ addr.sun_family = AF_UNIX;
+ strlcpy(addr.sun_path, path, sizeof(addr.sun_path) - 1);
+
+ if (connect(fd, (struct sockaddr *) &addr, sizeof(addr)) == -1) {
+ perror("failed: connect");
+ close(fd);
+ return -1;
+ }
+
+ return fd;
+}
+
static void daemon__exit(struct daemon *daemon)
{
free(daemon->config_real);
@@ -222,6 +316,47 @@ static int __cmd_start(struct daemon *daemon, struct option parent_options[],
return err;
}

+__maybe_unused
+static int send_cmd(struct daemon *daemon, union cmd *cmd)
+{
+ int ret = -1, fd;
+ char *line = NULL;
+ size_t len = 0;
+ ssize_t nread;
+ FILE *in = NULL;
+
+ if (setup_client_config(daemon))
+ return -1;
+
+ fd = setup_client_socket(daemon);
+ if (fd < 0)
+ return -1;
+
+ if (sizeof(*cmd) != writen(fd, cmd, sizeof(*cmd))) {
+ perror("failed: write");
+ goto out;
+ }
+
+ in = fdopen(fd, "r");
+ if (!in) {
+ perror("failed: fdopen");
+ goto out;
+ }
+
+ while ((nread = getline(&line, &len, in)) != -1) {
+ fwrite(line, nread, 1, stdout);
+ fflush(stdout);
+ }
+
+ ret = 0;
+ fclose(in);
+out:
+ /* If in is defined, then fd is closed via fclose. */
+ if (!in)
+ close(fd);
+ return ret;
+}
+
int cmd_daemon(int argc, const char **argv)
{
struct option daemon_options[] = {
--
2.29.2

2021-02-08 21:14:08

by Jiri Olsa

[permalink] [raw]
Subject: [PATCH 04/24] perf daemon: Add server socket support

Add support to create server socket that listens for client
commands and process them.

This patch adds only the core support, all commands using
this functionality are coming in following patches.

Signed-off-by: Jiri Olsa <[email protected]>
---
tools/perf/builtin-daemon.c | 117 +++++++++++++++++++++++++++++++++++-
1 file changed, 116 insertions(+), 1 deletion(-)

diff --git a/tools/perf/builtin-daemon.c b/tools/perf/builtin-daemon.c
index ce0373f453d6..495e4ff120ed 100644
--- a/tools/perf/builtin-daemon.c
+++ b/tools/perf/builtin-daemon.c
@@ -1,12 +1,19 @@
// SPDX-License-Identifier: GPL-2.0
+#include <internal/lib.h>
#include <subcmd/parse-options.h>
+#include <api/fd/array.h>
#include <linux/limits.h>
+#include <linux/string.h>
#include <string.h>
#include <signal.h>
#include <stdlib.h>
#include <stdio.h>
#include <unistd.h>
#include <errno.h>
+#include <sys/types.h>
+#include <sys/socket.h>
+#include <sys/un.h>
+#include <poll.h>
#include "builtin.h"
#include "perf.h"
#include "debug.h"
@@ -37,6 +44,92 @@ static void sig_handler(int sig __maybe_unused)
done = true;
}

+static int setup_server_socket(struct daemon *daemon)
+{
+ struct sockaddr_un addr;
+ char path[PATH_MAX];
+ int fd = socket(AF_UNIX, SOCK_STREAM, 0);
+
+ if (fd < 0) {
+ fprintf(stderr, "socket: %s\n", strerror(errno));
+ return -1;
+ }
+
+ if (fcntl(fd, F_SETFD, FD_CLOEXEC)) {
+ perror("failed: fcntl FD_CLOEXEC");
+ close(fd);
+ return -1;
+ }
+
+ scnprintf(path, sizeof(path), "%s/control", daemon->base);
+
+ if (strlen(path) + 1 >= sizeof(addr.sun_path)) {
+ pr_err("failed: control path too long '%s'\n", path);
+ close(fd);
+ return -1;
+ }
+
+ memset(&addr, 0, sizeof(addr));
+ addr.sun_family = AF_UNIX;
+
+ strlcpy(addr.sun_path, path, sizeof(addr.sun_path) - 1);
+ unlink(path);
+
+ if (bind(fd, (struct sockaddr *)&addr, sizeof(addr)) == -1) {
+ perror("failed: bind");
+ close(fd);
+ return -1;
+ }
+
+ if (listen(fd, 1) == -1) {
+ perror("failed: listen");
+ close(fd);
+ return -1;
+ }
+
+ return fd;
+}
+
+union cmd {
+ int cmd;
+};
+
+static int handle_server_socket(struct daemon *daemon __maybe_unused, int sock_fd)
+{
+ int ret = -1, fd;
+ FILE *out = NULL;
+ union cmd cmd;
+
+ fd = accept(sock_fd, NULL, NULL);
+ if (fd < 0) {
+ perror("failed: accept");
+ return -1;
+ }
+
+ if (sizeof(cmd) != readn(fd, &cmd, sizeof(cmd))) {
+ perror("failed: read");
+ goto out;
+ }
+
+ out = fdopen(fd, "w");
+ if (!out) {
+ perror("failed: fdopen");
+ goto out;
+ }
+
+ switch (cmd.cmd) {
+ default:
+ break;
+ }
+
+ fclose(out);
+out:
+ /* If out is defined, then fd is closed via fclose. */
+ if (!out)
+ close(fd);
+ return ret;
+}
+
static void daemon__exit(struct daemon *daemon)
{
free(daemon->config_real);
@@ -77,6 +170,9 @@ static int __cmd_start(struct daemon *daemon, struct option parent_options[],
OPT_PARENT(parent_options),
OPT_END()
};
+ int sock_fd = -1;
+ int sock_pos;
+ struct fdarray fda;
int err = 0;

argc = parse_options(argc, argv, start_options, daemon_usage, 0);
@@ -93,15 +189,34 @@ static int __cmd_start(struct daemon *daemon, struct option parent_options[],

pr_info("daemon started (pid %d)\n", getpid());

+ fdarray__init(&fda, 1);
+
+ sock_fd = setup_server_socket(daemon);
+ if (sock_fd < 0)
+ goto out;
+
+ sock_pos = fdarray__add(&fda, sock_fd, POLLIN|POLLERR|POLLHUP, 0);
+ if (sock_pos < 0)
+ goto out;
+
signal(SIGINT, sig_handler);
signal(SIGTERM, sig_handler);

while (!done && !err) {
- sleep(1);
+ if (fdarray__poll(&fda, -1)) {
+ if (fda.entries[sock_pos].revents & POLLIN)
+ err = handle_server_socket(daemon, sock_fd);
+ }
}

+out:
+ fdarray__exit(&fda);
+
daemon__exit(daemon);

+ if (sock_fd != -1)
+ close(sock_fd);
+
pr_info("daemon exited\n");
fclose(daemon->out);
return err;
--
2.29.2

2021-02-08 21:14:49

by Jiri Olsa

[permalink] [raw]
Subject: [PATCH 09/24] perf daemon: Add signalfd support

Using signalfd fd for tracking SIGCHLD signals as
notification for perf session termination.

This way we don't need to actively check for child
status, but we are notified if there's change.

Suggested-by: Alexei Budankov <[email protected]>
Signed-off-by: Jiri Olsa <[email protected]>
---
tools/perf/builtin-daemon.c | 161 ++++++++++++++++++++++++++++++++++--
1 file changed, 155 insertions(+), 6 deletions(-)

diff --git a/tools/perf/builtin-daemon.c b/tools/perf/builtin-daemon.c
index 2d7f282809b6..8b834a5d91ff 100644
--- a/tools/perf/builtin-daemon.c
+++ b/tools/perf/builtin-daemon.c
@@ -9,6 +9,7 @@
#include <string.h>
#include <signal.h>
#include <stdlib.h>
+#include <time.h>
#include <stdio.h>
#include <unistd.h>
#include <errno.h>
@@ -18,6 +19,8 @@
#include <sys/socket.h>
#include <sys/un.h>
#include <sys/stat.h>
+#include <sys/signalfd.h>
+#include <sys/wait.h>
#include <poll.h>
#include <sys/stat.h>
#include "builtin.h"
@@ -83,6 +86,7 @@ struct daemon {
struct list_head sessions;
FILE *out;
char perf[PATH_MAX];
+ int signal_fd;
};

static struct daemon __daemon = {
@@ -368,6 +372,116 @@ static int daemon_session__run(struct daemon_session *session,
return -1;
}

+static pid_t handle_signalfd(struct daemon *daemon)
+{
+ struct daemon_session *session;
+ struct signalfd_siginfo si;
+ ssize_t err;
+ int status;
+ pid_t pid;
+
+ err = read(daemon->signal_fd, &si, sizeof(struct signalfd_siginfo));
+ if (err != sizeof(struct signalfd_siginfo))
+ return -1;
+
+ list_for_each_entry(session, &daemon->sessions, list) {
+
+ if (session->pid != (int) si.ssi_pid)
+ continue;
+
+ pid = waitpid(session->pid, &status, 0);
+ if (pid == session->pid) {
+ if (WIFEXITED(status)) {
+ pr_info("session '%s' exited, status=%d\n",
+ session->name, WEXITSTATUS(status));
+ } else if (WIFSIGNALED(status)) {
+ pr_info("session '%s' killed (signal %d)\n",
+ session->name, WTERMSIG(status));
+ } else if (WIFSTOPPED(status)) {
+ pr_info("session '%s' stopped (signal %d)\n",
+ session->name, WSTOPSIG(status));
+ } else {
+ pr_info("session '%s' Unexpected status (0x%x)\n",
+ session->name, status);
+ }
+ }
+
+ session->state = KILL;
+ session->pid = -1;
+ return pid;
+ }
+
+ return 0;
+}
+
+static int daemon_session__wait(struct daemon_session *session, struct daemon *daemon,
+ int secs)
+{
+ struct pollfd pollfd = {
+ .fd = daemon->signal_fd,
+ .events = POLLIN,
+ };
+ pid_t wpid = 0, pid = session->pid;
+ time_t start;
+
+ start = time(NULL);
+
+ do {
+ int err = poll(&pollfd, 1, 1000);
+
+ if (err > 0) {
+ wpid = handle_signalfd(daemon);
+ } else if (err < 0) {
+ perror("failed: poll\n");
+ return -1;
+ }
+
+ if (start + secs < time(NULL))
+ return -1;
+ } while (wpid != pid);
+
+ return 0;
+}
+
+static bool daemon__has_alive_session(struct daemon *daemon)
+{
+ struct daemon_session *session;
+
+ list_for_each_entry(session, &daemon->sessions, list) {
+ if (session->pid != -1)
+ return true;
+ }
+
+ return false;
+}
+
+static int daemon__wait(struct daemon *daemon, int secs)
+{
+ struct pollfd pollfd = {
+ .fd = daemon->signal_fd,
+ .events = POLLIN,
+ };
+ time_t start;
+
+ start = time(NULL);
+
+ do {
+ int err = poll(&pollfd, 1, 1000);
+
+ if (err > 0) {
+ handle_signalfd(daemon);
+ } else if (err < 0) {
+ perror("failed: poll\n");
+ return -1;
+ }
+
+ if (start + secs < time(NULL))
+ return -1;
+ } while (daemon__has_alive_session(daemon));
+
+ return 0;
+}
+
static int setup_server_socket(struct daemon *daemon)
{
struct sockaddr_un addr;
@@ -493,9 +607,14 @@ static int daemon_session__signal(struct daemon_session *session, int sig)
return kill(session->pid, sig);
}

-static void daemon_session__kill(struct daemon_session *session)
+static void daemon_session__kill(struct daemon_session *session,
+ struct daemon *daemon)
{
daemon_session__signal(session, SIGTERM);
+ if (daemon_session__wait(session, daemon, 10)) {
+ daemon_session__signal(session, SIGKILL);
+ daemon_session__wait(session, daemon, 10);
+ }
}

static void daemon__signal(struct daemon *daemon, int sig)
@@ -523,6 +642,10 @@ static void daemon_session__remove(struct daemon_session *session)
static void daemon__kill(struct daemon *daemon)
{
daemon__signal(daemon, SIGTERM);
+ if (daemon__wait(daemon, 10)) {
+ daemon__signal(daemon, SIGKILL);
+ daemon__wait(daemon, 10);
+ }
}

static void daemon__exit(struct daemon *daemon)
@@ -549,7 +672,7 @@ static int daemon__reconfig(struct daemon *daemon)
/* Remove session. */
if (session->state == KILL) {
if (session->pid > 0) {
- daemon_session__kill(session);
+ daemon_session__kill(session, daemon);
pr_info("reconfig: session '%s' killed\n", session->name);
}
daemon_session__remove(session);
@@ -558,7 +681,7 @@ static int daemon__reconfig(struct daemon *daemon)

/* Reconfig session. */
if (session->pid > 0) {
- daemon_session__kill(session);
+ daemon_session__kill(session, daemon);
pr_info("reconfig: session '%s' killed\n", session->name);
}
if (daemon_session__run(session, daemon))
@@ -724,6 +847,20 @@ static int go_background(struct daemon *daemon)
return 0;
}

+static int setup_signalfd(struct daemon *daemon)
+{
+ sigset_t mask;
+
+ sigemptyset(&mask);
+ sigaddset(&mask, SIGCHLD);
+
+ if (sigprocmask(SIG_BLOCK, &mask, NULL) == -1)
+ return -1;
+
+ daemon->signal_fd = signalfd(-1, &mask, SFD_NONBLOCK|SFD_CLOEXEC);
+ return daemon->signal_fd;
+}
+
static int __cmd_start(struct daemon *daemon, struct option parent_options[],
int argc, const char **argv)
{
@@ -733,8 +870,8 @@ static int __cmd_start(struct daemon *daemon, struct option parent_options[],
OPT_PARENT(parent_options),
OPT_END()
};
- int sock_fd = -1, conf_fd = -1;
- int sock_pos, file_pos;
+ int sock_fd = -1, conf_fd = -1, signal_fd = -1;
+ int sock_pos, file_pos, signal_pos;
struct fdarray fda;
int err = 0;

@@ -766,7 +903,7 @@ static int __cmd_start(struct daemon *daemon, struct option parent_options[],

pr_info("daemon started (pid %d)\n", getpid());

- fdarray__init(&fda, 2);
+ fdarray__init(&fda, 3);

sock_fd = setup_server_socket(daemon);
if (sock_fd < 0)
@@ -776,6 +913,10 @@ static int __cmd_start(struct daemon *daemon, struct option parent_options[],
if (conf_fd < 0)
goto out;

+ signal_fd = setup_signalfd(daemon);
+ if (signal_fd < 0)
+ goto out;
+
sock_pos = fdarray__add(&fda, sock_fd, POLLIN|POLLERR|POLLHUP, 0);
if (sock_pos < 0)
goto out;
@@ -784,6 +925,10 @@ static int __cmd_start(struct daemon *daemon, struct option parent_options[],
if (file_pos < 0)
goto out;

+ signal_pos = fdarray__add(&fda, signal_fd, POLLIN|POLLERR|POLLHUP, 0);
+ if (signal_pos < 0)
+ goto out;
+
signal(SIGINT, sig_handler);
signal(SIGTERM, sig_handler);

@@ -797,6 +942,8 @@ static int __cmd_start(struct daemon *daemon, struct option parent_options[],
err = handle_server_socket(daemon, sock_fd);
if (fda.entries[file_pos].revents & POLLIN)
err = handle_config_changes(daemon, conf_fd, &reconfig);
+ if (fda.entries[signal_pos].revents & POLLIN)
+ err = handle_signalfd(daemon) < 0;

if (reconfig)
err = setup_server_config(daemon);
@@ -813,6 +960,8 @@ static int __cmd_start(struct daemon *daemon, struct option parent_options[],
close(sock_fd);
if (conf_fd != -1)
close(conf_fd);
+ if (conf_fd != -1)
+ close(signal_fd);

pr_info("daemon exited\n");
fclose(daemon->out);
--
2.29.2

2021-02-08 21:15:03

by Jiri Olsa

[permalink] [raw]
Subject: [PATCH 10/24] perf daemon: Add list command

Adding list command to display all running sessions.
It's the default command if no other command is specified.

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

List sessions:

# perf daemon
[771394:daemon] base: /opt/perfdata
[771395:cycles] perf record -m 10M -e cycles --overwrite --switch-output -a
[771396:sched] perf record -m 20M -e sched:* --overwrite --switch-output -a

List sessions with more info:

# perf daemon -v
[771394:daemon] base: /opt/perfdata
output: /opt/perfdata/output
[771395:cycles] perf record -m 10M -e cycles --overwrite --switch-output -a
base: /opt/perfdata/session-cycles
output: /opt/perfdata/session-cycles/output
[771396:sched] perf record -m 20M -e sched:* --overwrite --switch-output -a
base: /opt/perfdata/session-sched
output: /opt/perfdata/session-sched/output

The 'output' file is perf record output for specific session.

Note you have to stop all running perf processes manually at
this point, stop command is coming in following patches.

Signed-off-by: Jiri Olsa <[email protected]>
---
tools/perf/builtin-daemon.c | 89 +++++++++++++++++++++++++++++++++++--
1 file changed, 86 insertions(+), 3 deletions(-)

diff --git a/tools/perf/builtin-daemon.c b/tools/perf/builtin-daemon.c
index 8b834a5d91ff..8a0994b0dfce 100644
--- a/tools/perf/builtin-daemon.c
+++ b/tools/perf/builtin-daemon.c
@@ -81,6 +81,7 @@ struct daemon {
const char *config;
char *config_real;
char *config_base;
+ const char *csv_sep;
const char *base_user;
char *base;
struct list_head sessions;
@@ -528,11 +529,78 @@ static int setup_server_socket(struct daemon *daemon)
return fd;
}

+enum {
+ CMD_LIST = 0,
+ CMD_MAX,
+};
+
union cmd {
int cmd;
+
+ /* CMD_LIST */
+ struct {
+ int cmd;
+ int verbose;
+ char csv_sep;
+ } list;
};

-static int handle_server_socket(struct daemon *daemon __maybe_unused, int sock_fd)
+static int cmd_session_list(struct daemon *daemon, union cmd *cmd, FILE *out)
+{
+ char csv_sep = cmd->list.csv_sep;
+ struct daemon_session *session;
+
+ if (csv_sep) {
+ fprintf(out, "%d%c%s%c%s%c%s/%s",
+ /* pid daemon */
+ getpid(), csv_sep, "daemon",
+ /* base */
+ csv_sep, daemon->base,
+ /* output */
+ csv_sep, daemon->base, SESSION_OUTPUT);
+
+ 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);
+ }
+ }
+
+ list_for_each_entry(session, &daemon->sessions, list) {
+ if (csv_sep) {
+ fprintf(out, "%d%c%s%c%s",
+ /* pid */
+ session->pid,
+ /* name */
+ csv_sep, session->name,
+ /* base */
+ csv_sep, session->run);
+
+ fprintf(out, "%c%s%c%s/%s",
+ /* session dir */
+ csv_sep, session->base,
+ /* session output */
+ csv_sep, session->base, SESSION_OUTPUT);
+
+ fprintf(out, "\n");
+ } else {
+ fprintf(out, "[%d:%s] perf record %s\n",
+ session->pid, session->name, session->run);
+ if (!cmd->list.verbose)
+ continue;
+ fprintf(out, " base: %s\n",
+ session->base);
+ fprintf(out, " output: %s/%s\n",
+ session->base, SESSION_OUTPUT);
+ }
+ }
+
+ return 0;
+}
+
+static int handle_server_socket(struct daemon *daemon, int sock_fd)
{
int ret = -1, fd;
FILE *out = NULL;
@@ -556,6 +624,9 @@ static int handle_server_socket(struct daemon *daemon __maybe_unused, int sock_f
}

switch (cmd.cmd) {
+ case CMD_LIST:
+ ret = cmd_session_list(daemon, &cmd, out);
+ break;
default:
break;
}
@@ -968,7 +1039,6 @@ static int __cmd_start(struct daemon *daemon, struct option parent_options[],
return err;
}

-__maybe_unused
static int send_cmd(struct daemon *daemon, union cmd *cmd)
{
int ret = -1, fd;
@@ -1009,6 +1079,17 @@ static int send_cmd(struct daemon *daemon, union cmd *cmd)
return ret;
}

+static int send_cmd_list(struct daemon *daemon)
+{
+ union cmd cmd = {
+ .list.cmd = CMD_LIST,
+ .list.verbose = verbose,
+ };
+
+ cmd.list.csv_sep = daemon->csv_sep ? *daemon->csv_sep : 0;
+ return send_cmd(daemon, &cmd);
+}
+
int cmd_daemon(int argc, const char **argv)
{
struct option daemon_options[] = {
@@ -1017,6 +1098,8 @@ int cmd_daemon(int argc, const char **argv)
"config file", "config file path"),
OPT_STRING(0, "base", &__daemon.base_user,
"directory", "base directory"),
+ OPT_STRING_OPTARG('x', "field-separator", &__daemon.csv_sep,
+ "field separator", "print counts with custom separator", ","),
OPT_END()
};

@@ -1039,5 +1122,5 @@ int cmd_daemon(int argc, const char **argv)
return -1;
}

- return -1;
+ return send_cmd_list(&__daemon);
}
--
2.29.2

2021-02-08 21:15:22

by Jiri Olsa

[permalink] [raw]
Subject: [PATCH 08/24] perf daemon: Add background support

Adding support to put daemon process in the background.

It's now enabled by default and -f option is added to
keep daemon process on the console for debugging.

Signed-off-by: Jiri Olsa <[email protected]>
---
tools/perf/Documentation/perf-daemon.txt | 4 ++
tools/perf/builtin-daemon.c | 62 ++++++++++++++++++++++++
2 files changed, 66 insertions(+)

diff --git a/tools/perf/Documentation/perf-daemon.txt b/tools/perf/Documentation/perf-daemon.txt
index 173b3f9f3a41..af5916d1c3e0 100644
--- a/tools/perf/Documentation/perf-daemon.txt
+++ b/tools/perf/Documentation/perf-daemon.txt
@@ -56,6 +56,10 @@ START COMMAND
-------------
The start command creates the daemon process.

+-f::
+--foreground::
+ Do not put the process in background.
+

CONFIG FILE
-----------
diff --git a/tools/perf/builtin-daemon.c b/tools/perf/builtin-daemon.c
index bdddaa2ea388..2d7f282809b6 100644
--- a/tools/perf/builtin-daemon.c
+++ b/tools/perf/builtin-daemon.c
@@ -675,10 +675,61 @@ static int setup_config(struct daemon *daemon)
return daemon->config_real ? 0 : -1;
}

+static int go_background(struct daemon *daemon)
+{
+ int pid, fd;
+
+ pid = fork();
+ if (pid < 0)
+ return -1;
+
+ if (pid > 0)
+ return 1;
+
+ if (setsid() < 0)
+ return -1;
+
+ umask(0);
+
+ if (chdir(daemon->base)) {
+ perror("failed: chdir");
+ return -1;
+ }
+
+ fd = open("output", O_RDWR|O_CREAT|O_TRUNC, 0644);
+ if (fd < 0) {
+ perror("failed: open");
+ return -1;
+ }
+
+ if (fcntl(fd, F_SETFD, FD_CLOEXEC)) {
+ perror("failed: fcntl FD_CLOEXEC");
+ close(fd);
+ return -1;
+ }
+
+ close(0);
+ dup2(fd, 1);
+ dup2(fd, 2);
+ close(fd);
+
+ daemon->out = fdopen(1, "w");
+ if (!daemon->out) {
+ close(1);
+ close(2);
+ return -1;
+ }
+
+ setbuf(daemon->out, NULL);
+ return 0;
+}
+
static int __cmd_start(struct daemon *daemon, struct option parent_options[],
int argc, const char **argv)
{
+ bool foreground = false;
struct option start_options[] = {
+ OPT_BOOLEAN('f', "foreground", &foreground, "stay on console"),
OPT_PARENT(parent_options),
OPT_END()
};
@@ -699,6 +750,17 @@ static int __cmd_start(struct daemon *daemon, struct option parent_options[],
if (setup_server_config(daemon))
return -1;

+ if (!foreground) {
+ err = go_background(daemon);
+ if (err) {
+ /* original process, exit normally */
+ if (err == 1)
+ err = 0;
+ daemon__exit(daemon);
+ return err;
+ }
+ }
+
debug_set_file(daemon->out);
debug_set_display_time(true);

--
2.29.2

2021-02-08 21:15:44

by Jiri Olsa

[permalink] [raw]
Subject: [PATCH 12/24] perf daemon: Add stop command

Add 'perf daemon stop' command to stop daemon process
and all running sessions.

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

Stopping the daemon

# perf daemon stop

Daemon is not running, nothing to connect to:

# perf daemon
connect error: Connection refused

Signed-off-by: Jiri Olsa <[email protected]>
---
tools/perf/Documentation/perf-daemon.txt | 6 +++++
tools/perf/builtin-daemon.c | 29 ++++++++++++++++++++++++
2 files changed, 35 insertions(+)

diff --git a/tools/perf/Documentation/perf-daemon.txt b/tools/perf/Documentation/perf-daemon.txt
index 9cd47ec959e9..94d5e09a1e17 100644
--- a/tools/perf/Documentation/perf-daemon.txt
+++ b/tools/perf/Documentation/perf-daemon.txt
@@ -13,6 +13,7 @@ SYNOPSIS
'perf daemon'
'perf daemon' [<options>]
'perf daemon start' [<options>]
+'perf daemon stop' [<options>]
'perf daemon signal' [<options>]


@@ -62,6 +63,11 @@ The start command creates the daemon process.
Do not put the process in background.


+STOP COMMAND
+------------
+The stop command stops all the session and the daemon process.
+
+
SIGNAL COMMAND
--------------
The signal command sends signal to configured sessions.
diff --git a/tools/perf/builtin-daemon.c b/tools/perf/builtin-daemon.c
index 027e1e58b67b..2ef7fe9200f3 100644
--- a/tools/perf/builtin-daemon.c
+++ b/tools/perf/builtin-daemon.c
@@ -532,6 +532,7 @@ static int setup_server_socket(struct daemon *daemon)
enum {
CMD_LIST = 0,
CMD_SIGNAL = 1,
+ CMD_STOP = 2,
CMD_MAX,
};

@@ -665,6 +666,11 @@ static int handle_server_socket(struct daemon *daemon, int sock_fd)
case CMD_SIGNAL:
ret = cmd_session_kill(daemon, &cmd, out);
break;
+ case CMD_STOP:
+ done = 1;
+ ret = 0;
+ pr_debug("perf daemon is exciting\n");
+ break;
default:
break;
}
@@ -1149,6 +1155,27 @@ static int __cmd_signal(struct daemon *daemon, struct option parent_options[],
return send_cmd(daemon, &cmd);
}

+static int __cmd_stop(struct daemon *daemon, struct option parent_options[],
+ int argc, const char **argv)
+{
+ struct option start_options[] = {
+ OPT_PARENT(parent_options),
+ OPT_END()
+ };
+ union cmd cmd = { .cmd = CMD_STOP, };
+
+ argc = parse_options(argc, argv, start_options, daemon_usage, 0);
+ if (argc)
+ usage_with_options(daemon_usage, start_options);
+
+ if (setup_config(daemon)) {
+ pr_err("failed: config not found\n");
+ return -1;
+ }
+
+ return send_cmd(daemon, &cmd);
+}
+
int cmd_daemon(int argc, const char **argv)
{
struct option daemon_options[] = {
@@ -1173,6 +1200,8 @@ int cmd_daemon(int argc, const char **argv)
return __cmd_start(&__daemon, daemon_options, argc, argv);
if (!strcmp(argv[0], "signal"))
return __cmd_signal(&__daemon, daemon_options, argc, argv);
+ else if (!strcmp(argv[0], "stop"))
+ return __cmd_stop(&__daemon, daemon_options, argc, argv);

pr_err("failed: unknown command '%s'\n", argv[0]);
return -1;
--
2.29.2

2021-02-08 21:16:19

by Jiri Olsa

[permalink] [raw]
Subject: [PATCH 03/24] perf daemon: Add base option

Adding base option allowing user to specify base directory.
It will have precedence over config file base definition
coming in following patches.

Signed-off-by: Jiri Olsa <[email protected]>
---
tools/perf/Documentation/perf-daemon.txt | 4 ++++
tools/perf/builtin-daemon.c | 11 +++++++++++
2 files changed, 15 insertions(+)

diff --git a/tools/perf/Documentation/perf-daemon.txt b/tools/perf/Documentation/perf-daemon.txt
index ba3f88510aee..1a4158cd973e 100644
--- a/tools/perf/Documentation/perf-daemon.txt
+++ b/tools/perf/Documentation/perf-daemon.txt
@@ -31,6 +31,10 @@ OPTIONS
Config file path. If not provided, perf will check system and default
locations (/etc/perfconfig, $HOME/.perfconfig).

+--base=<PATH>::
+ Base directory path. Each daemon instance is running on top
+ of base directory.
+
All generic options are available also under commands.


diff --git a/tools/perf/builtin-daemon.c b/tools/perf/builtin-daemon.c
index 90b5a8ea9dda..ce0373f453d6 100644
--- a/tools/perf/builtin-daemon.c
+++ b/tools/perf/builtin-daemon.c
@@ -6,6 +6,7 @@
#include <stdlib.h>
#include <stdio.h>
#include <unistd.h>
+#include <errno.h>
#include "builtin.h"
#include "perf.h"
#include "debug.h"
@@ -15,6 +16,7 @@
struct daemon {
const char *config;
char *config_real;
+ const char *base_user;
char *base;
FILE *out;
char perf[PATH_MAX];
@@ -38,10 +40,17 @@ static void sig_handler(int sig __maybe_unused)
static void daemon__exit(struct daemon *daemon)
{
free(daemon->config_real);
+ free(daemon->base);
}

static int setup_config(struct daemon *daemon)
{
+ if (daemon->base_user) {
+ daemon->base = strdup(daemon->base_user);
+ if (!daemon->base)
+ return -ENOMEM;
+ }
+
if (daemon->config) {
char *real = realpath(daemon->config, NULL);

@@ -104,6 +113,8 @@ int cmd_daemon(int argc, const char **argv)
OPT_INCR('v', "verbose", &verbose, "be more verbose"),
OPT_STRING(0, "config", &__daemon.config,
"config file", "config file path"),
+ OPT_STRING(0, "base", &__daemon.base_user,
+ "directory", "base directory"),
OPT_END()
};

--
2.29.2

2021-02-08 21:16:23

by Jiri Olsa

[permalink] [raw]
Subject: [PATCH 13/24] perf daemon: Allow only one daemon over base directory

Add 'lock' file under daemon base and flock it, so only one
perf daemon can run on top of it.

Each daemon tries to create and lock BASE/lock file, if it's
successful we are sure we're the only daemon running over
the BASE.

Once daemon is finished, file descriptor to lock file is
closed and lock is released.

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 <[email protected]>
---
tools/perf/Documentation/perf-daemon.txt | 3 +-
tools/perf/builtin-daemon.c | 61 ++++++++++++++++++++++++
2 files changed, 63 insertions(+), 1 deletion(-)

diff --git a/tools/perf/Documentation/perf-daemon.txt b/tools/perf/Documentation/perf-daemon.txt
index 94d5e09a1e17..3c9e265858b2 100644
--- a/tools/perf/Documentation/perf-daemon.txt
+++ b/tools/perf/Documentation/perf-daemon.txt
@@ -49,7 +49,8 @@ OPTIONS

--base=<PATH>::
Base directory path. Each daemon instance is running on top
- of base directory.
+ of base directory. Only one instance of server can run on
+ top of one directory at the time.

All generic options are available also under commands.

diff --git a/tools/perf/builtin-daemon.c b/tools/perf/builtin-daemon.c
index 2ef7fe9200f3..22b2ec18b01b 100644
--- a/tools/perf/builtin-daemon.c
+++ b/tools/perf/builtin-daemon.c
@@ -2,11 +2,13 @@
#include <internal/lib.h>
#include <subcmd/parse-options.h>
#include <api/fd/array.h>
+#include <api/fs/fs.h>
#include <linux/zalloc.h>
#include <linux/string.h>
#include <linux/limits.h>
#include <linux/string.h>
#include <string.h>
+#include <sys/file.h>
#include <signal.h>
#include <stdlib.h>
#include <time.h>
@@ -570,12 +572,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);
}
}

@@ -906,6 +914,53 @@ static int setup_config(struct daemon *daemon)
return daemon->config_real ? 0 : -1;
}

+/*
+ * Each daemon tries to create and lock BASE/lock file,
+ * if it's successful we are sure we're the only daemon
+ * running over the BASE.
+ *
+ * Once daemon is finished, file descriptor to lock file
+ * is closed and lock is released.
+ */
+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);
+ close(fd);
+ return -1;
+ }
+
+ scnprintf(buf, sizeof(buf), "%d", getpid());
+ len = strlen(buf);
+
+ if (write(fd, buf, len) != len) {
+ perror("failed: write");
+ close(fd);
+ return -1;
+ }
+
+ if (ftruncate(fd, len)) {
+ perror("failed: ftruncate");
+ close(fd);
+ return -1;
+ }
+
+ return 0;
+}
+
static int go_background(struct daemon *daemon)
{
int pid, fd;
@@ -920,6 +975,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)) {
@@ -995,6 +1053,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) {
err = go_background(daemon);
if (err) {
--
2.29.2

2021-02-08 21:16:24

by Jiri Olsa

[permalink] [raw]
Subject: [PATCH 11/24] perf daemon: Add signal command

Allow perf daemon to send SIGUSR2 to all running sessions
or just to a specific session.

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

Send signal to all running sessions:

# perf daemon signal
signal 12 sent to session 'cycles [773738]'
signal 12 sent to session 'sched [773739]'

Or to specific one:

# perf daemon signal --session sched
signal 12 sent to session 'sched [773739]'

And verify signals were delivered and perf.data dumped:

# cat /opt/perfdata/session-cycles/output
rounding mmap pages size to 32M (8192 pages)
[ perf record: dump data: Woken up 1 times ]
[ perf record: Dump perf.data.2021010220382490 ]

# car /opt/perfdata/session-sched/output
rounding mmap pages size to 32M (8192 pages)
[ perf record: dump data: Woken up 1 times ]
[ perf record: Dump perf.data.2021010220382489 ]
[ perf record: dump data: Woken up 1 times ]
[ perf record: Dump perf.data.2021010220393745 ]

Signed-off-by: Jiri Olsa <[email protected]>
---
tools/perf/Documentation/perf-daemon.txt | 9 +++
tools/perf/builtin-daemon.c | 75 +++++++++++++++++++++---
2 files changed, 77 insertions(+), 7 deletions(-)

diff --git a/tools/perf/Documentation/perf-daemon.txt b/tools/perf/Documentation/perf-daemon.txt
index af5916d1c3e0..9cd47ec959e9 100644
--- a/tools/perf/Documentation/perf-daemon.txt
+++ b/tools/perf/Documentation/perf-daemon.txt
@@ -13,6 +13,7 @@ SYNOPSIS
'perf daemon'
'perf daemon' [<options>]
'perf daemon start' [<options>]
+'perf daemon signal' [<options>]


DESCRIPTION
@@ -61,6 +62,14 @@ The start command creates the daemon process.
Do not put the process in background.


+SIGNAL COMMAND
+--------------
+The signal command sends signal to configured sessions.
+
+--session::
+ Send signal to specific session.
+
+
CONFIG FILE
-----------
The daemon is configured within standard perf config file by
diff --git a/tools/perf/builtin-daemon.c b/tools/perf/builtin-daemon.c
index 8a0994b0dfce..027e1e58b67b 100644
--- a/tools/perf/builtin-daemon.c
+++ b/tools/perf/builtin-daemon.c
@@ -531,9 +531,12 @@ static int setup_server_socket(struct daemon *daemon)

enum {
CMD_LIST = 0,
+ CMD_SIGNAL = 1,
CMD_MAX,
};

+#define SESSION_MAX 64
+
union cmd {
int cmd;

@@ -543,6 +546,13 @@ union cmd {
int verbose;
char csv_sep;
} list;
+
+ /* CMD_SIGNAL */
+ struct {
+ int cmd;
+ int sig;
+ char name[SESSION_MAX];
+ } signal;
};

static int cmd_session_list(struct daemon *daemon, union cmd *cmd, FILE *out)
@@ -600,6 +610,31 @@ static int cmd_session_list(struct daemon *daemon, union cmd *cmd, FILE *out)
return 0;
}

+static int daemon_session__signal(struct daemon_session *session, int sig)
+{
+ if (session->pid < 0)
+ return -1;
+ return kill(session->pid, sig);
+}
+
+static int cmd_session_kill(struct daemon *daemon, union cmd *cmd, FILE *out)
+{
+ struct daemon_session *session;
+ bool all = false;
+
+ all = !strcmp(cmd->signal.name, "all");
+
+ list_for_each_entry(session, &daemon->sessions, list) {
+ if (all || !strcmp(cmd->signal.name, session->name)) {
+ daemon_session__signal(session, cmd->signal.sig);
+ fprintf(out, "signal %d sent to session '%s [%d]'\n",
+ cmd->signal.sig, session->name, session->pid);
+ }
+ }
+
+ return 0;
+}
+
static int handle_server_socket(struct daemon *daemon, int sock_fd)
{
int ret = -1, fd;
@@ -627,6 +662,9 @@ static int handle_server_socket(struct daemon *daemon, int sock_fd)
case CMD_LIST:
ret = cmd_session_list(daemon, &cmd, out);
break;
+ case CMD_SIGNAL:
+ ret = cmd_session_kill(daemon, &cmd, out);
+ break;
default:
break;
}
@@ -671,13 +709,6 @@ static int setup_client_socket(struct daemon *daemon)
return fd;
}

-static int daemon_session__signal(struct daemon_session *session, int sig)
-{
- if (session->pid < 0)
- return -1;
- return kill(session->pid, sig);
-}
-
static void daemon_session__kill(struct daemon_session *session,
struct daemon *daemon)
{
@@ -1090,6 +1121,34 @@ static int send_cmd_list(struct daemon *daemon)
return send_cmd(daemon, &cmd);
}

+static int __cmd_signal(struct daemon *daemon, struct option parent_options[],
+ int argc, const char **argv)
+{
+ const char *name = "all";
+ struct option start_options[] = {
+ OPT_STRING(0, "session", &name, "session",
+ "Sent signal to specific session"),
+ OPT_PARENT(parent_options),
+ OPT_END()
+ };
+ union cmd cmd;
+
+ argc = parse_options(argc, argv, start_options, daemon_usage, 0);
+ if (argc)
+ usage_with_options(daemon_usage, start_options);
+
+ if (setup_config(daemon)) {
+ pr_err("failed: config not found\n");
+ return -1;
+ }
+
+ cmd.signal.cmd = CMD_SIGNAL,
+ cmd.signal.sig = SIGUSR2;
+ strncpy(cmd.signal.name, name, sizeof(cmd.signal.name) - 1);
+
+ return send_cmd(daemon, &cmd);
+}
+
int cmd_daemon(int argc, const char **argv)
{
struct option daemon_options[] = {
@@ -1112,6 +1171,8 @@ int cmd_daemon(int argc, const char **argv)
if (argc) {
if (!strcmp(argv[0], "start"))
return __cmd_start(&__daemon, daemon_options, argc, argv);
+ if (!strcmp(argv[0], "signal"))
+ return __cmd_signal(&__daemon, daemon_options, argc, argv);

pr_err("failed: unknown command '%s'\n", argv[0]);
return -1;
--
2.29.2

2021-02-08 21:16:58

by Jiri Olsa

[permalink] [raw]
Subject: [PATCH 06/24] perf daemon: Add config file support

Adding support to configure daemon with config file.

Each client or server invocation of perf daemon needs to know the
base directory, where all sessions data is stored.

The base is defined with:

daemon.base
Base path for daemon data. All sessions data are stored under
this path.

The daemon allows to create record sessions. Each session is a
record command spawned and monitored by perf daemon.

The session is defined with:

session-<NAME>.run
Defines new record session for daemon. The value is record's
command line without the 'record' keyword.

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

Example above defines '/opt/perfdata' as a base directory
and 2 record sessions.

# perf daemon start
[2021-01-28 19:47:33.454413] daemon started (pid 16015)
[2021-01-28 19:47:33.455910] reconfig: ruining session [cycles:16016]: -m 10M -e cycles --overwrite --switch-output -a
[2021-01-28 19:47:33.456599] reconfig: ruining session [sched:16017]: -m 20M -e sched:* --overwrite --switch-output -a

# ps -ef | grep perf
... perf daemon start
... /home/jolsa/.../perf record -m 20M -e cycles --overwrite --switch-output -a
... /home/jolsa/.../perf record -m 20M -e sched:* --overwrite --switch-output -a

The base directory is populated with:

# find /opt/perfdata/
/opt/perfdata/
/opt/perfdata/control <- control socket
/opt/perfdata/session-cycles <- data for session 'cycles':
/opt/perfdata/session-cycles/output <- perf record output
/opt/perfdata/session-cycles/perf.data <- perf data
/opt/perfdata/session-sched <- ditto for session 'sched'
/opt/perfdata/session-sched/output
/opt/perfdata/session-sched/perf.data

Signed-off-by: Jiri Olsa <[email protected]>
---
tools/perf/Documentation/perf-config.txt | 14 +
tools/perf/Documentation/perf-daemon.txt | 30 ++
tools/perf/builtin-daemon.c | 351 ++++++++++++++++++++++-
3 files changed, 393 insertions(+), 2 deletions(-)

diff --git a/tools/perf/Documentation/perf-config.txt b/tools/perf/Documentation/perf-config.txt
index c3ce48f1b379..153bde14bbe0 100644
--- a/tools/perf/Documentation/perf-config.txt
+++ b/tools/perf/Documentation/perf-config.txt
@@ -703,6 +703,20 @@ auxtrace.*::
If the directory does not exist or has the wrong file type,
the current directory is used.

+daemon.*::
+
+ daemon.base::
+ Base path for daemon data. All sessions data are stored under
+ this path.
+
+session-<NAME>.*::
+
+ session-<NAME>.run::
+
+ Defines new record session for daemon. The value is record's
+ command line without the 'record' keyword.
+
+
SEE ALSO
--------
linkperf:perf[1]
diff --git a/tools/perf/Documentation/perf-daemon.txt b/tools/perf/Documentation/perf-daemon.txt
index 1a4158cd973e..173b3f9f3a41 100644
--- a/tools/perf/Documentation/perf-daemon.txt
+++ b/tools/perf/Documentation/perf-daemon.txt
@@ -20,6 +20,20 @@ DESCRIPTION
This command allows to run simple daemon process that starts and
monitors configured record sessions.

+You can imagine 'perf daemon' of background process with several
+'perf record' child tasks, like:
+
+ # ps axjf
+ ...
+ 1 916507 ... perf daemon start
+ 916507 916508 ... \_ perf record --control=fifo:control,ack -m 10M -e cycles --overwrite --switch-output -a
+ 916507 916509 ... \_ perf record --control=fifo:control,ack -m 20M -e sched:* --overwrite --switch-output -a
+
+Not every 'perf record' session is suitable for running under daemon.
+User need perf session that either produces data on query, like the
+flight recorder sessions in above example or session that is configured
+to produce data periodically, like with --switch-output configuration
+for time and size.

OPTIONS
-------
@@ -43,6 +57,22 @@ START COMMAND
The start command creates the daemon process.


+CONFIG FILE
+-----------
+The daemon is configured within standard perf config file by
+following new variables:
+
+daemon.base:
+ Base path for daemon data. All sessions data are
+ stored under this path.
+
+session-<NAME>.run:
+ Defines new record session. The value is record's command
+ line without the 'record' keyword.
+
+Each perf record session is run in daemon.base/<NAME> directory.
+
+
SEE ALSO
--------
linkperf:perf-record[1], linkperf:perf-config[1]
diff --git a/tools/perf/builtin-daemon.c b/tools/perf/builtin-daemon.c
index e0880c5ee39c..15b328f6bd9a 100644
--- a/tools/perf/builtin-daemon.c
+++ b/tools/perf/builtin-daemon.c
@@ -2,6 +2,8 @@
#include <internal/lib.h>
#include <subcmd/parse-options.h>
#include <api/fd/array.h>
+#include <linux/zalloc.h>
+#include <linux/string.h>
#include <linux/limits.h>
#include <linux/string.h>
#include <string.h>
@@ -15,22 +17,74 @@
#include <sys/un.h>
#include <sys/stat.h>
#include <poll.h>
+#include <sys/stat.h>
#include "builtin.h"
#include "perf.h"
#include "debug.h"
#include "config.h"
#include "util.h"

+#define SESSION_OUTPUT "output"
+
+/*
+ * Session states:
+ *
+ * OK - session is up and running
+ * RECONFIG - session is pending for reconfiguration,
+ * new values are already loaded in session object
+ * KILL - session is pending to be killed
+ *
+ * Session object life and its state is maintained by
+ * following functions:
+ *
+ * setup_server_config
+ * - reads config file and setup session objects
+ * with following states:
+ *
+ * OK - no change needed
+ * RECONFIG - session needs to be changed
+ * (run variable changed)
+ * KILL - session needs to be killed
+ * (session is no longer in config file)
+ *
+ * daemon__reconfig
+ * - scans session objects and does following actions
+ * for states:
+ *
+ * OK - skip
+ * RECONFIG - session is killed and re-run with new config
+ * KILL - session is killed
+ *
+ * - all sessions have OK state on the function exit
+ */
+enum daemon_session_state {
+ OK,
+ RECONFIG,
+ KILL,
+};
+
+struct daemon_session {
+ char *base;
+ char *name;
+ char *run;
+ int pid;
+ struct list_head list;
+ enum daemon_session_state state;
+};
+
struct daemon {
const char *config;
char *config_real;
const char *base_user;
char *base;
+ struct list_head sessions;
FILE *out;
char perf[PATH_MAX];
};

-static struct daemon __daemon = { };
+static struct daemon __daemon = {
+ .sessions = LIST_HEAD_INIT(__daemon.sessions),
+};

static const char * const daemon_usage[] = {
"perf daemon start [<options>]",
@@ -45,6 +99,125 @@ static void sig_handler(int sig __maybe_unused)
done = true;
}

+static struct daemon_session *daemon__add_session(struct daemon *config, char *name)
+{
+ struct daemon_session *session = zalloc(sizeof(*session));
+
+ if (!session)
+ return NULL;
+
+ session->name = strdup(name);
+ if (!session->name) {
+ free(session);
+ return NULL;
+ }
+
+ session->pid = -1;
+ list_add_tail(&session->list, &config->sessions);
+ return session;
+}
+
+static struct daemon_session *daemon__find_session(struct daemon *daemon, char *name)
+{
+ struct daemon_session *session;
+
+ list_for_each_entry(session, &daemon->sessions, list) {
+ if (!strcmp(session->name, name))
+ return session;
+ }
+
+ return NULL;
+}
+
+static int get_session_name(const char *var, char *session, int len)
+{
+ const char *p = var + sizeof("session-") - 1;
+
+ while (*p != '.' && *p != 0x0 && len--)
+ *session++ = *p++;
+
+ *session = 0;
+ return *p == '.' ? 0 : -EINVAL;
+}
+
+static int session_config(struct daemon *daemon, const char *var, const char *value)
+{
+ struct daemon_session *session;
+ char name[100];
+
+ if (get_session_name(var, name, sizeof(name)))
+ return -EINVAL;
+
+ var = strchr(var, '.');
+ if (!var)
+ return -EINVAL;
+
+ var++;
+
+ session = daemon__find_session(daemon, name);
+
+ if (!session) {
+ /* New session is defined. */
+ session = daemon__add_session(daemon, name);
+ if (!session)
+ return -ENOMEM;
+
+ pr_debug("reconfig: found new session %s\n", name);
+
+ /* Trigger reconfig to start it. */
+ session->state = RECONFIG;
+ } else if (session->state == KILL) {
+ /* Current session is defined, no action needed. */
+ pr_debug("reconfig: found current session %s\n", name);
+ session->state = OK;
+ }
+
+ if (!strcmp(var, "run")) {
+ bool same = false;
+
+ if (session->run)
+ same = !strcmp(session->run, value);
+
+ if (!same) {
+ if (session->run) {
+ free(session->run);
+ pr_debug("reconfig: session %s is changed\n", name);
+ }
+
+ session->run = strdup(value);
+ if (!session->run)
+ return -ENOMEM;
+
+ /*
+ * Either new or changed run value is defined,
+ * trigger reconfig for the session.
+ */
+ session->state = RECONFIG;
+ }
+ }
+
+ return 0;
+}
+
+static int server_config(const char *var, const char *value, void *cb)
+{
+ struct daemon *daemon = cb;
+
+ if (strstarts(var, "session-")) {
+ return session_config(daemon, var, value);
+ } else if (!strcmp(var, "daemon.base") && !daemon->base_user) {
+ if (daemon->base && strcmp(daemon->base, value)) {
+ pr_err("failed: can't redefine base, bailing out\n");
+ return -EINVAL;
+ }
+ daemon->base = strdup(value);
+ if (!daemon->base)
+ return -ENOMEM;
+ }
+
+ return 0;
+}
+
static int client_config(const char *var, const char *value, void *cb)
{
struct daemon *daemon = cb;
@@ -106,6 +279,92 @@ static int setup_client_config(struct daemon *daemon)
return err ?: check_base(daemon);
}

+static int setup_server_config(struct daemon *daemon)
+{
+ struct perf_config_set *set;
+ struct daemon_session *session;
+ int err = -ENOMEM;
+
+ pr_debug("reconfig: started\n");
+
+ /*
+ * Mark all sessions for kill, the server config
+ * will set following states, see explanation at
+ * enum daemon_session_state declaration.
+ */
+ list_for_each_entry(session, &daemon->sessions, list)
+ session->state = KILL;
+
+ set = perf_config_set__load_file(daemon->config_real);
+ if (set) {
+ err = perf_config_set(set, server_config, daemon);
+ perf_config_set__delete(set);
+ }
+
+ return err ?: check_base(daemon);
+}
+
+static int daemon_session__run(struct daemon_session *session,
+ struct daemon *daemon)
+{
+ char buf[PATH_MAX];
+ char **argv;
+ int argc, fd;
+
+ if (asprintf(&session->base, "%s/session-%s",
+ daemon->base, session->name) < 0) {
+ perror("failed: asprintf");
+ return -1;
+ }
+
+ if (mkdir(session->base, 0755) && errno != EEXIST) {
+ perror("failed: mkdir");
+ return -1;
+ }
+
+ session->pid = fork();
+ if (session->pid < 0)
+ return -1;
+ if (session->pid > 0) {
+ pr_info("reconfig: ruining session [%s:%d]: %s\n",
+ session->name, session->pid, session->run);
+ return 0;
+ }
+
+ if (chdir(session->base)) {
+ perror("failed: chdir");
+ return -1;
+ }
+
+ fd = open("/dev/null", O_RDONLY);
+ if (fd < 0) {
+ perror("failed: open /dev/null");
+ return -1;
+ }
+
+ dup2(fd, 0);
+ close(fd);
+
+ fd = open(SESSION_OUTPUT, O_RDWR|O_CREAT|O_TRUNC, 0644);
+ if (fd < 0) {
+ perror("failed: open session output");
+ return -1;
+ }
+
+ dup2(fd, 1);
+ dup2(fd, 2);
+ close(fd);
+
+ scnprintf(buf, sizeof(buf), "%s record %s", daemon->perf, session->run);
+
+ argv = argv_split(buf, &argc);
+ if (!argv)
+ exit(-1);
+
+ exit(execve(daemon->perf, argv, NULL));
+ return -1;
+}
+
static int setup_server_socket(struct daemon *daemon)
{
struct sockaddr_un addr;
@@ -224,12 +483,89 @@ static int setup_client_socket(struct daemon *daemon)
return fd;
}

+static int daemon_session__signal(struct daemon_session *session, int sig)
+{
+ if (session->pid < 0)
+ return -1;
+ return kill(session->pid, sig);
+}
+
+static void daemon_session__kill(struct daemon_session *session)
+{
+ daemon_session__signal(session, SIGTERM);
+}
+
+static void daemon__signal(struct daemon *daemon, int sig)
+{
+ struct daemon_session *session;
+
+ list_for_each_entry(session, &daemon->sessions, list)
+ daemon_session__signal(session, sig);
+}
+
+static void daemon_session__delete(struct daemon_session *session)
+{
+ free(session->base);
+ free(session->name);
+ free(session->run);
+ free(session);
+}
+
+static void daemon_session__remove(struct daemon_session *session)
+{
+ list_del(&session->list);
+ daemon_session__delete(session);
+}
+
+static void daemon__kill(struct daemon *daemon)
+{
+ daemon__signal(daemon, SIGTERM);
+}
+
static void daemon__exit(struct daemon *daemon)
{
+ struct daemon_session *session, *h;
+
+ list_for_each_entry_safe(session, h, &daemon->sessions, list)
+ daemon_session__remove(session);
+
free(daemon->config_real);
free(daemon->base);
}

+static int daemon__reconfig(struct daemon *daemon)
+{
+ struct daemon_session *session, *n;
+
+ list_for_each_entry_safe(session, n, &daemon->sessions, list) {
+ /* No change. */
+ if (session->state == OK)
+ continue;
+
+ /* Remove session. */
+ if (session->state == KILL) {
+ if (session->pid > 0) {
+ daemon_session__kill(session);
+ pr_info("reconfig: session '%s' killed\n", session->name);
+ }
+ daemon_session__remove(session);
+ continue;
+ }
+
+ /* Reconfig session. */
+ if (session->pid > 0) {
+ daemon_session__kill(session);
+ pr_info("reconfig: session '%s' killed\n", session->name);
+ }
+ if (daemon_session__run(session, daemon))
+ return -1;
+
+ session->state = OK;
+ }
+
+ return 0;
+}
+
static int setup_config(struct daemon *daemon)
{
if (daemon->base_user) {
@@ -278,6 +614,9 @@ static int __cmd_start(struct daemon *daemon, struct option parent_options[],
return -1;
}

+ if (setup_server_config(daemon))
+ return -1;
+
debug_set_file(daemon->out);
debug_set_display_time(true);

@@ -297,15 +636,23 @@ static int __cmd_start(struct daemon *daemon, struct option parent_options[],
signal(SIGTERM, sig_handler);

while (!done && !err) {
- if (fdarray__poll(&fda, -1)) {
+ err = daemon__reconfig(daemon);
+
+ if (!err && fdarray__poll(&fda, -1)) {
+ bool reconfig = false;
+
if (fda.entries[sock_pos].revents & POLLIN)
err = handle_server_socket(daemon, sock_fd);
+
+ if (reconfig)
+ err = setup_server_config(daemon);
}
}

out:
fdarray__exit(&fda);

+ daemon__kill(daemon);
daemon__exit(daemon);

if (sock_fd != -1)
--
2.29.2

2021-02-08 21:17:02

by Jiri Olsa

[permalink] [raw]
Subject: [PATCH 07/24] perf daemon: Add config file change check

Adding support to detect daemon's config file changes
and re-read the configuration when that happens.

Using inotify file descriptor plugged into the main
fdarray object for polling.

Example:

# cat ~/.perfconfig
[daemon]
base=/opt/perfdata

[session-cycles]
run = -m 10M -e cycles --overwrite --switch-output -a

Starting the daemon:

# perf daemon start

Check sessions:

# perf daemon
[772262:daemon] base: /opt/perfdata
[772263:cycles] perf record -m 10M -e cycles --overwrite --switch-output -a

Change '-m 10M' to '-m 20M', and check daemon log:

# tail -f /opt/perfdata/output
[2021-01-02 20:31:41.234045] daemon started (pid 772262)
[2021-01-02 20:31:41.235072] reconfig: ruining session [cycles:772263]: -m 10M -e cycles --overwrite --switch-output -a
[2021-01-02 20:32:08.310137] reconfig: session 'cycles' killed
[2021-01-02 20:32:08.310847] reconfig: ruining session [cycles:772338]: -m 20M -e cycles --overwrite --switch-output -a

And the session list:

# perf daemon
[772262:daemon] base: /opt/perfdata
[772338:cycles] perf record -m 20M -e cycles --overwrite --switch-output -a

Note the changed '-m 20M' option is in place.

Signed-off-by: Jiri Olsa <[email protected]>
---
tools/perf/builtin-daemon.c | 100 ++++++++++++++++++++++++++++++++++--
1 file changed, 97 insertions(+), 3 deletions(-)

diff --git a/tools/perf/builtin-daemon.c b/tools/perf/builtin-daemon.c
index 15b328f6bd9a..bdddaa2ea388 100644
--- a/tools/perf/builtin-daemon.c
+++ b/tools/perf/builtin-daemon.c
@@ -12,6 +12,8 @@
#include <stdio.h>
#include <unistd.h>
#include <errno.h>
+#include <sys/inotify.h>
+#include <libgen.h>
#include <sys/types.h>
#include <sys/socket.h>
#include <sys/un.h>
@@ -75,6 +77,7 @@ struct daemon_session {
struct daemon {
const char *config;
char *config_real;
+ char *config_base;
const char *base_user;
char *base;
struct list_head sessions;
@@ -530,6 +533,7 @@ static void daemon__exit(struct daemon *daemon)
daemon_session__remove(session);

free(daemon->config_real);
+ free(daemon->config_base);
free(daemon->base);
}

@@ -566,6 +570,84 @@ static int daemon__reconfig(struct daemon *daemon)
return 0;
}

+static int setup_config_changes(struct daemon *daemon)
+{
+ char *basen = strdup(daemon->config_real);
+ char *dirn = strdup(daemon->config_real);
+ char *base, *dir;
+ int fd, wd = -1;
+
+ if (!dirn || !basen)
+ goto out;
+
+ fd = inotify_init1(IN_NONBLOCK|O_CLOEXEC);
+ if (fd < 0) {
+ perror("failed: inotify_init");
+ goto out;
+ }
+
+ dir = dirname(dirn);
+ base = basename(basen);
+ pr_debug("config file: %s, dir: %s\n", base, dir);
+
+ wd = inotify_add_watch(fd, dir, IN_CLOSE_WRITE);
+ if (wd >= 0) {
+ daemon->config_base = strdup(base);
+ if (!daemon->config_base) {
+ close(fd);
+ wd = -1;
+ }
+ } else {
+ perror("failed: inotify_add_watch");
+ }
+
+out:
+ free(basen);
+ free(dirn);
+ return wd < 0 ? -1 : fd;
+}
+
+static bool process_inotify_event(struct daemon *daemon, char *buf, ssize_t len)
+{
+ char *p = buf;
+
+ while (p < (buf + len)) {
+ struct inotify_event *event = (struct inotify_event *) p;
+
+ /*
+ * We monitor config directory, check if our
+ * config file was changes.
+ */
+ if ((event->mask & IN_CLOSE_WRITE) &&
+ !(event->mask & IN_ISDIR)) {
+ if (!strcmp(event->name, daemon->config_base))
+ return true;
+ }
+ p += sizeof(*event) + event->len;
+ }
+ return false;
+}
+
+static int handle_config_changes(struct daemon *daemon, int conf_fd,
+ bool *config_changed)
+{
+ char buf[4096];
+ ssize_t len;
+
+ while (!(*config_changed)) {
+ len = read(conf_fd, buf, sizeof(buf));
+ if (len == -1) {
+ if (errno != EAGAIN) {
+ perror("failed: read");
+ return -1;
+ }
+ return 0;
+ }
+ *config_changed = process_inotify_event(daemon, buf, len);
+ }
+ return 0;
+}
+
static int setup_config(struct daemon *daemon)
{
if (daemon->base_user) {
@@ -600,8 +682,8 @@ static int __cmd_start(struct daemon *daemon, struct option parent_options[],
OPT_PARENT(parent_options),
OPT_END()
};
- int sock_fd = -1;
- int sock_pos;
+ int sock_fd = -1, conf_fd = -1;
+ int sock_pos, file_pos;
struct fdarray fda;
int err = 0;

@@ -622,16 +704,24 @@ static int __cmd_start(struct daemon *daemon, struct option parent_options[],

pr_info("daemon started (pid %d)\n", getpid());

- fdarray__init(&fda, 1);
+ fdarray__init(&fda, 2);

sock_fd = setup_server_socket(daemon);
if (sock_fd < 0)
goto out;

+ conf_fd = setup_config_changes(daemon);
+ if (conf_fd < 0)
+ goto out;
+
sock_pos = fdarray__add(&fda, sock_fd, POLLIN|POLLERR|POLLHUP, 0);
if (sock_pos < 0)
goto out;

+ file_pos = fdarray__add(&fda, conf_fd, POLLIN|POLLERR|POLLHUP, 0);
+ if (file_pos < 0)
+ goto out;
+
signal(SIGINT, sig_handler);
signal(SIGTERM, sig_handler);

@@ -643,6 +733,8 @@ static int __cmd_start(struct daemon *daemon, struct option parent_options[],

if (fda.entries[sock_pos].revents & POLLIN)
err = handle_server_socket(daemon, sock_fd);
+ if (fda.entries[file_pos].revents & POLLIN)
+ err = handle_config_changes(daemon, conf_fd, &reconfig);

if (reconfig)
err = setup_server_config(daemon);
@@ -657,6 +749,8 @@ static int __cmd_start(struct daemon *daemon, struct option parent_options[],

if (sock_fd != -1)
close(sock_fd);
+ if (conf_fd != -1)
+ close(conf_fd);

pr_info("daemon exited\n");
fclose(daemon->out);
--
2.29.2

2021-02-08 21:17:33

by Jiri Olsa

[permalink] [raw]
Subject: [PATCH 14/24] perf daemon: Set control fifo for session

Setup control fifos for session and add --control
option to session arguments.

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

Use can list control fifos with (control and ack files):

# perf daemon -v
[776459:daemon] base: /opt/perfdata
output: /opt/perfdata/output
lock: /opt/perfdata/lock
[776460:cycles] perf record -m 20M -e cycles --overwrite --switch-output -a
base: /opt/perfdata/session-cycles
output: /opt/perfdata/session-cycles/output
control: /opt/perfdata/session-cycles/control
ack: /opt/perfdata/session-cycles/ack
[776461:sched] perf record -m 20M -e sched:* --overwrite --switch-output -a
base: /opt/perfdata/session-sched
output: /opt/perfdata/session-sched/output
control: /opt/perfdata/session-sched/control
ack: /opt/perfdata/session-sched/ack

Signed-off-by: Jiri Olsa <[email protected]>
---
tools/perf/Documentation/perf-daemon.txt | 3 +++
tools/perf/builtin-daemon.c | 26 +++++++++++++++++++++++-
2 files changed, 28 insertions(+), 1 deletion(-)

diff --git a/tools/perf/Documentation/perf-daemon.txt b/tools/perf/Documentation/perf-daemon.txt
index 3c9e265858b2..b1acd468b89c 100644
--- a/tools/perf/Documentation/perf-daemon.txt
+++ b/tools/perf/Documentation/perf-daemon.txt
@@ -37,6 +37,9 @@ flight recorder sessions in above example or session that is configured
to produce data periodically, like with --switch-output configuration
for time and size.

+Each session is started with control setup (with perf record --control
+options).
+
OPTIONS
-------
-v::
diff --git a/tools/perf/builtin-daemon.c b/tools/perf/builtin-daemon.c
index 22b2ec18b01b..7414041ae31d 100644
--- a/tools/perf/builtin-daemon.c
+++ b/tools/perf/builtin-daemon.c
@@ -32,6 +32,8 @@
#include "util.h"

#define SESSION_OUTPUT "output"
+#define SESSION_CONTROL "control"
+#define SESSION_ACK "ack"

/*
* Session states:
@@ -74,6 +76,7 @@ struct daemon_session {
char *base;
char *name;
char *run;
+ char *control;
int pid;
struct list_head list;
enum daemon_session_state state;
@@ -365,7 +368,18 @@ static int daemon_session__run(struct daemon_session *session,
dup2(fd, 2);
close(fd);

- scnprintf(buf, sizeof(buf), "%s record %s", daemon->perf, session->run);
+ if (mkfifo(SESSION_CONTROL, O_RDWR) && errno != EEXIST) {
+ perror("failed: create control fifo");
+ return -1;
+ }
+
+ if (mkfifo(SESSION_ACK, O_RDWR) && errno != EEXIST) {
+ perror("failed: create ack fifo");
+ return -1;
+ }
+
+ scnprintf(buf, sizeof(buf), "%s record --control=fifo:%s,%s %s",
+ daemon->perf, SESSION_CONTROL, SESSION_ACK, session->run);

argv = argv_split(buf, &argc);
if (!argv)
@@ -603,6 +617,12 @@ static int cmd_session_list(struct daemon *daemon, union cmd *cmd, FILE *out)
/* session output */
csv_sep, session->base, SESSION_OUTPUT);

+ fprintf(out, "%c%s/%s%c%s/%s",
+ /* session control */
+ csv_sep, session->base, SESSION_CONTROL,
+ /* session ack */
+ csv_sep, session->base, SESSION_ACK);
+
fprintf(out, "\n");
} else {
fprintf(out, "[%d:%s] perf record %s\n",
@@ -613,6 +633,10 @@ static int cmd_session_list(struct daemon *daemon, union cmd *cmd, FILE *out)
session->base);
fprintf(out, " output: %s/%s\n",
session->base, SESSION_OUTPUT);
+ fprintf(out, " control: %s/%s\n",
+ session->base, SESSION_CONTROL);
+ fprintf(out, " ack: %s/%s\n",
+ session->base, SESSION_ACK);
}
}

--
2.29.2

2021-02-08 21:18:55

by Jiri Olsa

[permalink] [raw]
Subject: [PATCH 18/24] perf daemon: Add examples to man page

Adding usage examples to the man page.

Signed-off-by: Jiri Olsa <[email protected]>
---
tools/perf/Documentation/perf-daemon.txt | 98 ++++++++++++++++++++++++
1 file changed, 98 insertions(+)

diff --git a/tools/perf/Documentation/perf-daemon.txt b/tools/perf/Documentation/perf-daemon.txt
index 90b20bea6356..f558f8e4bc9b 100644
--- a/tools/perf/Documentation/perf-daemon.txt
+++ b/tools/perf/Documentation/perf-daemon.txt
@@ -41,6 +41,10 @@ for time and size.
Each session is started with control setup (with perf record --control
options).

+Sessions are configured through config file, see CONFIG FILE section
+with EXAMPLES.
+
+
OPTIONS
-------
-v::
@@ -105,6 +109,100 @@ session-<NAME>.run:
Each perf record session is run in daemon.base/<NAME> directory.


+EXAMPLES
+--------
+Example with 2 record sessions:
+
+ # 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
+
+
+Check sessions:
+
+ # perf daemon
+ [603349:daemon] base: /opt/perfdata
+ [603350:cycles] perf record -m 10M -e cycles --overwrite --switch-output -a
+ [603351:sched] perf record -m 20M -e sched:* --overwrite --switch-output -a
+
+First line is daemon process info with configured daemon base.
+
+
+Check sessions with more info:
+
+ # perf daemon -v
+ [603349:daemon] base: /opt/perfdata
+ output: /opt/perfdata/output
+ lock: /opt/perfdata/lock
+ up: 1 minutes
+ [603350:cycles] perf record -m 10M -e cycles --overwrite --switch-output -a
+ base: /opt/perfdata/session-cycles
+ output: /opt/perfdata/session-cycles/output
+ control: /opt/perfdata/session-cycles/control
+ ack: /opt/perfdata/session-cycles/ack
+ up: 1 minutes
+ [603351:sched] perf record -m 20M -e sched:* --overwrite --switch-output -a
+ base: /opt/perfdata/session-sched
+ output: /opt/perfdata/session-sched/output
+ control: /opt/perfdata/session-sched/control
+ ack: /opt/perfdata/session-sched/ack
+ up: 1 minutes
+
+The 'base' path is daemon/session base.
+The 'lock' file is daemon's lock file guarding that no other
+daemon is running on top of the base.
+The 'output' file is perf record output for specific session.
+The 'control' and 'ack' files are perf control files.
+The 'up' number shows minutes daemon/session is running.
+
+
+Make sure control session is online:
+
+ # perf daemon ping
+ OK cycles
+ OK sched
+
+
+Send USR2 signal to session 'cycles' to generate perf.data file:
+
+ # perf daemon signal --session cycles
+ signal 12 sent to session 'cycles [603452]'
+
+ # tail -2 /opt/perfdata/session-cycles/output
+ [ perf record: dump data: Woken up 1 times ]
+ [ perf record: Dump perf.data.2020123017013149 ]
+
+
+Send USR2 signal to all sessions:
+
+ # perf daemon signal
+ signal 12 sent to session 'cycles [603452]'
+ signal 12 sent to session 'sched [603453]'
+
+ # tail -2 /opt/perfdata/session-cycles/output
+ [ perf record: dump data: Woken up 1 times ]
+ [ perf record: Dump perf.data.2020123017024689 ]
+ # tail -2 /opt/perfdata/session-sched/output
+ [ perf record: dump data: Woken up 1 times ]
+ [ perf record: Dump perf.data.2020123017024713 ]
+
+
+Stop daemon:
+
+ # perf daemon stop
+
+
SEE ALSO
--------
linkperf:perf-record[1], linkperf:perf-config[1]
--
2.29.2

2021-02-08 21:19:15

by Jiri Olsa

[permalink] [raw]
Subject: [PATCH 17/24] perf daemon: Add up time for daemon/session list

Display up time for both daemon and sessions.

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

Get the details with up time:

# perf daemon -v
[778315:daemon] base: /opt/perfdata
output: /opt/perfdata/output
lock: /opt/perfdata/lock
up: 15 minutes
[778316:cycles] perf record -m 20M -e cycles --overwrite --switch-output -a
base: /opt/perfdata/session-cycles
output: /opt/perfdata/session-cycles/output
control: /opt/perfdata/session-cycles/control
ack: /opt/perfdata/session-cycles/ack
up: 10 minutes
[778317:sched] perf record -m 20M -e sched:* --overwrite --switch-output -a
base: /opt/perfdata/session-sched
output: /opt/perfdata/session-sched/output
control: /opt/perfdata/session-sched/control
ack: /opt/perfdata/session-sched/ack
up: 2 minutes

Signed-off-by: Jiri Olsa <[email protected]>
---
tools/perf/builtin-daemon.c | 20 ++++++++++++++++++++
1 file changed, 20 insertions(+)

diff --git a/tools/perf/builtin-daemon.c b/tools/perf/builtin-daemon.c
index 11f9fac4cf12..90ef14353804 100644
--- a/tools/perf/builtin-daemon.c
+++ b/tools/perf/builtin-daemon.c
@@ -25,6 +25,7 @@
#include <sys/wait.h>
#include <poll.h>
#include <sys/stat.h>
+#include <time.h>
#include "builtin.h"
#include "perf.h"
#include "debug.h"
@@ -80,6 +81,7 @@ struct daemon_session {
int pid;
struct list_head list;
enum daemon_session_state state;
+ time_t start;
};

struct daemon {
@@ -93,6 +95,7 @@ struct daemon {
FILE *out;
char perf[PATH_MAX];
int signal_fd;
+ time_t start;
};

static struct daemon __daemon = {
@@ -335,6 +338,8 @@ static int daemon_session__run(struct daemon_session *session,
return -1;
}

+ session->start = time(NULL);
+
session->pid = fork();
if (session->pid < 0)
return -1;
@@ -666,6 +671,7 @@ static int cmd_session_list(struct daemon *daemon, union cmd *cmd, FILE *out)
{
char csv_sep = cmd->list.csv_sep;
struct daemon_session *session;
+ time_t curr = time(NULL);

if (csv_sep) {
fprintf(out, "%d%c%s%c%s%c%s/%s",
@@ -680,6 +686,10 @@ static int cmd_session_list(struct daemon *daemon, union cmd *cmd, FILE *out)
/* lock */
csv_sep, daemon->base, "lock");

+ fprintf(out, "%c%lu",
+ /* session up time */
+ csv_sep, (curr - daemon->start) / 60);
+
fprintf(out, "\n");
} else {
fprintf(out, "[%d:daemon] base: %s\n", getpid(), daemon->base);
@@ -688,6 +698,8 @@ static int cmd_session_list(struct daemon *daemon, union cmd *cmd, FILE *out)
daemon->base, SESSION_OUTPUT);
fprintf(out, " lock: %s/lock\n",
daemon->base);
+ fprintf(out, " up: %lu minutes\n",
+ (curr - daemon->start) / 60);
}
}

@@ -713,6 +725,10 @@ static int cmd_session_list(struct daemon *daemon, union cmd *cmd, FILE *out)
/* session ack */
csv_sep, session->base, SESSION_ACK);

+ fprintf(out, "%c%lu",
+ /* session up time */
+ csv_sep, (curr - session->start) / 60);
+
fprintf(out, "\n");
} else {
fprintf(out, "[%d:%s] perf record %s\n",
@@ -727,6 +743,8 @@ static int cmd_session_list(struct daemon *daemon, union cmd *cmd, FILE *out)
session->base, SESSION_CONTROL);
fprintf(out, " ack: %s/%s\n",
session->base, SESSION_ACK);
+ fprintf(out, " up: %lu minutes\n",
+ (curr - session->start) / 60);
}
}

@@ -1226,6 +1244,8 @@ static int __cmd_start(struct daemon *daemon, struct option parent_options[],
if (argc)
usage_with_options(daemon_usage, start_options);

+ daemon->start = time(NULL);
+
if (setup_config(daemon)) {
pr_err("failed: config not found\n");
return -1;
--
2.29.2

2021-02-08 21:19:58

by Jiri Olsa

[permalink] [raw]
Subject: [PATCH 22/24] perf tests: Add daemon signal command test

Adding test for perf daemon signal command. The test
sends a signal to configured sessions and verifies
the perf data files were generated accordingly.

Signed-off-by: Jiri Olsa <[email protected]>
---
tools/perf/tests/shell/daemon.sh | 40 ++++++++++++++++++++++++++++++++
1 file changed, 40 insertions(+)

diff --git a/tools/perf/tests/shell/daemon.sh b/tools/perf/tests/shell/daemon.sh
index 3b6b5aa5587c..2e414f2c9251 100755
--- a/tools/perf/tests/shell/daemon.sh
+++ b/tools/perf/tests/shell/daemon.sh
@@ -348,10 +348,50 @@ EOF
rm -f ${config}
}

+test_signal()
+{
+ echo "test daemon signal"
+
+ local config=$(mktemp /tmp/perf.daemon.config.XXX)
+ local base=$(mktemp -d /tmp/perf.daemon.base.XXX)
+
+ # prepare config
+ cat <<EOF > ${config}
+[daemon]
+base=BASE
+
+[session-test]
+run = -e cpu-clock --switch-output
+EOF
+
+ sed -i -e "s|BASE|${base}|" ${config}
+
+ # start daemon
+ daemon_start ${config} test
+
+ # send 2 signals
+ perf daemon signal --config ${config} --session test
+ perf daemon signal --config ${config}
+
+ # stop daemon
+ daemon_exit ${base} ${config}
+
+ # count is 2 perf.data for signals and 1 for perf record finished
+ count=`ls ${base}/session-test/ | grep perf.data | wc -l`
+ if [ ${count} -ne 3 ]; then
+ error=1
+ echo "FAILED: perf data no generated"
+ fi
+
+ rm -rf ${base}
+ rm -f ${config}
+}
+
error=0

test_list
test_reconfig
test_stop
+test_signal

exit ${error}
--
2.29.2

2021-02-08 21:19:58

by Jiri Olsa

[permalink] [raw]
Subject: [PATCH 15/24] perf daemon: Add ping command

Adding ping command to verify the perf record session
is up and operational.

It's used in following patches via test code to make
sure perf record is ready to receive signals.

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

Ping all sessions:

# perf daemon ping
OK cycles
OK sched

Ping specific session:

# perf daemon ping --session sched
OK sched

Signed-off-by: Jiri Olsa <[email protected]>
---
tools/perf/Documentation/perf-daemon.txt | 9 ++
tools/perf/builtin-daemon.c | 149 +++++++++++++++++++++++
2 files changed, 158 insertions(+)

diff --git a/tools/perf/Documentation/perf-daemon.txt b/tools/perf/Documentation/perf-daemon.txt
index b1acd468b89c..90b20bea6356 100644
--- a/tools/perf/Documentation/perf-daemon.txt
+++ b/tools/perf/Documentation/perf-daemon.txt
@@ -15,6 +15,7 @@ SYNOPSIS
'perf daemon start' [<options>]
'perf daemon stop' [<options>]
'perf daemon signal' [<options>]
+'perf daemon ping' [<options>]


DESCRIPTION
@@ -80,6 +81,14 @@ The signal command sends signal to configured sessions.
Send signal to specific session.


+PING COMMAND
+------------
+The ping command sends control ping to configured sessions.
+
+--session::
+ Send ping to specific session.
+
+
CONFIG FILE
-----------
The daemon is configured within standard perf config file by
diff --git a/tools/perf/builtin-daemon.c b/tools/perf/builtin-daemon.c
index 7414041ae31d..17ab8f9021ca 100644
--- a/tools/perf/builtin-daemon.c
+++ b/tools/perf/builtin-daemon.c
@@ -499,6 +499,78 @@ static int daemon__wait(struct daemon *daemon, int secs)
return 0;
}

+static int daemon_session__control(struct daemon_session *session,
+ const char *msg, bool do_ack)
+{
+ struct pollfd pollfd = { 0, };
+ char control_path[PATH_MAX];
+ char ack_path[PATH_MAX];
+ int control, ack = -1, len;
+ char buf[20];
+ int ret = -1;
+ ssize_t err;
+
+ /* open the control file */
+ scnprintf(control_path, sizeof(control_path), "%s/%s",
+ session->base, SESSION_CONTROL);
+
+ control = open(control_path, O_WRONLY|O_NONBLOCK);
+ if (!control)
+ return -1;
+
+ if (do_ack) {
+ /* open the ack file */
+ scnprintf(ack_path, sizeof(ack_path), "%s/%s",
+ session->base, SESSION_ACK);
+
+ ack = open(ack_path, O_RDONLY, O_NONBLOCK);
+ if (!ack) {
+ close(control);
+ return -1;
+ }
+ }
+
+ /* write the command */
+ len = strlen(msg);
+
+ err = writen(control, msg, len);
+ if (err != len) {
+ pr_err("failed: write to control pipe: %d (%s)\n",
+ errno, control_path);
+ goto out;
+ }
+
+ if (!do_ack)
+ goto out;
+
+ /* wait for an ack */
+ pollfd.fd = ack;
+ pollfd.events = POLLIN;
+
+ if (!poll(&pollfd, 1, 2000)) {
+ pr_err("failed: control ack timeout\n");
+ goto out;
+ }
+
+ if (!pollfd.revents & POLLIN) {
+ pr_err("failed: did not received an ack\n");
+ goto out;
+ }
+
+ err = read(ack, buf, sizeof(buf));
+ if (err > 0)
+ ret = strcmp(buf, "ack\n");
+ else
+ perror("failed: read ack %d\n");
+
+out:
+ if (ack != -1)
+ close(ack);
+
+ close(control);
+ return ret;
+}
+
static int setup_server_socket(struct daemon *daemon)
{
struct sockaddr_un addr;
@@ -549,6 +621,7 @@ enum {
CMD_LIST = 0,
CMD_SIGNAL = 1,
CMD_STOP = 2,
+ CMD_PING = 3,
CMD_MAX,
};

@@ -570,8 +643,25 @@ union cmd {
int sig;
char name[SESSION_MAX];
} signal;
+
+ /* CMD_PING */
+ struct {
+ int cmd;
+ char name[SESSION_MAX];
+ } ping;
+};
+
+enum {
+ PING_OK = 0,
+ PING_FAIL = 1,
+ PING_MAX,
};

+static int daemon_session__ping(struct daemon_session *session)
+{
+ return daemon_session__control(session, "ping", true) ? PING_FAIL : PING_OK;
+}
+
static int cmd_session_list(struct daemon *daemon, union cmd *cmd, FILE *out)
{
char csv_sep = cmd->list.csv_sep;
@@ -668,6 +758,34 @@ static int cmd_session_kill(struct daemon *daemon, union cmd *cmd, FILE *out)
return 0;
}

+static const char *ping_str[PING_MAX] = {
+ [PING_OK] = "OK",
+ [PING_FAIL] = "FAIL",
+};
+
+static int cmd_session_ping(struct daemon *daemon, union cmd *cmd, FILE *out)
+{
+ struct daemon_session *session;
+ bool all = false, found = false;
+
+ all = !strcmp(cmd->ping.name, "all");
+
+ list_for_each_entry(session, &daemon->sessions, list) {
+ if (all || !strcmp(cmd->ping.name, session->name)) {
+ int state = daemon_session__ping(session);
+
+ fprintf(out, "%-4s %s\n", ping_str[state], session->name);
+ found = true;
+ }
+ }
+
+ if (!found && !all) {
+ fprintf(out, "%-4s %s (not found)\n",
+ ping_str[PING_FAIL], cmd->ping.name);
+ }
+ return 0;
+}
+
static int handle_server_socket(struct daemon *daemon, int sock_fd)
{
int ret = -1, fd;
@@ -703,6 +821,9 @@ static int handle_server_socket(struct daemon *daemon, int sock_fd)
ret = 0;
pr_debug("perf daemon is exciting\n");
break;
+ case CMD_PING:
+ ret = cmd_session_ping(daemon, &cmd, out);
+ break;
default:
break;
}
@@ -1124,6 +1245,7 @@ static int __cmd_start(struct daemon *daemon, struct option parent_options[],

signal(SIGINT, sig_handler);
signal(SIGTERM, sig_handler);
+ signal(SIGPIPE, SIG_IGN);

while (!done && !err) {
err = daemon__reconfig(daemon);
@@ -1261,6 +1383,31 @@ static int __cmd_stop(struct daemon *daemon, struct option parent_options[],
return send_cmd(daemon, &cmd);
}

+static int __cmd_ping(struct daemon *daemon, struct option parent_options[],
+ int argc, const char **argv)
+{
+ const char *name = "all";
+ struct option ping_options[] = {
+ OPT_STRING(0, "session", &name, "session",
+ "Ping to specific session"),
+ OPT_PARENT(parent_options),
+ OPT_END()
+ };
+ union cmd cmd = { .cmd = CMD_PING, };
+
+ argc = parse_options(argc, argv, ping_options, daemon_usage, 0);
+ if (argc)
+ usage_with_options(daemon_usage, ping_options);
+
+ if (setup_config(daemon)) {
+ pr_err("failed: config not found\n");
+ return -1;
+ }
+
+ scnprintf(cmd.ping.name, sizeof(cmd.ping.name), "%s", name);
+ return send_cmd(daemon, &cmd);
+}
+
int cmd_daemon(int argc, const char **argv)
{
struct option daemon_options[] = {
@@ -1287,6 +1434,8 @@ int cmd_daemon(int argc, const char **argv)
return __cmd_signal(&__daemon, daemon_options, argc, argv);
else if (!strcmp(argv[0], "stop"))
return __cmd_stop(&__daemon, daemon_options, argc, argv);
+ else if (!strcmp(argv[0], "ping"))
+ return __cmd_ping(&__daemon, daemon_options, argc, argv);

pr_err("failed: unknown command '%s'\n", argv[0]);
return -1;
--
2.29.2

2021-02-08 21:21:32

by Jiri Olsa

[permalink] [raw]
Subject: [PATCH 16/24] perf daemon: Use control to stop session

Using 'stop' control command to stop perf record session.
If that fails, falling back to current SIGTERM/SIGKILL pair.

Signed-off-by: Jiri Olsa <[email protected]>
---
tools/perf/builtin-daemon.c | 56 ++++++++++++++++++++++++++++++-------
1 file changed, 46 insertions(+), 10 deletions(-)

diff --git a/tools/perf/builtin-daemon.c b/tools/perf/builtin-daemon.c
index 17ab8f9021ca..11f9fac4cf12 100644
--- a/tools/perf/builtin-daemon.c
+++ b/tools/perf/builtin-daemon.c
@@ -871,11 +871,25 @@ static int setup_client_socket(struct daemon *daemon)
static void daemon_session__kill(struct daemon_session *session,
struct daemon *daemon)
{
- daemon_session__signal(session, SIGTERM);
- if (daemon_session__wait(session, daemon, 10)) {
- daemon_session__signal(session, SIGKILL);
- daemon_session__wait(session, daemon, 10);
- }
+ int how = 0;
+
+ do {
+ switch (how) {
+ case 0:
+ daemon_session__control(session, "stop", false);
+ break;
+ case 1:
+ daemon_session__signal(session, SIGTERM);
+ break;
+ case 2:
+ daemon_session__signal(session, SIGKILL);
+ break;
+ default:
+ break;
+ }
+ how++;
+
+ } while (daemon_session__wait(session, daemon, 10));
}

static void daemon__signal(struct daemon *daemon, int sig)
@@ -900,13 +914,35 @@ static void daemon_session__remove(struct daemon_session *session)
daemon_session__delete(session);
}

+static void daemon__stop(struct daemon *daemon)
+{
+ struct daemon_session *session;
+
+ list_for_each_entry(session, &daemon->sessions, list)
+ daemon_session__control(session, "stop", false);
+}
+
static void daemon__kill(struct daemon *daemon)
{
- daemon__signal(daemon, SIGTERM);
- if (daemon__wait(daemon, 10)) {
- daemon__signal(daemon, SIGKILL);
- daemon__wait(daemon, 10);
- }
+ int how = 0;
+
+ do {
+ switch (how) {
+ case 0:
+ daemon__stop(daemon);
+ break;
+ case 1:
+ daemon__signal(daemon, SIGTERM);
+ break;
+ case 2:
+ daemon__signal(daemon, SIGKILL);
+ break;
+ default:
+ break;
+ }
+ how++;
+
+ } while (daemon__wait(daemon, 10));
}

static void daemon__exit(struct daemon *daemon)
--
2.29.2

2021-02-08 21:21:42

by Jiri Olsa

[permalink] [raw]
Subject: [PATCH 20/24] perf tests: Add daemon reconfig test

Adding test for daemon reconfiguration. The test changes
the configuration file and checks that the session is
changed properly.

Signed-off-by: Jiri Olsa <[email protected]>
---
tools/perf/tests/shell/daemon.sh | 119 +++++++++++++++++++++++++++++++
1 file changed, 119 insertions(+)

diff --git a/tools/perf/tests/shell/daemon.sh b/tools/perf/tests/shell/daemon.sh
index 874e1fd77c7e..90a6e8caddfb 100755
--- a/tools/perf/tests/shell/daemon.sh
+++ b/tools/perf/tests/shell/daemon.sh
@@ -177,8 +177,127 @@ EOF
rm -f ${config}
}

+test_reconfig()
+{
+ echo "test daemon reconfig"
+
+ local config=$(mktemp /tmp/perf.daemon.config.XXX)
+ local base=$(mktemp -d /tmp/perf.daemon.base.XXX)
+
+ # prepare config
+ cat <<EOF > ${config}
+[daemon]
+base=BASE
+
+[session-size]
+run = -e cpu-clock
+
+[session-time]
+run = -e task-clock
+EOF
+
+ sed -i -e "s|BASE|${base}|" ${config}
+
+ # start daemon
+ daemon_start ${config} size
+
+ # check 2nd session
+ # pid:time:-e task-clock:base/time:base/time/output:base/time/control:base/time/ack:0
+ local line=`perf daemon --config ${config} -x: | head -3 | tail -1`
+ check_line_other "${line}" time "-e task-clock" ${base}/session-time \
+ ${base}/session-time/output ${base}/session-time/control ${base}/session-time/ack "0"
+ local pid=`echo "${line}" | awk 'BEGIN { FS = ":" } ; { print $1 }'`
+
+ # prepare new config
+ local config_new=${config}.new
+ cat <<EOF > ${config_new}
+[daemon]
+base=BASE
+
+[session-size]
+run = -e cpu-clock
+
+[session-time]
+run = -e cpu-clock
+EOF
+
+ # TEST 1 - change config
+
+ sed -i -e "s|BASE|${base}|" ${config_new}
+ cp ${config_new} ${config}
+
+ # wait for old session to finish
+ tail --pid=${pid} -f /dev/null
+
+ # wait for new one to start
+ local state="FAIL"
+ while [ "${state}" != "OK" ]; do
+ state=`perf daemon ping --config ${config} --session time | awk '{ print $1 }'`
+ done
+
+ # check reconfigured 2nd session
+ # pid:time:-e task-clock:base/time:base/time/output:base/time/control:base/time/ack:0
+ local line=`perf daemon --config ${config} -x: | head -3 | tail -1`
+ check_line_other "${line}" time "-e cpu-clock" ${base}/session-time \
+ ${base}/session-time/output ${base}/session-time/control ${base}/session-time/ack "0"
+
+ # TEST 2 - empty config
+
+ local config_empty=${config}.empty
+ cat <<EOF > ${config_empty}
+[daemon]
+base=BASE
+EOF
+
+ # change config
+ sed -i -e "s|BASE|${base}|" ${config_empty}
+ cp ${config_empty} ${config}
+
+ # wait for sessions to finish
+ local state="OK"
+ while [ "${state}" != "FAIL" ]; do
+ state=`perf daemon ping --config ${config} --session time | awk '{ print $1 }'`
+ done
+
+ local state="OK"
+ while [ "${state}" != "FAIL" ]; do
+ state=`perf daemon ping --config ${config} --session size | awk '{ print $1 }'`
+ done
+
+ local one=`perf daemon --config ${config} -x: | wc -l`
+
+ if [ ${one} -ne "1" ]; then
+ echo "FAILED: wrong list output"
+ error=1
+ fi
+
+ # TEST 3 - config again
+
+ cp ${config_new} ${config}
+
+ # wait for size to start
+ local state="FAIL"
+ while [ "${state}" != "OK" ]; do
+ state=`perf daemon ping --config ${config} --session size | awk '{ print $1 }'`
+ done
+
+ # wait for time to start
+ local state="FAIL"
+ while [ "${state}" != "OK" ]; do
+ state=`perf daemon ping --config ${config} --session time | awk '{ print $1 }'`
+ done
+
+ # stop daemon
+ daemon_exit ${base} ${config}
+
+ rm -rf ${base}
+ rm -f ${config}
+ rm -f ${config_new}
+ rm -f ${config_empty}
+}
error=0

test_list
+test_reconfig

exit ${error}
--
2.29.2

2021-02-08 21:21:51

by Jiri Olsa

[permalink] [raw]
Subject: [PATCH 19/24] perf tests: Add daemon list command test

Adding test for basic perf daemon listing via the CSV
output mode (-x option).

Checking that configured sessions display expected values.

Signed-off-by: Jiri Olsa <[email protected]>
---
tools/perf/tests/shell/daemon.sh | 184 +++++++++++++++++++++++++++++++
1 file changed, 184 insertions(+)
create mode 100755 tools/perf/tests/shell/daemon.sh

diff --git a/tools/perf/tests/shell/daemon.sh b/tools/perf/tests/shell/daemon.sh
new file mode 100755
index 000000000000..874e1fd77c7e
--- /dev/null
+++ b/tools/perf/tests/shell/daemon.sh
@@ -0,0 +1,184 @@
+#!/bin/sh
+# daemon operations
+# SPDX-License-Identifier: GPL-2.0
+
+check_line_first()
+{
+ local line=$1
+ local name=$2
+ local base=$3
+ local output=$4
+ local lock=$5
+ local up=$6
+
+ local line_name=`echo "${line}" | awk 'BEGIN { FS = ":" } ; { print $2 }'`
+ local line_base=`echo "${line}" | awk 'BEGIN { FS = ":" } ; { print $3 }'`
+ local line_output=`echo "${line}" | awk 'BEGIN { FS = ":" } ; { print $4 }'`
+ local line_lock=`echo "${line}" | awk 'BEGIN { FS = ":" } ; { print $5 }'`
+ local line_up=`echo "${line}" | awk 'BEGIN { FS = ":" } ; { print $6 }'`
+
+ if [ "${name}" != "${line_name}" ]; then
+ echo "FAILED: wrong name"
+ error=1
+ fi
+
+ if [ "${base}" != "${line_base}" ]; then
+ echo "FAILED: wrong base"
+ error=1
+ fi
+
+ if [ "${output}" != "${line_output}" ]; then
+ echo "FAILED: wrong output"
+ error=1
+ fi
+
+ if [ "${lock}" != "${line_lock}" ]; then
+ echo "FAILED: wrong lock"
+ error=1
+ fi
+
+ if [ "${up}" != "${line_up}" ]; then
+ echo "FAILED: wrong up"
+ error=1
+ fi
+}
+
+check_line_other()
+{
+ local line=$1
+ local name=$2
+ local run=$3
+ local base=$4
+ local output=$5
+ local control=$6
+ local ack=$7
+ local up=$8
+
+ local line_name=`echo "${line}" | awk 'BEGIN { FS = ":" } ; { print $2 }'`
+ local line_run=`echo "${line}" | awk 'BEGIN { FS = ":" } ; { print $3 }'`
+ local line_base=`echo "${line}" | awk 'BEGIN { FS = ":" } ; { print $4 }'`
+ local line_output=`echo "${line}" | awk 'BEGIN { FS = ":" } ; { print $5 }'`
+ local line_control=`echo "${line}" | awk 'BEGIN { FS = ":" } ; { print $6 }'`
+ local line_ack=`echo "${line}" | awk 'BEGIN { FS = ":" } ; { print $7 }'`
+ local line_up=`echo "${line}" | awk 'BEGIN { FS = ":" } ; { print $8 }'`
+
+ if [ "${name}" != "${line_name}" ]; then
+ echo "FAILED: wrong name"
+ error=1
+ fi
+
+ if [ "${run}" != "${line_run}" ]; then
+ echo "FAILED: wrong run"
+ error=1
+ fi
+
+ if [ "${base}" != "${line_base}" ]; then
+ echo "FAILED: wrong base"
+ error=1
+ fi
+
+ if [ "${output}" != "${line_output}" ]; then
+ echo "FAILED: wrong output"
+ error=1
+ fi
+
+ if [ "${control}" != "${line_control}" ]; then
+ echo "FAILED: wrong control"
+ error=1
+ fi
+
+ if [ "${ack}" != "${line_ack}" ]; then
+ echo "FAILED: wrong ack"
+ error=1
+ fi
+
+ if [ "${up}" != "${line_up}" ]; then
+ echo "FAILED: wrong up"
+ error=1
+ fi
+}
+
+daemon_start()
+{
+ local config=$1
+ local session=$2
+
+ perf daemon start --config ${config}
+
+ # wait for the session to ping
+ local state="FAIL"
+ while [ "${state}" != "OK" ]; do
+ state=`perf daemon ping --config ${config} --session ${session} | awk '{ print $1 }'`
+ sleep 0.05
+ done
+}
+
+daemon_exit()
+{
+ local base=$1
+ local config=$2
+
+ local line=`perf daemon --config ${config} -x: | head -1`
+ local pid=`echo "${line}" | awk 'BEGIN { FS = ":" } ; { print $1 }'`
+
+ # stop daemon
+ perf daemon stop --config ${config}
+
+ # ... and wait for the pid to go away
+ tail --pid=${pid} -f /dev/null
+}
+
+test_list()
+{
+ echo "test daemon list"
+
+ local config=$(mktemp /tmp/perf.daemon.config.XXX)
+ local base=$(mktemp -d /tmp/perf.daemon.base.XXX)
+
+ cat <<EOF > ${config}
+[daemon]
+base=BASE
+
+[session-size]
+run = -e cpu-clock
+
+[session-time]
+run = -e task-clock
+EOF
+
+ sed -i -e "s|BASE|${base}|" ${config}
+
+ # start daemon
+ daemon_start ${config} size
+
+ # check first line
+ # pid:daemon:base:base/output:base/lock
+ local line=`perf daemon --config ${config} -x: | head -1`
+ check_line_first ${line} daemon ${base} ${base}/output ${base}/lock "0"
+
+ # check 1st session
+ # pid:size:-e cpu-clock:base/size:base/size/output:base/size/control:base/size/ack:0
+ local line=`perf daemon --config ${config} -x: | head -2 | tail -1`
+ check_line_other "${line}" size "-e cpu-clock" ${base}/session-size \
+ ${base}/session-size/output ${base}/session-size/control \
+ ${base}/session-size/ack "0"
+
+ # check 2nd session
+ # pid:time:-e task-clock:base/time:base/time/output:base/time/control:base/time/ack:0
+ local line=`perf daemon --config ${config} -x: | head -3 | tail -1`
+ check_line_other "${line}" time "-e task-clock" ${base}/session-time \
+ ${base}/session-time/output ${base}/session-time/control \
+ ${base}/session-time/ack "0"
+
+ # stop daemon
+ daemon_exit ${base} ${config}
+
+ rm -rf ${base}
+ rm -f ${config}
+}
+
+error=0
+
+test_list
+
+exit ${error}
--
2.29.2

2021-02-08 21:22:49

by Jiri Olsa

[permalink] [raw]
Subject: [PATCH 21/24] perf tests: Add daemon stop command test

Adding test for perf daemon stop command. The test
stops the daemon and verifies all the configured
sessions are properly terminated.

Signed-off-by: Jiri Olsa <[email protected]>
---
tools/perf/tests/shell/daemon.sh | 54 ++++++++++++++++++++++++++++++++
1 file changed, 54 insertions(+)

diff --git a/tools/perf/tests/shell/daemon.sh b/tools/perf/tests/shell/daemon.sh
index 90a6e8caddfb..3b6b5aa5587c 100755
--- a/tools/perf/tests/shell/daemon.sh
+++ b/tools/perf/tests/shell/daemon.sh
@@ -295,9 +295,63 @@ EOF
rm -f ${config_new}
rm -f ${config_empty}
}
+
+test_stop()
+{
+ echo "test daemon stop"
+
+ local config=$(mktemp /tmp/perf.daemon.config.XXX)
+ local base=$(mktemp -d /tmp/perf.daemon.base.XXX)
+
+ # prepare config
+ cat <<EOF > ${config}
+[daemon]
+base=BASE
+
+[session-size]
+run = -e cpu-clock
+
+[session-time]
+run = -e task-clock
+EOF
+
+ sed -i -e "s|BASE|${base}|" ${config}
+
+ # start daemon
+ daemon_start ${config} size
+
+ local pid_size=`perf daemon --config ${config} -x: | head -2 | tail -1 | awk 'BEGIN { FS = ":" } ; { print $1 }'`
+ local pid_time=`perf daemon --config ${config} -x: | head -3 | tail -1 | awk 'BEGIN { FS = ":" } ; { print $1 }'`
+
+ # check that sessions are running
+ if [ ! -d "/proc/${pid_size}" ]; then
+ echo "FAILED: session size not up"
+ fi
+
+ if [ ! -d "/proc/${pid_time}" ]; then
+ echo "FAILED: session time not up"
+ fi
+
+ # stop daemon
+ daemon_exit ${base} ${config}
+
+ # check that sessions are gone
+ if [ -d "/proc/${pid_size}" ]; then
+ echo "FAILED: session size still up"
+ fi
+
+ if [ -d "/proc/${pid_time}" ]; then
+ echo "FAILED: session time still up"
+ fi
+
+ rm -rf ${base}
+ rm -f ${config}
+}
+
error=0

test_list
test_reconfig
+test_stop

exit ${error}
--
2.29.2

2021-02-08 21:23:40

by Jiri Olsa

[permalink] [raw]
Subject: [PATCH 23/24] perf tests: Add daemon ping command test

Adding test for perf daemon ping command. The tests
verifies the ping command gets proper answer from
sessions.

Signed-off-by: Jiri Olsa <[email protected]>
---
tools/perf/tests/shell/daemon.sh | 40 ++++++++++++++++++++++++++++++++
1 file changed, 40 insertions(+)

diff --git a/tools/perf/tests/shell/daemon.sh b/tools/perf/tests/shell/daemon.sh
index 2e414f2c9251..9fcb98768633 100755
--- a/tools/perf/tests/shell/daemon.sh
+++ b/tools/perf/tests/shell/daemon.sh
@@ -387,11 +387,51 @@ EOF
rm -f ${config}
}

+test_ping()
+{
+ echo "test daemon ping"
+
+ local config=$(mktemp /tmp/perf.daemon.config.XXX)
+ local base=$(mktemp -d /tmp/perf.daemon.base.XXX)
+
+ # prepare config
+ cat <<EOF > ${config}
+[daemon]
+base=BASE
+
+[session-size]
+run = -e cpu-clock
+
+[session-time]
+run = -e task-clock
+EOF
+
+ sed -i -e "s|BASE|${base}|" ${config}
+
+ # start daemon
+ daemon_start ${config} size
+
+ size=`perf daemon ping --config ${config} --session size | awk '{ print $1 }'`
+ type=`perf daemon ping --config ${config} --session time | awk '{ print $1 }'`
+
+ if [ ${size} != "OK" -o ${type} != "OK" ]; then
+ error=1
+ echo "FAILED: daemon ping failed"
+ fi
+
+ # stop daemon
+ daemon_exit ${base} ${config}
+
+ rm -rf ${base}
+ rm -f ${config}
+}
+
error=0

test_list
test_reconfig
test_stop
test_signal
+test_ping

exit ${error}
--
2.29.2

2021-02-08 21:23:53

by Jiri Olsa

[permalink] [raw]
Subject: [PATCH 24/24] perf tests: Add daemon lock test

Adding test for perf daemon lock ensuring only
one instance of daemon can run over one base
directory.

Signed-off-by: Jiri Olsa <[email protected]>
---
tools/perf/tests/shell/daemon.sh | 38 ++++++++++++++++++++++++++++++++
1 file changed, 38 insertions(+)

diff --git a/tools/perf/tests/shell/daemon.sh b/tools/perf/tests/shell/daemon.sh
index 9fcb98768633..e5b824dd08d9 100755
--- a/tools/perf/tests/shell/daemon.sh
+++ b/tools/perf/tests/shell/daemon.sh
@@ -426,6 +426,43 @@ EOF
rm -f ${config}
}

+test_lock()
+{
+ echo "test daemon lock"
+
+ local config=$(mktemp /tmp/perf.daemon.config.XXX)
+ local base=$(mktemp -d /tmp/perf.daemon.base.XXX)
+
+ # prepare config
+ cat <<EOF > ${config}
+[daemon]
+base=BASE
+
+[session-size]
+run = -e cpu-clock
+EOF
+
+ sed -i -e "s|BASE|${base}|" ${config}
+
+ # start daemon
+ daemon_start ${config} size
+
+ # start second daemon over the same config/base
+ failed=`perf daemon start --config ${config} 2>&1 | awk '{ print $1 }'`
+
+ # check that we failed properly
+ if [ ${failed} != "failed:" ]; then
+ error=1
+ echo "FAILED: daemon lock failed"
+ fi
+
+ # stop daemon
+ daemon_exit ${base} ${config}
+
+ rm -rf ${base}
+ rm -f ${config}
+}
+
error=0

test_list
@@ -433,5 +470,6 @@ test_reconfig
test_stop
test_signal
test_ping
+test_lock

exit ${error}
--
2.29.2

2021-02-10 12:55:34

by Arnaldo Carvalho de Melo

[permalink] [raw]
Subject: Re: [PATCH 15/24] perf daemon: Add ping command

Em Mon, Feb 08, 2021 at 09:08:59PM +0100, Jiri Olsa escreveu:
> +
> + if (!pollfd.revents & POLLIN) {
> + pr_err("failed: did not received an ack\n");
> + goto out;
> + }
> +

Fixed up this, pointed out by clang on many build containers, including
fedora:34:

Committer notes:

Fixed up bug pointed by clang:

Buggy:

if (!pollfd.revents & POLLIN)

Correct code:

if (!(pollfd.revents & POLLIN))

clang warning:

builtin-daemon.c:560:6: error: logical not is only applied to the left hand side of this bitwise operator [-Werror,-Wlogical-not-parentheses]
if (!pollfd.revents & POLLIN) {
^ ~
builtin-daemon.c:560:6: note: add parentheses after the '!' to evaluate the bitwise operator first


- Arnaldo

2021-02-10 13:31:19

by Jiri Olsa

[permalink] [raw]
Subject: Re: [PATCH 15/24] perf daemon: Add ping command

On Wed, Feb 10, 2021 at 09:51:46AM -0300, Arnaldo Carvalho de Melo wrote:
> Em Mon, Feb 08, 2021 at 09:08:59PM +0100, Jiri Olsa escreveu:
> > +
> > + if (!pollfd.revents & POLLIN) {
> > + pr_err("failed: did not received an ack\n");
> > + goto out;
> > + }
> > +
>
> Fixed up this, pointed out by clang on many build containers, including
> fedora:34:
>
> Committer notes:
>
> Fixed up bug pointed by clang:
>
> Buggy:
>
> if (!pollfd.revents & POLLIN)
>
> Correct code:
>
> if (!(pollfd.revents & POLLIN))
>
> clang warning:
>
> builtin-daemon.c:560:6: error: logical not is only applied to the left hand side of this bitwise operator [-Werror,-Wlogical-not-parentheses]
> if (!pollfd.revents & POLLIN) {
> ^ ~
> builtin-daemon.c:560:6: note: add parentheses after the '!' to evaluate the bitwise operator first

oops, thanks

jirka

2021-02-11 06:03:58

by Namhyung Kim

[permalink] [raw]
Subject: Re: [PATCH 06/24] perf daemon: Add config file support

Hi Jiri,

On Tue, Feb 9, 2021 at 5:09 AM Jiri Olsa <[email protected]> wrote:
> +static int daemon__reconfig(struct daemon *daemon)
> +{
> + struct daemon_session *session, *n;
> +
> + list_for_each_entry_safe(session, n, &daemon->sessions, list) {
> + /* No change. */
> + if (session->state == OK)
> + continue;
> +
> + /* Remove session. */
> + if (session->state == KILL) {
> + if (session->pid > 0) {
> + daemon_session__kill(session);
> + pr_info("reconfig: session '%s' killed\n", session->name);
> + }
> + daemon_session__remove(session);
> + continue;
> + }
> +
> + /* Reconfig session. */
> + if (session->pid > 0) {
> + daemon_session__kill(session);
> + pr_info("reconfig: session '%s' killed\n", session->name);
> + }
> + if (daemon_session__run(session, daemon))
> + return -1;

Shouldn't it be 'continue'? If there's a problematic session
it'll prevent others from being processed. And it seems this
code will try to run it again and again. Maybe we can put it
in the KILL state (or a new FAILED state) IMHO.

Thanks,
Namhyung

> +
> + session->state = OK;
> + }
> +
> + return 0;
> +}
> +
> static int setup_config(struct daemon *daemon)
> {
> if (daemon->base_user) {
> @@ -278,6 +614,9 @@ static int __cmd_start(struct daemon *daemon, struct option parent_options[],
> return -1;
> }
>
> + if (setup_server_config(daemon))
> + return -1;
> +
> debug_set_file(daemon->out);
> debug_set_display_time(true);
>
> @@ -297,15 +636,23 @@ static int __cmd_start(struct daemon *daemon, struct option parent_options[],
> signal(SIGTERM, sig_handler);
>
> while (!done && !err) {
> - if (fdarray__poll(&fda, -1)) {
> + err = daemon__reconfig(daemon);
> +
> + if (!err && fdarray__poll(&fda, -1)) {
> + bool reconfig = false;
> +
> if (fda.entries[sock_pos].revents & POLLIN)
> err = handle_server_socket(daemon, sock_fd);
> +
> + if (reconfig)
> + err = setup_server_config(daemon);
> }
> }
>
> out:
> fdarray__exit(&fda);
>
> + daemon__kill(daemon);
> daemon__exit(daemon);
>
> if (sock_fd != -1)
> --
> 2.29.2
>

2021-02-11 13:13:52

by Arnaldo Carvalho de Melo

[permalink] [raw]
Subject: Re: [PATCH 05/24] perf daemon: Add client socket support

Em Mon, Feb 08, 2021 at 09:08:49PM +0100, Jiri Olsa escreveu:
> +__maybe_unused
> +static int send_cmd(struct daemon *daemon, union cmd *cmd)
> +{
> + int ret = -1, fd;
> + char *line = NULL;
> + size_t len = 0;
> + ssize_t nread;
> + FILE *in = NULL;
> +
> + if (setup_client_config(daemon))
> + return -1;
> +
> + fd = setup_client_socket(daemon);
> + if (fd < 0)
> + return -1;
> +
> + if (sizeof(*cmd) != writen(fd, cmd, sizeof(*cmd))) {
> + perror("failed: write");
> + goto out;
> + }
> +
> + in = fdopen(fd, "r");
> + if (!in) {
> + perror("failed: fdopen");
> + goto out;
> + }
> +
> + while ((nread = getline(&line, &len, in)) != -1) {
> + fwrite(line, nread, 1, stdout);
> + fflush(stdout);
> + }
> +
> + ret = 0;
> + fclose(in);
> +out:
> + /* If in is defined, then fd is closed via fclose. */
> + if (!in)
> + close(fd);
> + return ret;
> +}
> +

So I had to add the patch below to deal with this in some systems:

cc1: warnings being treated as errors
builtin-daemon.c: In function 'send_cmd': MKDIR /tmp/build/perf/bench/

builtin-daemon.c:1368: error: ignoring return value of 'fwrite', declared with attribute warn_unused_result
MKDIR /tmp/build/perf/tests/
make[3]: *** [/tmp/build/perf/builtin-daemon.o] Error 1

And also to not leak the 'line' buffer allocated by getline(), since you
initialized line to NULL and len to zero, man page says:

If *lineptr is set to NULL and *n is set 0 before the call,
then getline() will allocate a buffer for storing the line.
This buffer should be freed by the user program even if
getline() failed.


diff --git a/tools/perf/builtin-daemon.c b/tools/perf/builtin-daemon.c
index e0880c5ee39c89bd..0a282c4e23a9fd9a 100644
--- a/tools/perf/builtin-daemon.c
+++ b/tools/perf/builtin-daemon.c
@@ -344,12 +344,15 @@ static int send_cmd(struct daemon *daemon, union cmd *cmd)
}

while ((nread = getline(&line, &len, in)) != -1) {
- fwrite(line, nread, 1, stdout);
+ if (fwrite(line, nread, 1, stdout) != 1)
+ goto out_fclose;
fflush(stdout);
}

ret = 0;
+out_fclose:
fclose(in);
+ free(line);
out:
/* If in is defined, then fd is closed via fclose. */
if (!in)

2021-02-11 13:42:05

by Arnaldo Carvalho de Melo

[permalink] [raw]
Subject: Re: [PATCH 13/24] perf daemon: Allow only one daemon over base directory

Em Mon, Feb 08, 2021 at 09:08:57PM +0100, Jiri Olsa escreveu:
> Add 'lock' file under daemon base and flock it, so only one
> perf daemon can run on top of it.
>
> Each daemon tries to create and lock BASE/lock file, if it's
> successful we are sure we're the only daemon running over
> the BASE.
>
> Once daemon is finished, file descriptor to lock file is
> closed and lock is released.
>
> 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.

Had to add this:

Committer notes:

Provide lockf(F_TLOCK) when not available, i.e. transform:

lockf(fd, F_TLOCK, 0);

into:

flock(fd, LOCK_EX | LOCK_NB);

Which should be equivalent.

Noticed when cross building to some odd Android NDK.

------

Patch:

[acme@five perf]$ git diff
diff --git a/tools/perf/builtin-daemon.c b/tools/perf/builtin-daemon.c
index 1c17c9e10ca6a656..2890573540f7d027 100644
--- a/tools/perf/builtin-daemon.c
+++ b/tools/perf/builtin-daemon.c
@@ -914,6 +914,20 @@ static int setup_config(struct daemon *daemon)
return daemon->config_real ? 0 : -1;
}

+#ifndef F_TLOCK
+#define F_TLOCK 2
+
+#include <sys/file.h>
+
+static int lockf(int fd, int cmd, off_t len)
+{
+ if (cmd != F_TLOCK || len != 0)
+ return -1;
+
+ return flock(fd, LOCK_EX | LOCK_NB);
+}
+#endif // F_TLOCK
+
/*
* Each daemon tries to create and lock BASE/lock file,
* if it's successful we are sure we're the only daemon
[acme@five perf]$

2021-02-11 17:38:48

by Jiri Olsa

[permalink] [raw]
Subject: Re: [PATCH 13/24] perf daemon: Allow only one daemon over base directory

On Thu, Feb 11, 2021 at 10:20:18AM -0300, Arnaldo Carvalho de Melo wrote:
> Em Mon, Feb 08, 2021 at 09:08:57PM +0100, Jiri Olsa escreveu:
> > Add 'lock' file under daemon base and flock it, so only one
> > perf daemon can run on top of it.
> >
> > Each daemon tries to create and lock BASE/lock file, if it's
> > successful we are sure we're the only daemon running over
> > the BASE.
> >
> > Once daemon is finished, file descriptor to lock file is
> > closed and lock is released.
> >
> > 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.
>
> Had to add this:
>
> Committer notes:
>
> Provide lockf(F_TLOCK) when not available, i.e. transform:
>
> lockf(fd, F_TLOCK, 0);
>
> into:
>
> flock(fd, LOCK_EX | LOCK_NB);
>
> Which should be equivalent.
>
> Noticed when cross building to some odd Android NDK.
>
> ------
>
> Patch:
>
> [acme@five perf]$ git diff
> diff --git a/tools/perf/builtin-daemon.c b/tools/perf/builtin-daemon.c
> index 1c17c9e10ca6a656..2890573540f7d027 100644
> --- a/tools/perf/builtin-daemon.c
> +++ b/tools/perf/builtin-daemon.c
> @@ -914,6 +914,20 @@ static int setup_config(struct daemon *daemon)
> return daemon->config_real ? 0 : -1;
> }
>
> +#ifndef F_TLOCK
> +#define F_TLOCK 2
> +
> +#include <sys/file.h>
> +
> +static int lockf(int fd, int cmd, off_t len)
> +{
> + if (cmd != F_TLOCK || len != 0)
> + return -1;
> +
> + return flock(fd, LOCK_EX | LOCK_NB);
> +}
> +#endif // F_TLOCK
> +
> /*
> * Each daemon tries to create and lock BASE/lock file,
> * if it's successful we are sure we're the only daemon
> [acme@five perf]$
>

nice, thanks

jirka

2021-02-11 17:40:52

by Jiri Olsa

[permalink] [raw]
Subject: Re: [PATCH 05/24] perf daemon: Add client socket support

On Thu, Feb 11, 2021 at 09:52:05AM -0300, Arnaldo Carvalho de Melo wrote:
> Em Mon, Feb 08, 2021 at 09:08:49PM +0100, Jiri Olsa escreveu:
> > +__maybe_unused
> > +static int send_cmd(struct daemon *daemon, union cmd *cmd)
> > +{
> > + int ret = -1, fd;
> > + char *line = NULL;
> > + size_t len = 0;
> > + ssize_t nread;
> > + FILE *in = NULL;
> > +
> > + if (setup_client_config(daemon))
> > + return -1;
> > +
> > + fd = setup_client_socket(daemon);
> > + if (fd < 0)
> > + return -1;
> > +
> > + if (sizeof(*cmd) != writen(fd, cmd, sizeof(*cmd))) {
> > + perror("failed: write");
> > + goto out;
> > + }
> > +
> > + in = fdopen(fd, "r");
> > + if (!in) {
> > + perror("failed: fdopen");
> > + goto out;
> > + }
> > +
> > + while ((nread = getline(&line, &len, in)) != -1) {
> > + fwrite(line, nread, 1, stdout);
> > + fflush(stdout);
> > + }
> > +
> > + ret = 0;
> > + fclose(in);
> > +out:
> > + /* If in is defined, then fd is closed via fclose. */
> > + if (!in)
> > + close(fd);
> > + return ret;
> > +}
> > +
>
> So I had to add the patch below to deal with this in some systems:
>
> cc1: warnings being treated as errors
> builtin-daemon.c: In function 'send_cmd': MKDIR /tmp/build/perf/bench/
>
> builtin-daemon.c:1368: error: ignoring return value of 'fwrite', declared with attribute warn_unused_result
> MKDIR /tmp/build/perf/tests/
> make[3]: *** [/tmp/build/perf/builtin-daemon.o] Error 1
>
> And also to not leak the 'line' buffer allocated by getline(), since you
> initialized line to NULL and len to zero, man page says:
>
> If *lineptr is set to NULL and *n is set 0 before the call,
> then getline() will allocate a buffer for storing the line.
> This buffer should be freed by the user program even if
> getline() failed.
>
>
> diff --git a/tools/perf/builtin-daemon.c b/tools/perf/builtin-daemon.c
> index e0880c5ee39c89bd..0a282c4e23a9fd9a 100644
> --- a/tools/perf/builtin-daemon.c
> +++ b/tools/perf/builtin-daemon.c
> @@ -344,12 +344,15 @@ static int send_cmd(struct daemon *daemon, union cmd *cmd)
> }
>
> while ((nread = getline(&line, &len, in)) != -1) {
> - fwrite(line, nread, 1, stdout);
> + if (fwrite(line, nread, 1, stdout) != 1)
> + goto out_fclose;
> fflush(stdout);
> }
>
> ret = 0;
> +out_fclose:
> fclose(in);
> + free(line);
> out:
> /* If in is defined, then fd is closed via fclose. */
> if (!in)
>

ok, thanks

jirka

2021-02-11 17:45:43

by Jiri Olsa

[permalink] [raw]
Subject: Re: [PATCH 06/24] perf daemon: Add config file support

On Thu, Feb 11, 2021 at 03:01:12PM +0900, Namhyung Kim wrote:
> Hi Jiri,
>
> On Tue, Feb 9, 2021 at 5:09 AM Jiri Olsa <[email protected]> wrote:
> > +static int daemon__reconfig(struct daemon *daemon)
> > +{
> > + struct daemon_session *session, *n;
> > +
> > + list_for_each_entry_safe(session, n, &daemon->sessions, list) {
> > + /* No change. */
> > + if (session->state == OK)
> > + continue;
> > +
> > + /* Remove session. */
> > + if (session->state == KILL) {
> > + if (session->pid > 0) {
> > + daemon_session__kill(session);
> > + pr_info("reconfig: session '%s' killed\n", session->name);
> > + }
> > + daemon_session__remove(session);
> > + continue;
> > + }
> > +
> > + /* Reconfig session. */
> > + if (session->pid > 0) {
> > + daemon_session__kill(session);
> > + pr_info("reconfig: session '%s' killed\n", session->name);
> > + }
> > + if (daemon_session__run(session, daemon))
> > + return -1;
>
> Shouldn't it be 'continue'? If there's a problematic session
> it'll prevent others from being processed. And it seems this
> code will try to run it again and again. Maybe we can put it
> in the KILL state (or a new FAILED state) IMHO.

so if there is misconfigured session, it will get executed,
and then we catch that it exited, like:

# ./perf daemon start -f
[2021-02-11 17:38:19.718166] daemon started (pid 1167439)
[2021-02-11 17:38:19.719757] reconfig: ruining session [cycles:1167440]: abc -m 10M -e cycles --overwrite --switch-output -a
[2021-02-11 17:38:19.720861] reconfig: ruining session [sched:1167441]: -m 20M -e sched:* --overwrite --switch-output -a
[2021-02-11 17:38:21.132511] session 'cycles' exited, status=255

session will be removed

when the config is fixed and updated, daemon will pick it up:

[2021-02-11 17:42:59.241140] reconfig: ruining session [cycles:1167642]: -m 10M -e cycles --overwrite --switch-output -a


daemon_session__run fails only there's no memory for allocation,
if mkdir fails (for other error than EEXIST) and if fork fails,
so pretty serious errors, where we want to bail out anyway

jirka

2021-02-11 17:50:16

by Arnaldo Carvalho de Melo

[permalink] [raw]
Subject: Re: [PATCH 06/24] perf daemon: Add config file support

Em Thu, Feb 11, 2021 at 05:45:38PM +0100, Jiri Olsa escreveu:
> On Thu, Feb 11, 2021 at 03:01:12PM +0900, Namhyung Kim wrote:
> > Hi Jiri,
> >
> > On Tue, Feb 9, 2021 at 5:09 AM Jiri Olsa <[email protected]> wrote:
> > > +static int daemon__reconfig(struct daemon *daemon)
> > > +{
> > > + struct daemon_session *session, *n;
> > > +
> > > + list_for_each_entry_safe(session, n, &daemon->sessions, list) {
> > > + /* No change. */
> > > + if (session->state == OK)
> > > + continue;
> > > +
> > > + /* Remove session. */
> > > + if (session->state == KILL) {
> > > + if (session->pid > 0) {
> > > + daemon_session__kill(session);
> > > + pr_info("reconfig: session '%s' killed\n", session->name);
> > > + }
> > > + daemon_session__remove(session);
> > > + continue;
> > > + }
> > > +
> > > + /* Reconfig session. */
> > > + if (session->pid > 0) {
> > > + daemon_session__kill(session);
> > > + pr_info("reconfig: session '%s' killed\n", session->name);
> > > + }
> > > + if (daemon_session__run(session, daemon))
> > > + return -1;
> >
> > Shouldn't it be 'continue'? If there's a problematic session
> > it'll prevent others from being processed. And it seems this
> > code will try to run it again and again. Maybe we can put it
> > in the KILL state (or a new FAILED state) IMHO.
>
> so if there is misconfigured session, it will get executed,
> and then we catch that it exited, like:
>
> # ./perf daemon start -f
> [2021-02-11 17:38:19.718166] daemon started (pid 1167439)
> [2021-02-11 17:38:19.719757] reconfig: ruining session [cycles:1167440]: abc -m 10M -e cycles --overwrite --switch-output -a
> [2021-02-11 17:38:19.720861] reconfig: ruining session [sched:1167441]: -m 20M -e sched:* --overwrite --switch-output -a
> [2021-02-11 17:38:21.132511] session 'cycles' exited, status=255
>
> session will be removed
>
> when the config is fixed and updated, daemon will pick it up:
>
> [2021-02-11 17:42:59.241140] reconfig: ruining session [cycles:1167642]: -m 10M -e cycles --overwrite --switch-output -a
>
>
> daemon_session__run fails only there's no memory for allocation,
> if mkdir fails (for other error than EEXIST) and if fork fails,
> so pretty serious errors, where we want to bail out anyway

I know you love documentation, that is why I think that it would be a
good idea to have these questions and answers in a FAQ for 'perf
daemon', don't you think? ;-)

- Arnaldo

2021-02-11 22:54:56

by Jiri Olsa

[permalink] [raw]
Subject: Re: [PATCH 06/24] perf daemon: Add config file support

On Thu, Feb 11, 2021 at 02:10:48PM -0300, Arnaldo Carvalho de Melo wrote:
> Em Thu, Feb 11, 2021 at 05:45:38PM +0100, Jiri Olsa escreveu:
> > On Thu, Feb 11, 2021 at 03:01:12PM +0900, Namhyung Kim wrote:
> > > Hi Jiri,
> > >
> > > On Tue, Feb 9, 2021 at 5:09 AM Jiri Olsa <[email protected]> wrote:
> > > > +static int daemon__reconfig(struct daemon *daemon)
> > > > +{
> > > > + struct daemon_session *session, *n;
> > > > +
> > > > + list_for_each_entry_safe(session, n, &daemon->sessions, list) {
> > > > + /* No change. */
> > > > + if (session->state == OK)
> > > > + continue;
> > > > +
> > > > + /* Remove session. */
> > > > + if (session->state == KILL) {
> > > > + if (session->pid > 0) {
> > > > + daemon_session__kill(session);
> > > > + pr_info("reconfig: session '%s' killed\n", session->name);
> > > > + }
> > > > + daemon_session__remove(session);
> > > > + continue;
> > > > + }
> > > > +
> > > > + /* Reconfig session. */
> > > > + if (session->pid > 0) {
> > > > + daemon_session__kill(session);
> > > > + pr_info("reconfig: session '%s' killed\n", session->name);
> > > > + }
> > > > + if (daemon_session__run(session, daemon))
> > > > + return -1;
> > >
> > > Shouldn't it be 'continue'? If there's a problematic session
> > > it'll prevent others from being processed. And it seems this
> > > code will try to run it again and again. Maybe we can put it
> > > in the KILL state (or a new FAILED state) IMHO.
> >
> > so if there is misconfigured session, it will get executed,
> > and then we catch that it exited, like:
> >
> > # ./perf daemon start -f
> > [2021-02-11 17:38:19.718166] daemon started (pid 1167439)
> > [2021-02-11 17:38:19.719757] reconfig: ruining session [cycles:1167440]: abc -m 10M -e cycles --overwrite --switch-output -a
> > [2021-02-11 17:38:19.720861] reconfig: ruining session [sched:1167441]: -m 20M -e sched:* --overwrite --switch-output -a
> > [2021-02-11 17:38:21.132511] session 'cycles' exited, status=255
> >
> > session will be removed
> >
> > when the config is fixed and updated, daemon will pick it up:
> >
> > [2021-02-11 17:42:59.241140] reconfig: ruining session [cycles:1167642]: -m 10M -e cycles --overwrite --switch-output -a
> >
> >
> > daemon_session__run fails only there's no memory for allocation,
> > if mkdir fails (for other error than EEXIST) and if fork fails,
> > so pretty serious errors, where we want to bail out anyway
>
> I know you love documentation, that is why I think that it would be a
> good idea to have these questions and answers in a FAQ for 'perf
> daemon', don't you think? ;-)

heh, sure.. I can add something like that as section to perf-daemon man page

jirka