Messages in this thread | | | Date | Sun, 12 Nov 2023 15:08:05 +0000 | From | Marc Zyngier <> | Subject | Re: [PATCH] irqdesc: Fail check on early_irq_init allocation. |
| |
On Sun, 12 Nov 2023 14:19:28 +0000, Paran Lee <p4ranlee@gmail.com> 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.
| |