Message-ID: <20151103154046.GQ21609@kernel.org>
Date:	Tue, 3 Nov 2015 12:40:46 -0300
From:	Arnaldo Carvalho de Melo <acme@...nel.org>
To:	Taeung Song <treeze.taeung@...il.com>
Cc:	linux-kernel@...r.kernel.org, jolsa@...hat.com,
	namhyung@...nel.org, Ingo Molnar <mingo@...hat.com>
Subject: Re: [PATCH v9 1/9] perf tools: Add 'perf-config' command

Em Tue, Nov 03, 2015 at 10:50:12AM +0900, Taeung Song escreveu:
> The perf configuration file contains many variables which can make
> the perf command's action more effective.
> But looking through state of configuration is difficult and there's no knowing
> what kind of other variables except variables in perfconfig.example exist.
> So This patch adds 'perf-config' command with '--list' option and a document for it.
> 
>     perf config [options]
> 
>     display current perf config variables.
>     # perf config
>     or
>     # perf config -l | --list


I made a number of suggestions below, but the text is long, and this is
taking a long time to get processed, so I suggest we either find someone
to do a good proofreading of this, or you could send first a really
basic perf-config.txt file and then go on adding patches for each
section, so that we could make progress faster in processing these
patches.

Your work is appreciated, but we need to do proper proof reading.

Thanks,

- Arnaldo
 
> Signed-off-by: Taeung Song <treeze.taeung@...il.com>
> ---
>  tools/perf/Build                         |   1 +
>  tools/perf/Documentation/perf-config.txt | 396 +++++++++++++++++++++++++++++++
>  tools/perf/builtin-config.c              |  61 +++++
>  tools/perf/builtin.h                     |   1 +
>  tools/perf/command-list.txt              |   1 +
>  tools/perf/perf.c                        |   1 +
>  6 files changed, 461 insertions(+)
>  create mode 100644 tools/perf/Documentation/perf-config.txt
>  create mode 100644 tools/perf/builtin-config.c
> 
> diff --git a/tools/perf/Build b/tools/perf/Build
> index 7223745..2c7aaf2 100644
> --- a/tools/perf/Build
> +++ b/tools/perf/Build
> @@ -1,5 +1,6 @@
>  perf-y += builtin-bench.o
>  perf-y += builtin-annotate.o
> +perf-y += builtin-config.o
>  perf-y += builtin-diff.o
>  perf-y += builtin-evlist.o
>  perf-y += builtin-help.o
> diff --git a/tools/perf/Documentation/perf-config.txt b/tools/perf/Documentation/perf-config.txt
> new file mode 100644
> index 0000000..a590319
> --- /dev/null
> +++ b/tools/perf/Documentation/perf-config.txt
> @@ -0,0 +1,396 @@
> +perf-config(1)
> +==============
> +
> +NAME
> +----
> +perf-config - Get and set variables in a configuration file.
> +
> +SYNOPSIS
> +--------
> +[verse]
> +'perf config' -l | --list
> +
> +DESCRIPTION
> +-----------
> +You can manage variables in a configuration file with this command.
> +
> +OPTIONS
> +-------
> +
> +-l::
> +--list::
> +	Show current config variables, name and value, for all sections.
> +
> +CONFIGURATION FILE
> +------------------

> +The Perf configuration file contains many variables which can make
> +the perf command's action more effective.

I suggest replacing above's paragraph with:

---------------
The perf configuration file contains many variables to change various
aspects of each of its tools, including output, disk usage, etc.
---------------

> +The '$HOME/.perfconfig' file is used to store a per-user configuration.

> +The file '$(sysconfdir)/perfconfig' can be used to
> +store a system-wide default configuration.

The next paragraph seems redundant, since you restate what it says in
the first paragraph under "Syntax", right after it.

> +The variables are divided into sections. In each section, the variables
> +that are composed of a name and value.

> +Syntax
> +~~~~~~
> +
> +The file consist of sections. A section starts with its name
> +surrounded by square brackets and continues till the next section
> +begins.

Break the paragraph here

> + Each variable belong to a section, which means that
> +there must be a section header before the first variable, as below:
> +Each variable are in the form 'name = value'.

Please consider replacing the above paragraph with:

---
Each variable must be in a section, and have the form 'name = value',
for example:
---

> +
> +	[section]
> +		name1 = value1
> +		name2 = value2
> +
> +Section names are case sensitive and can contain any characters except
> +newline (double quote `"` and backslash have to be escaped as `\"` and `\\`,
> +respectively). Section headers can't span multiple lines.
> +
> +Example
> +~~~~~~~
> +
> +Given a $HOME/.perfconfig like this:
> +
> +#
> +# This is the config file, and
> +# a '#' and ';' character indicates a comment
> +#
> +
> +[colors]
> +	# Color variables
> +	top = red, default
> +	medium = green, default
> +	normal = lightgray, default
> +	selected = white, lightgray
> +	code = blue, default
> +	addr = magenta, default
> +	root = white, blue
> +
> +[tui]
> +	# Defaults if linked with libslang
> +	report = on
> +	annotate = on
> +	top = on
> +
> +[buildid]
> +	# Default, disable using /dev/null
> +	dir = ~/.debug
> +
> +[annotate]
> +	# Defaults
> +	hide_src_code = false
> +	use_offset = true
> +	jump_arrows = true
> +	show_nr_jumps = false
> +
> +[help]
> +	# Format can be man, info, web or html
> +	format = man
> +	autocorrect = 0
> +
> +[ui]
> +	show-headers= true
> +
> +[call-graph]
> +	# fp (framepointer), dwarf
> +	record-mode = fp
> +	print-type = graph
> +	order = caller
> +	sort-key = function
> +
> +Variables
> +~~~~~~~~~
> +
> +colors.*::
> +	Color variables can customize colors of the output which is printed out
> +	from ‘report’, ‘top’, ’annotate’ on tui.
> +	Color variables are composed of foreground and background
> +	and should have two values, comma separated as below.
> +

You're being repetitive with "Color variables" here, perhaps you can
replace the above with:

----
  The variables for customizing the colors used in the output for the
'report', 'top' and annotate in the TUI. They should specify the
background and foreground colors, separated by a comma, for example:

----

> +		medium = green, lightgray
> +
> +	If you want to keep the background or the foregroud color set for your
> +	terminal, replace the desired value with 'default'. For instance:

----------
If you want to use the color configured for you terminal, just leave it 
as 'default', for example:

		medium = default, lightgray
----

IIRC 'default, default' is the same as not having to specify anything
(or should be :-) ).

> +
> +		medium = default, default
> +
> +	Available colors:
> +	red, green, default, black, blue, white, magenta, lightgray
> +
> +	colors.top::
> +		‘top’ means a overhead percentage which is more than 5%.
> +		And values of this variable specify colors of percentage.
                                                    percentage colors.
> +		Basic key values are foreground-color ’red’ and
> +		background-color ’default’.
> +	colors.medium::
> +		‘medium’ means a overhead percentage which has more than 0.5%.
> +		Default values are ’green’ and ’default’.
> +	colors.normal::
> +		‘normal’ means the rest of overhead percentages
> +		except ‘top’, ‘medium’, ‘selected’.
> +		Default values are ’lightgray’ and ’default’.
> +	colors.selected::
> +		This selects the colors for the current entry in a list of entries
> +		from sub-commands (top,report,annotate).
> +		Default values are ’white’ and ’lightgray’.
> +	colors.code::
> +		Colors for arrows and lines in jumps on  assembly code listings
> +		such as ‘jns’,’jmp’,’jane’,etc. Default values are ‘blue’, ‘default’.
> +	colors.addr::
> +		This selects colors for addresses from ’annotate’.
> +		Default values are ‘magenta’, ‘default’.
> +	colors.root::
> +		Colors for headers in the output of a sub-command ‘top’.
> +		Default values are ‘white’, ‘blue’.
> +
> +tui.*::
> +	A boolean value that controls if the TUI browser will be used
> +	for subcommands having that UI.
> +	By default, TUI is enabled if perf detects the required library during build
> +	and this config option can control it. Available subcommands are 'top',
> +	'report' and 'annotate'.
> +
> +gtk.*::
> +	A boolean value that controls if GTK+2 GUI browser for
> +	each subcommand.  By default, GUI can be enabled if perf detects the

"Control if ... for each subcommand", controls what?! Can you rephrase?
If it should be used by default?

> +	required library during build and this config option can control it.
> +	Available subcommands are 'top', 'report' and 'annotate'.
> +
> +buildid.*::
> +	buildid.dir::
> +		Each executable and shared library in modern distributions comes with a
> +		content based identified that, if available, will be inserted in a
                      
                         "identifier"

> +		'perf.data' file header to, at analysis time find what is needed to do
> +		symbol resolution, code anotation, etc.
                                        annotation
> +
> +		The recording tools also stores a hard link or copy in a per-user
> +		directory, $HOME/.debug/, of binaries, shared libraries, /proc/kallsyms
> +		and /proc/kcore files to be used at analysis time.
> +
> +		The buildid.dir variable can be used to either change this directory
> +		cache location, or to disable it altogether. If you want to disable it,
> +		set buildid.dir to /dev/null. The default is $HOME/.debug
> +
> +annotate.*::
> +	There’re options which work with a ’annotate’ sub-command.
> +	This options are in control of addresses, jump function, source code
        These
> +	in lines of assembly code from a specific program.
> +
> +	annotate.hide_src_code::
> +		If a program which is analyzed has source code,
> +		this option let ‘annotate’ print a list of assembly code with the source code.
                             lets
> +		For example, let’s see a part of a program. There’re four lines.
> +		If this option is ‘true’, they can be printed
> +		without source code from a program as below.
> +
> +		│        push   %rbp
> +		│        mov    %rsp,%rbp
> +		│        sub    $0x10,%rsp
> +		│        mov    (%rdi),%rdx
> +
> +		But if this option is ‘false’, source code of the part
> +		can be also printed as below.
> +
> +		│      struct rb_node *rb_next(const struct rb_node *node)
> +		│      {
> +		│        push   %rbp
> +		│        mov    %rsp,%rbp
> +		│        sub    $0x10,%rsp
> +		│              struct rb_node *parent;
> +		│
> +		│              if (RB_EMPTY_NODE(node))
> +		│        mov    (%rdi),%rdx
> +		│              return n;
> +
> +        annotate.use_offset::
> +		Basing on a first address of a loaded function, offset can be used.
> +		Instead of using original addresses of assembly code,
> +		addresses subtracted from a base address can be printed.
> +		Let’s illustrate a example.
> +		If a base address is 0XFFFFFFFF81624d50 as below,
> +
> +		ffffffff81624d50 <load0>
> +
> +		a address on assembly code has a specific absolute address as below
> +
> +		ffffffff816250b8:│  mov    0x8(%r14),%rdi
> +
> +		but if use_offset is ’true’, a address subtracted from a base address is printed.
> +		The default is true. This option is only applied to TUI.
> +
> +		             368:│  mov    0x8(%r14),%rdi
> +
> +	annotate.jump_arrows::
> +		There can be jump instruction among assembly code.
> +		Depending on a boolean value of jump_arrows,
> +		arrows can be printed or not which represent
> +		where do the instruction jump into as below.
> +
> +		│     ┌──jmp    1333
> +		│     │  xchg   %ax,%ax
> +		│1330:│  mov    %r15,%r10
> +		│1333:└─→cmp    %r15,%r14
> +
> +		If jump_arrow is ‘false’, the arrows isn’t printed as below.
> +
> +		│      ↓ jmp    1333
> +		│        xchg   %ax,%ax
> +		│1330:   mov    %r15,%r10
> +		│1333:   cmp    %r15,%r14
> +
> +        annotate.show_nr_jumps::
> +		Let’s see a part of assembly code.
> +
> +		│1382:   movb   $0x1,-0x270(%rbp)
> +
> +		If use this, the number of branches branching to that address can be printed as below.
> +
> +		│1 1382:   movb   $0x1,-0x270(%rbp)
> +
> +help.*::
> +	help.format:: = man
> +		A format of manual page can be ‘man’, ‘info’, ‘web’ or ‘html’.
> +		’man’ is default.
> +	help.autocorrect:: = 0
> +		Automatically correct and execute mistyped commands after
> +		waiting for the given number of deciseconds (0.1 sec).
> +		Let's see a example. If a mistyped sub-command is executed like 'perf mistyped-command'
> +		and this option is 0, the output is as below.
> +
> +		perf: 'mistyped-command' is not a perf-command. See 'perf --help’.
> +
> +		Or if this option is more than 1, the output can be such as.
> +
> +		WARNING: You called a perf program named 'mistyped-command', which does not exist.
> +		Continuing under the assumption that you meant 'with-kcore'
> +		in 0.1 seconds automatically...
> +		Usage: perf-with-kcore <perf sub-command> <perf.data directory> [<sub-command options> [ -- <workload>]]
> +		<perf sub-command> can be record, script, report or inject
> +		    or: perf-with-kcore fix_buildid_cache_permissions
> +
> +hist.*::
> +	hist.percentage::
> +		This option control a way to calcurate overhead of filtered entries -
> +		that means the value of this option is effective only if there's a
> +		filter (by comm, dso or symbol name).  Suppose a following example:
> +
> +		       Overhead  Symbols
> +		       ........  .......
> +		        33.33%     foo
> +		        33.33%     bar
> +		        33.33%     baz
> +
> +	       This is an original overhead and we'll filter out the first 'foo'
> +	       entry.  The value of 'relative' would increase the overhead of 'bar'
> +	       and 'baz' to 50.00% for each, while 'absolute' would show their
> +	       current overhead (33.33%).
> +
> +ui.*::
> +	ui.show-headers::
> +		There’re columns as header ‘Overhead’, ‘Children’, ‘Shared Object’, ‘Symbol’, ’self’.
> +		If this option is false, they are hiden. This option is only applied to TUI.
> +
> +call-graph.*::
> +	When sub-commands ‘top’ and ‘report’ work with -g/—-children
> +	there’re options in control of call-graph.
> +
> +	call-graph.record-mode::
> +		The record-mode can be ‘fp’ (frame pointer) and ‘dwarf’.
> +		The value of 'dwarf' is effective only if perf detect needed library
> +		(libunwind or a recent version of libdw).  Also it doesn't *require*
> +		the dump-size option since it can use the default value of 8192 if
> +		missing.
> +
> +	call-graph.dump-size::
> +		The size of stack to dump in order to do post-unwinding.  Default is 8192 (byte).
> +		When using dwarf into record-mode this option should have a value.
> +
> +	call-graph.print-type::
> +		The print-types can be graph (graph absolute), fractal (graph relative), flat.
> +		This option controls a way to show overhead for each callchain entry.
> +		Suppose a following example.
> +
> +		Overhead  Symbols
> +		........  .......
> +		  40.00%  foo
> +		      |
> +		      --- foo
> +		      |
> +		      |--50.00%-- bar
> +		      |           main
> +		      |
> +		      --50.00%-- baz
> +		                 main
> +
> +		This output is a default format which is 'fractal'.  The 'foo' came
> +		from 'bar' and 'baz' exactly half and half so 'fractal' shows 50.00%
> +		for each (meaning that it assumes 100% total overhead of 'foo').
> +
> +		The 'graph' uses absolute overhead value of 'foo' as total so each of
> +		'bar' and 'baz' callchain will have 20.00% of overhead.
> +
> +	call-graph.order::
> +		This option controls print order of callchains.  The default is
> +		'callee' which means callee is printed at top and then followed by its
> +		caller and so on.  The 'caller' prints it in reverse order.
> +
> +	call-graph.sort-key::
> +		The callchains are merged if they contain same information.
> +		The sort-key option determines a way to compare the callchains.
> +		A value of 'sort-key' can be 'function' or 'address’.
> +		The default is ‘function’.
> +
> +	call-graph.threshold::
> +		When there're many callchains it'd print tons of lines.  So perf omits
> +		small callchains under a certain overhead (threshold) and this option
> +		control the threashold.  Default is 0.5 (%).
> +
> +	call-graph.print-limit::
> +		This is another way to control the number of callchains printed for a
> +		single entry.  Default is 0 which means no limitation.
> +
> +report.*::
> +	report.percent-limit::
> +		This one is mostly same as call-graph.threshold but works for
> +		histogram entries.  Entries have overhead lower than this percentage
> +		will not be printed.  Default is 0.
> +		If percent-limit is 70, the output which has percentages of
> +		each overhead above 70% can be printed.
> +
> +	report.queue-size::
> +		option to setup the maximum allocation size for session's
> +		ordered events queue, if not set there's no default limit.
> +
> +	report.children::
> +		The children means that functions called from another function.
> +		If the option is true, accumulate callchain of children and show total overhead.
> +		Please refer to the perf-report manual.
> +
> +top.*::
> +	top.children::
> +		This option means same as report.children.
> +		So it is true, the output of ‘top’ is rearranged by each overhead into children group.
> +
> +man.*::
> +	man.viewer::
> +		This option can assign a manual tool with which a subcommand 'help' work.
> +		it can used as 'man', 'woman', 'konqueror'. Default value is 'man'.
> +
> +pager.*::
> +	pager.<subcommand>::
> +		When a subcommand work as stdio instead of TUI, use pager with it.
> +		Default value is 'true'.
> +
> +kmem.*::
> +	kmem.default::
> +		This option can decide which allocator is analyzed between 'slab' and 'page'
> +		without using options '--slab' and '--page'.
> +		Default value is 'slab'.
> +
> +SEE ALSO
> +--------
> +linkperf:perf[1], linkperf:perf-report[1]
> diff --git a/tools/perf/builtin-config.c b/tools/perf/builtin-config.c
> new file mode 100644
> index 0000000..2d16150
> --- /dev/null
> +++ b/tools/perf/builtin-config.c
> @@ -0,0 +1,61 @@
> +/*
> + * builtin-config.c
> + *
> + * Copyright (C) 2015, Taeung Song <treeze.taeung@...il.com>
> + *
> + */
> +#include "builtin.h"
> +
> +#include "perf.h"
> +
> +#include "util/cache.h"
> +#include "util/parse-options.h"
> +#include "util/util.h"
> +#include "util/debug.h"
> +
> +static const char * const config_usage[] = {
> +	"perf config [options]",
> +	NULL
> +};
> +
> +enum actions {
> +	ACTION_LIST = 1
> +} actions;
> +
> +static struct option config_options[] = {
> +	OPT_SET_UINT('l', "list", &actions,
> +		     "show current config variables", ACTION_LIST),
> +	OPT_END()
> +};
> +
> +static int show_config(const char *key, const char *value,
> +		       void *cb __maybe_unused)
> +{
> +	if (value)
> +		printf("%s=%s\n", key, value);
> +	else
> +		printf("%s\n", key);
> +
> +	return 0;
> +}
> +
> +int cmd_config(int argc, const char **argv, const char *prefix __maybe_unused)
> +{
> +	int ret = 0;
> +
> +	argc = parse_options(argc, argv, config_options, config_usage,
> +			     PARSE_OPT_STOP_AT_NON_OPTION);
> +
> +	switch (actions) {
> +	case ACTION_LIST:
> +	default:
> +		if (argc) {
> +			pr_err("Error: takes no arguments\n");
> +			parse_options_usage(config_usage, config_options, "l", 1);
> +			return -1;
> +		} else
> +			ret = perf_config(show_config, NULL);
> +	}
> +
> +	return ret;
> +}
> diff --git a/tools/perf/builtin.h b/tools/perf/builtin.h
> index 3688ad2..3f871b5 100644
> --- a/tools/perf/builtin.h
> +++ b/tools/perf/builtin.h
> @@ -17,6 +17,7 @@ extern int cmd_annotate(int argc, const char **argv, const char *prefix);
>  extern int cmd_bench(int argc, const char **argv, const char *prefix);
>  extern int cmd_buildid_cache(int argc, const char **argv, const char *prefix);
>  extern int cmd_buildid_list(int argc, const char **argv, const char *prefix);
> +extern int cmd_config(int argc, const char **argv, const char *prefix);
>  extern int cmd_diff(int argc, const char **argv, const char *prefix);
>  extern int cmd_evlist(int argc, const char **argv, const char *prefix);
>  extern int cmd_help(int argc, const char **argv, const char *prefix);
> diff --git a/tools/perf/command-list.txt b/tools/perf/command-list.txt
> index 00fcaf8..acc3ea7 100644
> --- a/tools/perf/command-list.txt
> +++ b/tools/perf/command-list.txt
> @@ -9,6 +9,7 @@ perf-buildid-cache		mainporcelain common
>  perf-buildid-list		mainporcelain common
>  perf-data			mainporcelain common
>  perf-diff			mainporcelain common
> +perf-config			mainporcelain common
>  perf-evlist			mainporcelain common
>  perf-inject			mainporcelain common
>  perf-kmem			mainporcelain common
> diff --git a/tools/perf/perf.c b/tools/perf/perf.c
> index 3d4c7c0..4bee53c 100644
> --- a/tools/perf/perf.c
> +++ b/tools/perf/perf.c
> @@ -39,6 +39,7 @@ struct cmd_struct {
>  static struct cmd_struct commands[] = {
>  	{ "buildid-cache", cmd_buildid_cache, 0 },
>  	{ "buildid-list", cmd_buildid_list, 0 },
> +	{ "config",	cmd_config,	0 },
>  	{ "diff",	cmd_diff,	0 },
>  	{ "evlist",	cmd_evlist,	0 },
>  	{ "help",	cmd_help,	0 },
> -- 
> 1.9.1
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Powered by blists - more mailing lists