* [PATCH] natsemi: NAPI support @ 2005-12-04 22:47 Mark Brown [not found] ` <20051204231209.GA28949@electric-eye.fr.zoreil.com> 0 siblings, 1 reply; 12+ messages in thread From: Mark Brown @ 2005-12-04 22:47 UTC (permalink / raw) To: Jeff Garzik, Tim Hockin; +Cc: Harald Welte, netdev, linux-kernel [-- Attachment #1: Type: text/plain, Size: 8906 bytes --] This patch against 2.6.14 converts the natsemi driver to use NAPI. It was originally based on one written by Harald Welte, though it has since been modified quite a bit, most extensively in order to remove the ability to disable NAPI since none of the other drivers seem to provide that functionality any more. Signed-off-by: Mark Brown <broonie@sirena.org.uk> --- linux-2.6.14/drivers/net/natsemi.c.orig 2005-11-29 19:29:12.000000000 +0000 +++ linux/drivers/net/natsemi.c 2005-12-04 22:13:33.000000000 +0000 @@ -3,6 +3,7 @@ Written/copyright 1999-2001 by Donald Becker. Portions copyright (c) 2001,2002 Sun Microsystems (thockin@sun.com) Portions copyright 2001,2002 Manfred Spraul (manfred@colorfullife.com) + Portions copyright 2004 Harald Welte <laforge@gnumonks.org> This software may be used and distributed according to the terms of the GNU General Public License (GPL), incorporated herein by reference. @@ -136,7 +137,6 @@ TODO: * big endian support with CFG:BEM instead of cpu_to_le32 * support for an external PHY - * NAPI */ #include <linux/config.h> @@ -160,6 +160,7 @@ #include <linux/mii.h> #include <linux/crc32.h> #include <linux/bitops.h> +#include <linux/prefetch.h> #include <asm/processor.h> /* Processor type for cache alignment. */ #include <asm/io.h> #include <asm/irq.h> @@ -183,8 +184,6 @@ NETIF_MSG_TX_ERR) static int debug = -1; -/* Maximum events (Rx packets, etc.) to handle at each interrupt. */ -static int max_interrupt_work = 20; static int mtu; /* Maximum number of multicast addresses to filter (vs. rx-all-multicast). @@ -251,14 +250,11 @@ MODULE_DESCRIPTION("National Semiconductor DP8381x series PCI Ethernet driver"); MODULE_LICENSE("GPL"); -module_param(max_interrupt_work, int, 0); module_param(mtu, int, 0); module_param(debug, int, 0); module_param(rx_copybreak, int, 0); module_param_array(options, int, NULL, 0); module_param_array(full_duplex, int, NULL, 0); -MODULE_PARM_DESC(max_interrupt_work, - "DP8381x maximum events handled per interrupt"); MODULE_PARM_DESC(mtu, "DP8381x MTU (all boards)"); MODULE_PARM_DESC(debug, "DP8381x default debug level"); MODULE_PARM_DESC(rx_copybreak, @@ -691,6 +687,8 @@ /* Based on MTU+slack. */ unsigned int rx_buf_sz; int oom; + /* Interrupt status */ + u32 intr_status; /* Do not touch the nic registers */ int hands_off; /* external phy that is used: only valid if dev->if_port != PORT_TP */ @@ -748,7 +746,8 @@ static int start_tx(struct sk_buff *skb, struct net_device *dev); static irqreturn_t intr_handler(int irq, void *dev_instance, struct pt_regs *regs); static void netdev_error(struct net_device *dev, int intr_status); -static void netdev_rx(struct net_device *dev); +static int natsemi_poll(struct net_device *dev, int *budget); +static void netdev_rx(struct net_device *dev, int *work_done, int work_to_do); static void netdev_tx_done(struct net_device *dev); static int natsemi_change_mtu(struct net_device *dev, int new_mtu); #ifdef CONFIG_NET_POLL_CONTROLLER @@ -776,6 +775,18 @@ return (void __iomem *) dev->base_addr; } +static inline void natsemi_irq_enable(struct net_device *dev) +{ + writel(1, ns_ioaddr(dev) + IntrEnable); + readl(ns_ioaddr(dev) + IntrEnable); +} + +static inline void natsemi_irq_disable(struct net_device *dev) +{ + writel(0, ns_ioaddr(dev) + IntrEnable); + readl(ns_ioaddr(dev) + IntrEnable); +} + static void move_int_phy(struct net_device *dev, int addr) { struct netdev_private *np = netdev_priv(dev); @@ -879,6 +890,7 @@ spin_lock_init(&np->lock); np->msg_enable = (debug >= 0) ? (1<<debug)-1 : NATSEMI_DEF_MSG; np->hands_off = 0; + np->intr_status = 0; /* Initial port: * - If the nic was configured to use an external phy and if find_mii @@ -932,6 +944,9 @@ dev->do_ioctl = &netdev_ioctl; dev->tx_timeout = &tx_timeout; dev->watchdog_timeo = TX_TIMEOUT; + dev->poll = natsemi_poll; + dev->weight = 64; + #ifdef CONFIG_NET_POLL_CONTROLLER dev->poll_controller = &natsemi_poll_controller; #endif @@ -2158,68 +2173,93 @@ } } -/* The interrupt handler does all of the Rx thread work and cleans up - after the Tx thread. */ +/* The interrupt handler doesn't actually handle interrupts itself, it + * schedules a NAPI poll if there is anything to do. */ static irqreturn_t intr_handler(int irq, void *dev_instance, struct pt_regs *rgs) { struct net_device *dev = dev_instance; struct netdev_private *np = netdev_priv(dev); void __iomem * ioaddr = ns_ioaddr(dev); - int boguscnt = max_interrupt_work; - unsigned int handled = 0; if (np->hands_off) return IRQ_NONE; - do { - /* Reading automatically acknowledges all int sources. */ - u32 intr_status = readl(ioaddr + IntrStatus); + + /* Reading automatically acknowledges. */ + np->intr_status = readl(ioaddr + IntrStatus); - if (netif_msg_intr(np)) - printk(KERN_DEBUG - "%s: Interrupt, status %#08x, mask %#08x.\n", - dev->name, intr_status, - readl(ioaddr + IntrMask)); + if (netif_msg_intr(np)) + printk(KERN_DEBUG + "%s: Interrupt, status %#08x, mask %#08x.\n", + dev->name, np->intr_status, + readl(ioaddr + IntrMask)); - if (intr_status == 0) - break; - handled = 1; + if (!np->intr_status) + return IRQ_NONE; - if (intr_status & - (IntrRxDone | IntrRxIntr | RxStatusFIFOOver | - IntrRxErr | IntrRxOverrun)) { - netdev_rx(dev); - } + prefetch(&np->rx_skbuff[np->cur_rx % RX_RING_SIZE]); + + if (netif_rx_schedule_prep(dev)) { + /* Disable interrupts and register for poll */ + natsemi_irq_disable(dev); + __netif_rx_schedule(dev); + } + return IRQ_HANDLED; +} + +/* This is the NAPI poll routine. As well as the standard RX handling + * it also handles all other interrupts that the chip might raise. + */ +static int natsemi_poll(struct net_device *dev, int *budget) +{ + struct netdev_private *np = netdev_priv(dev); + void __iomem * ioaddr = ns_ioaddr(dev); - if (intr_status & - (IntrTxDone | IntrTxIntr | IntrTxIdle | IntrTxErr)) { + int work_to_do = min(*budget, dev->quota); + int work_done = 0; + + spin_lock_irq(&np->lock); + if (np->hands_off) { + spin_unlock_irq(&np->lock); + return 0; + } + spin_unlock_irq(&np->lock); + + do { + if (np->intr_status & + (IntrTxDone | IntrTxIntr | IntrTxIdle | IntrTxErr)) { spin_lock(&np->lock); netdev_tx_done(dev); spin_unlock(&np->lock); } - + /* Abnormal error summary/uncommon events handlers. */ - if (intr_status & IntrAbnormalSummary) - netdev_error(dev, intr_status); - - if (--boguscnt < 0) { - if (netif_msg_intr(np)) - printk(KERN_WARNING - "%s: Too much work at interrupt, " - "status=%#08x.\n", - dev->name, intr_status); - break; + if (np->intr_status & IntrAbnormalSummary) + netdev_error(dev, np->intr_status); + + if (np->intr_status & + (IntrRxDone | IntrRxIntr | RxStatusFIFOOver | + IntrRxErr | IntrRxOverrun)) { + netdev_rx(dev, &work_done, work_to_do); } - } while (1); + + *budget -= work_done; + dev->quota -= work_done; - if (netif_msg_intr(np)) - printk(KERN_DEBUG "%s: exiting interrupt.\n", dev->name); + if (work_done >= work_to_do) + return 1; - return IRQ_RETVAL(handled); + np->intr_status = readl(ioaddr + IntrStatus); + } while (np->intr_status); + + netif_rx_complete(dev); + natsemi_irq_enable(dev); + + return 0; } /* This routine is logically part of the interrupt handler, but separated for clarity and better register allocation. */ -static void netdev_rx(struct net_device *dev) +static void netdev_rx(struct net_device *dev, int *work_done, int work_to_do) { struct netdev_private *np = netdev_priv(dev); int entry = np->cur_rx % RX_RING_SIZE; @@ -2237,6 +2277,12 @@ entry, desc_status); if (--boguscnt < 0) break; + + if (*work_done >= work_to_do) + break; + + (*work_done)++; + pkt_len = (desc_status & DescSizeMask) - 4; if ((desc_status&(DescMore|DescPktOK|DescRxLong)) != DescPktOK){ if (desc_status & DescMore) { @@ -2293,7 +2339,7 @@ np->rx_skbuff[entry] = NULL; } skb->protocol = eth_type_trans(skb, dev); - netif_rx(skb); + netif_receive_skb(skb); dev->last_rx = jiffies; np->stats.rx_packets++; np->stats.rx_bytes += pkt_len; @@ -3175,6 +3221,8 @@ disable_irq(dev->irq); spin_lock_irq(&np->lock); + netif_poll_disable(dev); + writel(0, ioaddr + IntrEnable); np->hands_off = 1; natsemi_stop_rxtx(dev); @@ -3235,6 +3283,7 @@ mod_timer(&np->timer, jiffies + 1*HZ); } netif_device_attach(dev); + netif_poll_enable(dev); out: rtnl_unlock(); return 0; [-- Attachment #2: Digital signature --] [-- Type: application/pgp-signature, Size: 307 bytes --] ^ permalink raw reply [flat|nested] 12+ messages in thread
[parent not found: <20051204231209.GA28949@electric-eye.fr.zoreil.com>]
* Re: [PATCH] natsemi: NAPI support [not found] ` <20051204231209.GA28949@electric-eye.fr.zoreil.com> @ 2005-12-05 23:23 ` Mark Brown 2005-12-06 0:19 ` Francois Romieu 0 siblings, 1 reply; 12+ messages in thread From: Mark Brown @ 2005-12-05 23:23 UTC (permalink / raw) To: Francois Romieu Cc: Jeff Garzik, Tim Hockin, Harald Welte, netdev, linux-kernel [-- Attachment #1: Type: text/plain, Size: 638 bytes --] On Mon, Dec 05, 2005 at 12:12:09AM +0100, Francois Romieu wrote: > -> netif_poll_disable() may sleep while a spinlock is held. So it can, thanks. > Btw, the poll/close routines seem racy with each other. I had been under the impression that the stack was supposed to make sure that no poll() is running before the driver close() gets called? I could well be missing something there, though. Indeed, now that I think about it the calls netif_poll_disable() in suspend() ought to mean that we don't need to look at hands_off inside poll(). -- "You grabbed my hand and we fell into it, like a daydream - or a fever." [-- Attachment #2: Digital signature --] [-- Type: application/pgp-signature, Size: 307 bytes --] ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] natsemi: NAPI support 2005-12-05 23:23 ` Mark Brown @ 2005-12-06 0:19 ` Francois Romieu 2005-12-06 21:17 ` Mark Brown 0 siblings, 1 reply; 12+ messages in thread From: Francois Romieu @ 2005-12-06 0:19 UTC (permalink / raw) To: broonie; +Cc: Jeff Garzik, Tim Hockin, Harald Welte, netdev, linux-kernel Mark Brown <broonie@sirena.org.uk> : [...] > I had been under the impression that the stack was supposed to make sure > that no poll() is running before the driver close() gets called ? Not exactly. dev_close() waits a bit but it can not be sure that the device driver will not schedule ->poll() from its irq handler later. -- Ueimor ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] natsemi: NAPI support 2005-12-06 0:19 ` Francois Romieu @ 2005-12-06 21:17 ` Mark Brown 2005-12-06 21:56 ` Francois Romieu 0 siblings, 1 reply; 12+ messages in thread From: Mark Brown @ 2005-12-06 21:17 UTC (permalink / raw) To: Francois Romieu Cc: Jeff Garzik, Tim Hockin, Harald Welte, netdev, linux-kernel [-- Attachment #1: Type: text/plain, Size: 702 bytes --] On Tue, Dec 06, 2005 at 01:19:34AM +0100, Francois Romieu wrote: > Mark Brown <broonie@sirena.org.uk> : > > I had been under the impression that the stack was supposed to make sure > > that no poll() is running before the driver close() gets called? > Not exactly. dev_close() waits a bit but it can not be sure that the > device driver will not schedule ->poll() from its irq handler later. Prior to waiting dev_close() clears __LINK_STATE_START which will cause netif_rx_schedule_prep() to return false. As far as I can see that should prevent the interrupt handler scheduling any further poll() calls? -- "You grabbed my hand and we fell into it, like a daydream - or a fever." [-- Attachment #2: Digital signature --] [-- Type: application/pgp-signature, Size: 307 bytes --] ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] natsemi: NAPI support 2005-12-06 21:17 ` Mark Brown @ 2005-12-06 21:56 ` Francois Romieu 2005-12-09 10:48 ` Mark Brown 0 siblings, 1 reply; 12+ messages in thread From: Francois Romieu @ 2005-12-06 21:56 UTC (permalink / raw) To: Mark Brown; +Cc: Jeff Garzik, Tim Hockin, Harald Welte, netdev, linux-kernel Mark Brown <broonie@sirena.org.uk> : [...] > Prior to waiting dev_close() clears __LINK_STATE_START which will cause > netif_rx_schedule_prep() to return false. > As far as I can see that should prevent the interrupt handler scheduling > any further poll() calls? netif_rx_schedule_prep return netif_running(dev) && dev_close clear_bit(__LINK_STATE_START, &dev->state); dev_close smp_mb__after_clear_bit(); /* Commit netif_running(). */ dev_close while (test_bit(__LINK_STATE_RX_SCHED, &dev->state)) { dev_close /* No hurry. */ dev_close msleep(1); dev_close } dev_close netif_rx_schedule_prep !test_and_set_bit(__LINK_STATE_RX_SCHED, &dev->state); -- Ueimor ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] natsemi: NAPI support 2005-12-06 21:56 ` Francois Romieu @ 2005-12-09 10:48 ` Mark Brown 2005-12-12 23:55 ` Mark Brown 0 siblings, 1 reply; 12+ messages in thread From: Mark Brown @ 2005-12-09 10:48 UTC (permalink / raw) To: Francois Romieu Cc: Jeff Garzik, Tim Hockin, Harald Welte, netdev, linux-kernel [-- Attachment #1: Type: text/plain, Size: 354 bytes --] On Tue, Dec 06, 2005 at 10:56:19PM +0100, Francois Romieu wrote: > netif_rx_schedule_prep return netif_running(dev) && > dev_close clear_bit(__LINK_STATE_START, &dev->state); Oh, of course - thanks for bearing wth me. Will fix that too and resubmit. -- "You grabbed my hand and we fell into it, like a daydream - or a fever." [-- Attachment #2: Digital signature --] [-- Type: application/pgp-signature, Size: 307 bytes --] ^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH] natsemi: NAPI support 2005-12-09 10:48 ` Mark Brown @ 2005-12-12 23:55 ` Mark Brown 2005-12-13 0:25 ` Jeff Garzik 0 siblings, 1 reply; 12+ messages in thread From: Mark Brown @ 2005-12-12 23:55 UTC (permalink / raw) To: Tim Hockin; +Cc: Francois Romieu, netdev, linux-kernel [-- Attachment #1: Type: text/plain, Size: 10414 bytes --] This patch against 2.6.14 converts the natsemi driver to use NAPI. It was originally based on one written by Harald Welte, though it has since been modified quite a bit, most extensively in order to remove the ability to disable NAPI since none of the other drivers seem to provide that functionality any more. Signed-off-by: Mark Brown <broonie@sirena.org.uk> --- This revision of the patch: - Doesn't sleep with the device spinlock held in suspend(). - Improves the synchronisation between poll() and the shutdown paths. --- linux-2.6.14/drivers/net/natsemi.c.orig 2005-11-29 19:29:12.000000000 +0000 +++ linux/drivers/net/natsemi.c 2005-12-11 14:55:48.000000000 +0000 @@ -3,6 +3,7 @@ Written/copyright 1999-2001 by Donald Becker. Portions copyright (c) 2001,2002 Sun Microsystems (thockin@sun.com) Portions copyright 2001,2002 Manfred Spraul (manfred@colorfullife.com) + Portions copyright 2004 Harald Welte <laforge@gnumonks.org> This software may be used and distributed according to the terms of the GNU General Public License (GPL), incorporated herein by reference. @@ -135,8 +136,6 @@ TODO: * big endian support with CFG:BEM instead of cpu_to_le32 - * support for an external PHY - * NAPI */ #include <linux/config.h> @@ -160,6 +159,7 @@ #include <linux/mii.h> #include <linux/crc32.h> #include <linux/bitops.h> +#include <linux/prefetch.h> #include <asm/processor.h> /* Processor type for cache alignment. */ #include <asm/io.h> #include <asm/irq.h> @@ -183,8 +183,6 @@ NETIF_MSG_TX_ERR) static int debug = -1; -/* Maximum events (Rx packets, etc.) to handle at each interrupt. */ -static int max_interrupt_work = 20; static int mtu; /* Maximum number of multicast addresses to filter (vs. rx-all-multicast). @@ -251,14 +249,11 @@ MODULE_AUTHOR("Donald Becker <becker@scy MODULE_DESCRIPTION("National Semiconductor DP8381x series PCI Ethernet driver"); MODULE_LICENSE("GPL"); -module_param(max_interrupt_work, int, 0); module_param(mtu, int, 0); module_param(debug, int, 0); module_param(rx_copybreak, int, 0); module_param_array(options, int, NULL, 0); module_param_array(full_duplex, int, NULL, 0); -MODULE_PARM_DESC(max_interrupt_work, - "DP8381x maximum events handled per interrupt"); MODULE_PARM_DESC(mtu, "DP8381x MTU (all boards)"); MODULE_PARM_DESC(debug, "DP8381x default debug level"); MODULE_PARM_DESC(rx_copybreak, @@ -691,6 +686,8 @@ struct netdev_private { /* Based on MTU+slack. */ unsigned int rx_buf_sz; int oom; + /* Interrupt status */ + u32 intr_status; /* Do not touch the nic registers */ int hands_off; /* external phy that is used: only valid if dev->if_port != PORT_TP */ @@ -748,7 +745,8 @@ static void init_registers(struct net_de static int start_tx(struct sk_buff *skb, struct net_device *dev); static irqreturn_t intr_handler(int irq, void *dev_instance, struct pt_regs *regs); static void netdev_error(struct net_device *dev, int intr_status); -static void netdev_rx(struct net_device *dev); +static int natsemi_poll(struct net_device *dev, int *budget); +static void netdev_rx(struct net_device *dev, int *work_done, int work_to_do); static void netdev_tx_done(struct net_device *dev); static int natsemi_change_mtu(struct net_device *dev, int new_mtu); #ifdef CONFIG_NET_POLL_CONTROLLER @@ -776,6 +774,18 @@ static inline void __iomem *ns_ioaddr(st return (void __iomem *) dev->base_addr; } +static inline void natsemi_irq_enable(struct net_device *dev) +{ + writel(1, ns_ioaddr(dev) + IntrEnable); + readl(ns_ioaddr(dev) + IntrEnable); +} + +static inline void natsemi_irq_disable(struct net_device *dev) +{ + writel(0, ns_ioaddr(dev) + IntrEnable); + readl(ns_ioaddr(dev) + IntrEnable); +} + static void move_int_phy(struct net_device *dev, int addr) { struct netdev_private *np = netdev_priv(dev); @@ -879,6 +889,7 @@ static int __devinit natsemi_probe1 (str spin_lock_init(&np->lock); np->msg_enable = (debug >= 0) ? (1<<debug)-1 : NATSEMI_DEF_MSG; np->hands_off = 0; + np->intr_status = 0; /* Initial port: * - If the nic was configured to use an external phy and if find_mii @@ -932,6 +943,9 @@ static int __devinit natsemi_probe1 (str dev->do_ioctl = &netdev_ioctl; dev->tx_timeout = &tx_timeout; dev->watchdog_timeo = TX_TIMEOUT; + dev->poll = natsemi_poll; + dev->weight = 64; + #ifdef CONFIG_NET_POLL_CONTROLLER dev->poll_controller = &natsemi_poll_controller; #endif @@ -2158,68 +2172,92 @@ static void netdev_tx_done(struct net_de } } -/* The interrupt handler does all of the Rx thread work and cleans up - after the Tx thread. */ +/* The interrupt handler doesn't actually handle interrupts itself, it + * schedules a NAPI poll if there is anything to do. */ static irqreturn_t intr_handler(int irq, void *dev_instance, struct pt_regs *rgs) { struct net_device *dev = dev_instance; struct netdev_private *np = netdev_priv(dev); void __iomem * ioaddr = ns_ioaddr(dev); - int boguscnt = max_interrupt_work; - unsigned int handled = 0; if (np->hands_off) return IRQ_NONE; - do { - /* Reading automatically acknowledges all int sources. */ - u32 intr_status = readl(ioaddr + IntrStatus); + + /* Reading automatically acknowledges. */ + np->intr_status = readl(ioaddr + IntrStatus); - if (netif_msg_intr(np)) - printk(KERN_DEBUG - "%s: Interrupt, status %#08x, mask %#08x.\n", - dev->name, intr_status, - readl(ioaddr + IntrMask)); + if (netif_msg_intr(np)) + printk(KERN_DEBUG + "%s: Interrupt, status %#08x, mask %#08x.\n", + dev->name, np->intr_status, + readl(ioaddr + IntrMask)); - if (intr_status == 0) - break; - handled = 1; + if (!np->intr_status) + return IRQ_NONE; - if (intr_status & - (IntrRxDone | IntrRxIntr | RxStatusFIFOOver | - IntrRxErr | IntrRxOverrun)) { - netdev_rx(dev); - } + prefetch(&np->rx_skbuff[np->cur_rx % RX_RING_SIZE]); + + if (netif_rx_schedule_prep(dev)) { + /* Disable interrupts and register for poll */ + natsemi_irq_disable(dev); + __netif_rx_schedule(dev); + } + return IRQ_HANDLED; +} - if (intr_status & - (IntrTxDone | IntrTxIntr | IntrTxIdle | IntrTxErr)) { +/* This is the NAPI poll routine. As well as the standard RX handling + * it also handles all other interrupts that the chip might raise. + */ +static int natsemi_poll(struct net_device *dev, int *budget) +{ + struct netdev_private *np = netdev_priv(dev); + void __iomem * ioaddr = ns_ioaddr(dev); + + int work_to_do = min(*budget, dev->quota); + int work_done = 0; + + do { + if (np->intr_status & + (IntrTxDone | IntrTxIntr | IntrTxIdle | IntrTxErr)) { spin_lock(&np->lock); netdev_tx_done(dev); spin_unlock(&np->lock); } /* Abnormal error summary/uncommon events handlers. */ - if (intr_status & IntrAbnormalSummary) - netdev_error(dev, intr_status); - - if (--boguscnt < 0) { - if (netif_msg_intr(np)) - printk(KERN_WARNING - "%s: Too much work at interrupt, " - "status=%#08x.\n", - dev->name, intr_status); - break; + if (np->intr_status & IntrAbnormalSummary) + netdev_error(dev, np->intr_status); + + if (np->intr_status & + (IntrRxDone | IntrRxIntr | RxStatusFIFOOver | + IntrRxErr | IntrRxOverrun)) { + netdev_rx(dev, &work_done, work_to_do); } - } while (1); + + *budget -= work_done; + dev->quota -= work_done; - if (netif_msg_intr(np)) - printk(KERN_DEBUG "%s: exiting interrupt.\n", dev->name); + if (work_done >= work_to_do) + return 1; + + np->intr_status = readl(ioaddr + IntrStatus); + } while (np->intr_status); - return IRQ_RETVAL(handled); + netif_rx_complete(dev); + + /* Reenable interrupts providing nothing is trying to shut + * the chip down. */ + spin_lock(&np->lock); + if (!np->hands_off && netif_running(dev)) + natsemi_irq_enable(dev); + spin_unlock(&np->lock); + + return 0; } /* This routine is logically part of the interrupt handler, but separated for clarity and better register allocation. */ -static void netdev_rx(struct net_device *dev) +static void netdev_rx(struct net_device *dev, int *work_done, int work_to_do) { struct netdev_private *np = netdev_priv(dev); int entry = np->cur_rx % RX_RING_SIZE; @@ -2237,6 +2275,12 @@ static void netdev_rx(struct net_device entry, desc_status); if (--boguscnt < 0) break; + + if (*work_done >= work_to_do) + break; + + (*work_done)++; + pkt_len = (desc_status & DescSizeMask) - 4; if ((desc_status&(DescMore|DescPktOK|DescRxLong)) != DescPktOK){ if (desc_status & DescMore) { @@ -2293,7 +2337,7 @@ static void netdev_rx(struct net_device np->rx_skbuff[entry] = NULL; } skb->protocol = eth_type_trans(skb, dev); - netif_rx(skb); + netif_receive_skb(skb); dev->last_rx = jiffies; np->stats.rx_packets++; np->stats.rx_bytes += pkt_len; @@ -3074,9 +3118,7 @@ static int netdev_close(struct net_devic del_timer_sync(&np->timer); disable_irq(dev->irq); spin_lock_irq(&np->lock); - /* Disable interrupts, and flush posted writes */ - writel(0, ioaddr + IntrEnable); - readl(ioaddr + IntrEnable); + natsemi_irq_disable(dev); np->hands_off = 1; spin_unlock_irq(&np->lock); enable_irq(dev->irq); @@ -3158,6 +3200,9 @@ static void __devexit natsemi_remove1 (s * * netdev_timer: timer stopped by natsemi_suspend. * * intr_handler: doesn't acquire the spinlock. suspend calls * disable_irq() to enforce synchronization. + * * natsemi_poll: checks before reenabling interrupts. suspend + * sets hands_off, disables interrupts and then waits with + * netif_poll_disable(). * * Interrupts must be disabled, otherwise hands_off can cause irq storms. */ @@ -3183,6 +3228,8 @@ static int natsemi_suspend (struct pci_d spin_unlock_irq(&np->lock); enable_irq(dev->irq); + netif_poll_disable(dev); + /* Update the error counts. */ __get_stats(dev); @@ -3235,6 +3282,7 @@ static int natsemi_resume (struct pci_de mod_timer(&np->timer, jiffies + 1*HZ); } netif_device_attach(dev); + netif_poll_enable(dev); out: rtnl_unlock(); return 0; [-- Attachment #2: Digital signature --] [-- Type: application/pgp-signature, Size: 307 bytes --] ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] natsemi: NAPI support 2005-12-12 23:55 ` Mark Brown @ 2005-12-13 0:25 ` Jeff Garzik 2005-12-13 22:59 ` Mark Brown 2005-12-21 23:48 ` Resubmit: " Mark Brown 0 siblings, 2 replies; 12+ messages in thread From: Jeff Garzik @ 2005-12-13 0:25 UTC (permalink / raw) To: Mark Brown; +Cc: Tim Hockin, Francois Romieu, netdev, linux-kernel Mark Brown wrote: > This patch against 2.6.14 converts the natsemi driver to use NAPI. It > was originally based on one written by Harald Welte, though it has since > been modified quite a bit, most extensively in order to remove the > ability to disable NAPI since none of the other drivers seem to provide > that functionality any more. > > Signed-off-by: Mark Brown <broonie@sirena.org.uk> Was it updated per the comments you received on the first posting? Jeff ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] natsemi: NAPI support 2005-12-13 0:25 ` Jeff Garzik @ 2005-12-13 22:59 ` Mark Brown 2005-12-21 23:48 ` Resubmit: " Mark Brown 1 sibling, 0 replies; 12+ messages in thread From: Mark Brown @ 2005-12-13 22:59 UTC (permalink / raw) To: Jeff Garzik; +Cc: Tim Hockin, Francois Romieu, netdev, linux-kernel [-- Attachment #1: Type: text/plain, Size: 255 bytes --] On Mon, Dec 12, 2005 at 07:25:20PM -0500, Jeff Garzik wrote: > Was it updated per the comments you received on the first posting? I think I addressed everything, yes. -- "You grabbed my hand and we fell into it, like a daydream - or a fever." [-- Attachment #2: Digital signature --] [-- Type: application/pgp-signature, Size: 307 bytes --] ^ permalink raw reply [flat|nested] 12+ messages in thread
* Resubmit: [PATCH] natsemi: NAPI support 2005-12-13 0:25 ` Jeff Garzik 2005-12-13 22:59 ` Mark Brown @ 2005-12-21 23:48 ` Mark Brown 2006-01-04 7:32 ` Mark Brown 1 sibling, 1 reply; 12+ messages in thread From: Mark Brown @ 2005-12-21 23:48 UTC (permalink / raw) To: Jeff Garzik; +Cc: Tim Hockin, netdev, linux-kernel [-- Attachment #1: Type: text/plain, Size: 10477 bytes --] This patch against 2.6.14 converts the natsemi driver to use NAPI. It was originally based on one written by Harald Welte, though it has since been modified quite a bit, most extensively in order to remove the ability to disable NAPI since none of the other drivers seem to provide that functionality any more. Signed-off-by: Mark Brown <broonie@sirena.org.uk> --- This revision of the patch: - Doesn't sleep with the device spinlock held in suspend(). - Improves the synchronisation between poll() and the shutdown paths. and should address all the issues people previously raised. --- linux-2.6.14/drivers/net/natsemi.c.orig 2005-11-29 19:29:12.000000000 +0000 +++ linux/drivers/net/natsemi.c 2005-12-11 14:55:48.000000000 +0000 @@ -3,6 +3,7 @@ Written/copyright 1999-2001 by Donald Becker. Portions copyright (c) 2001,2002 Sun Microsystems (thockin@sun.com) Portions copyright 2001,2002 Manfred Spraul (manfred@colorfullife.com) + Portions copyright 2004 Harald Welte <laforge@gnumonks.org> This software may be used and distributed according to the terms of the GNU General Public License (GPL), incorporated herein by reference. @@ -135,8 +136,6 @@ TODO: * big endian support with CFG:BEM instead of cpu_to_le32 - * support for an external PHY - * NAPI */ #include <linux/config.h> @@ -160,6 +159,7 @@ #include <linux/mii.h> #include <linux/crc32.h> #include <linux/bitops.h> +#include <linux/prefetch.h> #include <asm/processor.h> /* Processor type for cache alignment. */ #include <asm/io.h> #include <asm/irq.h> @@ -183,8 +183,6 @@ NETIF_MSG_TX_ERR) static int debug = -1; -/* Maximum events (Rx packets, etc.) to handle at each interrupt. */ -static int max_interrupt_work = 20; static int mtu; /* Maximum number of multicast addresses to filter (vs. rx-all-multicast). @@ -251,14 +249,11 @@ MODULE_AUTHOR("Donald Becker <becker@scy MODULE_DESCRIPTION("National Semiconductor DP8381x series PCI Ethernet driver"); MODULE_LICENSE("GPL"); -module_param(max_interrupt_work, int, 0); module_param(mtu, int, 0); module_param(debug, int, 0); module_param(rx_copybreak, int, 0); module_param_array(options, int, NULL, 0); module_param_array(full_duplex, int, NULL, 0); -MODULE_PARM_DESC(max_interrupt_work, - "DP8381x maximum events handled per interrupt"); MODULE_PARM_DESC(mtu, "DP8381x MTU (all boards)"); MODULE_PARM_DESC(debug, "DP8381x default debug level"); MODULE_PARM_DESC(rx_copybreak, @@ -691,6 +686,8 @@ struct netdev_private { /* Based on MTU+slack. */ unsigned int rx_buf_sz; int oom; + /* Interrupt status */ + u32 intr_status; /* Do not touch the nic registers */ int hands_off; /* external phy that is used: only valid if dev->if_port != PORT_TP */ @@ -748,7 +745,8 @@ static void init_registers(struct net_de static int start_tx(struct sk_buff *skb, struct net_device *dev); static irqreturn_t intr_handler(int irq, void *dev_instance, struct pt_regs *regs); static void netdev_error(struct net_device *dev, int intr_status); -static void netdev_rx(struct net_device *dev); +static int natsemi_poll(struct net_device *dev, int *budget); +static void netdev_rx(struct net_device *dev, int *work_done, int work_to_do); static void netdev_tx_done(struct net_device *dev); static int natsemi_change_mtu(struct net_device *dev, int new_mtu); #ifdef CONFIG_NET_POLL_CONTROLLER @@ -776,6 +774,18 @@ static inline void __iomem *ns_ioaddr(st return (void __iomem *) dev->base_addr; } +static inline void natsemi_irq_enable(struct net_device *dev) +{ + writel(1, ns_ioaddr(dev) + IntrEnable); + readl(ns_ioaddr(dev) + IntrEnable); +} + +static inline void natsemi_irq_disable(struct net_device *dev) +{ + writel(0, ns_ioaddr(dev) + IntrEnable); + readl(ns_ioaddr(dev) + IntrEnable); +} + static void move_int_phy(struct net_device *dev, int addr) { struct netdev_private *np = netdev_priv(dev); @@ -879,6 +889,7 @@ static int __devinit natsemi_probe1 (str spin_lock_init(&np->lock); np->msg_enable = (debug >= 0) ? (1<<debug)-1 : NATSEMI_DEF_MSG; np->hands_off = 0; + np->intr_status = 0; /* Initial port: * - If the nic was configured to use an external phy and if find_mii @@ -932,6 +943,9 @@ static int __devinit natsemi_probe1 (str dev->do_ioctl = &netdev_ioctl; dev->tx_timeout = &tx_timeout; dev->watchdog_timeo = TX_TIMEOUT; + dev->poll = natsemi_poll; + dev->weight = 64; + #ifdef CONFIG_NET_POLL_CONTROLLER dev->poll_controller = &natsemi_poll_controller; #endif @@ -2158,68 +2172,92 @@ static void netdev_tx_done(struct net_de } } -/* The interrupt handler does all of the Rx thread work and cleans up - after the Tx thread. */ +/* The interrupt handler doesn't actually handle interrupts itself, it + * schedules a NAPI poll if there is anything to do. */ static irqreturn_t intr_handler(int irq, void *dev_instance, struct pt_regs *rgs) { struct net_device *dev = dev_instance; struct netdev_private *np = netdev_priv(dev); void __iomem * ioaddr = ns_ioaddr(dev); - int boguscnt = max_interrupt_work; - unsigned int handled = 0; if (np->hands_off) return IRQ_NONE; - do { - /* Reading automatically acknowledges all int sources. */ - u32 intr_status = readl(ioaddr + IntrStatus); + + /* Reading automatically acknowledges. */ + np->intr_status = readl(ioaddr + IntrStatus); - if (netif_msg_intr(np)) - printk(KERN_DEBUG - "%s: Interrupt, status %#08x, mask %#08x.\n", - dev->name, intr_status, - readl(ioaddr + IntrMask)); + if (netif_msg_intr(np)) + printk(KERN_DEBUG + "%s: Interrupt, status %#08x, mask %#08x.\n", + dev->name, np->intr_status, + readl(ioaddr + IntrMask)); - if (intr_status == 0) - break; - handled = 1; + if (!np->intr_status) + return IRQ_NONE; - if (intr_status & - (IntrRxDone | IntrRxIntr | RxStatusFIFOOver | - IntrRxErr | IntrRxOverrun)) { - netdev_rx(dev); - } + prefetch(&np->rx_skbuff[np->cur_rx % RX_RING_SIZE]); + + if (netif_rx_schedule_prep(dev)) { + /* Disable interrupts and register for poll */ + natsemi_irq_disable(dev); + __netif_rx_schedule(dev); + } + return IRQ_HANDLED; +} - if (intr_status & - (IntrTxDone | IntrTxIntr | IntrTxIdle | IntrTxErr)) { +/* This is the NAPI poll routine. As well as the standard RX handling + * it also handles all other interrupts that the chip might raise. + */ +static int natsemi_poll(struct net_device *dev, int *budget) +{ + struct netdev_private *np = netdev_priv(dev); + void __iomem * ioaddr = ns_ioaddr(dev); + + int work_to_do = min(*budget, dev->quota); + int work_done = 0; + + do { + if (np->intr_status & + (IntrTxDone | IntrTxIntr | IntrTxIdle | IntrTxErr)) { spin_lock(&np->lock); netdev_tx_done(dev); spin_unlock(&np->lock); } /* Abnormal error summary/uncommon events handlers. */ - if (intr_status & IntrAbnormalSummary) - netdev_error(dev, intr_status); - - if (--boguscnt < 0) { - if (netif_msg_intr(np)) - printk(KERN_WARNING - "%s: Too much work at interrupt, " - "status=%#08x.\n", - dev->name, intr_status); - break; + if (np->intr_status & IntrAbnormalSummary) + netdev_error(dev, np->intr_status); + + if (np->intr_status & + (IntrRxDone | IntrRxIntr | RxStatusFIFOOver | + IntrRxErr | IntrRxOverrun)) { + netdev_rx(dev, &work_done, work_to_do); } - } while (1); + + *budget -= work_done; + dev->quota -= work_done; - if (netif_msg_intr(np)) - printk(KERN_DEBUG "%s: exiting interrupt.\n", dev->name); + if (work_done >= work_to_do) + return 1; + + np->intr_status = readl(ioaddr + IntrStatus); + } while (np->intr_status); - return IRQ_RETVAL(handled); + netif_rx_complete(dev); + + /* Reenable interrupts providing nothing is trying to shut + * the chip down. */ + spin_lock(&np->lock); + if (!np->hands_off && netif_running(dev)) + natsemi_irq_enable(dev); + spin_unlock(&np->lock); + + return 0; } /* This routine is logically part of the interrupt handler, but separated for clarity and better register allocation. */ -static void netdev_rx(struct net_device *dev) +static void netdev_rx(struct net_device *dev, int *work_done, int work_to_do) { struct netdev_private *np = netdev_priv(dev); int entry = np->cur_rx % RX_RING_SIZE; @@ -2237,6 +2275,12 @@ static void netdev_rx(struct net_device entry, desc_status); if (--boguscnt < 0) break; + + if (*work_done >= work_to_do) + break; + + (*work_done)++; + pkt_len = (desc_status & DescSizeMask) - 4; if ((desc_status&(DescMore|DescPktOK|DescRxLong)) != DescPktOK){ if (desc_status & DescMore) { @@ -2293,7 +2337,7 @@ static void netdev_rx(struct net_device np->rx_skbuff[entry] = NULL; } skb->protocol = eth_type_trans(skb, dev); - netif_rx(skb); + netif_receive_skb(skb); dev->last_rx = jiffies; np->stats.rx_packets++; np->stats.rx_bytes += pkt_len; @@ -3074,9 +3118,7 @@ static int netdev_close(struct net_devic del_timer_sync(&np->timer); disable_irq(dev->irq); spin_lock_irq(&np->lock); - /* Disable interrupts, and flush posted writes */ - writel(0, ioaddr + IntrEnable); - readl(ioaddr + IntrEnable); + natsemi_irq_disable(dev); np->hands_off = 1; spin_unlock_irq(&np->lock); enable_irq(dev->irq); @@ -3158,6 +3200,9 @@ static void __devexit natsemi_remove1 (s * * netdev_timer: timer stopped by natsemi_suspend. * * intr_handler: doesn't acquire the spinlock. suspend calls * disable_irq() to enforce synchronization. + * * natsemi_poll: checks before reenabling interrupts. suspend + * sets hands_off, disables interrupts and then waits with + * netif_poll_disable(). * * Interrupts must be disabled, otherwise hands_off can cause irq storms. */ @@ -3183,6 +3228,8 @@ static int natsemi_suspend (struct pci_d spin_unlock_irq(&np->lock); enable_irq(dev->irq); + netif_poll_disable(dev); + /* Update the error counts. */ __get_stats(dev); @@ -3235,6 +3282,7 @@ static int natsemi_resume (struct pci_de mod_timer(&np->timer, jiffies + 1*HZ); } netif_device_attach(dev); + netif_poll_enable(dev); out: rtnl_unlock(); return 0; [-- Attachment #2: Digital signature --] [-- Type: application/pgp-signature, Size: 307 bytes --] ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: Resubmit: [PATCH] natsemi: NAPI support 2005-12-21 23:48 ` Resubmit: " Mark Brown @ 2006-01-04 7:32 ` Mark Brown 2006-01-04 13:40 ` Harald Welte 0 siblings, 1 reply; 12+ messages in thread From: Mark Brown @ 2006-01-04 7:32 UTC (permalink / raw) To: Jeff Garzik; +Cc: Tim Hockin, netdev, linux-kernel [-- Attachment #1: Type: text/plain, Size: 10414 bytes --] This patch against 2.6.14 converts the natsemi driver to use NAPI. It was originally based on one written by Harald Welte, though it has since been modified quite a bit, most extensively in order to remove the ability to disable NAPI since none of the other drivers seem to provide that functionality any more. Signed-off-by: Mark Brown <broonie@sirena.org.uk> --- This revision of the patch: - Doesn't sleep with the device spinlock held in suspend(). - Improves the synchronisation between poll() and the shutdown paths. --- linux-2.6.14/drivers/net/natsemi.c.orig 2005-11-29 19:29:12.000000000 +0000 +++ linux/drivers/net/natsemi.c 2005-12-11 14:55:48.000000000 +0000 @@ -3,6 +3,7 @@ Written/copyright 1999-2001 by Donald Becker. Portions copyright (c) 2001,2002 Sun Microsystems (thockin@sun.com) Portions copyright 2001,2002 Manfred Spraul (manfred@colorfullife.com) + Portions copyright 2004 Harald Welte <laforge@gnumonks.org> This software may be used and distributed according to the terms of the GNU General Public License (GPL), incorporated herein by reference. @@ -135,8 +136,6 @@ TODO: * big endian support with CFG:BEM instead of cpu_to_le32 - * support for an external PHY - * NAPI */ #include <linux/config.h> @@ -160,6 +159,7 @@ #include <linux/mii.h> #include <linux/crc32.h> #include <linux/bitops.h> +#include <linux/prefetch.h> #include <asm/processor.h> /* Processor type for cache alignment. */ #include <asm/io.h> #include <asm/irq.h> @@ -183,8 +183,6 @@ NETIF_MSG_TX_ERR) static int debug = -1; -/* Maximum events (Rx packets, etc.) to handle at each interrupt. */ -static int max_interrupt_work = 20; static int mtu; /* Maximum number of multicast addresses to filter (vs. rx-all-multicast). @@ -251,14 +249,11 @@ MODULE_AUTHOR("Donald Becker <becker@scy MODULE_DESCRIPTION("National Semiconductor DP8381x series PCI Ethernet driver"); MODULE_LICENSE("GPL"); -module_param(max_interrupt_work, int, 0); module_param(mtu, int, 0); module_param(debug, int, 0); module_param(rx_copybreak, int, 0); module_param_array(options, int, NULL, 0); module_param_array(full_duplex, int, NULL, 0); -MODULE_PARM_DESC(max_interrupt_work, - "DP8381x maximum events handled per interrupt"); MODULE_PARM_DESC(mtu, "DP8381x MTU (all boards)"); MODULE_PARM_DESC(debug, "DP8381x default debug level"); MODULE_PARM_DESC(rx_copybreak, @@ -691,6 +686,8 @@ struct netdev_private { /* Based on MTU+slack. */ unsigned int rx_buf_sz; int oom; + /* Interrupt status */ + u32 intr_status; /* Do not touch the nic registers */ int hands_off; /* external phy that is used: only valid if dev->if_port != PORT_TP */ @@ -748,7 +745,8 @@ static void init_registers(struct net_de static int start_tx(struct sk_buff *skb, struct net_device *dev); static irqreturn_t intr_handler(int irq, void *dev_instance, struct pt_regs *regs); static void netdev_error(struct net_device *dev, int intr_status); -static void netdev_rx(struct net_device *dev); +static int natsemi_poll(struct net_device *dev, int *budget); +static void netdev_rx(struct net_device *dev, int *work_done, int work_to_do); static void netdev_tx_done(struct net_device *dev); static int natsemi_change_mtu(struct net_device *dev, int new_mtu); #ifdef CONFIG_NET_POLL_CONTROLLER @@ -776,6 +774,18 @@ static inline void __iomem *ns_ioaddr(st return (void __iomem *) dev->base_addr; } +static inline void natsemi_irq_enable(struct net_device *dev) +{ + writel(1, ns_ioaddr(dev) + IntrEnable); + readl(ns_ioaddr(dev) + IntrEnable); +} + +static inline void natsemi_irq_disable(struct net_device *dev) +{ + writel(0, ns_ioaddr(dev) + IntrEnable); + readl(ns_ioaddr(dev) + IntrEnable); +} + static void move_int_phy(struct net_device *dev, int addr) { struct netdev_private *np = netdev_priv(dev); @@ -879,6 +889,7 @@ static int __devinit natsemi_probe1 (str spin_lock_init(&np->lock); np->msg_enable = (debug >= 0) ? (1<<debug)-1 : NATSEMI_DEF_MSG; np->hands_off = 0; + np->intr_status = 0; /* Initial port: * - If the nic was configured to use an external phy and if find_mii @@ -932,6 +943,9 @@ static int __devinit natsemi_probe1 (str dev->do_ioctl = &netdev_ioctl; dev->tx_timeout = &tx_timeout; dev->watchdog_timeo = TX_TIMEOUT; + dev->poll = natsemi_poll; + dev->weight = 64; + #ifdef CONFIG_NET_POLL_CONTROLLER dev->poll_controller = &natsemi_poll_controller; #endif @@ -2158,68 +2172,92 @@ static void netdev_tx_done(struct net_de } } -/* The interrupt handler does all of the Rx thread work and cleans up - after the Tx thread. */ +/* The interrupt handler doesn't actually handle interrupts itself, it + * schedules a NAPI poll if there is anything to do. */ static irqreturn_t intr_handler(int irq, void *dev_instance, struct pt_regs *rgs) { struct net_device *dev = dev_instance; struct netdev_private *np = netdev_priv(dev); void __iomem * ioaddr = ns_ioaddr(dev); - int boguscnt = max_interrupt_work; - unsigned int handled = 0; if (np->hands_off) return IRQ_NONE; - do { - /* Reading automatically acknowledges all int sources. */ - u32 intr_status = readl(ioaddr + IntrStatus); + + /* Reading automatically acknowledges. */ + np->intr_status = readl(ioaddr + IntrStatus); - if (netif_msg_intr(np)) - printk(KERN_DEBUG - "%s: Interrupt, status %#08x, mask %#08x.\n", - dev->name, intr_status, - readl(ioaddr + IntrMask)); + if (netif_msg_intr(np)) + printk(KERN_DEBUG + "%s: Interrupt, status %#08x, mask %#08x.\n", + dev->name, np->intr_status, + readl(ioaddr + IntrMask)); - if (intr_status == 0) - break; - handled = 1; + if (!np->intr_status) + return IRQ_NONE; - if (intr_status & - (IntrRxDone | IntrRxIntr | RxStatusFIFOOver | - IntrRxErr | IntrRxOverrun)) { - netdev_rx(dev); - } + prefetch(&np->rx_skbuff[np->cur_rx % RX_RING_SIZE]); + + if (netif_rx_schedule_prep(dev)) { + /* Disable interrupts and register for poll */ + natsemi_irq_disable(dev); + __netif_rx_schedule(dev); + } + return IRQ_HANDLED; +} - if (intr_status & - (IntrTxDone | IntrTxIntr | IntrTxIdle | IntrTxErr)) { +/* This is the NAPI poll routine. As well as the standard RX handling + * it also handles all other interrupts that the chip might raise. + */ +static int natsemi_poll(struct net_device *dev, int *budget) +{ + struct netdev_private *np = netdev_priv(dev); + void __iomem * ioaddr = ns_ioaddr(dev); + + int work_to_do = min(*budget, dev->quota); + int work_done = 0; + + do { + if (np->intr_status & + (IntrTxDone | IntrTxIntr | IntrTxIdle | IntrTxErr)) { spin_lock(&np->lock); netdev_tx_done(dev); spin_unlock(&np->lock); } /* Abnormal error summary/uncommon events handlers. */ - if (intr_status & IntrAbnormalSummary) - netdev_error(dev, intr_status); - - if (--boguscnt < 0) { - if (netif_msg_intr(np)) - printk(KERN_WARNING - "%s: Too much work at interrupt, " - "status=%#08x.\n", - dev->name, intr_status); - break; + if (np->intr_status & IntrAbnormalSummary) + netdev_error(dev, np->intr_status); + + if (np->intr_status & + (IntrRxDone | IntrRxIntr | RxStatusFIFOOver | + IntrRxErr | IntrRxOverrun)) { + netdev_rx(dev, &work_done, work_to_do); } - } while (1); + + *budget -= work_done; + dev->quota -= work_done; - if (netif_msg_intr(np)) - printk(KERN_DEBUG "%s: exiting interrupt.\n", dev->name); + if (work_done >= work_to_do) + return 1; + + np->intr_status = readl(ioaddr + IntrStatus); + } while (np->intr_status); - return IRQ_RETVAL(handled); + netif_rx_complete(dev); + + /* Reenable interrupts providing nothing is trying to shut + * the chip down. */ + spin_lock(&np->lock); + if (!np->hands_off && netif_running(dev)) + natsemi_irq_enable(dev); + spin_unlock(&np->lock); + + return 0; } /* This routine is logically part of the interrupt handler, but separated for clarity and better register allocation. */ -static void netdev_rx(struct net_device *dev) +static void netdev_rx(struct net_device *dev, int *work_done, int work_to_do) { struct netdev_private *np = netdev_priv(dev); int entry = np->cur_rx % RX_RING_SIZE; @@ -2237,6 +2275,12 @@ static void netdev_rx(struct net_device entry, desc_status); if (--boguscnt < 0) break; + + if (*work_done >= work_to_do) + break; + + (*work_done)++; + pkt_len = (desc_status & DescSizeMask) - 4; if ((desc_status&(DescMore|DescPktOK|DescRxLong)) != DescPktOK){ if (desc_status & DescMore) { @@ -2293,7 +2337,7 @@ static void netdev_rx(struct net_device np->rx_skbuff[entry] = NULL; } skb->protocol = eth_type_trans(skb, dev); - netif_rx(skb); + netif_receive_skb(skb); dev->last_rx = jiffies; np->stats.rx_packets++; np->stats.rx_bytes += pkt_len; @@ -3074,9 +3118,7 @@ static int netdev_close(struct net_devic del_timer_sync(&np->timer); disable_irq(dev->irq); spin_lock_irq(&np->lock); - /* Disable interrupts, and flush posted writes */ - writel(0, ioaddr + IntrEnable); - readl(ioaddr + IntrEnable); + natsemi_irq_disable(dev); np->hands_off = 1; spin_unlock_irq(&np->lock); enable_irq(dev->irq); @@ -3158,6 +3200,9 @@ static void __devexit natsemi_remove1 (s * * netdev_timer: timer stopped by natsemi_suspend. * * intr_handler: doesn't acquire the spinlock. suspend calls * disable_irq() to enforce synchronization. + * * natsemi_poll: checks before reenabling interrupts. suspend + * sets hands_off, disables interrupts and then waits with + * netif_poll_disable(). * * Interrupts must be disabled, otherwise hands_off can cause irq storms. */ @@ -3183,6 +3228,8 @@ static int natsemi_suspend (struct pci_d spin_unlock_irq(&np->lock); enable_irq(dev->irq); + netif_poll_disable(dev); + /* Update the error counts. */ __get_stats(dev); @@ -3235,6 +3282,7 @@ static int natsemi_resume (struct pci_de mod_timer(&np->timer, jiffies + 1*HZ); } netif_device_attach(dev); + netif_poll_enable(dev); out: rtnl_unlock(); return 0; [-- Attachment #2: Digital signature --] [-- Type: application/pgp-signature, Size: 307 bytes --] ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: Resubmit: [PATCH] natsemi: NAPI support 2006-01-04 7:32 ` Mark Brown @ 2006-01-04 13:40 ` Harald Welte 0 siblings, 0 replies; 12+ messages in thread From: Harald Welte @ 2006-01-04 13:40 UTC (permalink / raw) To: Jeff Garzik, Tim Hockin, netdev, linux-kernel [-- Attachment #1: Type: text/plain, Size: 963 bytes --] On Wed, Jan 04, 2006 at 07:32:49AM +0000, Mark Brown wrote: > This patch against 2.6.14 converts the natsemi driver to use NAPI. It > was originally based on one written by Harald Welte, though it has since > been modified quite a bit, most extensively in order to remove the > ability to disable NAPI since none of the other drivers seem to provide > that functionality any more. Mark, sorry for not responding earlier to your emails with regard to your natsemi patch. Thanks for pushing this persistently. From reviewing your patch, I personally thin it's fine to be merged. I will test it on my natsemi based boxes later today. -- - Harald Welte <laforge@gnumonks.org> http://gnumonks.org/ ============================================================================ "Privacy in residential applications is a desirable marketing option." (ETSI EN 300 175-7 Ch. A6) [-- Attachment #2: Type: application/pgp-signature, Size: 189 bytes --] ^ permalink raw reply [flat|nested] 12+ messages in thread
end of thread, other threads:[~2006-01-04 13:40 UTC | newest] Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2005-12-04 22:47 [PATCH] natsemi: NAPI support Mark Brown [not found] ` <20051204231209.GA28949@electric-eye.fr.zoreil.com> 2005-12-05 23:23 ` Mark Brown 2005-12-06 0:19 ` Francois Romieu 2005-12-06 21:17 ` Mark Brown 2005-12-06 21:56 ` Francois Romieu 2005-12-09 10:48 ` Mark Brown 2005-12-12 23:55 ` Mark Brown 2005-12-13 0:25 ` Jeff Garzik 2005-12-13 22:59 ` Mark Brown 2005-12-21 23:48 ` Resubmit: " Mark Brown 2006-01-04 7:32 ` Mark Brown 2006-01-04 13:40 ` Harald Welte
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).