Messages in this thread Patch in this message | | | Date | Mon, 10 Nov 2008 10:40:33 +0100 | From | Ingo Molnar <> | Subject | Re: [RFC PATCH] sparse_irq aka dyn_irq |
| |
(Andrew, please see the early_kzalloc() reference below)
* Yinghai Lu <yinghai@kernel.org> wrote:
> On Sat, Nov 8, 2008 at 11:38 PM, Ingo Molnar <mingo@elte.hu> wrote: > > > > General impression: very nice patch! > > > > A lot of the structural problems have been addressed: the descriptor > > lookup is now hashed, the dynarray stuff got cleaned up / eliminated, > > the irq_desc->chip_data binding is very nice as well. > > > > (And the patch needs to be split up like it was in the past, once all > > review feedback has been seen and addressed.) > > > >> +config HAVE_SPARSE_IRQ > >> + bool > >> + default y > > > > i think it should be made user-configurable - at least initially. It > > should not cause extra complications, right? > > io_apic.c will get more complicated.
yes, with such constructs:
+#ifdef CONFIG_SPARSE_IRQ + struct irq_desc *desc; + + /* first time to refer irq_cfg, so with new */ + desc = irq_to_desc_alloc_cpu(irq, cpu); + cfg = desc->chip_data; +#else + cfg = irq_cfg(irq); +#endif
please introduce a proper helper that eliminates such complications. Any reason why chip_data could not be used in the !SPARSE_IRQ case? irq_cfg_alloc() perhaps?
> >> + if (irq < NR_IRQS_LEGACY) { > > > > please s/NR_IRQS_LEGACY/NR_IRQS_X86_LEGACY - this is never used > > outside of x86 code. > > will use that in kernel/irq/handle.c too, because dyn_array is dumped.
ah, i missed that. Okay - lets keep NR_IRQS_LEGACY then.
> >> @@ -987,6 +988,8 @@ void __init mem_init(void) > >> > >> set_highmem_pages_init(); > >> > >> + after_bootmem = 1; > > > > this hack can go away once we have a proper percpu_alloc() that can be > > used early enough. > > where is that fancy patch? current percpu_alloc(), will keep big > pointer in array..., instead of put that pointer in percpu_area > > 64bit has that after_bootmem already.
or at least introduce a "bootmem agnostic" allocator instead of open-coding the after_bootmem flag.
Something like:
early_kzalloc()
?
Andrew, any preferences? Ingo
| |