Re: [PATCH v3 1/5] powerpc/xive: make xive IPI allocation NULL-safe

On 7/30/26 7:36 PM, Cédric Le Goater wrote:
> On 7/30/26 15:53, Shrikanth Hegde wrote:
>>
>>
>> On 7/27/26 4:12 PM, Gou Hao wrote:
>>> __GFP_NOFAIL should not be used in new code [1].  xive_init_ipis()
>>> allocates the xive_ipis array with __GFP_NOFAIL, which makes the
>>> subsequent NULL check unreachable dead code.
>>>
>>> Remove __GFP_NOFAIL so the allocation can fail, and make all xive_ipis
>>> access paths NULL-safe:
>>>
>>> - Return XIVE_BAD_IRQ from xive_ipi_cpu_to_irq() when xive_ipis is NULL.
>>> - Set xive_ipis to NULL after kfree() in the error path to prevent
>>>    use-after-free.
>>> - Guard xive_setup_cpu_ipi() and xive_cleanup_cpu_ipi() against
>>>    xive_ipi_irq == XIVE_BAD_IRQ to avoid dereferencing an uninitialized
>>>    or already-freed xive_ipis array.
>>
>> I would rather prefer a BUG_ON if the allocation fails. That keeps the 
>> earlier
>> semantic.
>>
>> - If xive fails, then who will send the interrupts?. It is better to 
>> crash instead
>> of leaving the system in weird state.
> 
> Then do like XICS, which has BUG_ON().

Yes. Either panic() or BUG_ON(). panic maybe a better option.

I don't like this code bloat which might leave it in inconsistent state.
Either recover properly such that ipi works or just crash the system as 
it is early in the system booting process.

> I don't know why we took this
> direction when XIVE was first introduced.
> 
> C.

Maybe due to NOFAIL, no one expected to ever fail.