diff mbox series

[v2,6/8] drivers/perf: hisi_pcie: Relax the check on related events

Message ID 20240223103359.18669-7-yangyicong@huawei.com (mailing list archive)
State New, archived
Headers show
Series drivers/perf: hisi_pcie: Several updates for HiSilicon PCIe PMU driver | expand

Commit Message

Yicong Yang Feb. 23, 2024, 10:33 a.m. UTC
From: Junhao He <hejunhao3@huawei.com>

If we use two events with the same filter and related event type
(see the following example), the driver check whether they are related
events and are in the same group, otherwise the function
hisi_pcie_pmu_find_related_event() return -EINVAL, then the 2nd event
cannot count but the 1st event is running, although the PCIe PMU has
other idle counters.

In this case, The perf event scheduler will make the two events to
multiplex a counter, if the user use the formula
(1st event_value / 2nd event_value) to calculate the bandwidth, he/she
won't get the correct value, because they are not counting at the
same period.

This patch tries to fix this by making the related events to use
different idle counters if they are not in the same event group.

And finally, I'm going to say. The related events are best used in the
same group [1]. There are two ways to know if they are related events.
a) By event name, such as the latency events "xxx_latency, xxx_cnt" or
bandwidth events "xxx_flux, xxx_time".
b) By event type, such as "event=0xXXXX, event=0x1XXXX".

Use group to count the related events:
  [1] -e "{pmu_name/xxx_latency,port=1/,pmu_name/xxx_cnt,port=1/}"

  example:
    1st event: hisi_pcie0_core1/event=0x804,port=1
    2nd event: hisi_pcie0_core1/event=0x10804,port=1

  test cmd:
    perf stat -e hisi_pcie0_core1/event=0x804,port=1/ \
               -e hisi_pcie0_core1/event=0x10804,port=1/

  before patch:
            25,281      hisi_pcie0_core1/event=0x804,port=1/    (49.91%)
           470,598      hisi_pcie0_core1/event=0x10804,port=1/    (50.09%)

  after patch:
            24,147      hisi_pcie0_core1/event=0x804,port=1/
           474,558      hisi_pcie0_core1/event=0x10804,port=1/

Signed-off-by: Junhao He <hejunhao3@huawei.com>
Signed-off-by: Yicong Yang <yangyicong@hisilicon.com>
---
 drivers/perf/hisilicon/hisi_pcie_pmu.c | 8 ++------
 1 file changed, 2 insertions(+), 6 deletions(-)

Comments

Jonathan Cameron Feb. 26, 2024, 3:16 p.m. UTC | #1
On Fri, 23 Feb 2024 18:33:57 +0800
Yicong Yang <yangyicong@huawei.com> wrote:

> From: Junhao He <hejunhao3@huawei.com>
> 
> If we use two events with the same filter and related event type
> (see the following example), the driver check whether they are related
> events and are in the same group, otherwise the function
> hisi_pcie_pmu_find_related_event() return -EINVAL, then the 2nd event
> cannot count but the 1st event is running, although the PCIe PMU has
> other idle counters.
> 
> In this case, The perf event scheduler will make the two events to
> multiplex a counter, if the user use the formula
> (1st event_value / 2nd event_value) to calculate the bandwidth, he/she
> won't get the correct value, because they are not counting at the
> same period.
> 
> This patch tries to fix this by making the related events to use
> different idle counters if they are not in the same event group.
> 
> And finally, I'm going to say. The related events are best used in the
> same group [1]. There are two ways to know if they are related events.
> a) By event name, such as the latency events "xxx_latency, xxx_cnt" or
> bandwidth events "xxx_flux, xxx_time".
> b) By event type, such as "event=0xXXXX, event=0x1XXXX".
> 
> Use group to count the related events:
>   [1] -e "{pmu_name/xxx_latency,port=1/,pmu_name/xxx_cnt,port=1/}"
> 
>   example:
>     1st event: hisi_pcie0_core1/event=0x804,port=1
>     2nd event: hisi_pcie0_core1/event=0x10804,port=1
> 
>   test cmd:
>     perf stat -e hisi_pcie0_core1/event=0x804,port=1/ \
>                -e hisi_pcie0_core1/event=0x10804,port=1/
> 
>   before patch:
>             25,281      hisi_pcie0_core1/event=0x804,port=1/    (49.91%)
>            470,598      hisi_pcie0_core1/event=0x10804,port=1/    (50.09%)
> 
>   after patch:
>             24,147      hisi_pcie0_core1/event=0x804,port=1/
>            474,558      hisi_pcie0_core1/event=0x10804,port=1/
> 
> Signed-off-by: Junhao He <hejunhao3@huawei.com>
> Signed-off-by: Yicong Yang <yangyicong@hisilicon.com>

Reviewed-by: Jonathan Cameron <Jonathan.Cameron@huwei.com>

> ---
>  drivers/perf/hisilicon/hisi_pcie_pmu.c | 8 ++------
>  1 file changed, 2 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/perf/hisilicon/hisi_pcie_pmu.c b/drivers/perf/hisilicon/hisi_pcie_pmu.c
> index b2dde7559639..5b15f3698188 100644
> --- a/drivers/perf/hisilicon/hisi_pcie_pmu.c
> +++ b/drivers/perf/hisilicon/hisi_pcie_pmu.c
> @@ -409,14 +409,10 @@ static int hisi_pcie_pmu_find_related_event(struct hisi_pcie_pmu *pcie_pmu,
>  		if (!sibling)
>  			continue;
>  
> -		if (!hisi_pcie_pmu_cmp_event(sibling, event))
> -			continue;
> -
>  		/* Related events must be used in group */
> -		if (sibling->group_leader == event->group_leader)
> +		if (hisi_pcie_pmu_cmp_event(sibling, event) &&
> +		    sibling->group_leader == event->group_leader)
>  			return idx;
> -		else
> -			return -EINVAL;
>  	}
>  
>  	return idx;
diff mbox series

Patch

diff --git a/drivers/perf/hisilicon/hisi_pcie_pmu.c b/drivers/perf/hisilicon/hisi_pcie_pmu.c
index b2dde7559639..5b15f3698188 100644
--- a/drivers/perf/hisilicon/hisi_pcie_pmu.c
+++ b/drivers/perf/hisilicon/hisi_pcie_pmu.c
@@ -409,14 +409,10 @@  static int hisi_pcie_pmu_find_related_event(struct hisi_pcie_pmu *pcie_pmu,
 		if (!sibling)
 			continue;
 
-		if (!hisi_pcie_pmu_cmp_event(sibling, event))
-			continue;
-
 		/* Related events must be used in group */
-		if (sibling->group_leader == event->group_leader)
+		if (hisi_pcie_pmu_cmp_event(sibling, event) &&
+		    sibling->group_leader == event->group_leader)
 			return idx;
-		else
-			return -EINVAL;
 	}
 
 	return idx;