[2/2] perf record: Replace 'overwrite' by 'flightrecorder' for better naming

Message ID 20171101055327.141281-3-wangnan0@huawei.com
State New
Headers show
Series
  • perf record: Fix --overwrite and clarify concepts
Related show

Commit Message

Wang Nan Nov. 1, 2017, 5:53 a.m.
The meaning of perf record's "overwrite" option and many "overwrite" in
source code are not clear. In perf's code, the 'overwrite' has 2 meanings:
 1. Make ringbuffer readonly (perf_evlist__mmap_ex's argument).
 2. Set evsel's "backward" attribute (in apply_config_terms).

perf record doesn't use meaning 1 at all, but have a overwrite option, its
real meaning is setting backward.

This patch separates these two concepts, introduce 'flightrecorder' mode
which is what we really want. It combines these 2 concept together, wraps
them into a record mode. In flight recorder mode, perf only dumps data before
something happen.

Signed-off-by: Wang Nan <wangnan0@huawei.com>

---
 tools/perf/Documentation/perf-record.txt |  8 ++++----
 tools/perf/builtin-record.c              |  4 ++--
 tools/perf/perf.h                        |  2 +-
 tools/perf/util/evsel.c                  |  6 +++---
 tools/perf/util/evsel.h                  |  4 ++--
 tools/perf/util/parse-events.c           | 20 ++++++++++----------
 tools/perf/util/parse-events.h           |  4 ++--
 tools/perf/util/parse-events.l           |  4 ++--
 8 files changed, 26 insertions(+), 26 deletions(-)

-- 
2.10.1

Comments

Namhyung Kim Nov. 1, 2017, 10:03 a.m. | #1
On Wed, Nov 01, 2017 at 05:53:27AM +0000, Wang Nan wrote:
> The meaning of perf record's "overwrite" option and many "overwrite" in

> source code are not clear. In perf's code, the 'overwrite' has 2 meanings:

>  1. Make ringbuffer readonly (perf_evlist__mmap_ex's argument).

>  2. Set evsel's "backward" attribute (in apply_config_terms).

> 

> perf record doesn't use meaning 1 at all, but have a overwrite option, its

> real meaning is setting backward.

> 

> This patch separates these two concepts, introduce 'flightrecorder' mode

> which is what we really want. It combines these 2 concept together, wraps

> them into a record mode. In flight recorder mode, perf only dumps data before

> something happen.


I'm ok with the it but removing old name looks not good.  How about
keeping them for a while (as deprecated)?.

And 'flightrecorder' seems too long.  Maybe you can use an acronym
like FDR or fdr-mode?

Thanks,
Namhyung


> 

> Signed-off-by: Wang Nan <wangnan0@huawei.com>

> ---

>  tools/perf/Documentation/perf-record.txt |  8 ++++----

>  tools/perf/builtin-record.c              |  4 ++--

>  tools/perf/perf.h                        |  2 +-

>  tools/perf/util/evsel.c                  |  6 +++---

>  tools/perf/util/evsel.h                  |  4 ++--

>  tools/perf/util/parse-events.c           | 20 ++++++++++----------

>  tools/perf/util/parse-events.h           |  4 ++--

>  tools/perf/util/parse-events.l           |  4 ++--

>  8 files changed, 26 insertions(+), 26 deletions(-)

> 

> diff --git a/tools/perf/Documentation/perf-record.txt b/tools/perf/Documentation/perf-record.txt

> index 5a626ef..463c2d3 100644

> --- a/tools/perf/Documentation/perf-record.txt

> +++ b/tools/perf/Documentation/perf-record.txt

> @@ -467,19 +467,19 @@ the beginning of record, collect them during finalizing an output file.

>  The collected non-sample events reflects the status of the system when

>  record is finished.

>  

> ---overwrite::

> +--flight-recorder::

>  Makes all events use an overwritable ring buffer. An overwritable ring

>  buffer works like a flight recorder: when it gets full, the kernel will

>  overwrite the oldest records, that thus will never make it to the

>  perf.data file.

>  

> -When '--overwrite' and '--switch-output' are used perf records and drops

> +When '--flight-recorder' and '--switch-output' are used perf records and drops

>  events until it receives a signal, meaning that something unusual was

>  detected that warrants taking a snapshot of the most current events,

>  those fitting in the ring buffer at that moment.

>  

> -'overwrite' attribute can also be set or canceled for an event using

> -config terms. For example: 'cycles/overwrite/' and 'instructions/no-overwrite/'.

> +'flightrecorder' attribute can also be set or canceled separately for an event using

> +config terms. For example: 'cycles/flightrecorder/' and 'instructions/no-flightrecorder/'.

>  

>  Implies --tail-synthesize.

>  

> diff --git a/tools/perf/builtin-record.c b/tools/perf/builtin-record.c

> index f4d9fc5..315ea09 100644

> --- a/tools/perf/builtin-record.c

> +++ b/tools/perf/builtin-record.c

> @@ -1489,7 +1489,7 @@ static struct option __record_options[] = {

>  			"child tasks do not inherit counters"),

>  	OPT_BOOLEAN(0, "tail-synthesize", &record.opts.tail_synthesize,

>  		    "synthesize non-sample events at the end of output"),

> -	OPT_BOOLEAN(0, "overwrite", &record.opts.overwrite, "use overwrite mode"),

> +	OPT_BOOLEAN(0, "flight-recoder", &record.opts.flight_recorder, "use flight recoder 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",

> @@ -1733,7 +1733,7 @@ int cmd_record(int argc, const char **argv)

>  		}

>  	}

>  

> -	if (record.opts.overwrite)

> +	if (record.opts.flight_recorder)

>  		record.opts.tail_synthesize = true;

>  

>  	if (rec->evlist->nr_entries == 0 &&

> diff --git a/tools/perf/perf.h b/tools/perf/perf.h

> index fbb0a9c..a7f7618 100644

> --- a/tools/perf/perf.h

> +++ b/tools/perf/perf.h

> @@ -57,7 +57,7 @@ struct record_opts {

>  	bool	     all_kernel;

>  	bool	     all_user;

>  	bool	     tail_synthesize;

> -	bool	     overwrite;

> +	bool	     flight_recorder;

>  	bool	     ignore_missing_thread;

>  	unsigned int freq;

>  	unsigned int mmap_pages;

> diff --git a/tools/perf/util/evsel.c b/tools/perf/util/evsel.c

> index f894893..0e1e8e8 100644

> --- a/tools/perf/util/evsel.c

> +++ b/tools/perf/util/evsel.c

> @@ -772,8 +772,8 @@ static void apply_config_terms(struct perf_evsel *evsel,

>  			 */

>  			attr->inherit = term->val.inherit ? 1 : 0;

>  			break;

> -		case PERF_EVSEL__CONFIG_TERM_OVERWRITE:

> -			attr->write_backward = term->val.overwrite ? 1 : 0;

> +		case PERF_EVSEL__CONFIG_TERM_FLIGHTRECORDER:

> +			attr->write_backward = term->val.flightrecorder ? 1 : 0;

>  			break;

>  		default:

>  			break;

> @@ -856,7 +856,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;

> -	attr->write_backward = opts->overwrite ? 1 : 0;

> +	attr->write_backward = opts->flight_recorder ? 1 : 0;

>  

>  	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 64782b1..115b637 100644

> --- a/tools/perf/util/evsel.h

> +++ b/tools/perf/util/evsel.h

> @@ -45,7 +45,7 @@ enum {

>  	PERF_EVSEL__CONFIG_TERM_STACK_USER,

>  	PERF_EVSEL__CONFIG_TERM_INHERIT,

>  	PERF_EVSEL__CONFIG_TERM_MAX_STACK,

> -	PERF_EVSEL__CONFIG_TERM_OVERWRITE,

> +	PERF_EVSEL__CONFIG_TERM_FLIGHTRECORDER,

>  	PERF_EVSEL__CONFIG_TERM_DRV_CFG,

>  	PERF_EVSEL__CONFIG_TERM_BRANCH,

>  	PERF_EVSEL__CONFIG_TERM_MAX,

> @@ -63,7 +63,7 @@ struct perf_evsel_config_term {

>  		u64	stack_user;

>  		int	max_stack;

>  		bool	inherit;

> -		bool	overwrite;

> +		bool	flightrecorder;

>  		char	*branch;

>  	} val;

>  };

> diff --git a/tools/perf/util/parse-events.c b/tools/perf/util/parse-events.c

> index 04f35db..61ae3d3 100644

> --- a/tools/perf/util/parse-events.c

> +++ b/tools/perf/util/parse-events.c

> @@ -914,8 +914,8 @@ static const char *config_term_names[__PARSE_EVENTS__TERM_TYPE_NR] = {

>  	[PARSE_EVENTS__TERM_TYPE_NOINHERIT]		= "no-inherit",

>  	[PARSE_EVENTS__TERM_TYPE_INHERIT]		= "inherit",

>  	[PARSE_EVENTS__TERM_TYPE_MAX_STACK]		= "max-stack",

> -	[PARSE_EVENTS__TERM_TYPE_OVERWRITE]		= "overwrite",

> -	[PARSE_EVENTS__TERM_TYPE_NOOVERWRITE]		= "no-overwrite",

> +	[PARSE_EVENTS__TERM_TYPE_FLIGHTRECORDER]	= "flightrecorder",

> +	[PARSE_EVENTS__TERM_TYPE_NOFLIGHTRECORDER]	= "no-flightrecorder",

>  	[PARSE_EVENTS__TERM_TYPE_DRV_CFG]		= "driver-config",

>  };

>  

> @@ -1013,10 +1013,10 @@ do {									   \

>  	case PARSE_EVENTS__TERM_TYPE_NOINHERIT:

>  		CHECK_TYPE_VAL(NUM);

>  		break;

> -	case PARSE_EVENTS__TERM_TYPE_OVERWRITE:

> +	case PARSE_EVENTS__TERM_TYPE_FLIGHTRECORDER:

>  		CHECK_TYPE_VAL(NUM);

>  		break;

> -	case PARSE_EVENTS__TERM_TYPE_NOOVERWRITE:

> +	case PARSE_EVENTS__TERM_TYPE_NOFLIGHTRECORDER:

>  		CHECK_TYPE_VAL(NUM);

>  		break;

>  	case PARSE_EVENTS__TERM_TYPE_NAME:

> @@ -1072,8 +1072,8 @@ static int config_term_tracepoint(struct perf_event_attr *attr,

>  	case PARSE_EVENTS__TERM_TYPE_INHERIT:

>  	case PARSE_EVENTS__TERM_TYPE_NOINHERIT:

>  	case PARSE_EVENTS__TERM_TYPE_MAX_STACK:

> -	case PARSE_EVENTS__TERM_TYPE_OVERWRITE:

> -	case PARSE_EVENTS__TERM_TYPE_NOOVERWRITE:

> +	case PARSE_EVENTS__TERM_TYPE_FLIGHTRECORDER:

> +	case PARSE_EVENTS__TERM_TYPE_NOFLIGHTRECORDER:

>  		return config_term_common(attr, term, err);

>  	default:

>  		if (err) {

> @@ -1149,11 +1149,11 @@ do {								\

>  		case PARSE_EVENTS__TERM_TYPE_MAX_STACK:

>  			ADD_CONFIG_TERM(MAX_STACK, max_stack, term->val.num);

>  			break;

> -		case PARSE_EVENTS__TERM_TYPE_OVERWRITE:

> -			ADD_CONFIG_TERM(OVERWRITE, overwrite, term->val.num ? 1 : 0);

> +		case PARSE_EVENTS__TERM_TYPE_FLIGHTRECORDER:

> +			ADD_CONFIG_TERM(FLIGHTRECORDER, flightrecorder, term->val.num ? 1 : 0);

>  			break;

> -		case PARSE_EVENTS__TERM_TYPE_NOOVERWRITE:

> -			ADD_CONFIG_TERM(OVERWRITE, overwrite, term->val.num ? 0 : 1);

> +		case PARSE_EVENTS__TERM_TYPE_NOFLIGHTRECORDER:

> +			ADD_CONFIG_TERM(FLIGHTRECORDER, flightrecorder, term->val.num ? 0 : 1);

>  			break;

>  		case PARSE_EVENTS__TERM_TYPE_DRV_CFG:

>  			ADD_CONFIG_TERM(DRV_CFG, drv_cfg, term->val.str);

> diff --git a/tools/perf/util/parse-events.h b/tools/perf/util/parse-events.h

> index 3909ca0..cd0b4eae 100644

> --- a/tools/perf/util/parse-events.h

> +++ b/tools/perf/util/parse-events.h

> @@ -70,8 +70,8 @@ enum {

>  	PARSE_EVENTS__TERM_TYPE_NOINHERIT,

>  	PARSE_EVENTS__TERM_TYPE_INHERIT,

>  	PARSE_EVENTS__TERM_TYPE_MAX_STACK,

> -	PARSE_EVENTS__TERM_TYPE_NOOVERWRITE,

> -	PARSE_EVENTS__TERM_TYPE_OVERWRITE,

> +	PARSE_EVENTS__TERM_TYPE_NOFLIGHTRECORDER,

> +	PARSE_EVENTS__TERM_TYPE_FLIGHTRECORDER,

>  	PARSE_EVENTS__TERM_TYPE_DRV_CFG,

>  	__PARSE_EVENTS__TERM_TYPE_NR,

>  };

> diff --git a/tools/perf/util/parse-events.l b/tools/perf/util/parse-events.l

> index 38a42bd..7710d6d 100644

> --- a/tools/perf/util/parse-events.l

> +++ b/tools/perf/util/parse-events.l

> @@ -251,8 +251,8 @@ stack-size		{ return term(yyscanner, PARSE_EVENTS__TERM_TYPE_STACKSIZE); }

>  max-stack		{ return term(yyscanner, PARSE_EVENTS__TERM_TYPE_MAX_STACK); }

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

> +flightrecorder		{ return term(yyscanner, PARSE_EVENTS__TERM_TYPE_FLIGHTRECORDER); }

> +no-flightrecorder	{ return term(yyscanner, PARSE_EVENTS__TERM_TYPE_NOFLIGHTRECORDER); }

>  ,			{ return ','; }

>  "/"			{ BEGIN(INITIAL); return '/'; }

>  {name_minus}		{ return str(yyscanner, PE_NAME); }

> -- 

> 2.10.1

>
Wang Nan Nov. 1, 2017, 10:17 a.m. | #2
On 2017/11/1 18:03, Namhyung Kim wrote:
> On Wed, Nov 01, 2017 at 05:53:27AM +0000, Wang Nan wrote:

>> The meaning of perf record's "overwrite" option and many "overwrite" in

>> source code are not clear. In perf's code, the 'overwrite' has 2 meanings:

>>   1. Make ringbuffer readonly (perf_evlist__mmap_ex's argument).

>>   2. Set evsel's "backward" attribute (in apply_config_terms).

>>

>> perf record doesn't use meaning 1 at all, but have a overwrite option, its

>> real meaning is setting backward.

>>

>> This patch separates these two concepts, introduce 'flightrecorder' mode

>> which is what we really want. It combines these 2 concept together, wraps

>> them into a record mode. In flight recorder mode, perf only dumps data before

>> something happen.

> I'm ok with the it but removing old name looks not good.  How about

> keeping them for a while (as deprecated)?.


Is there a way to hide '--overwrite' from 'perf record --help' and print 
something
when user really use it?

> And 'flightrecorder' seems too long.  Maybe you can use an acronym

> like FDR or fdr-mode?


fdr-mode is a good name.

Thank you.
Namhyung Kim Nov. 1, 2017, 12:03 p.m. | #3
On Wed, Nov 01, 2017 at 06:17:26PM +0800, Wangnan (F) wrote:
> 

> 

> On 2017/11/1 18:03, Namhyung Kim wrote:

> > On Wed, Nov 01, 2017 at 05:53:27AM +0000, Wang Nan wrote:

> > > The meaning of perf record's "overwrite" option and many "overwrite" in

> > > source code are not clear. In perf's code, the 'overwrite' has 2 meanings:

> > >   1. Make ringbuffer readonly (perf_evlist__mmap_ex's argument).

> > >   2. Set evsel's "backward" attribute (in apply_config_terms).

> > > 

> > > perf record doesn't use meaning 1 at all, but have a overwrite option, its

> > > real meaning is setting backward.

> > > 

> > > This patch separates these two concepts, introduce 'flightrecorder' mode

> > > which is what we really want. It combines these 2 concept together, wraps

> > > them into a record mode. In flight recorder mode, perf only dumps data before

> > > something happen.

> > I'm ok with the it but removing old name looks not good.  How about

> > keeping them for a while (as deprecated)?.

> 

> Is there a way to hide '--overwrite' from 'perf record --help' and print

> something

> when user really use it?


You can set PARSE_OPT_HIDDEN flag to it.

Thanks,
Namhyung


> 

> > And 'flightrecorder' seems too long.  Maybe you can use an acronym

> > like FDR or fdr-mode?

> 

> fdr-mode is a good name.

> 

> Thank you.

>
Liang, Kan Nov. 1, 2017, 1:26 p.m. | #4
> The meaning of perf record's "overwrite" option and many "overwrite" in

> source code are not clear. In perf's code, the 'overwrite' has 2 meanings:

>  1. Make ringbuffer readonly (perf_evlist__mmap_ex's argument).

>  2. Set evsel's "backward" attribute (in apply_config_terms).

> 

> perf record doesn't use meaning 1 at all, but have a overwrite option, its

> real meaning is setting backward.

>


I don't understand here.
'overwrite' has 2 meanings. perf record only support 1.
It should be a bug, and need to be fixed.
Why do we need a new mode? 

I think a new mode just make the codes more complex.

Thanks,
Kan

> This patch separates these two concepts, introduce 'flightrecorder' mode

> which is what we really want. It combines these 2 concept together, wraps

> them into a record mode. In flight recorder mode, perf only dumps data

> before

> something happen.

> 

> Signed-off-by: Wang Nan <wangnan0@huawei.com>

> ---

>  tools/perf/Documentation/perf-record.txt |  8 ++++----

>  tools/perf/builtin-record.c              |  4 ++--

>  tools/perf/perf.h                        |  2 +-

>  tools/perf/util/evsel.c                  |  6 +++---

>  tools/perf/util/evsel.h                  |  4 ++--

>  tools/perf/util/parse-events.c           | 20 ++++++++++----------

>  tools/perf/util/parse-events.h           |  4 ++--

>  tools/perf/util/parse-events.l           |  4 ++--

>  8 files changed, 26 insertions(+), 26 deletions(-)

> 

> diff --git a/tools/perf/Documentation/perf-record.txt

> b/tools/perf/Documentation/perf-record.txt

> index 5a626ef..463c2d3 100644

> --- a/tools/perf/Documentation/perf-record.txt

> +++ b/tools/perf/Documentation/perf-record.txt

> @@ -467,19 +467,19 @@ the beginning of record, collect them during

> finalizing an output file.

>  The collected non-sample events reflects the status of the system when

>  record is finished.

> 

> ---overwrite::

> +--flight-recorder::

>  Makes all events use an overwritable ring buffer. An overwritable ring

>  buffer works like a flight recorder: when it gets full, the kernel will

>  overwrite the oldest records, that thus will never make it to the

>  perf.data file.

> 

> -When '--overwrite' and '--switch-output' are used perf records and drops

> +When '--flight-recorder' and '--switch-output' are used perf records and

> drops

>  events until it receives a signal, meaning that something unusual was

>  detected that warrants taking a snapshot of the most current events,

>  those fitting in the ring buffer at that moment.

> 

> -'overwrite' attribute can also be set or canceled for an event using

> -config terms. For example: 'cycles/overwrite/' and 'instructions/no-

> overwrite/'.

> +'flightrecorder' attribute can also be set or canceled separately for an event

> using

> +config terms. For example: 'cycles/flightrecorder/' and 'instructions/no-

> flightrecorder/'.

> 

>  Implies --tail-synthesize.

> 

> diff --git a/tools/perf/builtin-record.c b/tools/perf/builtin-record.c

> index f4d9fc5..315ea09 100644

> --- a/tools/perf/builtin-record.c

> +++ b/tools/perf/builtin-record.c

> @@ -1489,7 +1489,7 @@ static struct option __record_options[] = {

>  			"child tasks do not inherit counters"),

>  	OPT_BOOLEAN(0, "tail-synthesize", &record.opts.tail_synthesize,

>  		    "synthesize non-sample events at the end of output"),

> -	OPT_BOOLEAN(0, "overwrite", &record.opts.overwrite, "use

> overwrite mode"),

> +	OPT_BOOLEAN(0, "flight-recoder", &record.opts.flight_recorder,

> "use flight recoder 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",

> @@ -1733,7 +1733,7 @@ int cmd_record(int argc, const char **argv)

>  		}

>  	}

> 

> -	if (record.opts.overwrite)

> +	if (record.opts.flight_recorder)

>  		record.opts.tail_synthesize = true;

> 

>  	if (rec->evlist->nr_entries == 0 &&

> diff --git a/tools/perf/perf.h b/tools/perf/perf.h

> index fbb0a9c..a7f7618 100644

> --- a/tools/perf/perf.h

> +++ b/tools/perf/perf.h

> @@ -57,7 +57,7 @@ struct record_opts {

>  	bool	     all_kernel;

>  	bool	     all_user;

>  	bool	     tail_synthesize;

> -	bool	     overwrite;

> +	bool	     flight_recorder;

>  	bool	     ignore_missing_thread;

>  	unsigned int freq;

>  	unsigned int mmap_pages;

> diff --git a/tools/perf/util/evsel.c b/tools/perf/util/evsel.c

> index f894893..0e1e8e8 100644

> --- a/tools/perf/util/evsel.c

> +++ b/tools/perf/util/evsel.c

> @@ -772,8 +772,8 @@ static void apply_config_terms(struct perf_evsel

> *evsel,

>  			 */

>  			attr->inherit = term->val.inherit ? 1 : 0;

>  			break;

> -		case PERF_EVSEL__CONFIG_TERM_OVERWRITE:

> -			attr->write_backward = term->val.overwrite ? 1 : 0;

> +		case PERF_EVSEL__CONFIG_TERM_FLIGHTRECORDER:

> +			attr->write_backward = term->val.flightrecorder ? 1 :

> 0;

>  			break;

>  		default:

>  			break;

> @@ -856,7 +856,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;

> -	attr->write_backward = opts->overwrite ? 1 : 0;

> +	attr->write_backward = opts->flight_recorder ? 1 : 0;

> 

>  	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 64782b1..115b637 100644

> --- a/tools/perf/util/evsel.h

> +++ b/tools/perf/util/evsel.h

> @@ -45,7 +45,7 @@ enum {

>  	PERF_EVSEL__CONFIG_TERM_STACK_USER,

>  	PERF_EVSEL__CONFIG_TERM_INHERIT,

>  	PERF_EVSEL__CONFIG_TERM_MAX_STACK,

> -	PERF_EVSEL__CONFIG_TERM_OVERWRITE,

> +	PERF_EVSEL__CONFIG_TERM_FLIGHTRECORDER,

>  	PERF_EVSEL__CONFIG_TERM_DRV_CFG,

>  	PERF_EVSEL__CONFIG_TERM_BRANCH,

>  	PERF_EVSEL__CONFIG_TERM_MAX,

> @@ -63,7 +63,7 @@ struct perf_evsel_config_term {

>  		u64	stack_user;

>  		int	max_stack;

>  		bool	inherit;

> -		bool	overwrite;

> +		bool	flightrecorder;

>  		char	*branch;

>  	} val;

>  };

> diff --git a/tools/perf/util/parse-events.c b/tools/perf/util/parse-events.c

> index 04f35db..61ae3d3 100644

> --- a/tools/perf/util/parse-events.c

> +++ b/tools/perf/util/parse-events.c

> @@ -914,8 +914,8 @@ static const char

> *config_term_names[__PARSE_EVENTS__TERM_TYPE_NR] = {

>  	[PARSE_EVENTS__TERM_TYPE_NOINHERIT]		= "no-inherit",

>  	[PARSE_EVENTS__TERM_TYPE_INHERIT]		= "inherit",

>  	[PARSE_EVENTS__TERM_TYPE_MAX_STACK]		= "max-stack",

> -	[PARSE_EVENTS__TERM_TYPE_OVERWRITE]		= "overwrite",

> -	[PARSE_EVENTS__TERM_TYPE_NOOVERWRITE]		= "no-

> overwrite",

> +	[PARSE_EVENTS__TERM_TYPE_FLIGHTRECORDER]	=

> "flightrecorder",

> +	[PARSE_EVENTS__TERM_TYPE_NOFLIGHTRECORDER]	= "no-

> flightrecorder",

>  	[PARSE_EVENTS__TERM_TYPE_DRV_CFG]		= "driver-

> config",

>  };

> 

> @@ -1013,10 +1013,10 @@ do {

> 				   \

>  	case PARSE_EVENTS__TERM_TYPE_NOINHERIT:

>  		CHECK_TYPE_VAL(NUM);

>  		break;

> -	case PARSE_EVENTS__TERM_TYPE_OVERWRITE:

> +	case PARSE_EVENTS__TERM_TYPE_FLIGHTRECORDER:

>  		CHECK_TYPE_VAL(NUM);

>  		break;

> -	case PARSE_EVENTS__TERM_TYPE_NOOVERWRITE:

> +	case PARSE_EVENTS__TERM_TYPE_NOFLIGHTRECORDER:

>  		CHECK_TYPE_VAL(NUM);

>  		break;

>  	case PARSE_EVENTS__TERM_TYPE_NAME:

> @@ -1072,8 +1072,8 @@ static int config_term_tracepoint(struct

> perf_event_attr *attr,

>  	case PARSE_EVENTS__TERM_TYPE_INHERIT:

>  	case PARSE_EVENTS__TERM_TYPE_NOINHERIT:

>  	case PARSE_EVENTS__TERM_TYPE_MAX_STACK:

> -	case PARSE_EVENTS__TERM_TYPE_OVERWRITE:

> -	case PARSE_EVENTS__TERM_TYPE_NOOVERWRITE:

> +	case PARSE_EVENTS__TERM_TYPE_FLIGHTRECORDER:

> +	case PARSE_EVENTS__TERM_TYPE_NOFLIGHTRECORDER:

>  		return config_term_common(attr, term, err);

>  	default:

>  		if (err) {

> @@ -1149,11 +1149,11 @@ do {

> 			\

>  		case PARSE_EVENTS__TERM_TYPE_MAX_STACK:

>  			ADD_CONFIG_TERM(MAX_STACK, max_stack, term-

> >val.num);

>  			break;

> -		case PARSE_EVENTS__TERM_TYPE_OVERWRITE:

> -			ADD_CONFIG_TERM(OVERWRITE, overwrite, term-

> >val.num ? 1 : 0);

> +		case PARSE_EVENTS__TERM_TYPE_FLIGHTRECORDER:

> +			ADD_CONFIG_TERM(FLIGHTRECORDER,

> flightrecorder, term->val.num ? 1 : 0);

>  			break;

> -		case PARSE_EVENTS__TERM_TYPE_NOOVERWRITE:

> -			ADD_CONFIG_TERM(OVERWRITE, overwrite, term-

> >val.num ? 0 : 1);

> +		case PARSE_EVENTS__TERM_TYPE_NOFLIGHTRECORDER:

> +			ADD_CONFIG_TERM(FLIGHTRECORDER,

> flightrecorder, term->val.num ? 0 : 1);

>  			break;

>  		case PARSE_EVENTS__TERM_TYPE_DRV_CFG:

>  			ADD_CONFIG_TERM(DRV_CFG, drv_cfg, term-

> >val.str);

> diff --git a/tools/perf/util/parse-events.h b/tools/perf/util/parse-events.h

> index 3909ca0..cd0b4eae 100644

> --- a/tools/perf/util/parse-events.h

> +++ b/tools/perf/util/parse-events.h

> @@ -70,8 +70,8 @@ enum {

>  	PARSE_EVENTS__TERM_TYPE_NOINHERIT,

>  	PARSE_EVENTS__TERM_TYPE_INHERIT,

>  	PARSE_EVENTS__TERM_TYPE_MAX_STACK,

> -	PARSE_EVENTS__TERM_TYPE_NOOVERWRITE,

> -	PARSE_EVENTS__TERM_TYPE_OVERWRITE,

> +	PARSE_EVENTS__TERM_TYPE_NOFLIGHTRECORDER,

> +	PARSE_EVENTS__TERM_TYPE_FLIGHTRECORDER,

>  	PARSE_EVENTS__TERM_TYPE_DRV_CFG,

>  	__PARSE_EVENTS__TERM_TYPE_NR,

>  };

> diff --git a/tools/perf/util/parse-events.l b/tools/perf/util/parse-events.l

> index 38a42bd..7710d6d 100644

> --- a/tools/perf/util/parse-events.l

> +++ b/tools/perf/util/parse-events.l

> @@ -251,8 +251,8 @@ stack-size		{ return term(yyscanner,

> PARSE_EVENTS__TERM_TYPE_STACKSIZE); }

>  max-stack		{ return term(yyscanner,

> PARSE_EVENTS__TERM_TYPE_MAX_STACK); }

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

> +flightrecorder		{ return term(yyscanner,

> PARSE_EVENTS__TERM_TYPE_FLIGHTRECORDER); }

> +no-flightrecorder	{ return term(yyscanner,

> PARSE_EVENTS__TERM_TYPE_NOFLIGHTRECORDER); }

>  ,			{ return ','; }

>  "/"			{ BEGIN(INITIAL); return '/'; }

>  {name_minus}		{ return str(yyscanner, PE_NAME); }

> --

> 2.10.1
Wang Nan Nov. 1, 2017, 2:05 p.m. | #5
On 2017/11/1 21:26, Liang, Kan wrote:
>> The meaning of perf record's "overwrite" option and many "overwrite" in

>> source code are not clear. In perf's code, the 'overwrite' has 2 meanings:

>>   1. Make ringbuffer readonly (perf_evlist__mmap_ex's argument).

>>   2. Set evsel's "backward" attribute (in apply_config_terms).

>>

>> perf record doesn't use meaning 1 at all, but have a overwrite option, its

>> real meaning is setting backward.

>>

> I don't understand here.

> 'overwrite' has 2 meanings. perf record only support 1.

> It should be a bug, and need to be fixed.


Not a bug, but ambiguous.

Perf record doesn't need overwrite main channel (we have two channels:
evlist->mmap is main channel and evlist->backward_mmap is backward channel),
but some testcases require it, and your new patchset may require it.
'perf record --overwrite' doesn't set main channel overwrite. What it does
is moving all evsels to backward channel, and we can move some evsels back
to the main channel by /no-overwrite/ setting. This behavior is hard to
understand.

Thank you.
Liang, Kan Nov. 1, 2017, 2:22 p.m. | #6
> On 2017/11/1 21:26, Liang, Kan wrote:

> >> The meaning of perf record's "overwrite" option and many "overwrite"

> >> in source code are not clear. In perf's code, the 'overwrite' has 2 meanings:

> >>   1. Make ringbuffer readonly (perf_evlist__mmap_ex's argument).

> >>   2. Set evsel's "backward" attribute (in apply_config_terms).

> >>

> >> perf record doesn't use meaning 1 at all, but have a overwrite

> >> option, its real meaning is setting backward.

> >>

> > I don't understand here.

> > 'overwrite' has 2 meanings. perf record only support 1.

> > It should be a bug, and need to be fixed.

> 

> Not a bug, but ambiguous.

> 

> Perf record doesn't need overwrite main channel (we have two channels:

> evlist->mmap is main channel and evlist->backward_mmap is backward

> evlist->channel),

> but some testcases require it, and your new patchset may require it.

> 'perf record --overwrite' doesn't set main channel overwrite. What it does is

> moving all evsels to backward channel, and we can move some evsels back to

> the main channel by /no-overwrite/ setting. This behavior is hard to

> understand.

> 


As my understanding, the 'main channel' should depends on what user sets.
If --overwrite is applied, then evlist->backward_mmap should be the
'main channel'. evlist->overwrite should be set to true as well.

/no-overwrite/ setting is per-event setting.
Only when we finish the global setting, then the per-event setting will be
considered. You may refer to apply_config_terms.

Thanks,
Kan
Wang Nan Nov. 1, 2017, 2:44 p.m. | #7
On 2017/11/1 22:22, Liang, Kan wrote:
>> On 2017/11/1 21:26, Liang, Kan wrote:

>>>> The meaning of perf record's "overwrite" option and many "overwrite"

>>>> in source code are not clear. In perf's code, the 'overwrite' has 2 meanings:

>>>>    1. Make ringbuffer readonly (perf_evlist__mmap_ex's argument).

>>>>    2. Set evsel's "backward" attribute (in apply_config_terms).

>>>>

>>>> perf record doesn't use meaning 1 at all, but have a overwrite

>>>> option, its real meaning is setting backward.

>>>>

>>> I don't understand here.

>>> 'overwrite' has 2 meanings. perf record only support 1.

>>> It should be a bug, and need to be fixed.

>> Not a bug, but ambiguous.

>>

>> Perf record doesn't need overwrite main channel (we have two channels:

>> evlist->mmap is main channel and evlist->backward_mmap is backward

>> evlist->channel),

>> but some testcases require it, and your new patchset may require it.

>> 'perf record --overwrite' doesn't set main channel overwrite. What it does is

>> moving all evsels to backward channel, and we can move some evsels back to

>> the main channel by /no-overwrite/ setting. This behavior is hard to

>> understand.

>>

> As my understanding, the 'main channel' should depends on what user sets.

> If --overwrite is applied, then evlist->backward_mmap should be the

> 'main channel'. evlist->overwrite should be set to true as well.


Then it introduces a main channel switching mechanism, and we need
checking evlist->overwrite and another factor to determine which
one is the main channel. Make things more complex.

> /no-overwrite/ setting is per-event setting.

> Only when we finish the global setting, then the per-event setting will be

> considered. You may refer to apply_config_terms.


Yes.

> Thanks,

> Kan

>

>

>

>
Liang, Kan Nov. 1, 2017, 3:04 p.m. | #8
> On 2017/11/1 22:22, Liang, Kan wrote:

> >> On 2017/11/1 21:26, Liang, Kan wrote:

> >>>> The meaning of perf record's "overwrite" option and many "overwrite"

> >>>> in source code are not clear. In perf's code, the 'overwrite' has 2

> meanings:

> >>>>    1. Make ringbuffer readonly (perf_evlist__mmap_ex's argument).

> >>>>    2. Set evsel's "backward" attribute (in apply_config_terms).

> >>>>

> >>>> perf record doesn't use meaning 1 at all, but have a overwrite

> >>>> option, its real meaning is setting backward.

> >>>>

> >>> I don't understand here.

> >>> 'overwrite' has 2 meanings. perf record only support 1.

> >>> It should be a bug, and need to be fixed.

> >> Not a bug, but ambiguous.

> >>

> >> Perf record doesn't need overwrite main channel (we have two channels:

> >> evlist->mmap is main channel and evlist->backward_mmap is backward

> >> evlist->channel),

> >> but some testcases require it, and your new patchset may require it.

> >> 'perf record --overwrite' doesn't set main channel overwrite. What it does

> is

> >> moving all evsels to backward channel, and we can move some evsels

> back to

> >> the main channel by /no-overwrite/ setting. This behavior is hard to

> >> understand.

> >>

> > As my understanding, the 'main channel' should depends on what user sets.

> > If --overwrite is applied, then evlist->backward_mmap should be the

> > 'main channel'. evlist->overwrite should be set to true as well.

> 

> Then it introduces a main channel switching mechanism, and we need

> checking evlist->overwrite and another factor to determine which

> one is the main channel. Make things more complex.


We should check the evlist->overwrite.
Now, all perf tools force evlist->overwrite = false. I think it doesn’t make sense.

What is another factor?

I don't think it will be too complex.

In perf_evlist__mmap_ex, we just need to add a check.
If (!overwrite)
	evlist->mmap = perf_evlist__alloc_mmap(evlist);
else
	evlist->backward_mmap = perf_evlist__alloc_mmap(evlist);

In perf_evlist__mmap_per_evsel, we already handle per-event overwrite.
It just need to add some similar codes to handler per-event nonoverwrite.  

For other codes, they should already check evlist->mmap and evlist->backward_mmap.
So they probably don't need to change.


Thanks,
Kan
Wang Nan Nov. 1, 2017, 4 p.m. | #9
On 2017/11/1 23:04, Liang, Kan wrote:
>> On 2017/11/1 22:22, Liang, Kan wrote:

>>>> On 2017/11/1 21:26, Liang, Kan wrote:

>>>>>> The meaning of perf record's "overwrite" option and many "overwrite"

>>>>>> in source code are not clear. In perf's code, the 'overwrite' has 2

>> meanings:

>>>>>>     1. Make ringbuffer readonly (perf_evlist__mmap_ex's argument).

>>>>>>     2. Set evsel's "backward" attribute (in apply_config_terms).

>>>>>>

>>>>>> perf record doesn't use meaning 1 at all, but have a overwrite

>>>>>> option, its real meaning is setting backward.

>>>>>>

>>>>> I don't understand here.

>>>>> 'overwrite' has 2 meanings. perf record only support 1.

>>>>> It should be a bug, and need to be fixed.

>>>> Not a bug, but ambiguous.

>>>>

>>>> Perf record doesn't need overwrite main channel (we have two channels:

>>>> evlist->mmap is main channel and evlist->backward_mmap is backward

>>>> evlist->channel),

>>>> but some testcases require it, and your new patchset may require it.

>>>> 'perf record --overwrite' doesn't set main channel overwrite. What it does

>> is

>>>> moving all evsels to backward channel, and we can move some evsels

>> back to

>>>> the main channel by /no-overwrite/ setting. This behavior is hard to

>>>> understand.

>>>>

>>> As my understanding, the 'main channel' should depends on what user sets.

>>> If --overwrite is applied, then evlist->backward_mmap should be the

>>> 'main channel'. evlist->overwrite should be set to true as well.

>> Then it introduces a main channel switching mechanism, and we need

>> checking evlist->overwrite and another factor to determine which

>> one is the main channel. Make things more complex.

> We should check the evlist->overwrite.

> Now, all perf tools force evlist->overwrite = false. I think it doesn’t make sense.

>

> What is another factor?


If we support mixed channel as well as forward overwrite ring buffer,
evlist->overwrite is not enough.

> I don't think it will be too complex.

>

> In perf_evlist__mmap_ex, we just need to add a check.

> If (!overwrite)

> 	evlist->mmap = perf_evlist__alloc_mmap(evlist);

> else

> 	evlist->backward_mmap = perf_evlist__alloc_mmap(evlist);

>

> In perf_evlist__mmap_per_evsel, we already handle per-event overwrite.

> It just need to add some similar codes to handler per-event nonoverwrite.


Then the logic becomes:

  if (check write_backward) {
     maps = evlist->backward_mmap;
     if (!maps) {
       maps = perf_evlist__alloc_mmap(...);
       if (!maps) {
           /* error processing */
       }
       evlist->backward_mmap = maps;
       if (evlist->bkw_mmap_state == BKW_MMAP_NOTREADY)
         perf_evlist__toggle_bkw_mmap(evlist, BKW_MMAP_RUNNING);
     }
  } else {
     maps = evlist->mmap;
     if (!maps) {
       maps = perf_evlist__alloc_mmap(...);
       if (!maps) {
           /* error processing */
       }
       evlist->mmap = maps;
       ....
     }
  }

> For other codes, they should already check evlist->mmap and evlist->backward_mmap.

> So they probably don't need to change.

>

>

> Thanks,

> Kan

>

>
Liang, Kan Nov. 1, 2017, 4:13 p.m. | #10
> On 2017/11/1 23:04, Liang, Kan wrote:

> >> On 2017/11/1 22:22, Liang, Kan wrote:

> >>>> On 2017/11/1 21:26, Liang, Kan wrote:

> >>>>>> The meaning of perf record's "overwrite" option and many

> "overwrite"

> >>>>>> in source code are not clear. In perf's code, the 'overwrite' has

> >>>>>> 2

> >> meanings:

> >>>>>>     1. Make ringbuffer readonly (perf_evlist__mmap_ex's argument).

> >>>>>>     2. Set evsel's "backward" attribute (in apply_config_terms).

> >>>>>>

> >>>>>> perf record doesn't use meaning 1 at all, but have a overwrite

> >>>>>> option, its real meaning is setting backward.

> >>>>>>

> >>>>> I don't understand here.

> >>>>> 'overwrite' has 2 meanings. perf record only support 1.

> >>>>> It should be a bug, and need to be fixed.

> >>>> Not a bug, but ambiguous.

> >>>>

> >>>> Perf record doesn't need overwrite main channel (we have two

> channels:

> >>>> evlist->mmap is main channel and evlist->backward_mmap is backward

> >>>> evlist->channel),

> >>>> but some testcases require it, and your new patchset may require it.

> >>>> 'perf record --overwrite' doesn't set main channel overwrite. What

> >>>> it does

> >> is

> >>>> moving all evsels to backward channel, and we can move some evsels

> >> back to

> >>>> the main channel by /no-overwrite/ setting. This behavior is hard

> >>>> to understand.

> >>>>

> >>> As my understanding, the 'main channel' should depends on what user

> sets.

> >>> If --overwrite is applied, then evlist->backward_mmap should be the

> >>> 'main channel'. evlist->overwrite should be set to true as well.

> >> Then it introduces a main channel switching mechanism, and we need

> >> checking evlist->overwrite and another factor to determine which one

> >> is the main channel. Make things more complex.

> > We should check the evlist->overwrite.

> > Now, all perf tools force evlist->overwrite = false. I think it doesn’t make

> sense.

> >

> > What is another factor?

> 

> If we support mixed channel as well as forward overwrite ring buffer,

> evlist->overwrite is not enough.


I think you have a detailed analysis regarding to the weakness of 'forward overwrite'.
commit ID 9ecda41acb97 ("perf/core: Add ::write_backward attribute to perf event").

There is no perf tools use 'forward overwrite' mode.

The only users are three perf test cases. We can change them to 'backward overwrite'

I think it's OK to discard the 'forward overwrite' mode

> 

> > I don't think it will be too complex.

> >

> > In perf_evlist__mmap_ex, we just need to add a check.

> > If (!overwrite)

> > 	evlist->mmap = perf_evlist__alloc_mmap(evlist); else

> > 	evlist->backward_mmap = perf_evlist__alloc_mmap(evlist);

> >

> > In perf_evlist__mmap_per_evsel, we already handle per-event overwrite.

> > It just need to add some similar codes to handler per-event nonoverwrite.

> 

> Then the logic becomes:

> 

>   if (check write_backward) {

>      maps = evlist->backward_mmap;

>      if (!maps) {

>        maps = perf_evlist__alloc_mmap(...);

>        if (!maps) {

>            /* error processing */

>        }

>        evlist->backward_mmap = maps;

>        if (evlist->bkw_mmap_state == BKW_MMAP_NOTREADY)

>          perf_evlist__toggle_bkw_mmap(evlist, BKW_MMAP_RUNNING);

>      }

>   } else {

>      maps = evlist->mmap;

>      if (!maps) {

>        maps = perf_evlist__alloc_mmap(...);

>        if (!maps) {

>            /* error processing */

>        }

>        evlist->mmap = maps;

>        ....

>      }

>   }

> 


Thanks.
It looks good to me.

Kan

> > For other codes, they should already check evlist->mmap and evlist-

> >backward_mmap.

> > So they probably don't need to change.

> >

> >

> > Thanks,

> > Kan

> >

> >

>

Patch

diff --git a/tools/perf/Documentation/perf-record.txt b/tools/perf/Documentation/perf-record.txt
index 5a626ef..463c2d3 100644
--- a/tools/perf/Documentation/perf-record.txt
+++ b/tools/perf/Documentation/perf-record.txt
@@ -467,19 +467,19 @@  the beginning of record, collect them during finalizing an output file.
 The collected non-sample events reflects the status of the system when
 record is finished.
 
---overwrite::
+--flight-recorder::
 Makes all events use an overwritable ring buffer. An overwritable ring
 buffer works like a flight recorder: when it gets full, the kernel will
 overwrite the oldest records, that thus will never make it to the
 perf.data file.
 
-When '--overwrite' and '--switch-output' are used perf records and drops
+When '--flight-recorder' and '--switch-output' are used perf records and drops
 events until it receives a signal, meaning that something unusual was
 detected that warrants taking a snapshot of the most current events,
 those fitting in the ring buffer at that moment.
 
-'overwrite' attribute can also be set or canceled for an event using
-config terms. For example: 'cycles/overwrite/' and 'instructions/no-overwrite/'.
+'flightrecorder' attribute can also be set or canceled separately for an event using
+config terms. For example: 'cycles/flightrecorder/' and 'instructions/no-flightrecorder/'.
 
 Implies --tail-synthesize.
 
diff --git a/tools/perf/builtin-record.c b/tools/perf/builtin-record.c
index f4d9fc5..315ea09 100644
--- a/tools/perf/builtin-record.c
+++ b/tools/perf/builtin-record.c
@@ -1489,7 +1489,7 @@  static struct option __record_options[] = {
 			"child tasks do not inherit counters"),
 	OPT_BOOLEAN(0, "tail-synthesize", &record.opts.tail_synthesize,
 		    "synthesize non-sample events at the end of output"),
-	OPT_BOOLEAN(0, "overwrite", &record.opts.overwrite, "use overwrite mode"),
+	OPT_BOOLEAN(0, "flight-recoder", &record.opts.flight_recorder, "use flight recoder 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",
@@ -1733,7 +1733,7 @@  int cmd_record(int argc, const char **argv)
 		}
 	}
 
-	if (record.opts.overwrite)
+	if (record.opts.flight_recorder)
 		record.opts.tail_synthesize = true;
 
 	if (rec->evlist->nr_entries == 0 &&
diff --git a/tools/perf/perf.h b/tools/perf/perf.h
index fbb0a9c..a7f7618 100644
--- a/tools/perf/perf.h
+++ b/tools/perf/perf.h
@@ -57,7 +57,7 @@  struct record_opts {
 	bool	     all_kernel;
 	bool	     all_user;
 	bool	     tail_synthesize;
-	bool	     overwrite;
+	bool	     flight_recorder;
 	bool	     ignore_missing_thread;
 	unsigned int freq;
 	unsigned int mmap_pages;
diff --git a/tools/perf/util/evsel.c b/tools/perf/util/evsel.c
index f894893..0e1e8e8 100644
--- a/tools/perf/util/evsel.c
+++ b/tools/perf/util/evsel.c
@@ -772,8 +772,8 @@  static void apply_config_terms(struct perf_evsel *evsel,
 			 */
 			attr->inherit = term->val.inherit ? 1 : 0;
 			break;
-		case PERF_EVSEL__CONFIG_TERM_OVERWRITE:
-			attr->write_backward = term->val.overwrite ? 1 : 0;
+		case PERF_EVSEL__CONFIG_TERM_FLIGHTRECORDER:
+			attr->write_backward = term->val.flightrecorder ? 1 : 0;
 			break;
 		default:
 			break;
@@ -856,7 +856,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;
-	attr->write_backward = opts->overwrite ? 1 : 0;
+	attr->write_backward = opts->flight_recorder ? 1 : 0;
 
 	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 64782b1..115b637 100644
--- a/tools/perf/util/evsel.h
+++ b/tools/perf/util/evsel.h
@@ -45,7 +45,7 @@  enum {
 	PERF_EVSEL__CONFIG_TERM_STACK_USER,
 	PERF_EVSEL__CONFIG_TERM_INHERIT,
 	PERF_EVSEL__CONFIG_TERM_MAX_STACK,
-	PERF_EVSEL__CONFIG_TERM_OVERWRITE,
+	PERF_EVSEL__CONFIG_TERM_FLIGHTRECORDER,
 	PERF_EVSEL__CONFIG_TERM_DRV_CFG,
 	PERF_EVSEL__CONFIG_TERM_BRANCH,
 	PERF_EVSEL__CONFIG_TERM_MAX,
@@ -63,7 +63,7 @@  struct perf_evsel_config_term {
 		u64	stack_user;
 		int	max_stack;
 		bool	inherit;
-		bool	overwrite;
+		bool	flightrecorder;
 		char	*branch;
 	} val;
 };
diff --git a/tools/perf/util/parse-events.c b/tools/perf/util/parse-events.c
index 04f35db..61ae3d3 100644
--- a/tools/perf/util/parse-events.c
+++ b/tools/perf/util/parse-events.c
@@ -914,8 +914,8 @@  static const char *config_term_names[__PARSE_EVENTS__TERM_TYPE_NR] = {
 	[PARSE_EVENTS__TERM_TYPE_NOINHERIT]		= "no-inherit",
 	[PARSE_EVENTS__TERM_TYPE_INHERIT]		= "inherit",
 	[PARSE_EVENTS__TERM_TYPE_MAX_STACK]		= "max-stack",
-	[PARSE_EVENTS__TERM_TYPE_OVERWRITE]		= "overwrite",
-	[PARSE_EVENTS__TERM_TYPE_NOOVERWRITE]		= "no-overwrite",
+	[PARSE_EVENTS__TERM_TYPE_FLIGHTRECORDER]	= "flightrecorder",
+	[PARSE_EVENTS__TERM_TYPE_NOFLIGHTRECORDER]	= "no-flightrecorder",
 	[PARSE_EVENTS__TERM_TYPE_DRV_CFG]		= "driver-config",
 };
 
@@ -1013,10 +1013,10 @@  do {									   \
 	case PARSE_EVENTS__TERM_TYPE_NOINHERIT:
 		CHECK_TYPE_VAL(NUM);
 		break;
-	case PARSE_EVENTS__TERM_TYPE_OVERWRITE:
+	case PARSE_EVENTS__TERM_TYPE_FLIGHTRECORDER:
 		CHECK_TYPE_VAL(NUM);
 		break;
-	case PARSE_EVENTS__TERM_TYPE_NOOVERWRITE:
+	case PARSE_EVENTS__TERM_TYPE_NOFLIGHTRECORDER:
 		CHECK_TYPE_VAL(NUM);
 		break;
 	case PARSE_EVENTS__TERM_TYPE_NAME:
@@ -1072,8 +1072,8 @@  static int config_term_tracepoint(struct perf_event_attr *attr,
 	case PARSE_EVENTS__TERM_TYPE_INHERIT:
 	case PARSE_EVENTS__TERM_TYPE_NOINHERIT:
 	case PARSE_EVENTS__TERM_TYPE_MAX_STACK:
-	case PARSE_EVENTS__TERM_TYPE_OVERWRITE:
-	case PARSE_EVENTS__TERM_TYPE_NOOVERWRITE:
+	case PARSE_EVENTS__TERM_TYPE_FLIGHTRECORDER:
+	case PARSE_EVENTS__TERM_TYPE_NOFLIGHTRECORDER:
 		return config_term_common(attr, term, err);
 	default:
 		if (err) {
@@ -1149,11 +1149,11 @@  do {								\
 		case PARSE_EVENTS__TERM_TYPE_MAX_STACK:
 			ADD_CONFIG_TERM(MAX_STACK, max_stack, term->val.num);
 			break;
-		case PARSE_EVENTS__TERM_TYPE_OVERWRITE:
-			ADD_CONFIG_TERM(OVERWRITE, overwrite, term->val.num ? 1 : 0);
+		case PARSE_EVENTS__TERM_TYPE_FLIGHTRECORDER:
+			ADD_CONFIG_TERM(FLIGHTRECORDER, flightrecorder, term->val.num ? 1 : 0);
 			break;
-		case PARSE_EVENTS__TERM_TYPE_NOOVERWRITE:
-			ADD_CONFIG_TERM(OVERWRITE, overwrite, term->val.num ? 0 : 1);
+		case PARSE_EVENTS__TERM_TYPE_NOFLIGHTRECORDER:
+			ADD_CONFIG_TERM(FLIGHTRECORDER, flightrecorder, term->val.num ? 0 : 1);
 			break;
 		case PARSE_EVENTS__TERM_TYPE_DRV_CFG:
 			ADD_CONFIG_TERM(DRV_CFG, drv_cfg, term->val.str);
diff --git a/tools/perf/util/parse-events.h b/tools/perf/util/parse-events.h
index 3909ca0..cd0b4eae 100644
--- a/tools/perf/util/parse-events.h
+++ b/tools/perf/util/parse-events.h
@@ -70,8 +70,8 @@  enum {
 	PARSE_EVENTS__TERM_TYPE_NOINHERIT,
 	PARSE_EVENTS__TERM_TYPE_INHERIT,
 	PARSE_EVENTS__TERM_TYPE_MAX_STACK,
-	PARSE_EVENTS__TERM_TYPE_NOOVERWRITE,
-	PARSE_EVENTS__TERM_TYPE_OVERWRITE,
+	PARSE_EVENTS__TERM_TYPE_NOFLIGHTRECORDER,
+	PARSE_EVENTS__TERM_TYPE_FLIGHTRECORDER,
 	PARSE_EVENTS__TERM_TYPE_DRV_CFG,
 	__PARSE_EVENTS__TERM_TYPE_NR,
 };
diff --git a/tools/perf/util/parse-events.l b/tools/perf/util/parse-events.l
index 38a42bd..7710d6d 100644
--- a/tools/perf/util/parse-events.l
+++ b/tools/perf/util/parse-events.l
@@ -251,8 +251,8 @@  stack-size		{ return term(yyscanner, PARSE_EVENTS__TERM_TYPE_STACKSIZE); }
 max-stack		{ return term(yyscanner, PARSE_EVENTS__TERM_TYPE_MAX_STACK); }
 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); }
+flightrecorder		{ return term(yyscanner, PARSE_EVENTS__TERM_TYPE_FLIGHTRECORDER); }
+no-flightrecorder	{ return term(yyscanner, PARSE_EVENTS__TERM_TYPE_NOFLIGHTRECORDER); }
 ,			{ return ','; }
 "/"			{ BEGIN(INITIAL); return '/'; }
 {name_minus}		{ return str(yyscanner, PE_NAME); }