lkml.org 
[lkml]   [2008]   [Nov]   [10]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    Patch in this message
    /
    Date
    From
    SubjectRe: [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


    \
     
     \ /
      Last update: 2008-11-10 10:43    [W:2.555 / U:0.000 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site