Re: [PATCH 1/6] irqchip/gic-v3-its: Fix LPI range leak and refactor error handler in its_lpi_alloc()

[Marc Zyngier <maz@kernel.org>]

On Tue, 16 Jun 2026 02:31:22 +0100,
Kemeng Shi <shikemeng@huaweicloud.com> wrote:
> 
> 在 2026/6/15 16:52:56, Marc Zyngier 写道:
> > On Mon, 15 Jun 2026 04:29:05 +0100,
> > Kemeng Shi <shikemeng@huaweicloud.com> wrote:
> >>
> >> Fix the LIP range leak when bitmap_zalloc() failed. Besides refactor
> > 
> > Typo.
> > 
> >> error handling code to make it a little simpler.
> > 
> > No. Please don't mix fixes and (totally pointless) refactoring.
> OK, I will only keep fix in this patch.> 
> >>
> >> Signed-off-by: Kemeng Shi <shikemeng@huaweicloud.com>
> >> ---
> >>  drivers/irqchip/irq-gic-v3-its.c | 21 +++++++++------------
> >>  1 file changed, 9 insertions(+), 12 deletions(-)
> >>
> >> diff --git a/drivers/irqchip/irq-gic-v3-its.c b/drivers/irqchip/irq-gic-v3-its.c
> >> index 291d7668cc8d..2b7b546c43c8 100644
> >> --- a/drivers/irqchip/irq-gic-v3-its.c
> >> +++ b/drivers/irqchip/irq-gic-v3-its.c
> >> @@ -2217,10 +2217,9 @@ static int __init its_lpi_init(u32 id_bits)
> >>  static unsigned long *its_lpi_alloc(int nr_irqs, u32 *base, int *nr_ids)
> >>  {
> >>  	unsigned long *bitmap = NULL;
> >> -	int err = 0;
> >>  
> >>  	do {
> >> -		err = alloc_lpi_range(nr_irqs, base);
> >> +		int err = alloc_lpi_range(nr_irqs, base);
> >>  		if (!err)
> >>  			break;
> >>  
> >> @@ -2228,22 +2227,20 @@ static unsigned long *its_lpi_alloc(int nr_irqs, u32 *base, int *nr_ids)
> >>  	} while (nr_irqs > 0);
> >>  
> >>  	if (!nr_irqs)
> >> -		err = -ENOSPC;
> >> -
> >> -	if (err)
> >> -		goto out;
> >> +		goto err_out;
> >>  
> >>  	bitmap = bitmap_zalloc(nr_irqs, GFP_ATOMIC);
> >>  	if (!bitmap)
> >> -		goto out;
> >> +		goto err_free_lpi;
> >>  
> >>  	*nr_ids = nr_irqs;
> >> -
> >> -out:
> >> -	if (!bitmap)
> >> -		*base = *nr_ids = 0;
> >> -
> >>  	return bitmap;
> >> +
> >> +err_free_lpi:
> >> +	free_lpi_range(*base, nr_irqs);
> >> +err_out:
> >> +	*base = *nr_ids = 0;
> >> +	return NULL;
> >>  }
> >>  
> >>  static void its_lpi_free(unsigned long *bitmap, u32 base, u32 nr_ids)
> > 
> > Honestly, I question the validity of handling errors this way. You are
> > already unable to allocate a per-device bitmap. And yet you are
> > calling free_lpi_range(), which has the interesting property of
> > *allocating* memory. Which you don't have. Oh wait...
> You are right. I'm considering use xarray to track the lpi range or
> modify free_lpi_range to try merge first before memory allocation.
> What would you recommend?

My personal take on this is that leaking a few LPIs is not a big deal,
given how many we have. You are trying to optimise for an error case
that never happens, and I really don't want to add more complexity to
this.

	M.

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