[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 <[email protected]>
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
* Yinghai Lu <[email protected]> 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
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 <[email protected]>
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
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 <[email protected]>
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);