* Re: [PATCH] perf/x86/intel/bts: allocate bts_ctx only if necessary
[not found] <20241030063521.4025-1-lirongqing@baidu.com>
@ 2024-10-30 7:26 ` Alexander Shishkin
0 siblings, 0 replies; 8+ messages in thread
From: Alexander Shishkin @ 2024-10-30 7:26 UTC (permalink / raw)
To: Li RongQing, peterz, mingo, acme, namhyung, mark.rutland, jolsa,
irogers, adrian.hunter, kan.liang, tglx, bp, dave.hansen, x86,
hpa, linux-perf-users, linux-kernel, zhiquan1.li,
alexander.shishkin
Cc: Li RongQing
Li RongQing <lirongqing@baidu.com> writes:
> Avoid unnecessary per-CPU memory allocation on unsupported CPUs,
> this can save 12K memory for every CPU
Looks reasonable, but have you tested it? This driver is in serious
danger of bitrot ever since KPTI. It might save even more if we simply
remove the whole thing.
Regards,
--
Alex
^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH] perf/x86/intel/bts: allocate bts_ctx only if necessary
@ 2025-01-22 7:41 lirongqing
2025-01-23 18:36 ` Adrian Hunter
` (2 more replies)
0 siblings, 3 replies; 8+ messages in thread
From: lirongqing @ 2025-01-22 7:41 UTC (permalink / raw)
To: peterz, mingo, acme, namhyung, mark.rutland, alexander.shishkin,
jolsa, irogers, adrian.hunter, kan.liang, tglx, bp, dave.hansen,
x86, hpa, linux-perf-users, linux-kernel
Cc: Li RongQing
From: Li RongQing <lirongqing@baidu.com>
Avoid unnecessary per-CPU memory allocation on unsupported CPUs,
this can save 12K memory for each CPU
Signed-off-by: Li RongQing <lirongqing@baidu.com>
---
arch/x86/events/intel/bts.c | 22 +++++++++++++---------
1 file changed, 13 insertions(+), 9 deletions(-)
diff --git a/arch/x86/events/intel/bts.c b/arch/x86/events/intel/bts.c
index 8f78b0c..8e09319 100644
--- a/arch/x86/events/intel/bts.c
+++ b/arch/x86/events/intel/bts.c
@@ -36,7 +36,7 @@ enum {
BTS_STATE_ACTIVE,
};
-static DEFINE_PER_CPU(struct bts_ctx, bts_ctx);
+static struct bts_ctx __percpu *bts_ctx;
#define BTS_RECORD_SIZE 24
#define BTS_SAFETY_MARGIN 4080
@@ -231,7 +231,7 @@ bts_buffer_reset(struct bts_buffer *buf, struct perf_output_handle *handle);
static void __bts_event_start(struct perf_event *event)
{
- struct bts_ctx *bts = this_cpu_ptr(&bts_ctx);
+ struct bts_ctx *bts = this_cpu_ptr(bts_ctx);
struct bts_buffer *buf = perf_get_aux(&bts->handle);
u64 config = 0;
@@ -260,7 +260,7 @@ static void __bts_event_start(struct perf_event *event)
static void bts_event_start(struct perf_event *event, int flags)
{
struct cpu_hw_events *cpuc = this_cpu_ptr(&cpu_hw_events);
- struct bts_ctx *bts = this_cpu_ptr(&bts_ctx);
+ struct bts_ctx *bts = this_cpu_ptr(bts_ctx);
struct bts_buffer *buf;
buf = perf_aux_output_begin(&bts->handle, event);
@@ -290,7 +290,7 @@ static void bts_event_start(struct perf_event *event, int flags)
static void __bts_event_stop(struct perf_event *event, int state)
{
- struct bts_ctx *bts = this_cpu_ptr(&bts_ctx);
+ struct bts_ctx *bts = this_cpu_ptr(bts_ctx);
/* ACTIVE -> INACTIVE(PMI)/STOPPED(->stop()) */
WRITE_ONCE(bts->state, state);
@@ -305,7 +305,7 @@ static void __bts_event_stop(struct perf_event *event, int state)
static void bts_event_stop(struct perf_event *event, int flags)
{
struct cpu_hw_events *cpuc = this_cpu_ptr(&cpu_hw_events);
- struct bts_ctx *bts = this_cpu_ptr(&bts_ctx);
+ struct bts_ctx *bts = this_cpu_ptr(bts_ctx);
struct bts_buffer *buf = NULL;
int state = READ_ONCE(bts->state);
@@ -338,7 +338,7 @@ static void bts_event_stop(struct perf_event *event, int flags)
void intel_bts_enable_local(void)
{
- struct bts_ctx *bts = this_cpu_ptr(&bts_ctx);
+ struct bts_ctx *bts = this_cpu_ptr(bts_ctx);
int state = READ_ONCE(bts->state);
/*
@@ -358,7 +358,7 @@ void intel_bts_enable_local(void)
void intel_bts_disable_local(void)
{
- struct bts_ctx *bts = this_cpu_ptr(&bts_ctx);
+ struct bts_ctx *bts = this_cpu_ptr(bts_ctx);
/*
* Here we transition from ACTIVE to INACTIVE;
@@ -450,7 +450,7 @@ bts_buffer_reset(struct bts_buffer *buf, struct perf_output_handle *handle)
int intel_bts_interrupt(void)
{
struct debug_store *ds = this_cpu_ptr(&cpu_hw_events)->ds;
- struct bts_ctx *bts = this_cpu_ptr(&bts_ctx);
+ struct bts_ctx *bts = this_cpu_ptr(bts_ctx);
struct perf_event *event = bts->handle.event;
struct bts_buffer *buf;
s64 old_head;
@@ -518,7 +518,7 @@ static void bts_event_del(struct perf_event *event, int mode)
static int bts_event_add(struct perf_event *event, int mode)
{
- struct bts_ctx *bts = this_cpu_ptr(&bts_ctx);
+ struct bts_ctx *bts = this_cpu_ptr(bts_ctx);
struct cpu_hw_events *cpuc = this_cpu_ptr(&cpu_hw_events);
struct hw_perf_event *hwc = &event->hw;
@@ -605,6 +605,10 @@ static __init int bts_init(void)
return -ENODEV;
}
+ bts_ctx = alloc_percpu(struct bts_ctx);
+ if (!bts_ctx)
+ return -ENOMEM;
+
bts_pmu.capabilities = PERF_PMU_CAP_AUX_NO_SG | PERF_PMU_CAP_ITRACE |
PERF_PMU_CAP_EXCLUSIVE;
bts_pmu.task_ctx_nr = perf_sw_context;
--
2.9.4
^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH] perf/x86/intel/bts: allocate bts_ctx only if necessary
2025-01-22 7:41 lirongqing
@ 2025-01-23 18:36 ` Adrian Hunter
2025-01-23 18:54 ` Liang, Kan
2025-03-05 13:47 ` Jiri Olsa
2 siblings, 0 replies; 8+ messages in thread
From: Adrian Hunter @ 2025-01-23 18:36 UTC (permalink / raw)
To: lirongqing, peterz, mingo, acme, namhyung, mark.rutland,
alexander.shishkin, jolsa, irogers, kan.liang, tglx, bp,
dave.hansen, x86, hpa, linux-perf-users, linux-kernel
On 22/01/25 09:41, lirongqing wrote:
> From: Li RongQing <lirongqing@baidu.com>
>
> Avoid unnecessary per-CPU memory allocation on unsupported CPUs,
> this can save 12K memory for each CPU
>
> Signed-off-by: Li RongQing <lirongqing@baidu.com>
Tested-by: Adrian Hunter <adrian.hunter@intel.com>
> ---
> arch/x86/events/intel/bts.c | 22 +++++++++++++---------
> 1 file changed, 13 insertions(+), 9 deletions(-)
>
> diff --git a/arch/x86/events/intel/bts.c b/arch/x86/events/intel/bts.c
> index 8f78b0c..8e09319 100644
> --- a/arch/x86/events/intel/bts.c
> +++ b/arch/x86/events/intel/bts.c
> @@ -36,7 +36,7 @@ enum {
> BTS_STATE_ACTIVE,
> };
>
> -static DEFINE_PER_CPU(struct bts_ctx, bts_ctx);
> +static struct bts_ctx __percpu *bts_ctx;
>
> #define BTS_RECORD_SIZE 24
> #define BTS_SAFETY_MARGIN 4080
> @@ -231,7 +231,7 @@ bts_buffer_reset(struct bts_buffer *buf, struct perf_output_handle *handle);
>
> static void __bts_event_start(struct perf_event *event)
> {
> - struct bts_ctx *bts = this_cpu_ptr(&bts_ctx);
> + struct bts_ctx *bts = this_cpu_ptr(bts_ctx);
> struct bts_buffer *buf = perf_get_aux(&bts->handle);
> u64 config = 0;
>
> @@ -260,7 +260,7 @@ static void __bts_event_start(struct perf_event *event)
> static void bts_event_start(struct perf_event *event, int flags)
> {
> struct cpu_hw_events *cpuc = this_cpu_ptr(&cpu_hw_events);
> - struct bts_ctx *bts = this_cpu_ptr(&bts_ctx);
> + struct bts_ctx *bts = this_cpu_ptr(bts_ctx);
> struct bts_buffer *buf;
>
> buf = perf_aux_output_begin(&bts->handle, event);
> @@ -290,7 +290,7 @@ static void bts_event_start(struct perf_event *event, int flags)
>
> static void __bts_event_stop(struct perf_event *event, int state)
> {
> - struct bts_ctx *bts = this_cpu_ptr(&bts_ctx);
> + struct bts_ctx *bts = this_cpu_ptr(bts_ctx);
>
> /* ACTIVE -> INACTIVE(PMI)/STOPPED(->stop()) */
> WRITE_ONCE(bts->state, state);
> @@ -305,7 +305,7 @@ static void __bts_event_stop(struct perf_event *event, int state)
> static void bts_event_stop(struct perf_event *event, int flags)
> {
> struct cpu_hw_events *cpuc = this_cpu_ptr(&cpu_hw_events);
> - struct bts_ctx *bts = this_cpu_ptr(&bts_ctx);
> + struct bts_ctx *bts = this_cpu_ptr(bts_ctx);
> struct bts_buffer *buf = NULL;
> int state = READ_ONCE(bts->state);
>
> @@ -338,7 +338,7 @@ static void bts_event_stop(struct perf_event *event, int flags)
>
> void intel_bts_enable_local(void)
> {
> - struct bts_ctx *bts = this_cpu_ptr(&bts_ctx);
> + struct bts_ctx *bts = this_cpu_ptr(bts_ctx);
> int state = READ_ONCE(bts->state);
>
> /*
> @@ -358,7 +358,7 @@ void intel_bts_enable_local(void)
>
> void intel_bts_disable_local(void)
> {
> - struct bts_ctx *bts = this_cpu_ptr(&bts_ctx);
> + struct bts_ctx *bts = this_cpu_ptr(bts_ctx);
>
> /*
> * Here we transition from ACTIVE to INACTIVE;
> @@ -450,7 +450,7 @@ bts_buffer_reset(struct bts_buffer *buf, struct perf_output_handle *handle)
> int intel_bts_interrupt(void)
> {
> struct debug_store *ds = this_cpu_ptr(&cpu_hw_events)->ds;
> - struct bts_ctx *bts = this_cpu_ptr(&bts_ctx);
> + struct bts_ctx *bts = this_cpu_ptr(bts_ctx);
> struct perf_event *event = bts->handle.event;
> struct bts_buffer *buf;
> s64 old_head;
> @@ -518,7 +518,7 @@ static void bts_event_del(struct perf_event *event, int mode)
>
> static int bts_event_add(struct perf_event *event, int mode)
> {
> - struct bts_ctx *bts = this_cpu_ptr(&bts_ctx);
> + struct bts_ctx *bts = this_cpu_ptr(bts_ctx);
> struct cpu_hw_events *cpuc = this_cpu_ptr(&cpu_hw_events);
> struct hw_perf_event *hwc = &event->hw;
>
> @@ -605,6 +605,10 @@ static __init int bts_init(void)
> return -ENODEV;
> }
>
> + bts_ctx = alloc_percpu(struct bts_ctx);
> + if (!bts_ctx)
> + return -ENOMEM;
> +
> bts_pmu.capabilities = PERF_PMU_CAP_AUX_NO_SG | PERF_PMU_CAP_ITRACE |
> PERF_PMU_CAP_EXCLUSIVE;
> bts_pmu.task_ctx_nr = perf_sw_context;
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] perf/x86/intel/bts: allocate bts_ctx only if necessary
2025-01-22 7:41 lirongqing
2025-01-23 18:36 ` Adrian Hunter
@ 2025-01-23 18:54 ` Liang, Kan
2025-03-05 13:47 ` Jiri Olsa
2 siblings, 0 replies; 8+ messages in thread
From: Liang, Kan @ 2025-01-23 18:54 UTC (permalink / raw)
To: lirongqing, peterz, mingo, acme, namhyung, mark.rutland,
alexander.shishkin, jolsa, irogers, adrian.hunter, tglx, bp,
dave.hansen, x86, hpa, linux-perf-users, linux-kernel
On 2025-01-22 2:41 a.m., lirongqing wrote:
> From: Li RongQing <lirongqing@baidu.com>
>
> Avoid unnecessary per-CPU memory allocation on unsupported CPUs,
> this can save 12K memory for each CPU
>
> Signed-off-by: Li RongQing <lirongqing@baidu.com>
Acked-by: Kan Liang <kan.liang@linux.intel.com>
Thanks,
Kan
> ---
> arch/x86/events/intel/bts.c | 22 +++++++++++++---------
> 1 file changed, 13 insertions(+), 9 deletions(-)
>
> diff --git a/arch/x86/events/intel/bts.c b/arch/x86/events/intel/bts.c
> index 8f78b0c..8e09319 100644
> --- a/arch/x86/events/intel/bts.c
> +++ b/arch/x86/events/intel/bts.c
> @@ -36,7 +36,7 @@ enum {
> BTS_STATE_ACTIVE,
> };
>
> -static DEFINE_PER_CPU(struct bts_ctx, bts_ctx);
> +static struct bts_ctx __percpu *bts_ctx;
>
> #define BTS_RECORD_SIZE 24
> #define BTS_SAFETY_MARGIN 4080
> @@ -231,7 +231,7 @@ bts_buffer_reset(struct bts_buffer *buf, struct perf_output_handle *handle);
>
> static void __bts_event_start(struct perf_event *event)
> {
> - struct bts_ctx *bts = this_cpu_ptr(&bts_ctx);
> + struct bts_ctx *bts = this_cpu_ptr(bts_ctx);
> struct bts_buffer *buf = perf_get_aux(&bts->handle);
> u64 config = 0;
>
> @@ -260,7 +260,7 @@ static void __bts_event_start(struct perf_event *event)
> static void bts_event_start(struct perf_event *event, int flags)
> {
> struct cpu_hw_events *cpuc = this_cpu_ptr(&cpu_hw_events);
> - struct bts_ctx *bts = this_cpu_ptr(&bts_ctx);
> + struct bts_ctx *bts = this_cpu_ptr(bts_ctx);
> struct bts_buffer *buf;
>
> buf = perf_aux_output_begin(&bts->handle, event);
> @@ -290,7 +290,7 @@ static void bts_event_start(struct perf_event *event, int flags)
>
> static void __bts_event_stop(struct perf_event *event, int state)
> {
> - struct bts_ctx *bts = this_cpu_ptr(&bts_ctx);
> + struct bts_ctx *bts = this_cpu_ptr(bts_ctx);
>
> /* ACTIVE -> INACTIVE(PMI)/STOPPED(->stop()) */
> WRITE_ONCE(bts->state, state);
> @@ -305,7 +305,7 @@ static void __bts_event_stop(struct perf_event *event, int state)
> static void bts_event_stop(struct perf_event *event, int flags)
> {
> struct cpu_hw_events *cpuc = this_cpu_ptr(&cpu_hw_events);
> - struct bts_ctx *bts = this_cpu_ptr(&bts_ctx);
> + struct bts_ctx *bts = this_cpu_ptr(bts_ctx);
> struct bts_buffer *buf = NULL;
> int state = READ_ONCE(bts->state);
>
> @@ -338,7 +338,7 @@ static void bts_event_stop(struct perf_event *event, int flags)
>
> void intel_bts_enable_local(void)
> {
> - struct bts_ctx *bts = this_cpu_ptr(&bts_ctx);
> + struct bts_ctx *bts = this_cpu_ptr(bts_ctx);
> int state = READ_ONCE(bts->state);
>
> /*
> @@ -358,7 +358,7 @@ void intel_bts_enable_local(void)
>
> void intel_bts_disable_local(void)
> {
> - struct bts_ctx *bts = this_cpu_ptr(&bts_ctx);
> + struct bts_ctx *bts = this_cpu_ptr(bts_ctx);
>
> /*
> * Here we transition from ACTIVE to INACTIVE;
> @@ -450,7 +450,7 @@ bts_buffer_reset(struct bts_buffer *buf, struct perf_output_handle *handle)
> int intel_bts_interrupt(void)
> {
> struct debug_store *ds = this_cpu_ptr(&cpu_hw_events)->ds;
> - struct bts_ctx *bts = this_cpu_ptr(&bts_ctx);
> + struct bts_ctx *bts = this_cpu_ptr(bts_ctx);
> struct perf_event *event = bts->handle.event;
> struct bts_buffer *buf;
> s64 old_head;
> @@ -518,7 +518,7 @@ static void bts_event_del(struct perf_event *event, int mode)
>
> static int bts_event_add(struct perf_event *event, int mode)
> {
> - struct bts_ctx *bts = this_cpu_ptr(&bts_ctx);
> + struct bts_ctx *bts = this_cpu_ptr(bts_ctx);
> struct cpu_hw_events *cpuc = this_cpu_ptr(&cpu_hw_events);
> struct hw_perf_event *hwc = &event->hw;
>
> @@ -605,6 +605,10 @@ static __init int bts_init(void)
> return -ENODEV;
> }
>
> + bts_ctx = alloc_percpu(struct bts_ctx);
> + if (!bts_ctx)
> + return -ENOMEM;
> +
> bts_pmu.capabilities = PERF_PMU_CAP_AUX_NO_SG | PERF_PMU_CAP_ITRACE |
> PERF_PMU_CAP_EXCLUSIVE;
> bts_pmu.task_ctx_nr = perf_sw_context;
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] perf/x86/intel/bts: allocate bts_ctx only if necessary
2025-01-22 7:41 lirongqing
2025-01-23 18:36 ` Adrian Hunter
2025-01-23 18:54 ` Liang, Kan
@ 2025-03-05 13:47 ` Jiri Olsa
2025-03-05 13:54 ` Jiri Olsa
2 siblings, 1 reply; 8+ messages in thread
From: Jiri Olsa @ 2025-03-05 13:47 UTC (permalink / raw)
To: lirongqing
Cc: peterz, mingo, acme, namhyung, mark.rutland, alexander.shishkin,
irogers, adrian.hunter, kan.liang, tglx, bp, dave.hansen, x86,
hpa, linux-perf-users, linux-kernel
On Wed, Jan 22, 2025 at 03:41:03PM +0800, lirongqing wrote:
> From: Li RongQing <lirongqing@baidu.com>
>
> Avoid unnecessary per-CPU memory allocation on unsupported CPUs,
> this can save 12K memory for each CPU
>
> Signed-off-by: Li RongQing <lirongqing@baidu.com>
> ---
> arch/x86/events/intel/bts.c | 22 +++++++++++++---------
> 1 file changed, 13 insertions(+), 9 deletions(-)
>
> diff --git a/arch/x86/events/intel/bts.c b/arch/x86/events/intel/bts.c
> index 8f78b0c..8e09319 100644
> --- a/arch/x86/events/intel/bts.c
> +++ b/arch/x86/events/intel/bts.c
> @@ -36,7 +36,7 @@ enum {
> BTS_STATE_ACTIVE,
> };
>
> -static DEFINE_PER_CPU(struct bts_ctx, bts_ctx);
> +static struct bts_ctx __percpu *bts_ctx;
>
> #define BTS_RECORD_SIZE 24
> #define BTS_SAFETY_MARGIN 4080
> @@ -231,7 +231,7 @@ bts_buffer_reset(struct bts_buffer *buf, struct perf_output_handle *handle);
>
> static void __bts_event_start(struct perf_event *event)
> {
> - struct bts_ctx *bts = this_cpu_ptr(&bts_ctx);
> + struct bts_ctx *bts = this_cpu_ptr(bts_ctx);
> struct bts_buffer *buf = perf_get_aux(&bts->handle);
> u64 config = 0;
>
> @@ -260,7 +260,7 @@ static void __bts_event_start(struct perf_event *event)
> static void bts_event_start(struct perf_event *event, int flags)
> {
> struct cpu_hw_events *cpuc = this_cpu_ptr(&cpu_hw_events);
> - struct bts_ctx *bts = this_cpu_ptr(&bts_ctx);
> + struct bts_ctx *bts = this_cpu_ptr(bts_ctx);
> struct bts_buffer *buf;
>
> buf = perf_aux_output_begin(&bts->handle, event);
> @@ -290,7 +290,7 @@ static void bts_event_start(struct perf_event *event, int flags)
>
> static void __bts_event_stop(struct perf_event *event, int state)
> {
> - struct bts_ctx *bts = this_cpu_ptr(&bts_ctx);
> + struct bts_ctx *bts = this_cpu_ptr(bts_ctx);
>
> /* ACTIVE -> INACTIVE(PMI)/STOPPED(->stop()) */
> WRITE_ONCE(bts->state, state);
> @@ -305,7 +305,7 @@ static void __bts_event_stop(struct perf_event *event, int state)
> static void bts_event_stop(struct perf_event *event, int flags)
> {
> struct cpu_hw_events *cpuc = this_cpu_ptr(&cpu_hw_events);
> - struct bts_ctx *bts = this_cpu_ptr(&bts_ctx);
> + struct bts_ctx *bts = this_cpu_ptr(bts_ctx);
> struct bts_buffer *buf = NULL;
> int state = READ_ONCE(bts->state);
>
> @@ -338,7 +338,7 @@ static void bts_event_stop(struct perf_event *event, int flags)
>
> void intel_bts_enable_local(void)
> {
> - struct bts_ctx *bts = this_cpu_ptr(&bts_ctx);
> + struct bts_ctx *bts = this_cpu_ptr(bts_ctx);
> int state = READ_ONCE(bts->state);
>
> /*
> @@ -358,7 +358,7 @@ void intel_bts_enable_local(void)
>
> void intel_bts_disable_local(void)
> {
> - struct bts_ctx *bts = this_cpu_ptr(&bts_ctx);
> + struct bts_ctx *bts = this_cpu_ptr(bts_ctx);
hi,
I'm getting crash below in here
I think it's because we allocate bts_ctx only if there's X86_FEATURE_PTI,
but then intel_bts_disable_local is called unconditionally from
intel_pmu_handle_irq and exploding on accessing bts_ctx
there's no crash when this change is reverted
jirka
>
> /*
> * Here we transition from ACTIVE to INACTIVE;
> @@ -450,7 +450,7 @@ bts_buffer_reset(struct bts_buffer *buf, struct perf_output_handle *handle)
> int intel_bts_interrupt(void)
> {
> struct debug_store *ds = this_cpu_ptr(&cpu_hw_events)->ds;
> - struct bts_ctx *bts = this_cpu_ptr(&bts_ctx);
> + struct bts_ctx *bts = this_cpu_ptr(bts_ctx);
> struct perf_event *event = bts->handle.event;
> struct bts_buffer *buf;
> s64 old_head;
> @@ -518,7 +518,7 @@ static void bts_event_del(struct perf_event *event, int mode)
>
> static int bts_event_add(struct perf_event *event, int mode)
> {
> - struct bts_ctx *bts = this_cpu_ptr(&bts_ctx);
> + struct bts_ctx *bts = this_cpu_ptr(bts_ctx);
> struct cpu_hw_events *cpuc = this_cpu_ptr(&cpu_hw_events);
> struct hw_perf_event *hwc = &event->hw;
>
> @@ -605,6 +605,10 @@ static __init int bts_init(void)
> return -ENODEV;
> }
>
> + bts_ctx = alloc_percpu(struct bts_ctx);
> + if (!bts_ctx)
> + return -ENOMEM;
> +
> bts_pmu.capabilities = PERF_PMU_CAP_AUX_NO_SG | PERF_PMU_CAP_ITRACE |
> PERF_PMU_CAP_EXCLUSIVE;
> bts_pmu.task_ctx_nr = perf_sw_context;
> --
> 2.9.4
>
---
[ 9.615480] BUG: unable to handle page fault for address: ffff9eb419247000
[ 9.615484] #PF: supervisor read access in kernel mode
[ 9.615485] #PF: error_code(0x0000) - not-present page
[ 9.615487] PGD ba801067 P4D ba801067 PUD 0
[ 9.615490] Oops: Oops: 0000 [#1] PREEMPT SMP NOPTI
[ 9.615492] CPU: 0 UID: 0 PID: 492 Comm: test_maps Not tainted 6.14.0-rc4-g5d70396a7df7-dirty #1
[ 9.615495] Hardware name: QEMU Ubuntu 24.04 PC (i440FX + PIIX, 1996), BIOS 1.16.3-debian-1.16.3-2 04/01/2014
[ 9.615497] RIP: 0010:intel_bts_disable_local+0x18/0x50
[ 9.615501] Code: 40 00 90 90 90 90 90 90 90 90 90 90 90 90 90 90 90 90 66 0f 1f 00 0f 1f 44 00 00 65 48 8b 0d bf 1f 7b 02 48 8b 05 e0 b7 7d 02 <8b> 94 01 00 20 00 00 83 fa 02 75 1f 48 83 3c 08 00 74 18 65 48 8b
[ 9.615503] RSP: 0018:fffffe71b3e00c78 EFLAGS: 00010002
[ 9.615504] RAX: 0000000000000000 RBX: fffffe71b3e00ef8 RCX: ffff9eb419245000
[ 9.615505] RDX: 0000000000000001 RSI: 000000023eb5576c RDI: fffffe71b3e00ef8
[ 9.615506] RBP: fffffe71b3e00e38 R08: 0000000000000002 R09: 0000000000000002
[ 9.615507] R10: ffffffffa023f735 R11: ffffffffa020dfe0 R12: fffffe71b3e00ef8
[ 9.615508] R13: 0000000000000000 R14: ffff9eb419245000 R15: 0000000000000001
[ 9.615508] FS: 00007f25164ac140(0000) GS:ffff9eb419245000(0000) knlGS:0000000000000000
[ 9.615510] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[ 9.615511] CR2: ffff9eb419247000 CR3: 000000010b31c006 CR4: 0000000000770ef0
[ 9.615514] DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
[ 9.615515] DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
[ 9.615516] PKRU: 55555554
[ 9.615516] Call Trace:
[ 9.615518] <NMI>
[ 9.615519] ? __die_body+0x6a/0xb0
[ 9.615521] ? page_fault_oops+0x36f/0x400
[ 9.615525] ? do_kern_addr_fault+0x9a/0xd0
[ 9.615526] ? exc_page_fault+0xab/0xf0
[ 9.615528] ? asm_exc_page_fault+0x26/0x30
[ 9.615531] ? __pfx_perf_event_nmi_handler+0x10/0x10
[ 9.615533] ? nmi_handle+0x55/0x2b0
[ 9.615536] ? intel_bts_disable_local+0x18/0x50
[ 9.615538] intel_pmu_handle_irq+0x88/0x8b0
[ 9.615545] ? look_up_lock_class+0x74/0x150
[ 9.615546] ? verify_lock_unused+0xb/0x80
[ 9.615549] ? lock_acquire+0x1a0/0x2a0
[ 9.615551] ? nmi_handle+0x55/0x2b0
[ 9.615553] perf_event_nmi_handler+0x28/0x50
[ 9.615555] nmi_handle+0x116/0x2b0
[ 9.615557] ? nmi_handle+0x55/0x2b0
[ 9.615558] ? _copy_from_user+0x37/0x80
[ 9.615561] default_do_nmi+0x41/0x100
[ 9.615564] exc_nmi+0xb5/0x110
[ 9.615566] end_repeat_nmi+0xf/0x53
[ 9.615568] RIP: 0010:_copy_from_user+0x37/0x80
[ 9.615570] Code: 89 f7 49 89 fe 48 c7 c7 19 16 49 a1 be a2 00 00 00 e8 8d 3e cb ff 48 b8 00 f0 ff ff ff 7f 00 00 49 39 c7 4c 0f 47 f8 0f 01 cb <4c> 89 f7 48 89 d9 4c 89 fe e8 fb cd 65 00 48 89 c8 0f 01 ca 48 85
[ 9.615571] RSP: 0018:ffffa49b403dfdb0 EFLAGS: 00040283
[ 9.615572] RAX: 00007ffffffff000 RBX: 0000000000000008 RCX: 0000000000000001
[ 9.615573] RDX: 0000000000000001 RSI: ffff9eb38b2c3950 RDI: ffff9eb389f40b58
[ 9.615574] RBP: 0000000000000000 R08: 0000000000000000 R09: ffffffffffffffff
[ 9.615574] R10: ffff9eb389f40b78 R11: 0000000000000000 R12: ffff9eb382711000
[ 9.615575] R13: 0000000000000000 R14: ffff9eb3824fd920 R15: 00007ffd584f7268
[ 9.615578] ? _copy_from_user+0x37/0x80
[ 9.615580] ? _copy_from_user+0x37/0x80
[ 9.615581] </NMI>
[ 9.615582] <TASK>
[ 9.615582] ___bpf_copy_key+0x52/0xb0
[ 9.615586] map_update_elem+0xd4/0x280
[ 9.615589] __sys_bpf+0x450/0x550
[ 9.615592] __x64_sys_bpf+0x1c/0x30
[ 9.615594] do_syscall_64+0x79/0x120
[ 9.615596] ? clear_bhb_loop+0x35/0x90
[ 9.615597] ? clear_bhb_loop+0x35/0x90
[ 9.615599] entry_SYSCALL_64_after_hwframe+0x76/0x7e
[ 9.615600] RIP: 0033:0x7f251668e25d
[ 9.615602] Code: ff c3 66 2e 0f 1f 84 00 00 00 00 00 90 f3 0f 1e fa 48 89 f8 48 89 f7 48 89 d6 48 89 ca 4d 89 c2 4d 89 c8 4c 8b 4c 24 08 0f 05 <48> 3d 01 f0 ff ff 73 01 c3 48 8b 0d 8b bb 0d 00 f7 d8 64 89 01 48
[ 9.615603] RSP: 002b:00007ffd584f7128 EFLAGS: 00000206 ORIG_RAX: 0000000000000141
[ 9.615604] RAX: ffffffffffffffda RBX: 00007ffd584f76b8 RCX: 00007f251668e25d
[ 9.615605] RDX: 0000000000000020 RSI: 00007ffd584f7170 RDI: 0000000000000002
[ 9.615606] RBP: 00007ffd584f7150 R08: 0000000000000000 R09: 0000000000000000
[ 9.615606] R10: 0000000000000000 R11: 0000000000000206 R12: 0000000000000001
[ 9.615607] R13: 0000000000000000 R14: 000055bd9b4e01b8 R15: 00007f2516801000
[ 9.615609] </TASK> [ 9.615610] Modules linked in:
[ 9.615611] CR2: ffff9eb419247000
[ 9.615613] ---[ end trace 0000000000000000 ]---
[ 9.615614] RIP: 0010:intel_bts_disable_local+0x18/0x50
[ 9.615615] Code: 40 00 90 90 90 90 90 90 90 90 90 90 90 90 90 90 90 90 66 0f 1f 00 0f 1f 44 00 00 65 48 8b 0d bf 1f 7b 02 48 8b 05 e0 b7 7d 02 <8b> 94 01 00 20 00 00 83 fa 02 75 1f 48 83 3c 08 00 74 18 65 48 8b
[ 9.615616] RSP: 0018:fffffe71b3e00c78 EFLAGS: 00010002
[ 9.615617] RAX: 0000000000000000 RBX: fffffe71b3e00ef8 RCX: ffff9eb419245000
[ 9.615618] RDX: 0000000000000001 RSI: 000000023eb5576c RDI: fffffe71b3e00ef8
[ 9.615618] RBP: fffffe71b3e00e38 R08: 0000000000000002 R09: 0000000000000002
[ 9.615619] R10: ffffffffa023f735 R11: ffffffffa020dfe0 R12: fffffe71b3e00ef8
[ 9.615619] R13: 0000000000000000 R14: ffff9eb419245000 R15: 0000000000000001
[ 9.615620] FS: 00007f25164ac140(0000) GS:ffff9eb419245000(0000) knlGS:0000000000000000
[ 9.615621] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[ 9.615622] CR2: ffff9eb419247000 CR3: 000000010b31c006 CR4: 0000000000770ef0
[ 9.615622] DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
[ 9.615623] DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
[ 9.615624] PKRU: 55555554
[ 9.615624] Kernel panic - not syncing: Fatal exception in interrupt
[ 9.615807] Kernel Offset: 0x1f000000 from 0xffffffff81000000 (relocation range: 0xffffffff80000000-0xffffffffbfffffff)
Failed to run command
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] perf/x86/intel/bts: allocate bts_ctx only if necessary
2025-03-05 13:47 ` Jiri Olsa
@ 2025-03-05 13:54 ` Jiri Olsa
2025-03-05 14:58 ` Adrian Hunter
0 siblings, 1 reply; 8+ messages in thread
From: Jiri Olsa @ 2025-03-05 13:54 UTC (permalink / raw)
To: Jiri Olsa
Cc: lirongqing, peterz, mingo, acme, namhyung, mark.rutland,
alexander.shishkin, irogers, adrian.hunter, kan.liang, tglx, bp,
dave.hansen, x86, hpa, linux-perf-users, linux-kernel
On Wed, Mar 05, 2025 at 02:47:09PM +0100, Jiri Olsa wrote:
> On Wed, Jan 22, 2025 at 03:41:03PM +0800, lirongqing wrote:
> > From: Li RongQing <lirongqing@baidu.com>
> >
> > Avoid unnecessary per-CPU memory allocation on unsupported CPUs,
> > this can save 12K memory for each CPU
> >
> > Signed-off-by: Li RongQing <lirongqing@baidu.com>
> > ---
> > arch/x86/events/intel/bts.c | 22 +++++++++++++---------
> > 1 file changed, 13 insertions(+), 9 deletions(-)
> >
> > diff --git a/arch/x86/events/intel/bts.c b/arch/x86/events/intel/bts.c
> > index 8f78b0c..8e09319 100644
> > --- a/arch/x86/events/intel/bts.c
> > +++ b/arch/x86/events/intel/bts.c
> > @@ -36,7 +36,7 @@ enum {
> > BTS_STATE_ACTIVE,
> > };
> >
> > -static DEFINE_PER_CPU(struct bts_ctx, bts_ctx);
> > +static struct bts_ctx __percpu *bts_ctx;
> >
> > #define BTS_RECORD_SIZE 24
> > #define BTS_SAFETY_MARGIN 4080
> > @@ -231,7 +231,7 @@ bts_buffer_reset(struct bts_buffer *buf, struct perf_output_handle *handle);
> >
> > static void __bts_event_start(struct perf_event *event)
> > {
> > - struct bts_ctx *bts = this_cpu_ptr(&bts_ctx);
> > + struct bts_ctx *bts = this_cpu_ptr(bts_ctx);
> > struct bts_buffer *buf = perf_get_aux(&bts->handle);
> > u64 config = 0;
> >
> > @@ -260,7 +260,7 @@ static void __bts_event_start(struct perf_event *event)
> > static void bts_event_start(struct perf_event *event, int flags)
> > {
> > struct cpu_hw_events *cpuc = this_cpu_ptr(&cpu_hw_events);
> > - struct bts_ctx *bts = this_cpu_ptr(&bts_ctx);
> > + struct bts_ctx *bts = this_cpu_ptr(bts_ctx);
> > struct bts_buffer *buf;
> >
> > buf = perf_aux_output_begin(&bts->handle, event);
> > @@ -290,7 +290,7 @@ static void bts_event_start(struct perf_event *event, int flags)
> >
> > static void __bts_event_stop(struct perf_event *event, int state)
> > {
> > - struct bts_ctx *bts = this_cpu_ptr(&bts_ctx);
> > + struct bts_ctx *bts = this_cpu_ptr(bts_ctx);
> >
> > /* ACTIVE -> INACTIVE(PMI)/STOPPED(->stop()) */
> > WRITE_ONCE(bts->state, state);
> > @@ -305,7 +305,7 @@ static void __bts_event_stop(struct perf_event *event, int state)
> > static void bts_event_stop(struct perf_event *event, int flags)
> > {
> > struct cpu_hw_events *cpuc = this_cpu_ptr(&cpu_hw_events);
> > - struct bts_ctx *bts = this_cpu_ptr(&bts_ctx);
> > + struct bts_ctx *bts = this_cpu_ptr(bts_ctx);
> > struct bts_buffer *buf = NULL;
> > int state = READ_ONCE(bts->state);
> >
> > @@ -338,7 +338,7 @@ static void bts_event_stop(struct perf_event *event, int flags)
> >
> > void intel_bts_enable_local(void)
> > {
> > - struct bts_ctx *bts = this_cpu_ptr(&bts_ctx);
> > + struct bts_ctx *bts = this_cpu_ptr(bts_ctx);
> > int state = READ_ONCE(bts->state);
> >
> > /*
> > @@ -358,7 +358,7 @@ void intel_bts_enable_local(void)
> >
> > void intel_bts_disable_local(void)
> > {
> > - struct bts_ctx *bts = this_cpu_ptr(&bts_ctx);
> > + struct bts_ctx *bts = this_cpu_ptr(bts_ctx);
>
> hi,
> I'm getting crash below in here
>
> I think it's because we allocate bts_ctx only if there's X86_FEATURE_PTI,
... .if there's not X86_FEATURE_PTI ;-) ...
jirka
> but then intel_bts_disable_local is called unconditionally from
> intel_pmu_handle_irq and exploding on accessing bts_ctx
>
> there's no crash when this change is reverted
>
> jirka
>
>
> >
> > /*
> > * Here we transition from ACTIVE to INACTIVE;
> > @@ -450,7 +450,7 @@ bts_buffer_reset(struct bts_buffer *buf, struct perf_output_handle *handle)
> > int intel_bts_interrupt(void)
> > {
> > struct debug_store *ds = this_cpu_ptr(&cpu_hw_events)->ds;
> > - struct bts_ctx *bts = this_cpu_ptr(&bts_ctx);
> > + struct bts_ctx *bts = this_cpu_ptr(bts_ctx);
> > struct perf_event *event = bts->handle.event;
> > struct bts_buffer *buf;
> > s64 old_head;
> > @@ -518,7 +518,7 @@ static void bts_event_del(struct perf_event *event, int mode)
> >
> > static int bts_event_add(struct perf_event *event, int mode)
> > {
> > - struct bts_ctx *bts = this_cpu_ptr(&bts_ctx);
> > + struct bts_ctx *bts = this_cpu_ptr(bts_ctx);
> > struct cpu_hw_events *cpuc = this_cpu_ptr(&cpu_hw_events);
> > struct hw_perf_event *hwc = &event->hw;
> >
> > @@ -605,6 +605,10 @@ static __init int bts_init(void)
> > return -ENODEV;
> > }
> >
> > + bts_ctx = alloc_percpu(struct bts_ctx);
> > + if (!bts_ctx)
> > + return -ENOMEM;
> > +
> > bts_pmu.capabilities = PERF_PMU_CAP_AUX_NO_SG | PERF_PMU_CAP_ITRACE |
> > PERF_PMU_CAP_EXCLUSIVE;
> > bts_pmu.task_ctx_nr = perf_sw_context;
> > --
> > 2.9.4
> >
>
> ---
>
> [ 9.615480] BUG: unable to handle page fault for address: ffff9eb419247000
> [ 9.615484] #PF: supervisor read access in kernel mode
> [ 9.615485] #PF: error_code(0x0000) - not-present page
> [ 9.615487] PGD ba801067 P4D ba801067 PUD 0
> [ 9.615490] Oops: Oops: 0000 [#1] PREEMPT SMP NOPTI
> [ 9.615492] CPU: 0 UID: 0 PID: 492 Comm: test_maps Not tainted 6.14.0-rc4-g5d70396a7df7-dirty #1
> [ 9.615495] Hardware name: QEMU Ubuntu 24.04 PC (i440FX + PIIX, 1996), BIOS 1.16.3-debian-1.16.3-2 04/01/2014
> [ 9.615497] RIP: 0010:intel_bts_disable_local+0x18/0x50
> [ 9.615501] Code: 40 00 90 90 90 90 90 90 90 90 90 90 90 90 90 90 90 90 66 0f 1f 00 0f 1f 44 00 00 65 48 8b 0d bf 1f 7b 02 48 8b 05 e0 b7 7d 02 <8b> 94 01 00 20 00 00 83 fa 02 75 1f 48 83 3c 08 00 74 18 65 48 8b
> [ 9.615503] RSP: 0018:fffffe71b3e00c78 EFLAGS: 00010002
> [ 9.615504] RAX: 0000000000000000 RBX: fffffe71b3e00ef8 RCX: ffff9eb419245000
> [ 9.615505] RDX: 0000000000000001 RSI: 000000023eb5576c RDI: fffffe71b3e00ef8
> [ 9.615506] RBP: fffffe71b3e00e38 R08: 0000000000000002 R09: 0000000000000002
> [ 9.615507] R10: ffffffffa023f735 R11: ffffffffa020dfe0 R12: fffffe71b3e00ef8
> [ 9.615508] R13: 0000000000000000 R14: ffff9eb419245000 R15: 0000000000000001
> [ 9.615508] FS: 00007f25164ac140(0000) GS:ffff9eb419245000(0000) knlGS:0000000000000000
> [ 9.615510] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> [ 9.615511] CR2: ffff9eb419247000 CR3: 000000010b31c006 CR4: 0000000000770ef0
> [ 9.615514] DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
> [ 9.615515] DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
> [ 9.615516] PKRU: 55555554
> [ 9.615516] Call Trace:
> [ 9.615518] <NMI>
> [ 9.615519] ? __die_body+0x6a/0xb0
> [ 9.615521] ? page_fault_oops+0x36f/0x400
> [ 9.615525] ? do_kern_addr_fault+0x9a/0xd0
> [ 9.615526] ? exc_page_fault+0xab/0xf0
> [ 9.615528] ? asm_exc_page_fault+0x26/0x30
> [ 9.615531] ? __pfx_perf_event_nmi_handler+0x10/0x10
> [ 9.615533] ? nmi_handle+0x55/0x2b0
> [ 9.615536] ? intel_bts_disable_local+0x18/0x50
> [ 9.615538] intel_pmu_handle_irq+0x88/0x8b0
> [ 9.615545] ? look_up_lock_class+0x74/0x150
> [ 9.615546] ? verify_lock_unused+0xb/0x80
> [ 9.615549] ? lock_acquire+0x1a0/0x2a0
> [ 9.615551] ? nmi_handle+0x55/0x2b0
> [ 9.615553] perf_event_nmi_handler+0x28/0x50
> [ 9.615555] nmi_handle+0x116/0x2b0
> [ 9.615557] ? nmi_handle+0x55/0x2b0
> [ 9.615558] ? _copy_from_user+0x37/0x80
> [ 9.615561] default_do_nmi+0x41/0x100
> [ 9.615564] exc_nmi+0xb5/0x110
> [ 9.615566] end_repeat_nmi+0xf/0x53
> [ 9.615568] RIP: 0010:_copy_from_user+0x37/0x80
> [ 9.615570] Code: 89 f7 49 89 fe 48 c7 c7 19 16 49 a1 be a2 00 00 00 e8 8d 3e cb ff 48 b8 00 f0 ff ff ff 7f 00 00 49 39 c7 4c 0f 47 f8 0f 01 cb <4c> 89 f7 48 89 d9 4c 89 fe e8 fb cd 65 00 48 89 c8 0f 01 ca 48 85
> [ 9.615571] RSP: 0018:ffffa49b403dfdb0 EFLAGS: 00040283
> [ 9.615572] RAX: 00007ffffffff000 RBX: 0000000000000008 RCX: 0000000000000001
> [ 9.615573] RDX: 0000000000000001 RSI: ffff9eb38b2c3950 RDI: ffff9eb389f40b58
> [ 9.615574] RBP: 0000000000000000 R08: 0000000000000000 R09: ffffffffffffffff
> [ 9.615574] R10: ffff9eb389f40b78 R11: 0000000000000000 R12: ffff9eb382711000
> [ 9.615575] R13: 0000000000000000 R14: ffff9eb3824fd920 R15: 00007ffd584f7268
> [ 9.615578] ? _copy_from_user+0x37/0x80
> [ 9.615580] ? _copy_from_user+0x37/0x80
> [ 9.615581] </NMI>
> [ 9.615582] <TASK>
> [ 9.615582] ___bpf_copy_key+0x52/0xb0
> [ 9.615586] map_update_elem+0xd4/0x280
> [ 9.615589] __sys_bpf+0x450/0x550
> [ 9.615592] __x64_sys_bpf+0x1c/0x30
> [ 9.615594] do_syscall_64+0x79/0x120
> [ 9.615596] ? clear_bhb_loop+0x35/0x90
> [ 9.615597] ? clear_bhb_loop+0x35/0x90
> [ 9.615599] entry_SYSCALL_64_after_hwframe+0x76/0x7e
> [ 9.615600] RIP: 0033:0x7f251668e25d
> [ 9.615602] Code: ff c3 66 2e 0f 1f 84 00 00 00 00 00 90 f3 0f 1e fa 48 89 f8 48 89 f7 48 89 d6 48 89 ca 4d 89 c2 4d 89 c8 4c 8b 4c 24 08 0f 05 <48> 3d 01 f0 ff ff 73 01 c3 48 8b 0d 8b bb 0d 00 f7 d8 64 89 01 48
> [ 9.615603] RSP: 002b:00007ffd584f7128 EFLAGS: 00000206 ORIG_RAX: 0000000000000141
> [ 9.615604] RAX: ffffffffffffffda RBX: 00007ffd584f76b8 RCX: 00007f251668e25d
> [ 9.615605] RDX: 0000000000000020 RSI: 00007ffd584f7170 RDI: 0000000000000002
> [ 9.615606] RBP: 00007ffd584f7150 R08: 0000000000000000 R09: 0000000000000000
> [ 9.615606] R10: 0000000000000000 R11: 0000000000000206 R12: 0000000000000001
> [ 9.615607] R13: 0000000000000000 R14: 000055bd9b4e01b8 R15: 00007f2516801000
> [ 9.615609] </TASK> [ 9.615610] Modules linked in:
> [ 9.615611] CR2: ffff9eb419247000
> [ 9.615613] ---[ end trace 0000000000000000 ]---
> [ 9.615614] RIP: 0010:intel_bts_disable_local+0x18/0x50
> [ 9.615615] Code: 40 00 90 90 90 90 90 90 90 90 90 90 90 90 90 90 90 90 66 0f 1f 00 0f 1f 44 00 00 65 48 8b 0d bf 1f 7b 02 48 8b 05 e0 b7 7d 02 <8b> 94 01 00 20 00 00 83 fa 02 75 1f 48 83 3c 08 00 74 18 65 48 8b
> [ 9.615616] RSP: 0018:fffffe71b3e00c78 EFLAGS: 00010002
> [ 9.615617] RAX: 0000000000000000 RBX: fffffe71b3e00ef8 RCX: ffff9eb419245000
> [ 9.615618] RDX: 0000000000000001 RSI: 000000023eb5576c RDI: fffffe71b3e00ef8
> [ 9.615618] RBP: fffffe71b3e00e38 R08: 0000000000000002 R09: 0000000000000002
> [ 9.615619] R10: ffffffffa023f735 R11: ffffffffa020dfe0 R12: fffffe71b3e00ef8
> [ 9.615619] R13: 0000000000000000 R14: ffff9eb419245000 R15: 0000000000000001
> [ 9.615620] FS: 00007f25164ac140(0000) GS:ffff9eb419245000(0000) knlGS:0000000000000000
> [ 9.615621] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> [ 9.615622] CR2: ffff9eb419247000 CR3: 000000010b31c006 CR4: 0000000000770ef0
> [ 9.615622] DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
> [ 9.615623] DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
> [ 9.615624] PKRU: 55555554
> [ 9.615624] Kernel panic - not syncing: Fatal exception in interrupt
> [ 9.615807] Kernel Offset: 0x1f000000 from 0xffffffff81000000 (relocation range: 0xffffffff80000000-0xffffffffbfffffff)
> Failed to run command
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] perf/x86/intel/bts: allocate bts_ctx only if necessary
2025-03-05 13:54 ` Jiri Olsa
@ 2025-03-05 14:58 ` Adrian Hunter
2025-03-05 16:11 ` Dave Hansen
0 siblings, 1 reply; 8+ messages in thread
From: Adrian Hunter @ 2025-03-05 14:58 UTC (permalink / raw)
To: Jiri Olsa
Cc: lirongqing, peterz, mingo, acme, namhyung, mark.rutland,
alexander.shishkin, irogers, kan.liang, tglx, bp, dave.hansen,
x86, hpa, linux-perf-users, linux-kernel
On 5/03/25 15:54, Jiri Olsa wrote:
> On Wed, Mar 05, 2025 at 02:47:09PM +0100, Jiri Olsa wrote:
>> On Wed, Jan 22, 2025 at 03:41:03PM +0800, lirongqing wrote:
>>> From: Li RongQing <lirongqing@baidu.com>
>>>
>>> Avoid unnecessary per-CPU memory allocation on unsupported CPUs,
>>> this can save 12K memory for each CPU
>>>
>>> Signed-off-by: Li RongQing <lirongqing@baidu.com>
>>> ---
>>> arch/x86/events/intel/bts.c | 22 +++++++++++++---------
>>> 1 file changed, 13 insertions(+), 9 deletions(-)
>>>
>>> diff --git a/arch/x86/events/intel/bts.c b/arch/x86/events/intel/bts.c
>>> index 8f78b0c..8e09319 100644
>>> --- a/arch/x86/events/intel/bts.c
>>> +++ b/arch/x86/events/intel/bts.c
>>> @@ -36,7 +36,7 @@ enum {
>>> BTS_STATE_ACTIVE,
>>> };
>>>
>>> -static DEFINE_PER_CPU(struct bts_ctx, bts_ctx);
>>> +static struct bts_ctx __percpu *bts_ctx;
>>>
>>> #define BTS_RECORD_SIZE 24
>>> #define BTS_SAFETY_MARGIN 4080
>>> @@ -231,7 +231,7 @@ bts_buffer_reset(struct bts_buffer *buf, struct perf_output_handle *handle);
>>>
>>> static void __bts_event_start(struct perf_event *event)
>>> {
>>> - struct bts_ctx *bts = this_cpu_ptr(&bts_ctx);
>>> + struct bts_ctx *bts = this_cpu_ptr(bts_ctx);
>>> struct bts_buffer *buf = perf_get_aux(&bts->handle);
>>> u64 config = 0;
>>>
>>> @@ -260,7 +260,7 @@ static void __bts_event_start(struct perf_event *event)
>>> static void bts_event_start(struct perf_event *event, int flags)
>>> {
>>> struct cpu_hw_events *cpuc = this_cpu_ptr(&cpu_hw_events);
>>> - struct bts_ctx *bts = this_cpu_ptr(&bts_ctx);
>>> + struct bts_ctx *bts = this_cpu_ptr(bts_ctx);
>>> struct bts_buffer *buf;
>>>
>>> buf = perf_aux_output_begin(&bts->handle, event);
>>> @@ -290,7 +290,7 @@ static void bts_event_start(struct perf_event *event, int flags)
>>>
>>> static void __bts_event_stop(struct perf_event *event, int state)
>>> {
>>> - struct bts_ctx *bts = this_cpu_ptr(&bts_ctx);
>>> + struct bts_ctx *bts = this_cpu_ptr(bts_ctx);
>>>
>>> /* ACTIVE -> INACTIVE(PMI)/STOPPED(->stop()) */
>>> WRITE_ONCE(bts->state, state);
>>> @@ -305,7 +305,7 @@ static void __bts_event_stop(struct perf_event *event, int state)
>>> static void bts_event_stop(struct perf_event *event, int flags)
>>> {
>>> struct cpu_hw_events *cpuc = this_cpu_ptr(&cpu_hw_events);
>>> - struct bts_ctx *bts = this_cpu_ptr(&bts_ctx);
>>> + struct bts_ctx *bts = this_cpu_ptr(bts_ctx);
>>> struct bts_buffer *buf = NULL;
>>> int state = READ_ONCE(bts->state);
>>>
>>> @@ -338,7 +338,7 @@ static void bts_event_stop(struct perf_event *event, int flags)
>>>
>>> void intel_bts_enable_local(void)
>>> {
>>> - struct bts_ctx *bts = this_cpu_ptr(&bts_ctx);
>>> + struct bts_ctx *bts = this_cpu_ptr(bts_ctx);
>>> int state = READ_ONCE(bts->state);
>>>
>>> /*
>>> @@ -358,7 +358,7 @@ void intel_bts_enable_local(void)
>>>
>>> void intel_bts_disable_local(void)
>>> {
>>> - struct bts_ctx *bts = this_cpu_ptr(&bts_ctx);
>>> + struct bts_ctx *bts = this_cpu_ptr(bts_ctx);
>>
>> hi,
>> I'm getting crash below in here
>>
>> I think it's because we allocate bts_ctx only if there's X86_FEATURE_PTI,
>
> ... .if there's not X86_FEATURE_PTI ;-) ...
>
> jirka
>
>> but then intel_bts_disable_local is called unconditionally from
>> intel_pmu_handle_irq and exploding on accessing bts_ctx
>>
>> there's no crash when this change is reverted
It looks like there are 3 functions affected:
intel_bts_enable_local()
intel_bts_disable_local()
intel_bts_interrupt()
Perhaps make them static calls?
>>
>> jirka
>>
>>
>>>
>>> /*
>>> * Here we transition from ACTIVE to INACTIVE;
>>> @@ -450,7 +450,7 @@ bts_buffer_reset(struct bts_buffer *buf, struct perf_output_handle *handle)
>>> int intel_bts_interrupt(void)
>>> {
>>> struct debug_store *ds = this_cpu_ptr(&cpu_hw_events)->ds;
>>> - struct bts_ctx *bts = this_cpu_ptr(&bts_ctx);
>>> + struct bts_ctx *bts = this_cpu_ptr(bts_ctx);
>>> struct perf_event *event = bts->handle.event;
>>> struct bts_buffer *buf;
>>> s64 old_head;
>>> @@ -518,7 +518,7 @@ static void bts_event_del(struct perf_event *event, int mode)
>>>
>>> static int bts_event_add(struct perf_event *event, int mode)
>>> {
>>> - struct bts_ctx *bts = this_cpu_ptr(&bts_ctx);
>>> + struct bts_ctx *bts = this_cpu_ptr(bts_ctx);
>>> struct cpu_hw_events *cpuc = this_cpu_ptr(&cpu_hw_events);
>>> struct hw_perf_event *hwc = &event->hw;
>>>
>>> @@ -605,6 +605,10 @@ static __init int bts_init(void)
>>> return -ENODEV;
>>> }
>>>
>>> + bts_ctx = alloc_percpu(struct bts_ctx);
>>> + if (!bts_ctx)
>>> + return -ENOMEM;
>>> +
>>> bts_pmu.capabilities = PERF_PMU_CAP_AUX_NO_SG | PERF_PMU_CAP_ITRACE |
>>> PERF_PMU_CAP_EXCLUSIVE;
>>> bts_pmu.task_ctx_nr = perf_sw_context;
>>> --
>>> 2.9.4
>>>
>>
>> ---
>>
>> [ 9.615480] BUG: unable to handle page fault for address: ffff9eb419247000
>> [ 9.615484] #PF: supervisor read access in kernel mode
>> [ 9.615485] #PF: error_code(0x0000) - not-present page
>> [ 9.615487] PGD ba801067 P4D ba801067 PUD 0
>> [ 9.615490] Oops: Oops: 0000 [#1] PREEMPT SMP NOPTI
>> [ 9.615492] CPU: 0 UID: 0 PID: 492 Comm: test_maps Not tainted 6.14.0-rc4-g5d70396a7df7-dirty #1
>> [ 9.615495] Hardware name: QEMU Ubuntu 24.04 PC (i440FX + PIIX, 1996), BIOS 1.16.3-debian-1.16.3-2 04/01/2014
>> [ 9.615497] RIP: 0010:intel_bts_disable_local+0x18/0x50
>> [ 9.615501] Code: 40 00 90 90 90 90 90 90 90 90 90 90 90 90 90 90 90 90 66 0f 1f 00 0f 1f 44 00 00 65 48 8b 0d bf 1f 7b 02 48 8b 05 e0 b7 7d 02 <8b> 94 01 00 20 00 00 83 fa 02 75 1f 48 83 3c 08 00 74 18 65 48 8b
>> [ 9.615503] RSP: 0018:fffffe71b3e00c78 EFLAGS: 00010002
>> [ 9.615504] RAX: 0000000000000000 RBX: fffffe71b3e00ef8 RCX: ffff9eb419245000
>> [ 9.615505] RDX: 0000000000000001 RSI: 000000023eb5576c RDI: fffffe71b3e00ef8
>> [ 9.615506] RBP: fffffe71b3e00e38 R08: 0000000000000002 R09: 0000000000000002
>> [ 9.615507] R10: ffffffffa023f735 R11: ffffffffa020dfe0 R12: fffffe71b3e00ef8
>> [ 9.615508] R13: 0000000000000000 R14: ffff9eb419245000 R15: 0000000000000001
>> [ 9.615508] FS: 00007f25164ac140(0000) GS:ffff9eb419245000(0000) knlGS:0000000000000000
>> [ 9.615510] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
>> [ 9.615511] CR2: ffff9eb419247000 CR3: 000000010b31c006 CR4: 0000000000770ef0
>> [ 9.615514] DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
>> [ 9.615515] DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
>> [ 9.615516] PKRU: 55555554
>> [ 9.615516] Call Trace:
>> [ 9.615518] <NMI>
>> [ 9.615519] ? __die_body+0x6a/0xb0
>> [ 9.615521] ? page_fault_oops+0x36f/0x400
>> [ 9.615525] ? do_kern_addr_fault+0x9a/0xd0
>> [ 9.615526] ? exc_page_fault+0xab/0xf0
>> [ 9.615528] ? asm_exc_page_fault+0x26/0x30
>> [ 9.615531] ? __pfx_perf_event_nmi_handler+0x10/0x10
>> [ 9.615533] ? nmi_handle+0x55/0x2b0
>> [ 9.615536] ? intel_bts_disable_local+0x18/0x50
>> [ 9.615538] intel_pmu_handle_irq+0x88/0x8b0
>> [ 9.615545] ? look_up_lock_class+0x74/0x150
>> [ 9.615546] ? verify_lock_unused+0xb/0x80
>> [ 9.615549] ? lock_acquire+0x1a0/0x2a0
>> [ 9.615551] ? nmi_handle+0x55/0x2b0
>> [ 9.615553] perf_event_nmi_handler+0x28/0x50
>> [ 9.615555] nmi_handle+0x116/0x2b0
>> [ 9.615557] ? nmi_handle+0x55/0x2b0
>> [ 9.615558] ? _copy_from_user+0x37/0x80
>> [ 9.615561] default_do_nmi+0x41/0x100
>> [ 9.615564] exc_nmi+0xb5/0x110
>> [ 9.615566] end_repeat_nmi+0xf/0x53
>> [ 9.615568] RIP: 0010:_copy_from_user+0x37/0x80
>> [ 9.615570] Code: 89 f7 49 89 fe 48 c7 c7 19 16 49 a1 be a2 00 00 00 e8 8d 3e cb ff 48 b8 00 f0 ff ff ff 7f 00 00 49 39 c7 4c 0f 47 f8 0f 01 cb <4c> 89 f7 48 89 d9 4c 89 fe e8 fb cd 65 00 48 89 c8 0f 01 ca 48 85
>> [ 9.615571] RSP: 0018:ffffa49b403dfdb0 EFLAGS: 00040283
>> [ 9.615572] RAX: 00007ffffffff000 RBX: 0000000000000008 RCX: 0000000000000001
>> [ 9.615573] RDX: 0000000000000001 RSI: ffff9eb38b2c3950 RDI: ffff9eb389f40b58
>> [ 9.615574] RBP: 0000000000000000 R08: 0000000000000000 R09: ffffffffffffffff
>> [ 9.615574] R10: ffff9eb389f40b78 R11: 0000000000000000 R12: ffff9eb382711000
>> [ 9.615575] R13: 0000000000000000 R14: ffff9eb3824fd920 R15: 00007ffd584f7268
>> [ 9.615578] ? _copy_from_user+0x37/0x80
>> [ 9.615580] ? _copy_from_user+0x37/0x80
>> [ 9.615581] </NMI>
>> [ 9.615582] <TASK>
>> [ 9.615582] ___bpf_copy_key+0x52/0xb0
>> [ 9.615586] map_update_elem+0xd4/0x280
>> [ 9.615589] __sys_bpf+0x450/0x550
>> [ 9.615592] __x64_sys_bpf+0x1c/0x30
>> [ 9.615594] do_syscall_64+0x79/0x120
>> [ 9.615596] ? clear_bhb_loop+0x35/0x90
>> [ 9.615597] ? clear_bhb_loop+0x35/0x90
>> [ 9.615599] entry_SYSCALL_64_after_hwframe+0x76/0x7e
>> [ 9.615600] RIP: 0033:0x7f251668e25d
>> [ 9.615602] Code: ff c3 66 2e 0f 1f 84 00 00 00 00 00 90 f3 0f 1e fa 48 89 f8 48 89 f7 48 89 d6 48 89 ca 4d 89 c2 4d 89 c8 4c 8b 4c 24 08 0f 05 <48> 3d 01 f0 ff ff 73 01 c3 48 8b 0d 8b bb 0d 00 f7 d8 64 89 01 48
>> [ 9.615603] RSP: 002b:00007ffd584f7128 EFLAGS: 00000206 ORIG_RAX: 0000000000000141
>> [ 9.615604] RAX: ffffffffffffffda RBX: 00007ffd584f76b8 RCX: 00007f251668e25d
>> [ 9.615605] RDX: 0000000000000020 RSI: 00007ffd584f7170 RDI: 0000000000000002
>> [ 9.615606] RBP: 00007ffd584f7150 R08: 0000000000000000 R09: 0000000000000000
>> [ 9.615606] R10: 0000000000000000 R11: 0000000000000206 R12: 0000000000000001
>> [ 9.615607] R13: 0000000000000000 R14: 000055bd9b4e01b8 R15: 00007f2516801000
>> [ 9.615609] </TASK> [ 9.615610] Modules linked in:
>> [ 9.615611] CR2: ffff9eb419247000
>> [ 9.615613] ---[ end trace 0000000000000000 ]---
>> [ 9.615614] RIP: 0010:intel_bts_disable_local+0x18/0x50
>> [ 9.615615] Code: 40 00 90 90 90 90 90 90 90 90 90 90 90 90 90 90 90 90 66 0f 1f 00 0f 1f 44 00 00 65 48 8b 0d bf 1f 7b 02 48 8b 05 e0 b7 7d 02 <8b> 94 01 00 20 00 00 83 fa 02 75 1f 48 83 3c 08 00 74 18 65 48 8b
>> [ 9.615616] RSP: 0018:fffffe71b3e00c78 EFLAGS: 00010002
>> [ 9.615617] RAX: 0000000000000000 RBX: fffffe71b3e00ef8 RCX: ffff9eb419245000
>> [ 9.615618] RDX: 0000000000000001 RSI: 000000023eb5576c RDI: fffffe71b3e00ef8
>> [ 9.615618] RBP: fffffe71b3e00e38 R08: 0000000000000002 R09: 0000000000000002
>> [ 9.615619] R10: ffffffffa023f735 R11: ffffffffa020dfe0 R12: fffffe71b3e00ef8
>> [ 9.615619] R13: 0000000000000000 R14: ffff9eb419245000 R15: 0000000000000001
>> [ 9.615620] FS: 00007f25164ac140(0000) GS:ffff9eb419245000(0000) knlGS:0000000000000000
>> [ 9.615621] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
>> [ 9.615622] CR2: ffff9eb419247000 CR3: 000000010b31c006 CR4: 0000000000770ef0
>> [ 9.615622] DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
>> [ 9.615623] DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
>> [ 9.615624] PKRU: 55555554
>> [ 9.615624] Kernel panic - not syncing: Fatal exception in interrupt
>> [ 9.615807] Kernel Offset: 0x1f000000 from 0xffffffff81000000 (relocation range: 0xffffffff80000000-0xffffffffbfffffff)
>> Failed to run command
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] perf/x86/intel/bts: allocate bts_ctx only if necessary
2025-03-05 14:58 ` Adrian Hunter
@ 2025-03-05 16:11 ` Dave Hansen
0 siblings, 0 replies; 8+ messages in thread
From: Dave Hansen @ 2025-03-05 16:11 UTC (permalink / raw)
To: Adrian Hunter, Jiri Olsa
Cc: lirongqing, peterz, mingo, acme, namhyung, mark.rutland,
alexander.shishkin, irogers, kan.liang, tglx, bp, dave.hansen,
x86, hpa, linux-perf-users, linux-kernel
Jiri, thanks for the report!
On 3/5/25 06:58, Adrian Hunter wrote:
> It looks like there are 3 functions affected:
>
> intel_bts_enable_local()
> intel_bts_disable_local()
> intel_bts_interrupt()
>
> Perhaps make them static calls?
That, or a few:
if (!bts_ctx)
return;
if you're not feeling as fancy would do.
Would someone be interested in sending an actual tested patch, ideally
the patch author of the regression? <hint, hint>
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2025-03-05 16:11 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <20241030063521.4025-1-lirongqing@baidu.com>
2024-10-30 7:26 ` [PATCH] perf/x86/intel/bts: allocate bts_ctx only if necessary Alexander Shishkin
2025-01-22 7:41 lirongqing
2025-01-23 18:36 ` Adrian Hunter
2025-01-23 18:54 ` Liang, Kan
2025-03-05 13:47 ` Jiri Olsa
2025-03-05 13:54 ` Jiri Olsa
2025-03-05 14:58 ` Adrian Hunter
2025-03-05 16:11 ` Dave Hansen
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).