Skip to contentSkip to conversation

forcedeth: make msi-x different name for rx-tx

PATCH Applied Yinghai Lu 3 participants 16 messages Feb 6, 2009 – Feb 9, 2009

Subsystems (2)
Networking Network drivers
lore
Share & tools
lore

Yinghai Lu

author
#0 lore raw

PATCH 1/1 forcedeth: make msi-x different name for rx-tx

Impact: make /proc/interrupts could show more info which irq is rx or other for msi-x

add three name fields for rx, tx, other

Signed-off-by Yinghai Lu <[email protected]>
---
 drivers/net/forcedeth.c |   17 ++++++++++++++---
 1 file changed, 14 insertions(+), 3 deletions(-)
drivers/net/forcedeth.c +14 -3
Index: linux-2.6/drivers/net/forcedeth.c
===================================================================
@@ -812,6 +812,11 @@ struct fe_priv {
 
 	/* power saved state */
 	u32 saved_config_space[NV_PCI_REGSZ_MAX/4];
+
+	/* for different msi-x irq type */
+	char name_rx[IFNAMSIZ + 3];       /* -rx    */
+	char name_tx[IFNAMSIZ + 3];       /* -tx    */
+	char name_other[IFNAMSIZ + 6];    /* -other */
 };
 
 /*

@@ -3918,21 +3923,27 @@ static int nv_request_irq(struct net_dev
 			np->msi_flags |= NV_MSI_X_ENABLED;
 			if (optimization_mode == NV_OPTIMIZATION_MODE_THROUGHPUT && !intr_test) {
 				/* Request irq for rx handling */
-				if (request_irq(np->msi_x_entry[NV_MSI_X_VECTOR_RX].vector, &nv_nic_irq_rx, IRQF_SHARED, dev->name, dev) != 0) {
+				sprintf(np->name_rx, "%s-rx", dev->name);
+				if (request_irq(np->msi_x_entry[NV_MSI_X_VECTOR_RX].vector,
+						&nv_nic_irq_rx, IRQF_SHARED, np->name_rx, dev) != 0) {
 					printk(KERN_INFO "forcedeth: request_irq failed for rx %d\n", ret);
 					pci_disable_msix(np->pci_dev);
 					np->msi_flags &= ~NV_MSI_X_ENABLED;
 					goto out_err;
 				}
 				/* Request irq for tx handling */
-				if (request_irq(np->msi_x_entry[NV_MSI_X_VECTOR_TX].vector, &nv_nic_irq_tx, IRQF_SHARED, dev->name, dev) != 0) {
+				sprintf(np->name_tx, "%s-tx", dev->name);
+				if (request_irq(np->msi_x_entry[NV_MSI_X_VECTOR_TX].vector,
+						&nv_nic_irq_tx, IRQF_SHARED, np->name_tx, dev) != 0) {
 					printk(KERN_INFO "forcedeth: request_irq failed for tx %d\n", ret);
 					pci_disable_msix(np->pci_dev);
 					np->msi_flags &= ~NV_MSI_X_ENABLED;
 					goto out_free_rx;
 				}
 				/* Request irq for link and timer handling */
-				if (request_irq(np->msi_x_entry[NV_MSI_X_VECTOR_OTHER].vector, &nv_nic_irq_other, IRQF_SHARED, dev->name, dev) != 0) {
+				sprintf(np->name_other, "%s-other", dev->name);
+				if (request_irq(np->msi_x_entry[NV_MSI_X_VECTOR_OTHER].vector,
+						&nv_nic_irq_other, IRQF_SHARED, np->name_other, dev) != 0) {
 					printk(KERN_INFO "forcedeth: request_irq failed for link %d\n", ret);
 					pci_disable_msix(np->pci_dev);
 					np->msi_flags &= ~NV_MSI_X_ENABLED;
author
#1 lore raw

PATCH 1/1 forcedeth: ck804 and mcp55 doesn't need timerirq

Impact: cleanup

so get less irq.

Signed-off-by Yinghai Lu <[email protected]>
---
 drivers/net/forcedeth.c |    8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)
drivers/net/forcedeth.c +4 -4
Index: linux-2.6/drivers/net/forcedeth.c
===================================================================
@@ -6070,11 +6070,11 @@ static struct pci_device_id pci_tbl[] =
 	},
 	{	/* CK804 Ethernet Controller */
 		PCI_DEVICE(PCI_VENDOR_ID_NVIDIA, PCI_DEVICE_ID_NVIDIA_NVENET_8),
-		.driver_data = DEV_NEED_TIMERIRQ|DEV_NEED_LINKTIMER|DEV_HAS_LARGEDESC|DEV_HAS_CHECKSUM|DEV_HAS_HIGH_DMA|DEV_HAS_STATISTICS_V1|DEV_NEED_TX_LIMIT,
+		.driver_data = DEV_NEED_LINKTIMER|DEV_HAS_LARGEDESC|DEV_HAS_CHECKSUM|DEV_HAS_HIGH_DMA|DEV_HAS_STATISTICS_V1|DEV_NEED_TX_LIMIT,
 	},
 	{	/* CK804 Ethernet Controller */
 		PCI_DEVICE(PCI_VENDOR_ID_NVIDIA, PCI_DEVICE_ID_NVIDIA_NVENET_9),
-		.driver_data = DEV_NEED_TIMERIRQ|DEV_NEED_LINKTIMER|DEV_HAS_LARGEDESC|DEV_HAS_CHECKSUM|DEV_HAS_HIGH_DMA|DEV_HAS_STATISTICS_V1|DEV_NEED_TX_LIMIT,
+		.driver_data = DEV_NEED_LINKTIMER|DEV_HAS_LARGEDESC|DEV_HAS_CHECKSUM|DEV_HAS_HIGH_DMA|DEV_HAS_STATISTICS_V1|DEV_NEED_TX_LIMIT,
 	},
 	{	/* MCP04 Ethernet Controller */
 		PCI_DEVICE(PCI_VENDOR_ID_NVIDIA, PCI_DEVICE_ID_NVIDIA_NVENET_10),

@@ -6094,11 +6094,11 @@ static struct pci_device_id pci_tbl[] =
 	},
 	{	/* MCP55 Ethernet Controller */
 		PCI_DEVICE(PCI_VENDOR_ID_NVIDIA, PCI_DEVICE_ID_NVIDIA_NVENET_14),
-		.driver_data = DEV_NEED_TIMERIRQ|DEV_NEED_LINKTIMER|DEV_HAS_LARGEDESC|DEV_HAS_CHECKSUM|DEV_HAS_HIGH_DMA|DEV_HAS_VLAN|DEV_HAS_MSI|DEV_HAS_MSI_X|DEV_HAS_POWER_CNTRL|DEV_HAS_PAUSEFRAME_TX_V1|DEV_HAS_STATISTICS_V2|DEV_HAS_TEST_EXTENDED|DEV_HAS_MGMT_UNIT|DEV_NEED_TX_LIMIT,
+		.driver_data = DEV_NEED_LINKTIMER|DEV_HAS_LARGEDESC|DEV_HAS_CHECKSUM|DEV_HAS_HIGH_DMA|DEV_HAS_VLAN|DEV_HAS_MSI|DEV_HAS_MSI_X|DEV_HAS_POWER_CNTRL|DEV_HAS_PAUSEFRAME_TX_V1|DEV_HAS_STATISTICS_V2|DEV_HAS_TEST_EXTENDED|DEV_HAS_MGMT_UNIT|DEV_NEED_TX_LIMIT,
 	},
 	{	/* MCP55 Ethernet Controller */
 		PCI_DEVICE(PCI_VENDOR_ID_NVIDIA, PCI_DEVICE_ID_NVIDIA_NVENET_15),
-		.driver_data = DEV_NEED_TIMERIRQ|DEV_NEED_LINKTIMER|DEV_HAS_LARGEDESC|DEV_HAS_CHECKSUM|DEV_HAS_HIGH_DMA|DEV_HAS_VLAN|DEV_HAS_MSI|DEV_HAS_MSI_X|DEV_HAS_POWER_CNTRL|DEV_HAS_PAUSEFRAME_TX_V1|DEV_HAS_STATISTICS_V2|DEV_HAS_TEST_EXTENDED|DEV_HAS_MGMT_UNIT|DEV_NEED_TX_LIMIT,
+		.driver_data = DEV_NEED_LINKTIMER|DEV_HAS_LARGEDESC|DEV_HAS_CHECKSUM|DEV_HAS_HIGH_DMA|DEV_HAS_VLAN|DEV_HAS_MSI|DEV_HAS_MSI_X|DEV_HAS_POWER_CNTRL|DEV_HAS_PAUSEFRAME_TX_V1|DEV_HAS_STATISTICS_V2|DEV_HAS_TEST_EXTENDED|DEV_HAS_MGMT_UNIT|DEV_NEED_TX_LIMIT,
 	},
 	{	/* MCP61 Ethernet Controller */
 		PCI_DEVICE(PCI_VENDOR_ID_NVIDIA, PCI_DEVICE_ID_NVIDIA_NVENET_16),
author
#3 lore raw

PATCH 1/1 forcedeth: don't clear nic_poll_irq too early

In reply to Yinghai Lu

Impact: fix bug

for msix, we still need that flag to enable irq respectively

Signed-off-by Yinghai Lu <[email protected]>
---
 drivers/net/forcedeth.c |    8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)
drivers/net/forcedeth.c +5 -3
Index: linux-2.6/drivers/net/forcedeth.c
===================================================================
@@ -4057,8 +4057,6 @@ static void nv_do_nic_poll(unsigned long
 			mask |= NVREG_IRQ_OTHER;
 		}
 	}
-	np->nic_poll_irq = 0;
-
 	/* disable_irq() contains synchronize_irq, thus no irq handler can run now */
 
 	if (np->recover_error) {

@@ -4096,11 +4094,11 @@ static void nv_do_nic_poll(unsigned long
 		}
 	}
 
-
 	writel(mask, base + NvRegIrqMask);
 	pci_push(base);
 
 	if (!using_multi_irqs(dev)) {
+		np->nic_poll_irq = 0;
 		if (nv_optimized(np))
 			nv_nic_irq_optimized(0, dev);
 		else

@@ -4111,18 +4109,22 @@ static void nv_do_nic_poll(unsigned long
 			enable_irq_lockdep(np->pci_dev->irq);
 	} else {
 		if (np->nic_poll_irq & NVREG_IRQ_RX_ALL) {
+			np->nic_poll_irq &= ~NVREG_IRQ_RX_ALL;
 			nv_nic_irq_rx(0, dev);
 			enable_irq_lockdep(np->msi_x_entry[NV_MSI_X_VECTOR_RX].vector);
 		}
 		if (np->nic_poll_irq & NVREG_IRQ_TX_ALL) {
+			np->nic_poll_irq &= ~NVREG_IRQ_TX_ALL;
 			nv_nic_irq_tx(0, dev);
 			enable_irq_lockdep(np->msi_x_entry[NV_MSI_X_VECTOR_TX].vector);
 		}
 		if (np->nic_poll_irq & NVREG_IRQ_OTHER) {
+			np->nic_poll_irq &= ~NVREG_IRQ_OTHER;
 			nv_nic_irq_other(0, dev);
 			enable_irq_lockdep(np->msi_x_entry[NV_MSI_X_VECTOR_OTHER].vector);
 		}
 	}
+
 }
 
 #ifdef CONFIG_NET_POLL_CONTROLLER
maintainer Applied
#4 lore raw

Re: [PATCH] forcedeth: don't clear nic_poll_irq too early

From: Yinghai Lu <[email protected]>
Date: Thu, 05 Feb 2009 21:00:30 -0800

Quoted 6 lines

Impact: fix bug

for msix, we still need that flag to enable irq respectively

Signed-off-by Yinghai Lu <[email protected]>

Applied to net-next-2.6

author
#5 lore raw

PATCH 1/1 forcedeth: disable irq at first before schedule rx

In reply to Yinghai Lu

Impact: clean up

schedule it later after disable it.

Signed-off-by Yinghai Lu <[email protected]>
---
 drivers/net/forcedeth.c |    4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)
drivers/net/forcedeth.c +2 -2
Index: linux-2.6/drivers/net/forcedeth.c
===================================================================
@@ -3708,13 +3708,13 @@ static irqreturn_t nv_nic_irq_rx(int foo
 	u32 events;
 
 	events = readl(base + NvRegMSIXIrqStatus) & NVREG_IRQ_RX_ALL;
-	writel(NVREG_IRQ_RX_ALL, base + NvRegMSIXIrqStatus);
 
 	if (events) {
-		netif_rx_schedule(&np->napi);
 		/* disable receive interrupts on the nic */
 		writel(NVREG_IRQ_RX_ALL, base + NvRegIrqMask);
 		pci_push(base);
+		writel(NVREG_IRQ_RX_ALL, base + NvRegMSIXIrqStatus);
+		netif_rx_schedule(&np->napi);
 	}
 	return IRQ_HANDLED;
 }
maintainer Applied
#6 lore raw

Re: [PATCH] forcedeth: disable irq at first before schedule rx

From: Yinghai Lu <[email protected]>
Date: Thu, 05 Feb 2009 21:01:06 -0800

Quoted 6 lines

Impact: clean up

schedule it later after disable it.

Signed-off-by Yinghai Lu <[email protected]>

Applied to net-next-2.6

I had to edit this patch to make it apply properly. In net-next-2.6 the netix_rx_*() have been renamed to napi_*() and the former interfaces deleted.

Please generate your patches properly against net-next-2.6 in the future.

Thanks.

author
#7 lore raw

PATCH 1/1 forcedeth: enable msix to default

In reply to Yinghai Lu

Impact: change default

msix and napic can work again

Signed-off-by Yinghai Lu <[email protected]>
---
 drivers/net/forcedeth.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/net/forcedeth.c +1 -1
Index: linux-2.6/drivers/net/forcedeth.c
===================================================================
@@ -862,7 +862,7 @@ enum {
 	NV_MSIX_INT_DISABLED,
 	NV_MSIX_INT_ENABLED
 };
-static int msix = NV_MSIX_INT_DISABLED;
+static int msix = NV_MSIX_INT_ENABLED;
 
 /*
  * DMA 64bit
#10 lore raw
In reply to Yinghai Lu

Cool cleanups!

Would you mind to also fix the following - on all my systems that have forcedeth gigabit ethernet i've been getting these bogus warnings for _years_, under moderate load:

eth2: too many iterations (16) in nv_nic_irq.

I think a 64 iterations limit will work much better. (i tried a limit of 50 a year ago for a while and it worked fine and had no side effects - and the bogus warnings were done.)

Ingo

author
#11 lore raw
Quoted 11 lines · Ingo Molnar

Cool cleanups!

Would you mind to also fix the following - on all my systems that have forcedeth gigabit ethernet i've been getting these bogus warnings for _years_, under moderate load:

eth2: too many iterations (16) in nv_nic_irq.

I think a 64 iterations limit will work much better. (i tried a limit of 50 a year ago for a while and it worked fine and had no side effects - and the bogus warnings were done.)

you may enable CONFIG_FORCEDETH_NAPI.

we should enable NAPI for forcedeth by default, and try to squash more bugs out.

YH

#12 lore raw
Quoted 9 lines · Yinghai Lu
Quoted 8 lines · Ingo Molnar

Cool cleanups!

Would you mind to also fix the following - on all my systems that have forcedeth gigabit ethernet i've been getting these bogus warnings for _years_, under moderate load:

eth2: too many iterations (16) in nv_nic_irq.

Quoted 7 lines
Quoted 3 lines

I think a 64 iterations limit will work much better. (i tried a limit of 50 a year ago for a while and it worked fine and had no side effects - and the bogus warnings were done.)

you may enable CONFIG_FORCEDETH_NAPI.

we should enable NAPI for forcedeth by default, and try to squash more bugs out.

i do have that:

CONFIG_FORCEDETH=y
CONFIG_FORCEDETH_NAPI=y

and the messages still come.

Ingo

author
#13 lore raw
Quoted 16 lines · Ingo Molnar
Quoted 14 lines · Yinghai Lu
Quoted 11 lines · Ingo Molnar

Cool cleanups!

Would you mind to also fix the following - on all my systems that have forcedeth gigabit ethernet i've been getting these bogus warnings for _years_, under moderate load:

eth2: too many iterations (16) in nv_nic_irq.

I think a 64 iterations limit will work much better. (i tried a limit of 50 a year ago for a while and it worked fine and had no side effects - and the bogus warnings were done.)

you may enable CONFIG_FORCEDETH_NAPI.

Quoted 8 lines
Quoted 1 line

we should enable NAPI for forcedeth by default, and try to squash more bugs out.

i do have that:

CONFIG_FORCEDETH=y
CONFIG_FORCEDETH_NAPI=y

and the messages still come.

so you systems are using ioapic routing, or msi.

and those messages are from
nv_nic_irq_optimized or nv_nic_irq
for TX or OTHER (timerirq).

YH

#14 lore raw
Quoted 24 lines · Yinghai Lu
Quoted 23 lines · Ingo Molnar
Quoted 7 lines · Yinghai Lu
Quoted 6 lines · Ingo Molnar

Cool cleanups!

Would you mind to also fix the following - on all my systems that have forcedeth gigabit ethernet i've been getting these bogus warnings for _years_, under moderate load:

Quoted 8 lines
Quoted 5 lines

eth2: too many iterations (16) in nv_nic_irq.

I think a 64 iterations limit will work much better. (i tried a limit of 50 a year ago for a while and it worked fine and had no side effects - and the bogus warnings were done.)

you may enable CONFIG_FORCEDETH_NAPI.

we should enable NAPI for forcedeth by default, and try to squash more bugs out.

i do have that:

CONFIG_FORCEDETH=y
CONFIG_FORCEDETH_NAPI=y

Quoted 7 lines
Quoted 1 line

and the messages still come.

so you systems are using ioapic routing, or msi.

and those messages are from
nv_nic_irq_optimized or nv_nic_irq
for TX or OTHER (timerirq).

yes, IO-APIC:

18: 81386 306713817 0 0 1413724087 0 0
0 0 0 0 0 0 0 0
0 IO-APIC-fasteoi eth2

Those messages do come and are annoying.

Ingo

author
#15 lore raw
Quoted 32 lines · Ingo Molnar
Quoted 23 lines · Yinghai Lu
Quoted 22 lines · Ingo Molnar
Quoted 7 lines · Yinghai Lu
Quoted 6 lines · Ingo Molnar

Cool cleanups!

Would you mind to also fix the following - on all my systems that have forcedeth gigabit ethernet i've been getting these bogus warnings for _years_, under moderate load:

Quoted 8 lines
Quoted 5 lines

eth2: too many iterations (16) in nv_nic_irq.

I think a 64 iterations limit will work much better. (i tried a limit of 50 a year ago for a while and it worked fine and had no side effects - and the bogus warnings were done.)

you may enable CONFIG_FORCEDETH_NAPI.

we should enable NAPI for forcedeth by default, and try to squash more bugs out.

i do have that:

CONFIG_FORCEDETH=y
CONFIG_FORCEDETH_NAPI=y

Quoted 6 lines
Quoted 1 line

and the messages still come.

so you systems are using ioapic routing, or msi.

and those messages are from
nv_nic_irq_optimized or nv_nic_irq
for TX or OTHER (timerirq).

Quoted 7 lines

yes, IO-APIC:

18: 81386 306713817 0 0 1413724087 0 0
0 0 0 0 0 0 0 0
0 IO-APIC-fasteoi eth2

Those messages do come and are annoying.

it seems timerirq by nic cause extra irq...
need to figure out why all devices need that workaround..., checked ck804 and mcp55 doesn't need it.

YH