assign_irq_vector() should not panic

PATCH In review Kenji Kaneshige 2 participants 3 messages Jul 1, 2005 – Jul 11, 2005

lore

Share & tools

lore

Kenji Kaneshige

author

2005-07-01 09:53 UTC #0 lore raw

PATCH 1/1 assign_irq_vector() should not panic

Hi,

This is the updated version of the patch I've posted at:

http://marc.theaimsgroup.com/?l=linux-ia64&m1803534229544&w=2

Tony, could you consider applying this?

Thanks,
Kenji Kaneshige

Current assign_irq_vector() will panic if interrupt vectors is running out. But I think how to handle the case of lack of interrupt vectors should be handled by the caller of this function. For example, some PCI devices can raise the interrupt signal via both MSI and I/O APIC. So even if the driver for these device fails to allocate a vector for MSI, the driver still has a chance to use I/O APIC based interrupt. But currently there is no chance for these driver to use I/O APIC based interrupt because kernel will panic when assign_irq_vector() fails to allocate interrupt vector.

The following patch changes assign_irq_vector() for ia64 to return negative value on error instead of panic.

Signed-off-by Kenji Kaneshige <[email protected]>

---

 linux-2.6.13-rc1-kanesige/arch/ia64/kernel/iosapic.c  |   13 +++++++++----
 linux-2.6.13-rc1-kanesige/arch/ia64/kernel/irq_ia64.c |   13 +------------
 linux-2.6.13-rc1-kanesige/include/asm-ia64/hw_irq.h   |    1 -
 3 files changed, 10 insertions(+), 17 deletions(-)

arch/ia64/kernel/iosapic.c renamed from arch/ia64/kernel/iosapic.c~remove-panic-assign_irq_vector-ia64 +9 -4

diff -puN arch/ia64/kernel/iosapic.c~remove-panic-assign_irq_vector-ia64 arch/ia64/kernel/iosapic.c
@@ -489,8 +489,6 @@ static int iosapic_find_sharable_vector 
 			}
 		}
 	}
-	if (vector < 0)
-		panic("%s: out of interrupt vectors!\n", __FUNCTION__);
 
 	return vector;
 }

@@ -506,6 +504,8 @@ iosapic_reassign_vector (int vector)
 
 	if (!list_empty(&iosapic_intr_info[vector].rtes)) {
 		new_vector = assign_irq_vector(AUTO_ASSIGN);
+		if (new_vector < 0)
+			panic("%s: out of interrupt vectors!\n", __FUNCTION__);
 		printk(KERN_INFO "Reassigning vector %d to %d\n", vector, new_vector);
 		memcpy(&iosapic_intr_info[new_vector], &iosapic_intr_info[vector],
 		       sizeof(struct iosapic_intr_info));

@@ -734,9 +734,12 @@ again:
 	spin_unlock_irqrestore(&iosapic_lock, flags);
 
 	/* If vector is running out, we try to find a sharable vector */
-	vector = assign_irq_vector_nopanic(AUTO_ASSIGN);
-	if (vector < 0)
+	vector = assign_irq_vector(AUTO_ASSIGN);
+	if (vector < 0) {
 		vector = iosapic_find_sharable_vector(trigger, polarity);
+		if (vector < 0)
+			panic("%s: out of interrupt vectors!\n", __FUNCTION__);
+	}
 
 	spin_lock_irqsave(&irq_descp(vector)->lock, flags);
 	spin_lock(&iosapic_lock);

@@ -884,6 +887,8 @@ iosapic_register_platform_intr (u32 int_
 		break;
 	      case ACPI_INTERRUPT_INIT:
 		vector = assign_irq_vector(AUTO_ASSIGN);
+		if (vector < 0)
+			panic("%s: out of interrupt vectors!\n", __FUNCTION__);
 		delivery = IOSAPIC_INIT;
 		break;
 	      case ACPI_INTERRUPT_CPEI:

arch/ia64/kernel/irq_ia64.c renamed from arch/ia64/kernel/irq_ia64.c~remove-panic-assign_irq_vector-ia64 +1 ~~-12~~

diff -puN arch/ia64/kernel/irq_ia64.c~remove-panic-assign_irq_vector-ia64 arch/ia64/kernel/irq_ia64.c
@@ -63,7 +63,7 @@ EXPORT_SYMBOL(isa_irq_to_vector_map);
 static unsigned long ia64_vector_mask[BITS_TO_LONGS(IA64_NUM_DEVICE_VECTORS)];
 
 int
-assign_irq_vector_nopanic (int irq)
+assign_irq_vector (int irq)
 {
 	int pos, vector;
  again:

@@ -76,17 +76,6 @@ assign_irq_vector_nopanic (int irq)
 	return vector;
 }
 
-int
-assign_irq_vector (int irq)
-{
-	int vector = assign_irq_vector_nopanic(irq);
-
-	if (vector < 0)
-		panic("assign_irq_vector: out of interrupt vectors!");
-
-	return vector;
-}
-
 void
 free_irq_vector (int vector)
 {

include/asm-ia64/hw_irq.h renamed from include/asm-ia64/hw_irq.h~remove-panic-assign_irq_vector-ia64 +0 -1

diff -puN include/asm-ia64/hw_irq.h~remove-panic-assign_irq_vector-ia64 include/asm-ia64/hw_irq.h
@@ -81,7 +81,6 @@ extern __u8 isa_irq_to_vector_map[16];
 
 extern struct hw_interrupt_type irq_type_ia64_lsapic;	/* CPU-internal interrupt controller */
 
-extern int assign_irq_vector_nopanic (int irq); /* allocate a free vector without panic */
 extern int assign_irq_vector (int irq);	/* allocate a free vector */
 extern void free_irq_vector (int vector);
 extern void ia64_send_ipi (int cpu, int vector, int delivery_mode, int redirect);

Luck, Tony

2005-07-08 21:34 UTC #1 lore raw

Quoted 3 lines

The following patch changes assign_irq_vector() for ia64 to return negative value on error instead of panic.

The i386 and x86_64 versions of this return -ENOSPC rather than -1. Perhaps we should do the same?

There are a couple of ia64 specific uses of assign_irq_vector in hp/sim/simeth.c and hp/sim/simserial.c that don't check the return value ... if you make this change, then they need to check for failure too.

-Tony

Kenji Kaneshige

author

2005-07-11 04:48 UTC #2 lore raw

In reply to Kenji Kaneshige

Hi Tony,

Thank you very much for reviewing.

Quoted 2 lines

The i386 and x86_64 versions of this return -ENOSPC rather than -1. Perhaps we should do the same?

Yes, I think you're right.

Quoted 3 lines

There are a couple of ia64 specific uses of assign_irq_vector in hp/sim/simeth.c and hp/sim/simserial.c that don't check the return value ... if you make this change, then they need to check for failure too.

Oh, I was very silly.
Thank you for pointing this out.

I'm attaching the updated patch that incorporates all of your comment. Please take a look.

Thanks,
Kenji Kaneshige

---

The following patch changes assign_irq_vector() for ia64 to return negative value on error instead of panic.

Signed-off-by Kenji Kaneshige <[email protected]>

---

 linux-2.6.13-rc1-kanesige/arch/ia64/hp/sim/simeth.c    |    6 ++++--
 linux-2.6.13-rc1-kanesige/arch/ia64/hp/sim/simserial.c |    7 +++++--
 linux-2.6.13-rc1-kanesige/arch/ia64/kernel/iosapic.c   |   13 +++++++++----
 linux-2.6.13-rc1-kanesige/arch/ia64/kernel/irq_ia64.c  |   15 ++-------------
 linux-2.6.13-rc1-kanesige/include/asm-ia64/hw_irq.h    |    1 -
 5 files changed, 20 insertions(+), 22 deletions(-)

arch/ia64/hp/sim/simeth.c renamed from arch/ia64/hp/sim/simeth.c~remove-panic-assign_irq_vector-ia64 +4 -2

diff -puN arch/ia64/hp/sim/simeth.c~remove-panic-assign_irq_vector-ia64 arch/ia64/hp/sim/simeth.c
@@ -191,7 +191,7 @@ simeth_probe1(void)
 	unsigned char mac_addr[ETH_ALEN];
 	struct simeth_local *local;
 	struct net_device *dev;
-	int fd, i, err;
+	int fd, i, err, rc;
 
 	/*
 	 * XXX Fix me

@@ -228,7 +228,9 @@ simeth_probe1(void)
 		return err;
 	}
 
-	dev->irq = assign_irq_vector(AUTO_ASSIGN);
+	if ((rc = assign_irq_vector(AUTO_ASSIGN)) < 0)
+		panic("%s: out of interrupt vectors!\n", __FUNCTION__);
+	dev->irq = rc;
 
 	/*
 	 * attach the interrupt in the simulator, this does enable interrupts

arch/ia64/hp/sim/simserial.c renamed from arch/ia64/hp/sim/simserial.c~remove-panic-assign_irq_vector-ia64 +5 -2

diff -puN arch/ia64/hp/sim/simserial.c~remove-panic-assign_irq_vector-ia64 arch/ia64/hp/sim/simserial.c
@@ -976,7 +976,7 @@ static struct tty_operations hp_ops = {
 static int __init
 simrs_init (void)
 {
-	int			i;
+	int			i, rc;
 	struct serial_state	*state;
 
 	if (!ia64_platform_is("hpsim"))

@@ -1011,7 +1011,10 @@ simrs_init (void)
 		if (state->type = PORT_UNKNOWN) continue;
 
 		if (!state->irq) {
-			state->irq = assign_irq_vector(AUTO_ASSIGN);
+			if ((rc = assign_irq_vector(AUTO_ASSIGN)) < 0)
+				panic("%s: out of interrupt vectors!\n",
+				      __FUNCTION__);
+			state->irq = rc;
 			ia64_ssc_connect_irq(KEYBOARD_INTR, state->irq);
 		}

arch/ia64/kernel/iosapic.c renamed from arch/ia64/kernel/iosapic.c~remove-panic-assign_irq_vector-ia64 +9 -4

diff -puN arch/ia64/kernel/iosapic.c~remove-panic-assign_irq_vector-ia64 arch/ia64/kernel/iosapic.c
@@ -489,8 +489,6 @@ static int iosapic_find_sharable_vector 
 			}
 		}
 	}
-	if (vector < 0)
-		panic("%s: out of interrupt vectors!\n", __FUNCTION__);
 
 	return vector;
 }

@@ -506,6 +504,8 @@ iosapic_reassign_vector (int vector)
 
 	if (!list_empty(&iosapic_intr_info[vector].rtes)) {
 		new_vector = assign_irq_vector(AUTO_ASSIGN);
+		if (new_vector < 0)
+			panic("%s: out of interrupt vectors!\n", __FUNCTION__);
 		printk(KERN_INFO "Reassigning vector %d to %d\n", vector, new_vector);
 		memcpy(&iosapic_intr_info[new_vector], &iosapic_intr_info[vector],
 		       sizeof(struct iosapic_intr_info));

@@ -734,9 +734,12 @@ again:
 	spin_unlock_irqrestore(&iosapic_lock, flags);
 
 	/* If vector is running out, we try to find a sharable vector */
-	vector = assign_irq_vector_nopanic(AUTO_ASSIGN);
-	if (vector < 0)
+	vector = assign_irq_vector(AUTO_ASSIGN);
+	if (vector < 0) {
 		vector = iosapic_find_sharable_vector(trigger, polarity);
+		if (vector < 0)
+			panic("%s: out of interrupt vectors!\n", __FUNCTION__);
+	}
 
 	spin_lock_irqsave(&irq_descp(vector)->lock, flags);
 	spin_lock(&iosapic_lock);

@@ -884,6 +887,8 @@ iosapic_register_platform_intr (u32 int_
 		break;
 	      case ACPI_INTERRUPT_INIT:
 		vector = assign_irq_vector(AUTO_ASSIGN);
+		if (vector < 0)
+			panic("%s: out of interrupt vectors!\n", __FUNCTION__);
 		delivery = IOSAPIC_INIT;
 		break;
 	      case ACPI_INTERRUPT_CPEI:

arch/ia64/kernel/irq_ia64.c renamed from arch/ia64/kernel/irq_ia64.c~remove-panic-assign_irq_vector-ia64 +2 ~~-13~~

diff -puN arch/ia64/kernel/irq_ia64.c~remove-panic-assign_irq_vector-ia64 arch/ia64/kernel/irq_ia64.c
@@ -63,30 +63,19 @@ EXPORT_SYMBOL(isa_irq_to_vector_map);
 static unsigned long ia64_vector_mask[BITS_TO_LONGS(IA64_NUM_DEVICE_VECTORS)];
 
 int
-assign_irq_vector_nopanic (int irq)
+assign_irq_vector (int irq)
 {
 	int pos, vector;
  again:
 	pos = find_first_zero_bit(ia64_vector_mask, IA64_NUM_DEVICE_VECTORS);
 	vector = IA64_FIRST_DEVICE_VECTOR + pos;
 	if (vector > IA64_LAST_DEVICE_VECTOR)
-		return -1;
+		return -ENOSPC;
 	if (test_and_set_bit(pos, ia64_vector_mask))
 		goto again;
 	return vector;
 }
 
-int
-assign_irq_vector (int irq)
-{
-	int vector = assign_irq_vector_nopanic(irq);
-
-	if (vector < 0)
-		panic("assign_irq_vector: out of interrupt vectors!");
-
-	return vector;
-}
-
 void
 free_irq_vector (int vector)
 {

include/asm-ia64/hw_irq.h renamed from include/asm-ia64/hw_irq.h~remove-panic-assign_irq_vector-ia64 +0 -1

diff -puN include/asm-ia64/hw_irq.h~remove-panic-assign_irq_vector-ia64 include/asm-ia64/hw_irq.h
@@ -81,7 +81,6 @@ extern __u8 isa_irq_to_vector_map[16];
 
 extern struct hw_interrupt_type irq_type_ia64_lsapic;	/* CPU-internal interrupt controller */
 
-extern int assign_irq_vector_nopanic (int irq); /* allocate a free vector without panic */
 extern int assign_irq_vector (int irq);	/* allocate a free vector */
 extern void free_irq_vector (int vector);
 extern void ia64_send_ipi (int cpu, int vector, int delivery_mode, int redirect);