[PATCH] forcedeth: make msi-x different name for rx-tx

[PATCH] forcedeth: make msi-x different name for rx-tx

[Yinghai Lu]

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 <yinghai@kernel.org>

---
 drivers/net/forcedeth.c |   17 ++++++++++++++---
 1 file changed, 14 insertions(+), 3 deletions(-)

Index: linux-2.6/drivers/net/forcedeth.c
===================================================================
--- linux-2.6.orig/drivers/net/forcedeth.c
+++ 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;

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

[Yinghai Lu]

Impact: fix bug

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

Signed-off-by: Yinghai Lu <yinghai@kernel.org>

---
 drivers/net/forcedeth.c |    8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

Index: linux-2.6/drivers/net/forcedeth.c
===================================================================
--- linux-2.6.orig/drivers/net/forcedeth.c
+++ 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

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

[David Miller]

From: Yinghai Lu <yinghai@kernel.org>
Date: Thu, 05 Feb 2009 21:00:30 -0800

> 
> Impact: fix bug
> 
> for msix, we still need that flag to enable irq respectively
> 
> Signed-off-by: Yinghai Lu <yinghai@kernel.org>

Applied to net-next-2.6

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

[Yinghai Lu]

Impact: clean up

schedule it later after disable it.

Signed-off-by: Yinghai Lu <yinghai@kernel.org>

---
 drivers/net/forcedeth.c |    4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

Index: linux-2.6/drivers/net/forcedeth.c
===================================================================
--- linux-2.6.orig/drivers/net/forcedeth.c
+++ 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;
 }

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

[David Miller]

From: Yinghai Lu <yinghai@kernel.org>
Date: Thu, 05 Feb 2009 21:01:06 -0800

> 
> Impact: clean up
> 
> schedule it later after disable it.
> 
> Signed-off-by: Yinghai Lu <yinghai@kernel.org>

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.

[PATCH] forcedeth: ck804 and mcp55 doesn't need timerirq

[Yinghai Lu]

Impact: cleanup

so get less irq.

Signed-off-by: Yinghai Lu <yinghai@kernel.org>

---
 drivers/net/forcedeth.c |    8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

Index: linux-2.6/drivers/net/forcedeth.c
===================================================================
--- linux-2.6.orig/drivers/net/forcedeth.c
+++ 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),

Re: [PATCH] forcedeth: ck804 and mcp55 doesn't need timerirq

[David Miller]

From: Yinghai Lu <yinghai@kernel.org>
Date: Thu, 05 Feb 2009 21:01:49 -0800

> 
> Impact: cleanup
> 
> so get less irq.
> 
> Signed-off-by: Yinghai Lu <yinghai@kernel.org>

Applied to net-next-2.6

[PATCH] forcedeth: enable msix to default

[Yinghai Lu]

Impact: change default

msix and napic can work again

Signed-off-by: Yinghai Lu <yinghai@kernel.org>
---
 drivers/net/forcedeth.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Index: linux-2.6/drivers/net/forcedeth.c
===================================================================
--- linux-2.6.orig/drivers/net/forcedeth.c
+++ 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

Re: [PATCH] forcedeth: enable msix to default

[David Miller]

From: Yinghai Lu <yinghai@kernel.org>
Date: Thu, 05 Feb 2009 21:02:37 -0800

> 
> Impact: change default
> 
> msix and napic can work again
> 
> Signed-off-by: Yinghai Lu <yinghai@kernel.org>

Applied to net-next-2.6

Re: [PATCH] forcedeth: make msi-x different name for rx-tx

[David Miller]

From: Yinghai Lu <yinghai@kernel.org>
Date: Thu, 05 Feb 2009 20:59:20 -0800

> 
> 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 <yinghai@kernel.org>

Applied to net-next-2.6

Re: [PATCH] forcedeth: make msi-x different name for rx-tx

[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.)

	Ingo

Re: [PATCH] forcedeth: make msi-x different name for rx-tx

[Yinghai Lu]

Ingo Molnar wrote:
> 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

Re: [PATCH] forcedeth: make msi-x different name for rx-tx

[Ingo Molnar]

* Yinghai Lu <yinghai@kernel.org> wrote:

> Ingo Molnar wrote:
> > 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.

i do have that:

 CONFIG_FORCEDETH=y
 CONFIG_FORCEDETH_NAPI=y

and the messages still come.

	Ingo

Re: [PATCH] forcedeth: make msi-x different name for rx-tx

[Yinghai Lu]

Ingo Molnar wrote:
> * Yinghai Lu <yinghai@kernel.org> wrote:
> 
>> Ingo Molnar wrote:
>>> 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.
> 
> 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

Re: [PATCH] forcedeth: make msi-x different name for rx-tx

[Ingo Molnar]

* Yinghai Lu <yinghai@kernel.org> wrote:

> Ingo Molnar wrote:
> > * Yinghai Lu <yinghai@kernel.org> wrote:
> > 
> >> Ingo Molnar wrote:
> >>> 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.
> > 
> > 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).

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

Re: [PATCH] forcedeth: make msi-x different name for rx-tx

[Yinghai Lu]

Ingo Molnar wrote:
> * Yinghai Lu <yinghai@kernel.org> wrote:
> 
>> Ingo Molnar wrote:
>>> * Yinghai Lu <yinghai@kernel.org> wrote:
>>>
>>>> Ingo Molnar wrote:
>>>>> 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.
>>> 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).
> 
> 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