irq: core changes for x86 ioapic hotplug

Hi,

These patches are core changes for x86 ioapic hotplug support.

First part for kill old irq_reserve_irqs:
During reviewing ioapic hotplug patchset, Thomas pointed out that should not extend irq_reserve_irq for that purpose as that is not actually reserve.
Neet to clean up old irq_reserve_irq before introduce reserve/alloc_reserved method for ioapic hotplug.
So here patchset that kill irq_reserve_irq that actually set allocated_irqs. First remove irq_reserve_irqs for x86, and remove irq_reserve_irq for sh.
Then in set_irq_chip use irq_alloc_desc instead of irq_reserve_irq. Next will mark bits in allocated_irqs early for init irqs in !SPARSE_IRQ

Second parts are new reserve/alloc_reserved functions:
It will introduce reserved_irqs bit maps to track reserved irqs. New irq_alloc_reserved_desc() will only allocate desc when that irq is reserved, at the same time irq_alloc_desc will only allocate desc when irq is not reserved.

Thanks

Yinghai

-v2: add new reserve/alloc_reserved sets.
-v3: make irq_alloc_descs to avoid reserved irqs.

Yinghai Lu (8):
x86, irq: Remove not needed irq_reserve_irqs calling sh, irq: Remove irq_reserve_irq calling
irq: Use irq_alloc_desc_at instead of irq_reserve_irq
s390: Mark bits in allocated_irqs in general code
irq: Kill irq_reserve_irq/irq_reserve_irqs
irq: Add new reserved_irqs clear/mark functions
irq: Add irq_alloc_reserved_desc()
irq: Do not free unallocated irq descriptors

 arch/s390/kernel/irq.c         |   6 +-
 arch/x86/kernel/apic/io_apic.c |   3 -
 drivers/sh/intc/core.c         |   7 +-
 include/linux/irq.h            |  13 ++--
 kernel/irq/chip.c              |  14 ++--
 kernel/irq/irqdesc.c           | 144 ++++++++++++++++++++++++++++++++++-------
 6 files changed, 141 insertions(+), 46 deletions(-)

1.8.4.5

Now x86 only support sparseirq path, for that path, calling path like:

	early_irq_init
		==> arch_probe_nr_irqs : return legacy irq number
		==> alloc_desc for legacy irqs and set bits in allocated_irqs
		==> arch_early_irq_init
			==> irq_reserve_irqs : set bits again

so we can kill one irq_reserve_irqs calling.

---
 arch/x86/kernel/apic/io_apic.c | 3 ---
 1 file changed, 3 deletions(-)

index 6ad4658..398f9c4 100644
@@ -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);

1.8.4.5

in sh calling path:
register_intc_controller
=> irq_create_identity_mapping/irq_create_strict_mappins
=>irq_alloc_desc: it will set bits on allocate_irq
=> intc_register_irq
=> irq_reserve_irq: set bits again

so we can kill this irq_reserve_irq calling.

-v2: Fix comment style according to Sergei Shtylyov.

---
 drivers/sh/intc/core.c | 7 +------
 1 file changed, 1 insertion(+), 6 deletions(-)

index 8f32a13..d146ce1 100644
@@ -80,12 +80,7 @@ static void __init intc_register_irq(struct intc_desc *desc,
 	unsigned int data[2], primary;
 	unsigned long flags;
 
-	/*
-	 * Register the IRQ position with the global IRQ map, then insert
-	 * it in to the radix tree.
-	 */
-	irq_reserve_irq(irq);
-
+	/* Insert it in to the radix tree. */
 	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);

1.8.4.5

irq_reserve_irq actually only set bit allocated_irq, and it is not really "reserve" and cause confusion.

For !CONFIG_SPARSE_IRQ path, irq_alloc_desc_at() will only set bit in allocated_irq.

We can use that instead, kill one irq_reserve_irq() calling.

---
 kernel/irq/chip.c | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

index 6397df2..aff7481 100644
@@ -28,8 +28,13 @@
 int irq_set_chip(unsigned int irq, struct irq_chip *chip)
 {
 	unsigned long flags;
-	struct irq_desc *desc = irq_get_desc_lock(irq, &flags, 0);
+	struct irq_desc *desc;
+
+#ifndef CONFIG_SPARSE_IRQ
+	irq_alloc_desc_at(irq, 0);
+#endif
 
+	desc = irq_get_desc_lock(irq, &flags, 0);
 	if (!desc)
 		return -EINVAL;
 

@@ -38,12 +43,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);

1.8.4.5

Second irq_reserve_irqs calling is from arch s390, and s390 does not use SPARSE_IRQ yet.

We could set bits for legacy bits early in !SPARSE_IRQ version early_irq_init() directly instead of calling irq_reserve_irqs later.

Adding weak version arch_proble_early_allocate_nr_irqs() for !SPARESE_IRQ, and let s390 to have specific version to pass correct irq number that need to be marked in allocated_irqs.

---
 arch/s390/kernel/irq.c |  6 +++++-
 include/linux/irq.h    |  1 +
 kernel/irq/irqdesc.c   | 10 ++++++++++
 3 files changed, 16 insertions(+), 1 deletion(-)

index c7463aa..47e29be 100644
@@ -90,9 +90,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();

index 10a0b1a..28cbd3e 100644
@@ -624,6 +624,7 @@ int __irq_alloc_descs(int irq, unsigned int from, unsigned int cnt, int 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)
 {

index a7174617..dfb971c 100644
@@ -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();
 }
 

1.8.4.5

No user any more.

---
 include/linux/irq.h  |  6 ------
 kernel/irq/irqdesc.c | 25 -------------------------
 2 files changed, 31 deletions(-)

index 28cbd3e..02dc0e4 100644
@@ -623,7 +623,6 @@ 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)

@@ -631,11 +630,6 @@ 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

index dfb971c..49bf891 100644
@@ -400,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
  *

1.8.4.5

Prepare for ioapic hotplug.

-v2: Add reserved_or_allocated_irqs to simplify the code.

---
 include/linux/irq.h  |  3 +++
 kernel/irq/irqdesc.c | 65 ++++++++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 68 insertions(+)

index 02dc0e4..2ba3245 100644
@@ -603,6 +603,9 @@ static inline u32 irq_get_trigger_type(unsigned int irq)
 	return d ? irqd_get_trigger_type(d) : 0;
 }
 
+int irq_clear_reserved_irqs(unsigned int from, unsigned int cnt);
+int irq_mark_reserved_irqs(int irq, unsigned int from, unsigned int cnt);
+
 int __irq_alloc_descs(int irq, unsigned int from, unsigned int cnt, int node,
 		struct module *owner);
 

1.8.4.5

For ioapic hot-add support, it would be easy if we have continuous irq numbers for hot added ioapic controller.

We can reserve irq range at first, and later allocate desc for those pre-reserved irqs when they are needed.

The reasons for not allocating them during reserving:
1. only several pins of one ioapic are used, allocate for all pins, will waste memory for not used pins.
2. allocate later when is needed could make sure irq_desc is allocated on local node ram, as dev->node is set at that point.

-v2: update changelog by adding reasons, requested by Konrad. -v3: according to tglx:
separate core code change with arch code change. change function name to irq_alloc_reserved_desc. kill __irq_is_reserved().
remove not need exports.
according to Sebastian:
spare one comments by put two functions together.
-v4: Fix the bug that tglx pointed out.
irq_alloc_descs() should avoid reserved irqs.

---
 include/linux/irq.h  |  3 ++
 kernel/irq/irqdesc.c | 97 ++++++++++++++++++++++++++++++++++++++--------------
 2 files changed, 74 insertions(+), 26 deletions(-)

index 2ba3245..97b60b9 100644
@@ -608,10 +608,13 @@ int irq_mark_reserved_irqs(int irq, unsigned int from, unsigned int cnt);
 
 int __irq_alloc_descs(int irq, unsigned int from, unsigned int cnt, int node,
 		struct module *owner);
+int __irq_alloc_reserved_desc(int at, int node, struct module *owner);
 
 /* use macros to avoid needing export.h for THIS_MODULE */
 #define irq_alloc_descs(irq, from, cnt, node)	\
 	__irq_alloc_descs(irq, from, cnt, node, THIS_MODULE)
+#define irq_alloc_reserved_desc_at(at, node)	\
+	__irq_alloc_reserved_desc(at, node, THIS_MODULE)
 
 #define irq_alloc_desc(node)			\
 	irq_alloc_descs(-1, 0, 1, node)

1.8.4.5

Hot-added interrupt controllers can reserve a range of interrupt numbers, but only allocate some of them. To simplify the release on hot-remove allow them to iterate over the reserved range, let the free_desc() code return early when the descriptor does not exist

-v2: changelog from tglx

---
 kernel/irq/irqdesc.c | 3 +++
 1 file changed, 3 insertions(+)

index 7b2b83a..b46ab9e 100644
@@ -167,6 +167,9 @@ static void free_desc(unsigned int irq)
 {
 	struct irq_desc *desc = irq_to_desc(irq);
 
+	if (!desc)
+		return;
+
 	unregister_irq_proc(irq, desc);
 
 	mutex_lock(&sparse_irq_lock);

1.8.4.5

That's not a proper description of the problem and the solution. The same problem of lack of proper explanation permeates most of your patches as well and that's very far from the quality threshold that core IRQ patches need to meet.

Crappy descriptions and passive-aggressive responses where you pretend you don't understand our complaints and just minimally address the problems are not acceptable.

So I concur with Thomas and this series earns a big fat NAK from me. (Please Cc: me to all (if any) future resubmissions so I can track progress (or the lack thereof) and see whether I can lift my NAK.)

Thanks,

Ingo