From: Ingo Molnar Date: Mon, 26 Jun 2006 11:57:16 +0000 (+0200) Subject: [PATCH] x86_64: fix vector_lock deadlock in io_apic.c X-Git-Tag: v2.6.18-rc1~612^2~57 X-Git-Url: https://err.no/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=26a3c49cec96ffb9cfcc30dfa0cd05ccc25dcb3a;p=linux-2.6 [PATCH] x86_64: fix vector_lock deadlock in io_apic.c Fix a potential deadlock scenario introduced by io_apic.c's new vector_lock on i386 and x86_64. Found by the locking correctness validator. The patch was boot-tested on x86. For details of the deadlock scenario, see the validator output: ====================================================== [ BUG: hard-safe -> hard-unsafe lock order detected! ] ------------------------------------------------------ idle/1 [HC0[0]:SC0[0]:HE0:SE1] is trying to acquire: (msi_lock){....}, at: [] startup_msi_irq_wo_maskbit+0x10/0x35 and this task is already holding: (&irq_desc[i].lock){++..}, at: [] probe_irq_on+0x36/0x107 which would create a new lock dependency: (&irq_desc[i].lock){++..} -> (msi_lock){....} but this new dependency connects a hard-irq-safe lock: (&irq_desc[i].lock){++..} ... which became hard-irq-safe at: [] lockdep_acquire+0x68/0x84 [] _spin_lock+0x21/0x2f [] __do_IRQ+0x3d/0x113 [] do_IRQ+0x8c/0xad to a hard-irq-unsafe lock: (vector_lock){--..} ... which became hard-irq-unsafe at: ... [] lockdep_acquire+0x68/0x84 [] _spin_lock+0x21/0x2f [] assign_irq_vector+0x34/0xc8 [] setup_IO_APIC+0x45a/0xcff [] smp_prepare_cpus+0x5ea/0x8aa [] init+0x32/0x2cb [] kernel_thread_helper+0x5/0xb which could potentially lead to deadlocks! other info that might help us debug this: 3 locks held by idle/1: #0: (port_mutex){--..}, at: [] uart_add_one_port+0x61/0x289 #1: (&state->mutex){--..}, at: [] uart_add_one_port+0x73/0x289 #2: (&irq_desc[i].lock){++..}, at: [] probe_irq_on+0x36/0x107 the hard-irq-safe lock's dependencies: -> (&irq_desc[i].lock){++..} ops: 9861 { initial-use at: [] lockdep_acquire+0x68/0x84 [] _spin_lock_irqsave+0x2a/0x3a [] setup_irq+0x9b/0x14d [] time_init_hook+0xf/0x11 [] time_init+0x44/0x46 [] start_kernel+0x191/0x38f [] 0xc0100210 in-hardirq-W at: [] lockdep_acquire+0x68/0x84 [] _spin_lock+0x21/0x2f [] __do_IRQ+0x3d/0x113 [] do_IRQ+0x8c/0xad in-softirq-W at: [] lockdep_acquire+0x68/0x84 [] _spin_lock+0x21/0x2f [] __do_IRQ+0x3d/0x113 [] do_IRQ+0x8c/0xad } ... key at: [] irq_desc_lock_type+0x0/0x20 -> (i8259A_lock){++..} ops: 5149 { initial-use at: [] lockdep_acquire+0x68/0x84 [] _spin_lock_irqsave+0x2a/0x3a [] init_8259A+0x11/0x8f [] init_ISA_irqs+0x12/0x4d [] pre_intr_init_hook+0x8/0xa [] init_IRQ+0xe/0x65 [] start_kernel+0x178/0x38f [] 0xc0100210 in-hardirq-W at: [] lockdep_acquire+0x68/0x84 [] _spin_lock_irqsave+0x2a/0x3a [] mask_and_ack_8259A+0x1b/0xcc [] __do_IRQ+0x4f/0x113 [] do_IRQ+0x8c/0xad in-softirq-W at: [] lockdep_acquire+0x68/0x84 [] _spin_lock_irqsave+0x2a/0x3a [] mask_and_ack_8259A+0x1b/0xcc [] __do_IRQ+0x4f/0x113 [] do_IRQ+0x8c/0xad } ... key at: [] i8259A_lock+0x14/0x40 ... acquired at: [] lockdep_acquire+0x68/0x84 [] _spin_lock_irqsave+0x2a/0x3a [] enable_8259A_irq+0x10/0x47 [] startup_8259A_irq+0x8/0xc [] setup_irq+0xe4/0x14d [] time_init_hook+0xf/0x11 [] time_init+0x44/0x46 [] start_kernel+0x191/0x38f [] 0xc0100210 -> (ioapic_lock){+...} ops: 122 { initial-use at: [] lockdep_acquire+0x68/0x84 [] _spin_lock_irqsave+0x2a/0x3a [] io_apic_get_version+0x16/0x55 [] mp_register_ioapic+0xc6/0x127 [] acpi_parse_ioapic+0x2d/0x39 [] acpi_table_parse_madt_family+0xb4/0x100 [] acpi_table_parse_madt+0x16/0x18 [] acpi_boot_init+0x132/0x251 [] setup_arch+0xd36/0xe37 [] start_kernel+0x66/0x38f [] 0xc0100210 in-hardirq-W at: [] lockdep_acquire+0x68/0x84 [] _spin_lock_irqsave+0x2a/0x3a [] mask_IO_APIC_irq+0x11/0x31 [] ack_edge_ioapic_vector+0x31/0x41 [] __do_IRQ+0x4f/0x113 [] do_IRQ+0x8c/0xad } ... key at: [] ioapic_lock+0x14/0x3c -> (i8259A_lock){++..} ops: 5149 { initial-use at: [] lockdep_acquire+0x68/0x84 [] _spin_lock_irqsave+0x2a/0x3a [] init_8259A+0x11/0x8f [] init_ISA_irqs+0x12/0x4d [] pre_intr_init_hook+0x8/0xa [] init_IRQ+0xe/0x65 [] start_kernel+0x178/0x38f [] 0xc0100210 in-hardirq-W at: [] lockdep_acquire+0x68/0x84 [] _spin_lock_irqsave+0x2a/0x3a [] mask_and_ack_8259A+0x1b/0xcc [] __do_IRQ+0x4f/0x113 [] do_IRQ+0x8c/0xad in-softirq-W at: [] lockdep_acquire+0x68/0x84 [] _spin_lock_irqsave+0x2a/0x3a [] mask_and_ack_8259A+0x1b/0xcc [] __do_IRQ+0x4f/0x113 [] do_IRQ+0x8c/0xad } ... key at: [] i8259A_lock+0x14/0x40 ... acquired at: [] lockdep_acquire+0x68/0x84 [] _spin_lock_irqsave+0x2a/0x3a [] disable_8259A_irq+0x10/0x47 [] startup_edge_ioapic_vector+0x31/0x58 [] setup_irq+0xe4/0x14d [] request_irq+0xda/0xf9 [] rtc_init+0x6a/0x1a7 [] init+0x14a/0x2cb [] kernel_thread_helper+0x5/0xb ... acquired at: [] lockdep_acquire+0x68/0x84 [] _spin_lock_irqsave+0x2a/0x3a [] mask_IO_APIC_irq+0x11/0x31 [] ack_edge_ioapic_vector+0x31/0x41 [] __do_IRQ+0x4f/0x113 [] do_IRQ+0x8c/0xad the hard-irq-unsafe lock's dependencies: -> (vector_lock){--..} ops: 31 { initial-use at: [] lockdep_acquire+0x68/0x84 [] _spin_lock+0x21/0x2f [] assign_irq_vector+0x34/0xc8 [] setup_IO_APIC+0x45a/0xcff [] smp_prepare_cpus+0x5ea/0x8aa [] init+0x32/0x2cb [] kernel_thread_helper+0x5/0xb softirq-on-W at: [] lockdep_acquire+0x68/0x84 [] _spin_lock+0x21/0x2f [] assign_irq_vector+0x34/0xc8 [] setup_IO_APIC+0x45a/0xcff [] smp_prepare_cpus+0x5ea/0x8aa [] init+0x32/0x2cb [] kernel_thread_helper+0x5/0xb hardirq-on-W at: [] lockdep_acquire+0x68/0x84 [] _spin_lock+0x21/0x2f [] assign_irq_vector+0x34/0xc8 [] setup_IO_APIC+0x45a/0xcff [] smp_prepare_cpus+0x5ea/0x8aa [] init+0x32/0x2cb [] kernel_thread_helper+0x5/0xb } ... key at: [] vector_lock+0x14/0x3c stack backtrace: [] show_trace+0xd/0xf [] dump_stack+0x17/0x19 [] check_usage+0x1f6/0x203 [] __lockdep_acquire+0x8c2/0xaa5 [] lockdep_acquire+0x68/0x84 [] _spin_lock_irqsave+0x2a/0x3a [] startup_msi_irq_wo_maskbit+0x10/0x35 [] probe_irq_on+0x44/0x107 [] serial8250_config_port+0x84b/0x986 [] uart_add_one_port+0x105/0x289 [] serial8250_init+0xc3/0x10a [] init+0x14a/0x2cb [] kernel_thread_helper+0x5/0xb Signed-off-by: Ingo Molnar Cc: Jan Beulich Cc: Andi Kleen Signed-off-by: Andrew Morton Signed-off-by: Andi Kleen Signed-off-by: Linus Torvalds --- diff --git a/arch/i386/kernel/io_apic.c b/arch/i386/kernel/io_apic.c index 61317f4be4..72ae414e4d 100644 --- a/arch/i386/kernel/io_apic.c +++ b/arch/i386/kernel/io_apic.c @@ -1163,14 +1163,15 @@ u8 irq_vector[NR_IRQ_VECTORS] __read_mostly = { FIRST_DEVICE_VECTOR , 0 }; int assign_irq_vector(int irq) { static int current_vector = FIRST_DEVICE_VECTOR, offset = 0; + unsigned long flags; int vector; BUG_ON(irq != AUTO_ASSIGN && (unsigned)irq >= NR_IRQ_VECTORS); - spin_lock(&vector_lock); + spin_lock_irqsave(&vector_lock, flags); if (irq != AUTO_ASSIGN && IO_APIC_VECTOR(irq) > 0) { - spin_unlock(&vector_lock); + spin_unlock_irqrestore(&vector_lock, flags); return IO_APIC_VECTOR(irq); } next: @@ -1181,7 +1182,7 @@ next: if (current_vector >= FIRST_SYSTEM_VECTOR) { offset++; if (!(offset%8)) { - spin_unlock(&vector_lock); + spin_unlock_irqrestore(&vector_lock, flags); return -ENOSPC; } current_vector = FIRST_DEVICE_VECTOR + offset; @@ -1192,7 +1193,7 @@ next: if (irq != AUTO_ASSIGN) IO_APIC_VECTOR(irq) = vector; - spin_unlock(&vector_lock); + spin_unlock_irqrestore(&vector_lock, flags); return vector; } diff --git a/arch/x86_64/kernel/io_apic.c b/arch/x86_64/kernel/io_apic.c index 38a3ff30bd..519cd4e6f9 100644 --- a/arch/x86_64/kernel/io_apic.c +++ b/arch/x86_64/kernel/io_apic.c @@ -836,14 +836,15 @@ u8 irq_vector[NR_IRQ_VECTORS] __read_mostly = { FIRST_DEVICE_VECTOR , 0 }; int assign_irq_vector(int irq) { static int current_vector = FIRST_DEVICE_VECTOR, offset = 0; + unsigned long flags; int vector; BUG_ON(irq != AUTO_ASSIGN && (unsigned)irq >= NR_IRQ_VECTORS); - spin_lock(&vector_lock); + spin_lock_irqsave(&vector_lock, flags); if (irq != AUTO_ASSIGN && IO_APIC_VECTOR(irq) > 0) { - spin_unlock(&vector_lock); + spin_unlock_irqrestore(&vector_lock, flags); return IO_APIC_VECTOR(irq); } next: @@ -862,7 +863,7 @@ next: if (irq != AUTO_ASSIGN) IO_APIC_VECTOR(irq) = vector; - spin_unlock(&vector_lock); + spin_unlock_irqrestore(&vector_lock, flags); return vector; }