[PATCH 0/3] arm_pmu: acpi: avoid allocations in atomic context

[PATCH 0/3] arm_pmu: acpi: avoid allocations in atomic context

[Mark Rutland]

This series attempts to make the arm_pmu ACPI probing code lpay nicely
with PREEMPT_RT by moving work out of atomic context.

The arm_pmu ACPI probing code tries to do a number of things in atomic
context which is generally not good, and especially problematic for
PREEMPT_RT, as reported by Valentin and Pierre:

  https://lore.kernel.org/all/20210810134127.1394269-2-valentin.schneider@arm.com/
  https://lore.kernel.org/linux-arm-kernel/20220912155105.1443303-1-pierre.gondois@arm.com/

We'd previously tried to bodge around this, e.g. in commits:

* 0dc1a1851af1d593 ("arm_pmu: add armpmu_alloc_atomic()")
* 167e61438da0664c ("arm_pmu: acpi: request IRQs up-front")

... but this isn't good enough for PREEMPT_RT, and as reported by Pierre
the probing code can trigger splats:

| BUG: sleeping function called from invalid context at kernel/locking/spinlock_rt.c:46
| in_atomic(): 0, irqs_disabled(): 128, non_block: 0, pid: 24, name: cpuhp/0
| preempt_count: 0, expected: 0
| RCU nest depth: 0, expected: 0
| 3 locks held by cpuhp/0/24:
| #0: ffffd8a22c8870d0 (cpu_hotplug_lock){++++}-{0:0}, at: cpuhp_thread_fun (linux/kernel/cpu.c:754)
| #1: ffffd8a22c887120 (cpuhp_state-up){+.+.}-{0:0}, at: cpuhp_thread_fun (linux/kernel/cpu.c:754)
| #2: ffff083e7f0d97b8 ((&c->lock)){+.+.}-{3:3}, at: ___slab_alloc (linux/mm/slub.c:2954)
| irq event stamp: 42
| hardirqs last enabled at (41): finish_task_switch (linux/./arch/arm64/include/asm/irqflags.h:35)
| hardirqs last disabled at (42): cpuhp_thread_fun (linux/kernel/cpu.c:776 (discriminator 1))
| softirqs last enabled at (0): copy_process (linux/./include/linux/lockdep.h:191)
| softirqs last disabled at (0): 0x0
| CPU: 0 PID: 24 Comm: cpuhp/0 Tainted: G        W         5.19.0-rc3-rt4-custom-piegon01-rt_0 #142
| Hardware name: WIWYNN Mt.Jade Server System B81.03001.0005/Mt.Jade Motherboard, BIOS 1.08.20220218 (SCP: 1.08.20220218) 2022/02/18
| Call trace:
| dump_backtrace (linux/arch/arm64/kernel/stacktrace.c:200)
| show_stack (linux/arch/arm64/kernel/stacktrace.c:207)
| dump_stack_lvl (linux/lib/dump_stack.c:107)
| dump_stack (linux/lib/dump_stack.c:114)
| __might_resched (linux/kernel/sched/core.c:9929)
| rt_spin_lock (linux/kernel/locking/rtmutex.c:1732 (discriminator 4))
| ___slab_alloc (linux/mm/slub.c:2954)
| __slab_alloc.isra.0 (linux/mm/slub.c:3116)
| kmem_cache_alloc_trace (linux/mm/slub.c:3207)
| __armpmu_alloc (linux/./include/linux/slab.h:600)
| armpmu_alloc_atomic (linux/drivers/perf/arm_pmu.c:927)
| arm_pmu_acpi_cpu_starting (linux/drivers/perf/arm_pmu_acpi.c:204)
| cpuhp_invoke_callback (linux/kernel/cpu.c:192)
| cpuhp_thread_fun (linux/kernel/cpu.c:777 (discriminator 3))
| smpboot_thread_fn (linux/kernel/smpboot.c:164 (discriminator 3))
| kthread (linux/kernel/kthread.c:376)
| ret_from_fork (linux/arch/arm64/kernel/entry.S:868)

Thomas Gleixner suggested that we could pre-allocate structures to avoid
this issue:

  https://lore.kernel.org/all/87y299oyyq.ffs@tglx/

... and Pierre implemented that:

  https://lore.kernel.org/linux-arm-kernel/20220912155105.1443303-1-pierre.gondois@arm.com/

... but in practice this gets pretty hairy due to having to manage the
lifetime of those pre-allocated objects across various stages of the
probing flow.

This series reworks the code to perform all the allocation and
registration with perf at boot time, by scannign the set of online CPUs
and regsiter a PMU for each unique MIDR (which we use today to identify
distinct PMUs). This avoids the need for allocation in the hotplug
paths, and brings the ACPI probing code into line with the DT/platform
probing code.

When a CPU is late hotplugged, either:

(a) It matches an existing PMU's MIDR, and will be associated with that
    PMU.

(b) It does not match an existing PMU's MIDR, and will not be
    associated with a PMU (and a warning is logged to dmesg).

    Aside from the warning, this matches the existing behaviour, as we
    only register CPU PMUs with perf at boot time, and not for late
    hotplugged CPUs.

I've tested the series in a VM, using ACPI and faked MIDR values to test
a few homogeneous and heterogeneous configurations, using the 'maxcpus'
kernel argument to test the late-hotplug behaviour:

* On a system where all CPUs have the same MIDR, late-onlining a CPU causes it
  to be associated with a matching PMU:

  | # ls /sys/bus/event_source/devices/
  | armv8_pmuv3_0  breakpoint     software       tracepoint
  | # cat /sys/bus/event_source/devices/armv8_pmuv3_0/cpus 
  | 0-7
  | # echo 1 > /sys/devices/system/cpu/cpu10/online 
  | Detected PIPT I-cache on CPU10
  | GICv3: CPU10: found redistributor a region 0:0x00000000081e0000
  | GICv3: CPU10: using allocated LPI pending table @0x00000000402b0000
  | CPU10: Booted secondary processor 0x000000000a [0x431f0af1]
  | # ls /sys/bus/event_source/devices/
  | armv8_pmuv3_0  breakpoint     software       tracepoint
  | # cat /sys/bus/event_source/devices/armv8_pmuv3_0/cpus 
  | 0-7,10

* On a system where all CPUs have a unique MIDR, each of the boot-time
  CPUs gets a unique PMU:

  | # ls /sys/bus/event_source/devices/
  | armv8_pmuv3_0  armv8_pmuv3_3  armv8_pmuv3_6  software
  | armv8_pmuv3_1  armv8_pmuv3_4  armv8_pmuv3_7  tracepoint
  | armv8_pmuv3_2  armv8_pmuv3_5  breakpoint

* On a system where all CPUs have a unique MIDR, late-onlining a CPU
  results in that CPU not being associated with a PMU, but the CPU is
  successfully onlined:

  | # echo 1 > /sys/devices/system/cpu/cpu8/online
  | Detected PIPT I-cache on CPU8
  | GICv3: CPU8: found redistributor 8 region 0:0x00000000081a0000
  | GICv3: CPU8: using allocated LPI pending table @0x0000000040290000
  | Unable to associate CPU8 with a PMU
  | CPU8: Booted secondary processor 0x0000000008 [0x431f0af1]

Thanks,
Mark.

Mark Rutland (3):
  arm_pmu: acpi: factor out PMU<->CPU association
  arm_pmu: factor out PMU matching
  arm_pmu: rework ACPI probing

 drivers/perf/arm_pmu.c       |  17 +-----
 drivers/perf/arm_pmu_acpi.c  | 113 ++++++++++++++++++++---------------
 include/linux/perf/arm_pmu.h |   1 -
 3 files changed, 69 insertions(+), 62 deletions(-)

-- 
2.30.2


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH 1/3] arm_pmu: acpi: factor out PMU<->CPU association

[Mark Rutland]

A subsequent patch will rework the ACPI probing of PMUs, and we'll need
to associate a CPU with a PMU in two separate paths.

Factor out the association logic into a helper function so that it can
be reused.

There should be no functional change as a result of this patch.

Signed-off-by: Mark Rutland <mark.rutland@arm.com>
Cc: Pierre Gondois <pierre.gondois@arm.com>
Cc: Will Deacon <will@kernel.org>
---
 drivers/perf/arm_pmu_acpi.c | 29 +++++++++++++++++------------
 1 file changed, 17 insertions(+), 12 deletions(-)

diff --git a/drivers/perf/arm_pmu_acpi.c b/drivers/perf/arm_pmu_acpi.c
index 96ffadd654ff..a52a4aafd629 100644
--- a/drivers/perf/arm_pmu_acpi.c
+++ b/drivers/perf/arm_pmu_acpi.c
@@ -242,6 +242,22 @@ static bool pmu_irq_matches(struct arm_pmu *pmu, int irq)
 	return true;
 }
 
+static void arm_pmu_acpi_associate_pmu_cpu(struct arm_pmu *pmu,
+					   unsigned int cpu)
+{
+	int irq = per_cpu(pmu_irqs, cpu);
+
+	per_cpu(probed_pmus, cpu) = pmu;
+
+	if (pmu_irq_matches(pmu, irq)) {
+		struct pmu_hw_events __percpu *hw_events;
+		hw_events = pmu->hw_events;
+		per_cpu(hw_events->irq, cpu) = irq;
+	}
+
+	cpumask_set_cpu(cpu, &pmu->supported_cpus);
+}
+
 /*
  * This must run before the common arm_pmu hotplug logic, so that we can
  * associate a CPU and its interrupt before the common code tries to manage the
@@ -254,27 +270,16 @@ static bool pmu_irq_matches(struct arm_pmu *pmu, int irq)
 static int arm_pmu_acpi_cpu_starting(unsigned int cpu)
 {
 	struct arm_pmu *pmu;
-	struct pmu_hw_events __percpu *hw_events;
-	int irq;
 
 	/* If we've already probed this CPU, we have nothing to do */
 	if (per_cpu(probed_pmus, cpu))
 		return 0;
 
-	irq = per_cpu(pmu_irqs, cpu);
-
 	pmu = arm_pmu_acpi_find_alloc_pmu();
 	if (!pmu)
 		return -ENOMEM;
 
-	per_cpu(probed_pmus, cpu) = pmu;
-
-	if (pmu_irq_matches(pmu, irq)) {
-		hw_events = pmu->hw_events;
-		per_cpu(hw_events->irq, cpu) = irq;
-	}
-
-	cpumask_set_cpu(cpu, &pmu->supported_cpus);
+	arm_pmu_acpi_associate_pmu_cpu(pmu, cpu);
 
 	/*
 	 * Ideally, we'd probe the PMU here when we find the first matching
-- 
2.30.2


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH 2/3] arm_pmu: factor out PMU matching

[Mark Rutland]

A subsequent patch will rework the ACPI probing of PMUs, and we'll need
to match a CPU with a known cpuid in two separate paths.

Factor out the matching logic into a helper function so that it can be
reused.

There should be no functional change as a result of this patch.

Signed-off-by: Mark Rutland <mark.rutland@arm.com>
Cc: Pierre Gondois <pierre.gondois@arm.com>
Cc: Will Deacon <will@kernel.org>
---
 drivers/perf/arm_pmu_acpi.c | 15 +++++++++++++--
 1 file changed, 13 insertions(+), 2 deletions(-)

diff --git a/drivers/perf/arm_pmu_acpi.c b/drivers/perf/arm_pmu_acpi.c
index a52a4aafd629..99abea3b2cc9 100644
--- a/drivers/perf/arm_pmu_acpi.c
+++ b/drivers/perf/arm_pmu_acpi.c
@@ -187,7 +187,7 @@ static int arm_pmu_acpi_parse_irqs(void)
 	return err;
 }
 
-static struct arm_pmu *arm_pmu_acpi_find_alloc_pmu(void)
+static struct arm_pmu *arm_pmu_acpi_find_pmu(void)
 {
 	unsigned long cpuid = read_cpuid_id();
 	struct arm_pmu *pmu;
@@ -201,6 +201,17 @@ static struct arm_pmu *arm_pmu_acpi_find_alloc_pmu(void)
 		return pmu;
 	}
 
+	return NULL;
+}
+
+static struct arm_pmu *arm_pmu_acpi_find_alloc_pmu(void)
+{
+	struct arm_pmu *pmu;
+
+	pmu = arm_pmu_acpi_find_pmu();
+	if (pmu)
+		return pmu;
+
 	pmu = armpmu_alloc_atomic();
 	if (!pmu) {
 		pr_warn("Unable to allocate PMU for CPU%d\n",
@@ -208,7 +219,7 @@ static struct arm_pmu *arm_pmu_acpi_find_alloc_pmu(void)
 		return NULL;
 	}
 
-	pmu->acpi_cpuid = cpuid;
+	pmu->acpi_cpuid = read_cpuid_id();
 
 	return pmu;
 }
-- 
2.30.2


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH 3/3] arm_pmu: rework ACPI probing

[Mark Rutland]

The current ACPI PMU probing logic tries to associate PMUs with CPUs
when the CPU is first brought online, in order to handle late hotplug,
though PMUs are only registered during early boot, and so for late
hotplugged CPUs this can only associate the CPU with an existing PMU.

We tried to be clever and the have the arm_pmu_acpi_cpu_starting()
callback allocate a struct arm_pmu when no matching instance is found,
in order to avoid duplication of logic. However, as above this doesn't
do anything useful for late hotplugged CPUs, and this requires us to
allocate memory in an atomic context, which is especially problematic
for PREEMPT_RT, as reported by Valentin and Pierre.

This patch reworks the probing to detect PMUs for all online CPUs in the
arm_pmu_acpi_probe() function, which is more aligned with how DT probing
works. The arm_pmu_acpi_cpu_starting() callback only tries to associate
CPUs with an existing arm_pmu instance, avoiding the problem of
allocating in atomic context.

Note that as we didn't previously register PMUs for late-hotplugged
CPUs, this change doesn't result in a loss of existing functionality,
though we will now warn when we cannot associate a CPU with a PMU.

This change allows us to pull the hotplug callback registration into the
arm_pmu_acpi_probe() function, as we no longer need the callbacks to be
invoked shortly after probing the boot CPUs, and can register it without
invoking the calls.

For the moment the arm_pmu_acpi_init() initcall remains to register the
SPE PMU, though in future this should probably be moved elsewhere (e.g.
the arm64 ACPI init code), since this doesn't need to be tied to the
regular CPU PMU code.

Signed-off-by: Mark Rutland <mark.rutland@arm.com>
Reported-by: Valentin Schneider <valentin.schneider@arm.com>
Link: https://lore.kernel.org/r/20210810134127.1394269-2-valentin.schneider@arm.com/
Reported-by: Pierre Gondois <pierre.gondois@arm.com>
Link: https://lore.kernel.org/linux-arm-kernel/20220912155105.1443303-1-pierre.gondois@arm.com/
Cc: Pierre Gondois <pierre.gondois@arm.com>
Cc: Valentin Schneider <vschneid@redhat.com>
Cc: Will Deacon <will@kernel.org>
---
 drivers/perf/arm_pmu.c       | 17 ++-----
 drivers/perf/arm_pmu_acpi.c  | 95 +++++++++++++++++++-----------------
 include/linux/perf/arm_pmu.h |  1 -
 3 files changed, 52 insertions(+), 61 deletions(-)

diff --git a/drivers/perf/arm_pmu.c b/drivers/perf/arm_pmu.c
index 3f07df5a7e95..82a6d22e8ee2 100644
--- a/drivers/perf/arm_pmu.c
+++ b/drivers/perf/arm_pmu.c
@@ -861,16 +861,16 @@ static void cpu_pmu_destroy(struct arm_pmu *cpu_pmu)
 					    &cpu_pmu->node);
 }
 
-static struct arm_pmu *__armpmu_alloc(gfp_t flags)
+struct arm_pmu *armpmu_alloc(void)
 {
 	struct arm_pmu *pmu;
 	int cpu;
 
-	pmu = kzalloc(sizeof(*pmu), flags);
+	pmu = kzalloc(sizeof(*pmu), GFP_KERNEL);
 	if (!pmu)
 		goto out;
 
-	pmu->hw_events = alloc_percpu_gfp(struct pmu_hw_events, flags);
+	pmu->hw_events = alloc_percpu_gfp(struct pmu_hw_events, GFP_KERNEL);
 	if (!pmu->hw_events) {
 		pr_info("failed to allocate per-cpu PMU data.\n");
 		goto out_free_pmu;
@@ -916,17 +916,6 @@ static struct arm_pmu *__armpmu_alloc(gfp_t flags)
 	return NULL;
 }
 
-struct arm_pmu *armpmu_alloc(void)
-{
-	return __armpmu_alloc(GFP_KERNEL);
-}
-
-struct arm_pmu *armpmu_alloc_atomic(void)
-{
-	return __armpmu_alloc(GFP_ATOMIC);
-}
-
-
 void armpmu_free(struct arm_pmu *pmu)
 {
 	free_percpu(pmu->hw_events);
diff --git a/drivers/perf/arm_pmu_acpi.c b/drivers/perf/arm_pmu_acpi.c
index 99abea3b2cc9..a085e45b509e 100644
--- a/drivers/perf/arm_pmu_acpi.c
+++ b/drivers/perf/arm_pmu_acpi.c
@@ -13,6 +13,7 @@
 #include <linux/percpu.h>
 #include <linux/perf/arm_pmu.h>
 
+#include <asm/cpu.h>
 #include <asm/cputype.h>
 
 static DEFINE_PER_CPU(struct arm_pmu *, probed_pmus);
@@ -204,26 +205,6 @@ static struct arm_pmu *arm_pmu_acpi_find_pmu(void)
 	return NULL;
 }
 
-static struct arm_pmu *arm_pmu_acpi_find_alloc_pmu(void)
-{
-	struct arm_pmu *pmu;
-
-	pmu = arm_pmu_acpi_find_pmu();
-	if (pmu)
-		return pmu;
-
-	pmu = armpmu_alloc_atomic();
-	if (!pmu) {
-		pr_warn("Unable to allocate PMU for CPU%d\n",
-			smp_processor_id());
-		return NULL;
-	}
-
-	pmu->acpi_cpuid = read_cpuid_id();
-
-	return pmu;
-}
-
 /*
  * Check whether the new IRQ is compatible with those already associated with
  * the PMU (e.g. we don't have mismatched PPIs).
@@ -286,26 +267,45 @@ static int arm_pmu_acpi_cpu_starting(unsigned int cpu)
 	if (per_cpu(probed_pmus, cpu))
 		return 0;
 
-	pmu = arm_pmu_acpi_find_alloc_pmu();
-	if (!pmu)
-		return -ENOMEM;
+	pmu = arm_pmu_acpi_find_pmu();
+	if (!pmu) {
+		pr_warn_ratelimited("Unable to associate CPU%d with a PMU\n",
+				    cpu);
+		return 0;
+	}
 
 	arm_pmu_acpi_associate_pmu_cpu(pmu, cpu);
-
-	/*
-	 * Ideally, we'd probe the PMU here when we find the first matching
-	 * CPU. We can't do that for several reasons; see the comment in
-	 * arm_pmu_acpi_init().
-	 *
-	 * So for the time being, we're done.
-	 */
 	return 0;
 }
 
+static void arm_pmu_acpi_probe_matching_cpus(struct arm_pmu *pmu,
+					     unsigned long cpuid)
+{
+	int cpu;
+
+	for_each_online_cpu(cpu) {
+		unsigned long cpu_cpuid = per_cpu(cpu_data, cpu).reg_midr;
+
+		if (cpu_cpuid == cpuid)
+			arm_pmu_acpi_associate_pmu_cpu(pmu, cpu);
+	}
+}
+
 int arm_pmu_acpi_probe(armpmu_init_fn init_fn)
 {
 	int pmu_idx = 0;
-	int cpu, ret;
+	unsigned int cpu;
+	int ret;
+
+	ret = arm_pmu_acpi_parse_irqs();
+	if (ret)
+		return ret;
+
+	ret = cpuhp_setup_state_nocalls(CPUHP_AP_PERF_ARM_ACPI_STARTING,
+					"perf/arm/pmu_acpi:starting",
+					arm_pmu_acpi_cpu_starting, NULL);
+	if (ret)
+		return ret;
 
 	/*
 	 * Initialise and register the set of PMUs which we know about right
@@ -320,13 +320,26 @@ int arm_pmu_acpi_probe(armpmu_init_fn init_fn)
 	 * For the moment, as with the platform/DT case, we need at least one
 	 * of a PMU's CPUs to be online at probe time.
 	 */
-	for_each_possible_cpu(cpu) {
+	for_each_online_cpu(cpu) {
 		struct arm_pmu *pmu = per_cpu(probed_pmus, cpu);
+		unsigned long cpuid;
 		char *base_name;
 
-		if (!pmu || pmu->name)
+		/* If we've already probed this CPU, we have nothing to do */
+		if (pmu)
 			continue;
 
+		pmu = armpmu_alloc();
+		if (!pmu) {
+			pr_warn("Unable to allocate PMU for CPU%d\n",
+				cpu);
+		}
+
+		cpuid = per_cpu(cpu_data, cpu).reg_midr;
+		pmu->acpi_cpuid = cpuid;
+
+		arm_pmu_acpi_probe_matching_cpus(pmu, cpuid);
+
 		ret = init_fn(pmu);
 		if (ret == -ENODEV) {
 			/* PMU not handled by this driver, or not present */
@@ -351,26 +364,16 @@ int arm_pmu_acpi_probe(armpmu_init_fn init_fn)
 		}
 	}
 
-	return 0;
+	return ret;
 }
 
 static int arm_pmu_acpi_init(void)
 {
-	int ret;
-
 	if (acpi_disabled)
 		return 0;
 
 	arm_spe_acpi_register_device();
 
-	ret = arm_pmu_acpi_parse_irqs();
-	if (ret)
-		return ret;
-
-	ret = cpuhp_setup_state(CPUHP_AP_PERF_ARM_ACPI_STARTING,
-				"perf/arm/pmu_acpi:starting",
-				arm_pmu_acpi_cpu_starting, NULL);
-
-	return ret;
+	return 0;
 }
 subsys_initcall(arm_pmu_acpi_init)
diff --git a/include/linux/perf/arm_pmu.h b/include/linux/perf/arm_pmu.h
index 0407a38b470a..049908af3595 100644
--- a/include/linux/perf/arm_pmu.h
+++ b/include/linux/perf/arm_pmu.h
@@ -173,7 +173,6 @@ void kvm_host_pmu_init(struct arm_pmu *pmu);
 
 /* Internal functions only for core arm_pmu code */
 struct arm_pmu *armpmu_alloc(void);
-struct arm_pmu *armpmu_alloc_atomic(void);
 void armpmu_free(struct arm_pmu *pmu);
 int armpmu_register(struct arm_pmu *pmu);
 int armpmu_request_irq(int irq, int cpu);
-- 
2.30.2


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 0/3] arm_pmu: acpi: avoid allocations in atomic context

[Pierre Gondois]

Hello Mark,

Reviewed-and-tested-by: Pierre Gondois <pierre.gondois@arm.com>

Thanks again,
Pierre

On 9/30/22 13:18, Mark Rutland wrote:
> This series attempts to make the arm_pmu ACPI probing code lpay nicely
> with PREEMPT_RT by moving work out of atomic context.
> 
> The arm_pmu ACPI probing code tries to do a number of things in atomic
> context which is generally not good, and especially problematic for
> PREEMPT_RT, as reported by Valentin and Pierre:
> 
>    https://lore.kernel.org/all/20210810134127.1394269-2-valentin.schneider@arm.com/
>    https://lore.kernel.org/linux-arm-kernel/20220912155105.1443303-1-pierre.gondois@arm.com/
> 
> We'd previously tried to bodge around this, e.g. in commits:
> 
> * 0dc1a1851af1d593 ("arm_pmu: add armpmu_alloc_atomic()")
> * 167e61438da0664c ("arm_pmu: acpi: request IRQs up-front")
> 
> ... but this isn't good enough for PREEMPT_RT, and as reported by Pierre
> the probing code can trigger splats:
> 
> | BUG: sleeping function called from invalid context at kernel/locking/spinlock_rt.c:46
> | in_atomic(): 0, irqs_disabled(): 128, non_block: 0, pid: 24, name: cpuhp/0
> | preempt_count: 0, expected: 0
> | RCU nest depth: 0, expected: 0
> | 3 locks held by cpuhp/0/24:
> | #0: ffffd8a22c8870d0 (cpu_hotplug_lock){++++}-{0:0}, at: cpuhp_thread_fun (linux/kernel/cpu.c:754)
> | #1: ffffd8a22c887120 (cpuhp_state-up){+.+.}-{0:0}, at: cpuhp_thread_fun (linux/kernel/cpu.c:754)
> | #2: ffff083e7f0d97b8 ((&c->lock)){+.+.}-{3:3}, at: ___slab_alloc (linux/mm/slub.c:2954)
> | irq event stamp: 42
> | hardirqs last enabled at (41): finish_task_switch (linux/./arch/arm64/include/asm/irqflags.h:35)
> | hardirqs last disabled at (42): cpuhp_thread_fun (linux/kernel/cpu.c:776 (discriminator 1))
> | softirqs last enabled at (0): copy_process (linux/./include/linux/lockdep.h:191)
> | softirqs last disabled at (0): 0x0
> | CPU: 0 PID: 24 Comm: cpuhp/0 Tainted: G        W         5.19.0-rc3-rt4-custom-piegon01-rt_0 #142
> | Hardware name: WIWYNN Mt.Jade Server System B81.03001.0005/Mt.Jade Motherboard, BIOS 1.08.20220218 (SCP: 1.08.20220218) 2022/02/18
> | Call trace:
> | dump_backtrace (linux/arch/arm64/kernel/stacktrace.c:200)
> | show_stack (linux/arch/arm64/kernel/stacktrace.c:207)
> | dump_stack_lvl (linux/lib/dump_stack.c:107)
> | dump_stack (linux/lib/dump_stack.c:114)
> | __might_resched (linux/kernel/sched/core.c:9929)
> | rt_spin_lock (linux/kernel/locking/rtmutex.c:1732 (discriminator 4))
> | ___slab_alloc (linux/mm/slub.c:2954)
> | __slab_alloc.isra.0 (linux/mm/slub.c:3116)
> | kmem_cache_alloc_trace (linux/mm/slub.c:3207)
> | __armpmu_alloc (linux/./include/linux/slab.h:600)
> | armpmu_alloc_atomic (linux/drivers/perf/arm_pmu.c:927)
> | arm_pmu_acpi_cpu_starting (linux/drivers/perf/arm_pmu_acpi.c:204)
> | cpuhp_invoke_callback (linux/kernel/cpu.c:192)
> | cpuhp_thread_fun (linux/kernel/cpu.c:777 (discriminator 3))
> | smpboot_thread_fn (linux/kernel/smpboot.c:164 (discriminator 3))
> | kthread (linux/kernel/kthread.c:376)
> | ret_from_fork (linux/arch/arm64/kernel/entry.S:868)
> 
> Thomas Gleixner suggested that we could pre-allocate structures to avoid
> this issue:
> 
>    https://lore.kernel.org/all/87y299oyyq.ffs@tglx/
> 
> ... and Pierre implemented that:
> 
>    https://lore.kernel.org/linux-arm-kernel/20220912155105.1443303-1-pierre.gondois@arm.com/
> 
> ... but in practice this gets pretty hairy due to having to manage the
> lifetime of those pre-allocated objects across various stages of the
> probing flow.
> 
> This series reworks the code to perform all the allocation and
> registration with perf at boot time, by scannign the set of online CPUs
> and regsiter a PMU for each unique MIDR (which we use today to identify
> distinct PMUs). This avoids the need for allocation in the hotplug
> paths, and brings the ACPI probing code into line with the DT/platform
> probing code.
> 
> When a CPU is late hotplugged, either:
> 
> (a) It matches an existing PMU's MIDR, and will be associated with that
>      PMU.
> 
> (b) It does not match an existing PMU's MIDR, and will not be
>      associated with a PMU (and a warning is logged to dmesg).
> 
>      Aside from the warning, this matches the existing behaviour, as we
>      only register CPU PMUs with perf at boot time, and not for late
>      hotplugged CPUs.
> 
> I've tested the series in a VM, using ACPI and faked MIDR values to test
> a few homogeneous and heterogeneous configurations, using the 'maxcpus'
> kernel argument to test the late-hotplug behaviour:
> 
> * On a system where all CPUs have the same MIDR, late-onlining a CPU causes it
>    to be associated with a matching PMU:
> 
>    | # ls /sys/bus/event_source/devices/
>    | armv8_pmuv3_0  breakpoint     software       tracepoint
>    | # cat /sys/bus/event_source/devices/armv8_pmuv3_0/cpus
>    | 0-7
>    | # echo 1 > /sys/devices/system/cpu/cpu10/online
>    | Detected PIPT I-cache on CPU10
>    | GICv3: CPU10: found redistributor a region 0:0x00000000081e0000
>    | GICv3: CPU10: using allocated LPI pending table @0x00000000402b0000
>    | CPU10: Booted secondary processor 0x000000000a [0x431f0af1]
>    | # ls /sys/bus/event_source/devices/
>    | armv8_pmuv3_0  breakpoint     software       tracepoint
>    | # cat /sys/bus/event_source/devices/armv8_pmuv3_0/cpus
>    | 0-7,10
> 
> * On a system where all CPUs have a unique MIDR, each of the boot-time
>    CPUs gets a unique PMU:
> 
>    | # ls /sys/bus/event_source/devices/
>    | armv8_pmuv3_0  armv8_pmuv3_3  armv8_pmuv3_6  software
>    | armv8_pmuv3_1  armv8_pmuv3_4  armv8_pmuv3_7  tracepoint
>    | armv8_pmuv3_2  armv8_pmuv3_5  breakpoint
> 
> * On a system where all CPUs have a unique MIDR, late-onlining a CPU
>    results in that CPU not being associated with a PMU, but the CPU is
>    successfully onlined:
> 
>    | # echo 1 > /sys/devices/system/cpu/cpu8/online
>    | Detected PIPT I-cache on CPU8
>    | GICv3: CPU8: found redistributor 8 region 0:0x00000000081a0000
>    | GICv3: CPU8: using allocated LPI pending table @0x0000000040290000
>    | Unable to associate CPU8 with a PMU
>    | CPU8: Booted secondary processor 0x0000000008 [0x431f0af1]
> 
> Thanks,
> Mark.
> 
> Mark Rutland (3):
>    arm_pmu: acpi: factor out PMU<->CPU association
>    arm_pmu: factor out PMU matching
>    arm_pmu: rework ACPI probing
> 
>   drivers/perf/arm_pmu.c       |  17 +-----
>   drivers/perf/arm_pmu_acpi.c  | 113 ++++++++++++++++++++---------------
>   include/linux/perf/arm_pmu.h |   1 -
>   3 files changed, 69 insertions(+), 62 deletions(-)
> 

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 0/3] arm_pmu: acpi: avoid allocations in atomic context

[Kunkun Jiang]

Hi Mark:

On 2022/9/30 19:18, Mark Rutland wrote:
> I've tested the series in a VM, using ACPI and faked MIDR values to test
> a few homogeneous and heterogeneous configurations, using the 'maxcpus'
> kernel argument to test the late-hotplug behaviour:
I did the same test as you(without this series) and encountered the same 
problem.
Nice to see this series while asking for help in the community. But why not
register their own CPU PMU for late hotplugged cpus with a unique MIDR? 
Are there
any restrictions here?

Thanks,
Kunkun Jiang
>
> * On a system where all CPUs have the same MIDR, late-onlining a CPU causes it
>    to be associated with a matching PMU:
>
>    | # ls /sys/bus/event_source/devices/
>    | armv8_pmuv3_0  breakpoint     software       tracepoint
>    | # cat /sys/bus/event_source/devices/armv8_pmuv3_0/cpus
>    | 0-7
>    | # echo 1 > /sys/devices/system/cpu/cpu10/online
>    | Detected PIPT I-cache on CPU10
>    | GICv3: CPU10: found redistributor a region 0:0x00000000081e0000
>    | GICv3: CPU10: using allocated LPI pending table @0x00000000402b0000
>    | CPU10: Booted secondary processor 0x000000000a [0x431f0af1]
>    | # ls /sys/bus/event_source/devices/
>    | armv8_pmuv3_0  breakpoint     software       tracepoint
>    | # cat /sys/bus/event_source/devices/armv8_pmuv3_0/cpus
>    | 0-7,10
>
> * On a system where all CPUs have a unique MIDR, each of the boot-time
>    CPUs gets a unique PMU:
>
>    | # ls /sys/bus/event_source/devices/
>    | armv8_pmuv3_0  armv8_pmuv3_3  armv8_pmuv3_6  software
>    | armv8_pmuv3_1  armv8_pmuv3_4  armv8_pmuv3_7  tracepoint
>    | armv8_pmuv3_2  armv8_pmuv3_5  breakpoint
>
> * On a system where all CPUs have a unique MIDR, late-onlining a CPU
>    results in that CPU not being associated with a PMU, but the CPU is
>    successfully onlined:
>
>    | # echo 1 > /sys/devices/system/cpu/cpu8/online
>    | Detected PIPT I-cache on CPU8
>    | GICv3: CPU8: found redistributor 8 region 0:0x00000000081a0000
>    | GICv3: CPU8: using allocated LPI pending table @0x0000000040290000
>    | Unable to associate CPU8 with a PMU
>    | CPU8: Booted secondary processor 0x0000000008 [0x431f0af1]
>
> Thanks,
> Mark.
>
> Mark Rutland (3):
>    arm_pmu: acpi: factor out PMU<->CPU association
>    arm_pmu: factor out PMU matching
>    arm_pmu: rework ACPI probing
>
>   drivers/perf/arm_pmu.c       |  17 +-----
>   drivers/perf/arm_pmu_acpi.c  | 113 ++++++++++++++++++++---------------
>   include/linux/perf/arm_pmu.h |   1 -
>   3 files changed, 69 insertions(+), 62 deletions(-)
>

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 0/3] arm_pmu: acpi: avoid allocations in atomic context

[Mark Rutland]

On Tue, Oct 18, 2022 at 09:53:09PM +0800, Kunkun Jiang wrote:
> Hi Mark:
> 
> On 2022/9/30 19:18, Mark Rutland wrote:
> > I've tested the series in a VM, using ACPI and faked MIDR values to test
> > a few homogeneous and heterogeneous configurations, using the 'maxcpus'
> > kernel argument to test the late-hotplug behaviour:
> I did the same test as you(without this series) and encountered the same
> problem.
> Nice to see this series while asking for help in the community. But why not
> register their own CPU PMU for late hotplugged cpus with a unique MIDR? Are
> there
> any restrictions here?

We can't allcoate memory and so on during the onlining, and generally we
do not expect late hotplug of CPU types that weren't present during
boot (as e.g. errata handling won't work).

Thanks,
Mark.

> 
> Thanks,
> Kunkun Jiang
> > 
> > * On a system where all CPUs have the same MIDR, late-onlining a CPU causes it
> >    to be associated with a matching PMU:
> > 
> >    | # ls /sys/bus/event_source/devices/
> >    | armv8_pmuv3_0  breakpoint     software       tracepoint
> >    | # cat /sys/bus/event_source/devices/armv8_pmuv3_0/cpus
> >    | 0-7
> >    | # echo 1 > /sys/devices/system/cpu/cpu10/online
> >    | Detected PIPT I-cache on CPU10
> >    | GICv3: CPU10: found redistributor a region 0:0x00000000081e0000
> >    | GICv3: CPU10: using allocated LPI pending table @0x00000000402b0000
> >    | CPU10: Booted secondary processor 0x000000000a [0x431f0af1]
> >    | # ls /sys/bus/event_source/devices/
> >    | armv8_pmuv3_0  breakpoint     software       tracepoint
> >    | # cat /sys/bus/event_source/devices/armv8_pmuv3_0/cpus
> >    | 0-7,10
> > 
> > * On a system where all CPUs have a unique MIDR, each of the boot-time
> >    CPUs gets a unique PMU:
> > 
> >    | # ls /sys/bus/event_source/devices/
> >    | armv8_pmuv3_0  armv8_pmuv3_3  armv8_pmuv3_6  software
> >    | armv8_pmuv3_1  armv8_pmuv3_4  armv8_pmuv3_7  tracepoint
> >    | armv8_pmuv3_2  armv8_pmuv3_5  breakpoint
> > 
> > * On a system where all CPUs have a unique MIDR, late-onlining a CPU
> >    results in that CPU not being associated with a PMU, but the CPU is
> >    successfully onlined:
> > 
> >    | # echo 1 > /sys/devices/system/cpu/cpu8/online
> >    | Detected PIPT I-cache on CPU8
> >    | GICv3: CPU8: found redistributor 8 region 0:0x00000000081a0000
> >    | GICv3: CPU8: using allocated LPI pending table @0x0000000040290000
> >    | Unable to associate CPU8 with a PMU
> >    | CPU8: Booted secondary processor 0x0000000008 [0x431f0af1]
> > 
> > Thanks,
> > Mark.
> > 
> > Mark Rutland (3):
> >    arm_pmu: acpi: factor out PMU<->CPU association
> >    arm_pmu: factor out PMU matching
> >    arm_pmu: rework ACPI probing
> > 
> >   drivers/perf/arm_pmu.c       |  17 +-----
> >   drivers/perf/arm_pmu_acpi.c  | 113 ++++++++++++++++++++---------------
> >   include/linux/perf/arm_pmu.h |   1 -
> >   3 files changed, 69 insertions(+), 62 deletions(-)
> > 

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 0/3] arm_pmu: acpi: avoid allocations in atomic context

[Will Deacon]

On Fri, 30 Sep 2022 12:18:41 +0100, Mark Rutland wrote:
> This series attempts to make the arm_pmu ACPI probing code lpay nicely
> with PREEMPT_RT by moving work out of atomic context.
> 
> The arm_pmu ACPI probing code tries to do a number of things in atomic
> context which is generally not good, and especially problematic for
> PREEMPT_RT, as reported by Valentin and Pierre:
> 
> [...]

Applied to arm64 (for-next/acpi), thanks!

[1/3] arm_pmu: acpi: factor out PMU<->CPU association
      https://git.kernel.org/arm64/c/ad51b5043bb3
[2/3] arm_pmu: factor out PMU matching
      https://git.kernel.org/arm64/c/6349a2470d07
[3/3] arm_pmu: rework ACPI probing
      https://git.kernel.org/arm64/c/fe40ffdb7656

Cheers,
-- 
Will

https://fixes.arm64.dev
https://next.arm64.dev
https://will.arm64.dev

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 3/3] arm_pmu: rework ACPI probing

[Will Deacon]

On Fri, Sep 30, 2022 at 12:18:44PM +0100, Mark Rutland wrote:
> The current ACPI PMU probing logic tries to associate PMUs with CPUs
> when the CPU is first brought online, in order to handle late hotplug,
> though PMUs are only registered during early boot, and so for late
> hotplugged CPUs this can only associate the CPU with an existing PMU.
> 
> We tried to be clever and the have the arm_pmu_acpi_cpu_starting()
> callback allocate a struct arm_pmu when no matching instance is found,
> in order to avoid duplication of logic. However, as above this doesn't
> do anything useful for late hotplugged CPUs, and this requires us to
> allocate memory in an atomic context, which is especially problematic
> for PREEMPT_RT, as reported by Valentin and Pierre.
> 
> This patch reworks the probing to detect PMUs for all online CPUs in the
> arm_pmu_acpi_probe() function, which is more aligned with how DT probing
> works. The arm_pmu_acpi_cpu_starting() callback only tries to associate
> CPUs with an existing arm_pmu instance, avoiding the problem of
> allocating in atomic context.
> 
> Note that as we didn't previously register PMUs for late-hotplugged
> CPUs, this change doesn't result in a loss of existing functionality,
> though we will now warn when we cannot associate a CPU with a PMU.
> 
> This change allows us to pull the hotplug callback registration into the
> arm_pmu_acpi_probe() function, as we no longer need the callbacks to be
> invoked shortly after probing the boot CPUs, and can register it without
> invoking the calls.
> 
> For the moment the arm_pmu_acpi_init() initcall remains to register the
> SPE PMU, though in future this should probably be moved elsewhere (e.g.
> the arm64 ACPI init code), since this doesn't need to be tied to the
> regular CPU PMU code.
> 
> Signed-off-by: Mark Rutland <mark.rutland@arm.com>
> Reported-by: Valentin Schneider <valentin.schneider@arm.com>
> Link: https://lore.kernel.org/r/20210810134127.1394269-2-valentin.schneider@arm.com/
> Reported-by: Pierre Gondois <pierre.gondois@arm.com>
> Link: https://lore.kernel.org/linux-arm-kernel/20220912155105.1443303-1-pierre.gondois@arm.com/
> Cc: Pierre Gondois <pierre.gondois@arm.com>
> Cc: Valentin Schneider <vschneid@redhat.com>
> Cc: Will Deacon <will@kernel.org>
> ---
>  drivers/perf/arm_pmu.c       | 17 ++-----
>  drivers/perf/arm_pmu_acpi.c  | 95 +++++++++++++++++++-----------------
>  include/linux/perf/arm_pmu.h |  1 -
>  3 files changed, 52 insertions(+), 61 deletions(-)

[...]

> @@ -320,13 +320,26 @@ int arm_pmu_acpi_probe(armpmu_init_fn init_fn)
>  	 * For the moment, as with the platform/DT case, we need at least one
>  	 * of a PMU's CPUs to be online at probe time.
>  	 */
> -	for_each_possible_cpu(cpu) {
> +	for_each_online_cpu(cpu) {
>  		struct arm_pmu *pmu = per_cpu(probed_pmus, cpu);
> +		unsigned long cpuid;
>  		char *base_name;
>  
> -		if (!pmu || pmu->name)
> +		/* If we've already probed this CPU, we have nothing to do */
> +		if (pmu)
>  			continue;
>  
> +		pmu = armpmu_alloc();
> +		if (!pmu) {
> +			pr_warn("Unable to allocate PMU for CPU%d\n",
> +				cpu);
> +		}
> +
> +		cpuid = per_cpu(cpu_data, cpu).reg_midr;
> +		pmu->acpi_cpuid = cpuid;

I've queued this, but if armpmu_alloc() fails we now deference NULL here
whereas we should probably propagate the error.

Please can you send a fix on top of for-next/acpi?

Thanks,

Will

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 3/3] arm_pmu: rework ACPI probing

[Mark Rutland]

On Mon, Nov 07, 2022 at 07:10:18PM +0000, Will Deacon wrote:
> On Fri, Sep 30, 2022 at 12:18:44PM +0100, Mark Rutland wrote:
> > @@ -320,13 +320,26 @@ int arm_pmu_acpi_probe(armpmu_init_fn init_fn)
> >  	 * For the moment, as with the platform/DT case, we need at least one
> >  	 * of a PMU's CPUs to be online at probe time.
> >  	 */
> > -	for_each_possible_cpu(cpu) {
> > +	for_each_online_cpu(cpu) {
> >  		struct arm_pmu *pmu = per_cpu(probed_pmus, cpu);
> > +		unsigned long cpuid;
> >  		char *base_name;
> >  
> > -		if (!pmu || pmu->name)
> > +		/* If we've already probed this CPU, we have nothing to do */
> > +		if (pmu)
> >  			continue;
> >  
> > +		pmu = armpmu_alloc();
> > +		if (!pmu) {
> > +			pr_warn("Unable to allocate PMU for CPU%d\n",
> > +				cpu);
> > +		}
> > +
> > +		cpuid = per_cpu(cpu_data, cpu).reg_midr;
> > +		pmu->acpi_cpuid = cpuid;
> 
> I've queued this, but if armpmu_alloc() fails we now deference NULL here
> whereas we should probably propagate the error.

Whoops; that was meant to return -ENOMEM, as with the other allocation failure.
 
> Please can you send a fix on top of for-next/acpi?

Done:

  https://lore.kernel.org/r/20221108093725.1239563-1-mark.rutland@arm.com

Thanks,
Mark.

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel