Re: [RFC PATCH] sparse_irq aka dyn_irq

[Ingo Molnar]

(Andrew, please see the early_kzalloc() reference below)

* Yinghai Lu <yinghai@xxxxxxxxxx> wrote:

> On Sat, Nov 8, 2008 at 11:38 PM, Ingo Molnar <mingo@xxxxxxx> 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
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/