[SPLAT 1/3] arm_pmu: Sleeping spinlocks down armpmu_alloc_atomic()

[SPLAT 1/3] arm_pmu: Sleeping spinlocks down armpmu_alloc_atomic()

[Valentin Schneider]

[    4.172817] BUG: sleeping function called from invalid context at kernel/locking/spinlock_rt.c:35
[    4.172820] in_atomic(): 0, irqs_disabled(): 128, non_block: 0, pid: 19, name: cpuhp/0
[    4.172822] 3 locks held by cpuhp/0/19:
[    4.172824] #0: ffff800011cf8690 (cpu_hotplug_lock){++++}-{0:0}, at: cpuhp_thread_fun (kernel/cpu.c:739) 
[    4.172833] #1: ffff800011cf86e0 (cpuhp_state-up){+.+.}-{0:0}, at: cpuhp_thread_fun (kernel/cpu.c:739) 
[    4.172838] #2: ffff008f36237c08 (&(&c->lock)->lock){+.+.}-{0:0}, at: ___slab_alloc (mm/slub.c:2872) 
[    4.172844] irq event stamp: 40
[    4.172845] hardirqs last enabled at (39): finish_task_switch (./arch/arm64/include/asm/irqflags.h:35 kernel/sched/sched.h:1319 kernel/sched/core.c:4531 kernel/sched/core.c:4649) 
[    4.172849] hardirqs last disabled at (40): cpuhp_thread_fun (kernel/cpu.c:761 (discriminator 1)) 
[    4.172851] softirqs last enabled at (0): copy_process (./include/linux/lockdep.h:195 ./include/linux/lockdep.h:202 ./include/linux/lockdep.h:208 ./include/linux/seqlock.h:78 kernel/fork.c:2084) 
[    4.172855] softirqs last disabled at (0): 0x0 
[    4.172860] CPU: 0 PID: 19 Comm: cpuhp/0 Tainted: G        W         5.14.0-rc4-rt6-torture+ #56
[    4.172863] Hardware name: MiTAC RAPTOR EV-883832-X3-0001/RAPTOR, BIOS 1.0 06/28/2019
[    4.172864] Call trace:
[    4.172865] dump_backtrace (arch/arm64/kernel/stacktrace.c:151) 
[    4.172867] show_stack (arch/arm64/kernel/stacktrace.c:217) 
[    4.172869] dump_stack_lvl (lib/dump_stack.c:106) 
[    4.172872] dump_stack (lib/dump_stack.c:113) 
[    4.172874] ___might_sleep (kernel/sched/core.c:9306) 
[    4.172877] rt_spin_lock (kernel/locking/rtmutex.c:1641 (discriminator 4) kernel/locking/spinlock_rt.c:30 (discriminator 4) kernel/locking/spinlock_rt.c:36 (discriminator 4) kernel/locking/spinlock_rt.c:44 (discriminator 4)) 
[    4.172879] ___slab_alloc (mm/slub.c:2872) 
[    4.172882] __slab_alloc.isra.0 (mm/slub.c:3038) 
[    4.172884] kmem_cache_alloc_trace (mm/slub.c:3129 mm/slub.c:3171 mm/slub.c:3188) 
[    4.172886] __armpmu_alloc (./include/linux/slab.h:591 ./include/linux/slab.h:721 drivers/perf/arm_pmu.c:869) 
[    4.172889] armpmu_alloc_atomic (drivers/perf/arm_pmu.c:927) 
[    4.172891] arm_pmu_acpi_cpu_starting (drivers/perf/arm_pmu_acpi.c:202 drivers/perf/arm_pmu_acpi.c:264) 
[    4.172893] cpuhp_invoke_callback (kernel/cpu.c:180) 
[    4.172895] cpuhp_thread_fun (kernel/cpu.c:762 (discriminator 3)) 
[    4.172896] smpboot_thread_fn (kernel/smpboot.c:164 (discriminator 3)) 
[    4.172900] kthread (kernel/kthread.c:327) 
[    4.172902] ret_from_fork (arch/arm64/kernel/entry.S:783) 

Re: [SPLAT 1/3] arm_pmu: Sleeping spinlocks down armpmu_alloc_atomic()

[Sebastian Andrzej Siewior]

On 2021-08-10 14:41:25 [+0100], Valentin Schneider wrote:
> [    4.172817] BUG: sleeping function called from invalid context at kernel/locking/spinlock_rt.c:35

Would it work to allocate the memory upfront and invoke the HP callback
via smp_function_call()? That is implemented via the hp-callback. It is
invoked after all CPUs are up. There is one "memory allocation" per
"node". This does not change during runtime, right?

Sebastian

Re: [SPLAT 1/3] arm_pmu: Sleeping spinlocks down armpmu_alloc_atomic()

[Thomas Gleixner]

On Tue, Aug 10 2021 at 15:54, Sebastian Andrzej Siewior wrote:
> On 2021-08-10 14:41:25 [+0100], Valentin Schneider wrote:
>> [    4.172817] BUG: sleeping function called from invalid context at kernel/locking/spinlock_rt.c:35
>
> Would it work to allocate the memory upfront and invoke the HP callback
> via smp_function_call()?

Huch? If the memory is allocated during the prepare stage then the
actual hotplug callback can just use it. So what's the SMP function call
for?

Thanks,

        tglx

Re: [SPLAT 1/3] arm_pmu: Sleeping spinlocks down armpmu_alloc_atomic()

[Sebastian Andrzej Siewior]

On 2021-08-10 19:16:19 [+0200], Thomas Gleixner wrote:
> On Tue, Aug 10 2021 at 15:54, Sebastian Andrzej Siewior wrote:
> > On 2021-08-10 14:41:25 [+0100], Valentin Schneider wrote:
> >> [    4.172817] BUG: sleeping function called from invalid context at kernel/locking/spinlock_rt.c:35
> >
> > Would it work to allocate the memory upfront and invoke the HP callback
> > via smp_function_call()?
> 
> Huch? If the memory is allocated during the prepare stage then the
> actual hotplug callback can just use it. So what's the SMP function call
> for?

You allocate the memory per-node (based on cpuid id) and you figure that
out on the target CPU. That is the smp-function call for unless there
another way to gather that information.
That is done in
 arm_pmu_acpi_find_alloc_pmu() -> read_cpuid_id()

> Thanks,
> 
>         tglx

Sebastian

Re: [SPLAT 1/3] arm_pmu: Sleeping spinlocks down armpmu_alloc_atomic()

[Thomas Gleixner]

On Tue, Aug 10 2021 at 20:39, Sebastian Andrzej Siewior wrote:
> On 2021-08-10 19:16:19 [+0200], Thomas Gleixner wrote:
>> On Tue, Aug 10 2021 at 15:54, Sebastian Andrzej Siewior wrote:
>> > On 2021-08-10 14:41:25 [+0100], Valentin Schneider wrote:
>> >> [    4.172817] BUG: sleeping function called from invalid context at kernel/locking/spinlock_rt.c:35
>> >
>> > Would it work to allocate the memory upfront and invoke the HP callback
>> > via smp_function_call()?
>> 
>> Huch? If the memory is allocated during the prepare stage then the
>> actual hotplug callback can just use it. So what's the SMP function call
>> for?
>
> You allocate the memory per-node (based on cpuid id) and you figure that
> out on the target CPU. That is the smp-function call for unless there
> another way to gather that information.
> That is done in
>  arm_pmu_acpi_find_alloc_pmu() -> read_cpuid_id()

There is no SMP function call involved. This is straight CPU hotplug
context on the upcoming CPU.

You cannot allocate upfront on the control CPU and do an SMP function
call to a not even started CPU. No idea what you are trying to do, but
this is going nowhere.

The reason why this atomic allocation is done is described in this
commit:

commit 0dc1a1851af1d593eee248b94c1277c7c7ccbbce
Author: Mark Rutland <mark.rutland@arm.com>
Date:   Mon Feb 5 16:41:58 2018 +0000

    arm_pmu: add armpmu_alloc_atomic()
    
    In ACPI systems, we don't know the makeup of CPUs until we hotplug them
    on, and thus have to allocate the PMU datastructures at hotplug time.
    Thus, we must use GFP_ATOMIC allocations.

I have no idea why ACPI does not expose topology and PMU information in
the first place, but do I really want to know?

So the easy way out is to preallocate one PMU and use that preallocation
when needed in the starting callback. Completely untested patch
below. Note, that because I'm lazy this will waste one preallocated PMU
at the end, but that's trivial enough to clean up.

Thanks,

        tglx
---
--- a/drivers/perf/arm_pmu.c
+++ b/drivers/perf/arm_pmu.c
@@ -861,12 +861,12 @@ static void cpu_pmu_destroy(struct arm_p
 					    &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;
 
@@ -916,17 +916,6 @@ static struct arm_pmu *__armpmu_alloc(gf
 	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);
--- a/drivers/perf/arm_pmu_acpi.c
+++ b/drivers/perf/arm_pmu_acpi.c
@@ -185,6 +185,8 @@ static int arm_pmu_acpi_parse_irqs(void)
 	return err;
 }
 
+static struct arm_pmu *arm_pmu_prealloced;
+
 static struct arm_pmu *arm_pmu_acpi_find_alloc_pmu(void)
 {
 	unsigned long cpuid = read_cpuid_id();
@@ -199,15 +201,9 @@ static struct arm_pmu *arm_pmu_acpi_find
 		return pmu;
 	}
 
-	pmu = armpmu_alloc_atomic();
-	if (!pmu) {
-		pr_warn("Unable to allocate PMU for CPU%d\n",
-			smp_processor_id());
-		return NULL;
-	}
-
+	pmu = arm_pmu_prealloced;
+	arm_pmu_prealloced = NULL;
 	pmu->acpi_cpuid = cpuid;
-
 	return pmu;
 }
 
@@ -284,6 +280,19 @@ static int arm_pmu_acpi_cpu_starting(uns
 	return 0;
 }
 
+static int arm_pmu_acpi_cpu_prepare(unsigned int cpu)
+{
+	if (per_cpu(probed_pmus, cpu) || arm_pmu_prealloced)
+		return 0;
+
+	arm_pmu_prealloced = armpmu_alloc();
+	if (!arm_pmu_prealloced) {
+		pr_warn("Unable to allocate PMU for CPU%d\n", cpu);
+		return -ENOMEM;
+	}
+	return 0;
+}
+
 int arm_pmu_acpi_probe(armpmu_init_fn init_fn)
 {
 	int pmu_idx = 0;
@@ -338,7 +347,7 @@ int arm_pmu_acpi_probe(armpmu_init_fn in
 
 static int arm_pmu_acpi_init(void)
 {
-	int ret;
+	int ret, dynstate;
 
 	if (acpi_disabled)
 		return 0;
@@ -349,9 +358,17 @@ static int arm_pmu_acpi_init(void)
 	if (ret)
 		return ret;
 
+	dynstate = cpuhp_setup_state(CPUHP_BP_PREPARE_DYN,
+				     "perf/arm/pmu_acpi:prepare",
+				     arm_pmu_acpi_cpu_prepare, NULL);
+	if (dynstate < 0)
+		return dynstate;
+
 	ret = cpuhp_setup_state(CPUHP_AP_PERF_ARM_ACPI_STARTING,
 				"perf/arm/pmu_acpi:starting",
 				arm_pmu_acpi_cpu_starting, NULL);
+	if (ret)
+		cpuhp_remove_state(dynstate);
 
 	return ret;
 }
--- a/include/linux/perf/arm_pmu.h
+++ b/include/linux/perf/arm_pmu.h
@@ -165,7 +165,6 @@ static inline int arm_pmu_acpi_probe(arm
 
 /* 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);

[SPLAT 2/3] irqchip/gic-v3-its: Sleeping spinlocks down gic_reserve_range()

[Valentin Schneider]

[    0.134518] BUG: sleeping function called from invalid context at kernel/locking/spinlock_rt.c:35
[    0.134520] in_atomic(): 1, irqs_disabled(): 128, non_block: 0, pid: 0, name: swapper/1
[    0.134522] 1 lock held by swapper/1/0:
[    0.134523] #0: ffff008f3624f728 ((lock).lock){+.+.}-{2:2}, at: get_page_from_freelist (mm/page_alloc.c:3673 mm/page_alloc.c:3704 mm/page_alloc.c:4166) 
[    0.134533] irq event stamp: 0
[    0.134534] hardirqs last enabled at (0): 0x0 
[    0.134538] hardirqs last disabled at (0): copy_process (./include/linux/lockdep.h:195 ./include/linux/lockdep.h:202 ./include/linux/lockdep.h:208 ./include/linux/seqlock.h:78 kernel/fork.c:2084) 
[    0.134542] softirqs last enabled at (0): copy_process (./include/linux/lockdep.h:195 ./include/linux/lockdep.h:202 ./include/linux/lockdep.h:208 ./include/linux/seqlock.h:78 kernel/fork.c:2084) 
[    0.134545] softirqs last disabled at (0): 0x0 
[    0.134547] Preemption disabled at:
[    0.134547] rt_mutex_slowunlock (kernel/locking/rtmutex.c:1223) 
[    0.134552] CPU: 1 PID: 0 Comm: swapper/1 Not tainted 5.14.0-rc4-rt6-torture+ #56
[    0.134555] Call trace:
[    0.134556] dump_backtrace (arch/arm64/kernel/stacktrace.c:151) 
[    0.134558] show_stack (arch/arm64/kernel/stacktrace.c:217) 
[    0.134559] dump_stack_lvl (lib/dump_stack.c:106) 
[    0.134563] dump_stack (lib/dump_stack.c:113) 
[    0.134565] ___might_sleep (kernel/sched/core.c:9306) 
[    0.134567] rt_spin_lock (kernel/locking/rtmutex.c:1641 (discriminator 4) kernel/locking/spinlock_rt.c:30 (discriminator 4) kernel/locking/spinlock_rt.c:36 (discriminator 4) kernel/locking/spinlock_rt.c:44 (discriminator 4)) 
[    0.134569] get_page_from_freelist (mm/page_alloc.c:3673 mm/page_alloc.c:3704 mm/page_alloc.c:4166) 
[    0.134571] __alloc_pages (mm/page_alloc.c:5391) 
[    0.134573] alloc_page_interleave (mm/mempolicy.c:2119) 
[    0.134576] alloc_pages (mm/mempolicy.c:2249) 
[    0.134577] new_slab (mm/slub.c:1740 mm/slub.c:1877 mm/slub.c:1940) 
[    0.134580] ___slab_alloc (mm/slub.c:2951) 
[    0.134582] __slab_alloc.isra.0 (mm/slub.c:3038) 
[    0.134584] kmem_cache_alloc_trace (mm/slub.c:3129 mm/slub.c:3171 mm/slub.c:3188) 
[    0.134587] efi_mem_reserve_iomem (drivers/firmware/efi/efi.c:905) 
[    0.134590] efi_mem_reserve_persistent (drivers/firmware/efi/efi.c:952) 
[    0.134593] its_cpu_init (drivers/irqchip/irq-gic-v3-its.c:3074 drivers/irqchip/irq-gic-v3-its.c:5196) 
[    0.134596] gic_starting_cpu (drivers/irqchip/irq-gic.c:798) 
[    0.134599] cpuhp_invoke_callback (kernel/cpu.c:180) 
[    0.134601] cpuhp_invoke_callback_range (kernel/cpu.c:656) 
[    0.134603] notify_cpu_starting (kernel/cpu.c:1270) 
[    0.134605] secondary_start_kernel (arch/arm64/kernel/smp.c:243) 
[    0.134608] __secondary_switched (arch/arm64/kernel/head.S:661) 

Re: [SPLAT 2/3] irqchip/gic-v3-its: Sleeping spinlocks down gic_reserve_range()

[Marc Zyngier]

[+ Ard]

On Tue, 10 Aug 2021 14:41:26 +0100,
Valentin Schneider <valentin.schneider@arm.com> wrote:
> 
> [    0.134518] BUG: sleeping function called from invalid context at kernel/locking/spinlock_rt.c:35
> [    0.134520] in_atomic(): 1, irqs_disabled(): 128, non_block: 0, pid: 0, name: swapper/1
> [    0.134522] 1 lock held by swapper/1/0:
> [    0.134523] #0: ffff008f3624f728 ((lock).lock){+.+.}-{2:2}, at: get_page_from_freelist (mm/page_alloc.c:3673 mm/page_alloc.c:3704 mm/page_alloc.c:4166) 
> [    0.134533] irq event stamp: 0
> [    0.134534] hardirqs last enabled at (0): 0x0 
> [    0.134538] hardirqs last disabled at (0): copy_process (./include/linux/lockdep.h:195 ./include/linux/lockdep.h:202 ./include/linux/lockdep.h:208 ./include/linux/seqlock.h:78 kernel/fork.c:2084) 
> [    0.134542] softirqs last enabled at (0): copy_process (./include/linux/lockdep.h:195 ./include/linux/lockdep.h:202 ./include/linux/lockdep.h:208 ./include/linux/seqlock.h:78 kernel/fork.c:2084) 
> [    0.134545] softirqs last disabled at (0): 0x0 
> [    0.134547] Preemption disabled at:
> [    0.134547] rt_mutex_slowunlock (kernel/locking/rtmutex.c:1223) 
> [    0.134552] CPU: 1 PID: 0 Comm: swapper/1 Not tainted 5.14.0-rc4-rt6-torture+ #56
> [    0.134555] Call trace:
> [    0.134556] dump_backtrace (arch/arm64/kernel/stacktrace.c:151) 
> [    0.134558] show_stack (arch/arm64/kernel/stacktrace.c:217) 
> [    0.134559] dump_stack_lvl (lib/dump_stack.c:106) 
> [    0.134563] dump_stack (lib/dump_stack.c:113) 
> [    0.134565] ___might_sleep (kernel/sched/core.c:9306) 
> [    0.134567] rt_spin_lock (kernel/locking/rtmutex.c:1641 (discriminator 4) kernel/locking/spinlock_rt.c:30 (discriminator 4) kernel/locking/spinlock_rt.c:36 (discriminator 4) kernel/locking/spinlock_rt.c:44 (discriminator 4)) 
> [    0.134569] get_page_from_freelist (mm/page_alloc.c:3673 mm/page_alloc.c:3704 mm/page_alloc.c:4166) 
> [    0.134571] __alloc_pages (mm/page_alloc.c:5391) 
> [    0.134573] alloc_page_interleave (mm/mempolicy.c:2119) 
> [    0.134576] alloc_pages (mm/mempolicy.c:2249) 
> [    0.134577] new_slab (mm/slub.c:1740 mm/slub.c:1877 mm/slub.c:1940) 
> [    0.134580] ___slab_alloc (mm/slub.c:2951) 
> [    0.134582] __slab_alloc.isra.0 (mm/slub.c:3038) 
> [    0.134584] kmem_cache_alloc_trace (mm/slub.c:3129 mm/slub.c:3171 mm/slub.c:3188) 
> [    0.134587] efi_mem_reserve_iomem (drivers/firmware/efi/efi.c:905) 
> [    0.134590] efi_mem_reserve_persistent (drivers/firmware/efi/efi.c:952) 
> [    0.134593] its_cpu_init (drivers/irqchip/irq-gic-v3-its.c:3074 drivers/irqchip/irq-gic-v3-its.c:5196) 
> [    0.134596] gic_starting_cpu (drivers/irqchip/irq-gic.c:798) 
> [    0.134599] cpuhp_invoke_callback (kernel/cpu.c:180) 
> [    0.134601] cpuhp_invoke_callback_range (kernel/cpu.c:656) 
> [    0.134603] notify_cpu_starting (kernel/cpu.c:1270) 
> [    0.134605] secondary_start_kernel (arch/arm64/kernel/smp.c:243) 
> [    0.134608] __secondary_switched (arch/arm64/kernel/head.S:661) 

The issue is that although the redistributor tables have been
allocated ahead of time (outside of any cpuhp callback), they cannot
be programmed into the RDs until the corresponding CPUs have been
brought up (the registers may not be accessible).

For the same reason, we don't know whether we can free them (because
there is already a table programmed there) or have to reserve them
with an efi_mem_reserve_persistent() call. efi_mem_reserve_iomem()
uses GFP_ATOMIC for its allocation, but this is not sufficient for RT
anymore.

We could postpone the reservation of the memory to a later point (it
is only useful for kexec), but it isn't clear where that point is. The
CPU is not quite up yet, and we can't easily IPI the boot CPU to do
the reserve call.

	M.

-- 
Without deviation from the norm, progress is not possible.

Re: [SPLAT 2/3] irqchip/gic-v3-its: Sleeping spinlocks down gic_reserve_range()

[Thomas Gleixner]

On Wed, Aug 11 2021 at 09:50, Marc Zyngier wrote:
> On Tue, 10 Aug 2021 14:41:26 +0100,
> Valentin Schneider <valentin.schneider@arm.com> wrote:
> The issue is that although the redistributor tables have been
> allocated ahead of time (outside of any cpuhp callback), they cannot
> be programmed into the RDs until the corresponding CPUs have been
> brought up (the registers may not be accessible).
>
> For the same reason, we don't know whether we can free them (because
> there is already a table programmed there) or have to reserve them
> with an efi_mem_reserve_persistent() call. efi_mem_reserve_iomem()
> uses GFP_ATOMIC for its allocation, but this is not sufficient for RT
> anymore.
>
> We could postpone the reservation of the memory to a later point (it
> is only useful for kexec), but it isn't clear where that point is. The
> CPU is not quite up yet, and we can't easily IPI the boot CPU to do
> the reserve call.

Right, but don't you know about the need for reservation _before_
bringing the CPU up?

Thanks,

        tglx

Re: [SPLAT 2/3] irqchip/gic-v3-its: Sleeping spinlocks down gic_reserve_range()

[Marc Zyngier]

On Wed, 11 Aug 2021 13:28:21 +0100,
Thomas Gleixner <tglx@linutronix.de> wrote:
> 
> On Wed, Aug 11 2021 at 09:50, Marc Zyngier wrote:
> > On Tue, 10 Aug 2021 14:41:26 +0100,
> > Valentin Schneider <valentin.schneider@arm.com> wrote:
> > The issue is that although the redistributor tables have been
> > allocated ahead of time (outside of any cpuhp callback), they cannot
> > be programmed into the RDs until the corresponding CPUs have been
> > brought up (the registers may not be accessible).
> >
> > For the same reason, we don't know whether we can free them (because
> > there is already a table programmed there) or have to reserve them
> > with an efi_mem_reserve_persistent() call. efi_mem_reserve_iomem()
> > uses GFP_ATOMIC for its allocation, but this is not sufficient for RT
> > anymore.
> >
> > We could postpone the reservation of the memory to a later point (it
> > is only useful for kexec), but it isn't clear where that point is. The
> > CPU is not quite up yet, and we can't easily IPI the boot CPU to do
> > the reserve call.
> 
> Right, but don't you know about the need for reservation _before_
> bringing the CPU up?

Unfortunately not. To find out, you need to access a pair of per-CPU
registers which are not guaranteed to be powered-on until the
corresponding CPU has made it into the kernel (the firmware will power
things on as part of bringing the CPU up).

Which is why we always allocate the memory upfront for all the CPUs,
and each CPU either frees the memory if it already had something in
its redistributor, or point the redistributor to the memory and
reserves it.

This is probably the most epic fail of the GICv3 architecture...

Thanks,

	M.

-- 
Without deviation from the norm, progress is not possible.

Re: [SPLAT 2/3] irqchip/gic-v3-its: Sleeping spinlocks down gic_reserve_range()

[Ard Biesheuvel]

On Wed, 11 Aug 2021 at 10:50, Marc Zyngier <maz@kernel.org> wrote:
>
> [+ Ard]
>
> On Tue, 10 Aug 2021 14:41:26 +0100,
> Valentin Schneider <valentin.schneider@arm.com> wrote:
> >
> > [    0.134518] BUG: sleeping function called from invalid context at kernel/locking/spinlock_rt.c:35
> > [    0.134520] in_atomic(): 1, irqs_disabled(): 128, non_block: 0, pid: 0, name: swapper/1
> > [    0.134522] 1 lock held by swapper/1/0:
> > [    0.134523] #0: ffff008f3624f728 ((lock).lock){+.+.}-{2:2}, at: get_page_from_freelist (mm/page_alloc.c:3673 mm/page_alloc.c:3704 mm/page_alloc.c:4166)
> > [    0.134533] irq event stamp: 0
> > [    0.134534] hardirqs last enabled at (0): 0x0
> > [    0.134538] hardirqs last disabled at (0): copy_process (./include/linux/lockdep.h:195 ./include/linux/lockdep.h:202 ./include/linux/lockdep.h:208 ./include/linux/seqlock.h:78 kernel/fork.c:2084)
> > [    0.134542] softirqs last enabled at (0): copy_process (./include/linux/lockdep.h:195 ./include/linux/lockdep.h:202 ./include/linux/lockdep.h:208 ./include/linux/seqlock.h:78 kernel/fork.c:2084)
> > [    0.134545] softirqs last disabled at (0): 0x0
> > [    0.134547] Preemption disabled at:
> > [    0.134547] rt_mutex_slowunlock (kernel/locking/rtmutex.c:1223)
> > [    0.134552] CPU: 1 PID: 0 Comm: swapper/1 Not tainted 5.14.0-rc4-rt6-torture+ #56
> > [    0.134555] Call trace:
> > [    0.134556] dump_backtrace (arch/arm64/kernel/stacktrace.c:151)
> > [    0.134558] show_stack (arch/arm64/kernel/stacktrace.c:217)
> > [    0.134559] dump_stack_lvl (lib/dump_stack.c:106)
> > [    0.134563] dump_stack (lib/dump_stack.c:113)
> > [    0.134565] ___might_sleep (kernel/sched/core.c:9306)
> > [    0.134567] rt_spin_lock (kernel/locking/rtmutex.c:1641 (discriminator 4) kernel/locking/spinlock_rt.c:30 (discriminator 4) kernel/locking/spinlock_rt.c:36 (discriminator 4) kernel/locking/spinlock_rt.c:44 (discriminator 4))
> > [    0.134569] get_page_from_freelist (mm/page_alloc.c:3673 mm/page_alloc.c:3704 mm/page_alloc.c:4166)
> > [    0.134571] __alloc_pages (mm/page_alloc.c:5391)
> > [    0.134573] alloc_page_interleave (mm/mempolicy.c:2119)
> > [    0.134576] alloc_pages (mm/mempolicy.c:2249)
> > [    0.134577] new_slab (mm/slub.c:1740 mm/slub.c:1877 mm/slub.c:1940)
> > [    0.134580] ___slab_alloc (mm/slub.c:2951)
> > [    0.134582] __slab_alloc.isra.0 (mm/slub.c:3038)
> > [    0.134584] kmem_cache_alloc_trace (mm/slub.c:3129 mm/slub.c:3171 mm/slub.c:3188)
> > [    0.134587] efi_mem_reserve_iomem (drivers/firmware/efi/efi.c:905)
> > [    0.134590] efi_mem_reserve_persistent (drivers/firmware/efi/efi.c:952)
> > [    0.134593] its_cpu_init (drivers/irqchip/irq-gic-v3-its.c:3074 drivers/irqchip/irq-gic-v3-its.c:5196)
> > [    0.134596] gic_starting_cpu (drivers/irqchip/irq-gic.c:798)
> > [    0.134599] cpuhp_invoke_callback (kernel/cpu.c:180)
> > [    0.134601] cpuhp_invoke_callback_range (kernel/cpu.c:656)
> > [    0.134603] notify_cpu_starting (kernel/cpu.c:1270)
> > [    0.134605] secondary_start_kernel (arch/arm64/kernel/smp.c:243)
> > [    0.134608] __secondary_switched (arch/arm64/kernel/head.S:661)
>
> The issue is that although the redistributor tables have been
> allocated ahead of time (outside of any cpuhp callback), they cannot
> be programmed into the RDs until the corresponding CPUs have been
> brought up (the registers may not be accessible).
>
> For the same reason, we don't know whether we can free them (because
> there is already a table programmed there) or have to reserve them
> with an efi_mem_reserve_persistent() call. efi_mem_reserve_iomem()
> uses GFP_ATOMIC for its allocation, but this is not sufficient for RT
> anymore.
>
> We could postpone the reservation of the memory to a later point (it
> is only useful for kexec), but it isn't clear where that point is. The
> CPU is not quite up yet, and we can't easily IPI the boot CPU to do
> the reserve call.
>

The kzalloc() call in question is used to allocate the struct resource
which is inserted in to the iomem resource tree. This could definitely
be postponed, given that the kernel itself does not care about these
entries, only user space (IIUC)

[SPLAT 3/3] gpio: dwapb: Sleeping spinlocks down IRQ mapping

[Valentin Schneider]

[   11.549741] BUG: sleeping function called from invalid context at kernel/locking/spinlock_rt.c:35
[   11.549745] in_atomic(): 1, irqs_disabled(): 128, non_block: 0, pid: 553, name: systemd-udevd
[   11.549747] 4 locks held by systemd-udevd/553:
[   11.549749] #0: ffff00080338b168 (&dev->mutex){....}-{0:0}, at: __device_driver_lock (drivers/base/dd.c:1029) 
[   11.549760] #1: ffff800011df0780 (irq_domain_mutex){+.+.}-{3:3}, at: irq_domain_associate (kernel/irq/irqdomain.c:576) 
[   11.549769] #2: ffff0008112a6900 (lock_class){....}-{2:2}, at: __irq_get_desc_lock (kernel/irq/irqdesc.c:857) 
[   11.549775] #3: ffff0008112a59c8 (&gc->bgpio_lock){+.+.}-{0:0}, at: dwapb_irq_ack (drivers/gpio/gpio-dwapb.c:151 drivers/gpio/gpio-dwapb.c:233) gpio_dwapb
[   11.549783] irq event stamp: 157476
[   11.549785] hardirqs last enabled at (157475): _raw_spin_unlock_irqrestore (./include/linux/spinlock_api_smp.h:160 kernel/locking/spinlock.c:194) 
[   11.549790] hardirqs last disabled at (157476): _raw_spin_lock_irqsave (./include/linux/spinlock_api_smp.h:108 kernel/locking/spinlock.c:162) 
[   11.549793] softirqs last enabled at (0): copy_process (./include/linux/lockdep.h:195 ./include/linux/lockdep.h:202 ./include/linux/lockdep.h:208 ./include/linux/seqlock.h:78 kernel/fork.c:2084) 
[   11.549797] softirqs last disabled at (0): 0x0 
[   11.549801] Preemption disabled at:
[   11.549802] __irq_get_desc_lock (kernel/irq/irqdesc.c:857) 
[   11.549806] CPU: 22 PID: 553 Comm: systemd-udevd Tainted: G        W         5.14.0-rc4-rt6-torture+ #56
[   11.549808] Hardware name: MiTAC RAPTOR EV-883832-X3-0001/RAPTOR, BIOS 1.0 06/28/2019
[   11.549810] Call trace:
[   11.549811] dump_backtrace (arch/arm64/kernel/stacktrace.c:151) 
[   11.549814] show_stack (arch/arm64/kernel/stacktrace.c:217) 
[   11.549815] dump_stack_lvl (lib/dump_stack.c:106) 
[   11.549819] dump_stack (lib/dump_stack.c:113) 
[   11.549821] ___might_sleep (kernel/sched/core.c:9306) 
[   11.549824] rt_spin_lock (kernel/locking/rtmutex.c:1641 (discriminator 4) kernel/locking/spinlock_rt.c:30 (discriminator 4) kernel/locking/spinlock_rt.c:36 (discriminator 4) kernel/locking/spinlock_rt.c:44 (discriminator 4)) 
[   11.549827] dwapb_irq_ack (drivers/gpio/gpio-dwapb.c:151 drivers/gpio/gpio-dwapb.c:233) gpio_dwapb
[   11.549831] __irq_do_set_handler (kernel/irq/chip.c:414 kernel/irq/chip.c:406 kernel/irq/chip.c:1009) 
[   11.549833] __irq_set_handler (kernel/irq/internals.h:178 kernel/irq/chip.c:1053) 
[   11.549836] irq_set_chip_and_handler_name (kernel/irq/chip.c:1080) 
[   11.549838] gpiochip_irq_map (drivers/gpio/gpiolib.c:1262) 
[   11.549842] irq_domain_associate (kernel/irq/irqdomain.c:579) 
[   11.549845] irq_create_mapping_affinity (kernel/irq/irqdomain.c:716) 
[   11.549848] gpiochip_to_irq (drivers/gpio/gpiolib.c:1365) 
[   11.549851] gpiod_to_irq (drivers/gpio/gpiolib.c:3082) 
[   11.549853] acpi_gpiochip_alloc_event (drivers/gpio/gpiolib-acpi.c:422) 
[   11.549856] acpi_walk_resource_buffer (drivers/acpi/acpica/rsxface.c:548) 
[   11.549859] acpi_walk_resources (drivers/acpi/acpica/rsxface.c:623 drivers/acpi/acpica/rsxface.c:594) 
[   11.549862] acpi_gpiochip_request_interrupts (drivers/gpio/gpiolib-acpi.c:505) 
[   11.549864] gpiochip_add_data_with_key (drivers/gpio/gpiolib.c:748) 
[   11.549867] devm_gpiochip_add_data_with_key (drivers/gpio/gpiolib-devres.c:510) 
[   11.549869] dwapb_gpio_probe (drivers/gpio/gpio-dwapb.c:517 drivers/gpio/gpio-dwapb.c:712) gpio_dwapb
[   11.549873] platform_probe (drivers/base/platform.c:1428) 
[   11.549876] really_probe (drivers/base/dd.c:517 drivers/base/dd.c:595 drivers/base/dd.c:541) 
[   11.549877] __driver_probe_device (drivers/base/dd.c:747) 
[   11.549879] driver_probe_device (drivers/base/dd.c:777) 
[   11.549881] __driver_attach (drivers/base/dd.c:1137) 
[   11.549883] bus_for_each_dev (drivers/base/bus.c:301) 
[   11.549885] driver_attach (drivers/base/dd.c:1154) 
[   11.549887] bus_add_driver (drivers/base/bus.c:619) 
[   11.549888] driver_register (drivers/base/driver.c:171) 
[   11.549890] __platform_driver_register (drivers/base/platform.c:875) 
[   11.549893] dwapb_gpio_driver_init (drivers/gpio/gpio-dwapb.c:343) gpio_dwapb
[   11.549897] do_one_initcall (init/main.c:1282) 
[   11.549900] do_init_module (kernel/module.c:3690) 
[   11.549904] load_module (kernel/module.c:4091) 
[   11.549906] __do_sys_finit_module (kernel/module.c:4182) 
[   11.549909] __arm64_sys_finit_module (kernel/module.c:4159) 
[   11.549911] invoke_syscall (./arch/arm64/include/asm/current.h:19 ./arch/arm64/include/asm/compat.h:183 arch/arm64/kernel/syscall.c:57) 
[   11.549914] el0_svc_common (./arch/arm64/include/asm/daifflags.h:28 arch/arm64/kernel/syscall.c:153) 
[   11.549917] do_el0_svc (arch/arm64/kernel/syscall.c:185) 
[   11.549919] el0_svc (arch/arm64/kernel/entry-common.c:512) 
[   11.549922] el0t_64_sync_handler (arch/arm64/kernel/entry-common.c:528) 
[   11.549924] el0t_64_sync (arch/arm64/kernel/entry.S:574) 

Re: [SPLAT 3/3] gpio: dwapb: Sleeping spinlocks down IRQ mapping

[Thomas Gleixner]

On Tue, Aug 10 2021 at 14:41, Valentin Schneider wrote:
> [   11.549824] rt_spin_lock (kernel/locking/rtmutex.c:1641 (discriminator 4) kernel/locking/spinlock_rt.c:30 (discriminator 4) kernel/locking/spinlock_rt.c:36 (discriminator 4) kernel/locking/spinlock_rt.c:44 (discriminator 4)) 
> [   11.549827] dwapb_irq_ack (drivers/gpio/gpio-dwapb.c:151 drivers/gpio/gpio-dwapb.c:233) gpio_dwapb
> [   11.549831] __irq_do_set_handler (kernel/irq/chip.c:414 kernel/irq/chip.c:406 kernel/irq/chip.c:1009) 
> [   11.549833] __irq_set_handler (kernel/irq/internals.h:178 kernel/irq/chip.c:1053) 

This is gpio_chip->bgpio_lock which is a regular spinlock. AFAICT this
lock should merely serializing access to MMIO registers, so it should
not be a problem to make this lock raw.

Except for the obligatory exception:

 grgpio_irq_handler() holds that lock from the demultiplexing
 handler and invokes all handlers for the individual GPIOs which have
 interrupts enabled without ever consulting a pending register.

 That drivers usage of that lock is interesting in general, see
 grgpio_map_irq() for illustration. Quality stuff for mission critical
 systems...

 But nevertheless it should just work with a raw lock on RT AFACIT.

Thanks,

        tglx

Re: [SPLAT 3/3] gpio: dwapb: Sleeping spinlocks down IRQ mapping

[Andy Shevchenko]

+Cc: Serge, maintainer of the driver

On Tue, Aug 10, 2021 at 7:34 PM Valentin Schneider
<valentin.schneider@arm.com> wrote:
>
> [   11.549741] BUG: sleeping function called from invalid context at kernel/locking/spinlock_rt.c:35
> [   11.549745] in_atomic(): 1, irqs_disabled(): 128, non_block: 0, pid: 553, name: systemd-udevd
> [   11.549747] 4 locks held by systemd-udevd/553:
> [   11.549749] #0: ffff00080338b168 (&dev->mutex){....}-{0:0}, at: __device_driver_lock (drivers/base/dd.c:1029)
> [   11.549760] #1: ffff800011df0780 (irq_domain_mutex){+.+.}-{3:3}, at: irq_domain_associate (kernel/irq/irqdomain.c:576)
> [   11.549769] #2: ffff0008112a6900 (lock_class){....}-{2:2}, at: __irq_get_desc_lock (kernel/irq/irqdesc.c:857)
> [   11.549775] #3: ffff0008112a59c8 (&gc->bgpio_lock){+.+.}-{0:0}, at: dwapb_irq_ack (drivers/gpio/gpio-dwapb.c:151 drivers/gpio/gpio-dwapb.c:233) gpio_dwapb
> [   11.549783] irq event stamp: 157476
> [   11.549785] hardirqs last enabled at (157475): _raw_spin_unlock_irqrestore (./include/linux/spinlock_api_smp.h:160 kernel/locking/spinlock.c:194)
> [   11.549790] hardirqs last disabled at (157476): _raw_spin_lock_irqsave (./include/linux/spinlock_api_smp.h:108 kernel/locking/spinlock.c:162)
> [   11.549793] softirqs last enabled at (0): copy_process (./include/linux/lockdep.h:195 ./include/linux/lockdep.h:202 ./include/linux/lockdep.h:208 ./include/linux/seqlock.h:78 kernel/fork.c:2084)
> [   11.549797] softirqs last disabled at (0): 0x0
> [   11.549801] Preemption disabled at:
> [   11.549802] __irq_get_desc_lock (kernel/irq/irqdesc.c:857)
> [   11.549806] CPU: 22 PID: 553 Comm: systemd-udevd Tainted: G        W         5.14.0-rc4-rt6-torture+ #56
> [   11.549808] Hardware name: MiTAC RAPTOR EV-883832-X3-0001/RAPTOR, BIOS 1.0 06/28/2019
> [   11.549810] Call trace:
> [   11.549811] dump_backtrace (arch/arm64/kernel/stacktrace.c:151)
> [   11.549814] show_stack (arch/arm64/kernel/stacktrace.c:217)
> [   11.549815] dump_stack_lvl (lib/dump_stack.c:106)
> [   11.549819] dump_stack (lib/dump_stack.c:113)
> [   11.549821] ___might_sleep (kernel/sched/core.c:9306)
> [   11.549824] rt_spin_lock (kernel/locking/rtmutex.c:1641 (discriminator 4) kernel/locking/spinlock_rt.c:30 (discriminator 4) kernel/locking/spinlock_rt.c:36 (discriminator 4) kernel/locking/spinlock_rt.c:44 (discriminator 4))
> [   11.549827] dwapb_irq_ack (drivers/gpio/gpio-dwapb.c:151 drivers/gpio/gpio-dwapb.c:233) gpio_dwapb
> [   11.549831] __irq_do_set_handler (kernel/irq/chip.c:414 kernel/irq/chip.c:406 kernel/irq/chip.c:1009)
> [   11.549833] __irq_set_handler (kernel/irq/internals.h:178 kernel/irq/chip.c:1053)
> [   11.549836] irq_set_chip_and_handler_name (kernel/irq/chip.c:1080)
> [   11.549838] gpiochip_irq_map (drivers/gpio/gpiolib.c:1262)
> [   11.549842] irq_domain_associate (kernel/irq/irqdomain.c:579)
> [   11.549845] irq_create_mapping_affinity (kernel/irq/irqdomain.c:716)
> [   11.549848] gpiochip_to_irq (drivers/gpio/gpiolib.c:1365)
> [   11.549851] gpiod_to_irq (drivers/gpio/gpiolib.c:3082)
> [   11.549853] acpi_gpiochip_alloc_event (drivers/gpio/gpiolib-acpi.c:422)
> [   11.549856] acpi_walk_resource_buffer (drivers/acpi/acpica/rsxface.c:548)
> [   11.549859] acpi_walk_resources (drivers/acpi/acpica/rsxface.c:623 drivers/acpi/acpica/rsxface.c:594)
> [   11.549862] acpi_gpiochip_request_interrupts (drivers/gpio/gpiolib-acpi.c:505)
> [   11.549864] gpiochip_add_data_with_key (drivers/gpio/gpiolib.c:748)
> [   11.549867] devm_gpiochip_add_data_with_key (drivers/gpio/gpiolib-devres.c:510)
> [   11.549869] dwapb_gpio_probe (drivers/gpio/gpio-dwapb.c:517 drivers/gpio/gpio-dwapb.c:712) gpio_dwapb
> [   11.549873] platform_probe (drivers/base/platform.c:1428)
> [   11.549876] really_probe (drivers/base/dd.c:517 drivers/base/dd.c:595 drivers/base/dd.c:541)
> [   11.549877] __driver_probe_device (drivers/base/dd.c:747)
> [   11.549879] driver_probe_device (drivers/base/dd.c:777)
> [   11.549881] __driver_attach (drivers/base/dd.c:1137)
> [   11.549883] bus_for_each_dev (drivers/base/bus.c:301)
> [   11.549885] driver_attach (drivers/base/dd.c:1154)
> [   11.549887] bus_add_driver (drivers/base/bus.c:619)
> [   11.549888] driver_register (drivers/base/driver.c:171)
> [   11.549890] __platform_driver_register (drivers/base/platform.c:875)
> [   11.549893] dwapb_gpio_driver_init (drivers/gpio/gpio-dwapb.c:343) gpio_dwapb
> [   11.549897] do_one_initcall (init/main.c:1282)
> [   11.549900] do_init_module (kernel/module.c:3690)
> [   11.549904] load_module (kernel/module.c:4091)
> [   11.549906] __do_sys_finit_module (kernel/module.c:4182)
> [   11.549909] __arm64_sys_finit_module (kernel/module.c:4159)
> [   11.549911] invoke_syscall (./arch/arm64/include/asm/current.h:19 ./arch/arm64/include/asm/compat.h:183 arch/arm64/kernel/syscall.c:57)
> [   11.549914] el0_svc_common (./arch/arm64/include/asm/daifflags.h:28 arch/arm64/kernel/syscall.c:153)
> [   11.549917] do_el0_svc (arch/arm64/kernel/syscall.c:185)
> [   11.549919] el0_svc (arch/arm64/kernel/entry-common.c:512)
> [   11.549922] el0t_64_sync_handler (arch/arm64/kernel/entry-common.c:528)
> [   11.549924] el0t_64_sync (arch/arm64/kernel/entry.S:574)

As Thomas said already the usual fix for GPIO drivers is to switch to
the raw version of spin locks. See other drivers for the example of
such conversion (yet, the IRQ handler cases might be needing the
additional care).

-- 
With Best Regards,
Andy Shevchenko