Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1755623AbbGUOrY (ORCPT ); Tue, 21 Jul 2015 10:47:24 -0400 Received: from mail-pd0-f172.google.com ([209.85.192.172]:35990 "EHLO mail-pd0-f172.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1754706AbbGUOrW (ORCPT ); Tue, 21 Jul 2015 10:47:22 -0400 Date: Tue, 21 Jul 2015 23:45:01 +0900 From: Namhyung Kim To: "Liang, Kan" Cc: Jiri Olsa , "acme@kernel.org" , "jolsa@kernel.org" , "ak@linux.intel.com" , "linux-kernel@vger.kernel.org" Subject: Re: [PATCH RFC V5 2/4] perf,tool: per-event time support Message-ID: <20150721144501.GA10689@danjae.kornet> References: <1437132655-15883-1-git-send-email-kan.liang@intel.com> <1437132655-15883-3-git-send-email-kan.liang@intel.com> <20150718124547.GA3759@krava.brq.redhat.com> <20150719032128.GA24219@danjae.kornet> <20150719191322.GA27893@krava.brq.redhat.com> <37D7C6CF3E00A74B8858931C1DB2F0770188F5E9@SHSMSX103.ccr.corp.intel.com> <20150720174046.GA27331@krava.brq.redhat.com> <37D7C6CF3E00A74B8858931C1DB2F0770188F94B@SHSMSX103.ccr.corp.intel.com> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline In-Reply-To: <37D7C6CF3E00A74B8858931C1DB2F0770188F94B@SHSMSX103.ccr.corp.intel.com> User-Agent: Mutt/1.5.23+89 (0255b37be491) (2014-03-12) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 2406 Lines: 72 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 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/