Messages in this thread Patch in this message | | | Date | Tue, 21 Jul 2015 23:45:01 +0900 | From | Namhyung Kim <> | Subject | Re: [PATCH RFC V5 2/4] perf,tool: per-event time support |
| |
Hi,
On Tue, Jul 21, 2015 at 03:39:17AM +0000, Liang, Kan wrote: > > 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 ;-) > > > > I agree. > > > 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 > > > > So you are going to change current behavior? > The current behavior is "global opts setting" > "per_event settring" > default > You are going to change it to "per_event settring" > "global opts setting" > > Default. Right?
Hmm.. I agree that changing current behavior is not good. But I think it makes more sense to prefer per-event settings over global settings in general.
Thanks, Namhyung
| |