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

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

[Jiri Olsa]

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

[PATCH 01/24] perf daemon: Add daemon command

[Jiri Olsa]

Adding daemon skeleton with minimal base (non) functionality,
covering various setup in start command.

Adding perf-daemon.txt with basic info.

Signed-off-by: Jiri Olsa <jolsa@kernel.org>
---
 tools/perf/Build                         |  1 +
 tools/perf/Documentation/perf-daemon.txt | 40 +++++++++++
 tools/perf/builtin-daemon.c              | 86 ++++++++++++++++++++++++
 tools/perf/builtin.h                     |  1 +
 tools/perf/command-list.txt              |  1 +
 tools/perf/perf.c                        |  1 +
 6 files changed, 130 insertions(+)
 create mode 100644 tools/perf/Documentation/perf-daemon.txt
 create mode 100644 tools/perf/builtin-daemon.c

diff --git a/tools/perf/Build b/tools/perf/Build
index 5f392dbb88fc..db61dbe2b543 100644
--- a/tools/perf/Build
+++ b/tools/perf/Build
@@ -24,6 +24,7 @@ perf-y += builtin-mem.o
 perf-y += builtin-data.o
 perf-y += builtin-version.o
 perf-y += builtin-c2c.o
+perf-y += builtin-daemon.o
 
 perf-$(CONFIG_TRACE) += builtin-trace.o
 perf-$(CONFIG_LIBELF) += builtin-probe.o
diff --git a/tools/perf/Documentation/perf-daemon.txt b/tools/perf/Documentation/perf-daemon.txt
new file mode 100644
index 000000000000..d05b8dab0a6a
--- /dev/null
+++ b/tools/perf/Documentation/perf-daemon.txt
@@ -0,0 +1,40 @@
+perf-daemon(1)
+==============
+
+
+NAME
+----
+perf-daemon - Run record sessions on background
+
+
+SYNOPSIS
+--------
+[verse]
+'perf daemon'
+'perf daemon' [<options>]
+'perf daemon start'  [<options>]
+
+
+DESCRIPTION
+-----------
+This command allows to run simple daemon process that starts and
+monitors configured record sessions.
+
+
+OPTIONS
+-------
+-v::
+--verbose::
+	Be more verbose.
+
+All generic options are available also under commands.
+
+
+START COMMAND
+-------------
+The start command creates the daemon process.
+
+
+SEE ALSO
+--------
+linkperf:perf-record[1], linkperf:perf-config[1]
diff --git a/tools/perf/builtin-daemon.c b/tools/perf/builtin-daemon.c
new file mode 100644
index 000000000000..8b13e455ac40
--- /dev/null
+++ b/tools/perf/builtin-daemon.c
@@ -0,0 +1,86 @@
+// SPDX-License-Identifier: GPL-2.0
+#include <subcmd/parse-options.h>
+#include <linux/limits.h>
+#include <string.h>
+#include <signal.h>
+#include <stdio.h>
+#include <unistd.h>
+#include "builtin.h"
+#include "perf.h"
+#include "debug.h"
+#include "util.h"
+
+struct daemon {
+	char			*base;
+	FILE			*out;
+	char			 perf[PATH_MAX];
+};
+
+static struct daemon __daemon = { };
+
+static const char * const daemon_usage[] = {
+	"perf daemon start [<options>]",
+	"perf daemon [<options>]",
+	NULL
+};
+
+static bool done;
+
+static void sig_handler(int sig __maybe_unused)
+{
+	done = true;
+}
+
+static int __cmd_start(struct daemon *daemon, struct option parent_options[],
+		       int argc, const char **argv)
+{
+	struct option start_options[] = {
+		OPT_PARENT(parent_options),
+		OPT_END()
+	};
+	int err = 0;
+
+	argc = parse_options(argc, argv, start_options, daemon_usage, 0);
+	if (argc)
+		usage_with_options(daemon_usage, start_options);
+
+	debug_set_file(daemon->out);
+	debug_set_display_time(true);
+
+	pr_info("daemon started (pid %d)\n", getpid());
+
+	signal(SIGINT, sig_handler);
+	signal(SIGTERM, sig_handler);
+
+	while (!done && !err) {
+		sleep(1);
+	}
+
+	pr_info("daemon exited\n");
+	fclose(daemon->out);
+	return err;
+}
+
+int cmd_daemon(int argc, const char **argv)
+{
+	struct option daemon_options[] = {
+		OPT_INCR('v', "verbose", &verbose, "be more verbose"),
+		OPT_END()
+	};
+
+	perf_exe(__daemon.perf, sizeof(__daemon.perf));
+	__daemon.out = stdout;
+
+	argc = parse_options(argc, argv, daemon_options, daemon_usage,
+			     PARSE_OPT_STOP_AT_NON_OPTION);
+
+	if (argc) {
+		if (!strcmp(argv[0], "start"))
+			return __cmd_start(&__daemon, daemon_options, argc, argv);
+
+		pr_err("failed: unknown command '%s'\n", argv[0]);
+		return -1;
+	}
+
+	return -1;
+}
diff --git a/tools/perf/builtin.h b/tools/perf/builtin.h
index 14a2db622a7b..7303e80a639c 100644
--- a/tools/perf/builtin.h
+++ b/tools/perf/builtin.h
@@ -37,6 +37,7 @@ int cmd_inject(int argc, const char **argv);
 int cmd_mem(int argc, const char **argv);
 int cmd_data(int argc, const char **argv);
 int cmd_ftrace(int argc, const char **argv);
+int cmd_daemon(int argc, const char **argv);
 
 int find_scripts(char **scripts_array, char **scripts_path_array, int num,
 		 int pathlen);
diff --git a/tools/perf/command-list.txt b/tools/perf/command-list.txt
index bc6c585f74fc..825a12e8d694 100644
--- a/tools/perf/command-list.txt
+++ b/tools/perf/command-list.txt
@@ -31,3 +31,4 @@ perf-timechart			mainporcelain common
 perf-top			mainporcelain common
 perf-trace			mainporcelain audit
 perf-version			mainporcelain common
+perf-daemon			mainporcelain common
diff --git a/tools/perf/perf.c b/tools/perf/perf.c
index 27f94b0bb874..20cb91ef06ff 100644
--- a/tools/perf/perf.c
+++ b/tools/perf/perf.c
@@ -88,6 +88,7 @@ static struct cmd_struct commands[] = {
 	{ "mem",	cmd_mem,	0 },
 	{ "data",	cmd_data,	0 },
 	{ "ftrace",	cmd_ftrace,	0 },
+	{ "daemon",	cmd_daemon,	0 },
 };
 
 struct pager_config {
-- 
2.29.2

[PATCH 02/24] perf daemon: Add config option

[Jiri Olsa]

Adding config option and base functionality that takes the option
argument (if specified) and other system config locations and
produces 'acting' config file path.

The actual config file processing is coming in following patches.

Signed-off-by: Jiri Olsa <jolsa@kernel.org>
---
 tools/perf/Documentation/perf-daemon.txt |  4 +++
 tools/perf/builtin-daemon.c              | 44 ++++++++++++++++++++++++
 2 files changed, 48 insertions(+)

diff --git a/tools/perf/Documentation/perf-daemon.txt b/tools/perf/Documentation/perf-daemon.txt
index d05b8dab0a6a..ba3f88510aee 100644
--- a/tools/perf/Documentation/perf-daemon.txt
+++ b/tools/perf/Documentation/perf-daemon.txt
@@ -27,6 +27,10 @@ OPTIONS
 --verbose::
 	Be more verbose.
 
+--config=<PATH>::
+	Config file path. If not provided, perf will check system and default
+	locations (/etc/perfconfig, $HOME/.perfconfig).
+
 All generic options are available also under commands.
 
 
diff --git a/tools/perf/builtin-daemon.c b/tools/perf/builtin-daemon.c
index 8b13e455ac40..90b5a8ea9dda 100644
--- a/tools/perf/builtin-daemon.c
+++ b/tools/perf/builtin-daemon.c
@@ -3,14 +3,18 @@
 #include <linux/limits.h>
 #include <string.h>
 #include <signal.h>
+#include <stdlib.h>
 #include <stdio.h>
 #include <unistd.h>
 #include "builtin.h"
 #include "perf.h"
 #include "debug.h"
+#include "config.h"
 #include "util.h"
 
 struct daemon {
+	const char		*config;
+	char			*config_real;
 	char			*base;
 	FILE			*out;
 	char			 perf[PATH_MAX];
@@ -31,6 +35,32 @@ static void sig_handler(int sig __maybe_unused)
 	done = true;
 }
 
+static void daemon__exit(struct daemon *daemon)
+{
+	free(daemon->config_real);
+}
+
+static int setup_config(struct daemon *daemon)
+{
+	if (daemon->config) {
+		char *real = realpath(daemon->config, NULL);
+
+		if (!real) {
+			perror("failed: realpath");
+			return -1;
+		}
+		daemon->config_real = real;
+		return 0;
+	}
+
+	if (perf_config_system() && !access(perf_etc_perfconfig(), R_OK))
+		daemon->config_real = strdup(perf_etc_perfconfig());
+	else if (perf_config_global() && perf_home_perfconfig())
+		daemon->config_real = strdup(perf_home_perfconfig());
+
+	return daemon->config_real ? 0 : -1;
+}
+
 static int __cmd_start(struct daemon *daemon, struct option parent_options[],
 		       int argc, const char **argv)
 {
@@ -44,6 +74,11 @@ static int __cmd_start(struct daemon *daemon, struct option parent_options[],
 	if (argc)
 		usage_with_options(daemon_usage, start_options);
 
+	if (setup_config(daemon)) {
+		pr_err("failed: config not found\n");
+		return -1;
+	}
+
 	debug_set_file(daemon->out);
 	debug_set_display_time(true);
 
@@ -56,6 +91,8 @@ static int __cmd_start(struct daemon *daemon, struct option parent_options[],
 		sleep(1);
 	}
 
+	daemon__exit(daemon);
+
 	pr_info("daemon exited\n");
 	fclose(daemon->out);
 	return err;
@@ -65,6 +102,8 @@ int cmd_daemon(int argc, const char **argv)
 {
 	struct option daemon_options[] = {
 		OPT_INCR('v', "verbose", &verbose, "be more verbose"),
+		OPT_STRING(0, "config", &__daemon.config,
+			"config file", "config file path"),
 		OPT_END()
 	};
 
@@ -82,5 +121,10 @@ int cmd_daemon(int argc, const char **argv)
 		return -1;
 	}
 
+	if (setup_config(&__daemon)) {
+		pr_err("failed: config not found\n");
+		return -1;
+	}
+
 	return -1;
 }
-- 
2.29.2

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

[Jiri Olsa]

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 <jolsa@kernel.org>
---
 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

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

[Jiri Olsa]

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 <jolsa@kernel.org>
---
 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

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

[Jiri Olsa]

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 <jolsa@kernel.org>
---
 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

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

[Jiri Olsa]

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 <jolsa@kernel.org>
---
 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

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

[Jiri Olsa]

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 <jolsa@kernel.org>
---
 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

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

[Jiri Olsa]

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 <jolsa@kernel.org>
---
 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

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

[Jiri Olsa]

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 <abudankov@huawei.com>
Signed-off-by: Jiri Olsa <jolsa@kernel.org>
---
 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

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

[Jiri Olsa]

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 <jolsa@kernel.org>
---
 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

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

[Jiri Olsa]

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 <jolsa@kernel.org>
---
 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

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

[Jiri Olsa]

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 <jolsa@kernel.org>
---
 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

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

[Jiri Olsa]

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 <jolsa@kernel.org>
---
 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

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

[Jiri Olsa]

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 <jolsa@kernel.org>
---
 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

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

[Jiri Olsa]

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 <jolsa@kernel.org>
---
 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

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

[Jiri Olsa]

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

Signed-off-by: Jiri Olsa <jolsa@kernel.org>
---
 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

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

[Jiri Olsa]

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 <jolsa@kernel.org>
---
 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

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

[Jiri Olsa]

Adding usage examples to the man page.

Signed-off-by: Jiri Olsa <jolsa@kernel.org>
---
 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

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

[Jiri Olsa]

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 <jolsa@kernel.org>
---
 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

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

[Jiri Olsa]

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

Signed-off-by: Jiri Olsa <jolsa@kernel.org>
---
 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

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

[Jiri Olsa]

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 <jolsa@kernel.org>
---
 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

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

[Jiri Olsa]

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 <jolsa@kernel.org>
---
 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

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

[Jiri Olsa]

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

Signed-off-by: Jiri Olsa <jolsa@kernel.org>
---
 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

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

[Jiri Olsa]

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

Signed-off-by: Jiri Olsa <jolsa@kernel.org>
---
 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

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

[Arnaldo Carvalho de Melo]

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

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

[Jiri Olsa]

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

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

[Namhyung Kim]

Hi Jiri,

On Tue, Feb 9, 2021 at 5:09 AM Jiri Olsa <jolsa@kernel.org> 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
>

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

[Arnaldo Carvalho de Melo]

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)

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

[Arnaldo Carvalho de Melo]

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]$

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

[Jiri Olsa]

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

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

[Jiri Olsa]

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

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

[Jiri Olsa]

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 <jolsa@kernel.org> 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

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

[Arnaldo Carvalho de Melo]

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 <jolsa@kernel.org> 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

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

[Jiri Olsa]

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 <jolsa@kernel.org> 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

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

[Namhyung Kim]

On Sun, Feb 7, 2021 at 7:35 AM Jiri Olsa <jolsa@redhat.com> wrote:
>
> On Sat, Feb 06, 2021 at 05:05:04PM +0900, Namhyung Kim wrote:
> > On Fri, Feb 5, 2021 at 9:56 PM Jiri Olsa <jolsa@redhat.com> wrote:
> > >
> > > On Fri, Feb 05, 2021 at 09:14:54PM +0900, Namhyung Kim wrote:
> > > > On Sun, Jan 31, 2021 at 8:49 AM Jiri Olsa <jolsa@kernel.org> wrote:
> > > > [SNIP]
> > > > > @@ -263,9 +605,16 @@ 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);
> > > >
> > > > I think it's confusing since you put the reconfig function here.
> > > > What not split normal and reconfig passes?
> > >
> > > hum, not sure what's confusing in here? I've been known to
> > > produce confusing code, but this one seems clear to me
> >
> > Maybe it's because of the name.  I thought reconfig is a
> > special case when it detects some change.  But you call
> > it in the loop unconditionally.
> >
> > >
> > > >
> > > > I mean something like below
> > > >
> > > >  __cmd_start()
> > > > {
> > > >     setup_server_config();
> > > >     daemon__run();
> > >
> > > what's daemon__run? the daemon operates in the while loop below
> >
> > I thought about starting the sessions in the config.
> >
> > >
> > > >
> > > >     while (!done && !err) {
> > > >         ...
> > > >         if (reconfig) {
> > > >             daemon__kill();
> > >
> > > you don't kill daemon for each reconfig change,
> > > we detect changed sessions and kill/restart only them
> >
> > Yep, we can make it that way.
> >
> > >
> > > >             setup_server_config();
> > > >             daemon__reconfig();
> > > >         }
> > > >     }
> > >
> > >
> > > so basically the current workflow is:
> > >
> > >         setup_server_config                                     <--- reads config file, prepares session objects
> > >
> > >         while (!done) {
> > >                 daemon__reconfig                                <--- check session objects states and run/stop them
> >
> > Hmm.. then how about rename it to daemon__handle_state()
> > or daemon__do_loop() or something?
>
> well it handles reconfig, so I don't think that there's
> better name than daemon__reconfig ;-)
>
> apart from handle_server_socket, all the other functions
> we call after poll can change session state, so we need
> to 'reconfig' sessions each time we do a loop

OK then.  Thanks for the explanation!

Thanks,
Namhyung

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

[Jiri Olsa]

On Sat, Feb 06, 2021 at 05:05:04PM +0900, Namhyung Kim wrote:
> On Fri, Feb 5, 2021 at 9:56 PM Jiri Olsa <jolsa@redhat.com> wrote:
> >
> > On Fri, Feb 05, 2021 at 09:14:54PM +0900, Namhyung Kim wrote:
> > > On Sun, Jan 31, 2021 at 8:49 AM Jiri Olsa <jolsa@kernel.org> wrote:
> > > [SNIP]
> > > > @@ -263,9 +605,16 @@ 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);
> > >
> > > I think it's confusing since you put the reconfig function here.
> > > What not split normal and reconfig passes?
> >
> > hum, not sure what's confusing in here? I've been known to
> > produce confusing code, but this one seems clear to me
> 
> Maybe it's because of the name.  I thought reconfig is a
> special case when it detects some change.  But you call
> it in the loop unconditionally.
> 
> >
> > >
> > > I mean something like below
> > >
> > >  __cmd_start()
> > > {
> > >     setup_server_config();
> > >     daemon__run();
> >
> > what's daemon__run? the daemon operates in the while loop below
> 
> I thought about starting the sessions in the config.
> 
> >
> > >
> > >     while (!done && !err) {
> > >         ...
> > >         if (reconfig) {
> > >             daemon__kill();
> >
> > you don't kill daemon for each reconfig change,
> > we detect changed sessions and kill/restart only them
> 
> Yep, we can make it that way.
> 
> >
> > >             setup_server_config();
> > >             daemon__reconfig();
> > >         }
> > >     }
> >
> >
> > so basically the current workflow is:
> >
> >         setup_server_config                                     <--- reads config file, prepares session objects
> >
> >         while (!done) {
> >                 daemon__reconfig                                <--- check session objects states and run/stop them
> 
> Hmm.. then how about rename it to daemon__handle_state()
> or daemon__do_loop() or something?

well it handles reconfig, so I don't think that there's
better name than daemon__reconfig ;-)

apart from handle_server_socket, all the other functions
we call after poll can change session state, so we need
to 'reconfig' sessions each time we do a loop

jirka

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

[Jiri Olsa]

On Thu, Feb 04, 2021 at 04:08:50PM +0100, Jiri Olsa wrote:

SNIP

> > > +
> > > +static void session__free(struct session *session)
> > > +{
> > > +	free(session->base);
> > > +	free(session->name);
> > > +	free(session->run);
> > 
> > zfree() so that if there is some dangling pointer to session, we'll get
> > NULL derefs
> 
> and won't be notified by crash about the error ;-) ok

oops, actualy it makes no sense to do it here, because we're
freeing session just in the next line

> 
> > 
> > > +	free(session);
> > > +}
> > > +
> > > +static void session__remove(struct session *session)
> > > +{
> > > +	list_del(&session->list);
> > 
> > list_del_init

same here

> > 
> > > +	session__free(session);
> > > +}
> > > +
> > > +static void daemon__kill(struct daemon *daemon)
> > > +{
> > > +	daemon__signal(daemon, SIGTERM);
> > > +}
> > > +
> > >  static void daemon__free(struct daemon *daemon)
> > >  {
> > > +	struct session *session, *h;
> > > +
> > > +	list_for_each_entry_safe(session, h, &daemon->sessions, list)
> > > +		session__remove(session);
> > 
> > Wouldn't be better to have:
> > 
> > 	 list_for_each_entry_safe(session, h, &daemon->sessions, list) {
> > 	 	list_del_init(&session->list);
> > 		session__free(session);
> > 	 }
> > 
> > Because naming that function "session__remove()" one thinks it is being
> > removed from some data structure, but not that it is being as well
> > deleted.

session__remove is being called also from daemon__reconfig,
so it's there not to repeat the code, I'm ok to rename it

thanks,
jirka

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

[Namhyung Kim]

On Fri, Feb 5, 2021 at 12:10 AM Jiri Olsa <jolsa@redhat.com> wrote:
>
> On Thu, Feb 04, 2021 at 09:58:19AM -0300, Arnaldo Carvalho de Melo wrote:
> > Em Thu, Feb 04, 2021 at 09:42:35PM +0900, Namhyung Kim escreveu:
> > > Hi Jiri,
> > >
> > > On Sun, Jan 31, 2021 at 8:49 AM Jiri Olsa <jolsa@kernel.org> wrote:
> > > [SNIP]
> > > > +#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 session_state {
> > > > +       SESSION_STATE__OK,
> > > > +       SESSION_STATE__RECONFIG,
> > > > +       SESSION_STATE__KILL,
> > > > +};
> > > > +
> > > > +struct session {
> > > > +       char                    *base;
> > > > +       char                    *name;
> > > > +       char                    *run;
> > > > +       int                      pid;
> > > > +       struct list_head         list;
> > > > +       enum session_state       state;
> > > > +};
> > >
> > > Although I think calling it 'session' is intuitive, it's also confusing
> > > as we already have struct perf_session...
>
> ok, how about daemon_session then?

I'm ok with it.

>
> >
> > Maybe 'struct server_session' ? If this ends up in tools/lib/perf, then
> > it gets renamed to 'struct perf_server_session', just like we have
> > 'struct perf_evsel' in libperf and 'struct evsel' in tools/perf/, right?
>
> let's have our grand-grand-grandkids worry about that ;-)

:)

Thanks,
Namhyung

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

[Namhyung Kim]

On Fri, Feb 5, 2021 at 9:56 PM Jiri Olsa <jolsa@redhat.com> wrote:
>
> On Fri, Feb 05, 2021 at 09:14:54PM +0900, Namhyung Kim wrote:
> > On Sun, Jan 31, 2021 at 8:49 AM Jiri Olsa <jolsa@kernel.org> wrote:
> > [SNIP]
> > > @@ -263,9 +605,16 @@ 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);
> >
> > I think it's confusing since you put the reconfig function here.
> > What not split normal and reconfig passes?
>
> hum, not sure what's confusing in here? I've been known to
> produce confusing code, but this one seems clear to me

Maybe it's because of the name.  I thought reconfig is a
special case when it detects some change.  But you call
it in the loop unconditionally.

>
> >
> > I mean something like below
> >
> >  __cmd_start()
> > {
> >     setup_server_config();
> >     daemon__run();
>
> what's daemon__run? the daemon operates in the while loop below

I thought about starting the sessions in the config.

>
> >
> >     while (!done && !err) {
> >         ...
> >         if (reconfig) {
> >             daemon__kill();
>
> you don't kill daemon for each reconfig change,
> we detect changed sessions and kill/restart only them

Yep, we can make it that way.

>
> >             setup_server_config();
> >             daemon__reconfig();
> >         }
> >     }
>
>
> so basically the current workflow is:
>
>         setup_server_config                                     <--- reads config file, prepares session objects
>
>         while (!done) {
>                 daemon__reconfig                                <--- check session objects states and run/stop them

Hmm.. then how about rename it to daemon__handle_state()
or daemon__do_loop() or something?

Thanks,
Namhyung


>
>                 if (fdarray__poll(&fda, -1)) {
>
>                         handle_config_changes(&reconfig)        <--- was there a config file change?
>
>                         if (reconfig)                           <--- yes,
>                                 setup_server_config             <---      change session objects/states
>                 }
>         }
>
> thanks,
> jirka
>

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

[Jiri Olsa]

On Fri, Feb 05, 2021 at 09:14:54PM +0900, Namhyung Kim wrote:
> On Sun, Jan 31, 2021 at 8:49 AM Jiri Olsa <jolsa@kernel.org> wrote:
> [SNIP]
> > @@ -263,9 +605,16 @@ 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);
> 
> I think it's confusing since you put the reconfig function here.
> What not split normal and reconfig passes?

hum, not sure what's confusing in here? I've been known to
produce confusing code, but this one seems clear to me

> 
> I mean something like below
> 
>  __cmd_start()
> {
>     setup_server_config();
>     daemon__run();

what's daemon__run? the daemon operates in the while loop below

> 
>     while (!done && !err) {
>         ...
>         if (reconfig) {
>             daemon__kill();

you don't kill daemon for each reconfig change,
we detect changed sessions and kill/restart only them

>             setup_server_config();
>             daemon__reconfig();
>         }
>     }


so basically the current workflow is:

	setup_server_config					<--- reads config file, prepares session objects

	while (!done) {
		daemon__reconfig				<--- check session objects states and run/stop them

		if (fdarray__poll(&fda, -1)) {

			handle_config_changes(&reconfig)	<--- was there a config file change?

			if (reconfig)				<--- yes,
				setup_server_config		<---      change session objects/states
		}
	}

thanks,
jirka

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

[Namhyung Kim]

On Sun, Jan 31, 2021 at 8:49 AM Jiri Olsa <jolsa@kernel.org> wrote:
[SNIP]
> @@ -263,9 +605,16 @@ 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);

I think it's confusing since you put the reconfig function here.
What not split normal and reconfig passes?

I mean something like below

 __cmd_start()
{
    setup_server_config();
    daemon__run();

    while (!done && !err) {
        ...
        if (reconfig) {
            daemon__kill();
            setup_server_config();
            daemon__reconfig();
        }
    }

Thanks,
Namhyung


> +
> +               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);
>                 }
>         }
>
> --
> 2.29.2
>

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

[Jiri Olsa]

On Thu, Feb 04, 2021 at 09:58:19AM -0300, Arnaldo Carvalho de Melo wrote:
> Em Thu, Feb 04, 2021 at 09:42:35PM +0900, Namhyung Kim escreveu:
> > Hi Jiri,
> > 
> > On Sun, Jan 31, 2021 at 8:49 AM Jiri Olsa <jolsa@kernel.org> wrote:
> > [SNIP]
> > > +#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 session_state {
> > > +       SESSION_STATE__OK,
> > > +       SESSION_STATE__RECONFIG,
> > > +       SESSION_STATE__KILL,
> > > +};
> > > +
> > > +struct session {
> > > +       char                    *base;
> > > +       char                    *name;
> > > +       char                    *run;
> > > +       int                      pid;
> > > +       struct list_head         list;
> > > +       enum session_state       state;
> > > +};
> > 
> > Although I think calling it 'session' is intuitive, it's also confusing
> > as we already have struct perf_session...

ok, how about daemon_session then?

> 
> Maybe 'struct server_session' ? If this ends up in tools/lib/perf, then
> it gets renamed to 'struct perf_server_session', just like we have
> 'struct perf_evsel' in libperf and 'struct evsel' in tools/perf/, right?

let's have our grand-grand-grandkids worry about that ;-)

thanks,
jirka

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

[Jiri Olsa]

On Wed, Feb 03, 2021 at 06:12:11PM -0300, Arnaldo Carvalho de Melo wrote:

SNIP

> > -static struct daemon __daemon = { };
> > +static struct daemon __daemon = {
> > +	.sessions = LIST_HEAD_INIT(__daemon.sessions),
> > +};
> >  
> >  static const char * const daemon_usage[] = {
> >  	"perf daemon start [<options>]",
> > @@ -43,6 +97,128 @@ static void sig_handler(int sig __maybe_unused)
> >  	done = true;
> >  }
> >  
> > +static struct session*
> > +daemon__add_session(struct daemon *config, char *name)
> > +{
> > +	struct session *session;
> > +
> > +	session = zalloc(sizeof(*session));
> 
> 	
> 	struct 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 session*
> 
> add space after type name
> 
>    static struct session *
> 
> And we could have it in the same line:
> 
>   static struct session *daemon__find_session(struct daemon *daemon, char *name)

ok

SNIP

> > +			/*
> > +			 * Either new or changed run value is defined,
> > +			 * trigger reconfig for the session.
> > +			 */
> > +			session->state = 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 else uses {}, if should too

ok

SNIP

> > +
> > +static void session__free(struct session *session)
> > +{
> > +	free(session->base);
> > +	free(session->name);
> > +	free(session->run);
> 
> zfree() so that if there is some dangling pointer to session, we'll get
> NULL derefs

and won't be notified by crash about the error ;-) ok

> 
> > +	free(session);
> > +}
> > +
> > +static void session__remove(struct session *session)
> > +{
> > +	list_del(&session->list);
> 
> list_del_init
> 
> > +	session__free(session);
> > +}
> > +
> > +static void daemon__kill(struct daemon *daemon)
> > +{
> > +	daemon__signal(daemon, SIGTERM);
> > +}
> > +
> >  static void daemon__free(struct daemon *daemon)
> >  {
> > +	struct session *session, *h;
> > +
> > +	list_for_each_entry_safe(session, h, &daemon->sessions, list)
> > +		session__remove(session);
> 
> Wouldn't be better to have:
> 
> 	 list_for_each_entry_safe(session, h, &daemon->sessions, list) {
> 	 	list_del_init(&session->list);
> 		session__free(session);
> 	 }
> 
> Because naming that function "session__remove()" one thinks it is being
> removed from some data structure, but not that it is being as well
> deleted.
> 
> Please rename session__free() to session__delete() to keep it consistent
> with other places.

ok

> 
> > +
> >  	free(daemon->config_real);
> >  	free(daemon->base);
> >  }
> >  
> >  static void daemon__exit(struct daemon *daemon)
> >  {
> > +	daemon__kill(daemon);
> >  	daemon__free(daemon);
> 
> Ditto for daemon__free(): please rename it to daemon__delete()

ok

thanks,
jirka

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

[Arnaldo Carvalho de Melo]

Em Thu, Feb 04, 2021 at 09:42:35PM +0900, Namhyung Kim escreveu:
> Hi Jiri,
> 
> On Sun, Jan 31, 2021 at 8:49 AM Jiri Olsa <jolsa@kernel.org> wrote:
> [SNIP]
> > +#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 session_state {
> > +       SESSION_STATE__OK,
> > +       SESSION_STATE__RECONFIG,
> > +       SESSION_STATE__KILL,
> > +};
> > +
> > +struct session {
> > +       char                    *base;
> > +       char                    *name;
> > +       char                    *run;
> > +       int                      pid;
> > +       struct list_head         list;
> > +       enum session_state       state;
> > +};
> 
> Although I think calling it 'session' is intuitive, it's also confusing
> as we already have struct perf_session...

Maybe 'struct server_session' ? If this ends up in tools/lib/perf, then
it gets renamed to 'struct perf_server_session', just like we have
'struct perf_evsel' in libperf and 'struct evsel' in tools/perf/, right?

- Arnaldo

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

[Namhyung Kim]

Hi Jiri,

On Sun, Jan 31, 2021 at 8:49 AM Jiri Olsa <jolsa@kernel.org> wrote:
[SNIP]
> +#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 session_state {
> +       SESSION_STATE__OK,
> +       SESSION_STATE__RECONFIG,
> +       SESSION_STATE__KILL,
> +};
> +
> +struct session {
> +       char                    *base;
> +       char                    *name;
> +       char                    *run;
> +       int                      pid;
> +       struct list_head         list;
> +       enum session_state       state;
> +};

Although I think calling it 'session' is intuitive, it's also confusing
as we already have struct perf_session...

Thanks,
Namhyung

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

[Arnaldo Carvalho de Melo]

Em Sun, Jan 31, 2021 at 12:48:38AM +0100, Jiri Olsa escreveu:
> 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 <jolsa@kernel.org>
> ---
>  tools/perf/builtin-daemon.c | 353 +++++++++++++++++++++++++++++++++++-
>  1 file changed, 351 insertions(+), 2 deletions(-)
> 
> diff --git a/tools/perf/builtin-daemon.c b/tools/perf/builtin-daemon.c
> index eada3ceb9b0c..be2ade9967b3 100644
> --- a/tools/perf/builtin-daemon.c
> +++ b/tools/perf/builtin-daemon.c
> @@ -1,6 +1,8 @@
>  // SPDX-License-Identifier: GPL-2.0
>  #include <subcmd/parse-options.h>
>  #include <api/fd/array.h>
> +#include <linux/zalloc.h>
> +#include <linux/string.h>
>  #include <linux/limits.h>
>  #include <string.h>
>  #include <signal.h>
> @@ -13,22 +15,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 session_state {
> +	SESSION_STATE__OK,
> +	SESSION_STATE__RECONFIG,
> +	SESSION_STATE__KILL,
> +};
> +
> +struct session {
> +	char			*base;
> +	char			*name;
> +	char			*run;
> +	int			 pid;
> +	struct list_head	 list;
> +	enum 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>]",
> @@ -43,6 +97,128 @@ static void sig_handler(int sig __maybe_unused)
>  	done = true;
>  }
>  
> +static struct session*
> +daemon__add_session(struct daemon *config, char *name)
> +{
> +	struct session *session;
> +
> +	session = zalloc(sizeof(*session));

	
	struct 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 session*

add space after type name

   static struct session *

And we could have it in the same line:

  static struct session *daemon__find_session(struct daemon *daemon, char *name)

> +daemon__find_session(struct daemon *daemon, char *name)
> +{
> +	struct 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 session *session;
> +	char name[100];
> +
> +	if (get_session_name(var, name, sizeof(name)))

Good , using sizeof () :)

> +		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 = SESSION_STATE__RECONFIG;
> +	} else if (session->state == SESSION_STATE__KILL) {
> +		/* Current session is defined, no action needed. */
> +		pr_debug("reconfig: found current session %s\n", name);
> +		session->state = 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 = 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 else uses {}, if should too

> +		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;
> @@ -87,6 +263,91 @@ 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 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 session_state declaration.
> +	 */
> +	list_for_each_entry(session, &daemon->sessions, list)
> +		session->state = 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 session__run(struct 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;
> @@ -185,17 +446,95 @@ static int setup_client_socket(struct daemon *daemon)
>  	return fd;
>  }
>  
> +static int session__signal(struct session *session, int sig)
> +{
> +	if (session->pid < 0)
> +		return -1;
> +	return kill(session->pid, sig);
> +}
> +
> +static void session__kill(struct session *session)
> +{
> +	session__signal(session, SIGTERM);
> +}
> +
> +static void daemon__signal(struct daemon *daemon, int sig)
> +{
> +	struct session *session;
> +
> +	list_for_each_entry(session, &daemon->sessions, list)
> +		session__signal(session, sig);
> +}
> +
> +static void session__free(struct session *session)
> +{
> +	free(session->base);
> +	free(session->name);
> +	free(session->run);

zfree() so that if there is some dangling pointer to session, we'll get
NULL derefs

> +	free(session);
> +}
> +
> +static void session__remove(struct session *session)
> +{
> +	list_del(&session->list);

list_del_init

> +	session__free(session);
> +}
> +
> +static void daemon__kill(struct daemon *daemon)
> +{
> +	daemon__signal(daemon, SIGTERM);
> +}
> +
>  static void daemon__free(struct daemon *daemon)
>  {
> +	struct session *session, *h;
> +
> +	list_for_each_entry_safe(session, h, &daemon->sessions, list)
> +		session__remove(session);

Wouldn't be better to have:

	 list_for_each_entry_safe(session, h, &daemon->sessions, list) {
	 	list_del_init(&session->list);
		session__free(session);
	 }

Because naming that function "session__remove()" one thinks it is being
removed from some data structure, but not that it is being as well
deleted.

Please rename session__free() to session__delete() to keep it consistent
with other places.

> +
>  	free(daemon->config_real);
>  	free(daemon->base);
>  }
>  
>  static void daemon__exit(struct daemon *daemon)
>  {
> +	daemon__kill(daemon);
>  	daemon__free(daemon);

Ditto for daemon__free(): please rename it to daemon__delete()

>  }
>  
> +static int daemon__reconfig(struct daemon *daemon)
> +{
> +	struct session *session, *n;
> +
> +	list_for_each_entry_safe(session, n, &daemon->sessions, list) {
> +		/* No change. */
> +		if (session->state == SESSION_STATE__OK)
> +			continue;
> +
> +		/* Remove session. */
> +		if (session->state == SESSION_STATE__KILL) {
> +			if (session->pid > 0) {
> +				session__kill(session);
> +				pr_info("reconfig: session '%s' killed\n", session->name);
> +			}
> +			session__remove(session);
> +			continue;
> +		}
> +
> +		/* Reconfig session. */
> +		if (session->pid > 0) {
> +			session__kill(session);
> +			pr_info("reconfig: session '%s' killed\n", session->name);
> +		}
> +		if (session__run(session, daemon))
> +			return -1;
> +
> +		session->state = SESSION_STATE__OK;
> +	}
> +
> +	return 0;
> +}
> +
>  static int setup_config(struct daemon *daemon)
>  {
>  	if (daemon->base_user) {
> @@ -244,6 +583,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);
>  
> @@ -263,9 +605,16 @@ 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);
>  		}
>  	}
>  
> -- 
> 2.29.2
> 

-- 

- Arnaldo

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

[Jiri Olsa]

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 <jolsa@kernel.org>
---
 tools/perf/builtin-daemon.c | 353 +++++++++++++++++++++++++++++++++++-
 1 file changed, 351 insertions(+), 2 deletions(-)

diff --git a/tools/perf/builtin-daemon.c b/tools/perf/builtin-daemon.c
index eada3ceb9b0c..be2ade9967b3 100644
--- a/tools/perf/builtin-daemon.c
+++ b/tools/perf/builtin-daemon.c
@@ -1,6 +1,8 @@
 // SPDX-License-Identifier: GPL-2.0
 #include <subcmd/parse-options.h>
 #include <api/fd/array.h>
+#include <linux/zalloc.h>
+#include <linux/string.h>
 #include <linux/limits.h>
 #include <string.h>
 #include <signal.h>
@@ -13,22 +15,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 session_state {
+	SESSION_STATE__OK,
+	SESSION_STATE__RECONFIG,
+	SESSION_STATE__KILL,
+};
+
+struct session {
+	char			*base;
+	char			*name;
+	char			*run;
+	int			 pid;
+	struct list_head	 list;
+	enum 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>]",
@@ -43,6 +97,128 @@ static void sig_handler(int sig __maybe_unused)
 	done = true;
 }
 
+static struct session*
+daemon__add_session(struct daemon *config, char *name)
+{
+	struct session *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 session*
+daemon__find_session(struct daemon *daemon, char *name)
+{
+	struct 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 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 = SESSION_STATE__RECONFIG;
+	} else if (session->state == SESSION_STATE__KILL) {
+		/* Current session is defined, no action needed. */
+		pr_debug("reconfig: found current session %s\n", name);
+		session->state = 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 = 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;
@@ -87,6 +263,91 @@ 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 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 session_state declaration.
+	 */
+	list_for_each_entry(session, &daemon->sessions, list)
+		session->state = 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 session__run(struct 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;
@@ -185,17 +446,95 @@ static int setup_client_socket(struct daemon *daemon)
 	return fd;
 }
 
+static int session__signal(struct session *session, int sig)
+{
+	if (session->pid < 0)
+		return -1;
+	return kill(session->pid, sig);
+}
+
+static void session__kill(struct session *session)
+{
+	session__signal(session, SIGTERM);
+}
+
+static void daemon__signal(struct daemon *daemon, int sig)
+{
+	struct session *session;
+
+	list_for_each_entry(session, &daemon->sessions, list)
+		session__signal(session, sig);
+}
+
+static void session__free(struct session *session)
+{
+	free(session->base);
+	free(session->name);
+	free(session->run);
+	free(session);
+}
+
+static void session__remove(struct session *session)
+{
+	list_del(&session->list);
+	session__free(session);
+}
+
+static void daemon__kill(struct daemon *daemon)
+{
+	daemon__signal(daemon, SIGTERM);
+}
+
 static void daemon__free(struct daemon *daemon)
 {
+	struct session *session, *h;
+
+	list_for_each_entry_safe(session, h, &daemon->sessions, list)
+		session__remove(session);
+
 	free(daemon->config_real);
 	free(daemon->base);
 }
 
 static void daemon__exit(struct daemon *daemon)
 {
+	daemon__kill(daemon);
 	daemon__free(daemon);
 }
 
+static int daemon__reconfig(struct daemon *daemon)
+{
+	struct session *session, *n;
+
+	list_for_each_entry_safe(session, n, &daemon->sessions, list) {
+		/* No change. */
+		if (session->state == SESSION_STATE__OK)
+			continue;
+
+		/* Remove session. */
+		if (session->state == SESSION_STATE__KILL) {
+			if (session->pid > 0) {
+				session__kill(session);
+				pr_info("reconfig: session '%s' killed\n", session->name);
+			}
+			session__remove(session);
+			continue;
+		}
+
+		/* Reconfig session. */
+		if (session->pid > 0) {
+			session__kill(session);
+			pr_info("reconfig: session '%s' killed\n", session->name);
+		}
+		if (session__run(session, daemon))
+			return -1;
+
+		session->state = SESSION_STATE__OK;
+	}
+
+	return 0;
+}
+
 static int setup_config(struct daemon *daemon)
 {
 	if (daemon->base_user) {
@@ -244,6 +583,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);
 
@@ -263,9 +605,16 @@ 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);
 		}
 	}
 
-- 
2.29.2