Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1281521 > unrolled thread

Re: [PATCH] perf record: Add snapshot mode support for perf's regular events

Started by"Wangnan (F)" <wangnan0@huawei.com>
First post2015-12-02 09:30 +0100
Last post2015-12-07 14:40 +0100
Articles 8 — 3 participants

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  Re: [PATCH] perf record: Add snapshot mode support for perf's regular  events "Wangnan (F)" <wangnan0@huawei.com> - 2015-12-02 09:30 +0100
    [RFC PATCH] perf/core: Put size of a sample at the end of it Wang Nan <wangnan0@huawei.com> - 2015-12-02 14:50 +0100
      Re: [RFC PATCH] perf/core: Put size of a sample at the end of it Peter Zijlstra <peterz@infradead.org> - 2015-12-03 11:10 +0100
        Re: [RFC PATCH] perf/core: Put size of a sample at the end of it "Wangnan (F)" <wangnan0@huawei.com> - 2015-12-03 11:40 +0100
      [RFC PATCH v2 2/3] perf tools: Enable overwrite settings Wang Nan <wangnan0@huawei.com> - 2015-12-07 14:40 +0100
      [RFC PATCH v2 0/3] perf core/perf tools: Utilizing overwrite ring buffer Wang Nan <wangnan0@huawei.com> - 2015-12-07 14:40 +0100
        [RFC PATCH v2 1/3] perf/core: Put size of a sample at the end of it Wang Nan <wangnan0@huawei.com> - 2015-12-07 14:40 +0100
        [RFC PATCH v2 3/3] perf record: Find tail pointer through size at end of event Wang Nan <wangnan0@huawei.com> - 2015-12-07 14:40 +0100

#1281521 — Re: [PATCH] perf record: Add snapshot mode support for perf's regular events

From"Wangnan (F)" <wangnan0@huawei.com>
Date2015-12-02 09:30 +0100
SubjectRe: [PATCH] perf record: Add snapshot mode support for perf's regular events
Message-ID<qBaXv-4EG-11@gated-at.bofh.it>
Hi Peter,

On 2015/11/25 17:27, Peter Zijlstra wrote:
> On Tue, Nov 24, 2015 at 10:00:31PM +0800, Yunlong Song wrote:
>> In our patch, we create and maintain a user space ring buffer to store
>> perf's tracing info, instead of directly writing to perf.data file as
>> before. In snapshot mode, only a SIGUSR2 signal can trigger perf to dump
>> the tracing info currently stored in the user space ring buffer to
>> perf.data file.
> I would very much like to first fix the perf overwrite mode: see
> lkml.kernel.org/r/20151023151205.GW11639@twins.programming.kicks-ass.net
I have an idea on this problem that, is it possible to
put the size of an event at the end of it when we working
on overwrite mode? So we can backtrack every events without
too much change to ring buffer?

Thank you.

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [next] | [standalone]

#1281807 — [RFC PATCH] perf/core: Put size of a sample at the end of it

FromWang Nan <wangnan0@huawei.com>
Date2015-12-02 14:50 +0100
Subject[RFC PATCH] perf/core: Put size of a sample at the end of it
Message-ID<qBfXe-7QB-61@gated-at.bofh.it>
In reply to#1281521
This is an RFC patch which is for overwrite mode ring buffer. I'd like
to discuss the correctness of this new idea for retriving as many
events as possible from overwrite mode ring buffer. If there's no
fundamental problem, I'll start perf side work.

The biggest problem for overwrite ring buffer is that it is hard to find
the start position of valid record. [1] and [2] tries to solve this
problem by introducing 'tail' and 'next_tail' into metadata page, and
update them each time the ring buffer is half full. Which adds more
instructions to event output code path, hurt performance. In addition,
even with them we still unable to recover all possible records. For
example:

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

If a record written at head pointer and it overwrites record A:

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

Record B is still valid but we can't get it through data_tail.

This patch suggests a different solution for this problem that, by
appending the length of a record at the end of it, user program is
possible to get every possible record in a backward manner, don't
need saving tail pointer.

For example:

   head
    |
    V
 +--+---+-------+----------+------+---+
 |E6|...|   B  8|   C    11|  D  7|E..|
 +--+---+-------+----------+------+---+

In this case, from the 'head' pointer provided by kernel, user program
can first see '6' by (*(head - sizeof(u16))), then it can get the start
pointer of record 'E', then it can read size and find start position
of record D, C, B in similar way.

Kernel side implementation is easy: simply adding a PERF_SAMPLE_SIZE
for the size output.

This sloution requires user program (perf) do more things. At least
following things and limitations should be considered:

 1. Before reading such ring buffer, perf must ensure all events which
    may output to it is already stopped, so the 'head' pointer it get
    is the end of the last record.

 2. We must ensure all events attached this ring buffer has
    'PERF_SAMPLE_SIZE' selected.

 3. There must no tracking events output to this ring buffer.

 4. 2 bytes extra space is required for each record.

Further improvement can be taken:

 1. If PERF_SAMPLE_SIZE is selected, we can avoid outputting the event
    size in header. Which eliminate extra space cose;

 2. We can find a way to append size information for tracking events
    also.

[1] http://lkml.kernel.org/r/20130708121557.GA17211@twins.programming.kicks-ass.net
[2] http://lkml.kernel.org/r/20151023151205.GW11639@twins.programming.kicks-ass.net

Signed-off-by: Wang Nan <wangnan0@huawei.com>
Cc: Adrian Hunter <adrian.hunter@intel.com>
Cc: Arnaldo Carvalho de Melo <acme@redhat.com>
Cc: David Ahern <dsahern@gmail.com>
Cc: Ingo Molnar <mingo@kernel.org>
Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
Cc: Yunlong Song <yunlong.song@huawei.com>
---
 include/uapi/linux/perf_event.h | 3 ++-
 kernel/events/core.c            | 6 ++++++
 2 files changed, 8 insertions(+), 1 deletion(-)

diff --git a/include/uapi/linux/perf_event.h b/include/uapi/linux/perf_event.h
index 1afe962..c4066da 100644
--- a/include/uapi/linux/perf_event.h
+++ b/include/uapi/linux/perf_event.h
@@ -139,8 +139,9 @@ enum perf_event_sample_format {
 	PERF_SAMPLE_IDENTIFIER			= 1U << 16,
 	PERF_SAMPLE_TRANSACTION			= 1U << 17,
 	PERF_SAMPLE_REGS_INTR			= 1U << 18,
+	PERF_SAMPLE_SIZE			= 1U << 19,
 
-	PERF_SAMPLE_MAX = 1U << 19,		/* non-ABI */
+	PERF_SAMPLE_MAX = 1U << 20,		/* non-ABI */
 };
 
 /*
diff --git a/kernel/events/core.c b/kernel/events/core.c
index 5854fcf..bbbacec 100644
--- a/kernel/events/core.c
+++ b/kernel/events/core.c
@@ -5473,6 +5473,9 @@ void perf_output_sample(struct perf_output_handle *handle,
 		}
 	}
 
+	if (sample_type & PERF_SAMPLE_SIZE)
+		perf_output_put(handle, header->size);
+
 	if (!event->attr.watermark) {
 		int wakeup_events = event->attr.wakeup_events;
 
@@ -5592,6 +5595,9 @@ void perf_prepare_sample(struct perf_event_header *header,
 
 		header->size += size;
 	}
+
+	if (sample_type & PERF_SAMPLE_SIZE)
+		header->size += sizeof(u16);
 }
 
 void perf_event_output(struct perf_event *event,
-- 
1.8.3.4

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1282890 — Re: [RFC PATCH] perf/core: Put size of a sample at the end of it

FromPeter Zijlstra <peterz@infradead.org>
Date2015-12-03 11:10 +0100
SubjectRe: [RFC PATCH] perf/core: Put size of a sample at the end of it
Message-ID<qByZP-3zg-3@gated-at.bofh.it>
In reply to#1281807
On Wed, Dec 02, 2015 at 01:38:19PM +0000, Wang Nan wrote:
> This sloution requires user program (perf) do more things. At least
> following things and limitations should be considered:
> 
>  1. Before reading such ring buffer, perf must ensure all events which
>     may output to it is already stopped, so the 'head' pointer it get
>     is the end of the last record.

Right, this is tricky, this would not allow two snapshots to happen back
to back since that would then result in a bunch of missed events.

Aside from this issue its a rather nice idea.

>  2. We must ensure all events attached this ring buffer has
>     'PERF_SAMPLE_SIZE' selected.

That can be easily enforced.

>  3. There must no tracking events output to this ring buffer.

That is rather unfortunate, we'd best fix that up.

>  4. 2 bytes extra space is required for each record.

8, perf records must be 8 byte aligned and sized.

> Further improvement can be taken:
> 
>  1. If PERF_SAMPLE_SIZE is selected, we can avoid outputting the event
>     size in header. Which eliminate extra space cose;

That would mandate you always parse the stream backwards. Which seems
rather unfortunate. Also, no you cannot recoup the extra space, see the
alignment and size requirement.

>  2. We can find a way to append size information for tracking events
>     also.

The !sample records you mean? Yes those had better have them too.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1282915 — Re: [RFC PATCH] perf/core: Put size of a sample at the end of it

From"Wangnan (F)" <wangnan0@huawei.com>
Date2015-12-03 11:40 +0100
SubjectRe: [RFC PATCH] perf/core: Put size of a sample at the end of it
Message-ID<qBzsS-3Jm-19@gated-at.bofh.it>
In reply to#1282890

On 2015/12/3 18:08, Peter Zijlstra wrote:
> On Wed, Dec 02, 2015 at 01:38:19PM +0000, Wang Nan wrote:
>> This sloution requires user program (perf) do more things. At least
>> following things and limitations should be considered:
>>
>>   1. Before reading such ring buffer, perf must ensure all events which
>>      may output to it is already stopped, so the 'head' pointer it get
>>      is the end of the last record.
> Right, this is tricky, this would not allow two snapshots to happen back
> to back since that would then result in a bunch of missed events.
>
> Aside from this issue its a rather nice idea.

Thank you for your attitude. We can start consider it seriously.

Now I'm working on perf side code to make a workable prototype.

>>   2. We must ensure all events attached this ring buffer has
>>      'PERF_SAMPLE_SIZE' selected.
> That can be easily enforced.

Yes.

>>   3. There must no tracking events output to this ring buffer.
> That is rather unfortunate, we'd best fix that up.
>
>>   4. 2 bytes extra space is required for each record.
> 8, perf records must be 8 byte aligned and sized.

So does it means we need to pad before outputing size?

>> Further improvement can be taken:
>>
>>   1. If PERF_SAMPLE_SIZE is selected, we can avoid outputting the event
>>      size in header. Which eliminate extra space cose;
> That would mandate you always parse the stream backwards. Which seems
> rather unfortunate. Also, no you cannot recoup the extra space, see the
> alignment and size requirement.

That's good. We don't need to consider this :)

Before receiving your comment I'm thinking about modifying
DEFINE_OUTPUT_COPY() to write first sizeof(header) bytes at the
end of reserved area, so it would work automatically for every
possible events.

>>   2. We can find a way to append size information for tracking events
>>      also.
> The !sample records you mean? Yes those had better have them too.

Thank you.

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1285304 — [RFC PATCH v2 2/3] perf tools: Enable overwrite settings

FromWang Nan <wangnan0@huawei.com>
Date2015-12-07 14:40 +0100
Subject[RFC PATCH v2 2/3] perf tools: Enable overwrite settings
Message-ID<qD4bg-52J-9@gated-at.bofh.it>
In reply to#1281807
This patch allows following config terms and option:

 # perf record --overwrite ...

   Globally set following events to overwrite;

 # perf record --event cycles/overwrite/ ...
 # perf record --event cycles/no-overwrite/ ...

Set specific events to be overwrite or no-overwrite.

Signed-off-by: Wang Nan <wangnan0@huawei.com>
---
 tools/perf/builtin-record.c    |  3 +++
 tools/perf/perf.h              |  2 ++
 tools/perf/util/evsel.c        |  4 ++++
 tools/perf/util/evsel.h        |  3 +++
 tools/perf/util/parse-events.c | 14 ++++++++++++++
 tools/perf/util/parse-events.h |  4 +++-
 tools/perf/util/parse-events.l |  2 ++
 7 files changed, 31 insertions(+), 1 deletion(-)

diff --git a/tools/perf/builtin-record.c b/tools/perf/builtin-record.c
index 2230b85..ec4135c 100644
--- a/tools/perf/builtin-record.c
+++ b/tools/perf/builtin-record.c
@@ -1061,6 +1061,9 @@ struct option __record_options[] = {
 	OPT_BOOLEAN_SET('i', "no-inherit", &record.opts.no_inherit,
 			&record.opts.no_inherit_set,
 			"child tasks do not inherit counters"),
+	OPT_BOOLEAN_SET(0, "overwrite", &record.opts.overwrite,
+			&record.opts.overwrite_set,
+			"use overwrite mode"),
 	OPT_UINTEGER('F', "freq", &record.opts.user_freq, "profile at this frequency"),
 	OPT_CALLBACK('m', "mmap-pages", &record.opts, "pages[,pages]",
 		     "number of mmap data pages and AUX area tracing mmap pages",
diff --git a/tools/perf/perf.h b/tools/perf/perf.h
index 90129ac..43d79fb 100644
--- a/tools/perf/perf.h
+++ b/tools/perf/perf.h
@@ -58,6 +58,8 @@ struct record_opts {
 	bool	     full_auxtrace;
 	bool	     auxtrace_snapshot_mode;
 	bool	     record_switch_events;
+	bool	     overwrite;
+	bool	     overwrite_set;
 	unsigned int freq;
 	unsigned int mmap_pages;
 	unsigned int auxtrace_mmap_pages;
diff --git a/tools/perf/util/evsel.c b/tools/perf/util/evsel.c
index 4dee8e3..3386437 100644
--- a/tools/perf/util/evsel.c
+++ b/tools/perf/util/evsel.c
@@ -668,6 +668,9 @@ static void apply_config_terms(struct perf_evsel *evsel,
 			 */
 			attr->inherit = term->val.inherit ? 1 : 0;
 			break;
+		case PERF_EVSEL__CONFIG_TERM_OVERWRITE:
+			evsel->overwrite = term->val.overwrite ? 1 : 0;
+			break;
 		default:
 			break;
 		}
@@ -743,6 +746,7 @@ void perf_evsel__config(struct perf_evsel *evsel, struct record_opts *opts)
 
 	attr->sample_id_all = perf_missing_features.sample_id_all ? 0 : 1;
 	attr->inherit	    = !opts->no_inherit;
+	evsel->overwrite    = opts->overwrite;
 
 	perf_evsel__set_sample_bit(evsel, IP);
 	perf_evsel__set_sample_bit(evsel, TID);
diff --git a/tools/perf/util/evsel.h b/tools/perf/util/evsel.h
index 51bab0f..c3b49e0 100644
--- a/tools/perf/util/evsel.h
+++ b/tools/perf/util/evsel.h
@@ -44,6 +44,7 @@ enum {
 	PERF_EVSEL__CONFIG_TERM_CALLGRAPH,
 	PERF_EVSEL__CONFIG_TERM_STACK_USER,
 	PERF_EVSEL__CONFIG_TERM_INHERIT,
+	PERF_EVSEL__CONFIG_TERM_OVERWRITE,
 	PERF_EVSEL__CONFIG_TERM_MAX,
 };
 
@@ -57,6 +58,7 @@ struct perf_evsel_config_term {
 		char	*callgraph;
 		u64	stack_user;
 		bool	inherit;
+		bool	overwrite;
 	} val;
 };
 
@@ -115,6 +117,7 @@ struct perf_evsel {
 	bool			tracking;
 	bool			per_pkg;
 	bool			precise_max;
+	bool			overwrite;
 	/* parse modifier helper */
 	int			exclude_GH;
 	int			nr_members;
diff --git a/tools/perf/util/parse-events.c b/tools/perf/util/parse-events.c
index c485b32..f20cc81 100644
--- a/tools/perf/util/parse-events.c
+++ b/tools/perf/util/parse-events.c
@@ -855,6 +855,12 @@ do {									   \
 	case PARSE_EVENTS__TERM_TYPE_NOINHERIT:
 		CHECK_TYPE_VAL(NUM);
 		break;
+	case PARSE_EVENTS__TERM_TYPE_OVERWRITE:
+		CHECK_TYPE_VAL(NUM);
+		break;
+	case PARSE_EVENTS__TERM_TYPE_NOOVERWRITE:
+		CHECK_TYPE_VAL(NUM);
+		break;
 	case PARSE_EVENTS__TERM_TYPE_NAME:
 		CHECK_TYPE_VAL(STR);
 		break;
@@ -892,6 +898,8 @@ static int config_term_tracepoint(struct perf_event_attr *attr,
 	case PARSE_EVENTS__TERM_TYPE_STACKSIZE:
 	case PARSE_EVENTS__TERM_TYPE_INHERIT:
 	case PARSE_EVENTS__TERM_TYPE_NOINHERIT:
+	case PARSE_EVENTS__TERM_TYPE_OVERWRITE:
+	case PARSE_EVENTS__TERM_TYPE_NOOVERWRITE:
 		return config_term_common(attr, term, err);
 	default:
 		if (err) {
@@ -961,6 +969,12 @@ do {								\
 		case PARSE_EVENTS__TERM_TYPE_NOINHERIT:
 			ADD_CONFIG_TERM(INHERIT, inherit, term->val.num ? 0 : 1);
 			break;
+		case PARSE_EVENTS__TERM_TYPE_OVERWRITE:
+			ADD_CONFIG_TERM(OVERWRITE, overwrite, term->val.num ? 1 : 0);
+			break;
+		case PARSE_EVENTS__TERM_TYPE_NOOVERWRITE:
+			ADD_CONFIG_TERM(OVERWRITE, overwrite, term->val.num ? 0 : 1);
+			break;
 		default:
 			break;
 		}
diff --git a/tools/perf/util/parse-events.h b/tools/perf/util/parse-events.h
index c34615f..29cc804 100644
--- a/tools/perf/util/parse-events.h
+++ b/tools/perf/util/parse-events.h
@@ -68,7 +68,9 @@ enum {
 	PARSE_EVENTS__TERM_TYPE_CALLGRAPH,
 	PARSE_EVENTS__TERM_TYPE_STACKSIZE,
 	PARSE_EVENTS__TERM_TYPE_NOINHERIT,
-	PARSE_EVENTS__TERM_TYPE_INHERIT
+	PARSE_EVENTS__TERM_TYPE_INHERIT,
+	PARSE_EVENTS__TERM_TYPE_NOOVERWRITE,
+	PARSE_EVENTS__TERM_TYPE_OVERWRITE,
 };
 
 struct parse_events_array {
diff --git a/tools/perf/util/parse-events.l b/tools/perf/util/parse-events.l
index 27d567f..2ef6f96 100644
--- a/tools/perf/util/parse-events.l
+++ b/tools/perf/util/parse-events.l
@@ -202,6 +202,8 @@ call-graph		{ return term(yyscanner, PARSE_EVENTS__TERM_TYPE_CALLGRAPH); }
 stack-size		{ return term(yyscanner, PARSE_EVENTS__TERM_TYPE_STACKSIZE); }
 inherit			{ return term(yyscanner, PARSE_EVENTS__TERM_TYPE_INHERIT); }
 no-inherit		{ return term(yyscanner, PARSE_EVENTS__TERM_TYPE_NOINHERIT); }
+overwrite		{ return term(yyscanner, PARSE_EVENTS__TERM_TYPE_OVERWRITE); }
+no-overwrite		{ return term(yyscanner, PARSE_EVENTS__TERM_TYPE_NOOVERWRITE); }
 ,			{ return ','; }
 "/"			{ BEGIN(INITIAL); return '/'; }
 {name_minus}		{ return str(yyscanner, PE_NAME); }
-- 
1.8.3.4

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1285305 — [RFC PATCH v2 0/3] perf core/perf tools: Utilizing overwrite ring buffer

FromWang Nan <wangnan0@huawei.com>
Date2015-12-07 14:40 +0100
Subject[RFC PATCH v2 0/3] perf core/perf tools: Utilizing overwrite ring buffer
Message-ID<qD4bg-52J-11@gated-at.bofh.it>
In reply to#1281807
This patch set explores the idea shown in [1], which puts size of every
events at the end of them in ring buffer so user space tool like perf
can parse the ring buffer backward and find the oldest event in it.

In this version:

 - Kernel side, rename PERF_SAMPLE_SIZE to PERF_SAMPLE_SIZE_AT_END,
   output 8 bytes to meet the alignment requirement.

 - Perf side, provide a prototype utilize this feature. With this
   prototype users are allowed to capture the last events in an
   overwrite ring buffer using:

   # ./perf record -e dummy -e syscalls:*/overwrite/ ...

[1] http://lkml.kernel.org/g/1449063499-236703-1-git-send-email-wangnan0@huawei.com

Wang Nan (3):
  perf/core: Put size of a sample at the end of it
  perf tools: Enable overwrite settings
  perf record: Find tail pointer through size at end of event

 include/uapi/linux/perf_event.h |  3 +-
 kernel/events/core.c            |  9 +++++
 tools/perf/builtin-record.c     | 76 +++++++++++++++++++++++++++++++++++++++--
 tools/perf/perf.h               |  2 ++
 tools/perf/util/evlist.c        | 42 +++++++++++++++++------
 tools/perf/util/evsel.c         |  7 ++++
 tools/perf/util/evsel.h         |  3 ++
 tools/perf/util/parse-events.c  | 14 ++++++++
 tools/perf/util/parse-events.h  |  4 ++-
 tools/perf/util/parse-events.l  |  2 ++
 10 files changed, 147 insertions(+), 15 deletions(-)

-- 
1.8.3.4

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1285307 — [RFC PATCH v2 1/3] perf/core: Put size of a sample at the end of it

FromWang Nan <wangnan0@huawei.com>
Date2015-12-07 14:40 +0100
Subject[RFC PATCH v2 1/3] perf/core: Put size of a sample at the end of it
Message-ID<qD4bh-52J-25@gated-at.bofh.it>
In reply to#1285305
This is an RFC patch which is for overwrite mode ring buffer. I'd like
to discuss the correctness of this new idea for retriving as many
events as possible from overwrite mode ring buffer. If there's no
fundamental problem, I'll start perf side work.

The biggest problem for overwrite ring buffer is that it is hard to find
the start position of valid record. [1] and [2] tries to solve this
problem by introducing 'tail' and 'next_tail' into metadata page, and
update them each time the ring buffer is half full. Which adds more
instructions to event output code path, hurt performance. In addition,
even with them we still unable to recover all possible records. For
example:

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

If a record written at head pointer and it overwrites record A:

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

Record B is still valid but we can't get it through data_tail.

This patch suggests a different solution for this problem that, by
appending the length of a record at the end of it, user program is
possible to get every possible record in a backward manner, don't
need saving tail pointer.

For example:

   head
    |
    V
 +--+---+-------+----------+------+---+
 |E6|...|   B  8|   C    11|  D  7|E..|
 +--+---+-------+----------+------+---+

In this case, from the 'head' pointer provided by kernel, user program
can first see '6' by (*(head - sizeof(u16))), then it can get the start
pointer of record 'E', then it can read size and find start position
of record D, C, B in similar way.

Kernel side implementation is easy: simply adding a PERF_SAMPLE_SIZE_AT_END
for the size output.

This sloution requires user program (perf) do more things. At least
following things and limitations should be considered:

 1. Before reading such ring buffer, perf must ensure all events which
    may output to it is already stopped, so the 'head' pointer it get
    is the end of the last record.

Further improvement can be taken:

 1. We must ensure all events attached this ring buffer has
    'PERF_SAMPLE_SIZE_AT_END' selected.

 2. 8 bytes extra space is required for each record.

 3. We can find a way to append size information for tracking events.

[1] http://lkml.kernel.org/r/20130708121557.GA17211@twins.programming.kicks-ass.net
[2] http://lkml.kernel.org/r/20151023151205.GW11639@twins.programming.kicks-ass.net

Signed-off-by: Wang Nan <wangnan0@huawei.com>
Cc: Adrian Hunter <adrian.hunter@intel.com>
Cc: Arnaldo Carvalho de Melo <acme@redhat.com>
Cc: David Ahern <dsahern@gmail.com>
Cc: Ingo Molnar <mingo@kernel.org>
Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
Cc: Yunlong Song <yunlong.song@huawei.com>
---
 include/uapi/linux/perf_event.h | 3 ++-
 kernel/events/core.c            | 9 +++++++++
 2 files changed, 11 insertions(+), 1 deletion(-)

diff --git a/include/uapi/linux/perf_event.h b/include/uapi/linux/perf_event.h
index 1afe962..e79606c 100644
--- a/include/uapi/linux/perf_event.h
+++ b/include/uapi/linux/perf_event.h
@@ -139,8 +139,9 @@ enum perf_event_sample_format {
 	PERF_SAMPLE_IDENTIFIER			= 1U << 16,
 	PERF_SAMPLE_TRANSACTION			= 1U << 17,
 	PERF_SAMPLE_REGS_INTR			= 1U << 18,
+	PERF_SAMPLE_SIZE_AT_END			= 1U << 19,
 
-	PERF_SAMPLE_MAX = 1U << 19,		/* non-ABI */
+	PERF_SAMPLE_MAX = 1U << 20,		/* non-ABI */
 };
 
 /*
diff --git a/kernel/events/core.c b/kernel/events/core.c
index c3d61b9..cfe9336 100644
--- a/kernel/events/core.c
+++ b/kernel/events/core.c
@@ -5452,6 +5452,12 @@ void perf_output_sample(struct perf_output_handle *handle,
 		}
 	}
 
+	/* Should be the last one */
+	if (sample_type & PERF_SAMPLE_SIZE_AT_END) {
+		perf_output_skip(handle, sizeof(u64) - sizeof(header->size));
+		perf_output_put(handle, header->size);
+	}
+
 	if (!event->attr.watermark) {
 		int wakeup_events = event->attr.wakeup_events;
 
@@ -5571,6 +5577,9 @@ void perf_prepare_sample(struct perf_event_header *header,
 
 		header->size += size;
 	}
+
+	if (sample_type & PERF_SAMPLE_SIZE_AT_END)
+		header->size += sizeof(u64);
 }
 
 void perf_event_output(struct perf_event *event,
-- 
1.8.3.4

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1285308 — [RFC PATCH v2 3/3] perf record: Find tail pointer through size at end of event

FromWang Nan <wangnan0@huawei.com>
Date2015-12-07 14:40 +0100
Subject[RFC PATCH v2 3/3] perf record: Find tail pointer through size at end of event
Message-ID<qD4bh-52J-29@gated-at.bofh.it>
In reply to#1285305
This is an RFC patch which roughly shows the usage of
PERF_SAMPLE_SIZE_AT_END introduced by previous patches. In this
prototype we can use 'perf record' to capture data in overwritable
ringbuffer:

 # ./perf record -g --call-graph=dwarf,128 -m 1 -e dummy -e syscalls:*/overwrite/ dd if=/dev/zero of=/dev/null count=4096 bs=1
 4096+0 records in
 4096+0 records out
 4096 bytes (4.1 kB) copied, 8.54486 s, 0.5 kB/s
 [ perf record: Woken up 1 times to write data ]
 [ perf record: Captured and wrote 0.082 MB perf.data (9 samples) ]

 # ./perf script -F comm,event
              dd syscalls:sys_enter_write:
              dd syscalls:sys_exit_write:
              dd syscalls:sys_enter_write:
              dd syscalls:sys_exit_write:
              dd syscalls:sys_enter_write:
              dd syscalls:sys_exit_write:
              dd syscalls:sys_enter_close:
              dd syscalls:sys_exit_close:
              dd syscalls:sys_enter_exit_group:

 # ls -l ./perf.data
 -rw------- 1 root root 497438 Dec  7 12:48 ./perf.data

In this case perf uses 1 page for a overwritable ringbuffer. The result is
only the last 9 samples (see the sys_enter_exit_group) are captured.

 # ./perf record -g --call-graph=dwarf,128 -m 1 -e dummy -e syscalls:*/overwrite/ dd if=/dev/zero of=/dev/null count=8192 bs=1
 8192+0 records in
 8192+0 records out
 8192 bytes (8.2 kB) copied, 16.9867 s, 0.5 kB/s
 [ perf record: Woken up 1 times to write data ]
 [ perf record: Captured and wrote 0.082 MB perf.data (9 samples) ]
 # ls -l ./perf.data
 -rw------- 1 root root 497438 Dec  7 12:51 ./perf.data

Issuing more syscalls doesn't causes perf.data size increasing. Still 9
samples are captured.

record__search_read_start() is the core function I want to show in
this patch, which walks through the whole ring buffer backward, locates
the head of each events by the 'size' field at the tail of each event.
Finally it get the oldest event in the ring buffer, then start dump
there.

Other parts in this patch are dirty tricks and should be rewritten.

Limitation in this patch: there must have a '-e dummy' with no
overwrite set as the first event. Following command causes error:

 # ./perf record -e syscalls:*/overwrite/ ...

This is because we need this dummy event capture tracking events
like fork, mmap...

We'll go back to kernel to enforce the 'PERF_SAMPLE_SIZE_AT_END' flag
as mentioned in previous email:
 1. Doesn't allow mixed events attached at one ring buffer with some
    events has that flag but other events no;

 2. Append size to tracking events (no-sample events) so event with
    attr.tracking == 1 and PERF_SAMPLE_SIZE_AT_END set is acceptable.

From these perf size work I find following improvements should be done:
 1. Overwrite should not be a attribute of evlist, but an attribute of
    evsel;

 2. The principle of 'channel' should be introduced, so different
    events can output things to different places. Which would be useful
    for:

    1) separate overwrite mapping and normal mapping,
    2) allow outputting tracking events (through a dummy event) to an
       isolated mmap area so it would be possible for 'perf record' run
       as a daemon which periodically synthesizes those event without
       parsing samples,
    3) allow eBPF output event with no-buffer attribute set, so eBPF
       programs would be possible to control behavior of perf, for
       example, shut down events.

Signed-off-by: Wang Nan <wangnan0@huawei.com>
---
 tools/perf/builtin-record.c | 73 +++++++++++++++++++++++++++++++++++++++++++--
 tools/perf/util/evlist.c    | 42 +++++++++++++++++++-------
 tools/perf/util/evsel.c     |  3 ++
 3 files changed, 105 insertions(+), 13 deletions(-)

diff --git a/tools/perf/builtin-record.c b/tools/perf/builtin-record.c
index ec4135c..3817a9a 100644
--- a/tools/perf/builtin-record.c
+++ b/tools/perf/builtin-record.c
@@ -74,6 +74,54 @@ static int process_synthesized_event(struct perf_tool *tool,
 	return record__write(rec, event, event->header.size);
 }
 
+static int record__search_read_start(struct perf_mmap *md, u64 head, u64 *pstart)
+{
+	unsigned char *data = md->base + page_size;
+	u64 evt_head = head;
+	u16 *pevt_size;
+
+	while (true) {
+		struct perf_event_header *pheader;
+
+		pevt_size = (void *)&data[(evt_head - sizeof(*pevt_size)) & md->mask];
+		
+		if (*pevt_size % sizeof(u16) != 0) {
+			pr_err("strange event size: %d\n", (int)(*pevt_size));
+			return -1;
+		}
+
+		if (!*pevt_size) {
+			if (evt_head) {
+				pr_err("size is 0 but evt_head (0x%lx) not 0\n",
+					 (unsigned long)evt_head);
+				return -1;
+			}
+			break;
+		}
+
+		if (evt_head < *pevt_size)
+			break;
+
+		evt_head -= *pevt_size;
+		if (evt_head + (md->mask + 1) < head) {
+			evt_head += *pevt_size;
+			pr_debug("evt_head=%lx, head=%lx, size=%d\n", evt_head, head, *pevt_size);
+			break;
+		}
+
+		pheader = (struct perf_event_header *)(&data[evt_head & md->mask]);
+
+		if (pheader->size != *pevt_size) {
+			pr_err("size mismatch: %d vs %d\n",
+				 (int)pheader->size, (int)(*pevt_size));
+			return -1;
+		}
+	}
+
+	*pstart = evt_head;
+	return 0;
+}
+
 static int record__mmap_read(struct record *rec, int idx)
 {
 	struct perf_mmap *md = &rec->evlist->mmap[idx];
@@ -83,6 +131,17 @@ static int record__mmap_read(struct record *rec, int idx)
 	unsigned long size;
 	void *buf;
 	int rc = 0;
+	bool overwrite = rec->evlist->overwrite;
+	int err;
+
+	if (idx >= rec->evlist->nr_mmaps)
+		overwrite = !overwrite;
+
+	if (overwrite) {
+		err = record__search_read_start(md, head, &old);
+		if (err)
+			return 0;
+	}
 
 	if (old == head)
 		return 0;
@@ -400,7 +459,7 @@ static struct perf_event_header finished_round_event = {
 	.type = PERF_RECORD_FINISHED_ROUND,
 };
 
-static int record__mmap_read_all(struct record *rec)
+static int __record__mmap_read_all(struct record *rec, bool next_half)
 {
 	u64 bytes_written = rec->bytes_written;
 	int i;
@@ -408,9 +467,10 @@ static int record__mmap_read_all(struct record *rec)
 
 	for (i = 0; i < rec->evlist->nr_mmaps; i++) {
 		struct auxtrace_mmap *mm = &rec->evlist->mmap[i].auxtrace_mmap;
+		int idx = i + (next_half ? rec->evlist->nr_mmaps : 0);
 
-		if (rec->evlist->mmap[i].base) {
-			if (record__mmap_read(rec, i) != 0) {
+		if (rec->evlist->mmap[idx].base) {
+			if (record__mmap_read(rec, idx) != 0) {
 				rc = -1;
 				goto out;
 			}
@@ -434,6 +494,11 @@ out:
 	return rc;
 }
 
+static int record__mmap_read_all(struct record *rec)
+{
+	return __record__mmap_read_all(rec, false);
+}
+
 static void record__init_features(struct record *rec)
 {
 	struct perf_session *session = rec->session;
@@ -727,6 +792,8 @@ static int __cmd_record(struct record *rec, int argc, const char **argv)
 	}
 	auxtrace_snapshot_enabled = 0;
 
+	__record__mmap_read_all(rec, true);
+
 	if (forks && workload_exec_errno) {
 		char msg[STRERR_BUFSIZE];
 		const char *emsg = strerror_r(workload_exec_errno, msg, sizeof(msg));
diff --git a/tools/perf/util/evlist.c b/tools/perf/util/evlist.c
index 8dd59aa..a3421ee 100644
--- a/tools/perf/util/evlist.c
+++ b/tools/perf/util/evlist.c
@@ -800,7 +800,9 @@ void perf_evlist__mmap_consume(struct perf_evlist *evlist, int idx)
 {
 	struct perf_mmap *md = &evlist->mmap[idx];
 
-	if (!evlist->overwrite) {
+	if ((!evlist->overwrite && (idx < evlist->nr_mmaps)) ||
+		(evlist->overwrite && (idx >= evlist->nr_mmaps))) {
+
 		u64 old = md->prev;
 
 		perf_mmap__write_tail(md, old);
@@ -855,7 +857,7 @@ void perf_evlist__munmap(struct perf_evlist *evlist)
 	if (evlist->mmap == NULL)
 		return;
 
-	for (i = 0; i < evlist->nr_mmaps; i++)
+	for (i = 0; i < 2 * evlist->nr_mmaps; i++)
 		__perf_evlist__munmap(evlist, i);
 
 	zfree(&evlist->mmap);
@@ -866,7 +868,7 @@ static int perf_evlist__alloc_mmap(struct perf_evlist *evlist)
 	evlist->nr_mmaps = cpu_map__nr(evlist->cpus);
 	if (cpu_map__empty(evlist->cpus))
 		evlist->nr_mmaps = thread_map__nr(evlist->threads);
-	evlist->mmap = zalloc(evlist->nr_mmaps * sizeof(struct perf_mmap));
+	evlist->mmap = zalloc(2 * evlist->nr_mmaps * sizeof(struct perf_mmap));
 	return evlist->mmap != NULL ? 0 : -ENOMEM;
 }
 
@@ -897,6 +899,7 @@ static int __perf_evlist__mmap(struct perf_evlist *evlist, int idx,
 	evlist->mmap[idx].mask = mp->mask;
 	evlist->mmap[idx].base = mmap(NULL, evlist->mmap_len, mp->prot,
 				      MAP_SHARED, fd, 0);
+
 	if (evlist->mmap[idx].base == MAP_FAILED) {
 		pr_debug2("failed to mmap perf event ring buffer, error %d\n",
 			  errno);
@@ -911,18 +914,31 @@ static int __perf_evlist__mmap(struct perf_evlist *evlist, int idx,
 	return 0;
 }
 
-static int perf_evlist__mmap_per_evsel(struct perf_evlist *evlist, int idx,
-				       struct mmap_params *mp, int cpu,
-				       int thread, int *output)
+static int perf_evlist__mmap_per_evsel(struct perf_evlist *evlist, int _idx,
+				       struct mmap_params *_mp, int cpu,
+				       int thread, int *output1, int *output2)
 {
 	struct perf_evsel *evsel;
+	int *output;
+	struct mmap_params new_mp = *_mp;
 
 	evlist__for_each(evlist, evsel) {
 		int fd;
+		int idx = _idx;
+		struct mmap_params *mp = _mp;
 
 		if (evsel->system_wide && thread)
 			continue;
 
+		if (evsel->overwrite ^ evlist->overwrite) {
+			output = output2;
+			new_mp.prot ^= PROT_WRITE;
+			mp = &new_mp;
+			idx = _idx + evlist->nr_mmaps;
+		} else {
+			output = output1;
+		}
+
 		fd = FD(evsel, cpu, thread);
 
 		if (*output == -1) {
@@ -936,6 +952,8 @@ static int perf_evlist__mmap_per_evsel(struct perf_evlist *evlist, int idx,
 			perf_evlist__mmap_get(evlist, idx);
 		}
 
+		if (evsel->overwrite)
+			goto skip_poll_add;
 		/*
 		 * The system_wide flag causes a selected event to be opened
 		 * always without a pid.  Consequently it will never get a
@@ -949,6 +967,8 @@ static int perf_evlist__mmap_per_evsel(struct perf_evlist *evlist, int idx,
 			return -1;
 		}
 
+skip_poll_add:
+
 		if (evsel->attr.read_format & PERF_FORMAT_ID) {
 			if (perf_evlist__id_add_fd(evlist, evsel, cpu, thread,
 						   fd) < 0)
@@ -970,14 +990,15 @@ static int perf_evlist__mmap_per_cpu(struct perf_evlist *evlist,
 
 	pr_debug2("perf event ring buffer mmapped per cpu\n");
 	for (cpu = 0; cpu < nr_cpus; cpu++) {
-		int output = -1;
+		int output1 = -1;
+		int output2 = -1;
 
 		auxtrace_mmap_params__set_idx(&mp->auxtrace_mp, evlist, cpu,
 					      true);
 
 		for (thread = 0; thread < nr_threads; thread++) {
 			if (perf_evlist__mmap_per_evsel(evlist, cpu, mp, cpu,
-							thread, &output))
+							thread, &output1, &output2))
 				goto out_unmap;
 		}
 	}
@@ -998,13 +1019,14 @@ static int perf_evlist__mmap_per_thread(struct perf_evlist *evlist,
 
 	pr_debug2("perf event ring buffer mmapped per thread\n");
 	for (thread = 0; thread < nr_threads; thread++) {
-		int output = -1;
+		int output1 = -1;
+		int output2 = -1;
 
 		auxtrace_mmap_params__set_idx(&mp->auxtrace_mp, evlist, thread,
 					      false);
 
 		if (perf_evlist__mmap_per_evsel(evlist, thread, mp, 0, thread,
-						&output))
+						&output1, &output2))
 			goto out_unmap;
 	}
 
diff --git a/tools/perf/util/evsel.c b/tools/perf/util/evsel.c
index 3386437..8e40da9 100644
--- a/tools/perf/util/evsel.c
+++ b/tools/perf/util/evsel.c
@@ -910,6 +910,9 @@ void perf_evsel__config(struct perf_evsel *evsel, struct record_opts *opts)
 	 * it overloads any global configuration.
 	 */
 	apply_config_terms(evsel, opts);
+
+	if (evsel->overwrite)
+		perf_evsel__set_sample_bit(evsel, SIZE_AT_END);
 }
 
 static int perf_evsel__alloc_fd(struct perf_evsel *evsel, int ncpus, int nthreads)
-- 
1.8.3.4

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web