| Message ID | 20240204074527.47110-3-yangyicong@huawei.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Several updates for HiSilicon PCIe PMU driver | expand |
On Sun, 4 Feb 2024 15:45:22 +0800 Yicong Yang <yangyicong@huawei.com> wrote: > From: Yicong Yang <yangyicong@hisilicon.com> > > The metric counting shows incorrect results if the events in the > metric group using the same event but different filter options. > This is because we only judge the event code to decide whether > the event in the metric group should share the same hardware > counter, but ignore the settings of the filter. > > For example, on a platform of 2 ports 0x1 and 0x2 but only port > 0x1 has a downstream PCIe NVME device. The metric counting > shows both ports have the same counts because we misassign these > two events to one same hardware counter: > [root@localhost perf-iostat]# ./perf stat -e '{hisi_pcie0_core1/event=0x0104,port=0x2/,hisi_pcie0_core1/event=0x0104,port=0x1/}' > > Performance counter stats for 'system wide': > > 7907484924 hisi_pcie0_core1/event=0x0104,port=0x2/ > 7907484924 hisi_pcie0_core1/event=0x0104,port=0x1/ > > 10.153863691 seconds time elapsed > > Fix this by using the whole config rather than the event only > to judge whether two events are the same and should share the > same hardware counter. With this patch, the metric counting in > the above case tends to be corrected: > > [root@localhost perf-iostat]# ./perf stat -e '{hisi_pcie0_core1/event=0x0104,port=0x2/,hisi_pcie0_core1/event=0x0104,port=0x1/}' > > Performance counter stats for 'system wide': > > 0 hisi_pcie0_core1/event=0x0104,port=0x2/ > 8123122077 hisi_pcie0_core1/event=0x0104,port=0x1/ > > 10.152875631 seconds time elapsed > > Fixes: 8404b0fbc7fb ("drivers/perf: hisi: Add driver for HiSilicon PCIe PMU") > Signed-off-by: Yicong Yang <yangyicong@hisilicon.com> > --- > drivers/perf/hisilicon/hisi_pcie_pmu.c | 7 ++++++- > 1 file changed, 6 insertions(+), 1 deletion(-) > > diff --git a/drivers/perf/hisilicon/hisi_pcie_pmu.c b/drivers/perf/hisilicon/hisi_pcie_pmu.c > index 11a819cd07f2..9623bed93876 100644 > --- a/drivers/perf/hisilicon/hisi_pcie_pmu.c > +++ b/drivers/perf/hisilicon/hisi_pcie_pmu.c > @@ -314,10 +314,15 @@ static bool hisi_pcie_pmu_valid_filter(struct perf_event *event, > return true; > } > > +/* > + * Check Whether two events share the same config. The same config means not > + * only the event code, but also the filter settings of the two events are > + * the same. > + */ > static bool hisi_pcie_pmu_cmp_event(struct perf_event *target, > struct perf_event *event) > { > - return hisi_pcie_get_real_event(target) == hisi_pcie_get_real_event(event); > + return hisi_pcie_pmu_get_filter(target) == hisi_pcie_pmu_get_filter(event); This is why I looked more closely at what get_filter() was doing in previous patch. Not obvious from the naming here that this compares the event + filters rather than now just comparing the filters. > } > > static bool hisi_pcie_pmu_validate_event_group(struct perf_event *event)
diff --git a/drivers/perf/hisilicon/hisi_pcie_pmu.c b/drivers/perf/hisilicon/hisi_pcie_pmu.c index 11a819cd07f2..9623bed93876 100644 --- a/drivers/perf/hisilicon/hisi_pcie_pmu.c +++ b/drivers/perf/hisilicon/hisi_pcie_pmu.c @@ -314,10 +314,15 @@ static bool hisi_pcie_pmu_valid_filter(struct perf_event *event, return true; } +/* + * Check Whether two events share the same config. The same config means not + * only the event code, but also the filter settings of the two events are + * the same. + */ static bool hisi_pcie_pmu_cmp_event(struct perf_event *target, struct perf_event *event) { - return hisi_pcie_get_real_event(target) == hisi_pcie_get_real_event(event); + return hisi_pcie_pmu_get_filter(target) == hisi_pcie_pmu_get_filter(event); } static bool hisi_pcie_pmu_validate_event_group(struct perf_event *event)