[PATCH] forcedeth: seperate handler for msix and normal int.

[PATCH] forcedeth: seperate handler for msix and normal int.

[Yinghai Lu]

[PATCH] forcedeth: seperate handler for msix and normal int.

so we don't need to keep checking np->msi_flags to see if NV_MSI_X_ENABLED is set in
handler

Signed-off-by: Yinghai Lu <yinghai.lu@sun.com>

Index: linux-2.6/drivers/net/forcedeth.c
===================================================================
--- linux-2.6.orig/drivers/net/forcedeth.c
+++ linux-2.6/drivers/net/forcedeth.c
@@ -2979,7 +2979,7 @@ static void nv_link_irq(struct net_devic
 	dprintk(KERN_DEBUG "%s: link change notification done.\n", dev->name);
 }
 
-static irqreturn_t nv_nic_irq(int foo, void *data)
+static inline irqreturn_t nv_nic_irq_x(int foo, void *data, int msix)
 {
 	struct net_device *dev = (struct net_device *) data;
 	struct fe_priv *np = netdev_priv(dev);
@@ -2990,7 +2990,7 @@ static irqreturn_t nv_nic_irq(int foo, v
 	dprintk(KERN_DEBUG "%s: nv_nic_irq\n", dev->name);
 
 	for (i=0; ; i++) {
-		if (!(np->msi_flags & NV_MSI_X_ENABLED)) {
+		if (!msix) {
 			events = readl(base + NvRegIrqStatus) & NVREG_IRQSTAT_MASK;
 			writel(NVREG_IRQSTAT_MASK, base + NvRegIrqStatus);
 		} else {
@@ -3013,7 +3013,7 @@ static irqreturn_t nv_nic_irq(int foo, v
 			spin_lock(&np->lock);
 			np->irqmask &= ~NVREG_IRQ_RX_ALL;
 
-			if (np->msi_flags & NV_MSI_X_ENABLED)
+			if (msix)
 				writel(NVREG_IRQ_RX_ALL, base + NvRegIrqMask);
 			else
 				writel(np->irqmask, base + NvRegIrqMask);
@@ -3051,7 +3051,7 @@ static irqreturn_t nv_nic_irq(int foo, v
 		if (unlikely(events & NVREG_IRQ_RECOVER_ERROR)) {
 			spin_lock(&np->lock);
 			/* disable interrupts on the nic */
-			if (!(np->msi_flags & NV_MSI_X_ENABLED))
+			if (!msix)
 				writel(0, base + NvRegIrqMask);
 			else
 				writel(np->irqmask, base + NvRegIrqMask);
@@ -3068,7 +3068,7 @@ static irqreturn_t nv_nic_irq(int foo, v
 		if (unlikely(i > max_interrupt_work)) {
 			spin_lock(&np->lock);
 			/* disable interrupts on the nic */
-			if (!(np->msi_flags & NV_MSI_X_ENABLED))
+			if (!msix)
 				writel(0, base + NvRegIrqMask);
 			else
 				writel(np->irqmask, base + NvRegIrqMask);
@@ -3089,12 +3089,23 @@ static irqreturn_t nv_nic_irq(int foo, v
 	return IRQ_RETVAL(i);
 }
 
+static irqreturn_t nv_nic_irq_msix(int foo, void *data)
+{
+	return nv_nic_irq_x(foo, data, 1);
+}
+
+static irqreturn_t nv_nic_irq_normal(int foo, void *data)
+{
+	return nv_nic_irq_x(foo, data, 0);
+}
+
+
 /**
  * All _optimized functions are used to help increase performance
  * (reduce CPU and increase throughput). They use descripter version 3,
  * compiler directives, and reduce memory accesses.
  */
-static irqreturn_t nv_nic_irq_optimized(int foo, void *data)
+static inline irqreturn_t nv_nic_irq_optimized_x(int foo, void *data, int msix)
 {
 	struct net_device *dev = (struct net_device *) data;
 	struct fe_priv *np = netdev_priv(dev);
@@ -3105,7 +3116,7 @@ static irqreturn_t nv_nic_irq_optimized(
 	dprintk(KERN_DEBUG "%s: nv_nic_irq_optimized\n", dev->name);
 
 	for (i=0; ; i++) {
-		if (!(np->msi_flags & NV_MSI_X_ENABLED)) {
+		if (!msix) {
 			events = readl(base + NvRegIrqStatus) & NVREG_IRQSTAT_MASK;
 			writel(NVREG_IRQSTAT_MASK, base + NvRegIrqStatus);
 		} else {
@@ -3128,7 +3139,7 @@ static irqreturn_t nv_nic_irq_optimized(
 			spin_lock(&np->lock);
 			np->irqmask &= ~NVREG_IRQ_RX_ALL;
 
-			if (np->msi_flags & NV_MSI_X_ENABLED)
+			if (msix)
 				writel(NVREG_IRQ_RX_ALL, base + NvRegIrqMask);
 			else
 				writel(np->irqmask, base + NvRegIrqMask);
@@ -3166,7 +3177,7 @@ static irqreturn_t nv_nic_irq_optimized(
 		if (unlikely(events & NVREG_IRQ_RECOVER_ERROR)) {
 			spin_lock(&np->lock);
 			/* disable interrupts on the nic */
-			if (!(np->msi_flags & NV_MSI_X_ENABLED))
+			if (!msix)
 				writel(0, base + NvRegIrqMask);
 			else
 				writel(np->irqmask, base + NvRegIrqMask);
@@ -3184,7 +3195,7 @@ static irqreturn_t nv_nic_irq_optimized(
 		if (unlikely(i > max_interrupt_work)) {
 			spin_lock(&np->lock);
 			/* disable interrupts on the nic */
-			if (!(np->msi_flags & NV_MSI_X_ENABLED))
+			if (!msix)
 				writel(0, base + NvRegIrqMask);
 			else
 				writel(np->irqmask, base + NvRegIrqMask);
@@ -3205,6 +3216,16 @@ static irqreturn_t nv_nic_irq_optimized(
 	return IRQ_RETVAL(i);
 }
 
+static irqreturn_t nv_nic_irq_optimized_msix(int foo, void *data)
+{
+	return nv_nic_irq_optimized_x(foo, data, 1);
+}
+
+static irqreturn_t nv_nic_irq_optimized_normal(int foo, void *data)
+{
+	return nv_nic_irq_optimized_x(foo, data, 0);
+}
+
 static irqreturn_t nv_nic_irq_tx(int foo, void *data)
 {
 	struct net_device *dev = (struct net_device *) data;
@@ -3503,9 +3524,9 @@ static int nv_request_irq(struct net_dev
 		handler = nv_nic_irq_test;
 	} else {
 		if (np->desc_ver == DESC_VER_3)
-			handler = nv_nic_irq_optimized;
+			handler = nv_nic_irq_optimized_normal;
 		else
-			handler = nv_nic_irq;
+			handler = nv_nic_irq_normal;
 	}
 
 	if (np->msi_flags & NV_MSI_X_CAPABLE) {
@@ -3543,6 +3564,13 @@ static int nv_request_irq(struct net_dev
 				set_msix_vector_map(dev, NV_MSI_X_VECTOR_TX, NVREG_IRQ_TX_ALL);
 				set_msix_vector_map(dev, NV_MSI_X_VECTOR_OTHER, NVREG_IRQ_OTHER);
 			} else {
+				if (!intr_test) {
+					if (np->desc_ver == DESC_VER_3)
+						handler = nv_nic_irq_optimized_msix;
+					else
+						handler = nv_nic_irq_msix;
+				}
+
 				/* Request irq for all interrupts */
 				if (request_irq(np->msi_x_entry[NV_MSI_X_VECTOR_ALL].vector, handler, IRQF_SHARED, dev->name, dev) != 0) {
 					printk(KERN_INFO "forcedeth: request_irq failed %d\n", ret);
@@ -3689,10 +3717,17 @@ static void nv_do_nic_poll(unsigned long
 	pci_push(base);
 
 	if (!using_multi_irqs(dev)) {
-		if (np->desc_ver == DESC_VER_3)
-			nv_nic_irq_optimized(0, dev);
-		else
-			nv_nic_irq(0, dev);
+		if (np->msi_flags & NV_MSI_X_ENABLED) {
+			if (np->desc_ver == DESC_VER_3)
+				nv_nic_irq_optimized_msix(0, dev);
+			else
+				nv_nic_irq_msix(0, dev);
+		} else {
+			if (np->desc_ver == DESC_VER_3)
+				nv_nic_irq_optimized_normal(0, dev);
+			else
+				nv_nic_irq_normal(0, dev);
+		}
 		if (np->msi_flags & NV_MSI_X_ENABLED)
 			enable_irq_lockdep(np->msi_x_entry[NV_MSI_X_VECTOR_ALL].vector);
 		else

Re: [PATCH] forcedeth: seperate handler for msix and normal int.

[Ingo Molnar]

* Yinghai Lu <Yinghai.Lu@Sun.COM> wrote:

> [PATCH] forcedeth: seperate handler for msix and normal int.
> 
> so we don't need to keep checking np->msi_flags to see if 
> NV_MSI_X_ENABLED is set in handler

hm, why is this patch needed and what's the effect of this patch?

i'm also wondering, what happened to Jeff's very nice flow cleanups and 
simplifications to forcedeth.c. They are not in 2.6.24 - are they queued 
up for 2.6.25 perhaps?

	Ingo

Re: [PATCH] forcedeth: seperate handler for msix and normal int.

[Jeff Garzik]

Yinghai Lu wrote:
> [PATCH] forcedeth: seperate handler for msix and normal int.
> 
> so we don't need to keep checking np->msi_flags to see if NV_MSI_X_ENABLED is set in
> handler
> 
> Signed-off-by: Yinghai Lu <yinghai.lu@sun.com>

Two comments:

* can you regenerate this on top of 2.6.25-rc1?

* take a look at the 'fe' branch of 
git://git.kernel.org/pub/scm/linux/kernel/git/jgarzik/netdev-2.6.git

and see what you think about the work in there.  We need to get cleanups 
like that (and more) upstream.  It also intersects with Tom @ Google's 
"Reduce locking in TX path" patch.

	Jeff

[PATCH] forcedeth: seperate handler for msix and normal int v2

[Yinghai Lu]

so we don't need to keep check np->msi_flags to see if NV_MSI_X_ENABLED is set in
handler

Signed-off-by: Yinghai Lu <yinghai.lu@sun.com>

Index: linux-2.6/drivers/net/forcedeth.c
===================================================================
--- linux-2.6.orig/drivers/net/forcedeth.c
+++ linux-2.6/drivers/net/forcedeth.c
@@ -2901,12 +2901,12 @@ static void nv_link_irq(struct net_device *dev)
 	dprintk(KERN_DEBUG "%s: link change notification done.\n", dev->name);
 }
 
-static irqreturn_t __nv_nic_irq(struct net_device *dev, bool optimized)
+static inline irqreturn_t __nv_nic_irq(struct net_device *dev, bool optimized,
+				bool msix)
 {
 	struct fe_priv *np = netdev_priv(dev);
 	u8 __iomem *base = np->base;
 	u32 events, updmask = 0;
-	bool msix = np->msi_flags & NV_MSI_X_ENABLED;
 
 	dprintk(KERN_DEBUG "%s: __nv_nic_irq\n", dev->name);
 
@@ -2984,16 +2984,26 @@ out:
 	return IRQ_HANDLED;
 }
 
-static irqreturn_t nv_nic_irq(int foo, void *data)
+static irqreturn_t nv_nic_irq_msix(int foo, void *data)
 {
 	struct net_device *dev = data;
-	return __nv_nic_irq(dev, false);
+	return __nv_nic_irq(dev, false, true);
+}
+static irqreturn_t nv_nic_irq_normal(int foo, void *data)
+{
+	struct net_device *dev = data;
+	return __nv_nic_irq(dev, false, false);
 }
 
-static irqreturn_t nv_nic_irq_optimized(int foo, void *data)
+static irqreturn_t nv_nic_irq_optimized_msix(int foo, void *data)
 {
 	struct net_device *dev = data;
-	return __nv_nic_irq(dev, true);
+	return __nv_nic_irq(dev, true, true);
+}
+static irqreturn_t nv_nic_irq_optimized_normal(int foo, void *data)
+{
+	struct net_device *dev = data;
+	return __nv_nic_irq(dev, true, false);
 }
 
 static int nv_napi_poll(struct napi_struct *napi, int budget)
@@ -3222,9 +3232,9 @@ static int nv_request_irq(struct net_device *dev, int intr_test)
 		handler = nv_nic_irq_test;
 	} else {
 		if (nv_optimized(np))
-			handler = nv_nic_irq_optimized;
+			handler = nv_nic_irq_optimized_normal;
 		else
-			handler = nv_nic_irq;
+			handler = nv_nic_irq_normal;
 	}
 
 	if (np->msi_flags & NV_MSI_X_CAPABLE) {
@@ -3262,6 +3272,10 @@ static int nv_request_irq(struct net_device *dev, int intr_test)
 				set_msix_vector_map(dev, NV_MSI_X_VECTOR_TX, NVREG_IRQ_TX_ALL);
 				set_msix_vector_map(dev, NV_MSI_X_VECTOR_OTHER, NVREG_IRQ_OTHER);
 			} else {
+				if (nv_optimized(np))
+					handler = nv_nic_irq_optimized_msix;
+				else
+					handler = nv_nic_irq_msix;
 				/* Request irq for all interrupts */
 				if (request_irq(np->msi_x_entry[NV_MSI_X_VECTOR_ALL].vector, handler, IRQF_SHARED, dev->name, dev) != 0) {
 					printk(KERN_INFO "forcedeth: request_irq failed %d\n", ret);
@@ -3403,14 +3417,19 @@ static void nv_reset_task(struct work_struct *work)
 	pci_push(base);
 
 	if (!using_multi_irqs(dev)) {
-		if (nv_optimized(np))
-			nv_nic_irq_optimized(0, dev);
-		else
-			nv_nic_irq(0, dev);
-		if (np->msi_flags & NV_MSI_X_ENABLED)
+		if (np->msi_flags & NV_MSI_X_ENABLED) {
+			if (nv_optimized(np))
+				nv_nic_irq_optimized_msix(0, dev);
+			else
+				nv_nic_irq_msix(0, dev);
 			enable_irq_lockdep(np->msi_x_entry[NV_MSI_X_VECTOR_ALL].vector);
-		else
+		} else {
+			if (nv_optimized(np))
+				nv_nic_irq_optimized_normal(0, dev);
+			else
+				nv_nic_irq_normal(0, dev);
 			enable_irq_lockdep(np->pci_dev->irq);
+		}
 	} else {
 		if (np->nic_poll_irq & NVREG_IRQ_RX_ALL) {
 			nv_nic_irq_rx(0, dev);