Thread (22 messages) 22 messages, 7 authors, 2026-01-21

Re: [PATCH] irqchip/gic-v3-its: Don't acquire rt_spin_lock in allocate_vpe_l1_table()

From: Waiman Long <hidden>
Date: 2026-01-10 21:48:04
Also in: linux-rt-devel, lkml

On 1/8/26 3:26 AM, Marc Zyngier wrote:
On Wed, 07 Jan 2026 21:53:53 +0000,
Waiman Long [off-list ref] wrote:
quoted
When running a PREEMPT_RT debug kernel on a 2-socket Grace arm64 system,
the following bug report was produced at bootup time.

   BUG: sleeping function called from invalid context at kernel/locking/spinlock_rt.c:48
   in_atomic(): 1, irqs_disabled(): 1, non_block: 0, pid: 0, name: swapper/72
   preempt_count: 1, expected: 0
   RCU nest depth: 1, expected: 1
    :
   CPU: 72 UID: 0 PID: 0 Comm: swapper/72 Tainted: G        W           6.19.0-rc4-test+ #4 PREEMPT_{RT,(full)}
   Tainted: [W]=WARN
   Call trace:
     :
    rt_spin_lock+0xe4/0x408
    rmqueue_bulk+0x48/0x1de8
    __rmqueue_pcplist+0x410/0x650
    rmqueue.constprop.0+0x6a8/0x2b50
    get_page_from_freelist+0x3c0/0xe68
    __alloc_frozen_pages_noprof+0x1dc/0x348
    alloc_pages_mpol+0xe4/0x2f8
    alloc_frozen_pages_noprof+0x124/0x190
    allocate_slab+0x2f0/0x438
    new_slab+0x4c/0x80
    ___slab_alloc+0x410/0x798
    __slab_alloc.constprop.0+0x88/0x1e0
    __kmalloc_cache_noprof+0x2dc/0x4b0
    allocate_vpe_l1_table+0x114/0x788
    its_cpu_init_lpis+0x344/0x790
    its_cpu_init+0x60/0x220
    gic_starting_cpu+0x64/0xe8
    cpuhp_invoke_callback+0x438/0x6d8
    __cpuhp_invoke_callback_range+0xd8/0x1f8
    notify_cpu_starting+0x11c/0x178
    secondary_start_kernel+0xc8/0x188
    __secondary_switched+0xc0/0xc8

This is due to the fact that allocate_vpe_l1_table() will call
kzalloc() to allocate a cpumask_t when the first CPU of the
second node of the 72-cpu Grace system is being called from the
CPUHP_AP_MIPS_GIC_TIMER_STARTING state inside the starting section of
Surely *not* that particular state.
My mistake, it should be CPUHP_AP_IRQ_GIC_STARTING. There are three 
static gic_starting_cpu() functions that confuse me.

quoted
the CPU hotplug bringup pipeline where interrupt is disabled. This is an
atomic context where sleeping is not allowed and acquiring a sleeping
rt_spin_lock within kzalloc() may lead to system hang in case there is
a lock contention.

To work around this issue, a static buffer is used for cpumask
allocation when running a PREEMPT_RT kernel via the newly introduced
vpe_alloc_cpumask() helper. The static buffer is currently set to be
4 kbytes in size. As only one cpumask is needed per node, the current
size should be big enough as long as (cpumask_size() * nr_node_ids)
is not bigger than 4k.
What role does the node play here? The GIC topology has nothing to do
with NUMA. It may be true on your particular toy, but that's
definitely not true architecturally. You could, at worse, end-up with
one such cpumask per *CPU*. That'd be a braindead system, but this
code is written to support the architecture, not any particular
implementation.
It is just what I have observed on the hardware that I used for 
reproducing the problem. I agree that it may be different in other arm64 
CPUs.
quoted
Signed-off-by: Waiman Long <longman@redhat.com>
---
  drivers/irqchip/irq-gic-v3-its.c | 26 +++++++++++++++++++++++++-
  1 file changed, 25 insertions(+), 1 deletion(-)
diff --git a/drivers/irqchip/irq-gic-v3-its.c b/drivers/irqchip/irq-gic-v3-its.c
index ada585bfa451..9185785524dc 100644
--- a/drivers/irqchip/irq-gic-v3-its.c
+++ b/drivers/irqchip/irq-gic-v3-its.c
@@ -2896,6 +2896,30 @@ static bool allocate_vpe_l2_table(int cpu, u32 id)
  	return true;
  }
  
+static void *vpe_alloc_cpumask(void)
+{
+	/*
+	 * With PREEMPT_RT kernel, we can't call any k*alloc() APIs as they
+	 * may acquire a sleeping rt_spin_lock in an atomic context. So use
+	 * a pre-allocated buffer instead.
+	 */
+	if (IS_ENABLED(CONFIG_PREEMPT_RT)) {
+		static unsigned long mask_buf[512];
+		static atomic_t	alloc_idx;
+		int idx, mask_size = cpumask_size();
+		int nr_cpumasks = sizeof(mask_buf)/mask_size;
+
+		/*
+		 * Fetch an allocation index and if it points to a buffer within
+		 * mask_buf[], return that. Fall back to kzalloc() otherwise.
+		 */
+		idx = atomic_fetch_inc(&alloc_idx);
+		if (idx < nr_cpumasks)
+			return &mask_buf[idx * mask_size/sizeof(long)];
+	}
Err, no. That's horrible. I can see three ways to address this in a
more appealing way:

- you give RT a generic allocator that works for (small) atomic
   allocations. I appreciate that's not easy, and even probably
   contrary to the RT goals. But I'm also pretty sure that the GIC code
   is not the only pile of crap being caught doing that.

- you pre-compute upfront how many cpumasks you are going to require,
   based on the actual GIC topology. You do that on CPU0, outside of
   the hotplug constraints, and allocate what you need. This is
   difficult as you need to ensure the RD<->CPU matching without the
   CPUs having booted, which means wading through the DT/ACPI gunk to
   try and guess what you have.

- you delay the allocation of L1 tables to a context where you can
   perform allocations, and before we have a chance of running a guest
   on this CPU. That's probably the simplest option (though dealing
   with late onlining while guests are already running could be
   interesting...).

But I'm always going to say no to something that is a poor hack and
ultimately falling back to the same broken behaviour.
Thanks for the suggestion. I will try  the first alternative of a more 
generic memory allocator.

Cheers,
Longman
Thanks,

	M.
  
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help