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] [day] [month] [year] [list]
Message-ID: <38906a72-d974-73c1-636e-54a3d87f0fa5@linux.ibm.com>
Date:   Tue, 26 Oct 2021 13:48:00 +0530
From:   kajoljain <kjain@...ux.ibm.com>
To:     Ian Rogers <irogers@...gle.com>, Andi Kleen <ak@...ux.intel.com>,
        Jiri Olsa <jolsa@...hat.com>,
        Jin Yao <yao.jin@...ux.intel.com>,
        Namhyung Kim <namhyung@...nel.org>,
        John Garry <john.garry@...wei.com>,
        "Paul A . Clarke" <pc@...ibm.com>,
        Arnaldo Carvalho de Melo <acme@...nel.org>,
        Riccardo Mancini <rickyman7@...il.com>,
        Kan Liang <kan.liang@...ux.intel.com>,
        Peter Zijlstra <peterz@...radead.org>,
        Ingo Molnar <mingo@...hat.com>,
        Mark Rutland <mark.rutland@....com>,
        Alexander Shishkin <alexander.shishkin@...ux.intel.com>,
        Kees Cook <keescook@...omium.org>,
        Sami Tolvanen <samitolvanen@...gle.com>,
        Nick Desaulniers <ndesaulniers@...gle.com>,
        Andrew Morton <akpm@...ux-foundation.org>,
        Jacob Keller <jacob.e.keller@...el.com>,
        Zhen Lei <thunder.leizhen@...wei.com>,
        ToastC <mrtoastcheng@...il.com>,
        Joakim Zhang <qiangqing.zhang@....com>,
        Felix Fietkau <nbd@....name>,
        Jiapeng Chong <jiapeng.chong@...ux.alibaba.com>,
        Song Liu <songliubraving@...com>, Fabian Hemmer <copy@...y.sh>,
        Alexander Antonov <alexander.antonov@...ux.intel.com>,
        Nicholas Fraser <nfraser@...eweavers.com>,
        Adrian Hunter <adrian.hunter@...el.com>,
        Denys Zagorui <dzagorui@...co.com>,
        Wan Jiabing <wanjiabing@...o.com>,
        Thomas Richter <tmricht@...ux.ibm.com>,
        Sumanth Korikkar <sumanthk@...ux.ibm.com>,
        Heiko Carstens <hca@...ux.ibm.com>,
        Changbin Du <changbin.du@...el.com>,
        linux-kernel@...r.kernel.org, linux-perf-users@...r.kernel.org,
        Andrew Kilroy <andrew.kilroy@....com>
Cc:     Stephane Eranian <eranian@...gle.com>
Subject: Re: [PATCH v2 06/21] perf metric: Add documentation and rename a
 variable.



On 10/15/21 10:51 PM, Ian Rogers wrote:
> Documentation to make current functionality clearer. Rename a variable
> called 'metric' to 'metric_name' as it can be ambiguous as to whether a
> string is the name of a metric or the expression.
> 
> Acked-by: Andi Kleen <ak@...ux.intel.com>
> Signed-off-by: Ian Rogers <irogers@...gle.com>
> ---

Patch looks good to me.

Reviewed-by: Kajol Jain<kjain@...ux.ibm.com>

Thanks,
Kajol Jain

>  tools/perf/util/metricgroup.c | 59 ++++++++++++++++++++++++++++++++---
>  1 file changed, 54 insertions(+), 5 deletions(-)
> 
> diff --git a/tools/perf/util/metricgroup.c b/tools/perf/util/metricgroup.c
> index 139f4a793f92..3e5f02938452 100644
> --- a/tools/perf/util/metricgroup.c
> +++ b/tools/perf/util/metricgroup.c
> @@ -776,13 +776,27 @@ int __weak arch_get_runtimeparam(const struct pmu_event *pe __maybe_unused)
>  
>  struct metricgroup_add_iter_data {
>  	struct list_head *metric_list;
> -	const char *metric;
> +	const char *metric_name;
>  	struct expr_ids *ids;
>  	int *ret;
>  	bool *has_match;
>  	bool metric_no_group;
>  };
>  
> +/**
> + * __add_metric - Add a metric to metric_list.
> + * @metric_list: The list the metric is added to.
> + * @pe: The pmu_event containing the metric to be added.
> + * @metric_no_group: Should events written to events be grouped "{}" or
> + *                   global. Grouping is the default but due to multiplexing the
> + *                   user may override.
> + * @runtime: A special argument for the parser only known at runtime.
> + * @mp: The pointer to a location holding the first metric added to metric
> + *      list. It is initialized here if this is the first metric.
> + * @parent: The last entry in a linked list of metrics being
> + *          added/resolved. This is maintained to detect recursion.
> + * @ids: Storage for parent list.
> + */
>  static int __add_metric(struct list_head *metric_list,
>  			const struct pmu_event *pe,
>  			bool metric_no_group,
> @@ -1076,7 +1090,7 @@ static int metricgroup__add_metric_sys_event_iter(const struct pmu_event *pe,
>  	struct metric *m = NULL;
>  	int ret;
>  
> -	if (!match_pe_metric(pe, d->metric))
> +	if (!match_pe_metric(pe, d->metric_name))
>  		return 0;
>  
>  	ret = add_metric(d->metric_list, pe, d->metric_no_group, &m, NULL, d->ids);
> @@ -1095,7 +1109,22 @@ static int metricgroup__add_metric_sys_event_iter(const struct pmu_event *pe,
>  	return ret;
>  }
>  
> -static int metricgroup__add_metric(const char *metric, bool metric_no_group,
> +/**
> + * metricgroup__add_metric - Find and add a metric, or a metric group.
> + * @metric_name: The name of the metric or metric group. For example, "IPC"
> + *               could be the name of a metric and "TopDownL1" the name of a
> + *               metric group.
> + * @metric_no_group: Should events written to events be grouped "{}" or
> + *                   global. Grouping is the default but due to multiplexing the
> + *                   user may override.
> + * @events: an out argument string of events that need to be parsed and
> + *          associated with the metric. For example, the metric "IPC" would
> + *          create an events string like "{instructions,cycles}:W".
> + * @metric_list: The list that the metric or metric group are added to.
> + * @map: The map that is searched for metrics, most commonly the table for the
> + *       architecture perf is running upon.
> + */
> +static int metricgroup__add_metric(const char *metric_name, bool metric_no_group,
>  				   struct strbuf *events,
>  				   struct list_head *metric_list,
>  				   const struct pmu_events_map *map)
> @@ -1107,7 +1136,11 @@ static int metricgroup__add_metric(const char *metric, bool metric_no_group,
>  	int i, ret;
>  	bool has_match = false;
>  
> -	map_for_each_metric(pe, i, map, metric) {
> +	/*
> +	 * Iterate over all metrics seeing if metric matches either the name or
> +	 * group. When it does add the metric to the list.
> +	 */
> +	map_for_each_metric(pe, i, map, metric_name) {
>  		has_match = true;
>  		m = NULL;
>  
> @@ -1130,7 +1163,7 @@ static int metricgroup__add_metric(const char *metric, bool metric_no_group,
>  			.fn = metricgroup__add_metric_sys_event_iter,
>  			.data = (void *) &(struct metricgroup_add_iter_data) {
>  				.metric_list = &list,
> -				.metric = metric,
> +				.metric_name = metric_name,
>  				.metric_no_group = metric_no_group,
>  				.ids = &ids,
>  				.has_match = &has_match,
> @@ -1169,6 +1202,22 @@ static int metricgroup__add_metric(const char *metric, bool metric_no_group,
>  	return ret;
>  }
>  
> +/**
> + * metricgroup__add_metric_list - Find and add metrics, or metric groups,
> + *                                specified in a list.
> + * @list: the list of metrics or metric groups. For example, "IPC,CPI,TopDownL1"
> + *        would match the IPC and CPI metrics, and TopDownL1 would match all
> + *        the metrics in the TopDownL1 group.
> + * @metric_no_group: Should events written to events be grouped "{}" or
> + *                   global. Grouping is the default but due to multiplexing the
> + *                   user may override.
> + * @events: an out argument string of events that need to be parsed and
> + *          associated with the metric. For example, the metric "IPC" would
> + *          create an events string like "{instructions,cycles}:W".
> + * @metric_list: The list that metrics are added to.
> + * @map: The map that is searched for metrics, most commonly the table for the
> + *       architecture perf is running upon.
> + */
>  static int metricgroup__add_metric_list(const char *list, bool metric_no_group,
>  					struct strbuf *events,
>  					struct list_head *metric_list,
> 

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ