RE: [PATCH] set_rte() should get iosapic_lock

From: Liu, Benjamin <benjamin.liu_at_intel.com>
Date: 2004-04-13 19:07:00
Hi, Kenji.

Lock is used to avoid racing during resource contention. It seems that your patch is just for 2.6.5 which incorporates Bjorn's patch for ACPI IRQ cleanup.

Here iosapic_lock is used in
1. mask_irq() to protect I/O SAPIC config register write(REG SELECT, and RTE's lower word), iosapic_intr_info read(iosapic_intr_info[vec].low32).
2. unmask_irq() to protect I/O SAPIC config register write(REG SELECT, and RTE's lower word), iosapic_intr_info read(iosapic_intr_info[vec].low32).
3. iosapic_set_affinity() to protect I/O SAPIC config register write(REG SELECT, and RTE's lower/higher words), iosapic_intr_info write(iosapic_intr_info[vec].low32).

And in set_rte(), your patch intends to protect I/O SAPIC config register write(REG SELECT, and RTE's lower/higher words), iosapic_intr_info write(iosapic_intr_info[vec].low32). Bjorn's patch has defered RTE setting until device driver initialization. So device module loading and "echo 0xff >/proc/irq/xxx/smp_affinity" will cause racing, if without the lock in set_rte().

My question is whether it need to be applied to other kernel versions(before ACPI IRQ cleanup patch)?

Thanks,
Pingping (Benjamin) Liu
 

>-----Original Message-----
>From: linux-ia64-owner@vger.kernel.org 
>[mailto:linux-ia64-owner@vger.kernel.org] On Behalf Of Kenji Kaneshige
>Sent: 2004412 20:37
>To: linux-ia64@vger.kernel.org
>Subject: [PATCH] set_rte() should get iosapic_lock
>
>Hi,
>
>Currently set_rte() changes RTE without iosapic_lock held. I guess it
>assumes to be called only at the boot time. But set_rte() can be
>called by PCI driver not only at the boot time. So I think set_rte()
>should get iosapic_lock.
>
>pci_enable_device(drivers/pci/pci.c)
>|
>+-> pci_enable_device_bars(drivers/pci/pci.c)
>    |
>    +-> pcibios_enable_device(arch/ia64/pci/pci.c)
>        |
>        +-> acpi_pci_irq_enable (drivers/acpi/pci_irq.c)
>            |
>            +-> iosapic_enable_intr (arch/ia64/kernel/iosapic.c)
>                |
>                +-> set_rte (arch_ia64/kernel/iosapic.c)
>
>A following patch fixes this issue.  I'm also attaching this patch
>because my mailer replace all tabs with blanks.
>
>Thanks,
>Kenji Kaneshige
>
>diff -Naur linux-2.6.5/arch/ia64/kernel/iosapic.c
>linux-2.6.5-changed/arch/ia64/kernel/iosapic.c
>--- linux-2.6.5/arch/ia64/kernel/iosapic.c	2004-04-04 
>12:37:06.000000000
>+0900
>+++ linux-2.6.5-changed/arch/ia64/kernel/iosapic.c	2004-04-12
>21:08:48.491220447 +0900
>@@ -172,7 +172,7 @@
> static void
> set_rte (unsigned int vector, unsigned int dest, int mask)
> {
>-	unsigned long pol, trigger, dmode;
>+	unsigned long pol, trigger, dmode, flags;
> 	u32 low32, high32;
> 	char *addr;
> 	int rte_index;
>@@ -211,11 +211,15 @@
> 	/* dest contains both id and eid */
> 	high32 = (dest << IOSAPIC_DEST_SHIFT);
>
>-	writel(IOSAPIC_RTE_HIGH(rte_index), addr + IOSAPIC_REG_SELECT);
>-	writel(high32, addr + IOSAPIC_WINDOW);
>-	writel(IOSAPIC_RTE_LOW(rte_index), addr + IOSAPIC_REG_SELECT);
>-	writel(low32, addr + IOSAPIC_WINDOW);
>-	iosapic_intr_info[vector].low32 = low32;
>+	spin_lock_irqsave(&iosapic_lock, flags);
>+	{
>+		writel(IOSAPIC_RTE_HIGH(rte_index), addr + 
>IOSAPIC_REG_SELECT);
>+		writel(high32, addr + IOSAPIC_WINDOW);
>+		writel(IOSAPIC_RTE_LOW(rte_index), addr + 
>IOSAPIC_REG_SELECT);
>+		writel(low32, addr + IOSAPIC_WINDOW);
>+		iosapic_intr_info[vector].low32 = low32;
>+	}
>+	spin_unlock_irqrestore(&iosapic_lock, flags);
> }
>
> static void
>
-
To unsubscribe from this list: send the line "unsubscribe linux-ia64" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Received on Tue Apr 13 05:12:13 2004

This archive was generated by hypermail 2.1.8 : 2005-08-02 09:20:25 EST