Re: [PATCH] riscv: panic from init_IRQ if IRQ handler stacks cannot be allocated

[Osama Abdelkader]

Hi Paul,

On Thu, Apr 02, 2026 at 04:11:59PM -0600, Paul Walmsley wrote:
> Hi,
> 
> On Fri, 27 Mar 2026, Osama Abdelkader wrote:
> 
> > init_irq_stacks() and init_irq_scs() may fail when arch_alloc_vmap_stack
> > or scs_alloc return NULL. Return -ENOMEM from both and call panic() once
> > from init_IRQ(), covering per-CPU IRQ stacks and shadow IRQ stacks
> > consistently.
> > 
> > Signed-off-by: Osama Abdelkader <osama.abdelkader@xxxxxxxxx>
> 
> This patch appears to be written with AI assistance.  If so, it should be 
> noted in the description.  Please see
> 
>   https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/process/coding-assistants.rst
> 

This is actually riscv version of arm64 patch:
[PATCH] arm64: panic if IRQ shadow call stack allocation fails
https://lore.kernel.org/lkml/20260324161545.5441-1-osama.abdelkader@xxxxxxxxx/
where I initially used panic, but in review, returning -ENOMEM was suggested,
and this patch also, so I submitted v2 there:
[PATCH v2] arm64: panic from init_IRQ if IRQ handler stacks cannot be allocated
https://lore.kernel.org/lkml/20260326225755.50297-1-osama.abdelkader@xxxxxxxxx/

> > ---
> >  arch/riscv/kernel/irq.c | 43 ++++++++++++++++++++++++++++++-----------
> >  1 file changed, 32 insertions(+), 11 deletions(-)
> > 
> > diff --git a/arch/riscv/kernel/irq.c b/arch/riscv/kernel/irq.c
> > index b6af20bc300f..43392276f7f8 100644
> > --- a/arch/riscv/kernel/irq.c
> > +++ b/arch/riscv/kernel/irq.c
> > @@ -5,6 +5,7 @@
> >   * Copyright (C) 2018 Christoph Hellwig
> >   */
> >  
> > +#include <linux/errno.h>
> >  #include <linux/interrupt.h>
> >  #include <linux/irqchip.h>
> >  #include <linux/irqdomain.h>
> > @@ -75,41 +76,53 @@ DECLARE_PER_CPU(ulong *, irq_shadow_call_stack_ptr);
> >  DEFINE_PER_CPU(ulong *, irq_shadow_call_stack_ptr);
> >  #endif
> >  
> > -static void init_irq_scs(void)
> > +static int __init init_irq_scs(void)
> >  {
> >  	int cpu;
> > +	void *s;
> >  
> >  	if (!scs_is_enabled())
> > -		return;
> > +		return 0;
> >  
> > -	for_each_possible_cpu(cpu)
> > -		per_cpu(irq_shadow_call_stack_ptr, cpu) =
> > -			scs_alloc(cpu_to_node(cpu));
> > +	for_each_possible_cpu(cpu) {
> > +		s = scs_alloc(cpu_to_node(cpu));
> > +		if (!s)
> > +			return -ENOMEM;
> 
> I don't see why this bothers to return an error value if the sole caller 
> is just going to panic.  Best to panic here.  Then no one reviewing this 
> patch needs to be concerned about memory leaks.
> 

Okay, I'm going to use panic in place instead.

> > +		per_cpu(irq_shadow_call_stack_ptr, cpu) = s;
> > +	}
> > +
> > +	return 0;
> >  }
> >  
> >  DEFINE_PER_CPU(ulong *, irq_stack_ptr);
> >  
> >  #ifdef CONFIG_VMAP_STACK
> > -static void init_irq_stacks(void)
> > +static int __init init_irq_stacks(void)
> >  {
> >  	int cpu;
> >  	ulong *p;
> >  
> >  	for_each_possible_cpu(cpu) {
> >  		p = arch_alloc_vmap_stack(IRQ_STACK_SIZE, cpu_to_node(cpu));
> > +		if (!p)
> > +			return -ENOMEM;
> 
> Same comment as the above.
> 
> >  		per_cpu(irq_stack_ptr, cpu) = p;
> >  	}
> > +
> > +	return 0;
> >  }
> >  #else
> >  /* irq stack only needs to be 16 byte aligned - not IRQ_STACK_SIZE aligned. */
> >  DEFINE_PER_CPU_ALIGNED(ulong [IRQ_STACK_SIZE/sizeof(ulong)], irq_stack);
> >  
> > -static void init_irq_stacks(void)
> > +static int __init init_irq_stacks(void)
> >  {
> >  	int cpu;
> >  
> >  	for_each_possible_cpu(cpu)
> >  		per_cpu(irq_stack_ptr, cpu) = per_cpu(irq_stack, cpu);
> > +
> > +	return 0;
> 
> Why?
> 
> >  }
> >  #endif /* CONFIG_VMAP_STACK */
> >  
> > @@ -129,8 +142,15 @@ void do_softirq_own_stack(void)
> >  #endif /* CONFIG_SOFTIRQ_ON_OWN_STACK */
> >  
> >  #else
> > -static void init_irq_scs(void) {}
> > -static void init_irq_stacks(void) {}
> > +static int __init init_irq_scs(void)
> > +{
> > +	return 0;
> > +}
> > +
> > +static int __init init_irq_stacks(void)
> > +{
> > +	return 0;
> > +}
> >  #endif /* CONFIG_IRQ_STACKS */
> >  
> >  int arch_show_interrupts(struct seq_file *p, int prec)
> > @@ -141,8 +161,9 @@ int arch_show_interrupts(struct seq_file *p, int prec)
> >  
> >  void __init init_IRQ(void)
> >  {
> > -	init_irq_scs();
> > -	init_irq_stacks();
> > +	if (init_irq_stacks() || init_irq_scs())
> > +		panic("Failed to allocate IRQ stack resources\n");
> 
> Any reason why the initialization order is reversed?
> 
It's a mistake, will reverse it back.

Thanks,
Osama

> > +
> >  	irqchip_init();
> >  	if (!handle_arch_irq)
> >  		panic("No interrupt controller found.");
> > -- 
> > 2.43.0
> 
> 
> - Paul