linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCHv2 00/22] perf tools: Add daemon command
@ 2021-01-02 22:04 Jiri Olsa
  2021-01-02 22:04 ` [PATCH 01/22] perf tools: Make perf_config_from_file static Jiri Olsa
                   ` (22 more replies)
  0 siblings, 23 replies; 44+ messages in thread
From: Jiri Olsa @ 2021-01-02 22:04 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo
  Cc: lkml, Peter Zijlstra, Ingo Molnar, Mark Rutland, Namhyung Kim,
	Alexander Shishkin, Michael Petlan, Ian Rogers, Stephane Eranian,
	Alexei Budankov

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.

The patchset is based on following control changes:
  https://lore.kernel.org/lkml/20201226232038.390883-1-jolsa@kernel.org/

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

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 (22):
      perf tools: Make perf_config_from_file static
      perf tools: Add config set interface
      perf tools: Add debug_set_display_time function
      perf tools: Add perf_home_perfconfig function
      perf tools: Make perf_config_system global
      perf tools: Make perf_config_global gobal
      perf daemon: Add daemon command
      perf daemon: Add config file change check
      perf daemon: Add signalfd support
      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 man page for perf-daemon
      perf test: Add daemon list command test
      perf test: Add daemon reconfig test
      perf test: Add daemon stop command test
      perf test: Add daemon signal command test
      perf test: Add daemon ping command test

 tools/perf/Build                         |    1 +
 tools/perf/Documentation/perf-config.txt |   14 ++
 tools/perf/Documentation/perf-daemon.txt |  187 ++++++++++++++++
 tools/perf/builtin-daemon.c              | 1327 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 tools/perf/builtin.h                     |    1 +
 tools/perf/command-list.txt              |    1 +
 tools/perf/perf.c                        |    1 +
 tools/perf/tests/shell/daemon.sh         |  388 ++++++++++++++++++++++++++++++++
 tools/perf/util/config.c                 |  123 +++++++----
 tools/perf/util/config.h                 |    7 +-
 tools/perf/util/debug.c                  |   34 ++-
 tools/perf/util/debug.h                  |    1 +
 12 files changed, 2037 insertions(+), 48 deletions(-)
 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


^ permalink raw reply	[flat|nested] 44+ messages in thread

* [PATCH 01/22] perf tools: Make perf_config_from_file static
  2021-01-02 22:04 [PATCHv2 00/22] perf tools: Add daemon command Jiri Olsa
@ 2021-01-02 22:04 ` Jiri Olsa
  2021-01-18 15:51   ` Arnaldo Carvalho de Melo
  2021-01-02 22:04 ` [PATCH 02/22] perf tools: Add config set interface Jiri Olsa
                   ` (21 subsequent siblings)
  22 siblings, 1 reply; 44+ messages in thread
From: Jiri Olsa @ 2021-01-02 22:04 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo
  Cc: lkml, Peter Zijlstra, Ingo Molnar, Mark Rutland, Namhyung Kim,
	Alexander Shishkin, Michael Petlan, Ian Rogers, Stephane Eranian,
	Alexei Budankov

It's not used outside config.c object.

Signed-off-by: Jiri Olsa <jolsa@kernel.org>
---
 tools/perf/util/config.c | 2 +-
 tools/perf/util/config.h | 1 -
 2 files changed, 1 insertion(+), 2 deletions(-)

diff --git a/tools/perf/util/config.c b/tools/perf/util/config.c
index 6969f82843ee..20be0504fb95 100644
--- a/tools/perf/util/config.c
+++ b/tools/perf/util/config.c
@@ -489,7 +489,7 @@ int perf_default_config(const char *var, const char *value,
 	return 0;
 }
 
-int perf_config_from_file(config_fn_t fn, const char *filename, void *data)
+static int perf_config_from_file(config_fn_t fn, const char *filename, void *data)
 {
 	int ret;
 	FILE *f = fopen(filename, "r");
diff --git a/tools/perf/util/config.h b/tools/perf/util/config.h
index 8c881e3a3ec3..2f753b2a034b 100644
--- a/tools/perf/util/config.h
+++ b/tools/perf/util/config.h
@@ -27,7 +27,6 @@ extern const char *config_exclusive_filename;
 
 typedef int (*config_fn_t)(const char *, const char *, void *);
 
-int perf_config_from_file(config_fn_t fn, const char *filename, void *data);
 int perf_default_config(const char *, const char *, void *);
 int perf_config(config_fn_t fn, void *);
 int perf_config_int(int *dest, const char *, const char *);
-- 
2.26.2


^ permalink raw reply related	[flat|nested] 44+ messages in thread

* [PATCH 02/22] perf tools: Add config set interface
  2021-01-02 22:04 [PATCHv2 00/22] perf tools: Add daemon command Jiri Olsa
  2021-01-02 22:04 ` [PATCH 01/22] perf tools: Make perf_config_from_file static Jiri Olsa
@ 2021-01-02 22:04 ` Jiri Olsa
  2021-01-18 15:53   ` Arnaldo Carvalho de Melo
  2021-01-02 22:04 ` [PATCH 03/22] perf tools: Add debug_set_display_time function Jiri Olsa
                   ` (20 subsequent siblings)
  22 siblings, 1 reply; 44+ messages in thread
From: Jiri Olsa @ 2021-01-02 22:04 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo
  Cc: lkml, Peter Zijlstra, Ingo Molnar, Mark Rutland, Namhyung Kim,
	Alexander Shishkin, Michael Petlan, Ian Rogers, Stephane Eranian,
	Alexei Budankov

Add interface to load config set from custom file
by using perf_config_set__load_file function.

It will be used in perf daemon command to process
custom config file.

Signed-off-by: Jiri Olsa <jolsa@kernel.org>
---
 tools/perf/util/config.c | 28 +++++++++++++++++++++++-----
 tools/perf/util/config.h |  3 +++
 2 files changed, 26 insertions(+), 5 deletions(-)

diff --git a/tools/perf/util/config.c b/tools/perf/util/config.c
index 20be0504fb95..222cb2e2de25 100644
--- a/tools/perf/util/config.c
+++ b/tools/perf/util/config.c
@@ -738,6 +738,18 @@ struct perf_config_set *perf_config_set__new(void)
 	return set;
 }
 
+struct perf_config_set *perf_config_set__load_file(const char *file)
+{
+	struct perf_config_set *set = zalloc(sizeof(*set));
+
+	if (set) {
+		INIT_LIST_HEAD(&set->sections);
+		perf_config_from_file(collect_config, file, set);
+	}
+
+	return set;
+}
+
 static int perf_config__init(void)
 {
 	if (config_set == NULL)
@@ -746,17 +758,15 @@ static int perf_config__init(void)
 	return config_set == NULL;
 }
 
-int perf_config(config_fn_t fn, void *data)
+int perf_config_set(struct perf_config_set *set,
+		    config_fn_t fn, void *data)
 {
 	int ret = 0;
 	char key[BUFSIZ];
 	struct perf_config_section *section;
 	struct perf_config_item *item;
 
-	if (config_set == NULL && perf_config__init())
-		return -1;
-
-	perf_config_set__for_each_entry(config_set, section, item) {
+	perf_config_set__for_each_entry(set, section, item) {
 		char *value = item->value;
 
 		if (value) {
@@ -778,6 +788,14 @@ int perf_config(config_fn_t fn, void *data)
 	return ret;
 }
 
+int perf_config(config_fn_t fn, void *data)
+{
+	if (config_set == NULL && perf_config__init())
+		return -1;
+
+	return perf_config_set(config_set, fn, data);
+}
+
 void perf_config__exit(void)
 {
 	perf_config_set__delete(config_set);
diff --git a/tools/perf/util/config.h b/tools/perf/util/config.h
index 2f753b2a034b..ee5a242446e9 100644
--- a/tools/perf/util/config.h
+++ b/tools/perf/util/config.h
@@ -29,6 +29,8 @@ typedef int (*config_fn_t)(const char *, const char *, void *);
 
 int perf_default_config(const char *, const char *, void *);
 int perf_config(config_fn_t fn, void *);
+int perf_config_set(struct perf_config_set *set,
+		    config_fn_t fn, void *data);
 int perf_config_int(int *dest, const char *, const char *);
 int perf_config_u8(u8 *dest, const char *name, const char *value);
 int perf_config_u64(u64 *dest, const char *, const char *);
@@ -37,6 +39,7 @@ int config_error_nonbool(const char *);
 const char *perf_etc_perfconfig(void);
 
 struct perf_config_set *perf_config_set__new(void);
+struct perf_config_set *perf_config_set__load_file(const char *file);
 void perf_config_set__delete(struct perf_config_set *set);
 int perf_config_set__collect(struct perf_config_set *set, const char *file_name,
 			     const char *var, const char *value);
-- 
2.26.2


^ permalink raw reply related	[flat|nested] 44+ messages in thread

* [PATCH 03/22] perf tools: Add debug_set_display_time function
  2021-01-02 22:04 [PATCHv2 00/22] perf tools: Add daemon command Jiri Olsa
  2021-01-02 22:04 ` [PATCH 01/22] perf tools: Make perf_config_from_file static Jiri Olsa
  2021-01-02 22:04 ` [PATCH 02/22] perf tools: Add config set interface Jiri Olsa
@ 2021-01-02 22:04 ` Jiri Olsa
  2021-01-18 16:02   ` Arnaldo Carvalho de Melo
  2021-01-19 14:59   ` Arnaldo Carvalho de Melo
  2021-01-02 22:04 ` [PATCH 04/22] perf tools: Add perf_home_perfconfig function Jiri Olsa
                   ` (19 subsequent siblings)
  22 siblings, 2 replies; 44+ messages in thread
From: Jiri Olsa @ 2021-01-02 22:04 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo
  Cc: lkml, Peter Zijlstra, Ingo Molnar, Mark Rutland, Namhyung Kim,
	Alexander Shishkin, Michael Petlan, Ian Rogers, Stephane Eranian,
	Alexei Budankov

Allow to display time in perf debug output via new
debug_set_display_time function.

It will be used in perf daemon command to get verbose
output into log file.

The debug time format is:

  [2020-12-03 18:25:31.822152] affinity: SYS
  [2020-12-03 18:25:31.822164] mmap flush: 1
  [2020-12-03 18:25:31.822175] comp level: 0
  [2020-12-03 18:25:32.002047] mmap size 528384B

Signed-off-by: Jiri Olsa <jolsa@kernel.org>
---
 tools/perf/util/debug.c | 34 +++++++++++++++++++++++++++++++---
 tools/perf/util/debug.h |  1 +
 2 files changed, 32 insertions(+), 3 deletions(-)

diff --git a/tools/perf/util/debug.c b/tools/perf/util/debug.c
index 50fd6a4be4e0..dc148b08814b 100644
--- a/tools/perf/util/debug.c
+++ b/tools/perf/util/debug.c
@@ -10,6 +10,7 @@
 #include <api/debug.h>
 #include <linux/kernel.h>
 #include <linux/time64.h>
+#include <sys/time.h>
 #ifdef HAVE_BACKTRACE_SUPPORT
 #include <execinfo.h>
 #endif
@@ -31,21 +32,48 @@ int debug_ordered_events;
 static int redirect_to_stderr;
 int debug_data_convert;
 static FILE *debug_file;
+bool debug_display_time;
 
 void debug_set_file(FILE *file)
 {
 	debug_file = file;
 }
 
+void debug_set_display_time(bool set)
+{
+	debug_display_time = set;
+}
+
+static int fprintf_time(FILE *file)
+{
+	struct timeval tod;
+	struct tm ltime;
+	char date[64];
+
+	if (!debug_display_time)
+		return 0;
+
+	if (gettimeofday(&tod, NULL) != 0)
+		return 0;
+
+	if (localtime_r(&tod.tv_sec, &ltime) == NULL)
+		return 0;
+
+	strftime(date, sizeof(date),  "%F %H:%M:%S", &ltime);
+	return fprintf(file, "[%s.%06lu] ", date, tod.tv_usec);
+}
+
 int veprintf(int level, int var, const char *fmt, va_list args)
 {
 	int ret = 0;
 
 	if (var >= level) {
-		if (use_browser >= 1 && !redirect_to_stderr)
+		if (use_browser >= 1 && !redirect_to_stderr) {
 			ui_helpline__vshow(fmt, args);
-		else
-			ret = vfprintf(debug_file, fmt, args);
+		} else {
+			ret = fprintf_time(debug_file);
+			ret += vfprintf(debug_file, fmt, args);
+		}
 	}
 
 	return ret;
diff --git a/tools/perf/util/debug.h b/tools/perf/util/debug.h
index 43f712295645..48f631966067 100644
--- a/tools/perf/util/debug.h
+++ b/tools/perf/util/debug.h
@@ -64,6 +64,7 @@ int veprintf(int level, int var, const char *fmt, va_list args);
 
 int perf_debug_option(const char *str);
 void debug_set_file(FILE *file);
+void debug_set_display_time(bool set);
 void perf_debug_setup(void);
 int perf_quiet_option(void);
 
-- 
2.26.2


^ permalink raw reply related	[flat|nested] 44+ messages in thread

* [PATCH 04/22] perf tools: Add perf_home_perfconfig function
  2021-01-02 22:04 [PATCHv2 00/22] perf tools: Add daemon command Jiri Olsa
                   ` (2 preceding siblings ...)
  2021-01-02 22:04 ` [PATCH 03/22] perf tools: Add debug_set_display_time function Jiri Olsa
@ 2021-01-02 22:04 ` Jiri Olsa
  2021-01-18 16:05   ` Arnaldo Carvalho de Melo
  2021-01-02 22:04 ` [PATCH 05/22] perf tools: Make perf_config_system global Jiri Olsa
                   ` (18 subsequent siblings)
  22 siblings, 1 reply; 44+ messages in thread
From: Jiri Olsa @ 2021-01-02 22:04 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo
  Cc: lkml, Peter Zijlstra, Ingo Molnar, Mark Rutland, Namhyung Kim,
	Alexander Shishkin, Michael Petlan, Ian Rogers, Stephane Eranian,
	Alexei Budankov

Factor out the perf_home_perfconfig, that looks for
.perfconfig in home directory including check for
PERF_CONFIG_NOGLOBAL and for proper permission.

Signed-off-by: Jiri Olsa <jolsa@kernel.org>
---
 tools/perf/util/config.c | 89 ++++++++++++++++++++++++----------------
 tools/perf/util/config.h |  1 +
 2 files changed, 54 insertions(+), 36 deletions(-)

diff --git a/tools/perf/util/config.c b/tools/perf/util/config.c
index 222cb2e2de25..34fe80ccdad1 100644
--- a/tools/perf/util/config.c
+++ b/tools/perf/util/config.c
@@ -531,6 +531,56 @@ static int perf_config_global(void)
 	return !perf_env_bool("PERF_CONFIG_NOGLOBAL", 0);
 }
 
+static char *home_perfconfig(void)
+{
+	const char *home = NULL;
+	char *config;
+	struct stat st;
+
+	home = getenv("HOME");
+
+	/*
+	 * Skip reading user config if:
+	 *   - there is no place to read it from (HOME)
+	 *   - we are asked not to (PERF_CONFIG_NOGLOBAL=1)
+	 */
+	if (!home || !*home || !perf_config_global())
+		return NULL;
+
+	config = strdup(mkpath("%s/.perfconfig", home));
+	if (config == NULL) {
+		pr_warning("Not enough memory to process %s/.perfconfig, ignoring it.", home);
+		return NULL;
+	}
+
+	if (stat(config, &st) < 0)
+		goto out_free;
+
+	if (st.st_uid && (st.st_uid != geteuid())) {
+		pr_warning("File %s not owned by current user or root, ignoring it.", config);
+		goto out_free;
+	}
+
+	if (st.st_size)
+		return config;
+
+out_free:
+	free(config);
+	return NULL;
+}
+
+const char *perf_home_perfconfig(void)
+{
+	static const char *config;
+	static bool failed;
+
+	config = failed ? NULL : home_perfconfig();
+	if (!config)
+		failed = true;
+
+	return config;
+}
+
 static struct perf_config_section *find_section(struct list_head *sections,
 						const char *section_name)
 {
@@ -676,9 +726,6 @@ int perf_config_set__collect(struct perf_config_set *set, const char *file_name,
 static int perf_config_set__init(struct perf_config_set *set)
 {
 	int ret = -1;
-	const char *home = NULL;
-	char *user_config;
-	struct stat st;
 
 	/* Setting $PERF_CONFIG makes perf read _only_ the given config file. */
 	if (config_exclusive_filename)
@@ -687,41 +734,11 @@ static int perf_config_set__init(struct perf_config_set *set)
 		if (perf_config_from_file(collect_config, perf_etc_perfconfig(), set) < 0)
 			goto out;
 	}
-
-	home = getenv("HOME");
-
-	/*
-	 * Skip reading user config if:
-	 *   - there is no place to read it from (HOME)
-	 *   - we are asked not to (PERF_CONFIG_NOGLOBAL=1)
-	 */
-	if (!home || !*home || !perf_config_global())
-		return 0;
-
-	user_config = strdup(mkpath("%s/.perfconfig", home));
-	if (user_config == NULL) {
-		pr_warning("Not enough memory to process %s/.perfconfig, ignoring it.", home);
-		goto out;
-	}
-
-	if (stat(user_config, &st) < 0) {
-		if (errno == ENOENT)
-			ret = 0;
-		goto out_free;
-	}
-
-	ret = 0;
-
-	if (st.st_uid && (st.st_uid != geteuid())) {
-		pr_warning("File %s not owned by current user or root, ignoring it.", user_config);
-		goto out_free;
+	if (perf_config_global() && perf_home_perfconfig()) {
+		if (perf_config_from_file(collect_config, perf_home_perfconfig(), set) < 0)
+			goto out;
 	}
 
-	if (st.st_size)
-		ret = perf_config_from_file(collect_config, user_config, set);
-
-out_free:
-	free(user_config);
 out:
 	return ret;
 }
diff --git a/tools/perf/util/config.h b/tools/perf/util/config.h
index ee5a242446e9..d6c4f80f367c 100644
--- a/tools/perf/util/config.h
+++ b/tools/perf/util/config.h
@@ -37,6 +37,7 @@ int perf_config_u64(u64 *dest, const char *, const char *);
 int perf_config_bool(const char *, const char *);
 int config_error_nonbool(const char *);
 const char *perf_etc_perfconfig(void);
+const char *perf_home_perfconfig(void);
 
 struct perf_config_set *perf_config_set__new(void);
 struct perf_config_set *perf_config_set__load_file(const char *file);
-- 
2.26.2


^ permalink raw reply related	[flat|nested] 44+ messages in thread

* [PATCH 05/22] perf tools: Make perf_config_system global
  2021-01-02 22:04 [PATCHv2 00/22] perf tools: Add daemon command Jiri Olsa
                   ` (3 preceding siblings ...)
  2021-01-02 22:04 ` [PATCH 04/22] perf tools: Add perf_home_perfconfig function Jiri Olsa
@ 2021-01-02 22:04 ` Jiri Olsa
  2021-01-18 16:03   ` Arnaldo Carvalho de Melo
  2021-01-02 22:04 ` [PATCH 06/22] perf tools: Make perf_config_global gobal Jiri Olsa
                   ` (17 subsequent siblings)
  22 siblings, 1 reply; 44+ messages in thread
From: Jiri Olsa @ 2021-01-02 22:04 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo
  Cc: lkml, Peter Zijlstra, Ingo Molnar, Mark Rutland, Namhyung Kim,
	Alexander Shishkin, Michael Petlan, Ian Rogers, Stephane Eranian,
	Alexei Budankov

Making perf_config_system global, it will be used
outside the config.c object in following patches.

Signed-off-by: Jiri Olsa <jolsa@kernel.org>
---
 tools/perf/util/config.c | 2 +-
 tools/perf/util/config.h | 1 +
 2 files changed, 2 insertions(+), 1 deletion(-)

diff --git a/tools/perf/util/config.c b/tools/perf/util/config.c
index 34fe80ccdad1..4e0455a6bb5f 100644
--- a/tools/perf/util/config.c
+++ b/tools/perf/util/config.c
@@ -521,7 +521,7 @@ static int perf_env_bool(const char *k, int def)
 	return v ? perf_config_bool(k, v) : def;
 }
 
-static int perf_config_system(void)
+int perf_config_system(void)
 {
 	return !perf_env_bool("PERF_CONFIG_NOSYSTEM", 0);
 }
diff --git a/tools/perf/util/config.h b/tools/perf/util/config.h
index d6c4f80f367c..bf68e4acea73 100644
--- a/tools/perf/util/config.h
+++ b/tools/perf/util/config.h
@@ -38,6 +38,7 @@ int perf_config_bool(const char *, const char *);
 int config_error_nonbool(const char *);
 const char *perf_etc_perfconfig(void);
 const char *perf_home_perfconfig(void);
+int perf_config_system(void);
 
 struct perf_config_set *perf_config_set__new(void);
 struct perf_config_set *perf_config_set__load_file(const char *file);
-- 
2.26.2


^ permalink raw reply related	[flat|nested] 44+ messages in thread

* [PATCH 06/22] perf tools: Make perf_config_global gobal
  2021-01-02 22:04 [PATCHv2 00/22] perf tools: Add daemon command Jiri Olsa
                   ` (4 preceding siblings ...)
  2021-01-02 22:04 ` [PATCH 05/22] perf tools: Make perf_config_system global Jiri Olsa
@ 2021-01-02 22:04 ` Jiri Olsa
  2021-01-18 16:05   ` Arnaldo Carvalho de Melo
  2021-01-02 22:04 ` [PATCH 07/22] perf daemon: Add daemon command Jiri Olsa
                   ` (16 subsequent siblings)
  22 siblings, 1 reply; 44+ messages in thread
From: Jiri Olsa @ 2021-01-02 22:04 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo
  Cc: lkml, Peter Zijlstra, Ingo Molnar, Mark Rutland, Namhyung Kim,
	Alexander Shishkin, Michael Petlan, Ian Rogers, Stephane Eranian,
	Alexei Budankov

Making perf_config_global global, it will be used
outside the config.c object in following patches.

Signed-off-by: Jiri Olsa <jolsa@kernel.org>
---
 tools/perf/util/config.c | 2 +-
 tools/perf/util/config.h | 1 +
 2 files changed, 2 insertions(+), 1 deletion(-)

diff --git a/tools/perf/util/config.c b/tools/perf/util/config.c
index 4e0455a6bb5f..6984c77068a3 100644
--- a/tools/perf/util/config.c
+++ b/tools/perf/util/config.c
@@ -526,7 +526,7 @@ int perf_config_system(void)
 	return !perf_env_bool("PERF_CONFIG_NOSYSTEM", 0);
 }
 
-static int perf_config_global(void)
+int perf_config_global(void)
 {
 	return !perf_env_bool("PERF_CONFIG_NOGLOBAL", 0);
 }
diff --git a/tools/perf/util/config.h b/tools/perf/util/config.h
index bf68e4acea73..2fd77aaff4d2 100644
--- a/tools/perf/util/config.h
+++ b/tools/perf/util/config.h
@@ -39,6 +39,7 @@ int config_error_nonbool(const char *);
 const char *perf_etc_perfconfig(void);
 const char *perf_home_perfconfig(void);
 int perf_config_system(void);
+int perf_config_global(void);
 
 struct perf_config_set *perf_config_set__new(void);
 struct perf_config_set *perf_config_set__load_file(const char *file);
-- 
2.26.2


^ permalink raw reply related	[flat|nested] 44+ messages in thread

* [PATCH 07/22] perf daemon: Add daemon command
  2021-01-02 22:04 [PATCHv2 00/22] perf tools: Add daemon command Jiri Olsa
                   ` (5 preceding siblings ...)
  2021-01-02 22:04 ` [PATCH 06/22] perf tools: Make perf_config_global gobal Jiri Olsa
@ 2021-01-02 22:04 ` Jiri Olsa
  2021-01-19  4:08   ` Namhyung Kim
  2021-01-27  7:09   ` Namhyung Kim
  2021-01-02 22:04 ` [PATCH 08/22] perf daemon: Add config file change check Jiri Olsa
                   ` (15 subsequent siblings)
  22 siblings, 2 replies; 44+ messages in thread
From: Jiri Olsa @ 2021-01-02 22:04 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo
  Cc: lkml, Peter Zijlstra, Ingo Molnar, Mark Rutland, Namhyung Kim,
	Alexander Shishkin, Michael Petlan, Ian Rogers, Stephane Eranian,
	Alexei Budankov

Adding daemon command that allows to run record sessions
on background. Each session represents one perf record
process and is configured in config file.

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

Check 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

Check 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.

Adding empty perf-daemon.txt to skip compile warning,
the man page is populated in following patch.

Signed-off-by: Jiri Olsa <jolsa@kernel.org>
---
 tools/perf/Build                         |   1 +
 tools/perf/Documentation/perf-daemon.txt |   0
 tools/perf/builtin-daemon.c              | 753 +++++++++++++++++++++++
 tools/perf/builtin.h                     |   1 +
 tools/perf/command-list.txt              |   1 +
 tools/perf/perf.c                        |   1 +
 6 files changed, 757 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..e69de29bb2d1
diff --git a/tools/perf/builtin-daemon.c b/tools/perf/builtin-daemon.c
new file mode 100644
index 000000000000..c2a7738a6a4a
--- /dev/null
+++ b/tools/perf/builtin-daemon.c
@@ -0,0 +1,753 @@
+// SPDX-License-Identifier: GPL-2.0
+#include <subcmd/parse-options.h>
+#include <linux/compiler.h>
+#include <linux/list.h>
+#include <linux/zalloc.h>
+#include <linux/limits.h>
+#include <errno.h>
+#include <string.h>
+#include <sys/types.h>
+#include <sys/wait.h>
+#include <signal.h>
+#include <stdlib.h>
+#include <time.h>
+#include <stdio.h>
+#include <unistd.h>
+#include <sys/prctl.h>
+#include <sys/socket.h>
+#include <sys/un.h>
+#include <api/fd/array.h>
+#include <poll.h>
+#include <sys/stat.h>
+#include <unistd.h>
+#include <fcntl.h>
+#include <sys/inotify.h>
+#include <libgen.h>
+#include "builtin.h"
+#include "perf.h"
+#include "debug.h"
+#include "config.h"
+#include "string2.h"
+#include "asm/bug.h"
+#include "util.h"
+#include <api/fs/fs.h>
+
+#define SESSION_OUTPUT  "output"
+
+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;
+	char			*config_base;
+	const char		*csv_sep;
+	char			*base;
+	struct list_head	 sessions;
+	FILE			*out;
+	char			 perf[PATH_MAX];
+};
+
+static struct daemon __daemon = {
+	.sessions = LIST_HEAD_INIT(__daemon.sessions),
+};
+
+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 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) {
+		session = daemon__add_session(daemon, name);
+		if (!session)
+			return -ENOMEM;
+
+		pr_debug("reconfig: found new session %s\n", name);
+		/* This is new session, trigger reconfig to start it. */
+		session->state = SESSION_STATE__RECONFIG;
+	} else if (session->state == SESSION_STATE__KILL) {
+		/*
+		 * The session was marked to kill and we still
+		 * found it in config file.
+		 */
+		pr_debug("reconfig: found current session %s\n", name);
+		session->state = SESSION_STATE__OK;
+	}
+
+	if (!strcmp(var, "run")) {
+		if (session->run && strcmp(session->run, value)) {
+			free(session->run);
+			pr_debug("reconfig: session %s is changed\n", name);
+			session->state = SESSION_STATE__RECONFIG;
+		}
+		session->run = strdup(value);
+	}
+
+	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 = strdup(value);
+
+	return 0;
+}
+
+static int client_config(const char *var, const char *value, void *cb)
+{
+	struct daemon *daemon = cb;
+
+	if (!strcmp(var, "daemon.base"))
+		daemon->base = strdup(value);
+
+	return 0;
+}
+
+static int setup_client_config(struct daemon *daemon)
+{
+	struct perf_config_set *set;
+	int err = -ENOMEM;
+
+	set = perf_config_set__load_file(daemon->config_real);
+	if (set) {
+		err = perf_config_set(set, client_config, daemon);
+		perf_config_set__delete(set);
+	}
+
+	return err;
+}
+
+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 session for kill, the server config will
+	 * set proper state for found sessions.
+	 */
+	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;
+}
+
+static int session__signal(struct session *session, int sig)
+{
+	if (session->pid < 0)
+		return -1;
+	return kill(session->pid, sig);
+}
+
+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("asprintf failed");
+		return -1;
+	}
+
+	if (mkdir(session->base, 0755) && errno != EEXIST) {
+		perror("mkdir failed");
+		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("chdir failed");
+		return -1;
+	}
+
+	fd = open("/dev/null", O_RDONLY);
+	if (fd < 0) {
+		perror("failed to open /dev/null");
+		return -1;
+	}
+
+	close(0);
+	dup2(fd, 0);
+	close(fd);
+
+	fd = open(SESSION_OUTPUT, O_RDWR|O_CREAT|O_TRUNC, 0644);
+	if (fd < 0) {
+		perror("failed to open session output");
+		return -1;
+	}
+
+	close(1);
+	close(2);
+	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 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 session__kill(struct session *session)
+{
+	session__signal(session, SIGTERM);
+}
+
+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. */
+		pr_debug2("reconfig: session '%s' start\n", session->name);
+		if (session->pid > 0) {
+			session__kill(session);
+			pr_info("reconfig: session '%s' killed\n", session->name);
+		}
+		if (session__run(session, daemon))
+			return -1;
+		pr_debug2("reconfig: session '%s' done\n", session->name);
+		session->state = SESSION_STATE__OK;
+	}
+
+	return 0;
+}
+
+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);
+}
+
+static void daemon__exit(struct daemon *daemon)
+{
+	daemon__kill(daemon);
+	daemon__free(daemon);
+	pr_info("daemon exited\n");
+	fclose(daemon->out);
+}
+
+static int setup_server_socket(struct daemon *daemon)
+{
+	struct sockaddr_un addr;
+	char path[100];
+	int fd;
+
+	fd = socket(AF_UNIX, SOCK_STREAM, 0);
+	if (fd < 0) {
+		fprintf(stderr, "socket: %s\n", strerror(errno));
+		return -1;
+	}
+
+	fcntl(fd, F_SETFD, FD_CLOEXEC);
+
+	scnprintf(path, PATH_MAX, "%s/control", daemon->base);
+
+	memset(&addr, 0, sizeof(addr));
+	addr.sun_family = AF_UNIX;
+
+	strncpy(addr.sun_path, path, sizeof(addr.sun_path) - 1);
+	unlink(path);
+
+	if (bind(fd, (struct sockaddr *)&addr, sizeof(addr)) == -1) {
+		perror("bind error");
+		return -1;
+	}
+
+	if (listen(fd, 1) == -1) {
+		perror("listen error");
+		return -1;
+	}
+
+	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 cmd_session_list(struct daemon *daemon, union cmd *cmd, FILE *out)
+{
+	char csv_sep = cmd->list.csv_sep;
+	struct 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 = -EINVAL, fd;
+	union cmd cmd;
+	FILE *out;
+
+	fd = accept(sock_fd, NULL, NULL);
+	if (fd < 0) {
+		fprintf(stderr, "accept: %s\n", strerror(errno));
+		return -1;
+	}
+
+	if (sizeof(cmd) != read(fd, &cmd, sizeof(cmd))) {
+		fprintf(stderr, "read: %s\n", strerror(errno));
+		return -1;
+	}
+
+	out = fdopen(fd, "w");
+	if (!out) {
+		perror("fopen");
+		return -1;
+	}
+
+	switch (cmd.cmd) {
+	case CMD_LIST:
+		ret = cmd_session_list(daemon, &cmd, out);
+		break;
+	default:
+		break;
+	}
+
+	fclose(out);
+	close(fd);
+	return ret;
+}
+
+static int setup_client_socket(struct daemon *daemon)
+{
+	struct sockaddr_un addr;
+	char path[100];
+	int fd;
+
+	fd = socket(AF_UNIX, SOCK_STREAM, 0);
+	if (fd == -1) {
+		perror("socket error");
+		return -1;
+	}
+
+	scnprintf(path, PATH_MAX, "%s/control", daemon->base);
+
+	memset(&addr, 0, sizeof(addr));
+	addr.sun_family = AF_UNIX;
+	strncpy(addr.sun_path, path, sizeof(addr.sun_path) - 1);
+
+	if (connect(fd, (struct sockaddr *) &addr, sizeof(addr)) == -1) {
+		perror("connect error");
+		return -1;
+	}
+
+	return fd;
+}
+
+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("chdir failed");
+		return -1;
+	}
+
+	fd = open("output", O_RDWR|O_CREAT|O_TRUNC, 0644);
+	if (fd < 0) {
+		perror("open failed");
+		return -1;
+	}
+
+	fcntl(fd, F_SETFD, FD_CLOEXEC);
+
+	daemon->out = fdopen(fd, "w");
+	if (!daemon->out)
+		return -1;
+
+	close(0);
+	dup2(fd, 1);
+	dup2(fd, 2);
+	setbuf(daemon->out, NULL);
+	return 0;
+}
+
+static int setup_config(struct daemon *daemon)
+{
+	if (daemon->config) {
+		char *real = realpath(daemon->config, NULL);
+
+		if (!real) {
+			perror("realpath failed");
+			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)
+{
+	bool foreground = false;
+	struct option start_options[] = {
+		OPT_BOOLEAN('f', "foreground", &foreground, "stay on console"),
+		OPT_PARENT(parent_options),
+		OPT_END()
+	};
+	int sock_pos, sock_fd;
+	struct fdarray fda;
+	int err = 0;
+
+	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;
+	}
+
+	if (setup_server_config(daemon))
+		return -1;
+
+	if (!foreground && go_background(daemon))
+		return -1;
+
+	debug_set_file(daemon->out);
+	debug_set_display_time(true);
+
+	pr_info("daemon started (pid %d)\n", getpid());
+
+	sock_fd = setup_server_socket(daemon);
+	if (sock_fd < 0)
+		return -1;
+
+	/* socket, inotify */
+	fdarray__init(&fda, 2);
+
+	sock_pos = fdarray__add(&fda, sock_fd, POLLIN|POLLERR|POLLHUP, 0);
+	if (sock_pos < 0)
+		return -1;
+
+	signal(SIGINT, sig_handler);
+	signal(SIGTERM, sig_handler);
+
+	while (!done && !err) {
+		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);
+		}
+	}
+
+	fdarray__exit(&fda);
+	daemon__exit(daemon);
+
+	close(sock_fd);
+	return err;
+}
+
+static int send_cmd(struct daemon *daemon, union cmd *cmd)
+{
+	char *line = NULL;
+	size_t len = 0;
+	ssize_t nread;
+	FILE *in;
+	int fd;
+
+	setup_client_config(daemon);
+
+	fd = setup_client_socket(daemon);
+	if (fd < 0)
+		return -1;
+
+	if (sizeof(*cmd) != write(fd, cmd, sizeof(*cmd)))
+		return -1;
+
+	in = fdopen(fd, "r");
+	if (!in) {
+		perror("fopen");
+		return -1;
+	}
+
+	while ((nread = getline(&line, &len, in)) != -1) {
+		fwrite(line, nread, 1, stdout);
+		fflush(stdout);
+	}
+
+	close(fd);
+	return 0;
+}
+
+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[] = {
+		OPT_INCR('v', "verbose", &verbose, "be more verbose"),
+		OPT_STRING(0, "config", &__daemon.config,
+			"config file", "config file path"),
+		OPT_STRING_OPTARG('x', "field-separator", &__daemon.csv_sep,
+			"field separator", "print counts with custom separator", ":"),
+		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 && !strcmp(argv[0], "start"))
+		return __cmd_start(&__daemon, daemon_options, argc, argv);
+
+	if (argc) {
+		pr_err("failed: unknown command '%s'\n", argv[0]);
+		return -1;
+	}
+
+	if (setup_config(&__daemon)) {
+		pr_err("failed: config not found\n");
+		return -1;
+	}
+
+	return send_cmd_list(&__daemon);
+}
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.26.2


^ permalink raw reply related	[flat|nested] 44+ messages in thread

* [PATCH 08/22] perf daemon: Add config file change check
  2021-01-02 22:04 [PATCHv2 00/22] perf tools: Add daemon command Jiri Olsa
                   ` (6 preceding siblings ...)
  2021-01-02 22:04 ` [PATCH 07/22] perf daemon: Add daemon command Jiri Olsa
@ 2021-01-02 22:04 ` Jiri Olsa
  2021-01-19  5:31   ` Namhyung Kim
  2021-01-02 22:04 ` [PATCH 09/22] perf daemon: Add signalfd support Jiri Olsa
                   ` (14 subsequent siblings)
  22 siblings, 1 reply; 44+ messages in thread
From: Jiri Olsa @ 2021-01-02 22:04 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo
  Cc: lkml, Peter Zijlstra, Ingo Molnar, Mark Rutland, Namhyung Kim,
	Alexander Shishkin, Michael Petlan, Ian Rogers, Stephane Eranian,
	Alexei Budankov

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

Using inotify file descriptor pluged 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 | 84 ++++++++++++++++++++++++++++++++++++-
 1 file changed, 83 insertions(+), 1 deletion(-)

diff --git a/tools/perf/builtin-daemon.c b/tools/perf/builtin-daemon.c
index c2a7738a6a4a..16b24c30722d 100644
--- a/tools/perf/builtin-daemon.c
+++ b/tools/perf/builtin-daemon.c
@@ -547,6 +547,77 @@ static int setup_client_socket(struct daemon *daemon)
 	return fd;
 }
 
+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;
+
+	if (!dirn || !basen)
+		return -ENOMEM;
+
+	fd = inotify_init1(IN_NONBLOCK|O_CLOEXEC);
+	if (fd < 0) {
+		perror("inotify_init failed");
+		return -1;
+	}
+
+	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)
+		perror("inotify_add_watch failed");
+	else
+		daemon->config_base = base;
+
+	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("read failed");
+				return -1;
+			}
+			return 0;
+		}
+		*config_changed = process_inotify_event(daemon, buf, len);
+	}
+	return 0;
+}
+
 static int go_background(struct daemon *daemon)
 {
 	int pid, fd;
@@ -617,7 +688,7 @@ static int __cmd_start(struct daemon *daemon, struct option parent_options[],
 		OPT_PARENT(parent_options),
 		OPT_END()
 	};
-	int sock_pos, sock_fd;
+	int sock_pos, file_pos, sock_fd, conf_fd;
 	struct fdarray fda;
 	int err = 0;
 
@@ -645,6 +716,10 @@ static int __cmd_start(struct daemon *daemon, struct option parent_options[],
 	if (sock_fd < 0)
 		return -1;
 
+	conf_fd = setup_config_changes(daemon);
+	if (conf_fd < 0)
+		return -1;
+
 	/* socket, inotify */
 	fdarray__init(&fda, 2);
 
@@ -652,6 +727,10 @@ static int __cmd_start(struct daemon *daemon, struct option parent_options[],
 	if (sock_pos < 0)
 		return -1;
 
+	file_pos = fdarray__add(&fda, conf_fd, POLLIN|POLLERR|POLLHUP, 0);
+	if (file_pos < 0)
+		return -1;
+
 	signal(SIGINT, sig_handler);
 	signal(SIGTERM, sig_handler);
 
@@ -663,6 +742,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);
@@ -673,6 +754,7 @@ static int __cmd_start(struct daemon *daemon, struct option parent_options[],
 	daemon__exit(daemon);
 
 	close(sock_fd);
+	close(conf_fd);
 	return err;
 }
 
-- 
2.26.2


^ permalink raw reply related	[flat|nested] 44+ messages in thread

* [PATCH 09/22] perf daemon: Add signalfd support
  2021-01-02 22:04 [PATCHv2 00/22] perf tools: Add daemon command Jiri Olsa
                   ` (7 preceding siblings ...)
  2021-01-02 22:04 ` [PATCH 08/22] perf daemon: Add config file change check Jiri Olsa
@ 2021-01-02 22:04 ` Jiri Olsa
  2021-01-02 22:04 ` [PATCH 10/22] perf daemon: Add signal command Jiri Olsa
                   ` (13 subsequent siblings)
  22 siblings, 0 replies; 44+ messages in thread
From: Jiri Olsa @ 2021-01-02 22:04 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo
  Cc: Alexei Budankov, lkml, Peter Zijlstra, Ingo Molnar, Mark Rutland,
	Namhyung Kim, Alexander Shishkin, Michael Petlan, Ian Rogers,
	Stephane Eranian

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

Suggested-by: Alexei Budankov <abudankov@huawei.com>
Signed-off-by: Jiri Olsa <jolsa@kernel.org>
---
 tools/perf/builtin-daemon.c | 142 ++++++++++++++++++++++++++++++++++--
 1 file changed, 137 insertions(+), 5 deletions(-)

diff --git a/tools/perf/builtin-daemon.c b/tools/perf/builtin-daemon.c
index 16b24c30722d..644f196d7f39 100644
--- a/tools/perf/builtin-daemon.c
+++ b/tools/perf/builtin-daemon.c
@@ -31,6 +31,7 @@
 #include "asm/bug.h"
 #include "util.h"
 #include <api/fs/fs.h>
+#include <sys/signalfd.h>
 
 #define SESSION_OUTPUT  "output"
 
@@ -58,6 +59,7 @@ struct daemon {
 	struct list_head	 sessions;
 	FILE			*out;
 	char			 perf[PATH_MAX];
+	int			 signal_fd;
 };
 
 static struct daemon __daemon = {
@@ -317,9 +319,110 @@ static void session__remove(struct session *session)
 	session__free(session);
 }
 
-static void session__kill(struct session *session)
+static pid_t handle_signalfd(struct daemon *daemon)
+{
+	struct signalfd_siginfo si;
+	struct session *session;
+	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 = SESSION_STATE__KILL;
+		session->pid = -1;
+		return pid;
+	}
+
+	return 0;
+}
+
+static int session__wait(struct 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 {
+		if (poll(&pollfd, 1, 1000))
+			wpid = handle_signalfd(daemon);
+
+		if (start + secs < time(NULL))
+			return -1;
+	} while (wpid != pid);
+
+	return 0;
+}
+
+static bool daemon__has_alive_session(struct daemon *daemon)
+{
+	struct session *session;
+
+	list_for_each_entry(session, &daemon->sessions, list) {
+		if (session->state == SESSION_STATE__OK)
+			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 {
+		if (poll(&pollfd, 1, 1000))
+			handle_signalfd(daemon);
+
+		if (start + secs < time(NULL))
+			return -1;
+	} while (daemon__has_alive_session(daemon));
+
+	return 0;
+}
+
+static void session__kill(struct session *session, struct daemon *daemon)
 {
 	session__signal(session, SIGTERM);
+	if (session__wait(session, daemon, 10)) {
+		session__signal(session, SIGKILL);
+		session__wait(session, daemon, 10);
+	}
 }
 
 static int daemon__reconfig(struct daemon *daemon)
@@ -334,7 +437,7 @@ static int daemon__reconfig(struct daemon *daemon)
 		/* Remove session. */
 		if (session->state == SESSION_STATE__KILL) {
 			if (session->pid > 0) {
-				session__kill(session);
+				session__kill(session, daemon);
 				pr_info("reconfig: session '%s' killed\n", session->name);
 			}
 			session__remove(session);
@@ -344,7 +447,7 @@ static int daemon__reconfig(struct daemon *daemon)
 		/* Reconfig session. */
 		pr_debug2("reconfig: session '%s' start\n", session->name);
 		if (session->pid > 0) {
-			session__kill(session);
+			session__kill(session, daemon);
 			pr_info("reconfig: session '%s' killed\n", session->name);
 		}
 		if (session__run(session, daemon))
@@ -359,6 +462,10 @@ static int daemon__reconfig(struct daemon *daemon)
 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__free(struct daemon *daemon)
@@ -679,6 +786,20 @@ static int setup_config(struct daemon *daemon)
 	return daemon->config_real ? 0 : -1;
 }
 
+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)
 {
@@ -688,7 +809,7 @@ static int __cmd_start(struct daemon *daemon, struct option parent_options[],
 		OPT_PARENT(parent_options),
 		OPT_END()
 	};
-	int sock_pos, file_pos, sock_fd, conf_fd;
+	int sock_pos, file_pos, signal_pos, sock_fd, conf_fd, signal_fd;
 	struct fdarray fda;
 	int err = 0;
 
@@ -720,8 +841,12 @@ static int __cmd_start(struct daemon *daemon, struct option parent_options[],
 	if (conf_fd < 0)
 		return -1;
 
+	signal_fd = setup_signalfd(daemon);
+	if (signal_fd < 0)
+		return -1;
+
 	/* socket, inotify */
-	fdarray__init(&fda, 2);
+	fdarray__init(&fda, 3);
 
 	sock_pos = fdarray__add(&fda, sock_fd, POLLIN|POLLERR|POLLHUP, 0);
 	if (sock_pos < 0)
@@ -731,6 +856,10 @@ static int __cmd_start(struct daemon *daemon, struct option parent_options[],
 	if (file_pos < 0)
 		return -1;
 
+	signal_pos = fdarray__add(&fda, signal_fd, POLLIN|POLLERR|POLLHUP, 0);
+	if (signal_pos < 0)
+		return -1;
+
 	signal(SIGINT, sig_handler);
 	signal(SIGTERM, sig_handler);
 
@@ -744,6 +873,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);
@@ -755,6 +886,7 @@ static int __cmd_start(struct daemon *daemon, struct option parent_options[],
 
 	close(sock_fd);
 	close(conf_fd);
+	close(signal_fd);
 	return err;
 }
 
-- 
2.26.2


^ permalink raw reply related	[flat|nested] 44+ messages in thread

* [PATCH 10/22] perf daemon: Add signal command
  2021-01-02 22:04 [PATCHv2 00/22] perf tools: Add daemon command Jiri Olsa
                   ` (8 preceding siblings ...)
  2021-01-02 22:04 ` [PATCH 09/22] perf daemon: Add signalfd support Jiri Olsa
@ 2021-01-02 22:04 ` Jiri Olsa
  2021-01-02 22:04 ` [PATCH 11/22] perf daemon: Add stop command Jiri Olsa
                   ` (12 subsequent siblings)
  22 siblings, 0 replies; 44+ messages in thread
From: Jiri Olsa @ 2021-01-02 22:04 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo
  Cc: lkml, Peter Zijlstra, Ingo Molnar, Mark Rutland, Namhyung Kim,
	Alexander Shishkin, Michael Petlan, Ian Rogers, Stephane Eranian,
	Alexei Budankov

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/builtin-daemon.c | 64 ++++++++++++++++++++++++++++++++++++-
 1 file changed, 63 insertions(+), 1 deletion(-)

diff --git a/tools/perf/builtin-daemon.c b/tools/perf/builtin-daemon.c
index 644f196d7f39..a7ffbecf8d14 100644
--- a/tools/perf/builtin-daemon.c
+++ b/tools/perf/builtin-daemon.c
@@ -522,10 +522,13 @@ static int setup_server_socket(struct daemon *daemon)
 }
 
 enum {
-	CMD_LIST = 0,
+	CMD_LIST   = 0,
+	CMD_SIGNAL = 1,
 	CMD_MAX,
 };
 
+#define SESSION_MAX 64
+
 union cmd {
 	int cmd;
 
@@ -535,6 +538,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)
@@ -592,6 +602,24 @@ static int cmd_session_list(struct daemon *daemon, union cmd *cmd, FILE *out)
 	return 0;
 }
 
+static int cmd_session_kill(struct daemon *daemon, union cmd *cmd, FILE *out)
+{
+	struct 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)) {
+			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 = -EINVAL, fd;
@@ -619,6 +647,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;
 	}
@@ -933,6 +964,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[] = {
@@ -954,6 +1013,9 @@ int cmd_daemon(int argc, const char **argv)
 		return __cmd_start(&__daemon, daemon_options, argc, argv);
 
 	if (argc) {
+		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.26.2


^ permalink raw reply related	[flat|nested] 44+ messages in thread

* [PATCH 11/22] perf daemon: Add stop command
  2021-01-02 22:04 [PATCHv2 00/22] perf tools: Add daemon command Jiri Olsa
                   ` (9 preceding siblings ...)
  2021-01-02 22:04 ` [PATCH 10/22] perf daemon: Add signal command Jiri Olsa
@ 2021-01-02 22:04 ` Jiri Olsa
  2021-01-19  5:35   ` Namhyung Kim
  2021-01-02 22:04 ` [PATCH 12/22] perf daemon: Allow only one daemon over base directory Jiri Olsa
                   ` (11 subsequent siblings)
  22 siblings, 1 reply; 44+ messages in thread
From: Jiri Olsa @ 2021-01-02 22:04 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo
  Cc: lkml, Peter Zijlstra, Ingo Molnar, Mark Rutland, Namhyung Kim,
	Alexander Shishkin, Michael Petlan, Ian Rogers, Stephane Eranian,
	Alexei Budankov

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/builtin-daemon.c | 28 ++++++++++++++++++++++++++++
 1 file changed, 28 insertions(+)

diff --git a/tools/perf/builtin-daemon.c b/tools/perf/builtin-daemon.c
index a7ffbecf8d14..45748bb471ec 100644
--- a/tools/perf/builtin-daemon.c
+++ b/tools/perf/builtin-daemon.c
@@ -524,6 +524,7 @@ static int setup_server_socket(struct daemon *daemon)
 enum {
 	CMD_LIST   = 0,
 	CMD_SIGNAL = 1,
+	CMD_STOP   = 2,
 	CMD_MAX,
 };
 
@@ -650,6 +651,10 @@ 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;
+		pr_debug("perf daemon is exciting\n");
+		break;
 	default:
 		break;
 	}
@@ -992,6 +997,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[] = {
@@ -1015,6 +1041,8 @@ int cmd_daemon(int argc, const char **argv)
 	if (argc) {
 		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.26.2


^ permalink raw reply related	[flat|nested] 44+ messages in thread

* [PATCH 12/22] perf daemon: Allow only one daemon over base directory
  2021-01-02 22:04 [PATCHv2 00/22] perf tools: Add daemon command Jiri Olsa
                   ` (10 preceding siblings ...)
  2021-01-02 22:04 ` [PATCH 11/22] perf daemon: Add stop command Jiri Olsa
@ 2021-01-02 22:04 ` Jiri Olsa
  2021-01-19  5:37   ` Namhyung Kim
  2021-01-02 22:04 ` [PATCH 13/22] perf daemon: Set control fifo for session Jiri Olsa
                   ` (10 subsequent siblings)
  22 siblings, 1 reply; 44+ messages in thread
From: Jiri Olsa @ 2021-01-02 22:04 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo
  Cc: lkml, Peter Zijlstra, Ingo Molnar, Mark Rutland, Namhyung Kim,
	Alexander Shishkin, Michael Petlan, Ian Rogers, Stephane Eranian,
	Alexei Budankov

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

Example:

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

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

  [session-sched]
  run = -m 20M -e sched:* --overwrite --switch-output -a

Starting the daemon:

  # perf daemon start

And try once more:

  # perf daemon start
  failed: another perf daemon (pid 775594) owns /opt/perfdata

will end up with an error, because there's already one running
on top of /opt/perfdata.

Signed-off-by: Jiri Olsa <jolsa@kernel.org>
---
 tools/perf/builtin-daemon.c | 49 +++++++++++++++++++++++++++++++++++++
 1 file changed, 49 insertions(+)

diff --git a/tools/perf/builtin-daemon.c b/tools/perf/builtin-daemon.c
index 45748bb471ec..1982eedd3f3f 100644
--- a/tools/perf/builtin-daemon.c
+++ b/tools/perf/builtin-daemon.c
@@ -8,6 +8,7 @@
 #include <string.h>
 #include <sys/types.h>
 #include <sys/wait.h>
+#include <sys/file.h>
 #include <signal.h>
 #include <stdlib.h>
 #include <time.h>
@@ -562,12 +563,18 @@ static int cmd_session_list(struct daemon *daemon, union cmd *cmd, FILE *out)
 			/* output */
 			csv_sep, daemon->base, SESSION_OUTPUT);
 
+		fprintf(out, "%c%s/%s",
+			/* lock */
+			csv_sep, daemon->base, "lock");
+
 		fprintf(out, "\n");
 	} else {
 		fprintf(out, "[%d:daemon] base: %s\n", getpid(), daemon->base);
 		if (cmd->list.verbose) {
 			fprintf(out, "  output:  %s/%s\n",
 				daemon->base, SESSION_OUTPUT);
+			fprintf(out, "  lock:    %s/lock\n",
+				daemon->base);
 		}
 	}
 
@@ -761,6 +768,42 @@ static int handle_config_changes(struct daemon *daemon, int conf_fd,
 	return 0;
 }
 
+static int check_lock(struct daemon *daemon)
+{
+	char path[PATH_MAX];
+	char buf[20];
+	int fd, pid;
+	ssize_t len;
+
+	scnprintf(path, sizeof(path), "%s/lock", daemon->base);
+
+	fd = open(path, O_RDWR|O_CREAT|O_CLOEXEC, 0640);
+	if (fd < 0)
+		return -1;
+
+	if (lockf(fd, F_TLOCK, 0) < 0) {
+		filename__read_int(path, &pid);
+		fprintf(stderr, "failed: another perf daemon (pid %d) owns %s\n",
+			pid, daemon->base);
+		return -1;
+	}
+
+	scnprintf(buf, sizeof(buf), "%d", getpid());
+	len = strlen(buf);
+
+	if (write(fd, buf, len) != len) {
+		perror("write failed");
+		return -1;
+	}
+
+	if (ftruncate(fd, len)) {
+		perror("ftruncate failed");
+		return -1;
+	}
+
+	return 0;
+}
+
 static int go_background(struct daemon *daemon)
 {
 	int pid, fd;
@@ -775,6 +818,9 @@ static int go_background(struct daemon *daemon)
 	if (setsid() < 0)
 		return -1;
 
+	if (check_lock(daemon))
+		return -1;
+
 	umask(0);
 
 	if (chdir(daemon->base)) {
@@ -861,6 +907,9 @@ static int __cmd_start(struct daemon *daemon, struct option parent_options[],
 	if (setup_server_config(daemon))
 		return -1;
 
+	if (foreground && check_lock(daemon))
+		return -1;
+
 	if (!foreground && go_background(daemon))
 		return -1;
 
-- 
2.26.2


^ permalink raw reply related	[flat|nested] 44+ messages in thread

* [PATCH 13/22] perf daemon: Set control fifo for session
  2021-01-02 22:04 [PATCHv2 00/22] perf tools: Add daemon command Jiri Olsa
                   ` (11 preceding siblings ...)
  2021-01-02 22:04 ` [PATCH 12/22] perf daemon: Allow only one daemon over base directory Jiri Olsa
@ 2021-01-02 22:04 ` Jiri Olsa
  2021-01-02 22:04 ` [PATCH 14/22] perf daemon: Add ping command Jiri Olsa
                   ` (9 subsequent siblings)
  22 siblings, 0 replies; 44+ messages in thread
From: Jiri Olsa @ 2021-01-02 22:04 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo
  Cc: lkml, Peter Zijlstra, Ingo Molnar, Mark Rutland, Namhyung Kim,
	Alexander Shishkin, Michael Petlan, Ian Rogers, Stephane Eranian,
	Alexei Budankov

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/builtin-daemon.c | 26 +++++++++++++++++++++++++-
 1 file changed, 25 insertions(+), 1 deletion(-)

diff --git a/tools/perf/builtin-daemon.c b/tools/perf/builtin-daemon.c
index 1982eedd3f3f..0c82fe9603e8 100644
--- a/tools/perf/builtin-daemon.c
+++ b/tools/perf/builtin-daemon.c
@@ -35,6 +35,8 @@
 #include <sys/signalfd.h>
 
 #define SESSION_OUTPUT  "output"
+#define SESSION_CONTROL "control"
+#define SESSION_ACK     "ack"
 
 enum session_state {
 	SESSION_STATE__OK,
@@ -46,6 +48,7 @@ struct session {
 	char			*base;
 	char			*name;
 	char			*run;
+	char			*control;
 	int			 pid;
 	struct list_head	 list;
 	enum session_state	 state;
@@ -288,7 +291,18 @@ static int session__run(struct session *session, struct daemon *daemon)
 	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 to create control fifo");
+		return -1;
+	}
+
+	if (mkfifo(SESSION_ACK, O_RDWR) && errno != EEXIST) {
+		perror("failed to 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)
@@ -594,6 +608,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",
@@ -604,6 +624,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.26.2


^ permalink raw reply related	[flat|nested] 44+ messages in thread

* [PATCH 14/22] perf daemon: Add ping command
  2021-01-02 22:04 [PATCHv2 00/22] perf tools: Add daemon command Jiri Olsa
                   ` (12 preceding siblings ...)
  2021-01-02 22:04 ` [PATCH 13/22] perf daemon: Set control fifo for session Jiri Olsa
@ 2021-01-02 22:04 ` Jiri Olsa
  2021-01-02 22:04 ` [PATCH 15/22] perf daemon: Use control to stop session Jiri Olsa
                   ` (8 subsequent siblings)
  22 siblings, 0 replies; 44+ messages in thread
From: Jiri Olsa @ 2021-01-02 22:04 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo
  Cc: lkml, Peter Zijlstra, Ingo Molnar, Mark Rutland, Namhyung Kim,
	Alexander Shishkin, Michael Petlan, Ian Rogers, Stephane Eranian,
	Alexei Budankov

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
  OK   sched

Signed-off-by: Jiri Olsa <jolsa@kernel.org>
---
 tools/perf/builtin-daemon.c | 141 ++++++++++++++++++++++++++++++++++++
 1 file changed, 141 insertions(+)

diff --git a/tools/perf/builtin-daemon.c b/tools/perf/builtin-daemon.c
index 0c82fe9603e8..260bcffd9ae8 100644
--- a/tools/perf/builtin-daemon.c
+++ b/tools/perf/builtin-daemon.c
@@ -431,6 +431,75 @@ static int daemon__wait(struct daemon *daemon, int secs)
 	return 0;
 }
 
+static int
+session__control(struct 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, len;
+	char buf[20];
+	int ret = -1;
+	ssize_t err;
+
+	scnprintf(control_path, sizeof(control_path), "%s/%s",
+		  session->base, SESSION_CONTROL);
+
+	scnprintf(ack_path, sizeof(ack_path), "%s/%s",
+		  session->base, SESSION_ACK);
+
+	control = open(control_path, O_WRONLY|O_NONBLOCK);
+	if (!control)
+		return -1;
+
+	if (do_ack) {
+		ack = open(ack_path, O_RDONLY, O_NONBLOCK);
+		if (!ack) {
+			close(control);
+			return -1;
+		}
+	}
+
+	len = strlen(msg);
+
+	do {
+		err = write(control, msg, len);
+		if (err == -1 && errno != EAGAIN) {
+			pr_err("failed to write to control pipe: %d (%s)\n",
+			       errno, control_path);
+			goto out;
+		}
+	} while (err != len);
+
+	if (!do_ack)
+		goto out;
+
+	pollfd.fd = ack;
+	pollfd.events = POLLIN;
+
+	if (!poll(&pollfd, 1, 2000)) {
+		pr_err("control ack timeout\n");
+		goto out;
+	}
+
+	if (!pollfd.revents & POLLIN) {
+		pr_err("control: fid not receiveed ack\n");
+		goto out_ack;
+	}
+
+	err = read(ack, buf, sizeof(buf));
+	if (err > 0)
+		ret = strcmp(buf, "ack\n");
+	else
+		pr_err("errno read control %d\n", errno);
+
+out_ack:
+	close(ack);
+out:
+	close(control);
+	return ret;
+}
+
 static void session__kill(struct session *session, struct daemon *daemon)
 {
 	session__signal(session, SIGTERM);
@@ -540,6 +609,7 @@ enum {
 	CMD_LIST   = 0,
 	CMD_SIGNAL = 1,
 	CMD_STOP   = 2,
+	CMD_PING   = 3,
 	CMD_MAX,
 };
 
@@ -561,8 +631,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 session__ping(struct session *session)
+{
+	return 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;
@@ -652,6 +739,29 @@ 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 session *session;
+	bool all = 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 = session__ping(session);
+
+			fprintf(out, "%-4s %s\n", ping_str[state], session->name);
+		}
+	}
+
+	return 0;
+}
+
 static int handle_server_socket(struct daemon *daemon, int sock_fd)
 {
 	int ret = -EINVAL, fd;
@@ -686,6 +796,9 @@ static int handle_server_socket(struct daemon *daemon, int sock_fd)
 		done = 1;
 		pr_debug("perf daemon is exciting\n");
 		break;
+	case CMD_PING:
+		ret = cmd_session_ping(daemon, &cmd, out);
+		break;
 	default:
 		break;
 	}
@@ -971,6 +1084,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);
@@ -1091,6 +1205,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[] = {
@@ -1116,6 +1255,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.26.2


^ permalink raw reply related	[flat|nested] 44+ messages in thread

* [PATCH 15/22] perf daemon: Use control to stop session
  2021-01-02 22:04 [PATCHv2 00/22] perf tools: Add daemon command Jiri Olsa
                   ` (13 preceding siblings ...)
  2021-01-02 22:04 ` [PATCH 14/22] perf daemon: Add ping command Jiri Olsa
@ 2021-01-02 22:04 ` Jiri Olsa
  2021-01-02 22:04 ` [PATCH 16/22] perf daemon: Add up time for daemon/session list Jiri Olsa
                   ` (7 subsequent siblings)
  22 siblings, 0 replies; 44+ messages in thread
From: Jiri Olsa @ 2021-01-02 22:04 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo
  Cc: lkml, Peter Zijlstra, Ingo Molnar, Mark Rutland, Namhyung Kim,
	Alexander Shishkin, Michael Petlan, Ian Rogers, Stephane Eranian,
	Alexei Budankov

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 260bcffd9ae8..e45a1acaad18 100644
--- a/tools/perf/builtin-daemon.c
+++ b/tools/perf/builtin-daemon.c
@@ -502,11 +502,33 @@ session__control(struct session *session, const char *msg, bool do_ack)
 
 static void session__kill(struct session *session, struct daemon *daemon)
 {
-	session__signal(session, SIGTERM);
-	if (session__wait(session, daemon, 10)) {
-		session__signal(session, SIGKILL);
-		session__wait(session, daemon, 10);
-	}
+	int how = 0;
+
+	do {
+		switch (how) {
+		case 0:
+			session__control(session, "stop", false);
+			break;
+		case 1:
+			session__signal(session, SIGTERM);
+			break;
+		case 2:
+			session__signal(session, SIGKILL);
+			break;
+		default:
+			break;
+		}
+		how++;
+
+	} while (session__wait(session, daemon, 10));
+}
+
+static void daemon__stop(struct daemon *daemon)
+{
+	struct session *session;
+
+	list_for_each_entry(session, &daemon->sessions, list)
+		session__control(session, "stop", false);
 }
 
 static int daemon__reconfig(struct daemon *daemon)
@@ -545,11 +567,25 @@ static int daemon__reconfig(struct daemon *daemon)
 
 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__free(struct daemon *daemon)
-- 
2.26.2


^ permalink raw reply related	[flat|nested] 44+ messages in thread

* [PATCH 16/22] perf daemon: Add up time for daemon/session list
  2021-01-02 22:04 [PATCHv2 00/22] perf tools: Add daemon command Jiri Olsa
                   ` (14 preceding siblings ...)
  2021-01-02 22:04 ` [PATCH 15/22] perf daemon: Use control to stop session Jiri Olsa
@ 2021-01-02 22:04 ` Jiri Olsa
  2021-01-02 22:04 ` [PATCH 17/22] perf daemon: Add man page for perf-daemon Jiri Olsa
                   ` (6 subsequent siblings)
  22 siblings, 0 replies; 44+ messages in thread
From: Jiri Olsa @ 2021-01-02 22:04 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo
  Cc: lkml, Peter Zijlstra, Ingo Molnar, Mark Rutland, Namhyung Kim,
	Alexander Shishkin, Michael Petlan, Ian Rogers, Stephane Eranian,
	Alexei Budankov

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 e45a1acaad18..d99aee2a59da 100644
--- a/tools/perf/builtin-daemon.c
+++ b/tools/perf/builtin-daemon.c
@@ -24,6 +24,7 @@
 #include <fcntl.h>
 #include <sys/inotify.h>
 #include <libgen.h>
+#include <time.h>
 #include "builtin.h"
 #include "perf.h"
 #include "debug.h"
@@ -52,6 +53,7 @@ struct session {
 	int			 pid;
 	struct list_head	 list;
 	enum session_state	 state;
+	time_t			 start;
 };
 
 struct daemon {
@@ -64,6 +66,7 @@ struct daemon {
 	FILE			*out;
 	char			 perf[PATH_MAX];
 	int			 signal_fd;
+	time_t			 start;
 };
 
 static struct daemon __daemon = {
@@ -255,6 +258,8 @@ static int session__run(struct session *session, struct daemon *daemon)
 		return -1;
 	}
 
+	session->start = time(NULL);
+
 	session->pid = fork();
 	if (session->pid < 0)
 		return -1;
@@ -690,6 +695,7 @@ static int cmd_session_list(struct daemon *daemon, union cmd *cmd, FILE *out)
 {
 	char csv_sep = cmd->list.csv_sep;
 	struct session *session;
+	time_t curr = time(NULL);
 
 	if (csv_sep) {
 		fprintf(out, "%d%c%s%c%s%c%s/%s",
@@ -704,6 +710,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);
@@ -712,6 +722,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);
 		}
 	}
 
@@ -737,6 +749,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",
@@ -751,6 +767,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);
 		}
 	}
 
@@ -1072,6 +1090,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.26.2


^ permalink raw reply related	[flat|nested] 44+ messages in thread

* [PATCH 17/22] perf daemon: Add man page for perf-daemon
  2021-01-02 22:04 [PATCHv2 00/22] perf tools: Add daemon command Jiri Olsa
                   ` (15 preceding siblings ...)
  2021-01-02 22:04 ` [PATCH 16/22] perf daemon: Add up time for daemon/session list Jiri Olsa
@ 2021-01-02 22:04 ` Jiri Olsa
  2021-01-02 22:04 ` [PATCH 18/22] perf test: Add daemon list command test Jiri Olsa
                   ` (5 subsequent siblings)
  22 siblings, 0 replies; 44+ messages in thread
From: Jiri Olsa @ 2021-01-02 22:04 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo
  Cc: lkml, Peter Zijlstra, Ingo Molnar, Mark Rutland, Namhyung Kim,
	Alexander Shishkin, Michael Petlan, Ian Rogers, Stephane Eranian,
	Alexei Budankov

Adding man page for perf-daemon usage.

Signed-off-by: Jiri Olsa <jolsa@kernel.org>
---
 tools/perf/Documentation/perf-config.txt |  14 ++
 tools/perf/Documentation/perf-daemon.txt | 187 +++++++++++++++++++++++
 2 files changed, 201 insertions(+)

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 e69de29bb2d1..b0e1015476c2 100644
--- a/tools/perf/Documentation/perf-daemon.txt
+++ b/tools/perf/Documentation/perf-daemon.txt
@@ -0,0 +1,187 @@
+perf-daemon(1)
+==============
+
+NAME
+----
+perf-daemon - Run record sessions on background
+
+SYNOPSIS
+--------
+[verse]
+'perf daemon'
+'perf daemon' [<options>]
+'perf daemon start'  [<options>]
+'perf daemon stop'   [<options>]
+'perf daemon signal' [<options>]
+'perf daemon ping'   [<options>]
+
+
+DESCRIPTION
+-----------
+This command allows to run simple daemon process that starts and
+monitors configured record sessions.
+
+Each session represents one perf record process started with
+control setup (with perf record --control.. options).
+
+These sessions are configured through config file, see CONFIG FILE
+section with EXAMPLES.
+
+
+OPTIONS
+-------
+--config=<PATH>::
+	Config file path, if not perf will check system and default
+	locations (/etc/perfconfig, $HOME/.perfconfig).
+
+-v::
+--verbose::
+	Be more verbose.
+
+
+All generic options are available also under commands.
+
+
+START COMMAND
+-------------
+The start command creates the daemon process.
+
+-f::
+--foreground::
+	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.
+
+--session::
+	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
+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.
+
+
+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.26.2


^ permalink raw reply related	[flat|nested] 44+ messages in thread

* [PATCH 18/22] perf test: Add daemon list command test
  2021-01-02 22:04 [PATCHv2 00/22] perf tools: Add daemon command Jiri Olsa
                   ` (16 preceding siblings ...)
  2021-01-02 22:04 ` [PATCH 17/22] perf daemon: Add man page for perf-daemon Jiri Olsa
@ 2021-01-02 22:04 ` Jiri Olsa
  2021-01-02 22:04 ` [PATCH 19/22] perf test: Add daemon reconfig test Jiri Olsa
                   ` (4 subsequent siblings)
  22 siblings, 0 replies; 44+ messages in thread
From: Jiri Olsa @ 2021-01-02 22:04 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo
  Cc: lkml, Peter Zijlstra, Ingo Molnar, Mark Rutland, Namhyung Kim,
	Alexander Shishkin, Michael Petlan, Ian Rogers, Stephane Eranian,
	Alexei Budankov

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..52abd2f015ed
--- /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.26.2


^ permalink raw reply related	[flat|nested] 44+ messages in thread

* [PATCH 19/22] perf test: Add daemon reconfig test
  2021-01-02 22:04 [PATCHv2 00/22] perf tools: Add daemon command Jiri Olsa
                   ` (17 preceding siblings ...)
  2021-01-02 22:04 ` [PATCH 18/22] perf test: Add daemon list command test Jiri Olsa
@ 2021-01-02 22:04 ` Jiri Olsa
  2021-01-02 22:04 ` [PATCH 20/22] perf test: Add daemon stop command test Jiri Olsa
                   ` (3 subsequent siblings)
  22 siblings, 0 replies; 44+ messages in thread
From: Jiri Olsa @ 2021-01-02 22:04 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo
  Cc: lkml, Peter Zijlstra, Ingo Molnar, Mark Rutland, Namhyung Kim,
	Alexander Shishkin, Michael Petlan, Ian Rogers, Stephane Eranian,
	Alexei Budankov

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 | 70 ++++++++++++++++++++++++++++++++
 1 file changed, 70 insertions(+)

diff --git a/tools/perf/tests/shell/daemon.sh b/tools/perf/tests/shell/daemon.sh
index 52abd2f015ed..8360f61973e2 100755
--- a/tools/perf/tests/shell/daemon.sh
+++ b/tools/perf/tests/shell/daemon.sh
@@ -177,8 +177,78 @@ 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
+
+	# 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"
+
+	# stop daemon
+	daemon_exit ${base} ${config}
+
+	rm -rf ${base}
+	rm -f ${config}
+}
 error=0
 
 test_list
+test_reconfig
 
 exit ${error}
-- 
2.26.2


^ permalink raw reply related	[flat|nested] 44+ messages in thread

* [PATCH 20/22] perf test: Add daemon stop command test
  2021-01-02 22:04 [PATCHv2 00/22] perf tools: Add daemon command Jiri Olsa
                   ` (18 preceding siblings ...)
  2021-01-02 22:04 ` [PATCH 19/22] perf test: Add daemon reconfig test Jiri Olsa
@ 2021-01-02 22:04 ` Jiri Olsa
  2021-01-02 22:04 ` [PATCH 21/22] perf test: Add daemon signal " Jiri Olsa
                   ` (2 subsequent siblings)
  22 siblings, 0 replies; 44+ messages in thread
From: Jiri Olsa @ 2021-01-02 22:04 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo
  Cc: lkml, Peter Zijlstra, Ingo Molnar, Mark Rutland, Namhyung Kim,
	Alexander Shishkin, Michael Petlan, Ian Rogers, Stephane Eranian,
	Alexei Budankov

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 8360f61973e2..d86239c21f5b 100755
--- a/tools/perf/tests/shell/daemon.sh
+++ b/tools/perf/tests/shell/daemon.sh
@@ -246,9 +246,63 @@ EOF
 	rm -rf ${base}
 	rm -f ${config}
 }
+
+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.26.2


^ permalink raw reply related	[flat|nested] 44+ messages in thread

* [PATCH 21/22] perf test: Add daemon signal command test
  2021-01-02 22:04 [PATCHv2 00/22] perf tools: Add daemon command Jiri Olsa
                   ` (19 preceding siblings ...)
  2021-01-02 22:04 ` [PATCH 20/22] perf test: Add daemon stop command test Jiri Olsa
@ 2021-01-02 22:04 ` Jiri Olsa
  2021-01-02 22:04 ` [PATCH 22/22] perf test: Add daemon ping " Jiri Olsa
  2021-01-18 12:55 ` [PATCHv2 00/22] perf tools: Add daemon command Jiri Olsa
  22 siblings, 0 replies; 44+ messages in thread
From: Jiri Olsa @ 2021-01-02 22:04 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo
  Cc: lkml, Peter Zijlstra, Ingo Molnar, Mark Rutland, Namhyung Kim,
	Alexander Shishkin, Michael Petlan, Ian Rogers, Stephane Eranian,
	Alexei Budankov

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 d86239c21f5b..792a7a846830 100755
--- a/tools/perf/tests/shell/daemon.sh
+++ b/tools/perf/tests/shell/daemon.sh
@@ -299,10 +299,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.26.2


^ permalink raw reply related	[flat|nested] 44+ messages in thread

* [PATCH 22/22] perf test: Add daemon ping command test
  2021-01-02 22:04 [PATCHv2 00/22] perf tools: Add daemon command Jiri Olsa
                   ` (20 preceding siblings ...)
  2021-01-02 22:04 ` [PATCH 21/22] perf test: Add daemon signal " Jiri Olsa
@ 2021-01-02 22:04 ` Jiri Olsa
  2021-01-18 12:55 ` [PATCHv2 00/22] perf tools: Add daemon command Jiri Olsa
  22 siblings, 0 replies; 44+ messages in thread
From: Jiri Olsa @ 2021-01-02 22:04 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo
  Cc: lkml, Peter Zijlstra, Ingo Molnar, Mark Rutland, Namhyung Kim,
	Alexander Shishkin, Michael Petlan, Ian Rogers, Stephane Eranian,
	Alexei Budankov

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 792a7a846830..242470ff8d06 100755
--- a/tools/perf/tests/shell/daemon.sh
+++ b/tools/perf/tests/shell/daemon.sh
@@ -338,11 +338,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.26.2


^ permalink raw reply related	[flat|nested] 44+ messages in thread

* Re: [PATCHv2 00/22] perf tools: Add daemon command
  2021-01-02 22:04 [PATCHv2 00/22] perf tools: Add daemon command Jiri Olsa
                   ` (21 preceding siblings ...)
  2021-01-02 22:04 ` [PATCH 22/22] perf test: Add daemon ping " Jiri Olsa
@ 2021-01-18 12:55 ` Jiri Olsa
  22 siblings, 0 replies; 44+ messages in thread
From: Jiri Olsa @ 2021-01-18 12:55 UTC (permalink / raw)
  To: Jiri Olsa
  Cc: Arnaldo Carvalho de Melo, lkml, Peter Zijlstra, Ingo Molnar,
	Mark Rutland, Namhyung Kim, Alexander Shishkin, Michael Petlan,
	Ian Rogers, Stephane Eranian, Alexei Budankov

ping, any reviews?

thanks,
jirka

On Sat, Jan 02, 2021 at 11:04:19PM +0100, Jiri Olsa wrote:
> 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.
> 
> The patchset is based on following control changes:
>   https://lore.kernel.org/lkml/20201226232038.390883-1-jolsa@kernel.org/
> 
> Available also here:
>   git://git.kernel.org/pub/scm/linux/kernel/git/jolsa/perf.git
>   perf/daemon
> 
> 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 (22):
>       perf tools: Make perf_config_from_file static
>       perf tools: Add config set interface
>       perf tools: Add debug_set_display_time function
>       perf tools: Add perf_home_perfconfig function
>       perf tools: Make perf_config_system global
>       perf tools: Make perf_config_global gobal
>       perf daemon: Add daemon command
>       perf daemon: Add config file change check
>       perf daemon: Add signalfd support
>       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 man page for perf-daemon
>       perf test: Add daemon list command test
>       perf test: Add daemon reconfig test
>       perf test: Add daemon stop command test
>       perf test: Add daemon signal command test
>       perf test: Add daemon ping command test
> 
>  tools/perf/Build                         |    1 +
>  tools/perf/Documentation/perf-config.txt |   14 ++
>  tools/perf/Documentation/perf-daemon.txt |  187 ++++++++++++++++
>  tools/perf/builtin-daemon.c              | 1327 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
>  tools/perf/builtin.h                     |    1 +
>  tools/perf/command-list.txt              |    1 +
>  tools/perf/perf.c                        |    1 +
>  tools/perf/tests/shell/daemon.sh         |  388 ++++++++++++++++++++++++++++++++
>  tools/perf/util/config.c                 |  123 +++++++----
>  tools/perf/util/config.h                 |    7 +-
>  tools/perf/util/debug.c                  |   34 ++-
>  tools/perf/util/debug.h                  |    1 +
>  12 files changed, 2037 insertions(+), 48 deletions(-)
>  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
> 


^ permalink raw reply	[flat|nested] 44+ messages in thread

* Re: [PATCH 01/22] perf tools: Make perf_config_from_file static
  2021-01-02 22:04 ` [PATCH 01/22] perf tools: Make perf_config_from_file static Jiri Olsa
@ 2021-01-18 15:51   ` Arnaldo Carvalho de Melo
  0 siblings, 0 replies; 44+ messages in thread
From: Arnaldo Carvalho de Melo @ 2021-01-18 15:51 UTC (permalink / raw)
  To: Jiri Olsa
  Cc: lkml, Peter Zijlstra, Ingo Molnar, Mark Rutland, Namhyung Kim,
	Alexander Shishkin, Michael Petlan, Ian Rogers, Stephane Eranian,
	Alexei Budankov

Em Sat, Jan 02, 2021 at 11:04:20PM +0100, Jiri Olsa escreveu:
> It's not used outside config.c object.

Thanks, applied.
 
> Signed-off-by: Jiri Olsa <jolsa@kernel.org>
> ---
>  tools/perf/util/config.c | 2 +-
>  tools/perf/util/config.h | 1 -
>  2 files changed, 1 insertion(+), 2 deletions(-)
> 
> diff --git a/tools/perf/util/config.c b/tools/perf/util/config.c
> index 6969f82843ee..20be0504fb95 100644
> --- a/tools/perf/util/config.c
> +++ b/tools/perf/util/config.c
> @@ -489,7 +489,7 @@ int perf_default_config(const char *var, const char *value,
>  	return 0;
>  }
>  
> -int perf_config_from_file(config_fn_t fn, const char *filename, void *data)
> +static int perf_config_from_file(config_fn_t fn, const char *filename, void *data)
>  {
>  	int ret;
>  	FILE *f = fopen(filename, "r");
> diff --git a/tools/perf/util/config.h b/tools/perf/util/config.h
> index 8c881e3a3ec3..2f753b2a034b 100644
> --- a/tools/perf/util/config.h
> +++ b/tools/perf/util/config.h
> @@ -27,7 +27,6 @@ extern const char *config_exclusive_filename;
>  
>  typedef int (*config_fn_t)(const char *, const char *, void *);
>  
> -int perf_config_from_file(config_fn_t fn, const char *filename, void *data);
>  int perf_default_config(const char *, const char *, void *);
>  int perf_config(config_fn_t fn, void *);
>  int perf_config_int(int *dest, const char *, const char *);
> -- 
> 2.26.2
> 

-- 

- Arnaldo

^ permalink raw reply	[flat|nested] 44+ messages in thread

* Re: [PATCH 02/22] perf tools: Add config set interface
  2021-01-02 22:04 ` [PATCH 02/22] perf tools: Add config set interface Jiri Olsa
@ 2021-01-18 15:53   ` Arnaldo Carvalho de Melo
  0 siblings, 0 replies; 44+ messages in thread
From: Arnaldo Carvalho de Melo @ 2021-01-18 15:53 UTC (permalink / raw)
  To: Jiri Olsa
  Cc: lkml, Peter Zijlstra, Ingo Molnar, Mark Rutland, Namhyung Kim,
	Alexander Shishkin, Michael Petlan, Ian Rogers, Stephane Eranian,
	Alexei Budankov

Em Sat, Jan 02, 2021 at 11:04:21PM +0100, Jiri Olsa escreveu:
> Add interface to load config set from custom file
> by using perf_config_set__load_file function.
> 
> It will be used in perf daemon command to process
> custom config file.

Thanks, applied.

- Arnaldo

 
> Signed-off-by: Jiri Olsa <jolsa@kernel.org>
> ---
>  tools/perf/util/config.c | 28 +++++++++++++++++++++++-----
>  tools/perf/util/config.h |  3 +++
>  2 files changed, 26 insertions(+), 5 deletions(-)
> 
> diff --git a/tools/perf/util/config.c b/tools/perf/util/config.c
> index 20be0504fb95..222cb2e2de25 100644
> --- a/tools/perf/util/config.c
> +++ b/tools/perf/util/config.c
> @@ -738,6 +738,18 @@ struct perf_config_set *perf_config_set__new(void)
>  	return set;
>  }
>  
> +struct perf_config_set *perf_config_set__load_file(const char *file)
> +{
> +	struct perf_config_set *set = zalloc(sizeof(*set));
> +
> +	if (set) {
> +		INIT_LIST_HEAD(&set->sections);
> +		perf_config_from_file(collect_config, file, set);
> +	}
> +
> +	return set;
> +}
> +
>  static int perf_config__init(void)
>  {
>  	if (config_set == NULL)
> @@ -746,17 +758,15 @@ static int perf_config__init(void)
>  	return config_set == NULL;
>  }
>  
> -int perf_config(config_fn_t fn, void *data)
> +int perf_config_set(struct perf_config_set *set,
> +		    config_fn_t fn, void *data)
>  {
>  	int ret = 0;
>  	char key[BUFSIZ];
>  	struct perf_config_section *section;
>  	struct perf_config_item *item;
>  
> -	if (config_set == NULL && perf_config__init())
> -		return -1;
> -
> -	perf_config_set__for_each_entry(config_set, section, item) {
> +	perf_config_set__for_each_entry(set, section, item) {
>  		char *value = item->value;
>  
>  		if (value) {
> @@ -778,6 +788,14 @@ int perf_config(config_fn_t fn, void *data)
>  	return ret;
>  }
>  
> +int perf_config(config_fn_t fn, void *data)
> +{
> +	if (config_set == NULL && perf_config__init())
> +		return -1;
> +
> +	return perf_config_set(config_set, fn, data);
> +}
> +
>  void perf_config__exit(void)
>  {
>  	perf_config_set__delete(config_set);
> diff --git a/tools/perf/util/config.h b/tools/perf/util/config.h
> index 2f753b2a034b..ee5a242446e9 100644
> --- a/tools/perf/util/config.h
> +++ b/tools/perf/util/config.h
> @@ -29,6 +29,8 @@ typedef int (*config_fn_t)(const char *, const char *, void *);
>  
>  int perf_default_config(const char *, const char *, void *);
>  int perf_config(config_fn_t fn, void *);
> +int perf_config_set(struct perf_config_set *set,
> +		    config_fn_t fn, void *data);
>  int perf_config_int(int *dest, const char *, const char *);
>  int perf_config_u8(u8 *dest, const char *name, const char *value);
>  int perf_config_u64(u64 *dest, const char *, const char *);
> @@ -37,6 +39,7 @@ int config_error_nonbool(const char *);
>  const char *perf_etc_perfconfig(void);
>  
>  struct perf_config_set *perf_config_set__new(void);
> +struct perf_config_set *perf_config_set__load_file(const char *file);
>  void perf_config_set__delete(struct perf_config_set *set);
>  int perf_config_set__collect(struct perf_config_set *set, const char *file_name,
>  			     const char *var, const char *value);
> -- 
> 2.26.2
> 

-- 

- Arnaldo

^ permalink raw reply	[flat|nested] 44+ messages in thread

* Re: [PATCH 03/22] perf tools: Add debug_set_display_time function
  2021-01-02 22:04 ` [PATCH 03/22] perf tools: Add debug_set_display_time function Jiri Olsa
@ 2021-01-18 16:02   ` Arnaldo Carvalho de Melo
  2021-01-19 14:59   ` Arnaldo Carvalho de Melo
  1 sibling, 0 replies; 44+ messages in thread
From: Arnaldo Carvalho de Melo @ 2021-01-18 16:02 UTC (permalink / raw)
  To: Jiri Olsa
  Cc: lkml, Peter Zijlstra, Ingo Molnar, Mark Rutland, Namhyung Kim,
	Alexander Shishkin, Michael Petlan, Ian Rogers, Stephane Eranian,
	Alexei Budankov

Em Sat, Jan 02, 2021 at 11:04:22PM +0100, Jiri Olsa escreveu:
> Allow to display time in perf debug output via new
> debug_set_display_time function.
> 
> It will be used in perf daemon command to get verbose
> output into log file.
> 
> The debug time format is:
> 
>   [2020-12-03 18:25:31.822152] affinity: SYS
>   [2020-12-03 18:25:31.822164] mmap flush: 1
>   [2020-12-03 18:25:31.822175] comp level: 0
>   [2020-12-03 18:25:32.002047] mmap size 528384B

Thanks, applied.

- Arnaldo

 
> Signed-off-by: Jiri Olsa <jolsa@kernel.org>
> ---
>  tools/perf/util/debug.c | 34 +++++++++++++++++++++++++++++++---
>  tools/perf/util/debug.h |  1 +
>  2 files changed, 32 insertions(+), 3 deletions(-)
> 
> diff --git a/tools/perf/util/debug.c b/tools/perf/util/debug.c
> index 50fd6a4be4e0..dc148b08814b 100644
> --- a/tools/perf/util/debug.c
> +++ b/tools/perf/util/debug.c
> @@ -10,6 +10,7 @@
>  #include <api/debug.h>
>  #include <linux/kernel.h>
>  #include <linux/time64.h>
> +#include <sys/time.h>
>  #ifdef HAVE_BACKTRACE_SUPPORT
>  #include <execinfo.h>
>  #endif
> @@ -31,21 +32,48 @@ int debug_ordered_events;
>  static int redirect_to_stderr;
>  int debug_data_convert;
>  static FILE *debug_file;
> +bool debug_display_time;
>  
>  void debug_set_file(FILE *file)
>  {
>  	debug_file = file;
>  }
>  
> +void debug_set_display_time(bool set)
> +{
> +	debug_display_time = set;
> +}
> +
> +static int fprintf_time(FILE *file)
> +{
> +	struct timeval tod;
> +	struct tm ltime;
> +	char date[64];
> +
> +	if (!debug_display_time)
> +		return 0;
> +
> +	if (gettimeofday(&tod, NULL) != 0)
> +		return 0;
> +
> +	if (localtime_r(&tod.tv_sec, &ltime) == NULL)
> +		return 0;
> +
> +	strftime(date, sizeof(date),  "%F %H:%M:%S", &ltime);
> +	return fprintf(file, "[%s.%06lu] ", date, tod.tv_usec);
> +}
> +
>  int veprintf(int level, int var, const char *fmt, va_list args)
>  {
>  	int ret = 0;
>  
>  	if (var >= level) {
> -		if (use_browser >= 1 && !redirect_to_stderr)
> +		if (use_browser >= 1 && !redirect_to_stderr) {
>  			ui_helpline__vshow(fmt, args);
> -		else
> -			ret = vfprintf(debug_file, fmt, args);
> +		} else {
> +			ret = fprintf_time(debug_file);
> +			ret += vfprintf(debug_file, fmt, args);
> +		}
>  	}
>  
>  	return ret;
> diff --git a/tools/perf/util/debug.h b/tools/perf/util/debug.h
> index 43f712295645..48f631966067 100644
> --- a/tools/perf/util/debug.h
> +++ b/tools/perf/util/debug.h
> @@ -64,6 +64,7 @@ int veprintf(int level, int var, const char *fmt, va_list args);
>  
>  int perf_debug_option(const char *str);
>  void debug_set_file(FILE *file);
> +void debug_set_display_time(bool set);
>  void perf_debug_setup(void);
>  int perf_quiet_option(void);
>  
> -- 
> 2.26.2
> 

-- 

- Arnaldo

^ permalink raw reply	[flat|nested] 44+ messages in thread

* Re: [PATCH 05/22] perf tools: Make perf_config_system global
  2021-01-02 22:04 ` [PATCH 05/22] perf tools: Make perf_config_system global Jiri Olsa
@ 2021-01-18 16:03   ` Arnaldo Carvalho de Melo
  0 siblings, 0 replies; 44+ messages in thread
From: Arnaldo Carvalho de Melo @ 2021-01-18 16:03 UTC (permalink / raw)
  To: Jiri Olsa
  Cc: lkml, Peter Zijlstra, Ingo Molnar, Mark Rutland, Namhyung Kim,
	Alexander Shishkin, Michael Petlan, Ian Rogers, Stephane Eranian,
	Alexei Budankov

Em Sat, Jan 02, 2021 at 11:04:24PM +0100, Jiri Olsa escreveu:
> Making perf_config_system global, it will be used
> outside the config.c object in following patches.

Thanks, applied.

- Arnaldo

 
> Signed-off-by: Jiri Olsa <jolsa@kernel.org>
> ---
>  tools/perf/util/config.c | 2 +-
>  tools/perf/util/config.h | 1 +
>  2 files changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/tools/perf/util/config.c b/tools/perf/util/config.c
> index 34fe80ccdad1..4e0455a6bb5f 100644
> --- a/tools/perf/util/config.c
> +++ b/tools/perf/util/config.c
> @@ -521,7 +521,7 @@ static int perf_env_bool(const char *k, int def)
>  	return v ? perf_config_bool(k, v) : def;
>  }
>  
> -static int perf_config_system(void)
> +int perf_config_system(void)
>  {
>  	return !perf_env_bool("PERF_CONFIG_NOSYSTEM", 0);
>  }
> diff --git a/tools/perf/util/config.h b/tools/perf/util/config.h
> index d6c4f80f367c..bf68e4acea73 100644
> --- a/tools/perf/util/config.h
> +++ b/tools/perf/util/config.h
> @@ -38,6 +38,7 @@ int perf_config_bool(const char *, const char *);
>  int config_error_nonbool(const char *);
>  const char *perf_etc_perfconfig(void);
>  const char *perf_home_perfconfig(void);
> +int perf_config_system(void);
>  
>  struct perf_config_set *perf_config_set__new(void);
>  struct perf_config_set *perf_config_set__load_file(const char *file);
> -- 
> 2.26.2
> 

-- 

- Arnaldo

^ permalink raw reply	[flat|nested] 44+ messages in thread

* Re: [PATCH 04/22] perf tools: Add perf_home_perfconfig function
  2021-01-02 22:04 ` [PATCH 04/22] perf tools: Add perf_home_perfconfig function Jiri Olsa
@ 2021-01-18 16:05   ` Arnaldo Carvalho de Melo
  0 siblings, 0 replies; 44+ messages in thread
From: Arnaldo Carvalho de Melo @ 2021-01-18 16:05 UTC (permalink / raw)
  To: Jiri Olsa
  Cc: lkml, Peter Zijlstra, Ingo Molnar, Mark Rutland, Namhyung Kim,
	Alexander Shishkin, Michael Petlan, Ian Rogers, Stephane Eranian,
	Alexei Budankov

Em Sat, Jan 02, 2021 at 11:04:23PM +0100, Jiri Olsa escreveu:
> Factor out the perf_home_perfconfig, that looks for
> .perfconfig in home directory including check for
> PERF_CONFIG_NOGLOBAL and for proper permission.

Thanks, applied.

- Arnaldo

 
> Signed-off-by: Jiri Olsa <jolsa@kernel.org>
> ---
>  tools/perf/util/config.c | 89 ++++++++++++++++++++++++----------------
>  tools/perf/util/config.h |  1 +
>  2 files changed, 54 insertions(+), 36 deletions(-)
> 
> diff --git a/tools/perf/util/config.c b/tools/perf/util/config.c
> index 222cb2e2de25..34fe80ccdad1 100644
> --- a/tools/perf/util/config.c
> +++ b/tools/perf/util/config.c
> @@ -531,6 +531,56 @@ static int perf_config_global(void)
>  	return !perf_env_bool("PERF_CONFIG_NOGLOBAL", 0);
>  }
>  
> +static char *home_perfconfig(void)
> +{
> +	const char *home = NULL;
> +	char *config;
> +	struct stat st;
> +
> +	home = getenv("HOME");
> +
> +	/*
> +	 * Skip reading user config if:
> +	 *   - there is no place to read it from (HOME)
> +	 *   - we are asked not to (PERF_CONFIG_NOGLOBAL=1)
> +	 */
> +	if (!home || !*home || !perf_config_global())
> +		return NULL;
> +
> +	config = strdup(mkpath("%s/.perfconfig", home));
> +	if (config == NULL) {
> +		pr_warning("Not enough memory to process %s/.perfconfig, ignoring it.", home);
> +		return NULL;
> +	}
> +
> +	if (stat(config, &st) < 0)
> +		goto out_free;
> +
> +	if (st.st_uid && (st.st_uid != geteuid())) {
> +		pr_warning("File %s not owned by current user or root, ignoring it.", config);
> +		goto out_free;
> +	}
> +
> +	if (st.st_size)
> +		return config;
> +
> +out_free:
> +	free(config);
> +	return NULL;
> +}
> +
> +const char *perf_home_perfconfig(void)
> +{
> +	static const char *config;
> +	static bool failed;
> +
> +	config = failed ? NULL : home_perfconfig();
> +	if (!config)
> +		failed = true;
> +
> +	return config;
> +}
> +
>  static struct perf_config_section *find_section(struct list_head *sections,
>  						const char *section_name)
>  {
> @@ -676,9 +726,6 @@ int perf_config_set__collect(struct perf_config_set *set, const char *file_name,
>  static int perf_config_set__init(struct perf_config_set *set)
>  {
>  	int ret = -1;
> -	const char *home = NULL;
> -	char *user_config;
> -	struct stat st;
>  
>  	/* Setting $PERF_CONFIG makes perf read _only_ the given config file. */
>  	if (config_exclusive_filename)
> @@ -687,41 +734,11 @@ static int perf_config_set__init(struct perf_config_set *set)
>  		if (perf_config_from_file(collect_config, perf_etc_perfconfig(), set) < 0)
>  			goto out;
>  	}
> -
> -	home = getenv("HOME");
> -
> -	/*
> -	 * Skip reading user config if:
> -	 *   - there is no place to read it from (HOME)
> -	 *   - we are asked not to (PERF_CONFIG_NOGLOBAL=1)
> -	 */
> -	if (!home || !*home || !perf_config_global())
> -		return 0;
> -
> -	user_config = strdup(mkpath("%s/.perfconfig", home));
> -	if (user_config == NULL) {
> -		pr_warning("Not enough memory to process %s/.perfconfig, ignoring it.", home);
> -		goto out;
> -	}
> -
> -	if (stat(user_config, &st) < 0) {
> -		if (errno == ENOENT)
> -			ret = 0;
> -		goto out_free;
> -	}
> -
> -	ret = 0;
> -
> -	if (st.st_uid && (st.st_uid != geteuid())) {
> -		pr_warning("File %s not owned by current user or root, ignoring it.", user_config);
> -		goto out_free;
> +	if (perf_config_global() && perf_home_perfconfig()) {
> +		if (perf_config_from_file(collect_config, perf_home_perfconfig(), set) < 0)
> +			goto out;
>  	}
>  
> -	if (st.st_size)
> -		ret = perf_config_from_file(collect_config, user_config, set);
> -
> -out_free:
> -	free(user_config);
>  out:
>  	return ret;
>  }
> diff --git a/tools/perf/util/config.h b/tools/perf/util/config.h
> index ee5a242446e9..d6c4f80f367c 100644
> --- a/tools/perf/util/config.h
> +++ b/tools/perf/util/config.h
> @@ -37,6 +37,7 @@ int perf_config_u64(u64 *dest, const char *, const char *);
>  int perf_config_bool(const char *, const char *);
>  int config_error_nonbool(const char *);
>  const char *perf_etc_perfconfig(void);
> +const char *perf_home_perfconfig(void);
>  
>  struct perf_config_set *perf_config_set__new(void);
>  struct perf_config_set *perf_config_set__load_file(const char *file);
> -- 
> 2.26.2
> 

-- 

- Arnaldo

^ permalink raw reply	[flat|nested] 44+ messages in thread

* Re: [PATCH 06/22] perf tools: Make perf_config_global gobal
  2021-01-02 22:04 ` [PATCH 06/22] perf tools: Make perf_config_global gobal Jiri Olsa
@ 2021-01-18 16:05   ` Arnaldo Carvalho de Melo
  0 siblings, 0 replies; 44+ messages in thread
From: Arnaldo Carvalho de Melo @ 2021-01-18 16:05 UTC (permalink / raw)
  To: Jiri Olsa
  Cc: lkml, Peter Zijlstra, Ingo Molnar, Mark Rutland, Namhyung Kim,
	Alexander Shishkin, Michael Petlan, Ian Rogers, Stephane Eranian,
	Alexei Budankov

Em Sat, Jan 02, 2021 at 11:04:25PM +0100, Jiri Olsa escreveu:
> Making perf_config_global global, it will be used
> outside the config.c object in following patches.

Thanks, applied.

- Arnaldo

 
> Signed-off-by: Jiri Olsa <jolsa@kernel.org>
> ---
>  tools/perf/util/config.c | 2 +-
>  tools/perf/util/config.h | 1 +
>  2 files changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/tools/perf/util/config.c b/tools/perf/util/config.c
> index 4e0455a6bb5f..6984c77068a3 100644
> --- a/tools/perf/util/config.c
> +++ b/tools/perf/util/config.c
> @@ -526,7 +526,7 @@ int perf_config_system(void)
>  	return !perf_env_bool("PERF_CONFIG_NOSYSTEM", 0);
>  }
>  
> -static int perf_config_global(void)
> +int perf_config_global(void)
>  {
>  	return !perf_env_bool("PERF_CONFIG_NOGLOBAL", 0);
>  }
> diff --git a/tools/perf/util/config.h b/tools/perf/util/config.h
> index bf68e4acea73..2fd77aaff4d2 100644
> --- a/tools/perf/util/config.h
> +++ b/tools/perf/util/config.h
> @@ -39,6 +39,7 @@ int config_error_nonbool(const char *);
>  const char *perf_etc_perfconfig(void);
>  const char *perf_home_perfconfig(void);
>  int perf_config_system(void);
> +int perf_config_global(void);
>  
>  struct perf_config_set *perf_config_set__new(void);
>  struct perf_config_set *perf_config_set__load_file(const char *file);
> -- 
> 2.26.2
> 

-- 

- Arnaldo

^ permalink raw reply	[flat|nested] 44+ messages in thread

* Re: [PATCH 07/22] perf daemon: Add daemon command
  2021-01-02 22:04 ` [PATCH 07/22] perf daemon: Add daemon command Jiri Olsa
@ 2021-01-19  4:08   ` Namhyung Kim
  2021-01-19 18:31     ` Jiri Olsa
  2021-01-27  7:09   ` Namhyung Kim
  1 sibling, 1 reply; 44+ messages in thread
From: Namhyung Kim @ 2021-01-19  4:08 UTC (permalink / raw)
  To: Jiri Olsa
  Cc: Arnaldo Carvalho de Melo, lkml, Peter Zijlstra, Ingo Molnar,
	Mark Rutland, Alexander Shishkin, Michael Petlan, Ian Rogers,
	Stephane Eranian, Alexei Budankov

Hi Jiri,

On Sun, Jan 3, 2021 at 7:05 AM Jiri Olsa <jolsa@kernel.org> wrote:
>
> Adding daemon command that allows to run record sessions
> on background. Each session represents one perf record
> process and is configured in config file.
>
> 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
>
> Check 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
>
> Check 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.
>
> Adding empty perf-daemon.txt to skip compile warning,
> the man page is populated in following patch.
>
> Signed-off-by: Jiri Olsa <jolsa@kernel.org>
> ---
[SNIP]
> +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) {
> +               session = daemon__add_session(daemon, name);
> +               if (!session)
> +                       return -ENOMEM;
> +
> +               pr_debug("reconfig: found new session %s\n", name);
> +               /* This is new session, trigger reconfig to start it. */
> +               session->state = SESSION_STATE__RECONFIG;
> +       } else if (session->state == SESSION_STATE__KILL) {
> +               /*
> +                * The session was marked to kill and we still
> +                * found it in config file.
> +                */
> +               pr_debug("reconfig: found current session %s\n", name);
> +               session->state = SESSION_STATE__OK;
> +       }
> +
> +       if (!strcmp(var, "run")) {
> +               if (session->run && strcmp(session->run, value)) {
> +                       free(session->run);
> +                       pr_debug("reconfig: session %s is changed\n", name);
> +                       session->state = SESSION_STATE__RECONFIG;
> +               }
> +               session->run = strdup(value);

Please check the result.

> +       }
> +
> +       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 = strdup(value);

It seems these config items are mandatory.  Is there a check
for their presence?


> +
> +       return 0;
> +}
> +
> +static int client_config(const char *var, const char *value, void *cb)
> +{
> +       struct daemon *daemon = cb;
> +
> +       if (!strcmp(var, "daemon.base"))
> +               daemon->base = strdup(value);
> +
> +       return 0;
> +}
> +
> +static int setup_client_config(struct daemon *daemon)
> +{
> +       struct perf_config_set *set;
> +       int err = -ENOMEM;
> +
> +       set = perf_config_set__load_file(daemon->config_real);
> +       if (set) {
> +               err = perf_config_set(set, client_config, daemon);
> +               perf_config_set__delete(set);
> +       }
> +
> +       return err;
> +}
> +
> +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 session for kill, the server config will
> +        * set proper state for found sessions.
> +        */
> +       list_for_each_entry(session, &daemon->sessions, list)
> +               session->state = SESSION_STATE__KILL;

Probably we can put them in a different state like INIT or READY?

> +
> +       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;
> +}
> +
> +static int session__signal(struct session *session, int sig)
> +{
> +       if (session->pid < 0)
> +               return -1;
> +       return kill(session->pid, sig);
> +}
> +
> +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("asprintf failed");
> +               return -1;
> +       }
> +
> +       if (mkdir(session->base, 0755) && errno != EEXIST) {
> +               perror("mkdir failed");
> +               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("chdir failed");
> +               return -1;
> +       }
> +
> +       fd = open("/dev/null", O_RDONLY);
> +       if (fd < 0) {
> +               perror("failed to open /dev/null");
> +               return -1;
> +       }
> +
> +       close(0);
> +       dup2(fd, 0);

The man page says dup2() will close the second file descriptor.

> +       close(fd);
> +
> +       fd = open(SESSION_OUTPUT, O_RDWR|O_CREAT|O_TRUNC, 0644);
> +       if (fd < 0) {
> +               perror("failed to open session output");
> +               return -1;
> +       }
> +
> +       close(1);
> +       close(2);
> +       dup2(fd, 1);
> +       dup2(fd, 2);

Ditto.

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

[SNIP]
> +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. */
> +               pr_debug2("reconfig: session '%s' start\n", session->name);
> +               if (session->pid > 0) {
> +                       session__kill(session);
> +                       pr_info("reconfig: session '%s' killed\n", session->name);
> +               }
> +               if (session__run(session, daemon))

Does it call a config function?  Or is it called already?

> +                       return -1;
> +               pr_debug2("reconfig: session '%s' done\n", session->name);
> +               session->state = SESSION_STATE__OK;

I think RUNNING is a better name.

Thanks,
Namhyung


> +       }
> +
> +       return 0;
> +}
> +

^ permalink raw reply	[flat|nested] 44+ messages in thread

* Re: [PATCH 08/22] perf daemon: Add config file change check
  2021-01-02 22:04 ` [PATCH 08/22] perf daemon: Add config file change check Jiri Olsa
@ 2021-01-19  5:31   ` Namhyung Kim
  2021-01-19 17:49     ` Jiri Olsa
  0 siblings, 1 reply; 44+ messages in thread
From: Namhyung Kim @ 2021-01-19  5:31 UTC (permalink / raw)
  To: Jiri Olsa
  Cc: Arnaldo Carvalho de Melo, lkml, Peter Zijlstra, Ingo Molnar,
	Mark Rutland, Alexander Shishkin, Michael Petlan, Ian Rogers,
	Stephane Eranian, Alexei Budankov

On Sun, Jan 3, 2021 at 7:05 AM Jiri Olsa <jolsa@kernel.org> wrote:
>
> Adding support to detect daemon's config file changes
> and re-read the configuration when that happens.

Hmm.. maybe some of the code in the previous commit can be moved here.

Thanks,
Namhyung

>
> Using inotify file descriptor pluged 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>
> ---

^ permalink raw reply	[flat|nested] 44+ messages in thread

* Re: [PATCH 11/22] perf daemon: Add stop command
  2021-01-02 22:04 ` [PATCH 11/22] perf daemon: Add stop command Jiri Olsa
@ 2021-01-19  5:35   ` Namhyung Kim
  0 siblings, 0 replies; 44+ messages in thread
From: Namhyung Kim @ 2021-01-19  5:35 UTC (permalink / raw)
  To: Jiri Olsa
  Cc: Arnaldo Carvalho de Melo, lkml, Peter Zijlstra, Ingo Molnar,
	Mark Rutland, Alexander Shishkin, Michael Petlan, Ian Rogers,
	Stephane Eranian, Alexei Budankov

On Sun, Jan 3, 2021 at 7:05 AM Jiri Olsa <jolsa@kernel.org> wrote:
>
> 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/builtin-daemon.c | 28 ++++++++++++++++++++++++++++
>  1 file changed, 28 insertions(+)
>
> diff --git a/tools/perf/builtin-daemon.c b/tools/perf/builtin-daemon.c
> index a7ffbecf8d14..45748bb471ec 100644
> --- a/tools/perf/builtin-daemon.c
> +++ b/tools/perf/builtin-daemon.c
> @@ -524,6 +524,7 @@ static int setup_server_socket(struct daemon *daemon)
>  enum {
>         CMD_LIST   = 0,
>         CMD_SIGNAL = 1,
> +       CMD_STOP   = 2,
>         CMD_MAX,
>  };
>
> @@ -650,6 +651,10 @@ 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;
> +               pr_debug("perf daemon is exciting\n");

I can see how much you are excited. ;-)

Thanks,
Namhyung


> +               break;
>         default:
>                 break;
>         }
> @@ -992,6 +997,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[] = {
> @@ -1015,6 +1041,8 @@ int cmd_daemon(int argc, const char **argv)
>         if (argc) {
>                 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.26.2
>

^ permalink raw reply	[flat|nested] 44+ messages in thread

* Re: [PATCH 12/22] perf daemon: Allow only one daemon over base directory
  2021-01-02 22:04 ` [PATCH 12/22] perf daemon: Allow only one daemon over base directory Jiri Olsa
@ 2021-01-19  5:37   ` Namhyung Kim
  2021-01-19 17:44     ` Jiri Olsa
  0 siblings, 1 reply; 44+ messages in thread
From: Namhyung Kim @ 2021-01-19  5:37 UTC (permalink / raw)
  To: Jiri Olsa
  Cc: Arnaldo Carvalho de Melo, lkml, Peter Zijlstra, Ingo Molnar,
	Mark Rutland, Alexander Shishkin, Michael Petlan, Ian Rogers,
	Stephane Eranian, Alexei Budankov

On Sun, Jan 3, 2021 at 7:05 AM Jiri Olsa <jolsa@kernel.org> wrote:
>
> Add 'lock' file under daemon base and flock it, so only one
> perf daemon can run on top of it.
>
> Example:
>
>   # cat ~/.perfconfig
>   [daemon]
>   base=/opt/perfdata
>
>   [session-cycles]
>   run = -m 10M -e cycles --overwrite --switch-output -a
>
>   [session-sched]
>   run = -m 20M -e sched:* --overwrite --switch-output -a
>
> Starting the daemon:
>
>   # perf daemon start
>
> And try once more:
>
>   # perf daemon start
>   failed: another perf daemon (pid 775594) owns /opt/perfdata
>
> will end up with an error, because there's already one running
> on top of /opt/perfdata.
>
> Signed-off-by: Jiri Olsa <jolsa@kernel.org>
> ---
>  tools/perf/builtin-daemon.c | 49 +++++++++++++++++++++++++++++++++++++
>  1 file changed, 49 insertions(+)
>
> diff --git a/tools/perf/builtin-daemon.c b/tools/perf/builtin-daemon.c
> index 45748bb471ec..1982eedd3f3f 100644
> --- a/tools/perf/builtin-daemon.c
> +++ b/tools/perf/builtin-daemon.c
> @@ -8,6 +8,7 @@
>  #include <string.h>
>  #include <sys/types.h>
>  #include <sys/wait.h>
> +#include <sys/file.h>
>  #include <signal.h>
>  #include <stdlib.h>
>  #include <time.h>
> @@ -562,12 +563,18 @@ static int cmd_session_list(struct daemon *daemon, union cmd *cmd, FILE *out)
>                         /* output */
>                         csv_sep, daemon->base, SESSION_OUTPUT);
>
> +               fprintf(out, "%c%s/%s",
> +                       /* lock */
> +                       csv_sep, daemon->base, "lock");
> +
>                 fprintf(out, "\n");
>         } else {
>                 fprintf(out, "[%d:daemon] base: %s\n", getpid(), daemon->base);
>                 if (cmd->list.verbose) {
>                         fprintf(out, "  output:  %s/%s\n",
>                                 daemon->base, SESSION_OUTPUT);
> +                       fprintf(out, "  lock:    %s/lock\n",
> +                               daemon->base);
>                 }
>         }
>
> @@ -761,6 +768,42 @@ static int handle_config_changes(struct daemon *daemon, int conf_fd,
>         return 0;
>  }
>
> +static int check_lock(struct daemon *daemon)
> +{
> +       char path[PATH_MAX];
> +       char buf[20];
> +       int fd, pid;
> +       ssize_t len;
> +
> +       scnprintf(path, sizeof(path), "%s/lock", daemon->base);
> +
> +       fd = open(path, O_RDWR|O_CREAT|O_CLOEXEC, 0640);
> +       if (fd < 0)
> +               return -1;
> +
> +       if (lockf(fd, F_TLOCK, 0) < 0) {
> +               filename__read_int(path, &pid);
> +               fprintf(stderr, "failed: another perf daemon (pid %d) owns %s\n",
> +                       pid, daemon->base);
> +               return -1;
> +       }

So the fd is (a kind of) leaked and the lock is released only when
the daemon is going to die, right?

Thanks,
Namhyung

> +
> +       scnprintf(buf, sizeof(buf), "%d", getpid());
> +       len = strlen(buf);
> +
> +       if (write(fd, buf, len) != len) {
> +               perror("write failed");
> +               return -1;
> +       }
> +
> +       if (ftruncate(fd, len)) {
> +               perror("ftruncate failed");
> +               return -1;
> +       }
> +
> +       return 0;
> +}
> +
>  static int go_background(struct daemon *daemon)
>  {
>         int pid, fd;
> @@ -775,6 +818,9 @@ static int go_background(struct daemon *daemon)
>         if (setsid() < 0)
>                 return -1;
>
> +       if (check_lock(daemon))
> +               return -1;
> +
>         umask(0);
>
>         if (chdir(daemon->base)) {
> @@ -861,6 +907,9 @@ static int __cmd_start(struct daemon *daemon, struct option parent_options[],
>         if (setup_server_config(daemon))
>                 return -1;
>
> +       if (foreground && check_lock(daemon))
> +               return -1;
> +
>         if (!foreground && go_background(daemon))
>                 return -1;
>
> --
> 2.26.2
>

^ permalink raw reply	[flat|nested] 44+ messages in thread

* Re: [PATCH 03/22] perf tools: Add debug_set_display_time function
  2021-01-02 22:04 ` [PATCH 03/22] perf tools: Add debug_set_display_time function Jiri Olsa
  2021-01-18 16:02   ` Arnaldo Carvalho de Melo
@ 2021-01-19 14:59   ` Arnaldo Carvalho de Melo
  2021-01-19 17:39     ` Jiri Olsa
  1 sibling, 1 reply; 44+ messages in thread
From: Arnaldo Carvalho de Melo @ 2021-01-19 14:59 UTC (permalink / raw)
  To: Jiri Olsa
  Cc: lkml, Peter Zijlstra, Ingo Molnar, Mark Rutland, Namhyung Kim,
	Alexander Shishkin, Michael Petlan, Ian Rogers, Stephane Eranian,
	Alexei Budankov

Em Sat, Jan 02, 2021 at 11:04:22PM +0100, Jiri Olsa escreveu:
> Allow to display time in perf debug output via new
> debug_set_display_time function.
> 
> It will be used in perf daemon command to get verbose
> output into log file.
> 
> The debug time format is:
> 
>   [2020-12-03 18:25:31.822152] affinity: SYS
>   [2020-12-03 18:25:31.822164] mmap flush: 1
>   [2020-12-03 18:25:31.822175] comp level: 0
>   [2020-12-03 18:25:32.002047] mmap size 528384B
> 
> Signed-off-by: Jiri Olsa <jolsa@kernel.org>
> ---
>  tools/perf/util/debug.c | 34 +++++++++++++++++++++++++++++++---
>  tools/perf/util/debug.h |  1 +
>  2 files changed, 32 insertions(+), 3 deletions(-)
> 
> diff --git a/tools/perf/util/debug.c b/tools/perf/util/debug.c
> index 50fd6a4be4e0..dc148b08814b 100644
> --- a/tools/perf/util/debug.c
> +++ b/tools/perf/util/debug.c
> @@ -10,6 +10,7 @@
>  #include <api/debug.h>
>  #include <linux/kernel.h>
>  #include <linux/time64.h>
> +#include <sys/time.h>
>  #ifdef HAVE_BACKTRACE_SUPPORT
>  #include <execinfo.h>
>  #endif
> @@ -31,21 +32,48 @@ int debug_ordered_events;
>  static int redirect_to_stderr;
>  int debug_data_convert;
>  static FILE *debug_file;
> +bool debug_display_time;
>  
>  void debug_set_file(FILE *file)
>  {
>  	debug_file = file;
>  }
>  
> +void debug_set_display_time(bool set)
> +{
> +	debug_display_time = set;
> +}
> +
> +static int fprintf_time(FILE *file)
> +{
> +	struct timeval tod;
> +	struct tm ltime;
> +	char date[64];
> +
> +	if (!debug_display_time)
> +		return 0;
> +
> +	if (gettimeofday(&tod, NULL) != 0)
> +		return 0;
> +
> +	if (localtime_r(&tod.tv_sec, &ltime) == NULL)
> +		return 0;
> +
> +	strftime(date, sizeof(date),  "%F %H:%M:%S", &ltime);
> +	return fprintf(file, "[%s.%06lu] ", date, tod.tv_usec);
> +}

  78    12.70 ubuntu:18.04-x-sparc64        : FAIL sparc64-linux-gnu-gcc (Ubuntu 7.5.0-3ubuntu1~18.04) 7.5.0

util/debug.c: In function 'fprintf_time':
util/debug.c:63:32: error: format '%lu' expects argument of type 'long unsigned int', but argument 4 has type '__suseconds_t {aka int}' [-Werror=format=]
  return fprintf(file, "[%s.%06lu] ", date, tod.tv_usec);
                            ~~~~^           ~~~~~~~~~~~
                            %06u

I'll try to fix it.

- Arnaldo

> +
>  int veprintf(int level, int var, const char *fmt, va_list args)
>  {
>  	int ret = 0;
>  
>  	if (var >= level) {
> -		if (use_browser >= 1 && !redirect_to_stderr)
> +		if (use_browser >= 1 && !redirect_to_stderr) {
>  			ui_helpline__vshow(fmt, args);
> -		else
> -			ret = vfprintf(debug_file, fmt, args);
> +		} else {
> +			ret = fprintf_time(debug_file);
> +			ret += vfprintf(debug_file, fmt, args);
> +		}
>  	}
>  
>  	return ret;
> diff --git a/tools/perf/util/debug.h b/tools/perf/util/debug.h
> index 43f712295645..48f631966067 100644
> --- a/tools/perf/util/debug.h
> +++ b/tools/perf/util/debug.h
> @@ -64,6 +64,7 @@ int veprintf(int level, int var, const char *fmt, va_list args);
>  
>  int perf_debug_option(const char *str);
>  void debug_set_file(FILE *file);
> +void debug_set_display_time(bool set);
>  void perf_debug_setup(void);
>  int perf_quiet_option(void);
>  
> -- 
> 2.26.2
> 

-- 

- Arnaldo

^ permalink raw reply	[flat|nested] 44+ messages in thread

* Re: [PATCH 03/22] perf tools: Add debug_set_display_time function
  2021-01-19 14:59   ` Arnaldo Carvalho de Melo
@ 2021-01-19 17:39     ` Jiri Olsa
  2021-01-19 19:42       ` Arnaldo Carvalho de Melo
  0 siblings, 1 reply; 44+ messages in thread
From: Jiri Olsa @ 2021-01-19 17:39 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo
  Cc: Jiri Olsa, lkml, Peter Zijlstra, Ingo Molnar, Mark Rutland,
	Namhyung Kim, Alexander Shishkin, Michael Petlan, Ian Rogers,
	Stephane Eranian, Alexei Budankov

On Tue, Jan 19, 2021 at 11:59:22AM -0300, Arnaldo Carvalho de Melo wrote:

SNIP

> > +}
> 
>   78    12.70 ubuntu:18.04-x-sparc64        : FAIL sparc64-linux-gnu-gcc (Ubuntu 7.5.0-3ubuntu1~18.04) 7.5.0
> 
> util/debug.c: In function 'fprintf_time':
> util/debug.c:63:32: error: format '%lu' expects argument of type 'long unsigned int', but argument 4 has type '__suseconds_t {aka int}' [-Werror=format=]
>   return fprintf(file, "[%s.%06lu] ", date, tod.tv_usec);
>                             ~~~~^           ~~~~~~~~~~~
>                             %06u
> 
> I'll try to fix it.

thanks, hopefuly the %u will do

jirka


^ permalink raw reply	[flat|nested] 44+ messages in thread

* Re: [PATCH 12/22] perf daemon: Allow only one daemon over base directory
  2021-01-19  5:37   ` Namhyung Kim
@ 2021-01-19 17:44     ` Jiri Olsa
  0 siblings, 0 replies; 44+ messages in thread
From: Jiri Olsa @ 2021-01-19 17:44 UTC (permalink / raw)
  To: Namhyung Kim
  Cc: Jiri Olsa, Arnaldo Carvalho de Melo, lkml, Peter Zijlstra,
	Ingo Molnar, Mark Rutland, Alexander Shishkin, Michael Petlan,
	Ian Rogers, Stephane Eranian, Alexei Budankov

On Tue, Jan 19, 2021 at 02:37:42PM +0900, Namhyung Kim wrote:

SNIP

> >
> > +static int check_lock(struct daemon *daemon)
> > +{
> > +       char path[PATH_MAX];
> > +       char buf[20];
> > +       int fd, pid;
> > +       ssize_t len;
> > +
> > +       scnprintf(path, sizeof(path), "%s/lock", daemon->base);
> > +
> > +       fd = open(path, O_RDWR|O_CREAT|O_CLOEXEC, 0640);
> > +       if (fd < 0)
> > +               return -1;
> > +
> > +       if (lockf(fd, F_TLOCK, 0) < 0) {
> > +               filename__read_int(path, &pid);
> > +               fprintf(stderr, "failed: another perf daemon (pid %d) owns %s\n",
> > +                       pid, daemon->base);
> > +               return -1;
> > +       }
> 
> So the fd is (a kind of) leaked and the lock is released only when
> the daemon is going to die, right?

yes, that's the idea, I'll put it in some comment

thanks,
jirka


^ permalink raw reply	[flat|nested] 44+ messages in thread

* Re: [PATCH 08/22] perf daemon: Add config file change check
  2021-01-19  5:31   ` Namhyung Kim
@ 2021-01-19 17:49     ` Jiri Olsa
  2021-01-21  4:54       ` Namhyung Kim
  0 siblings, 1 reply; 44+ messages in thread
From: Jiri Olsa @ 2021-01-19 17:49 UTC (permalink / raw)
  To: Namhyung Kim
  Cc: Jiri Olsa, Arnaldo Carvalho de Melo, lkml, Peter Zijlstra,
	Ingo Molnar, Mark Rutland, Alexander Shishkin, Michael Petlan,
	Ian Rogers, Stephane Eranian, Alexei Budankov

On Tue, Jan 19, 2021 at 02:31:55PM +0900, Namhyung Kim wrote:
> On Sun, Jan 3, 2021 at 7:05 AM Jiri Olsa <jolsa@kernel.org> wrote:
> >
> > Adding support to detect daemon's config file changes
> > and re-read the configuration when that happens.
> 
> Hmm.. maybe some of the code in the previous commit can be moved here.

do you mean the config file support?

I think this patch is quite small and straightforward - adds
the inotify processing for checking config change

perhaps we can separate some of the code from previous commit,
and make it simpler, I'll check

thanks,
jirka

> 
> Thanks,
> Namhyung
> 
> >
> > Using inotify file descriptor pluged 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>
> > ---
> 


^ permalink raw reply	[flat|nested] 44+ messages in thread

* Re: [PATCH 07/22] perf daemon: Add daemon command
  2021-01-19  4:08   ` Namhyung Kim
@ 2021-01-19 18:31     ` Jiri Olsa
  2021-01-21  4:53       ` Namhyung Kim
  0 siblings, 1 reply; 44+ messages in thread
From: Jiri Olsa @ 2021-01-19 18:31 UTC (permalink / raw)
  To: Namhyung Kim
  Cc: Jiri Olsa, Arnaldo Carvalho de Melo, lkml, Peter Zijlstra,
	Ingo Molnar, Mark Rutland, Alexander Shishkin, Michael Petlan,
	Ian Rogers, Stephane Eranian, Alexei Budankov

On Tue, Jan 19, 2021 at 01:08:17PM +0900, Namhyung Kim wrote:

SNIP

> > +               if (!session)
> > +                       return -ENOMEM;
> > +
> > +               pr_debug("reconfig: found new session %s\n", name);
> > +               /* This is new session, trigger reconfig to start it. */
> > +               session->state = SESSION_STATE__RECONFIG;
> > +       } else if (session->state == SESSION_STATE__KILL) {
> > +               /*
> > +                * The session was marked to kill and we still
> > +                * found it in config file.
> > +                */
> > +               pr_debug("reconfig: found current session %s\n", name);
> > +               session->state = SESSION_STATE__OK;
> > +       }
> > +
> > +       if (!strcmp(var, "run")) {
> > +               if (session->run && strcmp(session->run, value)) {
> > +                       free(session->run);
> > +                       pr_debug("reconfig: session %s is changed\n", name);
> > +                       session->state = SESSION_STATE__RECONFIG;
> > +               }
> > +               session->run = strdup(value);
> 
> Please check the result.

right, will add

> 
> > +       }
> > +
> > +       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 = strdup(value);
> 
> It seems these config items are mandatory.  Is there a check
> for their presence?

base needs to be there, I'll add the check

missing session should just put daemon to sleep and it
should be woken up by config file change

that seems like a good test actualy, will try to add it

> 
> 
> > +
> > +       return 0;
> > +}
> > +
> > +static int client_config(const char *var, const char *value, void *cb)
> > +{
> > +       struct daemon *daemon = cb;
> > +
> > +       if (!strcmp(var, "daemon.base"))
> > +               daemon->base = strdup(value);
> > +
> > +       return 0;
> > +}
> > +
> > +static int setup_client_config(struct daemon *daemon)
> > +{
> > +       struct perf_config_set *set;
> > +       int err = -ENOMEM;
> > +
> > +       set = perf_config_set__load_file(daemon->config_real);
> > +       if (set) {
> > +               err = perf_config_set(set, client_config, daemon);
> > +               perf_config_set__delete(set);
> > +       }
> > +
> > +       return err;
> > +}
> > +
> > +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 session for kill, the server config will
> > +        * set proper state for found sessions.
> > +        */
> > +       list_for_each_entry(session, &daemon->sessions, list)
> > +               session->state = SESSION_STATE__KILL;
> 
> Probably we can put them in a different state like INIT or READY?

it's convenient, because all session we find will be changed
to OK and the rest will be killed

SNIP

> > +               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("chdir failed");
> > +               return -1;
> > +       }
> > +
> > +       fd = open("/dev/null", O_RDONLY);
> > +       if (fd < 0) {
> > +               perror("failed to open /dev/null");
> > +               return -1;
> > +       }
> > +
> > +       close(0);
> > +       dup2(fd, 0);
> 
> The man page says dup2() will close the second file descriptor.

right, I overlooekd that, close calls are not needed then

SNIP

> > +               /* Reconfig session. */
> > +               pr_debug2("reconfig: session '%s' start\n", session->name);
> > +               if (session->pid > 0) {
> > +                       session__kill(session);
> > +                       pr_info("reconfig: session '%s' killed\n", session->name);
> > +               }
> > +               if (session__run(session, daemon))
> 
> Does it call a config function?  Or is it called already?

daemon__reconfig kills and starts sessions based
on the config data read by setup_server_config

> 
> > +                       return -1;
> > +               pr_debug2("reconfig: session '%s' done\n", session->name);
> > +               session->state = SESSION_STATE__OK;
> 
> I think RUNNING is a better name.

I'll check, I'll put the state graph in comment,
so we can discuss the changes

thanks,
jirka


^ permalink raw reply	[flat|nested] 44+ messages in thread

* Re: [PATCH 03/22] perf tools: Add debug_set_display_time function
  2021-01-19 17:39     ` Jiri Olsa
@ 2021-01-19 19:42       ` Arnaldo Carvalho de Melo
  0 siblings, 0 replies; 44+ messages in thread
From: Arnaldo Carvalho de Melo @ 2021-01-19 19:42 UTC (permalink / raw)
  To: Jiri Olsa
  Cc: Jiri Olsa, lkml, Peter Zijlstra, Ingo Molnar, Mark Rutland,
	Namhyung Kim, Alexander Shishkin, Michael Petlan, Ian Rogers,
	Stephane Eranian, Alexei Budankov

Em Tue, Jan 19, 2021 at 06:39:37PM +0100, Jiri Olsa escreveu:
> On Tue, Jan 19, 2021 at 11:59:22AM -0300, Arnaldo Carvalho de Melo wrote:
> 
> SNIP
> 
> > > +}
> > 
> >   78    12.70 ubuntu:18.04-x-sparc64        : FAIL sparc64-linux-gnu-gcc (Ubuntu 7.5.0-3ubuntu1~18.04) 7.5.0
> > 
> > util/debug.c: In function 'fprintf_time':
> > util/debug.c:63:32: error: format '%lu' expects argument of type 'long unsigned int', but argument 4 has type '__suseconds_t {aka int}' [-Werror=format=]
> >   return fprintf(file, "[%s.%06lu] ", date, tod.tv_usec);
> >                             ~~~~^           ~~~~~~~~~~~
> >                             %06u
> > 
> > I'll try to fix it.
> 
> thanks, hopefuly the %u will do

 cast to long, i.e.:

    return fprintf(file, "[%s.%06lu] ", date, (long)tod.tv_usec);

- Arnaldo

^ permalink raw reply	[flat|nested] 44+ messages in thread

* Re: [PATCH 07/22] perf daemon: Add daemon command
  2021-01-19 18:31     ` Jiri Olsa
@ 2021-01-21  4:53       ` Namhyung Kim
  0 siblings, 0 replies; 44+ messages in thread
From: Namhyung Kim @ 2021-01-21  4:53 UTC (permalink / raw)
  To: Jiri Olsa
  Cc: Jiri Olsa, Arnaldo Carvalho de Melo, lkml, Peter Zijlstra,
	Ingo Molnar, Mark Rutland, Alexander Shishkin, Michael Petlan,
	Ian Rogers, Stephane Eranian, Alexei Budankov

On Wed, Jan 20, 2021 at 3:31 AM Jiri Olsa <jolsa@redhat.com> wrote:
>
> On Tue, Jan 19, 2021 at 01:08:17PM +0900, Namhyung Kim wrote:
> > > +               /* Reconfig session. */
> > > +               pr_debug2("reconfig: session '%s' start\n", session->name);
> > > +               if (session->pid > 0) {
> > > +                       session__kill(session);
> > > +                       pr_info("reconfig: session '%s' killed\n", session->name);
> > > +               }
> > > +               if (session__run(session, daemon))
> >
> > Does it call a config function?  Or is it called already?
>
> daemon__reconfig kills and starts sessions based
> on the config data read by setup_server_config
>
> >
> > > +                       return -1;
> > > +               pr_debug2("reconfig: session '%s' done\n", session->name);
> > > +               session->state = SESSION_STATE__OK;
> >
> > I think RUNNING is a better name.
>
> I'll check, I'll put the state graph in comment,
> so we can discuss the changes

Yeah, that would be helpful.

Thanks,
Namhyung

^ permalink raw reply	[flat|nested] 44+ messages in thread

* Re: [PATCH 08/22] perf daemon: Add config file change check
  2021-01-19 17:49     ` Jiri Olsa
@ 2021-01-21  4:54       ` Namhyung Kim
  0 siblings, 0 replies; 44+ messages in thread
From: Namhyung Kim @ 2021-01-21  4:54 UTC (permalink / raw)
  To: Jiri Olsa
  Cc: Jiri Olsa, Arnaldo Carvalho de Melo, lkml, Peter Zijlstra,
	Ingo Molnar, Mark Rutland, Alexander Shishkin, Michael Petlan,
	Ian Rogers, Stephane Eranian, Alexei Budankov

On Wed, Jan 20, 2021 at 3:51 AM Jiri Olsa <jolsa@redhat.com> wrote:
>
> On Tue, Jan 19, 2021 at 02:31:55PM +0900, Namhyung Kim wrote:
> > On Sun, Jan 3, 2021 at 7:05 AM Jiri Olsa <jolsa@kernel.org> wrote:
> > >
> > > Adding support to detect daemon's config file changes
> > > and re-read the configuration when that happens.
> >
> > Hmm.. maybe some of the code in the previous commit can be moved here.
>
> do you mean the config file support?
>
> I think this patch is quite small and straightforward - adds
> the inotify processing for checking config change
>
> perhaps we can separate some of the code from previous commit,
> and make it simpler, I'll check

I thought the reconfig logic could move to here.

Thanks,
Namhyung

^ permalink raw reply	[flat|nested] 44+ messages in thread

* Re: [PATCH 07/22] perf daemon: Add daemon command
  2021-01-02 22:04 ` [PATCH 07/22] perf daemon: Add daemon command Jiri Olsa
  2021-01-19  4:08   ` Namhyung Kim
@ 2021-01-27  7:09   ` Namhyung Kim
  2021-01-27 22:01     ` Jiri Olsa
  1 sibling, 1 reply; 44+ messages in thread
From: Namhyung Kim @ 2021-01-27  7:09 UTC (permalink / raw)
  To: Jiri Olsa
  Cc: Arnaldo Carvalho de Melo, lkml, Peter Zijlstra, Ingo Molnar,
	Mark Rutland, Alexander Shishkin, Michael Petlan, Ian Rogers,
	Stephane Eranian, Alexei Budankov

Hi Jiri,

On Sun, Jan 3, 2021 at 7:05 AM Jiri Olsa <jolsa@kernel.org> wrote:
> +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_STRING_OPTARG('x', "field-separator", &__daemon.csv_sep,
> +                       "field separator", "print counts with custom separator", ":"),

Oh, I didn't expect it's optional and default to ":" instead of ",".

Thanks,
Namhyung


> +               OPT_END()
> +       };

^ permalink raw reply	[flat|nested] 44+ messages in thread

* Re: [PATCH 07/22] perf daemon: Add daemon command
  2021-01-27  7:09   ` Namhyung Kim
@ 2021-01-27 22:01     ` Jiri Olsa
  0 siblings, 0 replies; 44+ messages in thread
From: Jiri Olsa @ 2021-01-27 22:01 UTC (permalink / raw)
  To: Namhyung Kim
  Cc: Jiri Olsa, Arnaldo Carvalho de Melo, lkml, Peter Zijlstra,
	Ingo Molnar, Mark Rutland, Alexander Shishkin, Michael Petlan,
	Ian Rogers, Stephane Eranian, Alexei Budankov

On Wed, Jan 27, 2021 at 04:09:18PM +0900, Namhyung Kim wrote:
> Hi Jiri,
> 
> On Sun, Jan 3, 2021 at 7:05 AM Jiri Olsa <jolsa@kernel.org> wrote:
> > +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_STRING_OPTARG('x', "field-separator", &__daemon.csv_sep,
> > +                       "field separator", "print counts with custom separator", ":"),
> 
> Oh, I didn't expect it's optional and default to ":" instead of ",".

np, I can change the default to ','

jirka

> 
> Thanks,
> Namhyung
> 
> 
> > +               OPT_END()
> > +       };
> 


^ permalink raw reply	[flat|nested] 44+ messages in thread

end of thread, other threads:[~2021-01-27 22:05 UTC | newest]

Thread overview: 44+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-01-02 22:04 [PATCHv2 00/22] perf tools: Add daemon command Jiri Olsa
2021-01-02 22:04 ` [PATCH 01/22] perf tools: Make perf_config_from_file static Jiri Olsa
2021-01-18 15:51   ` Arnaldo Carvalho de Melo
2021-01-02 22:04 ` [PATCH 02/22] perf tools: Add config set interface Jiri Olsa
2021-01-18 15:53   ` Arnaldo Carvalho de Melo
2021-01-02 22:04 ` [PATCH 03/22] perf tools: Add debug_set_display_time function Jiri Olsa
2021-01-18 16:02   ` Arnaldo Carvalho de Melo
2021-01-19 14:59   ` Arnaldo Carvalho de Melo
2021-01-19 17:39     ` Jiri Olsa
2021-01-19 19:42       ` Arnaldo Carvalho de Melo
2021-01-02 22:04 ` [PATCH 04/22] perf tools: Add perf_home_perfconfig function Jiri Olsa
2021-01-18 16:05   ` Arnaldo Carvalho de Melo
2021-01-02 22:04 ` [PATCH 05/22] perf tools: Make perf_config_system global Jiri Olsa
2021-01-18 16:03   ` Arnaldo Carvalho de Melo
2021-01-02 22:04 ` [PATCH 06/22] perf tools: Make perf_config_global gobal Jiri Olsa
2021-01-18 16:05   ` Arnaldo Carvalho de Melo
2021-01-02 22:04 ` [PATCH 07/22] perf daemon: Add daemon command Jiri Olsa
2021-01-19  4:08   ` Namhyung Kim
2021-01-19 18:31     ` Jiri Olsa
2021-01-21  4:53       ` Namhyung Kim
2021-01-27  7:09   ` Namhyung Kim
2021-01-27 22:01     ` Jiri Olsa
2021-01-02 22:04 ` [PATCH 08/22] perf daemon: Add config file change check Jiri Olsa
2021-01-19  5:31   ` Namhyung Kim
2021-01-19 17:49     ` Jiri Olsa
2021-01-21  4:54       ` Namhyung Kim
2021-01-02 22:04 ` [PATCH 09/22] perf daemon: Add signalfd support Jiri Olsa
2021-01-02 22:04 ` [PATCH 10/22] perf daemon: Add signal command Jiri Olsa
2021-01-02 22:04 ` [PATCH 11/22] perf daemon: Add stop command Jiri Olsa
2021-01-19  5:35   ` Namhyung Kim
2021-01-02 22:04 ` [PATCH 12/22] perf daemon: Allow only one daemon over base directory Jiri Olsa
2021-01-19  5:37   ` Namhyung Kim
2021-01-19 17:44     ` Jiri Olsa
2021-01-02 22:04 ` [PATCH 13/22] perf daemon: Set control fifo for session Jiri Olsa
2021-01-02 22:04 ` [PATCH 14/22] perf daemon: Add ping command Jiri Olsa
2021-01-02 22:04 ` [PATCH 15/22] perf daemon: Use control to stop session Jiri Olsa
2021-01-02 22:04 ` [PATCH 16/22] perf daemon: Add up time for daemon/session list Jiri Olsa
2021-01-02 22:04 ` [PATCH 17/22] perf daemon: Add man page for perf-daemon Jiri Olsa
2021-01-02 22:04 ` [PATCH 18/22] perf test: Add daemon list command test Jiri Olsa
2021-01-02 22:04 ` [PATCH 19/22] perf test: Add daemon reconfig test Jiri Olsa
2021-01-02 22:04 ` [PATCH 20/22] perf test: Add daemon stop command test Jiri Olsa
2021-01-02 22:04 ` [PATCH 21/22] perf test: Add daemon signal " Jiri Olsa
2021-01-02 22:04 ` [PATCH 22/22] perf test: Add daemon ping " Jiri Olsa
2021-01-18 12:55 ` [PATCHv2 00/22] perf tools: Add daemon command Jiri Olsa

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).