From: "liuqi (BA)" <liuqi115@huawei.com> To: Will Deacon <will@kernel.org>, Linuxarm <linuxarm@huawei.com> Cc: <mark.rutland@arm.com>, <bhelgaas@google.com>, <linux-pci@vger.kernel.org>, <linux-arm-kernel@lists.infradead.org>, <linux-kernel@vger.kernel.org>, <zhangshaokun@hisilicon.com> Subject: Re: [PATCH v8 2/2] drivers/perf: hisi: Add driver for HiSilicon PCIe PMU Date: Wed, 4 Aug 2021 15:29:54 +0800 [thread overview] Message-ID: <a56266c4-c434-f078-6027-f30c021bd593@huawei.com> (raw) In-Reply-To: <20210802100343.GA27282@willie-the-truck> Hi Will, > Hmm, I was hoping that you would expose all the events as proper perf_events > and get rid of the subevents entirely. > > Then userspace could do things like: > > // Count number of RX memory reads > $ perf stat -e hisi_pcie0_0/rx_memory_read/ > > // Count delay cycles > $ perf stat -e hisi_pcie0_0/latency/ > > // Count both of the above (events must be in the same group) > $ perf stat -g -e hisi_pcie0_0/latency/ -e hisi_pcie0_0/rx_memory_read/ > > Note that in all three of these cases the hardware will be programmed in > the same way and both HISI_PCIE_CNT and HISI_PCIE_EXT_CNT are allocated! > > So for example, doing this (i.e. without the '-g'): > > $ perf stat -e hisi_pcie0_0/latency/ -e hisi_pcie0_0/rx_memory_read/ > > would fail because the first event would allocate both of the counters. I'm confused with this situation when getting rid of subevent: $ perf stat -e hisi_pcie0_0/latency/ -e hisi_pcie0_0/rx_memory_read/ In this case, driver checks the relationship of "latency" and "rx_memory_read" in pmu->add() function and return a -EINVAL, but this seems lead to time division multiplexing. if (event->pmu->add(event, PERF_EF_START)) { perf_event_set_state(event, PERF_EVENT_STATE_INACTIVE); event->oncpu = -1; ret = -EAGAIN; goto out; } ... out: perf_pmu_enable(event->pmu); This result doesn't meet our expection, do I miss something here? How about add an array to record events and check the relationship in event_init() function? It seems that perf stat could only failed when driver return invalid value in pmu->event_init() function. Thanks, Qi > > All you need to do is check the counter scheduling constraints when > accepting an event group in the driver. No need for subevents at all. > > Does that make sense? > > Will > . >
next prev parent reply other threads:[~2021-08-04 7:30 UTC|newest] Thread overview: 6+ messages / expand[flat|nested] mbox.gz Atom feed top 2021-07-28 8:09 [PATCH v8 0/2] drivers/perf: hisi: Add support for " Qi Liu 2021-07-28 8:09 ` [PATCH v8 1/2] docs: perf: Add description for HiSilicon PCIe PMU driver Qi Liu 2021-07-28 8:09 ` [PATCH v8 2/2] drivers/perf: hisi: Add driver for HiSilicon PCIe PMU Qi Liu 2021-08-02 10:03 ` Will Deacon 2021-08-04 7:29 ` liuqi (BA) [this message] 2021-08-13 14:40 ` Will Deacon
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=a56266c4-c434-f078-6027-f30c021bd593@huawei.com \ --to=liuqi115@huawei.com \ --cc=bhelgaas@google.com \ --cc=linux-arm-kernel@lists.infradead.org \ --cc=linux-kernel@vger.kernel.org \ --cc=linux-pci@vger.kernel.org \ --cc=linuxarm@huawei.com \ --cc=mark.rutland@arm.com \ --cc=will@kernel.org \ --cc=zhangshaokun@hisilicon.com \ --subject='Re: [PATCH v8 2/2] drivers/perf: hisi: Add driver for HiSilicon PCIe PMU' \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: link
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).