[PATCH 00/10] perf tools: Use SIGUSR2 control data dumpping

[PATCH 00/10] perf tools: Use SIGUSR2 control data dumpping

[Wang Nan]

This patch set is a preparation to support overwritable ring buffer.
However, even without the kernel side core patch [1] is accept this
patch set is still useful.

With this patch set, perf switches output when receiving SIGUSR2. For
example:

 # perf record -a -F99 --switch-output &
 [1] 26435
 # kill -s SIGUSR2 26435
 [ perf record: dump data: Woken up 1 times ]
 # [ perf record: Dump perf.data.2016041323544373 ]
 # kill -s SIGUSR2 26435
 [ perf record: dump data: Woken up 1 times ]
 # [ perf record: Dump perf.data.2016041323544730 ]
 # fg
 perf record -a -F99 --switch-output
 ^C[ perf record: Woken up 1 times to write data ]
 [ perf record: Dump perf.data.2016041323545019 ]
 [ perf record: Captured and wrote 0.395 MB perf.data.<timestamp> ]

User can periodically generates perf trace with a simple script, then
remove most of them, only keeps scripts collected when something
unusual is detected.

After [1], perf can be totally silent before receiving SIGUSR2. Trace
is collected in kernel overwritable ring buffer, and dumpped when
SIGUSR2 is received.

[1] http://lkml.kernel.org/r/1459865478-53413-1-git-send-email-wangnan0@huawei.com

Cc: Wang Nan <wangnan0@huawei.com>
Cc: He Kuang <hekuang@huawei.com>
Cc: Arnaldo Carvalho de Melo <acme@redhat.com>
Cc: Jiri Olsa <jolsa@kernel.org>
Cc: Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com>
Cc: Namhyung Kim <namhyung@kernel.org>
Cc: Zefan Li <lizefan@huawei.com>
Cc: pi3orama@163.com

Wang Nan (10):
  perf tools: Make ordered_events reusable
  perf tools: Add perf_data_file__switch() helper
  perf record: Turns auxtrace_snapshot_enable into 3 states
  perf record: Add '--timestamp-filename' option to append timestamp to
    output filename
  perf record: Split output into multiple files via '--switch-output'
  perf record: Force enable --timestamp-filename when --switch-output is
    provided
  perf record: Disable buildid cache options by default in switch output
    mode
  perf record: Re-synthesize tracking events after output switching
  perf record: Generate tracking events for process forked by perf
  perf core: Add backward attribute to perf event

 include/linux/perf_event.h       |  28 +++++-
 include/uapi/linux/perf_event.h  |   3 +-
 kernel/events/core.c             |  48 ++++++++-
 kernel/events/ring_buffer.c      |  16 ++-
 tools/perf/builtin-record.c      | 208 +++++++++++++++++++++++++++++++++++----
 tools/perf/util/data.c           |  41 ++++++++
 tools/perf/util/data.h           |  11 ++-
 tools/perf/util/ordered-events.c |   9 ++
 tools/perf/util/ordered-events.h |   1 +
 tools/perf/util/session.c        |   6 +-
 10 files changed, 341 insertions(+), 30 deletions(-)

-- 
1.8.3.4

[PATCH 01/10] perf tools: Make ordered_events reusable

[Wang Nan]

ordered_events__free() leaves linked lists and timestamps not cleared,
so unable to be reused after ordered_events__free(). Which is inconvenient
after 'perf record' supports generating multiple perf.data output and
process build-ids for each of them.

Introduce ordered_events__reinit() for this.

Signed-off-by: Wang Nan <wangnan0@huawei.com>
Signed-off-by: He Kuang <hekuang@huawei.com>
Cc: Arnaldo Carvalho de Melo <acme@redhat.com>
Cc: Jiri Olsa <jolsa@kernel.org>
Cc: Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com>
Cc: Namhyung Kim <namhyung@kernel.org>
Cc: Zefan Li <lizefan@huawei.com>
Cc: pi3orama@163.com
---
 tools/perf/util/ordered-events.c | 9 +++++++++
 tools/perf/util/ordered-events.h | 1 +
 tools/perf/util/session.c        | 6 +++++-
 3 files changed, 15 insertions(+), 1 deletion(-)

diff --git a/tools/perf/util/ordered-events.c b/tools/perf/util/ordered-events.c
index b1b9e23..fe84df1 100644
--- a/tools/perf/util/ordered-events.c
+++ b/tools/perf/util/ordered-events.c
@@ -308,3 +308,12 @@ void ordered_events__free(struct ordered_events *oe)
 		free(event);
 	}
 }
+
+void ordered_events__reinit(struct ordered_events *oe)
+{
+	ordered_events__deliver_t old_deliver = oe->deliver;
+
+	ordered_events__free(oe);
+	memset(oe, '\0', sizeof(*oe));
+	ordered_events__init(oe, old_deliver);
+}
diff --git a/tools/perf/util/ordered-events.h b/tools/perf/util/ordered-events.h
index f403991..e11468a 100644
--- a/tools/perf/util/ordered-events.h
+++ b/tools/perf/util/ordered-events.h
@@ -49,6 +49,7 @@ void ordered_events__delete(struct ordered_events *oe, struct ordered_event *eve
 int ordered_events__flush(struct ordered_events *oe, enum oe_flush how);
 void ordered_events__init(struct ordered_events *oe, ordered_events__deliver_t deliver);
 void ordered_events__free(struct ordered_events *oe);
+void ordered_events__reinit(struct ordered_events *oe);
 
 static inline
 void ordered_events__set_alloc_size(struct ordered_events *oe, u64 size)
diff --git a/tools/perf/util/session.c b/tools/perf/util/session.c
index 91d4528..ca1827c 100644
--- a/tools/perf/util/session.c
+++ b/tools/perf/util/session.c
@@ -1836,7 +1836,11 @@ out:
 out_err:
 	ui_progress__finish();
 	perf_session__warn_about_errors(session);
-	ordered_events__free(&session->ordered_events);
+	/*
+	 * We may switching perf.data output, make ordered_events
+	 * reusable.
+	 */
+	ordered_events__reinit(&session->ordered_events);
 	auxtrace__free_events(session);
 	session->one_mmap = false;
 	return err;
-- 
1.8.3.4

Re: [PATCH 01/10] perf tools: Make ordered_events reusable

[Arnaldo Carvalho de Melo]

Em Wed, Apr 13, 2016 at 08:21:04AM +0000, Wang Nan escreveu:
> ordered_events__free() leaves linked lists and timestamps not cleared,
> so unable to be reused after ordered_events__free(). Which is inconvenient
> after 'perf record' supports generating multiple perf.data output and
> process build-ids for each of them.
> 
> Introduce ordered_events__reinit() for this.

Thanks for following Jiri's suggestion, I agreed with him, clearer now.

Please next time add a:

v2: Introduce reinit(), as per suggestion from Jiri Olsa.

So that credit is given and, more importantly for me, my need to go look
at previous versions of this patch for which I think I have read some
comment about it gets reduced 8-)

- Arnaldo
 
> Signed-off-by: Wang Nan <wangnan0@huawei.com>
> Signed-off-by: He Kuang <hekuang@huawei.com>
> Cc: Arnaldo Carvalho de Melo <acme@redhat.com>
> Cc: Jiri Olsa <jolsa@kernel.org>
> Cc: Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com>
> Cc: Namhyung Kim <namhyung@kernel.org>
> Cc: Zefan Li <lizefan@huawei.com>
> Cc: pi3orama@163.com
> ---
>  tools/perf/util/ordered-events.c | 9 +++++++++
>  tools/perf/util/ordered-events.h | 1 +
>  tools/perf/util/session.c        | 6 +++++-
>  3 files changed, 15 insertions(+), 1 deletion(-)
> 
> diff --git a/tools/perf/util/ordered-events.c b/tools/perf/util/ordered-events.c
> index b1b9e23..fe84df1 100644
> --- a/tools/perf/util/ordered-events.c
> +++ b/tools/perf/util/ordered-events.c
> @@ -308,3 +308,12 @@ void ordered_events__free(struct ordered_events *oe)
>  		free(event);
>  	}
>  }
> +
> +void ordered_events__reinit(struct ordered_events *oe)
> +{
> +	ordered_events__deliver_t old_deliver = oe->deliver;
> +
> +	ordered_events__free(oe);
> +	memset(oe, '\0', sizeof(*oe));
> +	ordered_events__init(oe, old_deliver);
> +}
> diff --git a/tools/perf/util/ordered-events.h b/tools/perf/util/ordered-events.h
> index f403991..e11468a 100644
> --- a/tools/perf/util/ordered-events.h
> +++ b/tools/perf/util/ordered-events.h
> @@ -49,6 +49,7 @@ void ordered_events__delete(struct ordered_events *oe, struct ordered_event *eve
>  int ordered_events__flush(struct ordered_events *oe, enum oe_flush how);
>  void ordered_events__init(struct ordered_events *oe, ordered_events__deliver_t deliver);
>  void ordered_events__free(struct ordered_events *oe);
> +void ordered_events__reinit(struct ordered_events *oe);
>  
>  static inline
>  void ordered_events__set_alloc_size(struct ordered_events *oe, u64 size)
> diff --git a/tools/perf/util/session.c b/tools/perf/util/session.c
> index 91d4528..ca1827c 100644
> --- a/tools/perf/util/session.c
> +++ b/tools/perf/util/session.c
> @@ -1836,7 +1836,11 @@ out:
>  out_err:
>  	ui_progress__finish();
>  	perf_session__warn_about_errors(session);
> -	ordered_events__free(&session->ordered_events);
> +	/*
> +	 * We may switching perf.data output, make ordered_events
> +	 * reusable.
> +	 */
> +	ordered_events__reinit(&session->ordered_events);
>  	auxtrace__free_events(session);
>  	session->one_mmap = false;
>  	return err;
> -- 
> 1.8.3.4

[tip:perf/core] perf ordered_events: Introduce reinit()

[tip-bot for Wang Nan]

Commit-ID:  4532f642974d871f9a50e9a09bc482eaed5394f6
Gitweb:     http://git.kernel.org/tip/4532f642974d871f9a50e9a09bc482eaed5394f6
Author:     Wang Nan <wangnan0@huawei.com>
AuthorDate: Wed, 13 Apr 2016 08:21:04 +0000
Committer:  Arnaldo Carvalho de Melo <acme@redhat.com>
CommitDate: Thu, 14 Apr 2016 08:57:54 -0300

perf ordered_events: Introduce reinit()

'perf record' will use this when outputting multiple perf.data files.

Signed-off-by: Wang Nan <wangnan0@huawei.com>
Cc: Jiri Olsa <jolsa@kernel.org>
Cc: Masami Hiramatsu <mhiramat@kernel.org>
Cc: Namhyung Kim <namhyung@kernel.org>
Cc: Zefan Li <lizefan@huawei.com>
Cc: pi3orama@163.com
Link: http://lkml.kernel.org/r/1460535673-159866-2-git-send-email-wangnan0@huawei.com
Signed-off-by: He Kuang <hekuang@huawei.com>
[ Split from larger patch ]
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/util/ordered-events.c | 9 +++++++++
 tools/perf/util/ordered-events.h | 1 +
 2 files changed, 10 insertions(+)

diff --git a/tools/perf/util/ordered-events.c b/tools/perf/util/ordered-events.c
index b1b9e23..fe84df1 100644
--- a/tools/perf/util/ordered-events.c
+++ b/tools/perf/util/ordered-events.c
@@ -308,3 +308,12 @@ void ordered_events__free(struct ordered_events *oe)
 		free(event);
 	}
 }
+
+void ordered_events__reinit(struct ordered_events *oe)
+{
+	ordered_events__deliver_t old_deliver = oe->deliver;
+
+	ordered_events__free(oe);
+	memset(oe, '\0', sizeof(*oe));
+	ordered_events__init(oe, old_deliver);
+}
diff --git a/tools/perf/util/ordered-events.h b/tools/perf/util/ordered-events.h
index f403991..e11468a 100644
--- a/tools/perf/util/ordered-events.h
+++ b/tools/perf/util/ordered-events.h
@@ -49,6 +49,7 @@ void ordered_events__delete(struct ordered_events *oe, struct ordered_event *eve
 int ordered_events__flush(struct ordered_events *oe, enum oe_flush how);
 void ordered_events__init(struct ordered_events *oe, ordered_events__deliver_t deliver);
 void ordered_events__free(struct ordered_events *oe);
+void ordered_events__reinit(struct ordered_events *oe);
 
 static inline
 void ordered_events__set_alloc_size(struct ordered_events *oe, u64 size)

[tip:perf/core] perf session: Make ordered_events reusable

[tip-bot for Wang Nan]

Commit-ID:  b26dc73018d2e3a68cad0cf0bad902a8637f9bdf
Gitweb:     http://git.kernel.org/tip/b26dc73018d2e3a68cad0cf0bad902a8637f9bdf
Author:     Wang Nan <wangnan0@huawei.com>
AuthorDate: Wed, 13 Apr 2016 08:21:04 +0000
Committer:  Arnaldo Carvalho de Melo <acme@redhat.com>
CommitDate: Thu, 14 Apr 2016 08:57:54 -0300

perf session: Make ordered_events reusable

ordered_events__free() leaves linked lists and timestamps not cleared,
so unable to be reused after ordered_events__free(). Which is inconvenient
after 'perf record' supports generating multiple perf.data output and
process build-ids for each of them.

Use ordered_events__reinit() for this.

Signed-off-by: Wang Nan <wangnan0@huawei.com>
Cc: Jiri Olsa <jolsa@kernel.org>
Cc: Masami Hiramatsu <mhiramat@kernel.org>
Cc: Namhyung Kim <namhyung@kernel.org>
Cc: Zefan Li <lizefan@huawei.com>
Cc: pi3orama@163.com
Link: http://lkml.kernel.org/r/1460535673-159866-2-git-send-email-wangnan0@huawei.com
Signed-off-by: He Kuang <hekuang@huawei.com>
[ Split from larger patch ]
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/util/session.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/tools/perf/util/session.c b/tools/perf/util/session.c
index 91d4528..ca1827c 100644
--- a/tools/perf/util/session.c
+++ b/tools/perf/util/session.c
@@ -1836,7 +1836,11 @@ out:
 out_err:
 	ui_progress__finish();
 	perf_session__warn_about_errors(session);
-	ordered_events__free(&session->ordered_events);
+	/*
+	 * We may switching perf.data output, make ordered_events
+	 * reusable.
+	 */
+	ordered_events__reinit(&session->ordered_events);
 	auxtrace__free_events(session);
 	session->one_mmap = false;
 	return err;

[PATCH 02/10] perf tools: Add perf_data_file__switch() helper

[Wang Nan]

perf_data_file__switch() closes current output file, renames it, then
open a new one to continue recording. It will be used by perf record
to split output into multiple perf.data files.

Signed-off-by: Wang Nan <wangnan0@huawei.com>
Signed-off-by: He Kuang <hekuang@huawei.com>
Cc: Arnaldo Carvalho de Melo <acme@redhat.com>
Cc: Jiri Olsa <jolsa@kernel.org>
Cc: Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com>
Cc: Namhyung Kim <namhyung@kernel.org>
Cc: Zefan Li <lizefan@huawei.com>
Cc: pi3orama@163.com
---
 tools/perf/util/data.c | 41 +++++++++++++++++++++++++++++++++++++++++
 tools/perf/util/data.h | 11 ++++++++++-
 2 files changed, 51 insertions(+), 1 deletion(-)

diff --git a/tools/perf/util/data.c b/tools/perf/util/data.c
index 1921942..be835161 100644
--- a/tools/perf/util/data.c
+++ b/tools/perf/util/data.c
@@ -136,3 +136,44 @@ ssize_t perf_data_file__write(struct perf_data_file *file,
 {
 	return writen(file->fd, buf, size);
 }
+
+int perf_data_file__switch(struct perf_data_file *file,
+			   const char *postfix,
+			   size_t pos, bool at_exit)
+{
+	char *new_filepath;
+	int ret;
+
+	if (check_pipe(file))
+		return -EINVAL;
+	if (perf_data_file__is_read(file))
+		return -EINVAL;
+
+	if (asprintf(&new_filepath, "%s.%s", file->path, postfix) < 0)
+		return -ENOMEM;
+
+	/*
+	 * Only fire a warning, don't return error, continue fill
+	 * original file.
+	 */
+	if (rename(file->path, new_filepath))
+		pr_warning("Failed to rename %s to %s\n", file->path, new_filepath);
+
+	if (!at_exit) {
+		close(file->fd);
+		ret = perf_data_file__open(file);
+		if (ret < 0)
+			goto out;
+
+		if (lseek(file->fd, pos, SEEK_SET) == (off_t)-1) {
+			ret = -errno;
+			pr_debug("Failed to lseek to %zu: %s",
+				 pos, strerror(errno));
+			goto out;
+		}
+	}
+	ret = file->fd;
+out:
+	free(new_filepath);
+	return ret;
+}
diff --git a/tools/perf/util/data.h b/tools/perf/util/data.h
index 2b15d0c..ae510ce 100644
--- a/tools/perf/util/data.h
+++ b/tools/perf/util/data.h
@@ -46,5 +46,14 @@ int perf_data_file__open(struct perf_data_file *file);
 void perf_data_file__close(struct perf_data_file *file);
 ssize_t perf_data_file__write(struct perf_data_file *file,
 			      void *buf, size_t size);
-
+/*
+ * If at_exit is set, only rename current perf.data to
+ * perf.data.<postfix>, continue write on original file.
+ * Set at_exit when flushing the last output.
+ *
+ * Return value is fd of new output.
+ */
+int perf_data_file__switch(struct perf_data_file *file,
+			   const char *postfix,
+			   size_t pos, bool at_exit);
 #endif /* __PERF_DATA_H */
-- 
1.8.3.4

[tip:perf/core] perf data: Add perf_data_file__switch() helper

[tip-bot for Wang Nan]

Commit-ID:  040f9915e99e604688c2880e0b1e5b59dbfefa54
Gitweb:     http://git.kernel.org/tip/040f9915e99e604688c2880e0b1e5b59dbfefa54
Author:     Wang Nan <wangnan0@huawei.com>
AuthorDate: Wed, 13 Apr 2016 08:21:05 +0000
Committer:  Arnaldo Carvalho de Melo <acme@redhat.com>
CommitDate: Thu, 14 Apr 2016 08:57:54 -0300

perf data: Add perf_data_file__switch() helper

perf_data_file__switch() closes current output file, renames it, then
open a new one to continue recording. It will be used by 'perf record'
to split output into multiple perf.data files.

Signed-off-by: Wang Nan <wangnan0@huawei.com>
Cc: Jiri Olsa <jolsa@kernel.org>
Cc: Masami Hiramatsu <mhiramat@kernel.org>
Cc: Namhyung Kim <namhyung@kernel.org>
Cc: Zefan Li <lizefan@huawei.com>
Cc: pi3orama@163.com
Link: http://lkml.kernel.org/r/1460535673-159866-3-git-send-email-wangnan0@huawei.com
Signed-off-by: He Kuang <hekuang@huawei.com>
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/util/data.c | 41 +++++++++++++++++++++++++++++++++++++++++
 tools/perf/util/data.h | 11 ++++++++++-
 2 files changed, 51 insertions(+), 1 deletion(-)

diff --git a/tools/perf/util/data.c b/tools/perf/util/data.c
index 1921942..be835161 100644
--- a/tools/perf/util/data.c
+++ b/tools/perf/util/data.c
@@ -136,3 +136,44 @@ ssize_t perf_data_file__write(struct perf_data_file *file,
 {
 	return writen(file->fd, buf, size);
 }
+
+int perf_data_file__switch(struct perf_data_file *file,
+			   const char *postfix,
+			   size_t pos, bool at_exit)
+{
+	char *new_filepath;
+	int ret;
+
+	if (check_pipe(file))
+		return -EINVAL;
+	if (perf_data_file__is_read(file))
+		return -EINVAL;
+
+	if (asprintf(&new_filepath, "%s.%s", file->path, postfix) < 0)
+		return -ENOMEM;
+
+	/*
+	 * Only fire a warning, don't return error, continue fill
+	 * original file.
+	 */
+	if (rename(file->path, new_filepath))
+		pr_warning("Failed to rename %s to %s\n", file->path, new_filepath);
+
+	if (!at_exit) {
+		close(file->fd);
+		ret = perf_data_file__open(file);
+		if (ret < 0)
+			goto out;
+
+		if (lseek(file->fd, pos, SEEK_SET) == (off_t)-1) {
+			ret = -errno;
+			pr_debug("Failed to lseek to %zu: %s",
+				 pos, strerror(errno));
+			goto out;
+		}
+	}
+	ret = file->fd;
+out:
+	free(new_filepath);
+	return ret;
+}
diff --git a/tools/perf/util/data.h b/tools/perf/util/data.h
index 2b15d0c..ae510ce 100644
--- a/tools/perf/util/data.h
+++ b/tools/perf/util/data.h
@@ -46,5 +46,14 @@ int perf_data_file__open(struct perf_data_file *file);
 void perf_data_file__close(struct perf_data_file *file);
 ssize_t perf_data_file__write(struct perf_data_file *file,
 			      void *buf, size_t size);
-
+/*
+ * If at_exit is set, only rename current perf.data to
+ * perf.data.<postfix>, continue write on original file.
+ * Set at_exit when flushing the last output.
+ *
+ * Return value is fd of new output.
+ */
+int perf_data_file__switch(struct perf_data_file *file,
+			   const char *postfix,
+			   size_t pos, bool at_exit);
 #endif /* __PERF_DATA_H */

Re: [PATCH 02/10] perf tools: Add perf_data_file__switch() helper

[Jiri Olsa]

On Wed, Apr 13, 2016 at 08:21:05AM +0000, Wang Nan wrote:
> perf_data_file__switch() closes current output file, renames it, then
> open a new one to continue recording. It will be used by perf record
> to split output into multiple perf.data files.
> 
> Signed-off-by: Wang Nan <wangnan0@huawei.com>
> Signed-off-by: He Kuang <hekuang@huawei.com>
> Cc: Arnaldo Carvalho de Melo <acme@redhat.com>
> Cc: Jiri Olsa <jolsa@kernel.org>
> Cc: Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com>
> Cc: Namhyung Kim <namhyung@kernel.org>
> Cc: Zefan Li <lizefan@huawei.com>
> Cc: pi3orama@163.com
> ---
>  tools/perf/util/data.c | 41 +++++++++++++++++++++++++++++++++++++++++
>  tools/perf/util/data.h | 11 ++++++++++-
>  2 files changed, 51 insertions(+), 1 deletion(-)
> 
> diff --git a/tools/perf/util/data.c b/tools/perf/util/data.c
> index 1921942..be835161 100644
> --- a/tools/perf/util/data.c
> +++ b/tools/perf/util/data.c
> @@ -136,3 +136,44 @@ ssize_t perf_data_file__write(struct perf_data_file *file,
>  {
>  	return writen(file->fd, buf, size);
>  }
> +
> +int perf_data_file__switch(struct perf_data_file *file,
> +			   const char *postfix,
> +			   size_t pos, bool at_exit)

could you please rename at_exit to reopen

I guess you follow the record object's at_exit naming,
but 'reopen' seems more clear to me

jirka

Re: [PATCH 02/10] perf tools: Add perf_data_file__switch() helper

[Arnaldo Carvalho de Melo]

Em Fri, Apr 15, 2016 at 12:41:08PM +0200, Jiri Olsa escreveu:
> On Wed, Apr 13, 2016 at 08:21:05AM +0000, Wang Nan wrote:
> > perf_data_file__switch() closes current output file, renames it, then
> > open a new one to continue recording. It will be used by perf record
> > to split output into multiple perf.data files.
> > 
> > Signed-off-by: Wang Nan <wangnan0@huawei.com>
> > Signed-off-by: He Kuang <hekuang@huawei.com>
> > Cc: Arnaldo Carvalho de Melo <acme@redhat.com>
> > Cc: Jiri Olsa <jolsa@kernel.org>
> > Cc: Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com>
> > Cc: Namhyung Kim <namhyung@kernel.org>
> > Cc: Zefan Li <lizefan@huawei.com>
> > Cc: pi3orama@163.com
> > ---
> >  tools/perf/util/data.c | 41 +++++++++++++++++++++++++++++++++++++++++
> >  tools/perf/util/data.h | 11 ++++++++++-
> >  2 files changed, 51 insertions(+), 1 deletion(-)
> > 
> > diff --git a/tools/perf/util/data.c b/tools/perf/util/data.c
> > index 1921942..be835161 100644
> > --- a/tools/perf/util/data.c
> > +++ b/tools/perf/util/data.c
> > @@ -136,3 +136,44 @@ ssize_t perf_data_file__write(struct perf_data_file *file,
> >  {
> >  	return writen(file->fd, buf, size);
> >  }
> > +
> > +int perf_data_file__switch(struct perf_data_file *file,
> > +			   const char *postfix,
> > +			   size_t pos, bool at_exit)
> 
> could you please rename at_exit to reopen
> 
> I guess you follow the record object's at_exit naming,
> but 'reopen' seems more clear to me
> 

I'll rename this, if not yet pushed to Ingo, checking...

- Arnaldo
> jirka

[PATCH 03/10] perf record: Turns auxtrace_snapshot_enable into 3 states

[Wang Nan]

auxtrace_snapshot_enable has only two states (0/1). Turns it into a
triple states enum so SIGUSR2 handler can safely do other works without
triggering auxtrace snapshot.

Signed-off-by: Wang Nan <wangnan0@huawei.com>
Signed-off-by: He Kuang <hekuang@huawei.com>
Acked-by: Jiri Olsa <jolsa@kernel.org>
Cc: Arnaldo Carvalho de Melo <acme@redhat.com>
Cc: Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com>
Cc: Namhyung Kim <namhyung@kernel.org>
Cc: Zefan Li <lizefan@huawei.com>
Cc: pi3orama@163.com
---
 tools/perf/builtin-record.c | 59 +++++++++++++++++++++++++++++++++++++--------
 1 file changed, 49 insertions(+), 10 deletions(-)

diff --git a/tools/perf/builtin-record.c b/tools/perf/builtin-record.c
index eb6a199..480033f 100644
--- a/tools/perf/builtin-record.c
+++ b/tools/perf/builtin-record.c
@@ -125,7 +125,43 @@ out:
 static volatile int done;
 static volatile int signr = -1;
 static volatile int child_finished;
-static volatile int auxtrace_snapshot_enabled;
+
+static volatile enum {
+	AUXTRACE_SNAPSHOT_OFF = -1,
+	AUXTRACE_SNAPSHOT_DISABLED = 0,
+	AUXTRACE_SNAPSHOT_ENABLED = 1,
+} auxtrace_snapshot_state = AUXTRACE_SNAPSHOT_OFF;
+
+static inline void
+auxtrace_snapshot_on(void)
+{
+	auxtrace_snapshot_state = AUXTRACE_SNAPSHOT_DISABLED;
+}
+
+static inline void
+auxtrace_snapshot_enable(void)
+{
+	if (auxtrace_snapshot_state == AUXTRACE_SNAPSHOT_OFF)
+		return;
+	auxtrace_snapshot_state = AUXTRACE_SNAPSHOT_ENABLED;
+}
+
+static inline void
+auxtrace_snapshot_disable(void)
+{
+	if (auxtrace_snapshot_state == AUXTRACE_SNAPSHOT_OFF)
+		return;
+	auxtrace_snapshot_state = AUXTRACE_SNAPSHOT_DISABLED;
+}
+
+static inline bool
+auxtrace_snapshot_is_enabled(void)
+{
+	if (auxtrace_snapshot_state == AUXTRACE_SNAPSHOT_OFF)
+		return false;
+	return auxtrace_snapshot_state == AUXTRACE_SNAPSHOT_ENABLED;
+}
+
 static volatile int auxtrace_snapshot_err;
 static volatile int auxtrace_record__snapshot_started;
 
@@ -249,7 +285,7 @@ static void record__read_auxtrace_snapshot(struct record *rec)
 	} else {
 		auxtrace_snapshot_err = auxtrace_record__snapshot_finish(rec->itr);
 		if (!auxtrace_snapshot_err)
-			auxtrace_snapshot_enabled = 1;
+			auxtrace_snapshot_enable();
 	}
 }
 
@@ -615,10 +651,13 @@ static int __cmd_record(struct record *rec, int argc, const char **argv)
 	signal(SIGCHLD, sig_handler);
 	signal(SIGINT, sig_handler);
 	signal(SIGTERM, sig_handler);
-	if (rec->opts.auxtrace_snapshot_mode)
+
+	if (rec->opts.auxtrace_snapshot_mode) {
 		signal(SIGUSR2, snapshot_sig_handler);
-	else
+		auxtrace_snapshot_on();
+	} else {
 		signal(SIGUSR2, SIG_IGN);
+	}
 
 	session = perf_session__new(file, false, tool);
 	if (session == NULL) {
@@ -744,12 +783,12 @@ static int __cmd_record(struct record *rec, int argc, const char **argv)
 		perf_evlist__enable(rec->evlist);
 	}
 
-	auxtrace_snapshot_enabled = 1;
+	auxtrace_snapshot_enable();
 	for (;;) {
 		unsigned long long hits = rec->samples;
 
 		if (record__mmap_read_all(rec) < 0) {
-			auxtrace_snapshot_enabled = 0;
+			auxtrace_snapshot_disable();
 			err = -1;
 			goto out_child;
 		}
@@ -787,12 +826,12 @@ static int __cmd_record(struct record *rec, int argc, const char **argv)
 		 * disable events in this case.
 		 */
 		if (done && !disabled && !target__none(&opts->target)) {
-			auxtrace_snapshot_enabled = 0;
+			auxtrace_snapshot_disable();
 			perf_evlist__disable(rec->evlist);
 			disabled = true;
 		}
 	}
-	auxtrace_snapshot_enabled = 0;
+	auxtrace_snapshot_disable();
 
 	if (forks && workload_exec_errno) {
 		char msg[STRERR_BUFSIZE];
@@ -1358,9 +1397,9 @@ out_symbol_exit:
 
 static void snapshot_sig_handler(int sig __maybe_unused)
 {
-	if (!auxtrace_snapshot_enabled)
+	if (!auxtrace_snapshot_is_enabled())
 		return;
-	auxtrace_snapshot_enabled = 0;
+	auxtrace_snapshot_disable();
 	auxtrace_snapshot_err = auxtrace_record__snapshot_start(record.itr);
 	auxtrace_record__snapshot_started = 1;
 }
-- 
1.8.3.4

Re: [PATCH 03/10] perf record: Turns auxtrace_snapshot_enable into 3 states

[Arnaldo Carvalho de Melo]

Em Wed, Apr 13, 2016 at 08:21:06AM +0000, Wang Nan escreveu:
> auxtrace_snapshot_enable has only two states (0/1). Turns it into a
> triple states enum so SIGUSR2 handler can safely do other works without
> triggering auxtrace snapshot.

Adrian, can you take a look at this? Is it ok with you?

Thanks,

- Arnaldo
 
> Signed-off-by: Wang Nan <wangnan0@huawei.com>
> Signed-off-by: He Kuang <hekuang@huawei.com>
> Acked-by: Jiri Olsa <jolsa@kernel.org>
> Cc: Arnaldo Carvalho de Melo <acme@redhat.com>
> Cc: Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com>
> Cc: Namhyung Kim <namhyung@kernel.org>
> Cc: Zefan Li <lizefan@huawei.com>
> Cc: pi3orama@163.com
> ---
>  tools/perf/builtin-record.c | 59 +++++++++++++++++++++++++++++++++++++--------
>  1 file changed, 49 insertions(+), 10 deletions(-)
> 
> diff --git a/tools/perf/builtin-record.c b/tools/perf/builtin-record.c
> index eb6a199..480033f 100644
> --- a/tools/perf/builtin-record.c
> +++ b/tools/perf/builtin-record.c
> @@ -125,7 +125,43 @@ out:
>  static volatile int done;
>  static volatile int signr = -1;
>  static volatile int child_finished;
> -static volatile int auxtrace_snapshot_enabled;
> +
> +static volatile enum {
> +	AUXTRACE_SNAPSHOT_OFF = -1,
> +	AUXTRACE_SNAPSHOT_DISABLED = 0,
> +	AUXTRACE_SNAPSHOT_ENABLED = 1,
> +} auxtrace_snapshot_state = AUXTRACE_SNAPSHOT_OFF;
> +
> +static inline void
> +auxtrace_snapshot_on(void)
> +{
> +	auxtrace_snapshot_state = AUXTRACE_SNAPSHOT_DISABLED;
> +}
> +
> +static inline void
> +auxtrace_snapshot_enable(void)
> +{
> +	if (auxtrace_snapshot_state == AUXTRACE_SNAPSHOT_OFF)
> +		return;
> +	auxtrace_snapshot_state = AUXTRACE_SNAPSHOT_ENABLED;
> +}
> +
> +static inline void
> +auxtrace_snapshot_disable(void)
> +{
> +	if (auxtrace_snapshot_state == AUXTRACE_SNAPSHOT_OFF)
> +		return;
> +	auxtrace_snapshot_state = AUXTRACE_SNAPSHOT_DISABLED;
> +}
> +
> +static inline bool
> +auxtrace_snapshot_is_enabled(void)
> +{
> +	if (auxtrace_snapshot_state == AUXTRACE_SNAPSHOT_OFF)
> +		return false;
> +	return auxtrace_snapshot_state == AUXTRACE_SNAPSHOT_ENABLED;
> +}
> +
>  static volatile int auxtrace_snapshot_err;
>  static volatile int auxtrace_record__snapshot_started;
>  
> @@ -249,7 +285,7 @@ static void record__read_auxtrace_snapshot(struct record *rec)
>  	} else {
>  		auxtrace_snapshot_err = auxtrace_record__snapshot_finish(rec->itr);
>  		if (!auxtrace_snapshot_err)
> -			auxtrace_snapshot_enabled = 1;
> +			auxtrace_snapshot_enable();
>  	}
>  }
>  
> @@ -615,10 +651,13 @@ static int __cmd_record(struct record *rec, int argc, const char **argv)
>  	signal(SIGCHLD, sig_handler);
>  	signal(SIGINT, sig_handler);
>  	signal(SIGTERM, sig_handler);
> -	if (rec->opts.auxtrace_snapshot_mode)
> +
> +	if (rec->opts.auxtrace_snapshot_mode) {
>  		signal(SIGUSR2, snapshot_sig_handler);
> -	else
> +		auxtrace_snapshot_on();
> +	} else {
>  		signal(SIGUSR2, SIG_IGN);
> +	}
>  
>  	session = perf_session__new(file, false, tool);
>  	if (session == NULL) {
> @@ -744,12 +783,12 @@ static int __cmd_record(struct record *rec, int argc, const char **argv)
>  		perf_evlist__enable(rec->evlist);
>  	}
>  
> -	auxtrace_snapshot_enabled = 1;
> +	auxtrace_snapshot_enable();
>  	for (;;) {
>  		unsigned long long hits = rec->samples;
>  
>  		if (record__mmap_read_all(rec) < 0) {
> -			auxtrace_snapshot_enabled = 0;
> +			auxtrace_snapshot_disable();
>  			err = -1;
>  			goto out_child;
>  		}
> @@ -787,12 +826,12 @@ static int __cmd_record(struct record *rec, int argc, const char **argv)
>  		 * disable events in this case.
>  		 */
>  		if (done && !disabled && !target__none(&opts->target)) {
> -			auxtrace_snapshot_enabled = 0;
> +			auxtrace_snapshot_disable();
>  			perf_evlist__disable(rec->evlist);
>  			disabled = true;
>  		}
>  	}
> -	auxtrace_snapshot_enabled = 0;
> +	auxtrace_snapshot_disable();
>  
>  	if (forks && workload_exec_errno) {
>  		char msg[STRERR_BUFSIZE];
> @@ -1358,9 +1397,9 @@ out_symbol_exit:
>  
>  static void snapshot_sig_handler(int sig __maybe_unused)
>  {
> -	if (!auxtrace_snapshot_enabled)
> +	if (!auxtrace_snapshot_is_enabled())
>  		return;
> -	auxtrace_snapshot_enabled = 0;
> +	auxtrace_snapshot_disable();
>  	auxtrace_snapshot_err = auxtrace_record__snapshot_start(record.itr);
>  	auxtrace_record__snapshot_started = 1;
>  }
> -- 
> 1.8.3.4

Re: [PATCH 03/10] perf record: Turns auxtrace_snapshot_enable into 3 states

[Adrian Hunter]

On 13/04/16 18:55, Arnaldo Carvalho de Melo wrote:
> Em Wed, Apr 13, 2016 at 08:21:06AM +0000, Wang Nan escreveu:
>> auxtrace_snapshot_enable has only two states (0/1). Turns it into a
>> triple states enum so SIGUSR2 handler can safely do other works without
>> triggering auxtrace snapshot.
> 
> Adrian, can you take a look at this? Is it ok with you?

Please forgive me if these are stupid questions:

First I am wondering why we wouldn't want to snapshot auxtrace data at the
same time as the perf buffer?

For that we would need continuous tracking information like MMAPS etc, but
isn't that needed anyway?


> 
> Thanks,
> 
> - Arnaldo
>  
>> Signed-off-by: Wang Nan <wangnan0@huawei.com>
>> Signed-off-by: He Kuang <hekuang@huawei.com>
>> Acked-by: Jiri Olsa <jolsa@kernel.org>
>> Cc: Arnaldo Carvalho de Melo <acme@redhat.com>
>> Cc: Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com>
>> Cc: Namhyung Kim <namhyung@kernel.org>
>> Cc: Zefan Li <lizefan@huawei.com>
>> Cc: pi3orama@163.com
>> ---
>>  tools/perf/builtin-record.c | 59 +++++++++++++++++++++++++++++++++++++--------
>>  1 file changed, 49 insertions(+), 10 deletions(-)
>>
>> diff --git a/tools/perf/builtin-record.c b/tools/perf/builtin-record.c
>> index eb6a199..480033f 100644
>> --- a/tools/perf/builtin-record.c
>> +++ b/tools/perf/builtin-record.c
>> @@ -125,7 +125,43 @@ out:
>>  static volatile int done;
>>  static volatile int signr = -1;
>>  static volatile int child_finished;
>> -static volatile int auxtrace_snapshot_enabled;
>> +
>> +static volatile enum {
>> +	AUXTRACE_SNAPSHOT_OFF = -1,
>> +	AUXTRACE_SNAPSHOT_DISABLED = 0,
>> +	AUXTRACE_SNAPSHOT_ENABLED = 1,
>> +} auxtrace_snapshot_state = AUXTRACE_SNAPSHOT_OFF;
>> +
>> +static inline void
>> +auxtrace_snapshot_on(void)
>> +{
>> +	auxtrace_snapshot_state = AUXTRACE_SNAPSHOT_DISABLED;
>> +}
>> +
>> +static inline void
>> +auxtrace_snapshot_enable(void)
>> +{
>> +	if (auxtrace_snapshot_state == AUXTRACE_SNAPSHOT_OFF)
>> +		return;
>> +	auxtrace_snapshot_state = AUXTRACE_SNAPSHOT_ENABLED;
>> +}
>> +
>> +static inline void
>> +auxtrace_snapshot_disable(void)
>> +{
>> +	if (auxtrace_snapshot_state == AUXTRACE_SNAPSHOT_OFF)
>> +		return;
>> +	auxtrace_snapshot_state = AUXTRACE_SNAPSHOT_DISABLED;
>> +}
>> +
>> +static inline bool
>> +auxtrace_snapshot_is_enabled(void)
>> +{
>> +	if (auxtrace_snapshot_state == AUXTRACE_SNAPSHOT_OFF)
>> +		return false;
>> +	return auxtrace_snapshot_state == AUXTRACE_SNAPSHOT_ENABLED;
>> +}
>> +
>>  static volatile int auxtrace_snapshot_err;
>>  static volatile int auxtrace_record__snapshot_started;
>>  
>> @@ -249,7 +285,7 @@ static void record__read_auxtrace_snapshot(struct record *rec)
>>  	} else {
>>  		auxtrace_snapshot_err = auxtrace_record__snapshot_finish(rec->itr);
>>  		if (!auxtrace_snapshot_err)
>> -			auxtrace_snapshot_enabled = 1;
>> +			auxtrace_snapshot_enable();
>>  	}
>>  }
>>  
>> @@ -615,10 +651,13 @@ static int __cmd_record(struct record *rec, int argc, const char **argv)
>>  	signal(SIGCHLD, sig_handler);
>>  	signal(SIGINT, sig_handler);
>>  	signal(SIGTERM, sig_handler);
>> -	if (rec->opts.auxtrace_snapshot_mode)
>> +
>> +	if (rec->opts.auxtrace_snapshot_mode) {
>>  		signal(SIGUSR2, snapshot_sig_handler);
>> -	else
>> +		auxtrace_snapshot_on();
>> +	} else {
>>  		signal(SIGUSR2, SIG_IGN);
>> +	}
>>  
>>  	session = perf_session__new(file, false, tool);
>>  	if (session == NULL) {
>> @@ -744,12 +783,12 @@ static int __cmd_record(struct record *rec, int argc, const char **argv)
>>  		perf_evlist__enable(rec->evlist);
>>  	}
>>  
>> -	auxtrace_snapshot_enabled = 1;
>> +	auxtrace_snapshot_enable();
>>  	for (;;) {
>>  		unsigned long long hits = rec->samples;
>>  
>>  		if (record__mmap_read_all(rec) < 0) {
>> -			auxtrace_snapshot_enabled = 0;
>> +			auxtrace_snapshot_disable();
>>  			err = -1;
>>  			goto out_child;
>>  		}
>> @@ -787,12 +826,12 @@ static int __cmd_record(struct record *rec, int argc, const char **argv)
>>  		 * disable events in this case.
>>  		 */
>>  		if (done && !disabled && !target__none(&opts->target)) {
>> -			auxtrace_snapshot_enabled = 0;
>> +			auxtrace_snapshot_disable();
>>  			perf_evlist__disable(rec->evlist);
>>  			disabled = true;
>>  		}
>>  	}
>> -	auxtrace_snapshot_enabled = 0;
>> +	auxtrace_snapshot_disable();
>>  
>>  	if (forks && workload_exec_errno) {
>>  		char msg[STRERR_BUFSIZE];
>> @@ -1358,9 +1397,9 @@ out_symbol_exit:
>>  
>>  static void snapshot_sig_handler(int sig __maybe_unused)
>>  {
>> -	if (!auxtrace_snapshot_enabled)
>> +	if (!auxtrace_snapshot_is_enabled())
>>  		return;
>> -	auxtrace_snapshot_enabled = 0;
>> +	auxtrace_snapshot_disable();
>>  	auxtrace_snapshot_err = auxtrace_record__snapshot_start(record.itr);
>>  	auxtrace_record__snapshot_started = 1;
>>  }
>> -- 
>> 1.8.3.4
> 

Re: [PATCH 03/10] perf record: Turns auxtrace_snapshot_enable into 3 states

[Wangnan (F)]

On 2016/4/14 15:15, Adrian Hunter wrote:
> On 13/04/16 18:55, Arnaldo Carvalho de Melo wrote:
>> Em Wed, Apr 13, 2016 at 08:21:06AM +0000, Wang Nan escreveu:
>>> auxtrace_snapshot_enable has only two states (0/1). Turns it into a
>>> triple states enum so SIGUSR2 handler can safely do other works without
>>> triggering auxtrace snapshot.
>> Adrian, can you take a look at this? Is it ok with you?
> Please forgive me if these are stupid questions:
>
> First I am wondering why we wouldn't want to snapshot auxtrace data at the
> same time as the perf buffer?

This patch doesn't prevent taking snapshot when receiving SIGUSR2.

If both --snapshot and --switch-outupt is provided, when SIGUSR2 received,
perf takes auxtrace snapshot and other perf buffer together.

Thank you.


> For that we would need continuous tracking information like MMAPS etc, but
> isn't that needed anyway?
>
>
>> Thanks,
>>
>> - Arnaldo
>>   
>>> Signed-off-by: Wang Nan <wangnan0@huawei.com>
>>> Signed-off-by: He Kuang <hekuang@huawei.com>
>>> Acked-by: Jiri Olsa <jolsa@kernel.org>
>>> Cc: Arnaldo Carvalho de Melo <acme@redhat.com>
>>> Cc: Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com>
>>> Cc: Namhyung Kim <namhyung@kernel.org>
>>> Cc: Zefan Li <lizefan@huawei.com>
>>> Cc: pi3orama@163.com
>>> ---
>>>   tools/perf/builtin-record.c | 59 +++++++++++++++++++++++++++++++++++++--------
>>>   1 file changed, 49 insertions(+), 10 deletions(-)
>>>
>>> diff --git a/tools/perf/builtin-record.c b/tools/perf/builtin-record.c
>>> index eb6a199..480033f 100644
>>> --- a/tools/perf/builtin-record.c
>>> +++ b/tools/perf/builtin-record.c
>>> @@ -125,7 +125,43 @@ out:
>>>   static volatile int done;
>>>   static volatile int signr = -1;
>>>   static volatile int child_finished;
>>> -static volatile int auxtrace_snapshot_enabled;
>>> +
>>> +static volatile enum {
>>> +	AUXTRACE_SNAPSHOT_OFF = -1,
>>> +	AUXTRACE_SNAPSHOT_DISABLED = 0,
>>> +	AUXTRACE_SNAPSHOT_ENABLED = 1,
>>> +} auxtrace_snapshot_state = AUXTRACE_SNAPSHOT_OFF;
>>> +
>>> +static inline void
>>> +auxtrace_snapshot_on(void)
>>> +{
>>> +	auxtrace_snapshot_state = AUXTRACE_SNAPSHOT_DISABLED;
>>> +}
>>> +
>>> +static inline void
>>> +auxtrace_snapshot_enable(void)
>>> +{
>>> +	if (auxtrace_snapshot_state == AUXTRACE_SNAPSHOT_OFF)
>>> +		return;
>>> +	auxtrace_snapshot_state = AUXTRACE_SNAPSHOT_ENABLED;
>>> +}
>>> +
>>> +static inline void
>>> +auxtrace_snapshot_disable(void)
>>> +{
>>> +	if (auxtrace_snapshot_state == AUXTRACE_SNAPSHOT_OFF)
>>> +		return;
>>> +	auxtrace_snapshot_state = AUXTRACE_SNAPSHOT_DISABLED;
>>> +}
>>> +
>>> +static inline bool
>>> +auxtrace_snapshot_is_enabled(void)
>>> +{
>>> +	if (auxtrace_snapshot_state == AUXTRACE_SNAPSHOT_OFF)
>>> +		return false;
>>> +	return auxtrace_snapshot_state == AUXTRACE_SNAPSHOT_ENABLED;
>>> +}
>>> +
>>>   static volatile int auxtrace_snapshot_err;
>>>   static volatile int auxtrace_record__snapshot_started;
>>>   
>>> @@ -249,7 +285,7 @@ static void record__read_auxtrace_snapshot(struct record *rec)
>>>   	} else {
>>>   		auxtrace_snapshot_err = auxtrace_record__snapshot_finish(rec->itr);
>>>   		if (!auxtrace_snapshot_err)
>>> -			auxtrace_snapshot_enabled = 1;
>>> +			auxtrace_snapshot_enable();
>>>   	}
>>>   }
>>>   
>>> @@ -615,10 +651,13 @@ static int __cmd_record(struct record *rec, int argc, const char **argv)
>>>   	signal(SIGCHLD, sig_handler);
>>>   	signal(SIGINT, sig_handler);
>>>   	signal(SIGTERM, sig_handler);
>>> -	if (rec->opts.auxtrace_snapshot_mode)
>>> +
>>> +	if (rec->opts.auxtrace_snapshot_mode) {
>>>   		signal(SIGUSR2, snapshot_sig_handler);
>>> -	else
>>> +		auxtrace_snapshot_on();
>>> +	} else {
>>>   		signal(SIGUSR2, SIG_IGN);
>>> +	}
>>>   
>>>   	session = perf_session__new(file, false, tool);
>>>   	if (session == NULL) {
>>> @@ -744,12 +783,12 @@ static int __cmd_record(struct record *rec, int argc, const char **argv)
>>>   		perf_evlist__enable(rec->evlist);
>>>   	}
>>>   
>>> -	auxtrace_snapshot_enabled = 1;
>>> +	auxtrace_snapshot_enable();
>>>   	for (;;) {
>>>   		unsigned long long hits = rec->samples;
>>>   
>>>   		if (record__mmap_read_all(rec) < 0) {
>>> -			auxtrace_snapshot_enabled = 0;
>>> +			auxtrace_snapshot_disable();
>>>   			err = -1;
>>>   			goto out_child;
>>>   		}
>>> @@ -787,12 +826,12 @@ static int __cmd_record(struct record *rec, int argc, const char **argv)
>>>   		 * disable events in this case.
>>>   		 */
>>>   		if (done && !disabled && !target__none(&opts->target)) {
>>> -			auxtrace_snapshot_enabled = 0;
>>> +			auxtrace_snapshot_disable();
>>>   			perf_evlist__disable(rec->evlist);
>>>   			disabled = true;
>>>   		}
>>>   	}
>>> -	auxtrace_snapshot_enabled = 0;
>>> +	auxtrace_snapshot_disable();
>>>   
>>>   	if (forks && workload_exec_errno) {
>>>   		char msg[STRERR_BUFSIZE];
>>> @@ -1358,9 +1397,9 @@ out_symbol_exit:
>>>   
>>>   static void snapshot_sig_handler(int sig __maybe_unused)
>>>   {
>>> -	if (!auxtrace_snapshot_enabled)
>>> +	if (!auxtrace_snapshot_is_enabled())
>>>   		return;
>>> -	auxtrace_snapshot_enabled = 0;
>>> +	auxtrace_snapshot_disable();
>>>   	auxtrace_snapshot_err = auxtrace_record__snapshot_start(record.itr);
>>>   	auxtrace_record__snapshot_started = 1;
>>>   }
>>> -- 
>>> 1.8.3.4

Re: [PATCH 03/10] perf record: Turns auxtrace_snapshot_enable into 3 states

[Adrian Hunter]

On 14/04/16 10:50, Wangnan (F) wrote:
> 
> 
> On 2016/4/14 15:15, Adrian Hunter wrote:
>> On 13/04/16 18:55, Arnaldo Carvalho de Melo wrote:
>>> Em Wed, Apr 13, 2016 at 08:21:06AM +0000, Wang Nan escreveu:
>>>> auxtrace_snapshot_enable has only two states (0/1). Turns it into a
>>>> triple states enum so SIGUSR2 handler can safely do other works without
>>>> triggering auxtrace snapshot.
>>> Adrian, can you take a look at this? Is it ok with you?
>> Please forgive me if these are stupid questions:
>>
>> First I am wondering why we wouldn't want to snapshot auxtrace data at the
>> same time as the perf buffer?
> 
> This patch doesn't prevent taking snapshot when receiving SIGUSR2.

So it was a stupid question ;-)

> 
> If both --snapshot and --switch-outupt is provided, when SIGUSR2 received,
> perf takes auxtrace snapshot and other perf buffer together.

How do you keep from losing tracking information like MMAP events?  Are they
is a different buffer?

> 
> Thank you.
> 
> 
>> For that we would need continuous tracking information like MMAPS etc, but
>> isn't that needed anyway?
>>
>>
>>> Thanks,
>>>
>>> - Arnaldo
>>>  
>>>> Signed-off-by: Wang Nan <wangnan0@huawei.com>
>>>> Signed-off-by: He Kuang <hekuang@huawei.com>
>>>> Acked-by: Jiri Olsa <jolsa@kernel.org>
>>>> Cc: Arnaldo Carvalho de Melo <acme@redhat.com>
>>>> Cc: Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com>
>>>> Cc: Namhyung Kim <namhyung@kernel.org>
>>>> Cc: Zefan Li <lizefan@huawei.com>
>>>> Cc: pi3orama@163.com
>>>> ---
>>>>   tools/perf/builtin-record.c | 59
>>>> +++++++++++++++++++++++++++++++++++++--------
>>>>   1 file changed, 49 insertions(+), 10 deletions(-)
>>>>
>>>> diff --git a/tools/perf/builtin-record.c b/tools/perf/builtin-record.c
>>>> index eb6a199..480033f 100644
>>>> --- a/tools/perf/builtin-record.c
>>>> +++ b/tools/perf/builtin-record.c
>>>> @@ -125,7 +125,43 @@ out:
>>>>   static volatile int done;
>>>>   static volatile int signr = -1;
>>>>   static volatile int child_finished;
>>>> -static volatile int auxtrace_snapshot_enabled;
>>>> +
>>>> +static volatile enum {
>>>> +    AUXTRACE_SNAPSHOT_OFF = -1,
>>>> +    AUXTRACE_SNAPSHOT_DISABLED = 0,
>>>> +    AUXTRACE_SNAPSHOT_ENABLED = 1,
>>>> +} auxtrace_snapshot_state = AUXTRACE_SNAPSHOT_OFF;
>>>> +
>>>> +static inline void
>>>> +auxtrace_snapshot_on(void)
>>>> +{
>>>> +    auxtrace_snapshot_state = AUXTRACE_SNAPSHOT_DISABLED;
>>>> +}
>>>> +
>>>> +static inline void
>>>> +auxtrace_snapshot_enable(void)
>>>> +{
>>>> +    if (auxtrace_snapshot_state == AUXTRACE_SNAPSHOT_OFF)
>>>> +        return;
>>>> +    auxtrace_snapshot_state = AUXTRACE_SNAPSHOT_ENABLED;
>>>> +}
>>>> +
>>>> +static inline void
>>>> +auxtrace_snapshot_disable(void)
>>>> +{
>>>> +    if (auxtrace_snapshot_state == AUXTRACE_SNAPSHOT_OFF)
>>>> +        return;
>>>> +    auxtrace_snapshot_state = AUXTRACE_SNAPSHOT_DISABLED;
>>>> +}
>>>> +
>>>> +static inline bool
>>>> +auxtrace_snapshot_is_enabled(void)
>>>> +{
>>>> +    if (auxtrace_snapshot_state == AUXTRACE_SNAPSHOT_OFF)
>>>> +        return false;
>>>> +    return auxtrace_snapshot_state == AUXTRACE_SNAPSHOT_ENABLED;
>>>> +}
>>>> +
>>>>   static volatile int auxtrace_snapshot_err;
>>>>   static volatile int auxtrace_record__snapshot_started;
>>>>   @@ -249,7 +285,7 @@ static void record__read_auxtrace_snapshot(struct
>>>> record *rec)
>>>>       } else {
>>>>           auxtrace_snapshot_err =
>>>> auxtrace_record__snapshot_finish(rec->itr);
>>>>           if (!auxtrace_snapshot_err)
>>>> -            auxtrace_snapshot_enabled = 1;
>>>> +            auxtrace_snapshot_enable();
>>>>       }
>>>>   }
>>>>   @@ -615,10 +651,13 @@ static int __cmd_record(struct record *rec, int
>>>> argc, const char **argv)
>>>>       signal(SIGCHLD, sig_handler);
>>>>       signal(SIGINT, sig_handler);
>>>>       signal(SIGTERM, sig_handler);
>>>> -    if (rec->opts.auxtrace_snapshot_mode)
>>>> +
>>>> +    if (rec->opts.auxtrace_snapshot_mode) {
>>>>           signal(SIGUSR2, snapshot_sig_handler);
>>>> -    else
>>>> +        auxtrace_snapshot_on();
>>>> +    } else {
>>>>           signal(SIGUSR2, SIG_IGN);
>>>> +    }
>>>>         session = perf_session__new(file, false, tool);
>>>>       if (session == NULL) {
>>>> @@ -744,12 +783,12 @@ static int __cmd_record(struct record *rec, int
>>>> argc, const char **argv)
>>>>           perf_evlist__enable(rec->evlist);
>>>>       }
>>>>   -    auxtrace_snapshot_enabled = 1;
>>>> +    auxtrace_snapshot_enable();
>>>>       for (;;) {
>>>>           unsigned long long hits = rec->samples;
>>>>             if (record__mmap_read_all(rec) < 0) {
>>>> -            auxtrace_snapshot_enabled = 0;
>>>> +            auxtrace_snapshot_disable();
>>>>               err = -1;
>>>>               goto out_child;
>>>>           }
>>>> @@ -787,12 +826,12 @@ static int __cmd_record(struct record *rec, int
>>>> argc, const char **argv)
>>>>            * disable events in this case.
>>>>            */
>>>>           if (done && !disabled && !target__none(&opts->target)) {
>>>> -            auxtrace_snapshot_enabled = 0;
>>>> +            auxtrace_snapshot_disable();
>>>>               perf_evlist__disable(rec->evlist);
>>>>               disabled = true;
>>>>           }
>>>>       }
>>>> -    auxtrace_snapshot_enabled = 0;
>>>> +    auxtrace_snapshot_disable();
>>>>         if (forks && workload_exec_errno) {
>>>>           char msg[STRERR_BUFSIZE];
>>>> @@ -1358,9 +1397,9 @@ out_symbol_exit:
>>>>     static void snapshot_sig_handler(int sig __maybe_unused)
>>>>   {
>>>> -    if (!auxtrace_snapshot_enabled)
>>>> +    if (!auxtrace_snapshot_is_enabled())
>>>>           return;
>>>> -    auxtrace_snapshot_enabled = 0;
>>>> +    auxtrace_snapshot_disable();
>>>>       auxtrace_snapshot_err = auxtrace_record__snapshot_start(record.itr);
>>>>       auxtrace_record__snapshot_started = 1;
>>>>   }
>>>> -- 
>>>> 1.8.3.4
> 
> 
> 

Re: [PATCH 03/10] perf record: Turns auxtrace_snapshot_enable into 3 states

[Wangnan (F)]

On 2016/4/14 16:30, Adrian Hunter wrote:
> On 14/04/16 10:50, Wangnan (F) wrote:
>>
>> On 2016/4/14 15:15, Adrian Hunter wrote:
>>> On 13/04/16 18:55, Arnaldo Carvalho de Melo wrote:
>>>> Em Wed, Apr 13, 2016 at 08:21:06AM +0000, Wang Nan escreveu:
>>>>> auxtrace_snapshot_enable has only two states (0/1). Turns it into a
>>>>> triple states enum so SIGUSR2 handler can safely do other works without
>>>>> triggering auxtrace snapshot.
>>>> Adrian, can you take a look at this? Is it ok with you?
>>> Please forgive me if these are stupid questions:
>>>
>>> First I am wondering why we wouldn't want to snapshot auxtrace data at the
>>> same time as the perf buffer?
>> This patch doesn't prevent taking snapshot when receiving SIGUSR2.
> So it was a stupid question ;-)

Still thank you for pointing this. I suddenly realized the 
'switch_output_started'
in patch 5/10 is also need to be turned to a 3 state enum. If not, a 
SIGUSR2 incorrectly
triggers output switching even '--switch-output' is not provided when 
'--snapshot' exist.

>
>> If both --snapshot and --switch-outupt is provided, when SIGUSR2 received,
>> perf takes auxtrace snapshot and other perf buffer together.
> How do you keep from losing tracking information like MMAP events?  Are they
> is a different buffer?

Please see patch 8/10 and 9/10. MMAP events are resynthesized each time
when output file switched, so at the *head* of each 'perf.data' you can find
many MMAP/COMM/FORK... events.

After overwritable ring buffer is supported, there is a more aggresive
patch [1] resynthesize tracking events and put them at the *end* of
perf.data.

[1] 
https://git.kernel.org/cgit/linux/kernel/git/pi3orama/linux.git/commit/?h=perf/overwrite&id=747e10300397b9c28b01bca5bfad943c8cf2dcce

Re: [PATCH 03/10] perf record: Turns auxtrace_snapshot_enable into 3 states

[Adrian Hunter]

On 14/04/16 12:07, Wangnan (F) wrote:
> 
> 
> On 2016/4/14 16:30, Adrian Hunter wrote:
>> On 14/04/16 10:50, Wangnan (F) wrote:
>>>
>>> On 2016/4/14 15:15, Adrian Hunter wrote:
>>>> On 13/04/16 18:55, Arnaldo Carvalho de Melo wrote:
>>>>> Em Wed, Apr 13, 2016 at 08:21:06AM +0000, Wang Nan escreveu:
>>>>>> auxtrace_snapshot_enable has only two states (0/1). Turns it into a
>>>>>> triple states enum so SIGUSR2 handler can safely do other works without
>>>>>> triggering auxtrace snapshot.
>>>>> Adrian, can you take a look at this? Is it ok with you?
>>>> Please forgive me if these are stupid questions:
>>>>
>>>> First I am wondering why we wouldn't want to snapshot auxtrace data at the
>>>> same time as the perf buffer?
>>> This patch doesn't prevent taking snapshot when receiving SIGUSR2.
>> So it was a stupid question ;-)
> 
> Still thank you for pointing this. I suddenly realized the
> 'switch_output_started'
> in patch 5/10 is also need to be turned to a 3 state enum. If not, a SIGUSR2
> incorrectly
> triggers output switching even '--switch-output' is not provided when
> '--snapshot' exist.
> 
>>
>>> If both --snapshot and --switch-outupt is provided, when SIGUSR2 received,
>>> perf takes auxtrace snapshot and other perf buffer together.
>> How do you keep from losing tracking information like MMAP events?  Are they
>> is a different buffer?
> 
> Please see patch 8/10 and 9/10. MMAP events are resynthesized each time
> when output file switched, so at the *head* of each 'perf.data' you can find
> many MMAP/COMM/FORK... events.

OK, I see.

> 
> After overwritable ring buffer is supported, there is a more aggresive
> patch [1] resynthesize tracking events and put them at the *end* of
> perf.data.
> 
> [1]
> https://git.kernel.org/cgit/linux/kernel/git/pi3orama/linux.git/commit/?h=perf/overwrite&id=747e10300397b9c28b01bca5bfad943c8cf2dcce
> 

Thanks for the information.  Auxtrace really needs complete MMAP
information, so I would probably need to look at other options as well.

Anyway for this patch:

Acked-by: Adrian Hunter <adrian.hunter@intel.com>

[tip:perf/core] perf record: Turns auxtrace_snapshot_enable into 3 states

[tip-bot for Wang Nan]

Commit-ID:  c0bdc1c461dd5b2e492c0746708a3e0da6b13515
Gitweb:     http://git.kernel.org/tip/c0bdc1c461dd5b2e492c0746708a3e0da6b13515
Author:     Wang Nan <wangnan0@huawei.com>
AuthorDate: Wed, 13 Apr 2016 08:21:06 +0000
Committer:  Arnaldo Carvalho de Melo <acme@redhat.com>
CommitDate: Thu, 14 Apr 2016 08:58:19 -0300

perf record: Turns auxtrace_snapshot_enable into 3 states

auxtrace_snapshot_enable has only two states (0/1). Turns it into a
triple states enum so SIGUSR2 handler can safely do other works without
triggering auxtrace snapshot.

Signed-off-by: Wang Nan <wangnan0@huawei.com>
Acked-by: Adrian Hunter <adrian.hunter@intel.com>
Acked-by: Jiri Olsa <jolsa@kernel.org>
Cc: Masami Hiramatsu <mhiramat@kernel.org>
Cc: Namhyung Kim <namhyung@kernel.org>
Cc: Zefan Li <lizefan@huawei.com>
Cc: pi3orama@163.com
Link: http://lkml.kernel.org/r/1460535673-159866-4-git-send-email-wangnan0@huawei.com
Signed-off-by: He Kuang <hekuang@huawei.com>
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/builtin-record.c | 59 +++++++++++++++++++++++++++++++++++++--------
 1 file changed, 49 insertions(+), 10 deletions(-)

diff --git a/tools/perf/builtin-record.c b/tools/perf/builtin-record.c
index eb6a199..480033f 100644
--- a/tools/perf/builtin-record.c
+++ b/tools/perf/builtin-record.c
@@ -125,7 +125,43 @@ out:
 static volatile int done;
 static volatile int signr = -1;
 static volatile int child_finished;
-static volatile int auxtrace_snapshot_enabled;
+
+static volatile enum {
+	AUXTRACE_SNAPSHOT_OFF = -1,
+	AUXTRACE_SNAPSHOT_DISABLED = 0,
+	AUXTRACE_SNAPSHOT_ENABLED = 1,
+} auxtrace_snapshot_state = AUXTRACE_SNAPSHOT_OFF;
+
+static inline void
+auxtrace_snapshot_on(void)
+{
+	auxtrace_snapshot_state = AUXTRACE_SNAPSHOT_DISABLED;
+}
+
+static inline void
+auxtrace_snapshot_enable(void)
+{
+	if (auxtrace_snapshot_state == AUXTRACE_SNAPSHOT_OFF)
+		return;
+	auxtrace_snapshot_state = AUXTRACE_SNAPSHOT_ENABLED;
+}
+
+static inline void
+auxtrace_snapshot_disable(void)
+{
+	if (auxtrace_snapshot_state == AUXTRACE_SNAPSHOT_OFF)
+		return;
+	auxtrace_snapshot_state = AUXTRACE_SNAPSHOT_DISABLED;
+}
+
+static inline bool
+auxtrace_snapshot_is_enabled(void)
+{
+	if (auxtrace_snapshot_state == AUXTRACE_SNAPSHOT_OFF)
+		return false;
+	return auxtrace_snapshot_state == AUXTRACE_SNAPSHOT_ENABLED;
+}
+
 static volatile int auxtrace_snapshot_err;
 static volatile int auxtrace_record__snapshot_started;
 
@@ -249,7 +285,7 @@ static void record__read_auxtrace_snapshot(struct record *rec)
 	} else {
 		auxtrace_snapshot_err = auxtrace_record__snapshot_finish(rec->itr);
 		if (!auxtrace_snapshot_err)
-			auxtrace_snapshot_enabled = 1;
+			auxtrace_snapshot_enable();
 	}
 }
 
@@ -615,10 +651,13 @@ static int __cmd_record(struct record *rec, int argc, const char **argv)
 	signal(SIGCHLD, sig_handler);
 	signal(SIGINT, sig_handler);
 	signal(SIGTERM, sig_handler);
-	if (rec->opts.auxtrace_snapshot_mode)
+
+	if (rec->opts.auxtrace_snapshot_mode) {
 		signal(SIGUSR2, snapshot_sig_handler);
-	else
+		auxtrace_snapshot_on();
+	} else {
 		signal(SIGUSR2, SIG_IGN);
+	}
 
 	session = perf_session__new(file, false, tool);
 	if (session == NULL) {
@@ -744,12 +783,12 @@ static int __cmd_record(struct record *rec, int argc, const char **argv)
 		perf_evlist__enable(rec->evlist);
 	}
 
-	auxtrace_snapshot_enabled = 1;
+	auxtrace_snapshot_enable();
 	for (;;) {
 		unsigned long long hits = rec->samples;
 
 		if (record__mmap_read_all(rec) < 0) {
-			auxtrace_snapshot_enabled = 0;
+			auxtrace_snapshot_disable();
 			err = -1;
 			goto out_child;
 		}
@@ -787,12 +826,12 @@ static int __cmd_record(struct record *rec, int argc, const char **argv)
 		 * disable events in this case.
 		 */
 		if (done && !disabled && !target__none(&opts->target)) {
-			auxtrace_snapshot_enabled = 0;
+			auxtrace_snapshot_disable();
 			perf_evlist__disable(rec->evlist);
 			disabled = true;
 		}
 	}
-	auxtrace_snapshot_enabled = 0;
+	auxtrace_snapshot_disable();
 
 	if (forks && workload_exec_errno) {
 		char msg[STRERR_BUFSIZE];
@@ -1358,9 +1397,9 @@ out_symbol_exit:
 
 static void snapshot_sig_handler(int sig __maybe_unused)
 {
-	if (!auxtrace_snapshot_enabled)
+	if (!auxtrace_snapshot_is_enabled())
 		return;
-	auxtrace_snapshot_enabled = 0;
+	auxtrace_snapshot_disable();
 	auxtrace_snapshot_err = auxtrace_record__snapshot_start(record.itr);
 	auxtrace_record__snapshot_started = 1;
 }

[PATCH 04/10] perf record: Add '--timestamp-filename' option to append timestamp to output filename

[Wang Nan]

This option appends current timestamp to output. For example:

 # perf record -a --timestamp-filename
 ^C[ perf record: Woken up 1 times to write data ]
 [ perf record: Dump perf.data.2015122622265847 ]
 [ perf record: Captured and wrote 0.742 MB perf.data.<timestamp> (90 samples) ]
 # ls
 perf.data.201512262226584

Timestamp is useful for identifying each perf.data after 'perf record'
support generating multiple output files.

Signed-off-by: Wang Nan <wangnan0@huawei.com>
Signed-off-by: He Kuang <hekuang@huawei.com>
Cc: Arnaldo Carvalho de Melo <acme@redhat.com>
Cc: Jiri Olsa <jolsa@kernel.org>
Cc: Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com>
Cc: Namhyung Kim <namhyung@kernel.org>
Cc: Zefan Li <lizefan@huawei.com>
Cc: pi3orama@163.com
---
 tools/perf/builtin-record.c | 53 +++++++++++++++++++++++++++++++++++++++++----
 1 file changed, 49 insertions(+), 4 deletions(-)

diff --git a/tools/perf/builtin-record.c b/tools/perf/builtin-record.c
index 480033f..3239a6e 100644
--- a/tools/perf/builtin-record.c
+++ b/tools/perf/builtin-record.c
@@ -56,6 +56,7 @@ struct record {
 	bool			no_buildid_cache;
 	bool			no_buildid_cache_set;
 	bool			buildid_all;
+	bool			timestamp_filename;
 	unsigned long long	samples;
 };
 
@@ -531,6 +532,37 @@ record__finish_output(struct record *rec)
 	return;
 }
 
+static int
+record__switch_output(struct record *rec, bool at_exit)
+{
+	struct perf_data_file *file = &rec->file;
+	int fd, err;
+
+	/* Same Size:      "2015122520103046"*/
+	char timestamp[] = "InvalidTimestamp";
+
+	rec->samples = 0;
+	record__finish_output(rec);
+	err = fetch_current_timestamp(timestamp, sizeof(timestamp));
+	if (err) {
+		pr_err("Failed to get current timestamp\n");
+		return -EINVAL;
+	}
+
+	fd = perf_data_file__switch(file, timestamp,
+				    rec->session->header.data_offset,
+				    at_exit);
+	if (fd >= 0 && !at_exit) {
+		rec->bytes_written = 0;
+		rec->session->header.data_size = 0;
+	}
+
+	if (!quiet)
+		fprintf(stderr, "[ perf record: Dump %s.%s ]\n",
+			file->path, timestamp);
+	return fd;
+}
+
 static volatile int workload_exec_errno;
 
 /*
@@ -865,11 +897,22 @@ out_child:
 	/* this will be recalculated during process_buildids() */
 	rec->samples = 0;
 
-	if (!err)
-		record__finish_output(rec);
+	if (!err) {
+		if (!rec->timestamp_filename) {
+			record__finish_output(rec);
+		} else {
+			fd = record__switch_output(rec, true);
+			if (fd < 0) {
+				status = fd;
+				goto out_delete_session;
+			}
+		}
+	}
 
 	if (!err && !quiet) {
 		char samples[128];
+		const char *postfix = rec->timestamp_filename ?
+					".<timestamp>" : "";
 
 		if (rec->samples && !rec->opts.full_auxtrace)
 			scnprintf(samples, sizeof(samples),
@@ -877,9 +920,9 @@ out_child:
 		else
 			samples[0] = '\0';
 
-		fprintf(stderr,	"[ perf record: Captured and wrote %.3f MB %s%s ]\n",
+		fprintf(stderr,	"[ perf record: Captured and wrote %.3f MB %s%s%s ]\n",
 			perf_data_file__size(file) / 1024.0 / 1024.0,
-			file->path, samples);
+			file->path, postfix, samples);
 	}
 
 out_delete_session:
@@ -1249,6 +1292,8 @@ struct option __record_options[] = {
 		   "file", "vmlinux pathname"),
 	OPT_BOOLEAN(0, "buildid-all", &record.buildid_all,
 		    "Record build-id of all DSOs regardless of hits"),
+	OPT_BOOLEAN(0, "timestamp-filename", &record.timestamp_filename,
+		    "append timestamp to output filename"),
 	OPT_END()
 };
 
-- 
1.8.3.4

[tip:perf/core] perf record: Add '--timestamp-filename' option to append timestamp to output file name

[tip-bot for Wang Nan]

Commit-ID:  ecfd7a9c044e98fc3da8937e652080bc5abe7918
Gitweb:     http://git.kernel.org/tip/ecfd7a9c044e98fc3da8937e652080bc5abe7918
Author:     Wang Nan <wangnan0@huawei.com>
AuthorDate: Wed, 13 Apr 2016 08:21:07 +0000
Committer:  Arnaldo Carvalho de Melo <acme@redhat.com>
CommitDate: Thu, 14 Apr 2016 09:00:39 -0300

perf record: Add '--timestamp-filename' option to append timestamp to output file name

This option appends current timestamp to the output file name.

For example:

  # perf record -a --timestamp-filename
  ^C[ perf record: Woken up 1 times to write data ]
  [ perf record: Dump perf.data.2015122622265847 ]
  [ perf record: Captured and wrote 0.742 MB perf.data.<timestamp> (90 samples) ]
  # ls
  perf.data.201512262226584

The timestamp will be useful for identifying each perf.data after the
'perf record' support for generating multiple output files gets
introduced.

Signed-off-by: Wang Nan <wangnan0@huawei.com>
Tested-by: Arnaldo Carvalho de Melo <acme@redhat.com>
Cc: Jiri Olsa <jolsa@kernel.org>
Cc: Masami Hiramatsu <mhiramat@kernel.org>
Cc: Namhyung Kim <namhyung@kernel.org>
Cc: Zefan Li <lizefan@huawei.com>
Cc: pi3orama@163.com
Link: http://lkml.kernel.org/r/1460535673-159866-5-git-send-email-wangnan0@huawei.com
Signed-off-by: He Kuang <hekuang@huawei.com>
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/builtin-record.c | 53 +++++++++++++++++++++++++++++++++++++++++----
 1 file changed, 49 insertions(+), 4 deletions(-)

diff --git a/tools/perf/builtin-record.c b/tools/perf/builtin-record.c
index 480033f..3239a6e 100644
--- a/tools/perf/builtin-record.c
+++ b/tools/perf/builtin-record.c
@@ -56,6 +56,7 @@ struct record {
 	bool			no_buildid_cache;
 	bool			no_buildid_cache_set;
 	bool			buildid_all;
+	bool			timestamp_filename;
 	unsigned long long	samples;
 };
 
@@ -531,6 +532,37 @@ record__finish_output(struct record *rec)
 	return;
 }
 
+static int
+record__switch_output(struct record *rec, bool at_exit)
+{
+	struct perf_data_file *file = &rec->file;
+	int fd, err;
+
+	/* Same Size:      "2015122520103046"*/
+	char timestamp[] = "InvalidTimestamp";
+
+	rec->samples = 0;
+	record__finish_output(rec);
+	err = fetch_current_timestamp(timestamp, sizeof(timestamp));
+	if (err) {
+		pr_err("Failed to get current timestamp\n");
+		return -EINVAL;
+	}
+
+	fd = perf_data_file__switch(file, timestamp,
+				    rec->session->header.data_offset,
+				    at_exit);
+	if (fd >= 0 && !at_exit) {
+		rec->bytes_written = 0;
+		rec->session->header.data_size = 0;
+	}
+
+	if (!quiet)
+		fprintf(stderr, "[ perf record: Dump %s.%s ]\n",
+			file->path, timestamp);
+	return fd;
+}
+
 static volatile int workload_exec_errno;
 
 /*
@@ -865,11 +897,22 @@ out_child:
 	/* this will be recalculated during process_buildids() */
 	rec->samples = 0;
 
-	if (!err)
-		record__finish_output(rec);
+	if (!err) {
+		if (!rec->timestamp_filename) {
+			record__finish_output(rec);
+		} else {
+			fd = record__switch_output(rec, true);
+			if (fd < 0) {
+				status = fd;
+				goto out_delete_session;
+			}
+		}
+	}
 
 	if (!err && !quiet) {
 		char samples[128];
+		const char *postfix = rec->timestamp_filename ?
+					".<timestamp>" : "";
 
 		if (rec->samples && !rec->opts.full_auxtrace)
 			scnprintf(samples, sizeof(samples),
@@ -877,9 +920,9 @@ out_child:
 		else
 			samples[0] = '\0';
 
-		fprintf(stderr,	"[ perf record: Captured and wrote %.3f MB %s%s ]\n",
+		fprintf(stderr,	"[ perf record: Captured and wrote %.3f MB %s%s%s ]\n",
 			perf_data_file__size(file) / 1024.0 / 1024.0,
-			file->path, samples);
+			file->path, postfix, samples);
 	}
 
 out_delete_session:
@@ -1249,6 +1292,8 @@ struct option __record_options[] = {
 		   "file", "vmlinux pathname"),
 	OPT_BOOLEAN(0, "buildid-all", &record.buildid_all,
 		    "Record build-id of all DSOs regardless of hits"),
+	OPT_BOOLEAN(0, "timestamp-filename", &record.timestamp_filename,
+		    "append timestamp to output filename"),
 	OPT_END()
 };
 

[PATCH 05/10] perf record: Split output into multiple files via '--switch-output'

[Wang Nan]

Allow 'perf record' splits its output into multiple files.

For example:

 # ~/perf record -a --timestamp-filename --switch-output &
 [1] 10763
 # kill -s SIGUSR2 10763
 [ perf record: dump data: Woken up 1 times ]
 # [ perf record: Dump perf.data.2015122622314468 ]

 # kill -s SIGUSR2 10763
 [ perf record: dump data: Woken up 1 times ]
 # [ perf record: Dump perf.data.2015122622314762 ]

 # kill -s SIGUSR2 10763
 [ perf record: dump data: Woken up 1 times ]
 #[ perf record: Dump perf.data.2015122622315171 ]

 # fg
 perf record -a --timestamp-filename --switch-output
 ^C[ perf record: Woken up 1 times to write data ]
 [ perf record: Dump perf.data.2015122622315513 ]
 [ perf record: Captured and wrote 0.014 MB perf.data.<timestamp> (296 samples) ]

 # ls -l
 total 920
 -rw------- 1 root root 797692 Dec 26 22:31 perf.data.2015122622314468
 -rw------- 1 root root  59960 Dec 26 22:31 perf.data.2015122622314762
 -rw------- 1 root root  59912 Dec 26 22:31 perf.data.2015122622315171
 -rw------- 1 root root  19220 Dec 26 22:31 perf.data.2015122622315513

Signed-off-by: Wang Nan <wangnan0@huawei.com>
Signed-off-by: He Kuang <hekuang@huawei.com>
Cc: Arnaldo Carvalho de Melo <acme@redhat.com>
Cc: Jiri Olsa <jolsa@kernel.org>
Cc: Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com>
Cc: Namhyung Kim <namhyung@kernel.org>
Cc: Zefan Li <lizefan@huawei.com>
Cc: pi3orama@163.com
---
 tools/perf/builtin-record.c | 33 +++++++++++++++++++++++++++------
 1 file changed, 27 insertions(+), 6 deletions(-)

diff --git a/tools/perf/builtin-record.c b/tools/perf/builtin-record.c
index 3239a6e..431afd0 100644
--- a/tools/perf/builtin-record.c
+++ b/tools/perf/builtin-record.c
@@ -57,6 +57,7 @@ struct record {
 	bool			no_buildid_cache_set;
 	bool			buildid_all;
 	bool			timestamp_filename;
+	bool			switch_output;
 	unsigned long long	samples;
 };
 
@@ -165,6 +166,7 @@ auxtrace_snapshot_is_enabled(void)
 
 static volatile int auxtrace_snapshot_err;
 static volatile int auxtrace_record__snapshot_started;
+static volatile int switch_output_started;
 
 static void sig_handler(int sig)
 {
@@ -684,7 +686,7 @@ static int __cmd_record(struct record *rec, int argc, const char **argv)
 	signal(SIGINT, sig_handler);
 	signal(SIGTERM, sig_handler);
 
-	if (rec->opts.auxtrace_snapshot_mode) {
+	if (rec->opts.auxtrace_snapshot_mode || rec->switch_output) {
 		signal(SIGUSR2, snapshot_sig_handler);
 		auxtrace_snapshot_on();
 	} else {
@@ -836,6 +838,21 @@ static int __cmd_record(struct record *rec, int argc, const char **argv)
 			}
 		}
 
+		if (switch_output_started) {
+			switch_output_started = 0;
+
+			if (!quiet)
+				fprintf(stderr, "[ perf record: dump data: Woken up %ld times ]\n",
+					waking);
+			waking = 0;
+			fd = record__switch_output(rec, false);
+			if (fd < 0) {
+				pr_err("Failed to switch to new file\n");
+				err = fd;
+				goto out_child;
+			}
+		}
+
 		if (hits == rec->samples) {
 			if (done || draining)
 				break;
@@ -1294,6 +1311,8 @@ struct option __record_options[] = {
 		    "Record build-id of all DSOs regardless of hits"),
 	OPT_BOOLEAN(0, "timestamp-filename", &record.timestamp_filename,
 		    "append timestamp to output filename"),
+	OPT_BOOLEAN(0, "switch-output", &record.switch_output,
+		    "Switch output when receive SIGUSR2"),
 	OPT_END()
 };
 
@@ -1442,9 +1461,11 @@ out_symbol_exit:
 
 static void snapshot_sig_handler(int sig __maybe_unused)
 {
-	if (!auxtrace_snapshot_is_enabled())
-		return;
-	auxtrace_snapshot_disable();
-	auxtrace_snapshot_err = auxtrace_record__snapshot_start(record.itr);
-	auxtrace_record__snapshot_started = 1;
+	if (auxtrace_snapshot_is_enabled()) {
+		auxtrace_snapshot_disable();
+		auxtrace_snapshot_err = auxtrace_record__snapshot_start(record.itr);
+		auxtrace_record__snapshot_started = 1;
+	}
+
+	switch_output_started = 1;
 }
-- 
1.8.3.4

[PATCH 06/10] perf record: Force enable --timestamp-filename when --switch-output is provided

[Wang Nan]

Without this patch, the last output doesn't have timestamp appended if
--timestamp-filename is not explicitly provided. For example:

 # perf record -a --switch-output &
 [1] 11224
 # kill -s SIGUSR2 11224
 [ perf record: dump data: Woken up 1 times ]
 # [ perf record: Dump perf.data.2015122622372823 ]

 # fg
 perf record -a --switch-output
 ^C[ perf record: Woken up 1 times to write data ]
 [ perf record: Captured and wrote 0.027 MB perf.data (540 samples) ]

 # ls -l
 total 836
 -rw------- 1 root root  33256 Dec 26 22:37 perf.data   <---- *Odd*
 -rw------- 1 root root 817156 Dec 26 22:37 perf.data.2015122622372823

Signed-off-by: Wang Nan <wangnan0@huawei.com>
Signed-off-by: He Kuang <hekuang@huawei.com>
Cc: Arnaldo Carvalho de Melo <acme@redhat.com>
Cc: Jiri Olsa <jolsa@kernel.org>
Cc: Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com>
Cc: Namhyung Kim <namhyung@kernel.org>
Cc: Zefan Li <lizefan@huawei.com>
Cc: pi3orama@163.com
---
 tools/perf/builtin-record.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/tools/perf/builtin-record.c b/tools/perf/builtin-record.c
index 431afd0..eacac5e 100644
--- a/tools/perf/builtin-record.c
+++ b/tools/perf/builtin-record.c
@@ -1368,6 +1368,9 @@ int cmd_record(int argc, const char **argv, const char *prefix __maybe_unused)
 		return -EINVAL;
 	}
 
+	if (rec->switch_output)
+		rec->timestamp_filename = true;
+
 	if (!rec->itr) {
 		rec->itr = auxtrace_record__init(rec->evlist, &err);
 		if (err)
-- 
1.8.3.4

[PATCH 07/10] perf record: Disable buildid cache options by default in switch output mode

[Wang Nan]

Cost of buildid cache processing is high: reading all events in output
perf.data, opening each elf file to read buildids then copying them into
~/.debug directory. In switch output mode, these heavy works block perf
from receiving perf events for too long.

Enable no-buildid and no-buildid-cache by default if --switch-output
is provided. Still allow user use --no-no-buildid to explicitly enable
buildid in this case.

Signed-off-by: Wang Nan <wangnan0@huawei.com>
Signed-off-by: He Kuang <hekuang@huawei.com>
Cc: Arnaldo Carvalho de Melo <acme@redhat.com>
Cc: Jiri Olsa <jolsa@kernel.org>
Cc: Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com>
Cc: Namhyung Kim <namhyung@kernel.org>
Cc: Zefan Li <lizefan@huawei.com>
Cc: pi3orama@163.com
---
 tools/perf/builtin-record.c | 30 +++++++++++++++++++++++++++++-
 1 file changed, 29 insertions(+), 1 deletion(-)

diff --git a/tools/perf/builtin-record.c b/tools/perf/builtin-record.c
index eacac5e..714d79d 100644
--- a/tools/perf/builtin-record.c
+++ b/tools/perf/builtin-record.c
@@ -1404,8 +1404,36 @@ int cmd_record(int argc, const char **argv, const char *prefix __maybe_unused)
 "If some relocation was applied (e.g. kexec) symbols may be misresolved\n"
 "even with a suitable vmlinux or kallsyms file.\n\n");
 
-	if (rec->no_buildid_cache || rec->no_buildid)
+	if (rec->no_buildid_cache || rec->no_buildid) {
 		disable_buildid_cache();
+	} else if (rec->switch_output) {
+		/*
+		 * In 'perf record --switch-output', disable buildid
+		 * generation by default to reduce data file switching
+		 * overhead. Still generate buildid if they are required
+		 * explicitly using
+		 *
+		 *  perf record --signal-trigger --no-no-buildid \
+		 *              --no-no-buildid-cache
+		 *
+		 * Following code equals to:
+		 *
+		 * if ((rec->no_buildid || !rec->no_buildid_set) &&
+		 *     (rec->no_buildid_cache || !rec->no_buildid_cache_set))
+		 *         disable_buildid_cache();
+		 */
+		bool disable = true;
+
+		if (rec->no_buildid_set && !rec->no_buildid)
+			disable = false;
+		if (rec->no_buildid_cache_set && !rec->no_buildid_cache)
+			disable = false;
+		if (disable) {
+			rec->no_buildid = true;
+			rec->no_buildid_cache = true;
+			disable_buildid_cache();
+		}
+	}
 
 	if (rec->evlist->nr_entries == 0 &&
 	    perf_evlist__add_default(rec->evlist) < 0) {
-- 
1.8.3.4

[PATCH 08/10] perf record: Re-synthesize tracking events after output switching

[Wang Nan]

Tracking events describe kernel and threads. They are generated by
reading /proc/kallsyms, /proc/*/maps and /proc/*/task/* during
initialization of 'perf record', serialized into event sequences and put
at the head of 'perf.data'. In case of output switching, each output
file should contain those events.

This patch calls record__synthesize() during output switching, so the
event sequences described above can be collected again.

Signed-off-by: Wang Nan <wangnan0@huawei.com>
Signed-off-by: He Kuang <hekuang@huawei.com>
Cc: Arnaldo Carvalho de Melo <acme@redhat.com>
Cc: Jiri Olsa <jolsa@kernel.org>
Cc: Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com>
Cc: Namhyung Kim <namhyung@kernel.org>
Cc: Zefan Li <lizefan@huawei.com>
Cc: pi3orama@163.com
---
 tools/perf/builtin-record.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/tools/perf/builtin-record.c b/tools/perf/builtin-record.c
index 714d79d..eedc50a 100644
--- a/tools/perf/builtin-record.c
+++ b/tools/perf/builtin-record.c
@@ -534,6 +534,8 @@ record__finish_output(struct record *rec)
 	return;
 }
 
+static int record__synthesize(struct record *rec);
+
 static int
 record__switch_output(struct record *rec, bool at_exit)
 {
@@ -562,6 +564,11 @@ record__switch_output(struct record *rec, bool at_exit)
 	if (!quiet)
 		fprintf(stderr, "[ perf record: Dump %s.%s ]\n",
 			file->path, timestamp);
+
+	/* Output tracking events */
+	if (!at_exit)
+		record__synthesize(rec);
+
 	return fd;
 }
 
-- 
1.8.3.4

[PATCH 09/10] perf record: Generate tracking events for process forked by perf

[Wang Nan]

With 'perf record --switch-output' without -a, record__synthesize() in
record__switch_output() won't generate tracking events because there's
no thread_map in evlist. Which causes newly created perf.data doesn't
contain map and comm information.

This patch creates a fake thread_map and directly call
perf_event__synthesize_thread_map() for those events.

Signed-off-by: Wang Nan <wangnan0@huawei.com>
Signed-off-by: He Kuang <hekuang@huawei.com>
Cc: Arnaldo Carvalho de Melo <acme@redhat.com>
Cc: Jiri Olsa <jolsa@kernel.org>
Cc: Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com>
Cc: Namhyung Kim <namhyung@kernel.org>
Cc: Zefan Li <lizefan@huawei.com>
Cc: pi3orama@163.com
---
 tools/perf/builtin-record.c | 31 ++++++++++++++++++++++++++++++-
 1 file changed, 30 insertions(+), 1 deletion(-)

diff --git a/tools/perf/builtin-record.c b/tools/perf/builtin-record.c
index eedc50a..cbd4d0f 100644
--- a/tools/perf/builtin-record.c
+++ b/tools/perf/builtin-record.c
@@ -534,6 +534,23 @@ record__finish_output(struct record *rec)
 	return;
 }
 
+static int record__synthesize_workload(struct record *rec)
+{
+	struct {
+		struct thread_map map;
+		struct thread_map_data map_data;
+	} thread_map;
+
+	thread_map.map.nr = 1;
+	thread_map.map.map[0].pid = rec->evlist->workload.pid;
+	thread_map.map.map[0].comm = NULL;
+	return perf_event__synthesize_thread_map(&rec->tool, &thread_map.map,
+						 process_synthesized_event,
+						 &rec->session->machines.host,
+						 rec->opts.sample_address,
+						 rec->opts.proc_map_timeout);
+}
+
 static int record__synthesize(struct record *rec);
 
 static int
@@ -566,9 +583,21 @@ record__switch_output(struct record *rec, bool at_exit)
 			file->path, timestamp);
 
 	/* Output tracking events */
-	if (!at_exit)
+	if (!at_exit) {
 		record__synthesize(rec);
 
+		/*
+		 * In 'perf record --switch-output' without -a,
+		 * record__synthesize() in record__switch_output() won't
+		 * generate tracking events because there's no thread_map
+		 * in evlist. Which causes newly created perf.data doesn't
+		 * contain map and comm information.
+		 * Create a fake thread_map and directly call
+		 * perf_event__synthesize_thread_map() for those events.
+		 */
+		if (target__none(&rec->opts.target))
+			record__synthesize_workload(rec);
+	}
 	return fd;
 }
 
-- 
1.8.3.4

[PATCH 10/10] perf core: Add backward attribute to perf event

[Wang Nan]

This patch introduces 'write_backward' bit to perf_event_attr, which
controls the direction of a ring buffer. After set, the corresponding
ring buffer is written from end to beginning. This feature is design to
support reading from overwritable ring buffer.

Ring buffer can be created by mapping a perf event fd. Kernel puts event
records into ring buffer, user programs like perf fetch them from
address returned by mmap(). To prevent racing between kernel and perf,
they communicate to each other through 'head' and 'tail' pointers.
Kernel maintains 'head' pointer, points it to the next free area (tail
of the last record). Perf maintains 'tail' pointer, points it to the
tail of last consumed record (record has already been fetched). Kernel
determines the available space in a ring buffer using these two
pointers to avoid overwrite unfetched records.

By mapping without 'PROT_WRITE', an overwritable ring buffer is created.
Different from normal ring buffer, perf is unable to maintain 'tail'
pointer because writing is forbidden. Therefore, for this type of ring
buffers, kernel overwrite old records unconditionally, works like flight
recorder. This feature would be useful if reading from overwritable ring
buffer were as easy as reading from normal ring buffer. However,
there's an obscure problem.

The following figure demonstrates a full overwritable ring buffer. In
this figure, the 'head' pointer points to the end of last record, and a
long record 'E' is pending. For a normal ring buffer, a 'tail' pointer
would have pointed to position (X), so kernel knows there's no more
space in the ring buffer. However, for an overwritable ring buffer,
kernel ignore the 'tail' pointer.

   (X)                              head
    .                                |
    .                                V
    +------+-------+----------+------+---+
    |A....A|B.....B|C........C|D....D|   |
    +------+-------+----------+------+---+

Record 'A' is overwritten by event 'E':

      head
       |
       V
    +--+---+-------+----------+------+---+
    |.E|..A|B.....B|C........C|D....D|E..|
    +--+---+-------+----------+------+---+

Now perf decides to read from this ring buffer. However, none of these
two natural positions, 'head' and the start of this ring buffer, are
pointing to the head of a record. Even the full ring buffer can be
accessed by perf, it is unable to find a position to start decoding.

The first attempt tries to solve this problem AFAIK can be found from
[1]. It makes kernel to maintain 'tail' pointer: updates it when ring
buffer is half full. However, this approach introduces overhead to
fast path. Test result shows a 1% overhead [2]. In addition, this method
utilizes no more tham 50% records.

Another attempt can be found from [3], which allows putting the size of
an event at the end of each record. This approach allows perf to find
records in a backword manner from 'head' pointer by reading size of a
record from its tail. However, because of alignment requirement, it
needs 8 bytes to record the size of a record, which is a huge waste. Its
performance is also not good, because more data need to be written.
This approach also introduces some extra branch instructions to fast
path.

'write_backward' is a better solution to this problem.

Following figure demonstrates the state of the overwritable ring buffer
when 'write_backward' is set before overwriting:

       head
        |
        V
    +---+------+----------+-------+------+
    |   |D....D|C........C|B.....B|A....A|
    +---+------+----------+-------+------+

and after overwriting:
                                     head
                                      |
                                      V
    +---+------+----------+-------+---+--+
    |..E|D....D|C........C|B.....B|A..|E.|
    +---+------+----------+-------+---+--+

In each situation, 'head' points to the beginning of the newest record.
>From this record, perf can iterate over the full ring buffer and fetch
records one by one.

The only limitation needs to consider is back-to-back reading. Due to
the non-deterministic of user program, it is impossible to ensure the
ring buffer keeps stable during reading. Consider an extreme situation:
perf is scheduled out after reading record 'D', then a burst of events
come, eat up the whole ring buffer (one or multiple rounds). When perf
comes back, reading after 'D' is incorrect now.

To prevent this problem, we need to find a way to ensure the ring buffer
is stable during reading. ioctl(PERF_EVENT_IOC_PAUSE_OUTPUT) is
suggested because its overhead is lower than
ioctl(PERF_EVENT_IOC_ENABLE).

By carefully verifying 'header' pointer, reader can avoid pausing the
ring-buffer. For example:

    /* A union of all possible events */
    union perf_event event;

    p = head = perf_mmap__read_head();
    while (true) {
        /* copy header of next event */
        fetch(&event.header, p, sizeof(event.header));

        /* read 'head' pointer */
        head = perf_mmap__read_head();

        /* check overwritten: is the header good? */
        if (!verify(sizeof(event.header), p, head))
            break;

        /* copy the whole event */
        fetch(&event, p, event.header.size);

        /* read 'head' pointer again */
        head = perf_mmap__read_head();

        /* is the whole event good? */
        if (!verify(event.header.size, p, head))
            break;
        p += event.header.size;
    }

However, the overhead is high because:

 a) In-place decoding is not safe.
    Copying-verifying-decoding is required.
 b) Fetching 'head' pointer requires additional synchronization.

(From Alexei Starovoitov:

Even this trick work, pause is needed for more than stability of
reading. When we collect the events into overwrite buffer we're waiting
for some other trigger (like all cpu utilization spike or just one cpu
running and all others are idle) and when it happens the buffer has
valuable info from the past. At this point new events are no longer
interesting and buffer should be paused, events read and unpaused until
next trigger comes.)

This patch utilizes event's default overflow_handler introduced
previously. perf_event_output_backward() is created as the default
overflow handler for backward ring buffers. To avoid extra overhead to
fast path, original perf_event_output() becomes __perf_event_output()
and marked '__always_inline'. In theory, there's no extra overhead
introduced to fast path.

Performance result:

Calling 3000000 times of 'close(-1)', use gettimeofday() to check
duration.  Use 'perf record -o /dev/null -e raw_syscalls:*' to capture
system calls. In ns.

Testing environment:

 CPU    : Intel(R) Core(TM) i7-4790 CPU @ 3.60GHz
 Kernel : v4.5.0
                   MEAN         STDVAR
BASE            800214.950    2853.083
PRE1           2253846.700    9997.014
PRE2           2257495.540    8516.293
POST           2250896.100    8933.921

Where 'BASE' is pure performance without capturing. 'PRE1' is test
result of pure 'v4.5.0' kernel. 'PRE2' is test result before this
patch. 'POST' is test result after this patch. See [4] for detail
experimental setup.

Considering the stdvar, this patch doesn't introduce performance
overhead to fast path.

[1] http://lkml.iu.edu/hypermail/linux/kernel/1304.1/04584.html
[2] http://lkml.iu.edu/hypermail/linux/kernel/1307.1/00535.html
[3] http://lkml.iu.edu/hypermail/linux/kernel/1512.0/01265.html
[4] http://lkml.kernel.org/g/56F89DCD.1040202@huawei.com

Signed-off-by: Wang Nan <wangnan0@huawei.com>
Acked-by: Alexei Starovoitov <ast@kernel.org>
Cc: He Kuang <hekuang@huawei.com>
Cc: Arnaldo Carvalho de Melo <acme@redhat.com>
Cc: Brendan Gregg <brendan.d.gregg@gmail.com>
Cc: Jiri Olsa <jolsa@kernel.org>
Cc: Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com>
Cc: Namhyung Kim <namhyung@kernel.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Zefan Li <lizefan@huawei.com>
Cc: pi3orama@163.com
---
 include/linux/perf_event.h      | 28 +++++++++++++++++++++---
 include/uapi/linux/perf_event.h |  3 ++-
 kernel/events/core.c            | 48 ++++++++++++++++++++++++++++++++++++-----
 kernel/events/ring_buffer.c     | 16 +++++++++++++-
 4 files changed, 85 insertions(+), 10 deletions(-)

diff --git a/include/linux/perf_event.h b/include/linux/perf_event.h
index 4065ca2..0cc36ad 100644
--- a/include/linux/perf_event.h
+++ b/include/linux/perf_event.h
@@ -834,14 +834,24 @@ extern int perf_event_overflow(struct perf_event *event,
 				 struct perf_sample_data *data,
 				 struct pt_regs *regs);
 
+extern void perf_event_output_forward(struct perf_event *event,
+				     struct perf_sample_data *data,
+				     struct pt_regs *regs);
+extern void perf_event_output_backward(struct perf_event *event,
+				       struct perf_sample_data *data,
+				       struct pt_regs *regs);
 extern void perf_event_output(struct perf_event *event,
-				struct perf_sample_data *data,
-				struct pt_regs *regs);
+			      struct perf_sample_data *data,
+			      struct pt_regs *regs);
 
 static inline bool
 is_default_overflow_handler(struct perf_event *event)
 {
-	return (event->overflow_handler == perf_event_output);
+	if (likely(event->overflow_handler == perf_event_output_forward))
+		return true;
+	if (unlikely(event->overflow_handler == perf_event_output_backward))
+		return true;
+	return false;
 }
 
 extern void
@@ -1042,8 +1052,20 @@ static inline bool has_aux(struct perf_event *event)
 	return event->pmu->setup_aux;
 }
 
+static inline bool is_write_backward(struct perf_event *event)
+{
+	return !!event->attr.write_backward;
+}
+
 extern int perf_output_begin(struct perf_output_handle *handle,
 			     struct perf_event *event, unsigned int size);
+extern int perf_output_begin_forward(struct perf_output_handle *handle,
+				    struct perf_event *event,
+				    unsigned int size);
+extern int perf_output_begin_backward(struct perf_output_handle *handle,
+				      struct perf_event *event,
+				      unsigned int size);
+
 extern void perf_output_end(struct perf_output_handle *handle);
 extern unsigned int perf_output_copy(struct perf_output_handle *handle,
 			     const void *buf, unsigned int len);
diff --git a/include/uapi/linux/perf_event.h b/include/uapi/linux/perf_event.h
index a3c1903..43fc8d2 100644
--- a/include/uapi/linux/perf_event.h
+++ b/include/uapi/linux/perf_event.h
@@ -340,7 +340,8 @@ struct perf_event_attr {
 				comm_exec      :  1, /* flag comm events that are due to an exec */
 				use_clockid    :  1, /* use @clockid for time fields */
 				context_switch :  1, /* context switch data */
-				__reserved_1   : 37;
+				write_backward :  1, /* Write ring buffer from end to beginning */
+				__reserved_1   : 36;
 
 	union {
 		__u32		wakeup_events;	  /* wakeup every n events */
diff --git a/kernel/events/core.c b/kernel/events/core.c
index 8c3b35f..263a9d8 100644
--- a/kernel/events/core.c
+++ b/kernel/events/core.c
@@ -5693,9 +5693,13 @@ void perf_prepare_sample(struct perf_event_header *header,
 	}
 }
 
-void perf_event_output(struct perf_event *event,
-			struct perf_sample_data *data,
-			struct pt_regs *regs)
+static void __always_inline
+__perf_event_output(struct perf_event *event,
+		    struct perf_sample_data *data,
+		    struct pt_regs *regs,
+		    int (*output_begin)(struct perf_output_handle *,
+					struct perf_event *,
+					unsigned int))
 {
 	struct perf_output_handle handle;
 	struct perf_event_header header;
@@ -5705,7 +5709,7 @@ void perf_event_output(struct perf_event *event,
 
 	perf_prepare_sample(&header, data, event, regs);
 
-	if (perf_output_begin(&handle, event, header.size))
+	if (output_begin(&handle, event, header.size))
 		goto exit;
 
 	perf_output_sample(&handle, &header, data, event);
@@ -5716,6 +5720,30 @@ exit:
 	rcu_read_unlock();
 }
 
+void
+perf_event_output_forward(struct perf_event *event,
+			 struct perf_sample_data *data,
+			 struct pt_regs *regs)
+{
+	__perf_event_output(event, data, regs, perf_output_begin_forward);
+}
+
+void
+perf_event_output_backward(struct perf_event *event,
+			   struct perf_sample_data *data,
+			   struct pt_regs *regs)
+{
+	__perf_event_output(event, data, regs, perf_output_begin_backward);
+}
+
+void
+perf_event_output(struct perf_event *event,
+		  struct perf_sample_data *data,
+		  struct pt_regs *regs)
+{
+	__perf_event_output(event, data, regs, perf_output_begin);
+}
+
 /*
  * read event_id
  */
@@ -8152,8 +8180,11 @@ perf_event_alloc(struct perf_event_attr *attr, int cpu,
 	if (overflow_handler) {
 		event->overflow_handler	= overflow_handler;
 		event->overflow_handler_context = context;
+	} else if (is_write_backward(event)){
+		event->overflow_handler = perf_event_output_backward;
+		event->overflow_handler_context = NULL;
 	} else {
-		event->overflow_handler = perf_event_output;
+		event->overflow_handler = perf_event_output_forward;
 		event->overflow_handler_context = NULL;
 	}
 
@@ -8388,6 +8419,13 @@ perf_event_set_output(struct perf_event *event, struct perf_event *output_event)
 		goto out;
 
 	/*
+	 * Either writing ring buffer from beginning or from end.
+	 * Mixing is not allowed.
+	 */
+	if (is_write_backward(output_event) != is_write_backward(event))
+		goto out;
+
+	/*
 	 * If both events generate aux data, they must be on the same PMU
 	 */
 	if (has_aux(event) && has_aux(output_event) &&
diff --git a/kernel/events/ring_buffer.c b/kernel/events/ring_buffer.c
index 60be55a..c49bab4 100644
--- a/kernel/events/ring_buffer.c
+++ b/kernel/events/ring_buffer.c
@@ -230,10 +230,24 @@ out:
 	return -ENOSPC;
 }
 
+int perf_output_begin_forward(struct perf_output_handle *handle,
+			     struct perf_event *event, unsigned int size)
+{
+	return __perf_output_begin(handle, event, size, false);
+}
+
+int perf_output_begin_backward(struct perf_output_handle *handle,
+			       struct perf_event *event, unsigned int size)
+{
+	return __perf_output_begin(handle, event, size, true);
+}
+
 int perf_output_begin(struct perf_output_handle *handle,
 		      struct perf_event *event, unsigned int size)
 {
-	return __perf_output_begin(handle, event, size, false);
+
+	return __perf_output_begin(handle, event, size,
+				   unlikely(is_write_backward(event)));
 }
 
 unsigned int perf_output_copy(struct perf_output_handle *handle,
-- 
1.8.3.4

Re: [PATCH 00/10] perf tools: Use SIGUSR2 control data dumpping

[Arnaldo Carvalho de Melo]

Em Wed, Apr 13, 2016 at 08:21:03AM +0000, Wang Nan escreveu:
> This patch set is a preparation to support overwritable ring buffer.
> However, even without the kernel side core patch [1] is accept this
> patch set is still useful.
 
> With this patch set, perf switches output when receiving SIGUSR2. For
> example:
 
>  # perf record -a -F99 --switch-output &
>  [1] 26435
>  # kill -s SIGUSR2 26435
>  [ perf record: dump data: Woken up 1 times ]
>  # [ perf record: Dump perf.data.2016041323544373 ]
>  # kill -s SIGUSR2 26435
>  [ perf record: dump data: Woken up 1 times ]
>  # [ perf record: Dump perf.data.2016041323544730 ]
>  # fg
>  perf record -a -F99 --switch-output
>  ^C[ perf record: Woken up 1 times to write data ]
>  [ perf record: Dump perf.data.2016041323545019 ]
>  [ perf record: Captured and wrote 0.395 MB perf.data.<timestamp> ]
> 
> User can periodically generates perf trace with a simple script, then
> remove most of them, only keeps scripts collected when something
> unusual is detected.

Looks useful, I'll test it, but it ends up adding overhead at the time
of the switch that sometimes may not be interesting. In such cases maybe
it would be interesting to instead of a signal and the perf.data file
switch to just add a marker, i.e. a bpf or perf probe event, even the
equivalent to a trace_printk(), to be used later for doing the slicing.

But yeah, for lower freq events doing like you do, to allow use of 'perf
script', unchanged, to determine what slices (files) should be kept
looks useful.
 
> After [1], perf can be totally silent before receiving SIGUSR2. Trace
> is collected in kernel overwritable ring buffer, and dumpped when
> SIGUSR2 is received.
> 
> [1] http://lkml.kernel.org/r/1459865478-53413-1-git-send-email-wangnan0@huawei.com
> 
> Cc: Wang Nan <wangnan0@huawei.com>
> Cc: He Kuang <hekuang@huawei.com>
> Cc: Arnaldo Carvalho de Melo <acme@redhat.com>
> Cc: Jiri Olsa <jolsa@kernel.org>
> Cc: Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com>
> Cc: Namhyung Kim <namhyung@kernel.org>
> Cc: Zefan Li <lizefan@huawei.com>
> Cc: pi3orama@163.com
> 
> Wang Nan (10):
>   perf tools: Make ordered_events reusable
>   perf tools: Add perf_data_file__switch() helper
>   perf record: Turns auxtrace_snapshot_enable into 3 states
>   perf record: Add '--timestamp-filename' option to append timestamp to
>     output filename
>   perf record: Split output into multiple files via '--switch-output'
>   perf record: Force enable --timestamp-filename when --switch-output is
>     provided
>   perf record: Disable buildid cache options by default in switch output
>     mode
>   perf record: Re-synthesize tracking events after output switching
>   perf record: Generate tracking events for process forked by perf
>   perf core: Add backward attribute to perf event
> 
>  include/linux/perf_event.h       |  28 +++++-
>  include/uapi/linux/perf_event.h  |   3 +-
>  kernel/events/core.c             |  48 ++++++++-
>  kernel/events/ring_buffer.c      |  16 ++-
>  tools/perf/builtin-record.c      | 208 +++++++++++++++++++++++++++++++++++----
>  tools/perf/util/data.c           |  41 ++++++++
>  tools/perf/util/data.h           |  11 ++-
>  tools/perf/util/ordered-events.c |   9 ++
>  tools/perf/util/ordered-events.h |   1 +
>  tools/perf/util/session.c        |   6 +-
>  10 files changed, 341 insertions(+), 30 deletions(-)
> 
> -- 
> 1.8.3.4

Re: [PATCH 00/10] perf tools: Use SIGUSR2 control data dumpping

[Jiri Olsa]

On Wed, Apr 13, 2016 at 08:21:03AM +0000, Wang Nan wrote:
> This patch set is a preparation to support overwritable ring buffer.
> However, even without the kernel side core patch [1] is accept this
> patch set is still useful.
> 
> With this patch set, perf switches output when receiving SIGUSR2. For
> example:
> 
>  # perf record -a -F99 --switch-output &
>  [1] 26435
>  # kill -s SIGUSR2 26435
>  [ perf record: dump data: Woken up 1 times ]
>  # [ perf record: Dump perf.data.2016041323544373 ]
>  # kill -s SIGUSR2 26435
>  [ perf record: dump data: Woken up 1 times ]
>  # [ perf record: Dump perf.data.2016041323544730 ]
>  # fg
>  perf record -a -F99 --switch-output
>  ^C[ perf record: Woken up 1 times to write data ]
>  [ perf record: Dump perf.data.2016041323545019 ]
>  [ perf record: Captured and wrote 0.395 MB perf.data.<timestamp> ]
> 
> User can periodically generates perf trace with a simple script, then
> remove most of them, only keeps scripts collected when something
> unusual is detected.
> 
> After [1], perf can be totally silent before receiving SIGUSR2. Trace
> is collected in kernel overwritable ring buffer, and dumpped when
> SIGUSR2 is received.
> 
> [1] http://lkml.kernel.org/r/1459865478-53413-1-git-send-email-wangnan0@huawei.com
> 
> Cc: Wang Nan <wangnan0@huawei.com>
> Cc: He Kuang <hekuang@huawei.com>
> Cc: Arnaldo Carvalho de Melo <acme@redhat.com>
> Cc: Jiri Olsa <jolsa@kernel.org>
> Cc: Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com>
> Cc: Namhyung Kim <namhyung@kernel.org>
> Cc: Zefan Li <lizefan@huawei.com>
> Cc: pi3orama@163.com
> 
> Wang Nan (10):
>   perf tools: Make ordered_events reusable
>   perf tools: Add perf_data_file__switch() helper
>   perf record: Turns auxtrace_snapshot_enable into 3 states
>   perf record: Add '--timestamp-filename' option to append timestamp to
>     output filename
>   perf record: Split output into multiple files via '--switch-output'
>   perf record: Force enable --timestamp-filename when --switch-output is
>     provided
>   perf record: Disable buildid cache options by default in switch output
>     mode
>   perf record: Re-synthesize tracking events after output switching
>   perf record: Generate tracking events for process forked by perf
>   perf core: Add backward attribute to perf event

I did not get 3/10 patch and the patchset did not apply cleanly,
git am failed.. would you have it in a branch somewhere?

thanks,
jirka

Re: [PATCH 00/10] perf tools: Use SIGUSR2 control data dumpping

[Wangnan (F)]

On 2016/4/15 18:40, Jiri Olsa wrote:
> On Wed, Apr 13, 2016 at 08:21:03AM +0000, Wang Nan wrote:
>> This patch set is a preparation to support overwritable ring buffer.
>> However, even without the kernel side core patch [1] is accept this
>> patch set is still useful.
>>
>> With this patch set, perf switches output when receiving SIGUSR2. For
>> example:
>>
>>   # perf record -a -F99 --switch-output &
>>   [1] 26435
>>   # kill -s SIGUSR2 26435
>>   [ perf record: dump data: Woken up 1 times ]
>>   # [ perf record: Dump perf.data.2016041323544373 ]
>>   # kill -s SIGUSR2 26435
>>   [ perf record: dump data: Woken up 1 times ]
>>   # [ perf record: Dump perf.data.2016041323544730 ]
>>   # fg
>>   perf record -a -F99 --switch-output
>>   ^C[ perf record: Woken up 1 times to write data ]
>>   [ perf record: Dump perf.data.2016041323545019 ]
>>   [ perf record: Captured and wrote 0.395 MB perf.data.<timestamp> ]
>>
>> User can periodically generates perf trace with a simple script, then
>> remove most of them, only keeps scripts collected when something
>> unusual is detected.
>>
>> After [1], perf can be totally silent before receiving SIGUSR2. Trace
>> is collected in kernel overwritable ring buffer, and dumpped when
>> SIGUSR2 is received.
>>
>> [1] http://lkml.kernel.org/r/1459865478-53413-1-git-send-email-wangnan0@huawei.com
>>
>> Cc: Wang Nan <wangnan0@huawei.com>
>> Cc: He Kuang <hekuang@huawei.com>
>> Cc: Arnaldo Carvalho de Melo <acme@redhat.com>
>> Cc: Jiri Olsa <jolsa@kernel.org>
>> Cc: Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com>
>> Cc: Namhyung Kim <namhyung@kernel.org>
>> Cc: Zefan Li <lizefan@huawei.com>
>> Cc: pi3orama@163.com
>>
>> Wang Nan (10):
>>    perf tools: Make ordered_events reusable
>>    perf tools: Add perf_data_file__switch() helper
>>    perf record: Turns auxtrace_snapshot_enable into 3 states
>>    perf record: Add '--timestamp-filename' option to append timestamp to
>>      output filename
>>    perf record: Split output into multiple files via '--switch-output'
>>    perf record: Force enable --timestamp-filename when --switch-output is
>>      provided
>>    perf record: Disable buildid cache options by default in switch output
>>      mode
>>    perf record: Re-synthesize tracking events after output switching
>>    perf record: Generate tracking events for process forked by perf
>>    perf core: Add backward attribute to perf event
> I did not get 3/10 patch and the patchset did not apply cleanly,
> git am failed.. would you have it in a branch somewhere?

Sorry, you are not in the CC list. 'git send-email' failed to extract your
email address from the Acked-by tag.

I'll inform you after I putting them into a git branch. Please wait.

Thank you.

Re: [PATCH 00/10] perf tools: Use SIGUSR2 control data dumpping

[Wangnan (F)]

On 2016/4/15 18:45, Wangnan (F) wrote:
>
>
> On 2016/4/15 18:40, Jiri Olsa wrote:
>> On Wed, Apr 13, 2016 at 08:21:03AM +0000, Wang Nan wrote: 

[SNIP]

>> I did not get 3/10 patch and the patchset did not apply cleanly,
>> git am failed.. would you have it in a branch somewhere?
>
> Sorry, you are not in the CC list. 'git send-email' failed to extract 
> your
> email address from the Acked-by tag.
>
> I'll inform you after I putting them into a git branch. Please wait.
>
> Thank you.

I just realized Arnaldo has already collected these patches set into
his perf/core. Please see it in his git tree [1]. You can also have a look
at my git tree [2] if you want :)

[1] 
https://git.kernel.org/cgit/linux/kernel/git/acme/linux.git/commit/?h=perf/core&id=c0bdc1c461dd5b2e492c0746708a3e0da6b13515
[2] 
https://git.kernel.org/cgit/linux/kernel/git/pi3orama/linux.git/log/?h=perf/overwrite

Re: [PATCH 00/10] perf tools: Use SIGUSR2 control data dumpping

[Arnaldo Carvalho de Melo]

Em Fri, Apr 15, 2016 at 07:40:44PM +0800, Wangnan (F) escreveu:
> 
> 
> On 2016/4/15 18:45, Wangnan (F) wrote:
> >
> >
> >On 2016/4/15 18:40, Jiri Olsa wrote:
> >>On Wed, Apr 13, 2016 at 08:21:03AM +0000, Wang Nan wrote:
> 
> [SNIP]
> 
> >>I did not get 3/10 patch and the patchset did not apply cleanly,
> >>git am failed.. would you have it in a branch somewhere?
> >
> >Sorry, you are not in the CC list. 'git send-email' failed to extract your
> >email address from the Acked-by tag.
> >
> >I'll inform you after I putting them into a git branch. Please wait.
> >
> >Thank you.
> 
> I just realized Arnaldo has already collected these patches set into
> his perf/core. Please see it in his git tree [1]. You can also have a look
> at my git tree [2] if you want :)

I haven't pushed them to Ingo yet, so I can fix up things if Jiri has
any fixes or other requests,

- Arnaldo
 
> [1] https://git.kernel.org/cgit/linux/kernel/git/acme/linux.git/commit/?h=perf/core&id=c0bdc1c461dd5b2e492c0746708a3e0da6b13515
> [2] https://git.kernel.org/cgit/linux/kernel/git/pi3orama/linux.git/log/?h=perf/overwrite

Re: [PATCH 00/10] perf tools: Use SIGUSR2 control data dumpping

[Arnaldo Carvalho de Melo]

Em Fri, Apr 15, 2016 at 10:09:32AM -0300, Arnaldo Carvalho de Melo escreveu:
> Em Fri, Apr 15, 2016 at 07:40:44PM +0800, Wangnan (F) escreveu:
> > On 2016/4/15 18:45, Wangnan (F) wrote:
> > >On 2016/4/15 18:40, Jiri Olsa wrote:
> > >>On Wed, Apr 13, 2016 at 08:21:03AM +0000, Wang Nan wrote:
> > 
> > [SNIP]
> > 
> > >>I did not get 3/10 patch and the patchset did not apply cleanly,
> > >>git am failed.. would you have it in a branch somewhere?
> > >
> > >Sorry, you are not in the CC list. 'git send-email' failed to extract your
> > >email address from the Acked-by tag.
> > >
> > >I'll inform you after I putting them into a git branch. Please wait.
> > >
> > >Thank you.
> > 
> > I just realized Arnaldo has already collected these patches set into
> > his perf/core. Please see it in his git tree [1]. You can also have a look
> > at my git tree [2] if you want :)
> 
> I haven't pushed them to Ingo yet, so I can fix up things if Jiri has
> any fixes or other requests,

I moved those patches to a separate branch, perf/switch_output, till we get a
bit more review, I think I was too fast on tentatively processing this patchset
and have some questions, for instance, this part I thin really confusing, in
the main record loop:

       	switch_output_enable();
        for (;;) {
                unsigned long long hits = rec->samples;

                if (record__mmap_read_all(rec) < 0) {
                        auxtrace_snapshot_disable();
                        err = -1;
                        goto out_child;
                }
<SNIP>
                if (switch_output_is_disabled()) {
                        switch_output_enable();

                        if (!quiet)
                                fprintf(stderr, "[ perf record: dump data: Woken up %ld times ]\n",
                                        waking);
                        waking = 0;
                        fd = record__switch_output(rec, false);
                        if (fd < 0) {
                                pr_err("Failed to switch to new file\n");
                                err = fd;
                                goto out_child;
                        }
                }
<SNIP>
	}

That switch_output_enable() one we can't get to because it is part of that
trigger_ thing, so just by looking here we think switch_output is being enabled
unconditionally, when in fact it will check if it is "OFF" and if so, will not
"enable", then when we see switch_output_is_disabled() the question will return
false if it is "OFF", but what we read is "hey, this is not disabled, so it
must be enabled, no? Confusing :-\

Perhaps we should have multiple record loops, one really simple, the original
one, one for auxtrace, that, from what we've discussed so far, doesn't lok will
be supported together with output switch, and one for output switch?

Would be something like:

	if (switch_output)
		err = record__switch_output_read_events()
	else if (auxtrace)
		err = record__auxtrace_read_events()
	else
		err = record__read_events();

And then each of these loops don't need to have all those branches per mmap_read.

- Arnaldo
 
> - Arnaldo
>  
> > [1] https://git.kernel.org/cgit/linux/kernel/git/acme/linux.git/commit/?h=perf/core&id=c0bdc1c461dd5b2e492c0746708a3e0da6b13515
> > [2] https://git.kernel.org/cgit/linux/kernel/git/pi3orama/linux.git/log/?h=perf/overwrite

Re: [PATCH 00/10] perf tools: Use SIGUSR2 control data dumpping

[Wangnan (F)]

On 2016/4/16 0:26, Arnaldo Carvalho de Melo wrote:
> Em Fri, Apr 15, 2016 at 10:09:32AM -0300, Arnaldo Carvalho de Melo escreveu:
>> Em Fri, Apr 15, 2016 at 07:40:44PM +0800, Wangnan (F) escreveu:
>>> On 2016/4/15 18:45, Wangnan (F) wrote:
>>>> On 2016/4/15 18:40, Jiri Olsa wrote:
>>>>> On Wed, Apr 13, 2016 at 08:21:03AM +0000, Wang Nan wrote:
>>> [SNIP]
>>>
>>>>> I did not get 3/10 patch and the patchset did not apply cleanly,
>>>>> git am failed.. would you have it in a branch somewhere?
>>>> Sorry, you are not in the CC list. 'git send-email' failed to extract your
>>>> email address from the Acked-by tag.
>>>>
>>>> I'll inform you after I putting them into a git branch. Please wait.
>>>>
>>>> Thank you.
>>> I just realized Arnaldo has already collected these patches set into
>>> his perf/core. Please see it in his git tree [1]. You can also have a look
>>> at my git tree [2] if you want :)
>> I haven't pushed them to Ingo yet, so I can fix up things if Jiri has
>> any fixes or other requests,
> I moved those patches to a separate branch, perf/switch_output, till we get a
> bit more review, I think I was too fast on tentatively processing this patchset
> and have some questions, for instance, this part I thin really confusing, in
> the main record loop:
>
>         	switch_output_enable();
>          for (;;) {
>                  unsigned long long hits = rec->samples;
>
>                  if (record__mmap_read_all(rec) < 0) {
>                          auxtrace_snapshot_disable();
>                          err = -1;
>                          goto out_child;
>                  }
> <SNIP>
>                  if (switch_output_is_disabled()) {
>                          switch_output_enable();
>
>                          if (!quiet)
>                                  fprintf(stderr, "[ perf record: dump data: Woken up %ld times ]\n",
>                                          waking);
>                          waking = 0;
>                          fd = record__switch_output(rec, false);
>                          if (fd < 0) {
>                                  pr_err("Failed to switch to new file\n");
>                                  err = fd;
>                                  goto out_child;
>                          }
>                  }
> <SNIP>
> 	}
>
> That switch_output_enable() one we can't get to because it is part of that
> trigger_ thing, so just by looking here we think switch_output is being enabled
> unconditionally, when in fact it will check if it is "OFF" and if so, will not
> "enable", then when we see switch_output_is_disabled() the question will return
> false if it is "OFF", but what we read is "hey, this is not disabled, so it
> must be enabled, no? Confusing :-\

You are right. I think we should change the naming in trigger stuff:

TRIGGER_OFF:       this trigger is turned off
TRIGGER_RELEASED:   preparing to be triggered
TRIGGER_TOGGLED: things happened

actions:

OFF -> RELEASED    : on
RELEASED -> TOGGLED: toggle
TOGGLED-> RELEASED : release

conditions:

is_released()
is_toggled()

I'll send a v3 soon.

Thank you.

> Perhaps we should have multiple record loops, one really simple, the original
> one, one for auxtrace, that, from what we've discussed so far, doesn't lok will
> be supported together with output switch, and one for output switch?
>
> Would be something like:
>
> 	if (switch_output)
> 		err = record__switch_output_read_events()
> 	else if (auxtrace)
> 		err = record__auxtrace_read_events()
> 	else
> 		err = record__read_events();
>
> And then each of these loops don't need to have all those branches per mmap_read.
>
> - Arnaldo
>   
>> - Arnaldo
>>   
>>> [1] https://git.kernel.org/cgit/linux/kernel/git/acme/linux.git/commit/?h=perf/core&id=c0bdc1c461dd5b2e492c0746708a3e0da6b13515
>>> [2] https://git.kernel.org/cgit/linux/kernel/git/pi3orama/linux.git/log/?h=perf/overwrite

Re: [PATCH 00/10] perf tools: Use SIGUSR2 control data dumpping

[Wangnan (F)]

On 2016/4/16 0:48, Wangnan (F) wrote:
>
>
> On 2016/4/16 0:26, Arnaldo Carvalho de Melo wrote:
>> Em Fri, Apr 15, 2016 at 10:09:32AM -0300, Arnaldo Carvalho de Melo 
>> escreveu:
>>> Em Fri, Apr 15, 2016 at 07:40:44PM +0800, Wangnan (F) escreveu:
>>>> On 2016/4/15 18:45, Wangnan (F) wrote:
>>>>> On 2016/4/15 18:40, Jiri Olsa wrote:
>>>>>> On Wed, Apr 13, 2016 at 08:21:03AM +0000, Wang Nan wrote:
>>>> [SNIP]
>>>>
>>>>>> I did not get 3/10 patch and the patchset did not apply cleanly,
>>>>>> git am failed.. would you have it in a branch somewhere?
>>>>> Sorry, you are not in the CC list. 'git send-email' failed to 
>>>>> extract your
>>>>> email address from the Acked-by tag.
>>>>>
>>>>> I'll inform you after I putting them into a git branch. Please wait.
>>>>>
>>>>> Thank you.
>>>> I just realized Arnaldo has already collected these patches set into
>>>> his perf/core. Please see it in his git tree [1]. You can also have 
>>>> a look
>>>> at my git tree [2] if you want :)
>>> I haven't pushed them to Ingo yet, so I can fix up things if Jiri has
>>> any fixes or other requests,
>> I moved those patches to a separate branch, perf/switch_output, till 
>> we get a
>> bit more review, I think I was too fast on tentatively processing 
>> this patchset
>> and have some questions, for instance, this part I thin really 
>> confusing, in
>> the main record loop:
>>
>>             switch_output_enable();
>>          for (;;) {
>>                  unsigned long long hits = rec->samples;
>>
>>                  if (record__mmap_read_all(rec) < 0) {
>>                          auxtrace_snapshot_disable();
>>                          err = -1;
>>                          goto out_child;
>>                  }
>> <SNIP>
>>                  if (switch_output_is_disabled()) {
>>                          switch_output_enable();
>>
>>                          if (!quiet)
>>                                  fprintf(stderr, "[ perf record: dump 
>> data: Woken up %ld times ]\n",
>>                                          waking);
>>                          waking = 0;
>>                          fd = record__switch_output(rec, false);
>>                          if (fd < 0) {
>>                                  pr_err("Failed to switch to new 
>> file\n");
>>                                  err = fd;
>>                                  goto out_child;
>>                          }
>>                  }
>> <SNIP>
>>     }
>>
>> That switch_output_enable() one we can't get to because it is part of 
>> that
>> trigger_ thing, so just by looking here we think switch_output is 
>> being enabled
>> unconditionally, when in fact it will check if it is "OFF" and if so, 
>> will not
>> "enable", then when we see switch_output_is_disabled() the question 
>> will return
>> false if it is "OFF", but what we read is "hey, this is not disabled, 
>> so it
>> must be enabled, no? Confusing :-\
>
> You are right. I think we should change the naming in trigger stuff:
>
> TRIGGER_OFF:       this trigger is turned off
> TRIGGER_RELEASED:   preparing to be triggered
> TRIGGER_TOGGLED: things happened
>
> actions:
>
> OFF -> RELEASED    : on
> RELEASED -> TOGGLED: toggle
> TOGGLED-> RELEASED : release
>
> conditions:
>
> is_released()
> is_toggled()
>
> I'll send a v3 soon.
>
> Thank you.
>
>> Perhaps we should have multiple record loops, one really simple, the 
>> original
>> one, one for auxtrace, that, from what we've discussed so far, 
>> doesn't lok will
>> be supported together with output switch, and one for output switch?
>>
>> Would be something like:
>>
>>     if (switch_output)
>>         err = record__switch_output_read_events()
>>     else if (auxtrace)
>>         err = record__auxtrace_read_events()
>>     else
>>         err = record__read_events();
>>
>> And then each of these loops don't need to have all those branches 
>> per mmap_read.
>>

Auxtrace and original events are not exclusive. Auxtrace and 
switch_output are
not necessarily exclusive. At lease intel_bts// works fine. It is 
intel_pt's own
limitation, not all auxtrace events.

Thank you.

>> - Arnaldo
>>> - Arnaldo
>>>> [1] 
>>>> https://git.kernel.org/cgit/linux/kernel/git/acme/linux.git/commit/?h=perf/core&id=c0bdc1c461dd5b2e492c0746708a3e0da6b13515
>>>> [2] 
>>>> https://git.kernel.org/cgit/linux/kernel/git/pi3orama/linux.git/log/?h=perf/overwrite
>