Skip to contentSkip to conversation

perf: Add PERF_SAMPLE_PHYS_ADDR

PATCH Superseded [email protected] 5 participants 10 messages Aug 23, 2017 – Aug 31, 2017

Subsystems (3)

v6 of 8 Superseded Latest is v8 →

lore
Share & tools
lore
#0 lore raw

PATCH v6 1/1 perf: Add PERF_SAMPLE_PHYS_ADDR

From: Kan Liang <[email protected]>

For understanding how the workload maps to memory channels and hardware behavior, it's very important to collect address maps with physical addresses. For example, 3D XPoint access can only be found by filtering the physical address.
However, perf doesn't collect physical address information in sampling.

The virtual address which stored in data->addr can be used to calculate the physical address.
For kernel direct mapping addresses, virt_to_phys is used to convert the virtual addresses to physical address.
For user virtual addresses, __get_user_pages_fast is used to walk the pages tables for user physical address.
This does not work for vmalloc addresses. Right now these are not resolved, but code to do that could be added.
For security, the physical address can only be exposed to root or privileged user.
A new sample type PERF_SAMPLE_PHYS_ADDR is introduced to expose the physical addresses.

---

This patch is kernel patch.
The user space patch can be found here.
https://www.spinics.net/lists/kernel/msg2587093.html

Changes since V5
- Move virt_to_phys to generic code (PeterZ)

 arch/x86/events/perf_event.h    |  2 +-
 include/linux/perf_event.h      |  2 ++
 include/uapi/linux/perf_event.h |  4 +++-
 kernel/events/core.c            | 47 +++++++++++++++++++++++++++++++++++++++++
 4 files changed, 53 insertions(+), 2 deletions(-)
arch/x86/events/perf_event.h +1 -1
index 476aec3..65bb91e 100644
@@ -91,7 +91,7 @@ struct amd_nb {
 	(PERF_SAMPLE_IP | PERF_SAMPLE_TID | PERF_SAMPLE_ADDR | \
 	PERF_SAMPLE_ID | PERF_SAMPLE_CPU | PERF_SAMPLE_STREAM_ID | \
 	PERF_SAMPLE_DATA_SRC | PERF_SAMPLE_IDENTIFIER | \
-	PERF_SAMPLE_TRANSACTION)
+	PERF_SAMPLE_TRANSACTION | PERF_SAMPLE_PHYS_ADDR)
 
 /*
  * A debug store configuration.
include/linux/perf_event.h +2 -0
index b14095b..74fb87e 100644
@@ -944,6 +944,8 @@ struct perf_sample_data {
 
 	struct perf_regs		regs_intr;
 	u64				stack_user_size;
+
+	u64				phys_addr;
 } ____cacheline_aligned;
 
 /* default value for data source */
include/uapi/linux/perf_event.h +3 -1
index 642db5f..cbea02f 100644
@@ -139,8 +139,9 @@ enum perf_event_sample_format {
 	PERF_SAMPLE_IDENTIFIER			= 1U << 16,
 	PERF_SAMPLE_TRANSACTION			= 1U << 17,
 	PERF_SAMPLE_REGS_INTR			= 1U << 18,
+	PERF_SAMPLE_PHYS_ADDR			= 1U << 19,
 
-	PERF_SAMPLE_MAX = 1U << 19,		/* non-ABI */
+	PERF_SAMPLE_MAX = 1U << 20,		/* non-ABI */
 };
 
 /*
@@ -814,6 +815,7 @@ enum perf_event_type {
 	 *	{ u64			transaction; } && PERF_SAMPLE_TRANSACTION
 	 *	{ u64			abi; # enum perf_sample_regs_abi
 	 *	  u64			regs[weight(mask)]; } && PERF_SAMPLE_REGS_INTR
+	 *	{ u64			phys_addr;} && PERF_SAMPLE_PHYS_ADDR
 	 * };
 	 */
 	PERF_RECORD_SAMPLE			= 9,
kernel/events/core.c +47 -0

77 lines not on this page · raw message

Signature · 1 line
2.4.3
maintainer
#1 lore raw

Michael, Maddy,

Since PPC implements PERF_SAMPLE_ADDR this affects you guys too, please have a look.

Quoted 8 lines · On Wed, Aug 23, 2017 at 10:22:46AM -0400, [email protected] wrote:

From: Kan Liang <[email protected]>

For understanding how the workload maps to memory channels and hardware behavior, it's very important to collect address maps with physical addresses. For example, 3D XPoint access can only be found by filtering the physical address.
However, perf doesn't collect physical address information in sampling.

Quoted 12 lines

The virtual address which stored in data->addr can be used to calculate the physical address.
For kernel direct mapping addresses, virt_to_phys is used to convert the virtual addresses to physical address.
For user virtual addresses, __get_user_pages_fast is used to walk the pages tables for user physical address.
This does not work for vmalloc addresses. Right now these are not resolved, but code to do that could be added.
For security, the physical address can only be exposed to root or privileged user.
A new sample type PERF_SAMPLE_PHYS_ADDR is introduced to expose the physical addresses.

Please reflow that..

Quoted 114 lines

---

This patch is kernel patch.
The user space patch can be found here.
https://www.spinics.net/lists/kernel/msg2587093.html

Changes since V5
- Move virt_to_phys to generic code (PeterZ)

 arch/x86/events/perf_event.h    |  2 +-
 include/linux/perf_event.h      |  2 ++
 include/uapi/linux/perf_event.h |  4 +++-
 kernel/events/core.c            | 47 +++++++++++++++++++++++++++++++++++++++++
 4 files changed, 53 insertions(+), 2 deletions(-)
arch/x86/events/perf_event.h +1 -1
index 476aec3..65bb91e 100644
@@ -91,7 +91,7 @@ struct amd_nb {
 	(PERF_SAMPLE_IP | PERF_SAMPLE_TID | PERF_SAMPLE_ADDR | \
 	PERF_SAMPLE_ID | PERF_SAMPLE_CPU | PERF_SAMPLE_STREAM_ID | \
 	PERF_SAMPLE_DATA_SRC | PERF_SAMPLE_IDENTIFIER | \
-	PERF_SAMPLE_TRANSACTION)
+	PERF_SAMPLE_TRANSACTION | PERF_SAMPLE_PHYS_ADDR)
 
 /*
  * A debug store configuration.
include/linux/perf_event.h +2 -0
index b14095b..74fb87e 100644
@@ -944,6 +944,8 @@ struct perf_sample_data {
 
 	struct perf_regs		regs_intr;
 	u64				stack_user_size;
+
+	u64				phys_addr;
 } ____cacheline_aligned;
 
 /* default value for data source */
include/uapi/linux/perf_event.h +3 -1
index 642db5f..cbea02f 100644
@@ -139,8 +139,9 @@ enum perf_event_sample_format {
 	PERF_SAMPLE_IDENTIFIER			= 1U << 16,
 	PERF_SAMPLE_TRANSACTION			= 1U << 17,
 	PERF_SAMPLE_REGS_INTR			= 1U << 18,
+	PERF_SAMPLE_PHYS_ADDR			= 1U << 19,
 
-	PERF_SAMPLE_MAX = 1U << 19,		/* non-ABI */
+	PERF_SAMPLE_MAX = 1U << 20,		/* non-ABI */
 };
 
 /*
@@ -814,6 +815,7 @@ enum perf_event_type {
 	 *	{ u64			transaction; } && PERF_SAMPLE_TRANSACTION
 	 *	{ u64			abi; # enum perf_sample_regs_abi
 	 *	  u64			regs[weight(mask)]; } && PERF_SAMPLE_REGS_INTR
+	 *	{ u64			phys_addr;} && PERF_SAMPLE_PHYS_ADDR
 	 * };
 	 */
 	PERF_RECORD_SAMPLE			= 9,
kernel/events/core.c +27 -0
index d704e23..b991af3 100644
@@ -1570,6 +1570,9 @@ static void __perf_event_header_size(struct perf_event *event, u64 sample_type)
 	if (sample_type & PERF_SAMPLE_TRANSACTION)
 		size += sizeof(data->txn);
 
+	if (sample_type & PERF_SAMPLE_PHYS_ADDR)
+		size += sizeof(data->phys_addr);
+
 	event->header_size = size;
 }
 
@@ -6012,6 +6015,9 @@ void perf_output_sample(struct perf_output_handle *handle,
 		}
 	}
 
+	if (sample_type & PERF_SAMPLE_PHYS_ADDR)
+		perf_output_put(handle, data->phys_addr);
+
 	if (!event->attr.watermark) {
 		int wakeup_events = event->attr.wakeup_events;
 
@@ -6027,6 +6033,38 @@ void perf_output_sample(struct perf_output_handle *handle,
 	}
 }
 
+static u64 perf_virt_to_phys(u64 virt)
+{
+	u64 phys_addr = 0;
+	struct page *p = NULL;
+
+	if (!virt)
+		return 0;
+
+	if (virt >= TASK_SIZE) {
+		/* If it's vmalloc()d memory, leave phys_addr as 0 */
+		if (virt_addr_valid(virt) &&
+		    !(virt >= VMALLOC_START && virt < VMALLOC_END))
+			phys_addr = (u64)virt_to_phys((void *)(uintptr_t)virt);
+	} else {
+		/*
+		 * Walking the pages tables for user address.
+		 * Interrupts are disabled, so it prevents any tear down
+		 * of the page tables.
+		 * Try IRQ-safe __get_user_pages_fast first.
+		 * If failed, leave phys_addr as 0.
+		 */

kernel/events/core.c void perf_output_sample(struct perf_output_handle *handle, · line 6066

Quoted 10 lines
kernel/events/core.c +10 -0
+		if ((current->mm != NULL) &&
+		    (__get_user_pages_fast(virt, 1, 0, &p) == 1))
+			phys_addr = page_to_phys(p) + virt % PAGE_SIZE;
+
+		if (p)
+			put_page(p);
+	}
+
+	return phys_addr;
+}

Michael, does this work for PPC as is?

kernel/events/core.c void perf_prepare_sample(struct perf_event_header *header, · line 6188

Quoted 11 lines
kernel/events/core.c +4 -0
+
 void perf_prepare_sample(struct perf_event_header *header,
 			 struct perf_sample_data *data,
 			 struct perf_event *event,
@@ -6145,6 +6183,9 @@ void perf_prepare_sample(struct perf_event_header *header,
 
 		header->size += size;
 	}
+
+	if (sample_type & PERF_SAMPLE_PHYS_ADDR)
+		data->phys_addr = perf_virt_to_phys(data->addr);

Only problem with this now is that it requires SAMPLE_ADDR to also be set in order to obtain data->addr.

Either fix all to set data->attr for (SAMPLE_ADDR || SAMPLE_PHYS_ADDR) or mandate SAMPLE_ADDR when SAMPLE_PHYS_ADDR.

Quoted 19 lines
kernel/events/core.c +6 -0
 }
 
 static void __always_inline
@@ -9892,6 +9933,12 @@ SYSCALL_DEFINE5(perf_event_open,
 			return -EINVAL;
 	}
 
+	/* Only privileged users can get kernel addresses */
+	if ((attr.sample_type & PERF_SAMPLE_PHYS_ADDR) &&
+	    perf_paranoid_kernel() &&
+	    !capable(CAP_SYS_ADMIN))
+		return -EACCES;
+
 	if (!attr.sample_max_stack)
 		attr.sample_max_stack = sysctl_perf_event_max_stack;
 
Signature · 1 line
2.4.3
maintainer
#2 lore raw
Quoted 11 lines · Peter Zijlstra

kernel/events/core.c void perf_prepare_sample(struct perf_event_header *header, · line 6188

Quoted 7 lines
kernel/events/core.c +3 -0
@@ -6145,6 +6183,9 @@ void perf_prepare_sample(struct perf_event_header *header,
 
 		header->size += size;
 	}
+
+	if (sample_type & PERF_SAMPLE_PHYS_ADDR)
+		data->phys_addr = perf_virt_to_phys(data->addr);

Only problem with this now is that it requires SAMPLE_ADDR to also be set in order to obtain data->addr.

Quoted 2 lines

Either fix all to set data->attr for (SAMPLE_ADDR || SAMPLE_PHYS_ADDR) or mandate SAMPLE_ADDR when SAMPLE_PHYS_ADDR.

I think the former suggestion is better, as it allows for smaller samples.

#3 lore raw
Quoted 12 lines · Peter Zijlstra
Quoted 11 lines · Peter Zijlstra

kernel/events/core.c void perf_prepare_sample(struct perf_event_header *header, · line 6188

Quoted 7 lines
kernel/events/core.c +3 -0
@@ -6145,6 +6183,9 @@ void perf_prepare_sample(struct perf_event_header *header,
 
            header->size += size;
    }
+
+   if (sample_type & PERF_SAMPLE_PHYS_ADDR)
+           data->phys_addr = perf_virt_to_phys(data->addr);

Only problem with this now is that it requires SAMPLE_ADDR to also be set in order to obtain data->addr.

Quoted 5 lines
Quoted 2 lines

Either fix all to set data->attr for (SAMPLE_ADDR || SAMPLE_PHYS_ADDR) or mandate SAMPLE_ADDR when SAMPLE_PHYS_ADDR.

I think the former suggestion is better, as it allows for smaller samples.

Agreed. I may only care about physical addresses, so SAMPLE_PHYS_ADDR should suffice from the user.
The kernel must save the virtual address in data->addr, and then convert it.

#4 lore raw

Em Wed, Aug 23, 2017 at 10:00:37AM -0700, Stephane Eranian escreveu:

Quoted 16 lines · Peter Zijlstra
Quoted 15 lines · Peter Zijlstra
Quoted 13 lines

kernel/events/core.c void perf_prepare_sample(struct perf_event_header *header, · line 6188

Quoted 7 lines
kernel/events/core.c +3 -0
@@ -6145,6 +6183,9 @@ void perf_prepare_sample(struct perf_event_header *header,
 
            header->size += size;
    }
+
+   if (sample_type & PERF_SAMPLE_PHYS_ADDR)
+           data->phys_addr = perf_virt_to_phys(data->addr);

Only problem with this now is that it requires SAMPLE_ADDR to also be set in order to obtain data->addr.

Either fix all to set data->attr for (SAMPLE_ADDR || SAMPLE_PHYS_ADDR) or mandate SAMPLE_ADDR when SAMPLE_PHYS_ADDR.

Quoted 6 lines
Quoted 2 lines

I think the former suggestion is better, as it allows for smaller samples.

Agreed. I may only care about physical addresses, so SAMPLE_PHYS_ADDR should suffice from the user.
The kernel must save the virtual address in data->addr, and then convert it.

Stephane, have you took the time to test this together with Kan's latest patchkit for tools/perf? v2? If so can I have your Tested-by or just an Acked-by?

Thanks,

- Arnaldo

author
#5 lore raw
In reply to Peter Zijlstra
Quoted 13 lines · Peter Zijlstra
Quoted 12 lines

Review void perf_prepare_sample(struct · line 6189

Quoted 8 lines
(quoted diff) +3 -0
@@ -6145,6 +6183,9 @@ void perf_prepare_sample(struct
 perf_event_header *header,
 
 		header->size += size;
 	}
+
+	if (sample_type & PERF_SAMPLE_PHYS_ADDR)
+		data->phys_addr = perf_virt_to_phys(data->addr);

Only problem with this now is that it requires SAMPLE_ADDR to also be set in order to obtain data->addr.

Quoted 4 lines
Quoted 2 lines

Either fix all to set data->attr for (SAMPLE_ADDR || SAMPLE_PHYS_ADDR) or mandate SAMPLE_ADDR when SAMPLE_PHYS_ADDR.

I think the former suggestion is better, as it allows for smaller samples.

For the latter, it's easy to be implemented, and already in the perf tool patch. But yes, it will bring extra overhead, if the user doesn't care about the virtual address.

For the former, do you mean data->type or event->attr? I don't think we can set either of them.
It will impact the perf_output_sample and perf_event__header_size.

For x86, I think we can do something as below. But I'm not sure other architectures.

arch/x86/events/intel/ds.c +1 -1
index a322fed..9bf29dc 100644
@@ -1175,7 +1175,7 @@ static void setup_pebs_sample_data(struct perf_event *event,
        else
                regs->flags &= ~PERF_EFLAGS_EXACT;
 
-       if ((sample_type & PERF_SAMPLE_ADDR) &&
+       if ((sample_type & (PERF_SAMPLE_ADDR | PERF_SAMPLE_PHYS_ADDR) &&
            x86_pmu.intel_cap.pebs_format >= 1)
                data->addr = pebs->dla;
 

Thanks,
Kan

maintainer
#6 lore raw
Quoted 1 line · Kan

For x86, I think we can do something as below. But I'm not sure other architectures.

If you'd done: git grep PERF_SAMPLE_ADDR, you'd have found:

arch/powerpc/perf/core-book3s.c: if (event->attr.sample_type & PERF_SAMPLE_ADDR)

Which suggests something rather similar..

arch/x86/events/intel/ds.c static void setup_pebs_sample_data(struct perf_event *event, · line 1180

Quoted 12 lines
arch/x86/events/intel/ds.c +1 -1
index a322fed..9bf29dc 100644
@@ -1175,7 +1175,7 @@ static void setup_pebs_sample_data(struct perf_event *event,
        else
                regs->flags &= ~PERF_EFLAGS_EXACT;
 
-       if ((sample_type & PERF_SAMPLE_ADDR) &&
+       if ((sample_type & (PERF_SAMPLE_ADDR | PERF_SAMPLE_PHYS_ADDR) &&
            x86_pmu.intel_cap.pebs_format >= 1)
                data->addr = pebs->dla;
arch/powerpc/perf/core-book3s.c +1 -1
index 6c2d4168daec..c833fb26f0bb 100644
@@ -2039,7 +2039,7 @@ static void record_and_restart(struct perf_event *event, unsigned long val,
 
 		perf_sample_data_init(&data, ~0ULL, event->hw.last_period);
 
-		if (event->attr.sample_type & PERF_SAMPLE_ADDR)
+		if (event->attr.sample_type & (PERF_SAMPLE_ADDR | PERF_SAMPLE_PHYS_ADDR))
 			perf_get_data_addr(regs, &data.addr);
 
 		if (event->attr.sample_type & PERF_SAMPLE_BRANCH_STACK) {
#7 lore raw
In reply to Peter Zijlstra
Quoted 33 lines · Peter Zijlstra
Quoted 33 lines
kernel/events/core.c +25 -0
index d704e23..b991af3 100644
@@ -6027,6 +6033,38 @@ void perf_output_sample(struct perf_output_handle *handle,
 	}
 }
 
+static u64 perf_virt_to_phys(u64 virt)
+{
+	u64 phys_addr = 0;
+	struct page *p = NULL;
+
+	if (!virt)
+		return 0;
+
+	if (virt >= TASK_SIZE) {
+		/* If it's vmalloc()d memory, leave phys_addr as 0 */
+		if (virt_addr_valid(virt) &&
+		    !(virt >= VMALLOC_START && virt < VMALLOC_END))
+			phys_addr = (u64)virt_to_phys((void *)(uintptr_t)virt);
+	} else {
+		/*
+		 * Walking the pages tables for user address.
+		 * Interrupts are disabled, so it prevents any tear down
+		 * of the page tables.
+		 * Try IRQ-safe __get_user_pages_fast first.
+		 * If failed, leave phys_addr as 0.
+		 */
+		if ((current->mm != NULL) &&
+		    (__get_user_pages_fast(virt, 1, 0, &p) == 1))
+			phys_addr = page_to_phys(p) + virt % PAGE_SIZE;
+
Quoted 8 lines

kernel/events/core.c void perf_output_sample(struct perf_output_handle *handle, · line 6066

Quoted 6 lines
kernel/events/core.c +6 -0
+		if (p)
+			put_page(p);
+	}
+
+	return phys_addr;
+}

Michael, does this work for PPC as is?

I think so.

We have about 8 MMUs and 32-bit and 64-bit so it's a bit hard to say for sure.

I'm pretty sure virt_addr_valid() will exclude everything except the linear mapping for us, so the vmalloc check is redundant but that's fine. So that looks safe AFAICS.

I'm not an expert on GUP fast, but AFAIK there's nothing special required for us.

cheers

maintainer
#8 lore raw
Quoted 43 lines · Michael Ellerman
Quoted 33 lines · Peter Zijlstra
Quoted 33 lines
kernel/events/core.c +25 -0
index d704e23..b991af3 100644
@@ -6027,6 +6033,38 @@ void perf_output_sample(struct perf_output_handle *handle,
 	}
 }
 
+static u64 perf_virt_to_phys(u64 virt)
+{
+	u64 phys_addr = 0;
+	struct page *p = NULL;
+
+	if (!virt)
+		return 0;
+
+	if (virt >= TASK_SIZE) {
+		/* If it's vmalloc()d memory, leave phys_addr as 0 */
+		if (virt_addr_valid(virt) &&
+		    !(virt >= VMALLOC_START && virt < VMALLOC_END))
+			phys_addr = (u64)virt_to_phys((void *)(uintptr_t)virt);
+	} else {
+		/*
+		 * Walking the pages tables for user address.
+		 * Interrupts are disabled, so it prevents any tear down
+		 * of the page tables.
+		 * Try IRQ-safe __get_user_pages_fast first.
+		 * If failed, leave phys_addr as 0.
+		 */
+		if ((current->mm != NULL) &&
+		    (__get_user_pages_fast(virt, 1, 0, &p) == 1))
+			phys_addr = page_to_phys(p) + virt % PAGE_SIZE;
+
Quoted 8 lines

kernel/events/core.c void perf_output_sample(struct perf_output_handle *handle, · line 6066

Quoted 6 lines
kernel/events/core.c +6 -0
+		if (p)
+			put_page(p);
+	}
+
+	return phys_addr;
+}

Michael, does this work for PPC as is?

Quoted 4 lines

I think so.

We have about 8 MMUs and 32-bit and 64-bit so it's a bit hard to say for sure.

You can limit it to those that include a PMU that supports data->addr, which is book3s, whatever that gets you.

Quoted 6 lines

I'm pretty sure virt_addr_valid() will exclude everything except the linear mapping for us, so the vmalloc check is redundant but that's fine. So that looks safe AFAICS.

I'm not an expert on GUP fast, but AFAIK there's nothing special required for us.

Yeah, gup fast should work for you.

#9 lore raw
Quoted 44 lines · Peter Zijlstra
Quoted 43 lines · Michael Ellerman
Quoted 41 lines · Peter Zijlstra
Quoted 29 lines
kernel/events/core.c +21 -0
index d704e23..b991af3 100644
@@ -6027,6 +6033,38 @@ void perf_output_sample(struct perf_output_handle *handle,
 	}
 }
 
+static u64 perf_virt_to_phys(u64 virt)
+{
+	u64 phys_addr = 0;
+	struct page *p = NULL;
+
+	if (!virt)
+		return 0;
+
+	if (virt >= TASK_SIZE) {
+		/* If it's vmalloc()d memory, leave phys_addr as 0 */
+		if (virt_addr_valid(virt) &&
+		    !(virt >= VMALLOC_START && virt < VMALLOC_END))
+			phys_addr = (u64)virt_to_phys((void *)(uintptr_t)virt);
+	} else {
+		/*
+		 * Walking the pages tables for user address.
+		 * Interrupts are disabled, so it prevents any tear down
+		 * of the page tables.
+		 * Try IRQ-safe __get_user_pages_fast first.
+		 * If failed, leave phys_addr as 0.
+		 */

kernel/events/core.c void perf_output_sample(struct perf_output_handle *handle, · line 6066

Quoted 10 lines
kernel/events/core.c +10 -0
+		if ((current->mm != NULL) &&
+		    (__get_user_pages_fast(virt, 1, 0, &p) == 1))
+			phys_addr = page_to_phys(p) + virt % PAGE_SIZE;
+
+		if (p)
+			put_page(p);
+	}
+
+	return phys_addr;
+}

Michael, does this work for PPC as is?

Quoted 7 lines
Quoted 4 lines

I think so.

We have about 8 MMUs and 32-bit and 64-bit so it's a bit hard to say for sure.

You can limit it to those that include a PMU that supports data->addr, which is book3s, whatever that gets you.

True. That's just hash and radix.

So that looks OK to me. At least OK enough to merge and we'll try and test it ASAP to confirm it works.

cheers