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@krava 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@krava 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 majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Reply via email to