[PATCH] perf/amd/ibs: add phy_addr_only software filter for ibs_op

[PATCH] perf/amd/ibs: add phy_addr_only software filter for ibs_op

[Huang Shijie]

We use the following command to track a process's memory footprint:
  #perf record -e ibs_fetch// -e ibs_op// -p xxx --phys-addr -- sleep 10

In the mysql tests, I catched over 1,804,973 records for ibs_op event.
I found that over 53% records(970,554) are like this:
        ibs_op//:  ffffffffb6017739               0

The physical address is 0 for these records. These records are not needed
by us, and they wasted the CPU cycles.

This patch adds an ibs_op/phy_addr_only term that discards samples
without a valid physical address (dc_phy_addr_valid == 0) in the interrupt
handler. And it avoids the phys_addr=0 noise when used with --phys-data.

Signed-off-by: Huang Shijie <huangsj@hygon.cn>
---
 arch/x86/events/amd/ibs.c | 23 ++++++++++++++++++++++-
 1 file changed, 22 insertions(+), 1 deletion(-)

diff --git a/arch/x86/events/amd/ibs.c b/arch/x86/events/amd/ibs.c
index e5376ad5b2ec..f258375c09bc 100644
--- a/arch/x86/events/amd/ibs.c
+++ b/arch/x86/events/amd/ibs.c
@@ -32,6 +32,7 @@ static u32 ibs_caps;
 
 /* attr.config2 */
 #define IBS_SW_FILTER_MASK	1
+#define IBS_PHY_ADDR_ONLY_MASK	2
 
 /* attr.config1 */
 #define IBS_OP_CONFIG1_LDLAT_MASK		(0xFFFULL <<  0)
@@ -303,6 +304,13 @@ static bool perf_ibs_strmst_event(struct perf_ibs *perf_ibs,
 	       (event->attr.config1 & IBS_OP_CONFIG1_STRMST_MASK);
 }
 
+static bool perf_ibs_phy_addr_only_event(struct perf_ibs *perf_ibs,
+				      struct perf_event *event)
+{
+	return perf_ibs == &perf_ibs_op &&
+	       (event->attr.config2 & IBS_PHY_ADDR_ONLY_MASK);
+}
+
 static int perf_ibs_init(struct perf_event *event)
 {
 	struct hw_perf_event *hwc = &event->hw;
@@ -721,6 +729,7 @@ static struct attribute_group empty_caps_group = {
 PMU_FORMAT_ATTR(rand_en,	"config:57");
 PMU_FORMAT_ATTR(cnt_ctl,	"config:19");
 PMU_FORMAT_ATTR(swfilt,		"config2:0");
+PMU_FORMAT_ATTR(phy_addr_only,	"config2:1");
 PMU_EVENT_ATTR_STRING(l3missonly, fetch_l3missonly, "config:59");
 PMU_EVENT_ATTR_STRING(l3missonly, op_l3missonly, "config:16");
 PMU_EVENT_ATTR_STRING(ldlat, ibs_op_ldlat_format, "config1:0-11");
@@ -890,6 +899,7 @@ cnt_ctl_is_visible(struct kobject *kobj, struct attribute *attr, int i)
 
 static struct attribute *op_attrs[] = {
 	&format_attr_swfilt.attr,
+	&format_attr_phy_addr_only.attr,
 	NULL,
 };
 
@@ -1357,7 +1367,8 @@ static int perf_ibs_get_offset_max(struct perf_ibs *perf_ibs,
 	if (event->attr.sample_type & PERF_SAMPLE_RAW ||
 	    perf_ibs_is_mem_sample_type(perf_ibs, event) ||
 	    perf_ibs_ldlat_event(perf_ibs, event) ||
-	    perf_ibs_fetch_lat_event(perf_ibs, event))
+	    perf_ibs_fetch_lat_event(perf_ibs, event) ||
+	    perf_ibs_phy_addr_only_event(perf_ibs, event))
 		return perf_ibs->offset_max;
 	else if (check_rip)
 		return 3;
@@ -1499,6 +1510,16 @@ static int perf_ibs_handle_irq(struct perf_ibs *perf_ibs, struct pt_regs *iregs)
 		}
 	}
 
+	if (perf_ibs_phy_addr_only_event(perf_ibs, event)) {
+		union ibs_op_data3 op_data3;
+
+		op_data3.val = ibs_data.regs[ibs_op_msr_idx(MSR_AMD64_IBSOPDATA3)];
+		if (!op_data3.dc_phy_addr_valid) {
+			throttle = perf_event_account_interrupt(event);
+			goto out;
+		}
+	}
+
 	if (perf_ibs_fetch_lat_event(perf_ibs, event)) {
 		union ibs_fetch_ctl fetch_ctl;
 
-- 
2.53.0

Re: [PATCH] perf/amd/ibs: add phy_addr_only software filter for ibs_op

[Ravi Bangoria]

> We use the following command to track a process's memory footprint:
>   #perf record -e ibs_fetch// -e ibs_op// -p xxx --phys-addr -- sleep 10
> 
> In the mysql tests, I catched over 1,804,973 records for ibs_op event.
> I found that over 53% records(970,554) are like this:
>         ibs_op//:  ffffffffb6017739               0
> 
> The physical address is 0 for these records. These records are not needed
> by us, and they wasted the CPU cycles.
> 
> This patch adds an ibs_op/phy_addr_only term that discards samples
> without a valid physical address (dc_phy_addr_valid == 0) in the interrupt
> handler. And it avoids the phys_addr=0 noise when used with --phys-data.

Why not simply use the BPF filter?

  $ perf record -e ibs_op// --phys-data --filter 'phys_addr != 0' ...

Thanks,
Ravi

Re: [PATCH] perf/amd/ibs: add phy_addr_only software filter for ibs_op

[Huang Shijie]

Hi Ravi,

Thanks for taking a look.

> Why not simply use the BPF filter?
>   $ perf record -e ibs_op// --phys-data --filter 'phys_addr != 0' ...

I did try the BPF filter.

Measurements below are system-wide ibs_op sampling for 5s on a
384-CPU Hygon box under a 400-thread sysbench oltp_read_only load
at three sampling periods:

    -c     filter        valid samples    
  ------  ------------  ---------- 
  100000  phy_addr_only  19,345,907 
  100000  BPF filter     17,939,819 

   30000  phy_addr_only  54,670,617 
   30000  BPF filter     47,926,784 

   10000  phy_addr_only  61,606,488 
   10000  BPF filter     40,826,086 


The phy_addr_only produces more *valid* samples than the BPF filter.

Thanks,
Huang Shijie

Re: [PATCH] perf/amd/ibs: add phy_addr_only software filter for ibs_op

[Ravi Bangoria]

Huang,

>> Why not simply use the BPF filter?
>>   $ perf record -e ibs_op// --phys-data --filter 'phys_addr != 0' ...
> 
> I did try the BPF filter.
> 
> Measurements below are system-wide ibs_op sampling for 5s on a
> 384-CPU Hygon box under a 400-thread sysbench oltp_read_only load
> at three sampling periods:
> 
>     -c     filter        valid samples    
>   ------  ------------  ---------- 
>   100000  phy_addr_only  19,345,907 
>   100000  BPF filter     17,939,819 
> 
>    30000  phy_addr_only  54,670,617 
>    30000  BPF filter     47,926,784 
> 
>    10000  phy_addr_only  61,606,488 
>    10000  BPF filter     40,826,086 
> 
> 
> The phy_addr_only produces more *valid* samples than the BPF filter.

Can you please confirm if those additional samples are genuinely valid.

Thanks,
Ravi

Re: [PATCH] perf/amd/ibs: add phy_addr_only software filter for ibs_op

[Huang Shijie]

Hi Ravi,

> Can you please confirm if those additional samples are genuinely valid.

Yes.  
To answer this properly I re-ran the mysql test,
and verified the physical address of every captured sample against
the System RAM ranges in /proc/iomem.

Setup: 384-CPU Hygon box, v7.3-based kernel, MySQL 8 under sysbench
oltp_read_only, 400 threads.  Each round is a fresh 60s
sysbench run; after 15s of warmup, perf records ibs_op for 3s:

  $ perf record -e ibs_op/phy_addr_only=1/ -c 100000 --phys-data \
        -p <mysqld>                    # my patch
  $ perf record -e ibs_op// --phys-data --filter 'phys_addr != 0' \
        -c 100000 -p <mysqld>          # your suggestion

Rounds alternate p1 -> b1 -> p2 -> b2 -> p3 -> b3 so both methods see
the same load conditions.  Each record's phys_addr is then dumped with
"perf script -F pid,tid,phys_addr" and bisect-checked against the
System RAM ranges (read from /proc/iomem as root).

                     samples    phys_addr==0    non-zero in
    round  filter    captured   (valid=1,MSR=0)  System RAM
    -----  --------  ---------  ---------------  -----------
     p1    phy_only  15710042        0.10 %          98.61 %
     b1    bpf       14041851        0.00 %          98.55 %
     p2    phy_only  15580497        0.10 %          98.56 %
     b2    bpf       15416187        0.00 %          98.58 %
     p3    phy_only  15732582        0.10 %          98.59 %
     b3    bpf       15447397        0.00 %          98.58 %

Two observations:
1. The validity ratio is identical for both methods.  Of the non-zero
   addresses phy_addr_only delivers, 98.6 % fall inside System RAM --
   the same ratio the BPF-filtered set shows. 

2. The only set-level difference is 0.10 % of samples where
   dc_phy_addr_valid is set but IBSDCPHYSAD reads 0. 

Thanks,
Huang Shijie