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

[Kemeng Shi]

在 2026/6/17 20:09:51, Marc Zyngier 写道:
> On Tue, 16 Jun 2026 02:31:22 +0100,
> Kemeng Shi <shikemeng@xxxxxxxxxxxxxxx> wrote:
>>
>> 在 2026/6/15 16:52:56, Marc Zyngier 写道:
>>> On Mon, 15 Jun 2026 04:29:05 +0100,
>>> Kemeng Shi <shikemeng@xxxxxxxxxxxxxxx> 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@xxxxxxxxxxxxxxx>
>>>> ---
>>>>  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.
OK. I will drop this patch in next version.

Thanks,
Kemeng> 
> 	M.
>