linux-kernel - Re: [PATCH 4/4] perf report: Treat an argument as a symbol filter

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20120313062105.GB24357@elte.hu>
Date:	Tue, 13 Mar 2012 07:21:09 +0100
From:	Ingo Molnar <mingo@...e.hu>
To:	Namhyung Kim <namhyung.kim@....com>
Cc:	Arnaldo Carvalho de Melo <acme@...stprotocols.net>,
	Peter Zijlstra <a.p.zijlstra@...llo.nl>,
	Paul Mackerras <paulus@...ba.org>,
	Namhyung Kim <namhyung@...il.com>,
	LKML <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH 4/4] perf report: Treat an argument as a symbol filter


* Namhyung Kim <namhyung.kim@....com> wrote:

> As Ingo requested, it'd be better off treating first (and the only)
> argument as a symbol filter, so that user doesn't need to input the
> symbol on the dialog window.
> 
> Signed-off-by: Namhyung Kim <namhyung.kim@....com>
> ---
>  tools/perf/builtin-report.c |   19 ++++++++++++++-----
>  1 files changed, 14 insertions(+), 5 deletions(-)
> 
> diff --git a/tools/perf/builtin-report.c b/tools/perf/builtin-report.c
> index 25d34d483e49..62c4a11bcf3e 100644
> --- a/tools/perf/builtin-report.c
> +++ b/tools/perf/builtin-report.c
> @@ -50,6 +50,7 @@ struct perf_report {
>  	const char		*pretty_printing_style;
>  	symbol_filter_t		annotate_init;
>  	const char		*cpu_list;
> +	const char		*symbol_filter_str;
>  	DECLARE_BITMAP(cpu_bitmap, MAX_NR_CPUS);
>  };
>  
> @@ -315,6 +316,9 @@ static int __cmd_report(struct perf_report *rep)
>  	list_for_each_entry(pos, &session->evlist->entries, node) {
>  		struct hists *hists = &pos->hists;
>  
> +		if (pos->idx == 0)
> +			hists->symbol_filter_str = rep->symbol_filter_str;
> +
>  		hists__collapse_resort(hists);
>  		hists__output_resort(hists);
>  		nr_samples += hists->stats.nr_events[PERF_RECORD_SAMPLE];
> @@ -586,11 +590,16 @@ int cmd_report(int argc, const char **argv, const char *prefix __used)
>  	} else
>  		symbol_conf.exclude_other = false;
>  
> -	/*
> -	 * Any (unrecognized) arguments left?
> -	 */
> -	if (argc)
> -		usage_with_options(report_usage, options);
> +	if (argc) {
> +		/*
> +		 * Special case: if there's an argument left then assume tha

s/tha/that

> +		 * it's a symbol filter:
> +		 */
> +		if (argc > 1)
> +			usage_with_options(report_usage, options);
> +
> +		report.symbol_filter_str = argv[0];
> +	}

Are people fine with this special case?

To play the devil's advocate it uses up valuable perf report 
command line real estate and we probably cannot change this 
workflow in the future, once people get used to it.

For example doing this would preclude us from adding subsystem 
subcommands to perf report in the future, such as:

 $ perf report sched
 $ perf report kvm
 ...

OTOH have 'perf sched' and 'perf kvm' and maybe those are better 
provided as:

 $ perf kvm report
 $ perf sched report

:-)

Thanks,

	Ingo
--
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/