linux-kernel - Re: [RFC PATCH] sparse_irq aka dyn

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20081110094033.GL22392@elte.hu>
Date:	Mon, 10 Nov 2008 10:40:33 +0100
From:	Ingo Molnar <mingo@...e.hu>
To:	Yinghai Lu <yinghai@...nel.org>,
	Andrew Morton <akpm@...ux-foundation.org>
Cc:	Thomas Gleixner <tglx@...utronix.de>,
	"H. Peter Anvin" <hpa@...or.com>,
	Andrew Morton <akpm@...ux-foundation.org>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: Re: [RFC PATCH] sparse_irq aka dyn_irq


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

* Yinghai Lu <yinghai@...nel.org> wrote:

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