From patchwork Sun Dec 7 07:37:24 2014 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Sukadev Bhattiprolu X-Patchwork-Id: 418450 Return-Path: X-Original-To: patchwork-incoming@ozlabs.org Delivered-To: patchwork-incoming@ozlabs.org Received: from lists.ozlabs.org (lists.ozlabs.org [IPv6:2401:3900:2:1::3]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 76362140119 for ; Sun, 7 Dec 2014 18:38:35 +1100 (AEDT) Received: from ozlabs.org (ozlabs.org [103.22.144.67]) by lists.ozlabs.org (Postfix) with ESMTP id 58AFE1A0C8C for ; Sun, 7 Dec 2014 18:38:35 +1100 (AEDT) X-Original-To: linuxppc-dev@lists.ozlabs.org Delivered-To: linuxppc-dev@lists.ozlabs.org Received: from e8.ny.us.ibm.com (e8.ny.us.ibm.com [32.97.182.138]) (using TLSv1 with cipher DHE-RSA-AES256-SHA (256/256 bits)) (No client certificate requested) by lists.ozlabs.org (Postfix) with ESMTPS id 6B6F71A04B2 for ; Sun, 7 Dec 2014 18:37:56 +1100 (AEDT) Received: from /spool/local by e8.ny.us.ibm.com with IBM ESMTP SMTP Gateway: Authorized Use Only! Violators will be prosecuted for from ; Sun, 7 Dec 2014 02:37:51 -0500 Received: from d01dlp03.pok.ibm.com (9.56.250.168) by e8.ny.us.ibm.com (192.168.1.108) with IBM ESMTP SMTP Gateway: Authorized Use Only! Violators will be prosecuted; Sun, 7 Dec 2014 02:37:50 -0500 Received: from b01cxnp22036.gho.pok.ibm.com (b01cxnp22036.gho.pok.ibm.com [9.57.198.26]) by d01dlp03.pok.ibm.com (Postfix) with ESMTP id EFA56C90043 for ; Sun, 7 Dec 2014 02:29:47 -0500 (EST) Received: from d01av02.pok.ibm.com (d01av02.pok.ibm.com [9.56.224.216]) by b01cxnp22036.gho.pok.ibm.com (8.14.9/8.14.9/NCO v10.0) with ESMTP id sB77bi1b23986286 for ; Sun, 7 Dec 2014 07:37:44 GMT Received: from d01av02.pok.ibm.com (localhost [127.0.0.1]) by d01av02.pok.ibm.com (8.14.4/8.14.4/NCO v10.0 AVout) with ESMTP id sB77bhlC018265 for ; Sun, 7 Dec 2014 02:37:43 -0500 Received: from suka-t410.localdomain (suka-t410.usor.ibm.com [9.70.94.25]) by d01av02.pok.ibm.com (8.14.4/8.14.4/NCO v10.0 AVin) with ESMTP id sB77bhKK018256; Sun, 7 Dec 2014 02:37:43 -0500 Received: by suka-t410.localdomain (Postfix, from userid 155514) id D46D6112230F; Sat, 6 Dec 2014 23:37:24 -0800 (PST) Date: Sat, 6 Dec 2014 23:37:24 -0800 From: Sukadev Bhattiprolu To: Jiri Olsa Subject: Re: [PATCH v5 1/4] tools/perf: support parsing parameterized events Message-ID: <20141207073724.GA32721@us.ibm.com> References: <1417572578-9051-1-git-send-email-sukadev@linux.vnet.ibm.com> <1417572578-9051-2-git-send-email-sukadev@linux.vnet.ibm.com> <20141204124422.GA4195@krava.brq.redhat.com> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <20141204124422.GA4195@krava.brq.redhat.com> X-Operating-System: Linux 2.0.32 on an i486 User-Agent: Mutt/1.5.21 (2010-09-15) X-TM-AS-MML: disable X-Content-Scanned: Fidelis XPS MAILER x-cbid: 14120707-0029-0000-0000-000001588563 Cc: Michael Ellerman , peterz@infradead.org, linux-kernel@vger.kernel.org, Arnaldo Carvalho de Melo , dev@codyps.com, Paul Mackerras , linuxppc-dev@lists.ozlabs.org X-BeenThere: linuxppc-dev@lists.ozlabs.org X-Mailman-Version: 2.1.18 Precedence: list List-Id: Linux on PowerPC Developers Mail List List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: linuxppc-dev-bounces+patchwork-incoming=ozlabs.org@lists.ozlabs.org Sender: "Linuxppc-dev" Jiri Olsa [jolsa@redhat.com] wrote: | anyway we could assign directly to the param term name as you do, | but I think we just need to mark the term as parametrized, like: | | in /sys/bus/event_source/devices/pmu/events/event_name you have: | param2=?,bar=1,param1=? I like the idea of just using a single ? for required parameters, but the problem I had with this approach can be seen with these two sysfs entries: $ cat HPM_0THRD_NON_IDLE_CCYC__PHYS_CORE domain=0x2,offset=0xe0,starting_index=core,lpar=0x0 $ cat HPM_0THRD_NON_IDLE_CCYC__VCPU_HOME_CORE domain=0x3,offset=0xe0,starting_index=vcpu,lpar=sibling_guest_id The parameter 'starting_index' refers to a core in one event and vcpu in another event. We were trying to give a hint as to what it refers to. Given that, 'starting_index' is not very intuitive, how about discarding starting_index and replacing with what it really means for the event and, use a simple '?' to indicate required parameter). $ cat HPM_0THRD_NON_IDLE_CCYC__PHYS_CORE domain=0x2,offset=0xe0,core=?,lpar=0x0 $ cat HPM_0THRD_NON_IDLE_CCYC__VCPU_HOME_CORE domain=0x3,offset=0xe0,vcpu=?,lpar=? perf list shows these as: hv_24x7/HPM_0THRD_NON_IDLE_CCYC__PHYS_CORE,core=?/ hv_24x7/HPM_0THRD_NON_IDLE_CCYC__VCPU_HOME_CHIP,vcpu=?,lpar=?/ command line would be -e hv_24x7/HPM_0THRD_NON_IDLE_CCYC__PHYS_CORE,core=2/ or -e hv_24x7/HPM_0THRD_NON_IDLE_CCYC__VCPU_HOME_CHIP,vcpu=2,lpar=7/ and would fail if a required parameter is missing. This would eliminate the need for new strings like 'sibling_guest_id' (or as Cody calls it monopolizing strings...) Following quick patch on top of the patchset shows the changes: diff --git a/arch/powerpc/perf/hv-24x7.c b/arch/powerpc/perf/hv-24x7.c index 73d5bfc..a82bc64 100644 --- a/arch/powerpc/perf/hv-24x7.c +++ b/arch/powerpc/perf/hv-24x7.c @@ -27,6 +27,8 @@ #include "hv-24x7-catalog.h" #include "hv-common.h" + +#if 0 static const char *domain_to_index_string(unsigned domain) { switch (domain) { @@ -40,6 +42,7 @@ static const char *domain_to_index_string(unsigned domain) return "UNKNOWN_DOMAIN_INDEX_STRING"; } } +#endif static const char *event_domain_suffix(unsigned domain) { @@ -114,7 +117,8 @@ static bool catalog_entry_domain_is_valid(unsigned domain) /* u3 0-6, one of HV_24X7_PERF_DOMAIN */ EVENT_DEFINE_RANGE_FORMAT(domain, config, 0, 3); /* u16 */ -EVENT_DEFINE_RANGE_FORMAT(starting_index, config, 16, 31); +EVENT_DEFINE_RANGE_FORMAT(core, config, 16, 31); +EVENT_DEFINE_RANGE_FORMAT(vcpu, config, 16, 31); /* u32, see "data_offset" */ EVENT_DEFINE_RANGE_FORMAT(offset, config, 32, 63); /* u16 */ @@ -127,7 +131,8 @@ EVENT_DEFINE_RANGE(reserved3, config2, 0, 63); static struct attribute *format_attrs[] = { &format_attr_domain.attr, &format_attr_offset.attr, - &format_attr_starting_index.attr, + &format_attr_core.attr, + &format_attr_vcpu.attr, &format_attr_lpar.attr, NULL, }; @@ -280,19 +285,23 @@ static unsigned core_domains[] = { static char *event_fmt(struct hv_24x7_event_data *event, unsigned domain) { + const char *sindex; const char *lpar; - if (is_physical_domain(domain)) + if (is_physical_domain(domain)) { lpar = "0x0"; - else - lpar = "$sibling_guest_id"; + sindex = "core"; + } else { + lpar = "?"; + sindex = "vcpu"; + } return kasprintf(GFP_KERNEL, - "domain=0x%x,offset=0x%x,starting_index=%s,lpar=%s", + "domain=0x%x,offset=0x%x,%s=?,lpar=%s", domain, be16_to_cpu(event->event_counter_offs) + be16_to_cpu(event->event_group_record_offs), - domain_to_index_string(domain), + sindex, lpar); } @@ -1061,9 +1070,17 @@ out: static unsigned long event_24x7_request(struct perf_event *event, u64 *res, bool success_expected) { + u16 idx; + unsigned domain = event_get_domain(event); + + if (is_physical_domain(domain)) + idx = event_get_core(event); + else + idx = event_get_vcpu(event); + return single_24x7_request(event_get_domain(event), event_get_offset(event), - event_get_starting_index(event), + idx, event_get_lpar(event), res, success_expected); diff --git a/tools/perf/util/pmu.c b/tools/perf/util/pmu.c index f8674c1..d208fef 100644 --- a/tools/perf/util/pmu.c +++ b/tools/perf/util/pmu.c @@ -826,7 +826,7 @@ static char *format_alias(char *buf, int len, struct perf_pmu *pmu, list_for_each_entry(term, &alias->terms, list) if (term->type_val == PARSE_EVENTS__TERM_TYPE_STR) used += snprintf(buf + used, sub_non_neg(len, used), - ",%s=$%s", term->config, + ",%s=%s", term->config, term->val.str); if (sub_non_neg(len, used) > 0) {