Re: [PATCH] irqdesc: Fail check on early_irq_init allocation.

[Marc Zyngier]

On Sun, 12 Nov 2023 14:19:28 +0000,
Paran Lee <p4ranlee@xxxxxxxxx> wrote:
> On 2023-11-12 오후 11:00, Marc Zyngier wrote:
> 
> Thanks for the code review Marc!
> 
> I think function alloc_descs() in irqdesc.c has also alloc_desc() fail
> handling, and there's kernel-wide code consistency checking for
> allocation failures, and I thought it would be nice to mark it.

alloc_descs() and early_irq_init() are very different beasts. The
former can be used *at any time* over the kernel's lifetime, while the
latter is only used *once*. This makes a whole lot a difference, don't
you think?

> So that the code is aware of it.
> 
> Even if it panics with a null derefence reference.

Don't you think it is a bit pointless to trade a fatal error for
another one?

> 
> > A failing allocation already results in a massive splat describing how
> > the allocation failed. Further use of the NULL pointer will also
> > result in a terminal oops, particularly if this happens this early in
> > the boot sequence.
> > 
> > So what do these BUG_ON() calls buy us?
> > 
> > 	M.
> > 
> 
> If anyone has any ideas on how to get a little fancier with the allocation,
> I'll send a v2 patch in that direction.

It's not about being fancy. It is about being useful. Your BUG_ON()s
are not making things any better for early allocation failures.

A much better idea would be to *get rid* of early allocation failures
altogether, by moving all architectures to SPARSE_IRQ and making sure
that NR_LEGAY_IRQ is always zero, meaning there is nothing to
allocate. That would be something useful.

But adding random BUG_ON() based on the dogma that all allocations
must be checked doesn't bring value to the kernel as a whole.

	M.

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