From: Arnaldo Carvalho de Melo <acme@kernel.org>
To: Matt Fleming <matt@console-pimps.org>
Cc: Peter Zijlstra <peterz@infradead.org>,
Ingo Molnar <mingo@kernel.org>, Jiri Olsa <jolsa@redhat.com>,
Andi Kleen <andi@firstfloor.org>,
Thomas Gleixner <tglx@linutronix.de>,
linux-kernel@vger.kernel.org, "H. Peter Anvin" <hpa@zytor.com>,
Matt Fleming <matt.fleming@intel.com>
Subject: Re: [PATCH v2 02/11] perf tools: Refactor unit and scale function parameters
Date: Wed, 1 Oct 2014 14:09:45 -0300 [thread overview]
Message-ID: <20141001170945.GE2799@kernel.org> (raw)
In-Reply-To: <1412174192-7010-3-git-send-email-matt@console-pimps.org>
Em Wed, Oct 01, 2014 at 03:36:23PM +0100, Matt Fleming escreveu:
> From: Matt Fleming <matt.fleming@intel.com>
>
> Passing pointers to alias modifiers 'unit' and 'scale' isn't very
> future-proof since if we add more modifiers to the list we'll end up
> passing more arguments.
>
> Instead wrap everything up in a struct perf_pmu_info, which can easily
> be expanded when additional alias modifiers are necessary in the future.
>
> Acked-by: Jiri Olsa <jolsa@kernel.org>
> Cc: Arnaldo Carvalho de Melo <acme@redhat.com>
> Cc: Peter Zijlstra <peterz@infradead.org>
> Signed-off-by: Matt Fleming <matt.fleming@intel.com>
> ---
>
> Changes in v2:
> - Added Jiri's ACK
This one was applied already, with Jiri's ack.
Its in
https://git.kernel.org/cgit/linux/kernel/git/acme/linux.git/log/?h=perf/core
Will be sent to Ingo in the next pull req I'll make.
- Arnaldo
> tools/perf/util/parse-events.c | 9 ++++-----
> tools/perf/util/pmu.c | 38 +++++++++++++++++++++++---------------
> tools/perf/util/pmu.h | 7 ++++++-
> 3 files changed, 33 insertions(+), 21 deletions(-)
>
> diff --git a/tools/perf/util/parse-events.c b/tools/perf/util/parse-events.c
> index 61be3e695ec2..9522cf22ad81 100644
> --- a/tools/perf/util/parse-events.c
> +++ b/tools/perf/util/parse-events.c
> @@ -634,10 +634,9 @@ int parse_events_add_pmu(struct list_head *list, int *idx,
> char *name, struct list_head *head_config)
> {
> struct perf_event_attr attr;
> + struct perf_pmu_info info;
> struct perf_pmu *pmu;
> struct perf_evsel *evsel;
> - const char *unit;
> - double scale;
>
> pmu = perf_pmu__find(name);
> if (!pmu)
> @@ -656,7 +655,7 @@ int parse_events_add_pmu(struct list_head *list, int *idx,
> return evsel ? 0 : -ENOMEM;
> }
>
> - if (perf_pmu__check_alias(pmu, head_config, &unit, &scale))
> + if (perf_pmu__check_alias(pmu, head_config, &info))
> return -EINVAL;
>
> /*
> @@ -671,8 +670,8 @@ int parse_events_add_pmu(struct list_head *list, int *idx,
> evsel = __add_event(list, idx, &attr, pmu_event_name(head_config),
> pmu->cpus);
> if (evsel) {
> - evsel->unit = unit;
> - evsel->scale = scale;
> + evsel->unit = info.unit;
> + evsel->scale = info.scale;
> }
>
> return evsel ? 0 : -ENOMEM;
> diff --git a/tools/perf/util/pmu.c b/tools/perf/util/pmu.c
> index 22a4ad5a927a..93a41ca96b8e 100644
> --- a/tools/perf/util/pmu.c
> +++ b/tools/perf/util/pmu.c
> @@ -210,6 +210,19 @@ static int perf_pmu__new_alias(struct list_head *list, char *dir, char *name, FI
> return 0;
> }
>
> +static inline bool pmu_alias_info_file(char *name)
> +{
> + size_t len;
> +
> + len = strlen(name);
> + if (len > 5 && !strcmp(name + len - 5, ".unit"))
> + return true;
> + if (len > 6 && !strcmp(name + len - 6, ".scale"))
> + return true;
> +
> + return false;
> +}
> +
> /*
> * Process all the sysfs attributes located under the directory
> * specified in 'dir' parameter.
> @@ -218,7 +231,6 @@ static int pmu_aliases_parse(char *dir, struct list_head *head)
> {
> struct dirent *evt_ent;
> DIR *event_dir;
> - size_t len;
> int ret = 0;
>
> event_dir = opendir(dir);
> @@ -234,13 +246,9 @@ static int pmu_aliases_parse(char *dir, struct list_head *head)
> continue;
>
> /*
> - * skip .unit and .scale info files
> - * parsed in perf_pmu__new_alias()
> + * skip info files parsed in perf_pmu__new_alias()
> */
> - len = strlen(name);
> - if (len > 5 && !strcmp(name + len - 5, ".unit"))
> - continue;
> - if (len > 6 && !strcmp(name + len - 6, ".scale"))
> + if (pmu_alias_info_file(name))
> continue;
>
> snprintf(path, PATH_MAX, "%s/%s", dir, name);
> @@ -645,7 +653,7 @@ static int check_unit_scale(struct perf_pmu_alias *alias,
> * defined for the alias
> */
> int perf_pmu__check_alias(struct perf_pmu *pmu, struct list_head *head_terms,
> - const char **unit, double *scale)
> + struct perf_pmu_info *info)
> {
> struct parse_events_term *term, *h;
> struct perf_pmu_alias *alias;
> @@ -655,8 +663,8 @@ int perf_pmu__check_alias(struct perf_pmu *pmu, struct list_head *head_terms,
> * Mark unit and scale as not set
> * (different from default values, see below)
> */
> - *unit = NULL;
> - *scale = 0.0;
> + info->unit = NULL;
> + info->scale = 0.0;
>
> list_for_each_entry_safe(term, h, head_terms, list) {
> alias = pmu_find_alias(pmu, term);
> @@ -666,7 +674,7 @@ int perf_pmu__check_alias(struct perf_pmu *pmu, struct list_head *head_terms,
> if (ret)
> return ret;
>
> - ret = check_unit_scale(alias, unit, scale);
> + ret = check_unit_scale(alias, &info->unit, &info->scale);
> if (ret)
> return ret;
>
> @@ -679,11 +687,11 @@ int perf_pmu__check_alias(struct perf_pmu *pmu, struct list_head *head_terms,
> * set defaults as for evsel
> * unit cannot left to NULL
> */
> - if (*unit == NULL)
> - *unit = "";
> + if (info->unit == NULL)
> + info->unit = "";
>
> - if (*scale == 0.0)
> - *scale = 1.0;
> + if (info->scale == 0.0)
> + info->scale = 1.0;
>
> return 0;
> }
> diff --git a/tools/perf/util/pmu.h b/tools/perf/util/pmu.h
> index 0f5c0a88fdc8..fe90a012c003 100644
> --- a/tools/perf/util/pmu.h
> +++ b/tools/perf/util/pmu.h
> @@ -25,6 +25,11 @@ struct perf_pmu {
> struct list_head list; /* ELEM */
> };
>
> +struct perf_pmu_info {
> + const char *unit;
> + double scale;
> +};
> +
> struct perf_pmu *perf_pmu__find(const char *name);
> int perf_pmu__config(struct perf_pmu *pmu, struct perf_event_attr *attr,
> struct list_head *head_terms);
> @@ -33,7 +38,7 @@ int perf_pmu__config_terms(struct list_head *formats,
> struct list_head *head_terms,
> bool zero);
> int perf_pmu__check_alias(struct perf_pmu *pmu, struct list_head *head_terms,
> - const char **unit, double *scale);
> + struct perf_pmu_info *info);
> struct list_head *perf_pmu__alias(struct perf_pmu *pmu,
> struct list_head *head_terms);
> int perf_pmu_wrap(void);
> --
> 1.9.3
next prev parent reply other threads:[~2014-10-01 17:09 UTC|newest]
Thread overview: 20+ messages / expand[flat|nested] mbox.gz Atom feed top
2014-10-01 14:36 [PATCH v2 00/11] perf: Intel Cache QoS Monitoring support Matt Fleming
2014-10-01 14:36 ` [PATCH 01/11] perf stat: Fix AGGR_CORE segfault on multi-socket system Matt Fleming
2014-10-01 17:08 ` Arnaldo Carvalho de Melo
2014-10-01 20:37 ` Matt Fleming
2014-10-01 14:36 ` [PATCH v2 02/11] perf tools: Refactor unit and scale function parameters Matt Fleming
2014-10-01 17:09 ` Arnaldo Carvalho de Melo [this message]
2014-10-01 14:36 ` [PATCH 03/11] perf tools: Parse event per-package info files Matt Fleming
2014-10-01 14:36 ` [PATCH 04/11] perf: Make perf_cgroup_from_task() global Matt Fleming
2014-10-01 14:36 ` [PATCH 05/11] perf: Add ->count() function to read per-package counters Matt Fleming
2014-10-01 14:36 ` [PATCH 06/11] perf: Move cgroup init before PMU ->event_init() Matt Fleming
2014-10-01 14:36 ` [PATCH 07/11] x86: Add support for Intel Cache QoS Monitoring (CQM) detection Matt Fleming
2014-10-01 14:36 ` [PATCH v2 08/11] perf/x86/intel: Add Intel Cache QoS Monitoring support Matt Fleming
2014-10-01 14:36 ` [PATCH 09/11] perf/x86/intel: Implement LRU monitoring ID allocation for CQM Matt Fleming
2014-10-01 14:36 ` [PATCH v2 10/11] perf/x86/intel: Support task events with Intel CQM Matt Fleming
2014-10-01 14:36 ` [PATCH 11/11] perf/x86/intel: Perform rotation on Intel CQM RMIDs Matt Fleming
2014-10-01 21:42 ` [PATCH v2 00/11] perf: Intel Cache QoS Monitoring support Andi Kleen
2014-10-02 6:51 ` Matt Fleming
2014-10-02 8:41 ` Peter Zijlstra
2014-10-02 15:04 ` Andi Kleen
2014-10-02 17:49 ` Matt Fleming
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20141001170945.GE2799@kernel.org \
--to=acme@kernel.org \
--cc=andi@firstfloor.org \
--cc=hpa@zytor.com \
--cc=jolsa@redhat.com \
--cc=linux-kernel@vger.kernel.org \
--cc=matt.fleming@intel.com \
--cc=matt@console-pimps.org \
--cc=mingo@kernel.org \
--cc=peterz@infradead.org \
--cc=tglx@linutronix.de \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.