[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20150720174046.GA27331@krava.brq.redhat.com>
Date: Mon, 20 Jul 2015 19:40:46 +0200
From: Jiri Olsa <jolsa@...hat.com>
To: "Liang, Kan" <kan.liang@...el.com>
Cc: Namhyung Kim <namhyung@...nel.org>,
"acme@...nel.org" <acme@...nel.org>,
"jolsa@...nel.org" <jolsa@...nel.org>,
"ak@...ux.intel.com" <ak@...ux.intel.com>,
"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH RFC V5 2/4] perf,tool: per-event time support
On Mon, Jul 20, 2015 at 03:04:20PM +0000, Liang, Kan wrote:
SNIP
> break;
> + case PARSE_EVENTS__TERM_TYPE_TIME:
> + if (time_set)
> + *time_set = true;
> + CHECK_TYPE_VAL(NUM);
> + if (term->val.num > 1)
> + return -EINVAL;
> + if (term->val.num == 1)
> + attr->sample_type |= PERF_SAMPLE_TIME;
> + break;
> case PARSE_EVENTS__TERM_TYPE_NAME:
> CHECK_TYPE_VAL(STR);
> break;
> @@ -611,12 +621,13 @@ do { \
>
> static int config_attr(struct perf_event_attr *attr,
> struct list_head *head,
> - struct parse_events_error *err)
> + struct parse_events_error *err,
> + bool *time_set)
> {
> struct parse_events_term *term;
>
> list_for_each_entry(term, head, list)
> - if (config_term(attr, term, err))
> + if (config_term(attr, term, err, time_set))
I think we should rather have some generic way to mart event specific
settings otherwise this function prototype will grow wild ;-)
how about posponing static terms configuration after
global config was set.. like in attached change
works for period now:
[jolsa@...va perf]$ ./perf record -e 'cpu/instructions,period=20000/' -c 1000 sleep 1
[ perf record: Woken up 1 times to write data ]
/proc/kcore requires CAP_SYS_RAWIO capability to access.
[ perf record: Captured and wrote 0.015 MB perf.data (35 samples) ]
[jolsa@...va perf]$ ./perf evlist -vv
cpu/instructions,period=20000/: type: 4, size: 112, config: 0xc0, { sample_period, sample_freq }: 20000
we'd just add new term for time the same way
also need to check if that will help for the backtrace per-event change
jirka
---
diff --git a/tools/perf/util/evsel.c b/tools/perf/util/evsel.c
index 83c08037e7e2..f635d6ba83b0 100644
--- a/tools/perf/util/evsel.c
+++ b/tools/perf/util/evsel.c
@@ -207,6 +207,7 @@ void perf_evsel__init(struct perf_evsel *evsel,
evsel->unit = "";
evsel->scale = 1.0;
INIT_LIST_HEAD(&evsel->node);
+ INIT_LIST_HEAD(&evsel->config_terms);
perf_evsel__object.init(evsel);
evsel->sample_size = __perf_evsel__sample_size(attr->sample_type);
perf_evsel__calc_id_pos(evsel);
@@ -585,6 +586,21 @@ perf_evsel__config_callgraph(struct perf_evsel *evsel,
}
}
+static void apply_config_terms(struct perf_event_attr *attr,
+ struct list_head *config_terms)
+{
+ struct perf_evsel_config_term *term;
+
+ list_for_each_entry(term, config_terms, list) {
+ switch (term->type) {
+ case PERF_EVSEL__CONFIG_TERM_PERIOD:
+ attr->sample_period = term->val.period;
+ default:
+ break;
+ }
+ }
+}
+
/*
* The enable_on_exec/disabled value strategy:
*
@@ -773,6 +789,8 @@ void perf_evsel__config(struct perf_evsel *evsel, struct record_opts *opts)
attr->use_clockid = 1;
attr->clockid = opts->clockid;
}
+
+ apply_config_terms(attr, &evsel->config_terms);
}
static int perf_evsel__alloc_fd(struct perf_evsel *evsel, int ncpus, int nthreads)
@@ -896,6 +914,16 @@ static void perf_evsel__free_id(struct perf_evsel *evsel)
zfree(&evsel->id);
}
+static void perf_evsel__free_config_terms(struct perf_evsel *evsel)
+{
+ struct perf_evsel_config_term *term, *h;
+
+ list_for_each_entry_safe(term, h, &evsel->config_terms, list) {
+ list_del(&term->list);
+ free(term);
+ }
+}
+
void perf_evsel__close_fd(struct perf_evsel *evsel, int ncpus, int nthreads)
{
int cpu, thread;
@@ -915,6 +943,7 @@ void perf_evsel__exit(struct perf_evsel *evsel)
assert(list_empty(&evsel->node));
perf_evsel__free_fd(evsel);
perf_evsel__free_id(evsel);
+ perf_evsel__free_config_terms(evsel);
close_cgroup(evsel->cgrp);
cpu_map__put(evsel->cpus);
thread_map__put(evsel->threads);
diff --git a/tools/perf/util/evsel.h b/tools/perf/util/evsel.h
index fe9f3279632b..de2ba4eb858c 100644
--- a/tools/perf/util/evsel.h
+++ b/tools/perf/util/evsel.h
@@ -31,6 +31,18 @@ struct perf_sample_id {
struct cgroup_sel;
+enum {
+ PERF_EVSEL__CONFIG_TERM_PERIOD,
+};
+
+struct perf_evsel_config_term {
+ struct list_head list;
+ int type;
+ union {
+ u64 period;
+ } val;
+};
+
/** struct perf_evsel - event selector
*
* @name - Can be set to retain the original event name passed by the user,
@@ -86,6 +98,7 @@ struct perf_evsel {
unsigned long *per_pkg_mask;
struct perf_evsel *leader;
char *group_name;
+ struct list_head config_terms;
};
union u64_swap {
diff --git a/tools/perf/util/parse-events.c b/tools/perf/util/parse-events.c
index a71eeb279ed2..d83ac773ab4f 100644
--- a/tools/perf/util/parse-events.c
+++ b/tools/perf/util/parse-events.c
@@ -276,7 +276,8 @@ const char *event_type(int type)
static struct perf_evsel *
__add_event(struct list_head *list, int *idx,
struct perf_event_attr *attr,
- char *name, struct cpu_map *cpus)
+ char *name, struct cpu_map *cpus,
+ struct list_head *config_terms)
{
struct perf_evsel *evsel;
@@ -291,14 +292,19 @@ __add_event(struct list_head *list, int *idx,
if (name)
evsel->name = strdup(name);
+
+ if (config_terms)
+ list_splice(config_terms, &evsel->config_terms);
+
list_add_tail(&evsel->node, list);
return evsel;
}
static int add_event(struct list_head *list, int *idx,
- struct perf_event_attr *attr, char *name)
+ struct perf_event_attr *attr, char *name,
+ struct list_head *config_terms)
{
- return __add_event(list, idx, attr, name, NULL) ? 0 : -ENOMEM;
+ return __add_event(list, idx, attr, name, NULL, config_terms) ? 0 : -ENOMEM;
}
static int parse_aliases(char *str, const char *names[][PERF_EVSEL__MAX_ALIASES], int size)
@@ -377,7 +383,7 @@ int parse_events_add_cache(struct list_head *list, int *idx,
memset(&attr, 0, sizeof(attr));
attr.config = cache_type | (cache_op << 8) | (cache_result << 16);
attr.type = PERF_TYPE_HW_CACHE;
- return add_event(list, idx, &attr, name);
+ return add_event(list, idx, &attr, name, NULL);
}
static int add_tracepoint(struct list_head *list, int *idx,
@@ -539,7 +545,7 @@ int parse_events_add_breakpoint(struct list_head *list, int *idx,
attr.type = PERF_TYPE_BREAKPOINT;
attr.sample_period = 1;
- return add_event(list, idx, &attr, NULL);
+ return add_event(list, idx, &attr, NULL, NULL);
}
static int check_type_val(struct parse_events_term *term,
@@ -561,7 +567,8 @@ static int check_type_val(struct parse_events_term *term,
static int config_term(struct perf_event_attr *attr,
struct parse_events_term *term,
- struct parse_events_error *err)
+ struct parse_events_error *err,
+ struct list_head *config_terms)
{
#define CHECK_TYPE_VAL(type) \
do { \
@@ -569,6 +576,20 @@ do { \
return -EINVAL; \
} while (0)
+#define ADD_EVSEL_CONFIG(__type, __name, __val) \
+do { \
+ struct perf_evsel_config_term *__t; \
+ \
+ __t = zalloc(sizeof(*__t)); \
+ if (!__t) \
+ return -ENOMEM; \
+ \
+ INIT_LIST_HEAD(&__t->list); \
+ __t->type = PERF_EVSEL__CONFIG_TERM_ ## __type; \
+ __t->val.__name = __val; \
+ list_add_tail(&__t->list, config_terms); \
+} while (0)
+
switch (term->type_term) {
case PARSE_EVENTS__TERM_TYPE_USER:
/*
@@ -590,7 +611,7 @@ do { \
break;
case PARSE_EVENTS__TERM_TYPE_SAMPLE_PERIOD:
CHECK_TYPE_VAL(NUM);
- attr->sample_period = term->val.num;
+ ADD_EVSEL_CONFIG(PERIOD, period, term->val.num);
break;
case PARSE_EVENTS__TERM_TYPE_BRANCH_SAMPLE_TYPE:
/*
@@ -606,17 +627,19 @@ do { \
}
return 0;
+#undef ADD_EVSEL_CONFIG
#undef CHECK_TYPE_VAL
}
static int config_attr(struct perf_event_attr *attr,
struct list_head *head,
- struct parse_events_error *err)
+ struct parse_events_error *err,
+ struct list_head *config_terms)
{
struct parse_events_term *term;
list_for_each_entry(term, head, list)
- if (config_term(attr, term, err))
+ if (config_term(attr, term, err, config_terms))
return -EINVAL;
return 0;
@@ -628,16 +651,17 @@ int parse_events_add_numeric(struct parse_events_evlist *data,
struct list_head *head_config)
{
struct perf_event_attr attr;
+ LIST_HEAD(config_terms);
memset(&attr, 0, sizeof(attr));
attr.type = type;
attr.config = config;
if (head_config &&
- config_attr(&attr, head_config, data->error))
+ config_attr(&attr, head_config, data->error, &config_terms))
return -EINVAL;
- return add_event(list, &data->idx, &attr, NULL);
+ return add_event(list, &data->idx, &attr, NULL, &config_terms);
}
static int parse_events__is_name_term(struct parse_events_term *term)
@@ -664,6 +688,7 @@ int parse_events_add_pmu(struct parse_events_evlist *data,
struct perf_pmu_info info;
struct perf_pmu *pmu;
struct perf_evsel *evsel;
+ LIST_HEAD(config_terms);
pmu = perf_pmu__find(name);
if (!pmu)
@@ -678,7 +703,7 @@ int parse_events_add_pmu(struct parse_events_evlist *data,
if (!head_config) {
attr.type = pmu->type;
- evsel = __add_event(list, &data->idx, &attr, NULL, pmu->cpus);
+ evsel = __add_event(list, &data->idx, &attr, NULL, pmu->cpus, NULL);
return evsel ? 0 : -ENOMEM;
}
@@ -689,14 +714,14 @@ int parse_events_add_pmu(struct parse_events_evlist *data,
* Configure hardcoded terms first, no need to check
* return value when called with fail == 0 ;)
*/
- if (config_attr(&attr, head_config, data->error))
+ if (config_attr(&attr, head_config, data->error, &config_terms))
return -EINVAL;
if (perf_pmu__config(pmu, &attr, head_config, data->error))
return -EINVAL;
evsel = __add_event(list, &data->idx, &attr,
- pmu_event_name(head_config), pmu->cpus);
+ pmu_event_name(head_config), pmu->cpus, &config_terms);
if (evsel) {
evsel->unit = info.unit;
evsel->scale = info.scale;
--
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