* [PATCH 1/8] perf tools: Add debug_set_file function
2020-12-12 10:43 [RFC 0/8] perf tools: Add daemon command Jiri Olsa
@ 2020-12-12 10:43 ` Jiri Olsa
2020-12-15 15:37 ` Arnaldo Carvalho de Melo
2020-12-12 10:43 ` [PATCH 2/8] perf tools: Add debug_set_display_time function Jiri Olsa
` (6 subsequent siblings)
7 siblings, 1 reply; 27+ messages in thread
From: Jiri Olsa @ 2020-12-12 10:43 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 set debug output file via new debug_set_file function.
It's called during perf startup in perf_debug_setup to set stderr
file as default and any perf command can set it later to different
file.
It will be used in perf daemon command to get verbose output into
log file.
Signed-off-by: Jiri Olsa <jolsa@kernel.org>
---
tools/perf/util/debug.c | 9 ++++++++-
tools/perf/util/debug.h | 2 ++
2 files changed, 10 insertions(+), 1 deletion(-)
diff --git a/tools/perf/util/debug.c b/tools/perf/util/debug.c
index 5cda5565777a..50fd6a4be4e0 100644
--- a/tools/perf/util/debug.c
+++ b/tools/perf/util/debug.c
@@ -30,6 +30,12 @@ bool dump_trace = false, quiet = false;
int debug_ordered_events;
static int redirect_to_stderr;
int debug_data_convert;
+static FILE *debug_file;
+
+void debug_set_file(FILE *file)
+{
+ debug_file = file;
+}
int veprintf(int level, int var, const char *fmt, va_list args)
{
@@ -39,7 +45,7 @@ int veprintf(int level, int var, const char *fmt, va_list args)
if (use_browser >= 1 && !redirect_to_stderr)
ui_helpline__vshow(fmt, args);
else
- ret = vfprintf(stderr, fmt, args);
+ ret = vfprintf(debug_file, fmt, args);
}
return ret;
@@ -227,6 +233,7 @@ DEBUG_WRAPPER(debug, 1);
void perf_debug_setup(void)
{
+ debug_set_file(stderr);
libapi_set_print(pr_warning_wrapper, pr_warning_wrapper, pr_debug_wrapper);
}
diff --git a/tools/perf/util/debug.h b/tools/perf/util/debug.h
index f1734abd98dd..43f712295645 100644
--- a/tools/perf/util/debug.h
+++ b/tools/perf/util/debug.h
@@ -5,6 +5,7 @@
#include <stdarg.h>
#include <stdbool.h>
+#include <stdio.h>
#include <linux/compiler.h>
extern int verbose;
@@ -62,6 +63,7 @@ int eprintf_time(int level, int var, u64 t, const char *fmt, ...) __printf(4, 5)
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 perf_debug_setup(void);
int perf_quiet_option(void);
--
2.26.2
^ permalink raw reply related [flat|nested] 27+ messages in thread* Re: [PATCH 1/8] perf tools: Add debug_set_file function
2020-12-12 10:43 ` [PATCH 1/8] perf tools: Add debug_set_file function Jiri Olsa
@ 2020-12-15 15:37 ` Arnaldo Carvalho de Melo
0 siblings, 0 replies; 27+ messages in thread
From: Arnaldo Carvalho de Melo @ 2020-12-15 15:37 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, Dec 12, 2020 at 11:43:51AM +0100, Jiri Olsa escreveu:
> Allow to set debug output file via new debug_set_file function.
>
> It's called during perf startup in perf_debug_setup to set stderr
> file as default and any perf command can set it later to different
> file.
>
> It will be used in perf daemon command to get verbose output into
> log file.
Thanks, applied.
- Arnaldo
> Signed-off-by: Jiri Olsa <jolsa@kernel.org>
> ---
> tools/perf/util/debug.c | 9 ++++++++-
> tools/perf/util/debug.h | 2 ++
> 2 files changed, 10 insertions(+), 1 deletion(-)
>
> diff --git a/tools/perf/util/debug.c b/tools/perf/util/debug.c
> index 5cda5565777a..50fd6a4be4e0 100644
> --- a/tools/perf/util/debug.c
> +++ b/tools/perf/util/debug.c
> @@ -30,6 +30,12 @@ bool dump_trace = false, quiet = false;
> int debug_ordered_events;
> static int redirect_to_stderr;
> int debug_data_convert;
> +static FILE *debug_file;
> +
> +void debug_set_file(FILE *file)
> +{
> + debug_file = file;
> +}
>
> int veprintf(int level, int var, const char *fmt, va_list args)
> {
> @@ -39,7 +45,7 @@ int veprintf(int level, int var, const char *fmt, va_list args)
> if (use_browser >= 1 && !redirect_to_stderr)
> ui_helpline__vshow(fmt, args);
> else
> - ret = vfprintf(stderr, fmt, args);
> + ret = vfprintf(debug_file, fmt, args);
> }
>
> return ret;
> @@ -227,6 +233,7 @@ DEBUG_WRAPPER(debug, 1);
>
> void perf_debug_setup(void)
> {
> + debug_set_file(stderr);
> libapi_set_print(pr_warning_wrapper, pr_warning_wrapper, pr_debug_wrapper);
> }
>
> diff --git a/tools/perf/util/debug.h b/tools/perf/util/debug.h
> index f1734abd98dd..43f712295645 100644
> --- a/tools/perf/util/debug.h
> +++ b/tools/perf/util/debug.h
> @@ -5,6 +5,7 @@
>
> #include <stdarg.h>
> #include <stdbool.h>
> +#include <stdio.h>
> #include <linux/compiler.h>
>
> extern int verbose;
> @@ -62,6 +63,7 @@ int eprintf_time(int level, int var, u64 t, const char *fmt, ...) __printf(4, 5)
> 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 perf_debug_setup(void);
> int perf_quiet_option(void);
>
> --
> 2.26.2
>
--
- Arnaldo
^ permalink raw reply [flat|nested] 27+ messages in thread
* [PATCH 2/8] perf tools: Add debug_set_display_time function
2020-12-12 10:43 [RFC 0/8] perf tools: Add daemon command Jiri Olsa
2020-12-12 10:43 ` [PATCH 1/8] perf tools: Add debug_set_file function Jiri Olsa
@ 2020-12-12 10:43 ` Jiri Olsa
2020-12-15 15:37 ` Arnaldo Carvalho de Melo
2020-12-12 10:43 ` [PATCH 3/8] perf tools: Add config set interface Jiri Olsa
` (5 subsequent siblings)
7 siblings, 1 reply; 27+ messages in thread
From: Jiri Olsa @ 2020-12-12 10:43 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, <ime) == NULL)
+ return 0;
+
+ strftime(date, sizeof(date), "%F %H:%M:%S", <ime);
+ 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] 27+ messages in thread* Re: [PATCH 2/8] perf tools: Add debug_set_display_time function
2020-12-12 10:43 ` [PATCH 2/8] perf tools: Add debug_set_display_time function Jiri Olsa
@ 2020-12-15 15:37 ` Arnaldo Carvalho de Melo
0 siblings, 0 replies; 27+ messages in thread
From: Arnaldo Carvalho de Melo @ 2020-12-15 15:37 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, Dec 12, 2020 at 11:43:52AM +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, <ime) == NULL)
> + return 0;
> +
> + strftime(date, sizeof(date), "%F %H:%M:%S", <ime);
> + 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] 27+ messages in thread
* [PATCH 3/8] perf tools: Add config set interface
2020-12-12 10:43 [RFC 0/8] perf tools: Add daemon command Jiri Olsa
2020-12-12 10:43 ` [PATCH 1/8] perf tools: Add debug_set_file function Jiri Olsa
2020-12-12 10:43 ` [PATCH 2/8] perf tools: Add debug_set_display_time function Jiri Olsa
@ 2020-12-12 10:43 ` Jiri Olsa
2020-12-15 15:41 ` Arnaldo Carvalho de Melo
2020-12-12 10:43 ` [PATCH 4/8] perf daemon: Add daemon command Jiri Olsa
` (4 subsequent siblings)
7 siblings, 1 reply; 27+ messages in thread
From: Jiri Olsa @ 2020-12-12 10:43 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__new_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 6969f82843ee..dc3f03f8bbf5 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__new_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 8c881e3a3ec3..f58b457e7e5f 100644
--- a/tools/perf/util/config.h
+++ b/tools/perf/util/config.h
@@ -30,6 +30,8 @@ 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_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 *);
@@ -38,6 +40,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__new_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] 27+ messages in thread* Re: [PATCH 3/8] perf tools: Add config set interface
2020-12-12 10:43 ` [PATCH 3/8] perf tools: Add config set interface Jiri Olsa
@ 2020-12-15 15:41 ` Arnaldo Carvalho de Melo
2020-12-15 19:11 ` Jiri Olsa
0 siblings, 1 reply; 27+ messages in thread
From: Arnaldo Carvalho de Melo @ 2020-12-15 15:41 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, Dec 12, 2020 at 11:43:53AM +0100, Jiri Olsa escreveu:
> Add interface to load config set from custom file
> by using perf_config_set__new_file function.
>
> It will be used in perf daemon command to process
> custom config file.
The naming may be misleading, as this is not _setting_ the perf config
to be that one, but to apply a set of changes to whatever is the current
configuration, right?
Perhaps 'perf_config_set__load_file()'
There is value in _resetting_ the config to some configuration, i.e.
moving everything to the strict defaults and then loading a file that
sets the configuration to a state similar to what perf does when it
first starts.
- 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 6969f82843ee..dc3f03f8bbf5 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__new_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 8c881e3a3ec3..f58b457e7e5f 100644
> --- a/tools/perf/util/config.h
> +++ b/tools/perf/util/config.h
> @@ -30,6 +30,8 @@ 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_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 *);
> @@ -38,6 +40,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__new_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] 27+ messages in thread* Re: [PATCH 3/8] perf tools: Add config set interface
2020-12-15 15:41 ` Arnaldo Carvalho de Melo
@ 2020-12-15 19:11 ` Jiri Olsa
0 siblings, 0 replies; 27+ messages in thread
From: Jiri Olsa @ 2020-12-15 19:11 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, Dec 15, 2020 at 12:41:18PM -0300, Arnaldo Carvalho de Melo wrote:
> Em Sat, Dec 12, 2020 at 11:43:53AM +0100, Jiri Olsa escreveu:
> > Add interface to load config set from custom file
> > by using perf_config_set__new_file function.
> >
> > It will be used in perf daemon command to process
> > custom config file.
>
> The naming may be misleading, as this is not _setting_ the perf config
> to be that one, but to apply a set of changes to whatever is the current
> configuration, right?
>
> Perhaps 'perf_config_set__load_file()'
yep, it's actually loading, will change
thanks,
jirka
>
> There is value in _resetting_ the config to some configuration, i.e.
> moving everything to the strict defaults and then loading a file that
> sets the configuration to a state similar to what perf does when it
> first starts.
>
> - 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 6969f82843ee..dc3f03f8bbf5 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__new_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 8c881e3a3ec3..f58b457e7e5f 100644
> > --- a/tools/perf/util/config.h
> > +++ b/tools/perf/util/config.h
> > @@ -30,6 +30,8 @@ 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_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 *);
> > @@ -38,6 +40,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__new_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] 27+ messages in thread
* [PATCH 4/8] perf daemon: Add daemon command
2020-12-12 10:43 [RFC 0/8] perf tools: Add daemon command Jiri Olsa
` (2 preceding siblings ...)
2020-12-12 10:43 ` [PATCH 3/8] perf tools: Add config set interface Jiri Olsa
@ 2020-12-12 10:43 ` Jiri Olsa
2020-12-15 15:40 ` Alexei Budankov
2020-12-15 15:44 ` Arnaldo Carvalho de Melo
2020-12-12 10:43 ` [PATCH 5/8] perf daemon: Add signal command Jiri Olsa
` (3 subsequent siblings)
7 siblings, 2 replies; 27+ messages in thread
From: Jiri Olsa @ 2020-12-12 10:43 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 config.daemon
[daemon]
base=/opt/perfdata
[session-1]
run = -m 10M -e cycles -o /opt/perfdata/1/perf.data --overwrite --switch-output -a
[session-2]
run = -m 20M -e sched:* -o /opt/perfdata/2/perf.data --overwrite --switch-output -a
Default perf config has the same daemon base:
# cat ~/.perfconfig
[daemon]
base=/opt/perfdata
Starting the daemon:
# perf daemon --config config.daemon
Check sessions:
# perf daemon
[1:92187] perf record -m 11M -e cycles -o /opt/perfdata/1/perf.data --overwrite --switch-output -a
[2:92188] perf record -m 20M -e sched:* -o /opt/perfdata/2/perf.data --overwrite --switch-output -a
Check sessions with more info:
# perf daemon -v
[1:92187] perf record -m 11M -e cycles -o /opt/perfdata/1/perf.data --overwrite --switch-output -a
output: /opt/perfdata/1/output
[2:92188] perf record -m 20M -e sched:* -o /opt/perfdata/2/perf.data --overwrite --switch-output -a
output: /opt/perfdata/2/output
The 'output' file is perf record output for specific session.
Signed-off-by: Jiri Olsa <jolsa@kernel.org>
---
tools/perf/Build | 3 +
tools/perf/Documentation/perf-daemon.txt | 97 +++
tools/perf/builtin-daemon.c | 794 +++++++++++++++++++++++
tools/perf/builtin.h | 1 +
tools/perf/command-list.txt | 1 +
tools/perf/perf.c | 1 +
6 files changed, 897 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..54aa38996fff 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
@@ -53,3 +54,5 @@ perf-y += scripts/
perf-$(CONFIG_TRACE) += trace/beauty/
gtk-y += ui/gtk/
+
+CFLAGS_builtin-daemon.o += -DPERF="BUILD_STR($(bindir_SQ)/perf)"
diff --git a/tools/perf/Documentation/perf-daemon.txt b/tools/perf/Documentation/perf-daemon.txt
new file mode 100644
index 000000000000..dee39be110ba
--- /dev/null
+++ b/tools/perf/Documentation/perf-daemon.txt
@@ -0,0 +1,97 @@
+perf-daemon(1)
+==============
+
+NAME
+----
+perf-daemon - Run record sessions on background
+
+SYNOPSIS
+--------
+[verse]
+'perf daemon'
+'perf daemon' [<options>]
+
+DESCRIPTION
+-----------
+This command allows to run simple daemon process that starts and
+monitors configured record sessions.
+
+Each session represents one perf record process.
+
+These sessions are configured through config file, see CONFIG FILE
+section with EXAMPLES.
+
+OPTIONS
+-------
+--config=<PATH>::
+ Config file path.
+
+-f::
+--foreground::
+ Do not put the process in background.
+
+-v::
+--verbose::
+ Be more verbose.
+
+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.
+
+EXAMPLES
+--------
+Example with 2 record sessions:
+
+ # cat config.daemon
+ [daemon]
+ base=/opt/perfdata
+
+ [session-1]
+ run = -m 10M -e cycles -o /opt/perfdata/1/perf.data --overwrite --switch-output -a
+
+ [session-2]
+ run = -m 20M -e sched:* -o /opt/perfdata/2/perf.data --overwrite --switch-output -a
+
+
+Default perf config has the same daemon base:
+
+ # cat ~/.perfconfig
+ [daemon]
+ base=/opt/perfdata
+
+
+Starting the daemon:
+
+ # perf daemon --config config.daemon
+
+
+Check sessions:
+
+ # perf daemon
+ [1:92187] perf record -m 11M -e cycles -o /opt/perfdata/1/perf.data --overwrite --switch-output -a
+ [2:92188] perf record -m 20M -e sched:* -o /opt/perfdata/2/perf.data --overwrite --switch-output -a
+
+
+Check sessions with more info:
+
+ # perf daemon -v
+ [1:92187] perf record -m 11M -e cycles -o /opt/perfdata/1/perf.data --overwrite --switch-output -a
+ output: /opt/perfdata/1/output
+ [2:92188] perf record -m 20M -e sched:* -o /opt/perfdata/2/perf.data --overwrite --switch-output -a
+ output: /opt/perfdata/2/output
+
+The 'output' file is perf record output for specific session.
+
+
+SEE ALSO
+--------
+linkperf:perf-record[1], linkperf:perf-config[1]
diff --git a/tools/perf/builtin-daemon.c b/tools/perf/builtin-daemon.c
new file mode 100644
index 000000000000..7f455837d58a
--- /dev/null
+++ b/tools/perf/builtin-daemon.c
@@ -0,0 +1,794 @@
+// 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 <api/fs/fs.h>
+
+#define SESSION_OUTPUT "output"
+
+enum session_state {
+ SESSION_STATE__OK,
+ SESSION_STATE__RECONFIG,
+ SESSION_STATE__KILL,
+};
+
+struct session {
+ char *name;
+ char *run;
+ int pid;
+ struct list_head list;
+ enum session_state state;
+};
+
+struct daemon {
+ char *config;
+ char *config_base;
+ char *base;
+ struct list_head sessions;
+ FILE *out;
+};
+
+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 session_name(const char *var, char *session, int len)
+{
+ const char *p = var + sizeof("session-") - 1;
+
+ while (*p != '.' && 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 (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_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__new_file(daemon->config);
+ if (set) {
+ err = perf_config_set(set, server_config, daemon);
+ perf_config_set__delete(set);
+ }
+
+ return err;
+}
+
+static int session__check(struct session *session, struct daemon *daemon)
+{
+ int err, status;
+
+ err = waitpid(session->pid, &status, WNOHANG);
+ if (err < 0) {
+ session->pid = -1;
+ return -1;
+ }
+
+ if (err && WIFEXITED(status)) {
+ fprintf(daemon->out, "session(%d) %s excited with %d\n",
+ session->pid, session->name, WEXITSTATUS(status));
+ session->state = SESSION_STATE__KILL;
+ session->pid = -1;
+ return -1;
+ }
+
+ return 0;
+}
+
+static int session__wait(struct session *session, struct daemon *daemon,
+ int secs)
+{
+ time_t current, start = 0;
+ int cnt;
+
+ start = current = time(NULL);
+
+ do {
+ usleep(500);
+ cnt = session__check(session, daemon);
+ if (cnt)
+ break;
+
+ current = time(NULL);
+ } while ((start + secs > current));
+
+ return cnt;
+}
+
+static int session__signal(struct session *session, int sig)
+{
+ if (session->pid < 0)
+ return -1;
+ return kill(session->pid, sig);
+}
+
+static void session__kill(struct session *session, struct daemon *daemon)
+{
+ session__signal(session, SIGTERM);
+ if (session__wait(session, daemon, 30))
+ session__signal(session, SIGKILL);
+}
+
+static int session__run(struct session *session, struct daemon *daemon)
+{
+ char base[PATH_MAX];
+ char buf[PATH_MAX];
+ char **argv;
+ int argc, fd;
+
+ scnprintf(base, PATH_MAX, "%s/%s", daemon->base, session->name);
+
+ if (mkdir(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(base)) {
+ perror("chdir failed");
+ return -1;
+ }
+
+ fd = open(SESSION_OUTPUT, O_RDWR|O_CREAT|O_TRUNC, 0644);
+ if (fd < 0) {
+ perror("open failed");
+ return -1;
+ }
+
+ close(0);
+ dup2(fd, 1);
+ dup2(fd, 2);
+ close(fd);
+
+ scnprintf(buf, sizeof(buf), "%s record %s", PERF, session->run);
+
+ argv = argv_split(buf, &argc);
+ if (!argv)
+ exit(-1);
+
+ exit(execve(PERF, argv, NULL));
+ return -1;
+}
+
+static int daemon__check(struct daemon *daemon)
+{
+ struct session *session;
+ int cnt = 0;
+
+ list_for_each_entry(session, &daemon->sessions, list) {
+ if (session__check(session, daemon))
+ continue;
+ cnt++;
+ }
+
+ return cnt;
+}
+
+static int daemon__wait(struct daemon *daemon, int secs)
+{
+ time_t current, start = 0;
+ int cnt;
+
+ start = current = time(NULL);
+
+ do {
+ usleep(100);
+ cnt = daemon__check(daemon);
+ if (!cnt)
+ break;
+
+ current = time(NULL);
+ } while ((start + secs > current));
+
+ return cnt;
+}
+
+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->name);
+ free(session->run);
+ free(session);
+}
+
+static void session__remove(struct session *session)
+{
+ list_del(&session->list);
+ session__free(session);
+}
+
+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, daemon);
+ 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, daemon);
+ 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);
+ if (daemon__wait(daemon, 30))
+ daemon__signal(daemon, SIGKILL);
+}
+
+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);
+}
+
+static void daemon__exit(struct daemon *daemon)
+{
+ daemon__kill(daemon);
+ daemon__free(daemon);
+ 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 {
+ CMD_LIST = 0,
+ CMD_LIST_VERBOSE = 1,
+ CMD_MAX,
+};
+
+static int cmd_session_list(struct daemon *daemon, FILE *out, bool simple)
+{
+ struct session *session;
+
+ list_for_each_entry(session, &daemon->sessions, list) {
+ fprintf(out, "[%s:%d] perf record %s\n",
+ session->name, session->pid, session->run);
+ if (simple)
+ continue;
+ fprintf(out, " output: %s/%s/" SESSION_OUTPUT "\n",
+ daemon->base, session->name);
+ }
+
+ return 0;
+}
+
+static int handle_server_socket(struct daemon *daemon, int sock_fd)
+{
+ int ret = -EINVAL, fd;
+ FILE *out;
+ u64 cmd;
+
+ 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) {
+ case CMD_LIST:
+ case CMD_LIST_VERBOSE:
+ ret = cmd_session_list(daemon, out, cmd == CMD_LIST);
+ 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 setup_config_changes(struct daemon *daemon)
+{
+ char *basen = strdup(daemon->config);
+ char *dirn = strdup(daemon->config);
+ 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;
+
+ 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 set_daemon_config(struct daemon *daemon, const char *config)
+{
+ char *real = realpath(config, NULL);
+
+ if (!real) {
+ perror("realpath failed");
+ return -1;
+ }
+ daemon->config = real;
+ return 0;
+}
+
+static int __cmd_daemon(struct daemon *daemon, bool foreground, const char *config)
+{
+ int sock_pos, file_pos, sock_fd, conf_fd;
+ bool reconfig = true;
+ struct fdarray fda;
+ int err = 0;
+
+ if (set_daemon_config(daemon, config))
+ 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;
+
+ conf_fd = setup_config_changes(daemon);
+ if (conf_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;
+
+ 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);
+
+ while (!done && !err) {
+ if (reconfig) {
+ err = daemon__reconfig(daemon);
+ reconfig = false;
+ }
+
+ if (fdarray__poll(&fda, 500)) {
+ 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);
+ }
+
+ if (!daemon__check(daemon)) {
+ fprintf(daemon->out, "no sessions left, bailing out\n");
+ break;
+ }
+ }
+
+ pr_info("daemon exited\n");
+
+ close(sock_fd);
+ close(conf_fd);
+
+ fdarray__exit(&fda);
+ daemon__exit(daemon);
+ return err;
+}
+
+static int send_cmd(struct daemon *daemon, u64 cmd)
+{
+ char *line = NULL;
+ size_t len = 0;
+ ssize_t nread;
+ FILE *in;
+ int fd;
+
+ perf_config(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 const char * const daemon_usage[] = {
+ "perf daemon [<options>]",
+ NULL
+};
+
+int cmd_daemon(int argc, const char **argv)
+{
+ bool foreground = false;
+ const char *config = NULL;
+ struct daemon daemon = {
+ .sessions = LIST_HEAD_INIT(daemon.sessions),
+ .out = stdout,
+ };
+ struct option daemon_options[] = {
+ OPT_INCR('v', "verbose", &verbose, "be more verbose"),
+ OPT_STRING(0, "config", &config,
+ "config file", "config file path"),
+ OPT_BOOLEAN('f', "foreground", &foreground, "stay on console"),
+ OPT_END()
+ };
+
+ argc = parse_options(argc, argv, daemon_options, daemon_usage, 0);
+ if (argc)
+ usage_with_options(daemon_usage, daemon_options);
+
+ if (config)
+ return __cmd_daemon(&daemon, foreground, config);
+
+ return send_cmd(&daemon, verbose ? CMD_LIST_VERBOSE : CMD_LIST);
+}
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] 27+ messages in thread* Re: [PATCH 4/8] perf daemon: Add daemon command
2020-12-12 10:43 ` [PATCH 4/8] perf daemon: Add daemon command Jiri Olsa
@ 2020-12-15 15:40 ` Alexei Budankov
2020-12-15 19:43 ` Jiri Olsa
2020-12-15 15:44 ` Arnaldo Carvalho de Melo
1 sibling, 1 reply; 27+ messages in thread
From: Alexei Budankov @ 2020-12-15 15:40 UTC (permalink / raw)
To: Jiri Olsa, Arnaldo Carvalho de Melo
Cc: lkml, Peter Zijlstra, Ingo Molnar, Mark Rutland, Namhyung Kim,
Alexander Shishkin, Michael Petlan, Ian Rogers, Stephane Eranian,
abudankov
Hi,
On 12.12.2020 13:43, Jiri Olsa 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 config.daemon
> [daemon]
> base=/opt/perfdata
It could probably make sense to consider using locations at /var/
directory, similar to other already existing daemon processes in
system so admin and user experience would be easily reusabe for
performance monitoring daemon (service).
>
> [session-1]
> run = -m 10M -e cycles -o /opt/perfdata/1/perf.data --overwrite --switch-output -a
>
> [session-2]
> run = -m 20M -e sched:* -o /opt/perfdata/2/perf.data --overwrite --switch-output -a
>
> Default perf config has the same daemon base:
>
> # cat ~/.perfconfig
> [daemon]
> base=/opt/perfdata
>
> Starting the daemon:
>
> # perf daemon --config config.daemon
It could make sense to name daemon config file similar to .perfconfig
e.g. like .perfconfig.daemon. perf daemon command would then assume, by
default, usage of .perfconfig.daemon config or the one specified on the
command line via --config option. It also would be helpfull have loaded
config file path printed into console:
# perf daemon
Daemon process <pid> started with config /path/to/.perfconfig.daemon
>
> Check sessions:
>
> # perf daemon
> [1:92187] perf record -m 11M -e cycles -o /opt/perfdata/1/perf.data --overwrite --switch-output -a
> [2:92188] perf record -m 20M -e sched:* -o /opt/perfdata/2/perf.data --overwrite --switch-output -a
>
> Check sessions with more info:
>
> # perf daemon -v
> [1:92187] perf record -m 11M -e cycles -o /opt/perfdata/1/perf.data --overwrite --switch-output -a
> output: /opt/perfdata/1/output
> [2:92188] perf record -m 20M -e sched:* -o /opt/perfdata/2/perf.data --overwrite --switch-output -a
> output: /opt/perfdata/2/output
>
> The 'output' file is perf record output for specific session.
>
> Signed-off-by: Jiri Olsa <jolsa@kernel.org>
> ---
> tools/perf/Build | 3 +
> tools/perf/Documentation/perf-daemon.txt | 97 +++
> tools/perf/builtin-daemon.c | 794 +++++++++++++++++++++++
> tools/perf/builtin.h | 1 +
> tools/perf/command-list.txt | 1 +
> tools/perf/perf.c | 1 +
> 6 files changed, 897 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..54aa38996fff 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
> @@ -53,3 +54,5 @@ perf-y += scripts/
> perf-$(CONFIG_TRACE) += trace/beauty/
>
> gtk-y += ui/gtk/
> +
> +CFLAGS_builtin-daemon.o += -DPERF="BUILD_STR($(bindir_SQ)/perf)"
> diff --git a/tools/perf/Documentation/perf-daemon.txt b/tools/perf/Documentation/perf-daemon.txt
> new file mode 100644
> index 000000000000..dee39be110ba
> --- /dev/null
> +++ b/tools/perf/Documentation/perf-daemon.txt
> @@ -0,0 +1,97 @@
> +perf-daemon(1)
> +==============
> +
> +NAME
> +----
> +perf-daemon - Run record sessions on background
> +
> +SYNOPSIS
> +--------
> +[verse]
> +'perf daemon'
> +'perf daemon' [<options>]
> +
> +DESCRIPTION
> +-----------
> +This command allows to run simple daemon process that starts and
> +monitors configured record sessions.
> +
> +Each session represents one perf record process.
> +
> +These sessions are configured through config file, see CONFIG FILE
> +section with EXAMPLES.
> +
> +OPTIONS
> +-------
> +--config=<PATH>::
> + Config file path.
> +
> +-f::
> +--foreground::
> + Do not put the process in background.
> +
> +-v::
> +--verbose::
> + Be more verbose.
> +
> +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.
> +
> +EXAMPLES
> +--------
> +Example with 2 record sessions:
> +
> + # cat config.daemon
> + [daemon]
> + base=/opt/perfdata
> +
> + [session-1]
> + run = -m 10M -e cycles -o /opt/perfdata/1/perf.data --overwrite --switch-output -a
> +
> + [session-2]
> + run = -m 20M -e sched:* -o /opt/perfdata/2/perf.data --overwrite --switch-output -a
> +
> +
> +Default perf config has the same daemon base:
> +
> + # cat ~/.perfconfig
> + [daemon]
> + base=/opt/perfdata
> +
> +
> +Starting the daemon:
> +
> + # perf daemon --config config.daemon
> +
> +
> +Check sessions:
> +
> + # perf daemon
> + [1:92187] perf record -m 11M -e cycles -o /opt/perfdata/1/perf.data --overwrite --switch-output -a
> + [2:92188] perf record -m 20M -e sched:* -o /opt/perfdata/2/perf.data --overwrite --switch-output -a
> +
> +
> +Check sessions with more info:
> +
> + # perf daemon -v
> + [1:92187] perf record -m 11M -e cycles -o /opt/perfdata/1/perf.data --overwrite --switch-output -a
> + output: /opt/perfdata/1/output
> + [2:92188] perf record -m 20M -e sched:* -o /opt/perfdata/2/perf.data --overwrite --switch-output -a
> + output: /opt/perfdata/2/output
> +
> +The 'output' file is perf record output for specific session.
> +
> +
> +SEE ALSO
> +--------
> +linkperf:perf-record[1], linkperf:perf-config[1]
> diff --git a/tools/perf/builtin-daemon.c b/tools/perf/builtin-daemon.c
> new file mode 100644
> index 000000000000..7f455837d58a
> --- /dev/null
> +++ b/tools/perf/builtin-daemon.c
> @@ -0,0 +1,794 @@
> +// 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 <api/fs/fs.h>
> +
> +#define SESSION_OUTPUT "output"
> +
> +enum session_state {
> + SESSION_STATE__OK,
> + SESSION_STATE__RECONFIG,
> + SESSION_STATE__KILL,
> +};
> +
> +struct session {
> + char *name;
> + char *run;
> + int pid;
> + struct list_head list;
> + enum session_state state;
> +};
> +
> +struct daemon {
> + char *config;
> + char *config_base;
> + char *base;
> + struct list_head sessions;
> + FILE *out;
> +};
> +
> +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 session_name(const char *var, char *session, int len)
should possibly name it get_session_name.
> +{
> + const char *p = var + sizeof("session-") - 1;
should possibly check that p still points inside [var, var+len).
> +
> + while (*p != '.' && 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 (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_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__new_file(daemon->config);
> + if (set) {
> + err = perf_config_set(set, server_config, daemon);
> + perf_config_set__delete(set);
> + }
> +
> + return err;
> +}
> +
> +static int session__check(struct session *session, struct daemon *daemon)
> +{
> + int err, status;
> +
> + err = waitpid(session->pid, &status, WNOHANG);
> + if (err < 0) {
> + session->pid = -1;
> + return -1;
> + }
> +
> + if (err && WIFEXITED(status)) {
> + fprintf(daemon->out, "session(%d) %s excited with %d\n",
> + session->pid, session->name, WEXITSTATUS(status));
> + session->state = SESSION_STATE__KILL;
> + session->pid = -1;
> + return -1;
> + }
> +
> + return 0;
> +}
> +
> +static int session__wait(struct session *session, struct daemon *daemon,
> + int secs)
> +{
> + time_t current, start = 0;
> + int cnt;
> +
> + start = current = time(NULL);
> +
> + do {
> + usleep(500);
This polling design is actually sub-optimal because it induces redundant
noise in a system. Ideally it should be implemented in async fashion so
kernel would atomically notify daemon process on event happened in some
of record processes e.g. using of poll-like() system call.
> + cnt = session__check(session, daemon);
> + if (cnt)
> + break;
> +
> + current = time(NULL);
> + } while ((start + secs > current));
> +
> + return cnt;
> +}
> +
> +static int session__signal(struct session *session, int sig)
> +{
> + if (session->pid < 0)
> + return -1;
> + return kill(session->pid, sig);
"Better" alternative could possibly be sending of some 'stop' command
via --control=fd.
Thanks,
Alexei
> +}
> +
> +static void session__kill(struct session *session, struct daemon *daemon)
> +{
> + session__signal(session, SIGTERM);
> + if (session__wait(session, daemon, 30))
> + session__signal(session, SIGKILL);
> +}
> +
> +static int session__run(struct session *session, struct daemon *daemon)
> +{
> + char base[PATH_MAX];
> + char buf[PATH_MAX];
> + char **argv;
> + int argc, fd;
> +
> + scnprintf(base, PATH_MAX, "%s/%s", daemon->base, session->name);
> +
> + if (mkdir(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(base)) {
> + perror("chdir failed");
> + return -1;
> + }
> +
> + fd = open(SESSION_OUTPUT, O_RDWR|O_CREAT|O_TRUNC, 0644);
> + if (fd < 0) {
> + perror("open failed");
> + return -1;
> + }
> +
> + close(0);
> + dup2(fd, 1);
> + dup2(fd, 2);
> + close(fd);
> +
> + scnprintf(buf, sizeof(buf), "%s record %s", PERF, session->run);
> +
> + argv = argv_split(buf, &argc);
> + if (!argv)
> + exit(-1);
> +
> + exit(execve(PERF, argv, NULL));
> + return -1;
> +}
> +
> +static int daemon__check(struct daemon *daemon)
> +{
> + struct session *session;
> + int cnt = 0;
> +
> + list_for_each_entry(session, &daemon->sessions, list) {
> + if (session__check(session, daemon))
> + continue;
> + cnt++;
> + }
> +
> + return cnt;
> +}
> +
> +static int daemon__wait(struct daemon *daemon, int secs)
> +{
> + time_t current, start = 0;
> + int cnt;
> +
> + start = current = time(NULL);
> +
> + do {
> + usleep(100);
> + cnt = daemon__check(daemon);
> + if (!cnt)
> + break;
> +
> + current = time(NULL);
> + } while ((start + secs > current));
> +
> + return cnt;
> +}
> +
> +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->name);
> + free(session->run);
> + free(session);
> +}
> +
> +static void session__remove(struct session *session)
> +{
> + list_del(&session->list);
> + session__free(session);
> +}
> +
> +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, daemon);
> + 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, daemon);
> + 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);
> + if (daemon__wait(daemon, 30))
> + daemon__signal(daemon, SIGKILL);
> +}
> +
> +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);
> +}
> +
> +static void daemon__exit(struct daemon *daemon)
> +{
> + daemon__kill(daemon);
> + daemon__free(daemon);
> + 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 {
> + CMD_LIST = 0,
> + CMD_LIST_VERBOSE = 1,
> + CMD_MAX,
> +};
> +
> +static int cmd_session_list(struct daemon *daemon, FILE *out, bool simple)
> +{
> + struct session *session;
> +
> + list_for_each_entry(session, &daemon->sessions, list) {
> + fprintf(out, "[%s:%d] perf record %s\n",
> + session->name, session->pid, session->run);
> + if (simple)
> + continue;
> + fprintf(out, " output: %s/%s/" SESSION_OUTPUT "\n",
> + daemon->base, session->name);
> + }
> +
> + return 0;
> +}
> +
> +static int handle_server_socket(struct daemon *daemon, int sock_fd)
> +{
> + int ret = -EINVAL, fd;
> + FILE *out;
> + u64 cmd;
> +
> + 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) {
> + case CMD_LIST:
> + case CMD_LIST_VERBOSE:
> + ret = cmd_session_list(daemon, out, cmd == CMD_LIST);
> + 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 setup_config_changes(struct daemon *daemon)
> +{
> + char *basen = strdup(daemon->config);
> + char *dirn = strdup(daemon->config);
> + 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;
> +
> + 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 set_daemon_config(struct daemon *daemon, const char *config)
> +{
> + char *real = realpath(config, NULL);
> +
> + if (!real) {
> + perror("realpath failed");
> + return -1;
> + }
> + daemon->config = real;
> + return 0;
> +}
> +
> +static int __cmd_daemon(struct daemon *daemon, bool foreground, const char *config)
> +{
> + int sock_pos, file_pos, sock_fd, conf_fd;
> + bool reconfig = true;
> + struct fdarray fda;
> + int err = 0;
> +
> + if (set_daemon_config(daemon, config))
> + 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;
> +
> + conf_fd = setup_config_changes(daemon);
> + if (conf_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;
> +
> + 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);
> +
> + while (!done && !err) {
> + if (reconfig) {
> + err = daemon__reconfig(daemon);
> + reconfig = false;
> + }
> +
> + if (fdarray__poll(&fda, 500)) {
> + 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);
> + }
> +
> + if (!daemon__check(daemon)) {
> + fprintf(daemon->out, "no sessions left, bailing out\n");
> + break;
> + }
> + }
> +
> + pr_info("daemon exited\n");
> +
> + close(sock_fd);
> + close(conf_fd);
> +
> + fdarray__exit(&fda);
> + daemon__exit(daemon);
> + return err;
> +}
> +
> +static int send_cmd(struct daemon *daemon, u64 cmd)
> +{
> + char *line = NULL;
> + size_t len = 0;
> + ssize_t nread;
> + FILE *in;
> + int fd;
> +
> + perf_config(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 const char * const daemon_usage[] = {
> + "perf daemon [<options>]",
> + NULL
> +};
> +
> +int cmd_daemon(int argc, const char **argv)
> +{
> + bool foreground = false;
> + const char *config = NULL;
> + struct daemon daemon = {
> + .sessions = LIST_HEAD_INIT(daemon.sessions),
> + .out = stdout,
> + };
> + struct option daemon_options[] = {
> + OPT_INCR('v', "verbose", &verbose, "be more verbose"),
> + OPT_STRING(0, "config", &config,
> + "config file", "config file path"),
> + OPT_BOOLEAN('f', "foreground", &foreground, "stay on console"),
> + OPT_END()
> + };
> +
> + argc = parse_options(argc, argv, daemon_options, daemon_usage, 0);
> + if (argc)
> + usage_with_options(daemon_usage, daemon_options);
> +
> + if (config)
> + return __cmd_daemon(&daemon, foreground, config);
> +
> + return send_cmd(&daemon, verbose ? CMD_LIST_VERBOSE : CMD_LIST);
> +}
> 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 {
>
^ permalink raw reply [flat|nested] 27+ messages in thread* Re: [PATCH 4/8] perf daemon: Add daemon command
2020-12-15 15:40 ` Alexei Budankov
@ 2020-12-15 19:43 ` Jiri Olsa
2020-12-16 7:54 ` Alexei Budankov
2020-12-18 13:25 ` Namhyung Kim
0 siblings, 2 replies; 27+ messages in thread
From: Jiri Olsa @ 2020-12-15 19:43 UTC (permalink / raw)
To: Alexei Budankov
Cc: Jiri Olsa, Arnaldo Carvalho de Melo, lkml, Peter Zijlstra,
Ingo Molnar, Mark Rutland, Namhyung Kim, Alexander Shishkin,
Michael Petlan, Ian Rogers, Stephane Eranian
On Tue, Dec 15, 2020 at 06:40:26PM +0300, Alexei Budankov wrote:
> Hi,
>
> On 12.12.2020 13:43, Jiri Olsa 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 config.daemon
> > [daemon]
> > base=/opt/perfdata
>
> It could probably make sense to consider using locations at /var/
> directory, similar to other already existing daemon processes in
> system so admin and user experience would be easily reusabe for
> performance monitoring daemon (service).
hm, you can specify any /var path in there if you like,
do you suggest to hardcode it?
>
> >
> > [session-1]
> > run = -m 10M -e cycles -o /opt/perfdata/1/perf.data --overwrite --switch-output -a
> >
> > [session-2]
> > run = -m 20M -e sched:* -o /opt/perfdata/2/perf.data --overwrite --switch-output -a
> >
> > Default perf config has the same daemon base:
> >
> > # cat ~/.perfconfig
> > [daemon]
> > base=/opt/perfdata
> >
> > Starting the daemon:
> >
> > # perf daemon --config config.daemon
>
> It could make sense to name daemon config file similar to .perfconfig
> e.g. like .perfconfig.daemon. perf daemon command would then assume, by
> default, usage of .perfconfig.daemon config or the one specified on the
> command line via --config option. It also would be helpfull have loaded
> config file path printed into console:
> # perf daemon
> Daemon process <pid> started with config /path/to/.perfconfig.daemon
so the current way is, that following creates daemon:
# perf daemon --config <CONFIG>
and any other 'non --config' option' is used to 'query/control' daemon:
# perf daemon
# perf daemon --signal
# perf daemon --stop
...
I'd like to keep short way checking on daemon, without too many
options, like:
# perf daemon
[690174:daemon] base: /opt/perfdata
[690175:top] perf record -e cycles --switch-output=1m --switch-max-files=6 -a
I think maybe we don't need any other .perfconfig, we could have
all in standard .perfconfig, like:
# cat .perfconfig:
[daemon]
base=/opt/perfdata
[session-1]
run = -m 1M -e cycles --overwrite --switch-output -a
[session-2]
run = -m 1M -e sched:* --overwrite --switch-output -a
and to run daemon on top of it:
# perf daemon --start
to run daemon with alternate config:
# perf daemon --start=<CONFIGFILE>
or:
# perf daemon --start --config=<CONFIGFILE>
and checking on daemon with default .perfconfig setup:
# perf daemon
checking on daemon with different base or config:
# perf daemon --base=<BASE>
# perf daemon --config=<CONFIGFILE>
# perf daemon --base=<BASE> --stop
# perf daemon --base=<BASE> --signal
# perf daemon --config=<CONFIGFILE> --stop
# perf daemon --config=<CONFIGFILE> --signal
how about that?
SNIP
> > +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 session_name(const char *var, char *session, int len)
>
> should possibly name it get_session_name.
ok
>
> > +{
> > + const char *p = var + sizeof("session-") - 1;
>
> should possibly check that p still points inside [var, var+len).
ok
SNIP
> > +static int session__wait(struct session *session, struct daemon *daemon,
> > + int secs)
> > +{
> > + time_t current, start = 0;
> > + int cnt;
> > +
> > + start = current = time(NULL);
> > +
> > + do {
> > + usleep(500);
>
> This polling design is actually sub-optimal because it induces redundant
> noise in a system. Ideally it should be implemented in async fashion so
> kernel would atomically notify daemon process on event happened in some
> of record processes e.g. using of poll-like() system call.
ok, any suggestion?
>
> > + cnt = session__check(session, daemon);
> > + if (cnt)
> > + break;
> > +
> > + current = time(NULL);
> > + } while ((start + secs > current));
> > +
> > + return cnt;
> > +}
> > +
> > +static int session__signal(struct session *session, int sig)
> > +{
> > + if (session->pid < 0)
> > + return -1;
> > + return kill(session->pid, sig);
>
> "Better" alternative could possibly be sending of some 'stop' command
> via --control=fd.
true, nice idea.. seems more clean and we already have control fd open
will add it to next version
thanks,
jirka
^ permalink raw reply [flat|nested] 27+ messages in thread* Re: [PATCH 4/8] perf daemon: Add daemon command
2020-12-15 19:43 ` Jiri Olsa
@ 2020-12-16 7:54 ` Alexei Budankov
2020-12-16 8:14 ` Jiri Olsa
2020-12-18 13:25 ` Namhyung Kim
1 sibling, 1 reply; 27+ messages in thread
From: Alexei Budankov @ 2020-12-16 7:54 UTC (permalink / raw)
To: Jiri Olsa
Cc: Jiri Olsa, Arnaldo Carvalho de Melo, lkml, Peter Zijlstra,
Ingo Molnar, Mark Rutland, Namhyung Kim, Alexander Shishkin,
Michael Petlan, Ian Rogers, Stephane Eranian
On 15.12.2020 22:43, Jiri Olsa wrote:
> On Tue, Dec 15, 2020 at 06:40:26PM +0300, Alexei Budankov wrote:
>> Hi,
>>
>> On 12.12.2020 13:43, Jiri Olsa 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 config.daemon
>>> [daemon]
>>> base=/opt/perfdata
>>
>> It could probably make sense to consider using locations at /var/
>> directory, similar to other already existing daemon processes in
>> system so admin and user experience would be easily reusabe for
>> performance monitoring daemon (service).
>
> hm, you can specify any /var path in there if you like,
> do you suggest to hardcode it?
This thing: https://en.wikipedia.org/wiki/Filesystem_Hierarchy_Standard
Since Perf is a part of OS it would better use some standardized locations.
>
>>
>>>
>>> [session-1]
>>> run = -m 10M -e cycles -o /opt/perfdata/1/perf.data --overwrite --switch-output -a
>>>
>>> [session-2]
>>> run = -m 20M -e sched:* -o /opt/perfdata/2/perf.data --overwrite --switch-output -a
>>>
>>> Default perf config has the same daemon base:
>>>
>>> # cat ~/.perfconfig
>>> [daemon]
>>> base=/opt/perfdata
>>>
>>> Starting the daemon:
>>>
>>> # perf daemon --config config.daemon
>>
>> It could make sense to name daemon config file similar to .perfconfig
>> e.g. like .perfconfig.daemon. perf daemon command would then assume, by
>> default, usage of .perfconfig.daemon config or the one specified on the
>> command line via --config option. It also would be helpfull have loaded
>> config file path printed into console:
>> # perf daemon
>> Daemon process <pid> started with config /path/to/.perfconfig.daemon
>
> so the current way is, that following creates daemon:
>
> # perf daemon --config <CONFIG>
>
> and any other 'non --config' option' is used to 'query/control' daemon:
>
> # perf daemon
> # perf daemon --signal
> # perf daemon --stop
> ...
>
>
> I'd like to keep short way checking on daemon, without too many
> options, like:
>
> # perf daemon
> [690174:daemon] base: /opt/perfdata
> [690175:top] perf record -e cycles --switch-output=1m --switch-max-files=6 -a
>
>
> I think maybe we don't need any other .perfconfig, we could have
> all in standard .perfconfig, like:
>
> # cat .perfconfig:
> [daemon]
> base=/opt/perfdata
>
> [session-1]
> run = -m 1M -e cycles --overwrite --switch-output -a
> [session-2]
> run = -m 1M -e sched:* --overwrite --switch-output -a
>
>
> and to run daemon on top of it:
>
> # perf daemon --start
>
>
> to run daemon with alternate config:
>
> # perf daemon --start=<CONFIGFILE>
>
> or:
>
> # perf daemon --start --config=<CONFIGFILE>
>
>
> and checking on daemon with default .perfconfig setup:
>
> # perf daemon
>
>
> checking on daemon with different base or config:
>
> # perf daemon --base=<BASE>
> # perf daemon --config=<CONFIGFILE>
> # perf daemon --base=<BASE> --stop
> # perf daemon --base=<BASE> --signal
> # perf daemon --config=<CONFIGFILE> --stop
> # perf daemon --config=<CONFIGFILE> --signal
>
> how about that?
Extending .perfconfig would look simpler for users, IHMO.
It looks like --base option actually implements --sandbox
or similar semantics.
>
> SNIP
>
>>> +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 session_name(const char *var, char *session, int len)
>>
>> should possibly name it get_session_name.
>
> ok
>
>>
>>> +{
>>> + const char *p = var + sizeof("session-") - 1;
>>
>> should possibly check that p still points inside [var, var+len).
>
> ok
>
> SNIP
>
>>> +static int session__wait(struct session *session, struct daemon *daemon,
>>> + int secs)
>>> +{
>>> + time_t current, start = 0;
>>> + int cnt;
>>> +
>>> + start = current = time(NULL);
>>> +
>>> + do {
>>> + usleep(500);
>>
>> This polling design is actually sub-optimal because it induces redundant
>> noise in a system. Ideally it should be implemented in async fashion so
>> kernel would atomically notify daemon process on event happened in some
>> of record processes e.g. using of poll-like() system call.
>
> ok, any suggestion?
Possibly, checking SIGCHLDs via signalfd [1] OR using pidfd [2] on kernel v5.3+
[1] https://man7.org/linux/man-pages/man2/signalfd.2.html
[2] https://man7.org/linux/man-pages/man2/pidfd_open.2.html
Thanks,
Alexei
>
>>
>>> + cnt = session__check(session, daemon);
>>> + if (cnt)
>>> + break;
>>> +
>>> + current = time(NULL);
>>> + } while ((start + secs > current));
>>> +
>>> + return cnt;
>>> +}
>>> +
>>> +static int session__signal(struct session *session, int sig)
>>> +{
>>> + if (session->pid < 0)
>>> + return -1;
>>> + return kill(session->pid, sig);
>>
>> "Better" alternative could possibly be sending of some 'stop' command
>> via --control=fd.
>
> true, nice idea.. seems more clean and we already have control fd open
>
> will add it to next version
>
> thanks,
> jirka
>
> .
>
^ permalink raw reply [flat|nested] 27+ messages in thread* Re: [PATCH 4/8] perf daemon: Add daemon command
2020-12-16 7:54 ` Alexei Budankov
@ 2020-12-16 8:14 ` Jiri Olsa
0 siblings, 0 replies; 27+ messages in thread
From: Jiri Olsa @ 2020-12-16 8:14 UTC (permalink / raw)
To: Alexei Budankov
Cc: Jiri Olsa, Arnaldo Carvalho de Melo, lkml, Peter Zijlstra,
Ingo Molnar, Mark Rutland, Namhyung Kim, Alexander Shishkin,
Michael Petlan, Ian Rogers, Stephane Eranian
On Wed, Dec 16, 2020 at 10:54:43AM +0300, Alexei Budankov wrote:
>
> On 15.12.2020 22:43, Jiri Olsa wrote:
> > On Tue, Dec 15, 2020 at 06:40:26PM +0300, Alexei Budankov wrote:
> >> Hi,
> >>
> >> On 12.12.2020 13:43, Jiri Olsa 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 config.daemon
> >>> [daemon]
> >>> base=/opt/perfdata
> >>
> >> It could probably make sense to consider using locations at /var/
> >> directory, similar to other already existing daemon processes in
> >> system so admin and user experience would be easily reusabe for
> >> performance monitoring daemon (service).
> >
> > hm, you can specify any /var path in there if you like,
> > do you suggest to hardcode it?
>
> This thing: https://en.wikipedia.org/wiki/Filesystem_Hierarchy_Standard
> Since Perf is a part of OS it would better use some standardized locations.
sure, user is free to configure that
SNIP
> >>> + start = current = time(NULL);
> >>> +
> >>> + do {
> >>> + usleep(500);
> >>
> >> This polling design is actually sub-optimal because it induces redundant
> >> noise in a system. Ideally it should be implemented in async fashion so
> >> kernel would atomically notify daemon process on event happened in some
> >> of record processes e.g. using of poll-like() system call.
> >
> > ok, any suggestion?
>
> Possibly, checking SIGCHLDs via signalfd [1] OR using pidfd [2] on kernel v5.3+
>
> [1] https://man7.org/linux/man-pages/man2/signalfd.2.html
> [2] https://man7.org/linux/man-pages/man2/pidfd_open.2.html
will check, thanks
jirka
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH 4/8] perf daemon: Add daemon command
2020-12-15 19:43 ` Jiri Olsa
2020-12-16 7:54 ` Alexei Budankov
@ 2020-12-18 13:25 ` Namhyung Kim
2020-12-18 19:30 ` Jiri Olsa
1 sibling, 1 reply; 27+ messages in thread
From: Namhyung Kim @ 2020-12-18 13:25 UTC (permalink / raw)
To: Jiri Olsa
Cc: Alexei Budankov, Jiri Olsa, Arnaldo Carvalho de Melo, lkml,
Peter Zijlstra, Ingo Molnar, Mark Rutland, Alexander Shishkin,
Michael Petlan, Ian Rogers, Stephane Eranian
Hi Jiri,
On Wed, Dec 16, 2020 at 4:44 AM Jiri Olsa <jolsa@redhat.com> wrote:
>
> On Tue, Dec 15, 2020 at 06:40:26PM +0300, Alexei Budankov wrote:
> > Hi,
> >
> > On 12.12.2020 13:43, Jiri Olsa 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 config.daemon
> > > [daemon]
> > > base=/opt/perfdata
> >
> > It could probably make sense to consider using locations at /var/
> > directory, similar to other already existing daemon processes in
> > system so admin and user experience would be easily reusabe for
> > performance monitoring daemon (service).
>
> hm, you can specify any /var path in there if you like,
> do you suggest to hardcode it?
>
> >
> > >
> > > [session-1]
> > > run = -m 10M -e cycles -o /opt/perfdata/1/perf.data --overwrite --switch-output -a
> > >
> > > [session-2]
> > > run = -m 20M -e sched:* -o /opt/perfdata/2/perf.data --overwrite --switch-output -a
> > >
> > > Default perf config has the same daemon base:
> > >
> > > # cat ~/.perfconfig
> > > [daemon]
> > > base=/opt/perfdata
> > >
> > > Starting the daemon:
> > >
> > > # perf daemon --config config.daemon
> >
> > It could make sense to name daemon config file similar to .perfconfig
> > e.g. like .perfconfig.daemon. perf daemon command would then assume, by
> > default, usage of .perfconfig.daemon config or the one specified on the
> > command line via --config option. It also would be helpfull have loaded
> > config file path printed into console:
> > # perf daemon
> > Daemon process <pid> started with config /path/to/.perfconfig.daemon
>
> so the current way is, that following creates daemon:
>
> # perf daemon --config <CONFIG>
>
> and any other 'non --config' option' is used to 'query/control' daemon:
>
> # perf daemon
> # perf daemon --signal
> # perf daemon --stop
> ...
My opinion is that it'd be better having sub-commands for essential
operations like start, stop. Also daemons tend to have 'status' or
'reload' operations too.
# perf daemon start --config ...
# perf daemon stop
As a system daemon, I agree it should follow the standard location
for the default base directory and config file.
Thanks,
Namhyung
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH 4/8] perf daemon: Add daemon command
2020-12-18 13:25 ` Namhyung Kim
@ 2020-12-18 19:30 ` Jiri Olsa
0 siblings, 0 replies; 27+ messages in thread
From: Jiri Olsa @ 2020-12-18 19:30 UTC (permalink / raw)
To: Namhyung Kim
Cc: Alexei Budankov, Jiri Olsa, Arnaldo Carvalho de Melo, lkml,
Peter Zijlstra, Ingo Molnar, Mark Rutland, Alexander Shishkin,
Michael Petlan, Ian Rogers, Stephane Eranian
On Fri, Dec 18, 2020 at 10:25:06PM +0900, Namhyung Kim wrote:
SNIP
> >
> > so the current way is, that following creates daemon:
> >
> > # perf daemon --config <CONFIG>
> >
> > and any other 'non --config' option' is used to 'query/control' daemon:
> >
> > # perf daemon
> > # perf daemon --signal
> > # perf daemon --stop
> > ...
>
> My opinion is that it'd be better having sub-commands for essential
> operations like start, stop. Also daemons tend to have 'status' or
> 'reload' operations too.
>
> # perf daemon start --config ...
> # perf daemon stop
ok, seems better
>
> As a system daemon, I agree it should follow the standard location
> for the default base directory and config file.
currently we have this order:
1. custom perfconfig if specified
2. system perf config /etc/perfconfig if exists
3. $HOME/.perfconfig if exists
I think we should keep that, when there's a perf systemd service config
file for this, it can use --config 'whatever'
jirka
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH 4/8] perf daemon: Add daemon command
2020-12-12 10:43 ` [PATCH 4/8] perf daemon: Add daemon command Jiri Olsa
2020-12-15 15:40 ` Alexei Budankov
@ 2020-12-15 15:44 ` Arnaldo Carvalho de Melo
2020-12-15 19:20 ` Jiri Olsa
1 sibling, 1 reply; 27+ messages in thread
From: Arnaldo Carvalho de Melo @ 2020-12-15 15:44 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, Dec 12, 2020 at 11:43:54AM +0100, Jiri Olsa escreveu:
> 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 config.daemon
> [daemon]
> base=/opt/perfdata
>
> [session-1]
> run = -m 10M -e cycles -o /opt/perfdata/1/perf.data --overwrite --switch-output -a
>
> [session-2]
> run = -m 20M -e sched:* -o /opt/perfdata/2/perf.data --overwrite --switch-output -a
>
> Default perf config has the same daemon base:
>
> # cat ~/.perfconfig
> [daemon]
> base=/opt/perfdata
>
> Starting the daemon:
>
> # perf daemon --config config.daemon
>
> Check sessions:
"Check"? You mean that using 'perf daemon' without any args will just
attach to whatever daemon is running and list the current sessions?
So we can only have one daemon running in a machine?
That seems constraining, perhaps we can give each daemon a name?
> # perf daemon
> [1:92187] perf record -m 11M -e cycles -o /opt/perfdata/1/perf.data --overwrite --switch-output -a
> [2:92188] perf record -m 20M -e sched:* -o /opt/perfdata/2/perf.data --overwrite --switch-output -a
>
> Check sessions with more info:
>
> # perf daemon -v
> [1:92187] perf record -m 11M -e cycles -o /opt/perfdata/1/perf.data --overwrite --switch-output -a
> output: /opt/perfdata/1/output
> [2:92188] perf record -m 20M -e sched:* -o /opt/perfdata/2/perf.data --overwrite --switch-output -a
> output: /opt/perfdata/2/output
>
> The 'output' file is perf record output for specific session.
>
> Signed-off-by: Jiri Olsa <jolsa@kernel.org>
> ---
> tools/perf/Build | 3 +
> tools/perf/Documentation/perf-daemon.txt | 97 +++
> tools/perf/builtin-daemon.c | 794 +++++++++++++++++++++++
> tools/perf/builtin.h | 1 +
> tools/perf/command-list.txt | 1 +
> tools/perf/perf.c | 1 +
> 6 files changed, 897 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..54aa38996fff 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
> @@ -53,3 +54,5 @@ perf-y += scripts/
> perf-$(CONFIG_TRACE) += trace/beauty/
>
> gtk-y += ui/gtk/
> +
> +CFLAGS_builtin-daemon.o += -DPERF="BUILD_STR($(bindir_SQ)/perf)"
> diff --git a/tools/perf/Documentation/perf-daemon.txt b/tools/perf/Documentation/perf-daemon.txt
> new file mode 100644
> index 000000000000..dee39be110ba
> --- /dev/null
> +++ b/tools/perf/Documentation/perf-daemon.txt
> @@ -0,0 +1,97 @@
> +perf-daemon(1)
> +==============
> +
> +NAME
> +----
> +perf-daemon - Run record sessions on background
> +
> +SYNOPSIS
> +--------
> +[verse]
> +'perf daemon'
> +'perf daemon' [<options>]
> +
> +DESCRIPTION
> +-----------
> +This command allows to run simple daemon process that starts and
> +monitors configured record sessions.
> +
> +Each session represents one perf record process.
> +
> +These sessions are configured through config file, see CONFIG FILE
> +section with EXAMPLES.
> +
> +OPTIONS
> +-------
> +--config=<PATH>::
> + Config file path.
> +
> +-f::
> +--foreground::
> + Do not put the process in background.
> +
> +-v::
> +--verbose::
> + Be more verbose.
> +
> +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.
> +
> +EXAMPLES
> +--------
> +Example with 2 record sessions:
> +
> + # cat config.daemon
> + [daemon]
> + base=/opt/perfdata
> +
> + [session-1]
> + run = -m 10M -e cycles -o /opt/perfdata/1/perf.data --overwrite --switch-output -a
> +
> + [session-2]
> + run = -m 20M -e sched:* -o /opt/perfdata/2/perf.data --overwrite --switch-output -a
> +
> +
> +Default perf config has the same daemon base:
> +
> + # cat ~/.perfconfig
> + [daemon]
> + base=/opt/perfdata
> +
> +
> +Starting the daemon:
> +
> + # perf daemon --config config.daemon
> +
> +
> +Check sessions:
> +
> + # perf daemon
> + [1:92187] perf record -m 11M -e cycles -o /opt/perfdata/1/perf.data --overwrite --switch-output -a
> + [2:92188] perf record -m 20M -e sched:* -o /opt/perfdata/2/perf.data --overwrite --switch-output -a
> +
> +
> +Check sessions with more info:
> +
> + # perf daemon -v
> + [1:92187] perf record -m 11M -e cycles -o /opt/perfdata/1/perf.data --overwrite --switch-output -a
> + output: /opt/perfdata/1/output
> + [2:92188] perf record -m 20M -e sched:* -o /opt/perfdata/2/perf.data --overwrite --switch-output -a
> + output: /opt/perfdata/2/output
> +
> +The 'output' file is perf record output for specific session.
> +
> +
> +SEE ALSO
> +--------
> +linkperf:perf-record[1], linkperf:perf-config[1]
> diff --git a/tools/perf/builtin-daemon.c b/tools/perf/builtin-daemon.c
> new file mode 100644
> index 000000000000..7f455837d58a
> --- /dev/null
> +++ b/tools/perf/builtin-daemon.c
> @@ -0,0 +1,794 @@
> +// 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 <api/fs/fs.h>
> +
> +#define SESSION_OUTPUT "output"
> +
> +enum session_state {
> + SESSION_STATE__OK,
> + SESSION_STATE__RECONFIG,
> + SESSION_STATE__KILL,
> +};
> +
> +struct session {
> + char *name;
> + char *run;
> + int pid;
> + struct list_head list;
> + enum session_state state;
> +};
> +
> +struct daemon {
> + char *config;
> + char *config_base;
> + char *base;
> + struct list_head sessions;
> + FILE *out;
> +};
> +
> +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 session_name(const char *var, char *session, int len)
> +{
> + const char *p = var + sizeof("session-") - 1;
> +
> + while (*p != '.' && 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 (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_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__new_file(daemon->config);
> + if (set) {
> + err = perf_config_set(set, server_config, daemon);
> + perf_config_set__delete(set);
> + }
> +
> + return err;
> +}
> +
> +static int session__check(struct session *session, struct daemon *daemon)
> +{
> + int err, status;
> +
> + err = waitpid(session->pid, &status, WNOHANG);
> + if (err < 0) {
> + session->pid = -1;
> + return -1;
> + }
> +
> + if (err && WIFEXITED(status)) {
> + fprintf(daemon->out, "session(%d) %s excited with %d\n",
> + session->pid, session->name, WEXITSTATUS(status));
> + session->state = SESSION_STATE__KILL;
> + session->pid = -1;
> + return -1;
> + }
> +
> + return 0;
> +}
> +
> +static int session__wait(struct session *session, struct daemon *daemon,
> + int secs)
> +{
> + time_t current, start = 0;
> + int cnt;
> +
> + start = current = time(NULL);
> +
> + do {
> + usleep(500);
> + cnt = session__check(session, daemon);
> + if (cnt)
> + break;
> +
> + current = time(NULL);
> + } while ((start + secs > current));
> +
> + return cnt;
> +}
> +
> +static int session__signal(struct session *session, int sig)
> +{
> + if (session->pid < 0)
> + return -1;
> + return kill(session->pid, sig);
> +}
> +
> +static void session__kill(struct session *session, struct daemon *daemon)
> +{
> + session__signal(session, SIGTERM);
> + if (session__wait(session, daemon, 30))
> + session__signal(session, SIGKILL);
> +}
> +
> +static int session__run(struct session *session, struct daemon *daemon)
> +{
> + char base[PATH_MAX];
> + char buf[PATH_MAX];
> + char **argv;
> + int argc, fd;
> +
> + scnprintf(base, PATH_MAX, "%s/%s", daemon->base, session->name);
> +
> + if (mkdir(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(base)) {
> + perror("chdir failed");
> + return -1;
> + }
> +
> + fd = open(SESSION_OUTPUT, O_RDWR|O_CREAT|O_TRUNC, 0644);
> + if (fd < 0) {
> + perror("open failed");
> + return -1;
> + }
> +
> + close(0);
> + dup2(fd, 1);
> + dup2(fd, 2);
> + close(fd);
> +
> + scnprintf(buf, sizeof(buf), "%s record %s", PERF, session->run);
> +
> + argv = argv_split(buf, &argc);
> + if (!argv)
> + exit(-1);
> +
> + exit(execve(PERF, argv, NULL));
> + return -1;
> +}
> +
> +static int daemon__check(struct daemon *daemon)
> +{
> + struct session *session;
> + int cnt = 0;
> +
> + list_for_each_entry(session, &daemon->sessions, list) {
> + if (session__check(session, daemon))
> + continue;
> + cnt++;
> + }
> +
> + return cnt;
> +}
> +
> +static int daemon__wait(struct daemon *daemon, int secs)
> +{
> + time_t current, start = 0;
> + int cnt;
> +
> + start = current = time(NULL);
> +
> + do {
> + usleep(100);
> + cnt = daemon__check(daemon);
> + if (!cnt)
> + break;
> +
> + current = time(NULL);
> + } while ((start + secs > current));
> +
> + return cnt;
> +}
> +
> +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->name);
> + free(session->run);
> + free(session);
> +}
> +
> +static void session__remove(struct session *session)
> +{
> + list_del(&session->list);
> + session__free(session);
> +}
> +
> +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, daemon);
> + 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, daemon);
> + 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);
> + if (daemon__wait(daemon, 30))
> + daemon__signal(daemon, SIGKILL);
> +}
> +
> +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);
> +}
> +
> +static void daemon__exit(struct daemon *daemon)
> +{
> + daemon__kill(daemon);
> + daemon__free(daemon);
> + 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 {
> + CMD_LIST = 0,
> + CMD_LIST_VERBOSE = 1,
> + CMD_MAX,
> +};
> +
> +static int cmd_session_list(struct daemon *daemon, FILE *out, bool simple)
> +{
> + struct session *session;
> +
> + list_for_each_entry(session, &daemon->sessions, list) {
> + fprintf(out, "[%s:%d] perf record %s\n",
> + session->name, session->pid, session->run);
> + if (simple)
> + continue;
> + fprintf(out, " output: %s/%s/" SESSION_OUTPUT "\n",
> + daemon->base, session->name);
> + }
> +
> + return 0;
> +}
> +
> +static int handle_server_socket(struct daemon *daemon, int sock_fd)
> +{
> + int ret = -EINVAL, fd;
> + FILE *out;
> + u64 cmd;
> +
> + 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) {
> + case CMD_LIST:
> + case CMD_LIST_VERBOSE:
> + ret = cmd_session_list(daemon, out, cmd == CMD_LIST);
> + 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 setup_config_changes(struct daemon *daemon)
> +{
> + char *basen = strdup(daemon->config);
> + char *dirn = strdup(daemon->config);
> + 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;
> +
> + 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 set_daemon_config(struct daemon *daemon, const char *config)
> +{
> + char *real = realpath(config, NULL);
> +
> + if (!real) {
> + perror("realpath failed");
> + return -1;
> + }
> + daemon->config = real;
> + return 0;
> +}
> +
> +static int __cmd_daemon(struct daemon *daemon, bool foreground, const char *config)
> +{
> + int sock_pos, file_pos, sock_fd, conf_fd;
> + bool reconfig = true;
> + struct fdarray fda;
> + int err = 0;
> +
> + if (set_daemon_config(daemon, config))
> + 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;
> +
> + conf_fd = setup_config_changes(daemon);
> + if (conf_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;
> +
> + 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);
> +
> + while (!done && !err) {
> + if (reconfig) {
> + err = daemon__reconfig(daemon);
> + reconfig = false;
> + }
> +
> + if (fdarray__poll(&fda, 500)) {
> + 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);
> + }
> +
> + if (!daemon__check(daemon)) {
> + fprintf(daemon->out, "no sessions left, bailing out\n");
> + break;
> + }
> + }
> +
> + pr_info("daemon exited\n");
> +
> + close(sock_fd);
> + close(conf_fd);
> +
> + fdarray__exit(&fda);
> + daemon__exit(daemon);
> + return err;
> +}
> +
> +static int send_cmd(struct daemon *daemon, u64 cmd)
> +{
> + char *line = NULL;
> + size_t len = 0;
> + ssize_t nread;
> + FILE *in;
> + int fd;
> +
> + perf_config(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 const char * const daemon_usage[] = {
> + "perf daemon [<options>]",
> + NULL
> +};
> +
> +int cmd_daemon(int argc, const char **argv)
> +{
> + bool foreground = false;
> + const char *config = NULL;
> + struct daemon daemon = {
> + .sessions = LIST_HEAD_INIT(daemon.sessions),
> + .out = stdout,
> + };
> + struct option daemon_options[] = {
> + OPT_INCR('v', "verbose", &verbose, "be more verbose"),
> + OPT_STRING(0, "config", &config,
> + "config file", "config file path"),
> + OPT_BOOLEAN('f', "foreground", &foreground, "stay on console"),
> + OPT_END()
> + };
> +
> + argc = parse_options(argc, argv, daemon_options, daemon_usage, 0);
> + if (argc)
> + usage_with_options(daemon_usage, daemon_options);
> +
> + if (config)
> + return __cmd_daemon(&daemon, foreground, config);
> +
> + return send_cmd(&daemon, verbose ? CMD_LIST_VERBOSE : CMD_LIST);
> +}
> 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
>
--
- Arnaldo
^ permalink raw reply [flat|nested] 27+ messages in thread* Re: [PATCH 4/8] perf daemon: Add daemon command
2020-12-15 15:44 ` Arnaldo Carvalho de Melo
@ 2020-12-15 19:20 ` Jiri Olsa
0 siblings, 0 replies; 27+ messages in thread
From: Jiri Olsa @ 2020-12-15 19:20 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, Dec 15, 2020 at 12:44:08PM -0300, Arnaldo Carvalho de Melo wrote:
> Em Sat, Dec 12, 2020 at 11:43:54AM +0100, Jiri Olsa escreveu:
> > 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 config.daemon
> > [daemon]
> > base=/opt/perfdata
> >
> > [session-1]
> > run = -m 10M -e cycles -o /opt/perfdata/1/perf.data --overwrite --switch-output -a
> >
> > [session-2]
> > run = -m 20M -e sched:* -o /opt/perfdata/2/perf.data --overwrite --switch-output -a
> >
> > Default perf config has the same daemon base:
> >
> > # cat ~/.perfconfig
> > [daemon]
> > base=/opt/perfdata
> >
> > Starting the daemon:
> >
> > # perf daemon --config config.daemon
> >
> > Check sessions:
>
> "Check"? You mean that using 'perf daemon' without any args will just
> attach to whatever daemon is running and list the current sessions?
>
> So we can only have one daemon running in a machine?
>
> That seems constraining, perhaps we can give each daemon a name?
as I explained in the other email, the difference is the base path
specified for both daemon and other commands
I made other changes showing line for daemon process itself
with base path:
$ sudo /opt/perf/bin/perf daemon
[690174:daemon] base: /opt/perfdata
[690175:top] perf record -e cycles --switch-output=1m --switch-max-files=6 -a
$ sudo /opt/perf/bin/perf daemon -v
[690174:daemon] base: /opt/perfdata
output: /opt/perfdata/output
lock: /opt/perfdata/lock
up: 268 minutes
[690175:top] perf record -e cycles --switch-output=1m --switch-max-files=6 -a
base: /opt/perfdata/top
output: /opt/perfdata/top/output
control: /opt/perfdata/top/control
ack: /opt/perfdata/top/ack
up: 268 minutes
jirka
^ permalink raw reply [flat|nested] 27+ messages in thread
* [PATCH 5/8] perf daemon: Add signal command
2020-12-12 10:43 [RFC 0/8] perf tools: Add daemon command Jiri Olsa
` (3 preceding siblings ...)
2020-12-12 10:43 ` [PATCH 4/8] perf daemon: Add daemon command Jiri Olsa
@ 2020-12-12 10:43 ` Jiri Olsa
2020-12-15 15:45 ` Arnaldo Carvalho de Melo
2020-12-12 10:43 ` [PATCH 6/8] perf daemon: Add stop command Jiri Olsa
` (2 subsequent siblings)
7 siblings, 1 reply; 27+ messages in thread
From: Jiri Olsa @ 2020-12-12 10:43 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:
# perf daemon
[1:364758] perf record -m 10M -e cycles -o /opt/perfdata/1/perf.data --overwrite --switch-output -a
[2:364759] perf record -m 10M -e sched:* -o /opt/perfdata/2/perf.data --overwrite --switch-output -a
# perf daemon -s
signal 12 sent to session '1 [92187]'
signal 12 sent to session '2 [92188]'
Or to specific one:
# perf daemon --signal=1
signal 12 sent to session '1 [364758]'
Signed-off-by: Jiri Olsa <jolsa@kernel.org>
---
tools/perf/Documentation/perf-daemon.txt | 24 +++++++++++
tools/perf/builtin-daemon.c | 51 +++++++++++++++++++++++-
2 files changed, 73 insertions(+), 2 deletions(-)
diff --git a/tools/perf/Documentation/perf-daemon.txt b/tools/perf/Documentation/perf-daemon.txt
index dee39be110ba..203ec4bf704c 100644
--- a/tools/perf/Documentation/perf-daemon.txt
+++ b/tools/perf/Documentation/perf-daemon.txt
@@ -30,6 +30,11 @@ OPTIONS
--foreground::
Do not put the process in background.
+-s::
+--signal[=session]::
+ Send SIGUSR2 to specific session, if session is not specified,
+ send SIGUSR2 to all sessions.
+
-v::
--verbose::
Be more verbose.
@@ -92,6 +97,25 @@ Check sessions with more info:
The 'output' file is perf record output for specific session.
+Send SIGUSR2 signal to all sessions:
+
+ # perf daemon -s
+ signal 12 sent to session '1 [92187]'
+ signal 12 sent to session '2 [92188]'
+
+Send SIGUSR2 signal to session '1':
+
+ # perf daemon --signal=1
+ signal 12 sent to session '1 [364758]'
+
+And check that the perf data dump was trigered:
+
+ # cat /opt/perfdata/2/output
+ rounding mmap pages size to 32M (8192 pages)
+ [ perf record: dump data: Woken up 1 times ]
+ [ perf record: Dump /opt/perfdata/2/perf.data.2020120715220385 ]
+
+
SEE ALSO
--------
linkperf:perf-record[1], linkperf:perf-config[1]
diff --git a/tools/perf/builtin-daemon.c b/tools/perf/builtin-daemon.c
index 7f455837d58a..c53d4ddc2b49 100644
--- a/tools/perf/builtin-daemon.c
+++ b/tools/perf/builtin-daemon.c
@@ -450,9 +450,15 @@ static int setup_server_socket(struct daemon *daemon)
enum cmd {
CMD_LIST = 0,
CMD_LIST_VERBOSE = 1,
+ CMD_SIGNAL = 2,
CMD_MAX,
};
+struct cmd_signal {
+ int sig;
+ char name[16];
+};
+
static int cmd_session_list(struct daemon *daemon, FILE *out, bool simple)
{
struct session *session;
@@ -469,6 +475,28 @@ static int cmd_session_list(struct daemon *daemon, FILE *out, bool simple)
return 0;
}
+static int cmd_session_kill(struct daemon *daemon, FILE *out, int fd)
+{
+ struct session *session;
+ struct cmd_signal data;
+ bool all = false;
+
+ if (sizeof(data) != read(fd, &data, sizeof(data)))
+ return -1;
+
+ all = !strcmp(data.name, "all");
+
+ list_for_each_entry(session, &daemon->sessions, list) {
+ if (all || !strcmp(data.name, session->name)) {
+ session__signal(session, data.sig);
+ fprintf(out, "signal %d sent to session '%s [%d]'\n",
+ data.sig, session->name, session->pid);
+ }
+ }
+
+ return 0;
+}
+
static int handle_server_socket(struct daemon *daemon, int sock_fd)
{
int ret = -EINVAL, fd;
@@ -497,6 +525,9 @@ static int handle_server_socket(struct daemon *daemon, int sock_fd)
case CMD_LIST_VERBOSE:
ret = cmd_session_list(daemon, out, cmd == CMD_LIST);
break;
+ case CMD_SIGNAL:
+ ret = cmd_session_kill(daemon, out, fd);
+ break;
default:
break;
}
@@ -730,8 +761,9 @@ static int __cmd_daemon(struct daemon *daemon, bool foreground, const char *conf
return err;
}
-static int send_cmd(struct daemon *daemon, u64 cmd)
+static int send_cmd(struct daemon *daemon, u64 cmd, const char *str)
{
+ struct cmd_signal data;
char *line = NULL;
size_t len = 0;
ssize_t nread;
@@ -747,6 +779,14 @@ static int send_cmd(struct daemon *daemon, u64 cmd)
if (sizeof(cmd) != write(fd, &cmd, sizeof(cmd)))
return -1;
+ if (cmd == CMD_SIGNAL) {
+ data.sig = SIGUSR2;
+ strncpy(data.name, str, sizeof(data.name) - 1);
+
+ if (sizeof(data) != write(fd, &data, sizeof(data)))
+ return -1;
+ }
+
in = fdopen(fd, "r");
if (!in) {
perror("fopen");
@@ -770,7 +810,9 @@ static const char * const daemon_usage[] = {
int cmd_daemon(int argc, const char **argv)
{
bool foreground = false;
+ bool signal = false;
const char *config = NULL;
+ const char *signal_str = NULL;
struct daemon daemon = {
.sessions = LIST_HEAD_INIT(daemon.sessions),
.out = stdout,
@@ -780,6 +822,8 @@ int cmd_daemon(int argc, const char **argv)
OPT_STRING(0, "config", &config,
"config file", "config file path"),
OPT_BOOLEAN('f', "foreground", &foreground, "stay on console"),
+ OPT_STRING_OPTARG_SET('s', "signal", &signal_str, &signal,
+ "signal", "send signal to session", "all"),
OPT_END()
};
@@ -790,5 +834,8 @@ int cmd_daemon(int argc, const char **argv)
if (config)
return __cmd_daemon(&daemon, foreground, config);
- return send_cmd(&daemon, verbose ? CMD_LIST_VERBOSE : CMD_LIST);
+ if (signal)
+ return send_cmd(&daemon, CMD_SIGNAL, signal_str);
+
+ return send_cmd(&daemon, verbose ? CMD_LIST_VERBOSE : CMD_LIST, NULL);
}
--
2.26.2
^ permalink raw reply related [flat|nested] 27+ messages in thread* Re: [PATCH 5/8] perf daemon: Add signal command
2020-12-12 10:43 ` [PATCH 5/8] perf daemon: Add signal command Jiri Olsa
@ 2020-12-15 15:45 ` Arnaldo Carvalho de Melo
2020-12-15 19:14 ` Jiri Olsa
0 siblings, 1 reply; 27+ messages in thread
From: Arnaldo Carvalho de Melo @ 2020-12-15 15:45 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, Dec 12, 2020 at 11:43:55AM +0100, Jiri Olsa escreveu:
> Allow perf daemon to send SIGUSR2 to all running sessions:
>
> # perf daemon
> [1:364758] perf record -m 10M -e cycles -o /opt/perfdata/1/perf.data --overwrite --switch-output -a
> [2:364759] perf record -m 10M -e sched:* -o /opt/perfdata/2/perf.data --overwrite --switch-output -a
The above looks ambiguous, is it starting those sessions at this point
or is it checking if there is a running daemon and then listing its
sessions?
- Arnaldo
> # perf daemon -s
> signal 12 sent to session '1 [92187]'
> signal 12 sent to session '2 [92188]'
>
> Or to specific one:
>
> # perf daemon --signal=1
> signal 12 sent to session '1 [364758]'
>
> Signed-off-by: Jiri Olsa <jolsa@kernel.org>
> ---
> tools/perf/Documentation/perf-daemon.txt | 24 +++++++++++
> tools/perf/builtin-daemon.c | 51 +++++++++++++++++++++++-
> 2 files changed, 73 insertions(+), 2 deletions(-)
>
> diff --git a/tools/perf/Documentation/perf-daemon.txt b/tools/perf/Documentation/perf-daemon.txt
> index dee39be110ba..203ec4bf704c 100644
> --- a/tools/perf/Documentation/perf-daemon.txt
> +++ b/tools/perf/Documentation/perf-daemon.txt
> @@ -30,6 +30,11 @@ OPTIONS
> --foreground::
> Do not put the process in background.
>
> +-s::
> +--signal[=session]::
> + Send SIGUSR2 to specific session, if session is not specified,
> + send SIGUSR2 to all sessions.
> +
> -v::
> --verbose::
> Be more verbose.
> @@ -92,6 +97,25 @@ Check sessions with more info:
> The 'output' file is perf record output for specific session.
>
>
> +Send SIGUSR2 signal to all sessions:
> +
> + # perf daemon -s
> + signal 12 sent to session '1 [92187]'
> + signal 12 sent to session '2 [92188]'
> +
> +Send SIGUSR2 signal to session '1':
> +
> + # perf daemon --signal=1
> + signal 12 sent to session '1 [364758]'
> +
> +And check that the perf data dump was trigered:
> +
> + # cat /opt/perfdata/2/output
> + rounding mmap pages size to 32M (8192 pages)
> + [ perf record: dump data: Woken up 1 times ]
> + [ perf record: Dump /opt/perfdata/2/perf.data.2020120715220385 ]
> +
> +
> SEE ALSO
> --------
> linkperf:perf-record[1], linkperf:perf-config[1]
> diff --git a/tools/perf/builtin-daemon.c b/tools/perf/builtin-daemon.c
> index 7f455837d58a..c53d4ddc2b49 100644
> --- a/tools/perf/builtin-daemon.c
> +++ b/tools/perf/builtin-daemon.c
> @@ -450,9 +450,15 @@ static int setup_server_socket(struct daemon *daemon)
> enum cmd {
> CMD_LIST = 0,
> CMD_LIST_VERBOSE = 1,
> + CMD_SIGNAL = 2,
> CMD_MAX,
> };
>
> +struct cmd_signal {
> + int sig;
> + char name[16];
> +};
> +
> static int cmd_session_list(struct daemon *daemon, FILE *out, bool simple)
> {
> struct session *session;
> @@ -469,6 +475,28 @@ static int cmd_session_list(struct daemon *daemon, FILE *out, bool simple)
> return 0;
> }
>
> +static int cmd_session_kill(struct daemon *daemon, FILE *out, int fd)
> +{
> + struct session *session;
> + struct cmd_signal data;
> + bool all = false;
> +
> + if (sizeof(data) != read(fd, &data, sizeof(data)))
> + return -1;
> +
> + all = !strcmp(data.name, "all");
> +
> + list_for_each_entry(session, &daemon->sessions, list) {
> + if (all || !strcmp(data.name, session->name)) {
> + session__signal(session, data.sig);
> + fprintf(out, "signal %d sent to session '%s [%d]'\n",
> + data.sig, session->name, session->pid);
> + }
> + }
> +
> + return 0;
> +}
> +
> static int handle_server_socket(struct daemon *daemon, int sock_fd)
> {
> int ret = -EINVAL, fd;
> @@ -497,6 +525,9 @@ static int handle_server_socket(struct daemon *daemon, int sock_fd)
> case CMD_LIST_VERBOSE:
> ret = cmd_session_list(daemon, out, cmd == CMD_LIST);
> break;
> + case CMD_SIGNAL:
> + ret = cmd_session_kill(daemon, out, fd);
> + break;
> default:
> break;
> }
> @@ -730,8 +761,9 @@ static int __cmd_daemon(struct daemon *daemon, bool foreground, const char *conf
> return err;
> }
>
> -static int send_cmd(struct daemon *daemon, u64 cmd)
> +static int send_cmd(struct daemon *daemon, u64 cmd, const char *str)
> {
> + struct cmd_signal data;
> char *line = NULL;
> size_t len = 0;
> ssize_t nread;
> @@ -747,6 +779,14 @@ static int send_cmd(struct daemon *daemon, u64 cmd)
> if (sizeof(cmd) != write(fd, &cmd, sizeof(cmd)))
> return -1;
>
> + if (cmd == CMD_SIGNAL) {
> + data.sig = SIGUSR2;
> + strncpy(data.name, str, sizeof(data.name) - 1);
> +
> + if (sizeof(data) != write(fd, &data, sizeof(data)))
> + return -1;
> + }
> +
> in = fdopen(fd, "r");
> if (!in) {
> perror("fopen");
> @@ -770,7 +810,9 @@ static const char * const daemon_usage[] = {
> int cmd_daemon(int argc, const char **argv)
> {
> bool foreground = false;
> + bool signal = false;
> const char *config = NULL;
> + const char *signal_str = NULL;
> struct daemon daemon = {
> .sessions = LIST_HEAD_INIT(daemon.sessions),
> .out = stdout,
> @@ -780,6 +822,8 @@ int cmd_daemon(int argc, const char **argv)
> OPT_STRING(0, "config", &config,
> "config file", "config file path"),
> OPT_BOOLEAN('f', "foreground", &foreground, "stay on console"),
> + OPT_STRING_OPTARG_SET('s', "signal", &signal_str, &signal,
> + "signal", "send signal to session", "all"),
> OPT_END()
> };
>
> @@ -790,5 +834,8 @@ int cmd_daemon(int argc, const char **argv)
> if (config)
> return __cmd_daemon(&daemon, foreground, config);
>
> - return send_cmd(&daemon, verbose ? CMD_LIST_VERBOSE : CMD_LIST);
> + if (signal)
> + return send_cmd(&daemon, CMD_SIGNAL, signal_str);
> +
> + return send_cmd(&daemon, verbose ? CMD_LIST_VERBOSE : CMD_LIST, NULL);
> }
> --
> 2.26.2
>
--
- Arnaldo
^ permalink raw reply [flat|nested] 27+ messages in thread* Re: [PATCH 5/8] perf daemon: Add signal command
2020-12-15 15:45 ` Arnaldo Carvalho de Melo
@ 2020-12-15 19:14 ` Jiri Olsa
0 siblings, 0 replies; 27+ messages in thread
From: Jiri Olsa @ 2020-12-15 19:14 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, Dec 15, 2020 at 12:45:21PM -0300, Arnaldo Carvalho de Melo wrote:
> Em Sat, Dec 12, 2020 at 11:43:55AM +0100, Jiri Olsa escreveu:
> > Allow perf daemon to send SIGUSR2 to all running sessions:
> >
> > # perf daemon
> > [1:364758] perf record -m 10M -e cycles -o /opt/perfdata/1/perf.data --overwrite --switch-output -a
> > [2:364759] perf record -m 10M -e sched:* -o /opt/perfdata/2/perf.data --overwrite --switch-output -a
>
> The above looks ambiguous, is it starting those sessions at this point
> or is it checking if there is a running daemon and then listing its
> sessions?
so 'perf daemon --config <CONFIGFILE>' runs daemon based
on CONFIGFILE settings
perf daemon <other options> reads standard .perfconfig and
expects:
[daemon]
base=PATH
and tries to connect to daemon that runs on top of PATH
I think --base should be also command line option
jirka
>
> - Arnaldo
>
> > # perf daemon -s
> > signal 12 sent to session '1 [92187]'
> > signal 12 sent to session '2 [92188]'
> >
> > Or to specific one:
> >
> > # perf daemon --signal=1
> > signal 12 sent to session '1 [364758]'
> >
> > Signed-off-by: Jiri Olsa <jolsa@kernel.org>
> > ---
> > tools/perf/Documentation/perf-daemon.txt | 24 +++++++++++
> > tools/perf/builtin-daemon.c | 51 +++++++++++++++++++++++-
> > 2 files changed, 73 insertions(+), 2 deletions(-)
> >
> > diff --git a/tools/perf/Documentation/perf-daemon.txt b/tools/perf/Documentation/perf-daemon.txt
> > index dee39be110ba..203ec4bf704c 100644
> > --- a/tools/perf/Documentation/perf-daemon.txt
> > +++ b/tools/perf/Documentation/perf-daemon.txt
> > @@ -30,6 +30,11 @@ OPTIONS
> > --foreground::
> > Do not put the process in background.
> >
> > +-s::
> > +--signal[=session]::
> > + Send SIGUSR2 to specific session, if session is not specified,
> > + send SIGUSR2 to all sessions.
> > +
> > -v::
> > --verbose::
> > Be more verbose.
> > @@ -92,6 +97,25 @@ Check sessions with more info:
> > The 'output' file is perf record output for specific session.
> >
> >
> > +Send SIGUSR2 signal to all sessions:
> > +
> > + # perf daemon -s
> > + signal 12 sent to session '1 [92187]'
> > + signal 12 sent to session '2 [92188]'
> > +
> > +Send SIGUSR2 signal to session '1':
> > +
> > + # perf daemon --signal=1
> > + signal 12 sent to session '1 [364758]'
> > +
> > +And check that the perf data dump was trigered:
> > +
> > + # cat /opt/perfdata/2/output
> > + rounding mmap pages size to 32M (8192 pages)
> > + [ perf record: dump data: Woken up 1 times ]
> > + [ perf record: Dump /opt/perfdata/2/perf.data.2020120715220385 ]
> > +
> > +
> > SEE ALSO
> > --------
> > linkperf:perf-record[1], linkperf:perf-config[1]
> > diff --git a/tools/perf/builtin-daemon.c b/tools/perf/builtin-daemon.c
> > index 7f455837d58a..c53d4ddc2b49 100644
> > --- a/tools/perf/builtin-daemon.c
> > +++ b/tools/perf/builtin-daemon.c
> > @@ -450,9 +450,15 @@ static int setup_server_socket(struct daemon *daemon)
> > enum cmd {
> > CMD_LIST = 0,
> > CMD_LIST_VERBOSE = 1,
> > + CMD_SIGNAL = 2,
> > CMD_MAX,
> > };
> >
> > +struct cmd_signal {
> > + int sig;
> > + char name[16];
> > +};
> > +
> > static int cmd_session_list(struct daemon *daemon, FILE *out, bool simple)
> > {
> > struct session *session;
> > @@ -469,6 +475,28 @@ static int cmd_session_list(struct daemon *daemon, FILE *out, bool simple)
> > return 0;
> > }
> >
> > +static int cmd_session_kill(struct daemon *daemon, FILE *out, int fd)
> > +{
> > + struct session *session;
> > + struct cmd_signal data;
> > + bool all = false;
> > +
> > + if (sizeof(data) != read(fd, &data, sizeof(data)))
> > + return -1;
> > +
> > + all = !strcmp(data.name, "all");
> > +
> > + list_for_each_entry(session, &daemon->sessions, list) {
> > + if (all || !strcmp(data.name, session->name)) {
> > + session__signal(session, data.sig);
> > + fprintf(out, "signal %d sent to session '%s [%d]'\n",
> > + data.sig, session->name, session->pid);
> > + }
> > + }
> > +
> > + return 0;
> > +}
> > +
> > static int handle_server_socket(struct daemon *daemon, int sock_fd)
> > {
> > int ret = -EINVAL, fd;
> > @@ -497,6 +525,9 @@ static int handle_server_socket(struct daemon *daemon, int sock_fd)
> > case CMD_LIST_VERBOSE:
> > ret = cmd_session_list(daemon, out, cmd == CMD_LIST);
> > break;
> > + case CMD_SIGNAL:
> > + ret = cmd_session_kill(daemon, out, fd);
> > + break;
> > default:
> > break;
> > }
> > @@ -730,8 +761,9 @@ static int __cmd_daemon(struct daemon *daemon, bool foreground, const char *conf
> > return err;
> > }
> >
> > -static int send_cmd(struct daemon *daemon, u64 cmd)
> > +static int send_cmd(struct daemon *daemon, u64 cmd, const char *str)
> > {
> > + struct cmd_signal data;
> > char *line = NULL;
> > size_t len = 0;
> > ssize_t nread;
> > @@ -747,6 +779,14 @@ static int send_cmd(struct daemon *daemon, u64 cmd)
> > if (sizeof(cmd) != write(fd, &cmd, sizeof(cmd)))
> > return -1;
> >
> > + if (cmd == CMD_SIGNAL) {
> > + data.sig = SIGUSR2;
> > + strncpy(data.name, str, sizeof(data.name) - 1);
> > +
> > + if (sizeof(data) != write(fd, &data, sizeof(data)))
> > + return -1;
> > + }
> > +
> > in = fdopen(fd, "r");
> > if (!in) {
> > perror("fopen");
> > @@ -770,7 +810,9 @@ static const char * const daemon_usage[] = {
> > int cmd_daemon(int argc, const char **argv)
> > {
> > bool foreground = false;
> > + bool signal = false;
> > const char *config = NULL;
> > + const char *signal_str = NULL;
> > struct daemon daemon = {
> > .sessions = LIST_HEAD_INIT(daemon.sessions),
> > .out = stdout,
> > @@ -780,6 +822,8 @@ int cmd_daemon(int argc, const char **argv)
> > OPT_STRING(0, "config", &config,
> > "config file", "config file path"),
> > OPT_BOOLEAN('f', "foreground", &foreground, "stay on console"),
> > + OPT_STRING_OPTARG_SET('s', "signal", &signal_str, &signal,
> > + "signal", "send signal to session", "all"),
> > OPT_END()
> > };
> >
> > @@ -790,5 +834,8 @@ int cmd_daemon(int argc, const char **argv)
> > if (config)
> > return __cmd_daemon(&daemon, foreground, config);
> >
> > - return send_cmd(&daemon, verbose ? CMD_LIST_VERBOSE : CMD_LIST);
> > + if (signal)
> > + return send_cmd(&daemon, CMD_SIGNAL, signal_str);
> > +
> > + return send_cmd(&daemon, verbose ? CMD_LIST_VERBOSE : CMD_LIST, NULL);
> > }
> > --
> > 2.26.2
> >
>
> --
>
> - Arnaldo
>
^ permalink raw reply [flat|nested] 27+ messages in thread
* [PATCH 6/8] perf daemon: Add stop command
2020-12-12 10:43 [RFC 0/8] perf tools: Add daemon command Jiri Olsa
` (4 preceding siblings ...)
2020-12-12 10:43 ` [PATCH 5/8] perf daemon: Add signal command Jiri Olsa
@ 2020-12-12 10:43 ` Jiri Olsa
2020-12-15 15:45 ` Arnaldo Carvalho de Melo
2020-12-12 10:43 ` [PATCH 7/8] perf daemon: Allow only one daemon over base directory Jiri Olsa
2020-12-12 10:43 ` [PATCH 8/8] perf daemon: Set control fifo for session Jiri Olsa
7 siblings, 1 reply; 27+ messages in thread
From: Jiri Olsa @ 2020-12-12 10:43 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 stop daemon process:
# perf daemon --stop
perf daemon is exciting
Signed-off-by: Jiri Olsa <jolsa@kernel.org>
---
tools/perf/Documentation/perf-daemon.txt | 8 ++++++++
tools/perf/builtin-daemon.c | 10 ++++++++++
2 files changed, 18 insertions(+)
diff --git a/tools/perf/Documentation/perf-daemon.txt b/tools/perf/Documentation/perf-daemon.txt
index 203ec4bf704c..87de2c77e4c7 100644
--- a/tools/perf/Documentation/perf-daemon.txt
+++ b/tools/perf/Documentation/perf-daemon.txt
@@ -35,6 +35,9 @@ OPTIONS
Send SIGUSR2 to specific session, if session is not specified,
send SIGUSR2 to all sessions.
+--stop::
+ Stop daemon.
+
-v::
--verbose::
Be more verbose.
@@ -116,6 +119,11 @@ And check that the perf data dump was trigered:
[ perf record: Dump /opt/perfdata/2/perf.data.2020120715220385 ]
+Stop daemon:
+
+ # perf daemon --stop
+ perf daemon is exciting
+
SEE ALSO
--------
linkperf:perf-record[1], linkperf:perf-config[1]
diff --git a/tools/perf/builtin-daemon.c b/tools/perf/builtin-daemon.c
index c53d4ddc2b49..855fed2fe364 100644
--- a/tools/perf/builtin-daemon.c
+++ b/tools/perf/builtin-daemon.c
@@ -451,6 +451,7 @@ enum cmd {
CMD_LIST = 0,
CMD_LIST_VERBOSE = 1,
CMD_SIGNAL = 2,
+ CMD_STOP = 3,
CMD_MAX,
};
@@ -528,6 +529,10 @@ static int handle_server_socket(struct daemon *daemon, int sock_fd)
case CMD_SIGNAL:
ret = cmd_session_kill(daemon, out, fd);
break;
+ case CMD_STOP:
+ done = 1;
+ pr_debug("perf daemon is exciting\n");
+ break;
default:
break;
}
@@ -811,6 +816,7 @@ int cmd_daemon(int argc, const char **argv)
{
bool foreground = false;
bool signal = false;
+ bool stop = false;
const char *config = NULL;
const char *signal_str = NULL;
struct daemon daemon = {
@@ -821,6 +827,7 @@ int cmd_daemon(int argc, const char **argv)
OPT_INCR('v', "verbose", &verbose, "be more verbose"),
OPT_STRING(0, "config", &config,
"config file", "config file path"),
+ OPT_BOOLEAN(0, "stop", &stop, "stop daemon"),
OPT_BOOLEAN('f', "foreground", &foreground, "stay on console"),
OPT_STRING_OPTARG_SET('s', "signal", &signal_str, &signal,
"signal", "send signal to session", "all"),
@@ -837,5 +844,8 @@ int cmd_daemon(int argc, const char **argv)
if (signal)
return send_cmd(&daemon, CMD_SIGNAL, signal_str);
+ if (stop)
+ return send_cmd(&daemon, CMD_STOP, NULL);
+
return send_cmd(&daemon, verbose ? CMD_LIST_VERBOSE : CMD_LIST, NULL);
}
--
2.26.2
^ permalink raw reply related [flat|nested] 27+ messages in thread* Re: [PATCH 6/8] perf daemon: Add stop command
2020-12-12 10:43 ` [PATCH 6/8] perf daemon: Add stop command Jiri Olsa
@ 2020-12-15 15:45 ` Arnaldo Carvalho de Melo
0 siblings, 0 replies; 27+ messages in thread
From: Arnaldo Carvalho de Melo @ 2020-12-15 15:45 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, Dec 12, 2020 at 11:43:56AM +0100, Jiri Olsa escreveu:
> Allow 'perf daemon' to stop daemon process:
>
> # perf daemon --stop
> perf daemon is exciting
It looks exciting, yes, a nice functionality! ;-)
- Arnaldo
> Signed-off-by: Jiri Olsa <jolsa@kernel.org>
> ---
> tools/perf/Documentation/perf-daemon.txt | 8 ++++++++
> tools/perf/builtin-daemon.c | 10 ++++++++++
> 2 files changed, 18 insertions(+)
>
> diff --git a/tools/perf/Documentation/perf-daemon.txt b/tools/perf/Documentation/perf-daemon.txt
> index 203ec4bf704c..87de2c77e4c7 100644
> --- a/tools/perf/Documentation/perf-daemon.txt
> +++ b/tools/perf/Documentation/perf-daemon.txt
> @@ -35,6 +35,9 @@ OPTIONS
> Send SIGUSR2 to specific session, if session is not specified,
> send SIGUSR2 to all sessions.
>
> +--stop::
> + Stop daemon.
> +
> -v::
> --verbose::
> Be more verbose.
> @@ -116,6 +119,11 @@ And check that the perf data dump was trigered:
> [ perf record: Dump /opt/perfdata/2/perf.data.2020120715220385 ]
>
>
> +Stop daemon:
> +
> + # perf daemon --stop
> + perf daemon is exciting
> +
> SEE ALSO
> --------
> linkperf:perf-record[1], linkperf:perf-config[1]
> diff --git a/tools/perf/builtin-daemon.c b/tools/perf/builtin-daemon.c
> index c53d4ddc2b49..855fed2fe364 100644
> --- a/tools/perf/builtin-daemon.c
> +++ b/tools/perf/builtin-daemon.c
> @@ -451,6 +451,7 @@ enum cmd {
> CMD_LIST = 0,
> CMD_LIST_VERBOSE = 1,
> CMD_SIGNAL = 2,
> + CMD_STOP = 3,
> CMD_MAX,
> };
>
> @@ -528,6 +529,10 @@ static int handle_server_socket(struct daemon *daemon, int sock_fd)
> case CMD_SIGNAL:
> ret = cmd_session_kill(daemon, out, fd);
> break;
> + case CMD_STOP:
> + done = 1;
> + pr_debug("perf daemon is exciting\n");
> + break;
> default:
> break;
> }
> @@ -811,6 +816,7 @@ int cmd_daemon(int argc, const char **argv)
> {
> bool foreground = false;
> bool signal = false;
> + bool stop = false;
> const char *config = NULL;
> const char *signal_str = NULL;
> struct daemon daemon = {
> @@ -821,6 +827,7 @@ int cmd_daemon(int argc, const char **argv)
> OPT_INCR('v', "verbose", &verbose, "be more verbose"),
> OPT_STRING(0, "config", &config,
> "config file", "config file path"),
> + OPT_BOOLEAN(0, "stop", &stop, "stop daemon"),
> OPT_BOOLEAN('f', "foreground", &foreground, "stay on console"),
> OPT_STRING_OPTARG_SET('s', "signal", &signal_str, &signal,
> "signal", "send signal to session", "all"),
> @@ -837,5 +844,8 @@ int cmd_daemon(int argc, const char **argv)
> if (signal)
> return send_cmd(&daemon, CMD_SIGNAL, signal_str);
>
> + if (stop)
> + return send_cmd(&daemon, CMD_STOP, NULL);
> +
> return send_cmd(&daemon, verbose ? CMD_LIST_VERBOSE : CMD_LIST, NULL);
> }
> --
> 2.26.2
>
--
- Arnaldo
^ permalink raw reply [flat|nested] 27+ messages in thread
* [PATCH 7/8] perf daemon: Allow only one daemon over base directory
2020-12-12 10:43 [RFC 0/8] perf tools: Add daemon command Jiri Olsa
` (5 preceding siblings ...)
2020-12-12 10:43 ` [PATCH 6/8] perf daemon: Add stop command Jiri Olsa
@ 2020-12-12 10:43 ` Jiri Olsa
2020-12-15 15:46 ` Arnaldo Carvalho de Melo
2020-12-12 10:43 ` [PATCH 8/8] perf daemon: Set control fifo for session Jiri Olsa
7 siblings, 1 reply; 27+ messages in thread
From: Jiri Olsa @ 2020-12-12 10:43 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.
# perf daemon --config ./config.daemon
# perf daemon --config ./config.daemon
failed: another perf daemon (pid 369675) owns /opt/perfdata
Signed-off-by: Jiri Olsa <jolsa@kernel.org>
---
tools/perf/builtin-daemon.c | 43 +++++++++++++++++++++++++++++++++++++
1 file changed, 43 insertions(+)
diff --git a/tools/perf/builtin-daemon.c b/tools/perf/builtin-daemon.c
index 855fed2fe364..1bd5432a57a3 100644
--- a/tools/perf/builtin-daemon.c
+++ b/tools/perf/builtin-daemon.c
@@ -8,6 +8,7 @@
#include <string.h>
#include <sys/types.h>
#include <sys/wait.h>
+#include <sys/file.h>
#include <signal.h>
#include <stdlib.h>
#include <time.h>
@@ -639,6 +640,42 @@ static int handle_config_changes(struct daemon *daemon, int conf_fd,
return 0;
}
+static int check_lock(struct daemon *daemon)
+{
+ char path[PATH_MAX];
+ char buf[20];
+ int fd, pid;
+ ssize_t len;
+
+ scnprintf(path, sizeof(path), "%s/lock", daemon->base);
+
+ fd = open(path, O_RDWR|O_CREAT, 0640);
+ if (fd < 0)
+ return -1;
+
+ if (lockf(fd, F_TLOCK, 0) < 0) {
+ filename__read_int(path, &pid);
+ fprintf(stderr, "failed: another perf daemon (pid %d) owns %s\n",
+ pid, daemon->base);
+ return -1;
+ }
+
+ scnprintf(buf, sizeof(buf), "%d", getpid());
+ len = strlen(buf);
+
+ if (write(fd, buf, len) != len) {
+ perror("write failed");
+ return -1;
+ }
+
+ if (ftruncate(fd, len)) {
+ perror("ftruncate failed");
+ return -1;
+ }
+
+ return 0;
+}
+
static int go_background(struct daemon *daemon)
{
int pid, fd;
@@ -653,6 +690,9 @@ static int go_background(struct daemon *daemon)
if (setsid() < 0)
return -1;
+ if (check_lock(daemon))
+ return -1;
+
umask(0);
if (chdir(daemon->base)) {
@@ -704,6 +744,9 @@ static int __cmd_daemon(struct daemon *daemon, bool foreground, const char *conf
if (setup_server_config(daemon))
return -1;
+ if (foreground && check_lock(daemon))
+ return -1;
+
if (!foreground && go_background(daemon))
return -1;
--
2.26.2
^ permalink raw reply related [flat|nested] 27+ messages in thread* Re: [PATCH 7/8] perf daemon: Allow only one daemon over base directory
2020-12-12 10:43 ` [PATCH 7/8] perf daemon: Allow only one daemon over base directory Jiri Olsa
@ 2020-12-15 15:46 ` Arnaldo Carvalho de Melo
2020-12-15 19:16 ` Jiri Olsa
0 siblings, 1 reply; 27+ messages in thread
From: Arnaldo Carvalho de Melo @ 2020-12-15 15:46 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, Dec 12, 2020 at 11:43:57AM +0100, Jiri Olsa escreveu:
> Add 'lock' file under daemon base and flock it, so only one
> perf daemon can run on top of it.
>
> # perf daemon --config ./config.daemon
> # perf daemon --config ./config.daemon
> failed: another perf daemon (pid 369675) owns /opt/perfdata
So the way to disambiguate is the "daemon base"?
- Arnaldo
> Signed-off-by: Jiri Olsa <jolsa@kernel.org>
> ---
> tools/perf/builtin-daemon.c | 43 +++++++++++++++++++++++++++++++++++++
> 1 file changed, 43 insertions(+)
>
> diff --git a/tools/perf/builtin-daemon.c b/tools/perf/builtin-daemon.c
> index 855fed2fe364..1bd5432a57a3 100644
> --- a/tools/perf/builtin-daemon.c
> +++ b/tools/perf/builtin-daemon.c
> @@ -8,6 +8,7 @@
> #include <string.h>
> #include <sys/types.h>
> #include <sys/wait.h>
> +#include <sys/file.h>
> #include <signal.h>
> #include <stdlib.h>
> #include <time.h>
> @@ -639,6 +640,42 @@ static int handle_config_changes(struct daemon *daemon, int conf_fd,
> return 0;
> }
>
> +static int check_lock(struct daemon *daemon)
> +{
> + char path[PATH_MAX];
> + char buf[20];
> + int fd, pid;
> + ssize_t len;
> +
> + scnprintf(path, sizeof(path), "%s/lock", daemon->base);
> +
> + fd = open(path, O_RDWR|O_CREAT, 0640);
> + if (fd < 0)
> + return -1;
> +
> + if (lockf(fd, F_TLOCK, 0) < 0) {
> + filename__read_int(path, &pid);
> + fprintf(stderr, "failed: another perf daemon (pid %d) owns %s\n",
> + pid, daemon->base);
> + return -1;
> + }
> +
> + scnprintf(buf, sizeof(buf), "%d", getpid());
> + len = strlen(buf);
> +
> + if (write(fd, buf, len) != len) {
> + perror("write failed");
> + return -1;
> + }
> +
> + if (ftruncate(fd, len)) {
> + perror("ftruncate failed");
> + return -1;
> + }
> +
> + return 0;
> +}
> +
> static int go_background(struct daemon *daemon)
> {
> int pid, fd;
> @@ -653,6 +690,9 @@ static int go_background(struct daemon *daemon)
> if (setsid() < 0)
> return -1;
>
> + if (check_lock(daemon))
> + return -1;
> +
> umask(0);
>
> if (chdir(daemon->base)) {
> @@ -704,6 +744,9 @@ static int __cmd_daemon(struct daemon *daemon, bool foreground, const char *conf
> if (setup_server_config(daemon))
> return -1;
>
> + if (foreground && check_lock(daemon))
> + return -1;
> +
> if (!foreground && go_background(daemon))
> return -1;
>
> --
> 2.26.2
>
--
- Arnaldo
^ permalink raw reply [flat|nested] 27+ messages in thread* Re: [PATCH 7/8] perf daemon: Allow only one daemon over base directory
2020-12-15 15:46 ` Arnaldo Carvalho de Melo
@ 2020-12-15 19:16 ` Jiri Olsa
0 siblings, 0 replies; 27+ messages in thread
From: Jiri Olsa @ 2020-12-15 19:16 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, Dec 15, 2020 at 12:46:27PM -0300, Arnaldo Carvalho de Melo wrote:
> Em Sat, Dec 12, 2020 at 11:43:57AM +0100, Jiri Olsa escreveu:
> > Add 'lock' file under daemon base and flock it, so only one
> > perf daemon can run on top of it.
> >
> > # perf daemon --config ./config.daemon
> > # perf daemon --config ./config.daemon
> > failed: another perf daemon (pid 369675) owns /opt/perfdata
>
> So the way to disambiguate is the "daemon base"?
yes, the 'base' is path where perf daemon will create
all control files and 'session' directories and does
chdir for 'perf record' session on top of that
jirka
>
> - Arnaldo
>
> > Signed-off-by: Jiri Olsa <jolsa@kernel.org>
> > ---
> > tools/perf/builtin-daemon.c | 43 +++++++++++++++++++++++++++++++++++++
> > 1 file changed, 43 insertions(+)
> >
> > diff --git a/tools/perf/builtin-daemon.c b/tools/perf/builtin-daemon.c
> > index 855fed2fe364..1bd5432a57a3 100644
> > --- a/tools/perf/builtin-daemon.c
> > +++ b/tools/perf/builtin-daemon.c
> > @@ -8,6 +8,7 @@
> > #include <string.h>
> > #include <sys/types.h>
> > #include <sys/wait.h>
> > +#include <sys/file.h>
> > #include <signal.h>
> > #include <stdlib.h>
> > #include <time.h>
> > @@ -639,6 +640,42 @@ static int handle_config_changes(struct daemon *daemon, int conf_fd,
> > return 0;
> > }
> >
> > +static int check_lock(struct daemon *daemon)
> > +{
> > + char path[PATH_MAX];
> > + char buf[20];
> > + int fd, pid;
> > + ssize_t len;
> > +
> > + scnprintf(path, sizeof(path), "%s/lock", daemon->base);
> > +
> > + fd = open(path, O_RDWR|O_CREAT, 0640);
> > + if (fd < 0)
> > + return -1;
> > +
> > + if (lockf(fd, F_TLOCK, 0) < 0) {
> > + filename__read_int(path, &pid);
> > + fprintf(stderr, "failed: another perf daemon (pid %d) owns %s\n",
> > + pid, daemon->base);
> > + return -1;
> > + }
> > +
> > + scnprintf(buf, sizeof(buf), "%d", getpid());
> > + len = strlen(buf);
> > +
> > + if (write(fd, buf, len) != len) {
> > + perror("write failed");
> > + return -1;
> > + }
> > +
> > + if (ftruncate(fd, len)) {
> > + perror("ftruncate failed");
> > + return -1;
> > + }
> > +
> > + return 0;
> > +}
> > +
> > static int go_background(struct daemon *daemon)
> > {
> > int pid, fd;
> > @@ -653,6 +690,9 @@ static int go_background(struct daemon *daemon)
> > if (setsid() < 0)
> > return -1;
> >
> > + if (check_lock(daemon))
> > + return -1;
> > +
> > umask(0);
> >
> > if (chdir(daemon->base)) {
> > @@ -704,6 +744,9 @@ static int __cmd_daemon(struct daemon *daemon, bool foreground, const char *conf
> > if (setup_server_config(daemon))
> > return -1;
> >
> > + if (foreground && check_lock(daemon))
> > + return -1;
> > +
> > if (!foreground && go_background(daemon))
> > return -1;
> >
> > --
> > 2.26.2
> >
>
> --
>
> - Arnaldo
>
^ permalink raw reply [flat|nested] 27+ messages in thread
* [PATCH 8/8] perf daemon: Set control fifo for session
2020-12-12 10:43 [RFC 0/8] perf tools: Add daemon command Jiri Olsa
` (6 preceding siblings ...)
2020-12-12 10:43 ` [PATCH 7/8] perf daemon: Allow only one daemon over base directory Jiri Olsa
@ 2020-12-12 10:43 ` Jiri Olsa
2020-12-15 15:47 ` Arnaldo Carvalho de Melo
7 siblings, 1 reply; 27+ messages in thread
From: Jiri Olsa @ 2020-12-12 10:43 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.
Use can list control fifos with:
# perf daemon -v
[1:92187] perf record -m 11M -e cycles -o /opt/perfdata/1/perf.data --overwrite --switch-output -a
output: /opt/perfdata/1/output
control: /opt/perfdata/1/control
ack: /opt/perfdata/1/ack
Signed-off-by: Jiri Olsa <jolsa@kernel.org>
---
tools/perf/Documentation/perf-daemon.txt | 8 +++++++-
tools/perf/builtin-daemon.c | 24 +++++++++++++++++++++++-
2 files changed, 30 insertions(+), 2 deletions(-)
diff --git a/tools/perf/Documentation/perf-daemon.txt b/tools/perf/Documentation/perf-daemon.txt
index 87de2c77e4c7..c507ba7c85cc 100644
--- a/tools/perf/Documentation/perf-daemon.txt
+++ b/tools/perf/Documentation/perf-daemon.txt
@@ -16,7 +16,8 @@ DESCRIPTION
This command allows to run simple daemon process that starts and
monitors configured record sessions.
-Each session represents one perf record process.
+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.
@@ -94,10 +95,15 @@ Check sessions with more info:
# perf daemon -v
[1:92187] perf record -m 11M -e cycles -o /opt/perfdata/1/perf.data --overwrite --switch-output -a
output: /opt/perfdata/1/output
+ control: /opt/perfdata/1/control
+ ack: /opt/perfdata/1/ack
[2:92188] perf record -m 20M -e sched:* -o /opt/perfdata/2/perf.data --overwrite --switch-output -a
output: /opt/perfdata/2/output
+ control: /opt/perfdata/2/control
+ ack: /opt/perfdata/2/ack
The 'output' file is perf record output for specific session.
+The 'control' and 'ack' files are perf control files.
Send SIGUSR2 signal to all sessions:
diff --git a/tools/perf/builtin-daemon.c b/tools/perf/builtin-daemon.c
index 1bd5432a57a3..765369a30414 100644
--- a/tools/perf/builtin-daemon.c
+++ b/tools/perf/builtin-daemon.c
@@ -33,6 +33,8 @@
#include <api/fs/fs.h>
#define SESSION_OUTPUT "output"
+#define SESSION_CONTROL "control"
+#define SESSION_ACK "ack"
enum session_state {
SESSION_STATE__OK,
@@ -43,6 +45,7 @@ enum session_state {
struct session {
char *name;
char *run;
+ char *control;
int pid;
struct list_head list;
enum session_state state;
@@ -254,6 +257,8 @@ static void session__kill(struct session *session, struct daemon *daemon)
static int session__run(struct session *session, struct daemon *daemon)
{
+ char control[PATH_MAX];
+ char ack[PATH_MAX];
char base[PATH_MAX];
char buf[PATH_MAX];
char **argv;
@@ -266,6 +271,18 @@ static int session__run(struct session *session, struct daemon *daemon)
return -1;
}
+ scnprintf(control, sizeof(control), "%s/" SESSION_CONTROL, base);
+ if (mkfifo(control, O_RDWR) && errno != EEXIST) {
+ perror("failed to create control fifo");
+ return -1;
+ }
+
+ scnprintf(ack, sizeof(ack), "%s/" SESSION_ACK, base);
+ if (mkfifo(ack, O_RDWR) && errno != EEXIST) {
+ perror("failed to create ack fifo");
+ return -1;
+ }
+
session->pid = fork();
if (session->pid < 0)
return -1;
@@ -291,7 +308,8 @@ static int session__run(struct session *session, struct daemon *daemon)
dup2(fd, 2);
close(fd);
- scnprintf(buf, sizeof(buf), "%s record %s", PERF, session->run);
+ scnprintf(buf, sizeof(buf), "%s record --control=fifo:%s,%s %s",
+ PERF, control, ack, session->run);
argv = argv_split(buf, &argc);
if (!argv)
@@ -472,6 +490,10 @@ static int cmd_session_list(struct daemon *daemon, FILE *out, bool simple)
continue;
fprintf(out, " output: %s/%s/" SESSION_OUTPUT "\n",
daemon->base, session->name);
+ fprintf(out, " control: %s/%s/" SESSION_CONTROL "\n",
+ daemon->base, session->name);
+ fprintf(out, " ack: %s/%s/" SESSION_ACK "\n",
+ daemon->base, session->name);
}
return 0;
--
2.26.2
^ permalink raw reply related [flat|nested] 27+ messages in thread* Re: [PATCH 8/8] perf daemon: Set control fifo for session
2020-12-12 10:43 ` [PATCH 8/8] perf daemon: Set control fifo for session Jiri Olsa
@ 2020-12-15 15:47 ` Arnaldo Carvalho de Melo
0 siblings, 0 replies; 27+ messages in thread
From: Arnaldo Carvalho de Melo @ 2020-12-15 15:47 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, Dec 12, 2020 at 11:43:58AM +0100, Jiri Olsa escreveu:
> Setup control fifos for session and add --control
> option to session arguments.
You're stating what this does, not why this is useful, can you add a
paragraph to that effect?
- Arnaldo
> Use can list control fifos with:
>
> # perf daemon -v
> [1:92187] perf record -m 11M -e cycles -o /opt/perfdata/1/perf.data --overwrite --switch-output -a
> output: /opt/perfdata/1/output
> control: /opt/perfdata/1/control
> ack: /opt/perfdata/1/ack
>
> Signed-off-by: Jiri Olsa <jolsa@kernel.org>
> ---
> tools/perf/Documentation/perf-daemon.txt | 8 +++++++-
> tools/perf/builtin-daemon.c | 24 +++++++++++++++++++++++-
> 2 files changed, 30 insertions(+), 2 deletions(-)
>
> diff --git a/tools/perf/Documentation/perf-daemon.txt b/tools/perf/Documentation/perf-daemon.txt
> index 87de2c77e4c7..c507ba7c85cc 100644
> --- a/tools/perf/Documentation/perf-daemon.txt
> +++ b/tools/perf/Documentation/perf-daemon.txt
> @@ -16,7 +16,8 @@ DESCRIPTION
> This command allows to run simple daemon process that starts and
> monitors configured record sessions.
>
> -Each session represents one perf record process.
> +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.
> @@ -94,10 +95,15 @@ Check sessions with more info:
> # perf daemon -v
> [1:92187] perf record -m 11M -e cycles -o /opt/perfdata/1/perf.data --overwrite --switch-output -a
> output: /opt/perfdata/1/output
> + control: /opt/perfdata/1/control
> + ack: /opt/perfdata/1/ack
> [2:92188] perf record -m 20M -e sched:* -o /opt/perfdata/2/perf.data --overwrite --switch-output -a
> output: /opt/perfdata/2/output
> + control: /opt/perfdata/2/control
> + ack: /opt/perfdata/2/ack
>
> The 'output' file is perf record output for specific session.
> +The 'control' and 'ack' files are perf control files.
>
>
> Send SIGUSR2 signal to all sessions:
> diff --git a/tools/perf/builtin-daemon.c b/tools/perf/builtin-daemon.c
> index 1bd5432a57a3..765369a30414 100644
> --- a/tools/perf/builtin-daemon.c
> +++ b/tools/perf/builtin-daemon.c
> @@ -33,6 +33,8 @@
> #include <api/fs/fs.h>
>
> #define SESSION_OUTPUT "output"
> +#define SESSION_CONTROL "control"
> +#define SESSION_ACK "ack"
>
> enum session_state {
> SESSION_STATE__OK,
> @@ -43,6 +45,7 @@ enum session_state {
> struct session {
> char *name;
> char *run;
> + char *control;
> int pid;
> struct list_head list;
> enum session_state state;
> @@ -254,6 +257,8 @@ static void session__kill(struct session *session, struct daemon *daemon)
>
> static int session__run(struct session *session, struct daemon *daemon)
> {
> + char control[PATH_MAX];
> + char ack[PATH_MAX];
> char base[PATH_MAX];
> char buf[PATH_MAX];
> char **argv;
> @@ -266,6 +271,18 @@ static int session__run(struct session *session, struct daemon *daemon)
> return -1;
> }
>
> + scnprintf(control, sizeof(control), "%s/" SESSION_CONTROL, base);
> + if (mkfifo(control, O_RDWR) && errno != EEXIST) {
> + perror("failed to create control fifo");
> + return -1;
> + }
> +
> + scnprintf(ack, sizeof(ack), "%s/" SESSION_ACK, base);
> + if (mkfifo(ack, O_RDWR) && errno != EEXIST) {
> + perror("failed to create ack fifo");
> + return -1;
> + }
> +
> session->pid = fork();
> if (session->pid < 0)
> return -1;
> @@ -291,7 +308,8 @@ static int session__run(struct session *session, struct daemon *daemon)
> dup2(fd, 2);
> close(fd);
>
> - scnprintf(buf, sizeof(buf), "%s record %s", PERF, session->run);
> + scnprintf(buf, sizeof(buf), "%s record --control=fifo:%s,%s %s",
> + PERF, control, ack, session->run);
>
> argv = argv_split(buf, &argc);
> if (!argv)
> @@ -472,6 +490,10 @@ static int cmd_session_list(struct daemon *daemon, FILE *out, bool simple)
> continue;
> fprintf(out, " output: %s/%s/" SESSION_OUTPUT "\n",
> daemon->base, session->name);
> + fprintf(out, " control: %s/%s/" SESSION_CONTROL "\n",
> + daemon->base, session->name);
> + fprintf(out, " ack: %s/%s/" SESSION_ACK "\n",
> + daemon->base, session->name);
> }
>
> return 0;
> --
> 2.26.2
>
--
- Arnaldo
^ permalink raw reply [flat|nested] 27+ messages in thread