[v5,02/33] genirq: Add irq_alloc_reserved_desc()

Commit Message

Yinghai Lu Feb. 22, 2014, 10:05 p.m. UTC

On Sat, Feb 22, 2014 at 9:28 AM, Yinghai Lu <[e-mail]> wrote:
> On Sat, Feb 22, 2014 at 2:08 AM, Thomas Gleixner <[e-mail]> wrote:
>>
>> As I said before irq_reserve_irq() is a misnomer and a
>> misconception. Of course this needs to be fixed as well.
>>
>> And you cannot just blindly change it because !SPARSE can use the
>> allocation. We are not creating stupid corner cases just to support
>> your sloppyness. Its not rocket science to do it the right way.
>>
>> That said, it might be worthwhile to get rid of the !SPARSE case
>> completely. That would probably make quite some stuff simpler.
>
> So we need to make all arches support SPARSE_IRQ at first?
>
> Now we have arm, arm64, c6x, metag, powerpc, sh, x86 support SPARSE_IRQ.
>
> The following  are not with SPARSE_IRQ yet:
> alpha, arc, avr32, blackfin, cris, frv, hexagon, m32r, m68k, microblaze,
> mips, mn10300, openrisc, parisc, s390, score, sparc, tile, um,
> unicore32, xtensa.

or add calling irq_alloc_desc_at() before irq_set_chip... for !SPARSE_IRQ.

Please check attached partial patch if you like it.

If you are happy with that, I will split it into pieces and add other
irq_alloc_desc_at()
calling.

Thanks

Yinghai

Comments

Thomas Gleixner Feb. 22, 2014, 11:38 p.m. UTC | #1

On Sat, 22 Feb 2014, Yinghai Lu wrote:
> On Sat, Feb 22, 2014 at 9:28 AM, Yinghai Lu <[e-mail]> wrote:
> > On Sat, Feb 22, 2014 at 2:08 AM, Thomas Gleixner <[e-mail]> wrote:
> >>
> >> As I said before irq_reserve_irq() is a misnomer and a
> >> misconception. Of course this needs to be fixed as well.
> >>
> >> And you cannot just blindly change it because !SPARSE can use the
> >> allocation. We are not creating stupid corner cases just to support
> >> your sloppyness. Its not rocket science to do it the right way.
> >>
> >> That said, it might be worthwhile to get rid of the !SPARSE case
> >> completely. That would probably make quite some stuff simpler.
> >
> > So we need to make all arches support SPARSE_IRQ at first?
> >
> > Now we have arm, arm64, c6x, metag, powerpc, sh, x86 support SPARSE_IRQ.
> >
> > The following  are not with SPARSE_IRQ yet:
> > alpha, arc, avr32, blackfin, cris, frv, hexagon, m32r, m68k, microblaze,
> > mips, mn10300, openrisc, parisc, s390, score, sparc, tile, um,
> > unicore32, xtensa.
> 
> or add calling irq_alloc_desc_at() before irq_set_chip... for !SPARSE_IRQ.
> 
> Please check attached partial patch if you like it.

OMG, you really mean that:

+++ b/arch/alpha/kernel/irq_i8259.c
@@ -92,6 +92,7 @@ init_i8259a_irqs(void)
 	outb(0xff, 0xA1);	/* mask all of 8259A-2 */
 
 	for (i = 0; i < 16; i++) {
+		irq_alloc_desc_at(i, 0);
 		irq_set_chip_and_handler(i, &i8259a_irq_type, handle_level_irq);
 	}

You can't be serious about that. There are tons of ways to call into
the core and access an irq descriptor aside of irq_set_chip* before it
is potentially allocated.

Are you going to analyze all of them and add an irq_alloc_desc_at()
before that call?

HELL, NO!

I'm really tired of that.

Stay away from kernel/irq/* and wait for people who are competent
enough and willing to spend the extra thoughts to come up with
solutions which are not completely ass backwards.

Thanks,

	tglx
--
To unsubscribe from this list: send the line "unsubscribe linux-pci" in
the body of a message to [e-mail]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Patch

diff --git a/arch/alpha/kernel/irq_alpha.c b/arch/alpha/kernel/irq_alpha.c
index 1c8625c..6c8728a 100644
--- a/arch/alpha/kernel/irq_alpha.c
+++ b/arch/alpha/kernel/irq_alpha.c
@@ -221,6 +221,7 @@  struct irqaction timer_irqaction = {
 void __init
 init_rtc_irq(void)
 {
+	irq_alloc_desc_at(RTC_IRQ, 0);
 	irq_set_chip_and_handler_name(RTC_IRQ, &dummy_irq_chip,
 				      handle_percpu_irq, "RTC");
 	setup_irq(RTC_IRQ, &timer_irqaction);
diff --git a/arch/alpha/kernel/irq_i8259.c b/arch/alpha/kernel/irq_i8259.c
index e1861c7..c522359 100644
--- a/arch/alpha/kernel/irq_i8259.c
+++ b/arch/alpha/kernel/irq_i8259.c
@@ -92,6 +92,7 @@  init_i8259a_irqs(void)
 	outb(0xff, 0xA1);	/* mask all of 8259A-2 */
 
 	for (i = 0; i < 16; i++) {
+		irq_alloc_desc_at(i, 0);
 		irq_set_chip_and_handler(i, &i8259a_irq_type, handle_level_irq);
 	}
 
diff --git a/arch/alpha/kernel/irq_pyxis.c b/arch/alpha/kernel/irq_pyxis.c
index 13c97a5..b6da6d1 100644
--- a/arch/alpha/kernel/irq_pyxis.c
+++ b/arch/alpha/kernel/irq_pyxis.c
@@ -102,6 +102,7 @@  init_pyxis_irqs(unsigned long ignore_mask)
 	for (i = 16; i < 48; ++i) {
 		if ((ignore_mask >> i) & 1)
 			continue;
+		irq_alloc_desc_at(i, 0);
 		irq_set_chip_and_handler(i, &pyxis_irq_type, handle_level_irq);
 		irq_set_status_flags(i, IRQ_LEVEL);
 	}
diff --git a/arch/s390/kernel/irq.c b/arch/s390/kernel/irq.c
index bb27a26..2d1567b 100644
--- a/arch/s390/kernel/irq.c
+++ b/arch/s390/kernel/irq.c
@@ -88,9 +88,13 @@  static const struct irq_class irqclass_sub_desc[NR_ARCH_IRQS] = {
 	[CPU_RST]    = {.name = "RST", .desc = "[CPU] CPU Restart"},
 };
 
+int arch_probe_early_allocate_nr_irqs(void)
+{
+	return THIN_INTERRUPT;
+}
+
 void __init init_IRQ(void)
 {
-	irq_reserve_irqs(0, THIN_INTERRUPT);
 	init_cio_interrupts();
 	init_airq_interrupts();
 	init_ext_interrupts();
diff --git a/arch/x86/kernel/apic/io_apic.c b/arch/x86/kernel/apic/io_apic.c
index 6ad4658..398f9c4 100644
--- a/arch/x86/kernel/apic/io_apic.c
+++ b/arch/x86/kernel/apic/io_apic.c
@@ -206,9 +206,6 @@  int __init arch_early_irq_init(void)
 	count = ARRAY_SIZE(irq_cfgx);
 	node = cpu_to_node(0);
 
-	/* Make sure the legacy interrupts are marked in the bitmap */
-	irq_reserve_irqs(0, legacy_pic->nr_legacy_irqs);
-
 	for (i = 0; i < count; i++) {
 		irq_set_chip_data(i, &cfg[i]);
 		zalloc_cpumask_var_node(&cfg[i].domain, GFP_KERNEL, node);
diff --git a/drivers/sh/intc/core.c b/drivers/sh/intc/core.c
index 8f32a13..05118f92 100644
--- a/drivers/sh/intc/core.c
+++ b/drivers/sh/intc/core.c
@@ -81,11 +81,8 @@  static void __init intc_register_irq(struct intc_desc *desc,
 	unsigned long flags;
 
 	/*
-	 * Register the IRQ position with the global IRQ map, then insert
-	 * it in to the radix tree.
+	 * insert it in to the radix tree.
 	 */
-	irq_reserve_irq(irq);
-
 	raw_spin_lock_irqsave(&intc_big_lock, flags);
 	radix_tree_insert(&d->tree, enum_id, intc_irq_xlate_get(irq));
 	raw_spin_unlock_irqrestore(&intc_big_lock, flags);
diff --git a/include/linux/irq.h b/include/linux/irq.h
index 7dc1003..584f0d7 100644
--- a/include/linux/irq.h
+++ b/include/linux/irq.h
@@ -613,18 +613,13 @@  int __irq_alloc_descs(int irq, unsigned int from, unsigned int cnt, int node,
 	irq_alloc_descs(-1, from, cnt, node)
 
 void irq_free_descs(unsigned int irq, unsigned int cnt);
-int irq_reserve_irqs(unsigned int from, unsigned int cnt);
+int arch_probe_early_allocate_nr_irqs(void);
 
 static inline void irq_free_desc(unsigned int irq)
 {
 	irq_free_descs(irq, 1);
 }
 
-static inline int irq_reserve_irq(unsigned int irq)
-{
-	return irq_reserve_irqs(irq, 1);
-}
-
 #ifndef irq_reg_writel
 # define irq_reg_writel(val, addr)	writel(val, addr)
 #endif
diff --git a/kernel/irq/chip.c b/kernel/irq/chip.c
index dc04c16..4d86cf5 100644
--- a/kernel/irq/chip.c
+++ b/kernel/irq/chip.c
@@ -38,12 +38,7 @@  int irq_set_chip(unsigned int irq, struct irq_chip *chip)
 
 	desc->irq_data.chip = chip;
 	irq_put_desc_unlock(desc, flags);
-	/*
-	 * For !CONFIG_SPARSE_IRQ make the irq show up in
-	 * allocated_irqs. For the CONFIG_SPARSE_IRQ case, it is
-	 * already marked, and this call is harmless.
-	 */
-	irq_reserve_irq(irq);
+
 	return 0;
 }
 EXPORT_SYMBOL(irq_set_chip);
diff --git a/kernel/irq/irqdesc.c b/kernel/irq/irqdesc.c
index 8ab8e93..4b2ab96 100644
--- a/kernel/irq/irqdesc.c
+++ b/kernel/irq/irqdesc.c
@@ -248,10 +248,16 @@  struct irq_desc irq_desc[NR_IRQS] __cacheline_aligned_in_smp = {
 	}
 };
 
+int __weak arch_probe_early_allocate_nr_irqs(void)
+{
+        return 0;
+}
+
 int __init early_irq_init(void)
 {
 	int count, i, node = first_online_node;
 	struct irq_desc *desc;
+	int nr = arch_probe_early_allocate_nr_irqs();
 
 	init_irq_default_affinity();
 
@@ -267,6 +273,10 @@  int __init early_irq_init(void)
 		lockdep_set_class(&desc[i].lock, &irq_desc_lock_class);
 		desc_set_defaults(i, &desc[i], node, NULL);
 	}
+
+	for (i = 0; i < nr; i++)
+		set_bit(i, allocated_irqs);
+
 	return arch_early_irq_init();
 }
 
@@ -390,31 +400,6 @@  err:
 EXPORT_SYMBOL_GPL(__irq_alloc_descs);
 
 /**
- * irq_reserve_irqs - mark irqs allocated
- * @from:	mark from irq number
- * @cnt:	number of irqs to mark
- *
- * Returns 0 on success or an appropriate error code
- */
-int irq_reserve_irqs(unsigned int from, unsigned int cnt)
-{
-	unsigned int start;
-	int ret = 0;
-
-	if (!cnt || (from + cnt) > nr_irqs)
-		return -EINVAL;
-
-	mutex_lock(&sparse_irq_lock);
-	start = bitmap_find_next_zero_area(allocated_irqs, nr_irqs, from, cnt, 0);
-	if (start == from)
-		bitmap_set(allocated_irqs, start, cnt);
-	else
-		ret = -EEXIST;
-	mutex_unlock(&sparse_irq_lock);
-	return ret;
-}
-
-/**
  * irq_get_next_irq - get next allocated irq number
  * @offset:	where to start the search
  *