netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/6] forcedeth interrupt and task overhaul, v2
@ 2007-10-17  5:52 Jeff Garzik
  2007-10-17  5:53 ` [PATCH 1/6] forcedeth: internal simplifications; changelog removal Jeff Garzik
                   ` (6 more replies)
  0 siblings, 7 replies; 8+ messages in thread
From: Jeff Garzik @ 2007-10-17  5:52 UTC (permalink / raw)
  To: netdev, LKML; +Cc: Manfred Spraul, Ingo Molnar, Ayaz Abdulla


These six changes can be found in the 'fe' branch of
git://git.kernel.org/pub/scm/linux/kernel/git/jgarzik/netdev-2.6.git

      [netdrvr] forcedeth: internal simplifications; changelog removal
      [netdrvr] forcedeth: timer overhaul
      [netdrvr] forcedeth: eliminate some duplicate irq handling code
      [netdrvr] forcedeth: unconditionally enable NAPI
      [netdrvr] forcedeth: use NAPI for TX completion
      [netdrvr] interrupt handling overhaul


I rewrote the previous forcedeth patchset, even though the direction and
methods are quite similar to the last patchset.

This hammers out bugs, problems, inconsistencies, mainly with interrupt
handling and locking.  This moves the driver towards removing the
disable_irq() usage, but does not actually take that step (yet).

A few changes just went upstream, too:

Ingo Molnar (1):
      forcedeth: fix NAPI rx poll function
Jeff Garzik (2):
      [netdrvr] forcedeth: improved probe info; dev_printk() cleanups
      [netdrvr] forcedeth: remove in-driver copy of net_device_stats

This patchset is diff'd against the latest linux-2.6.git as of an hour
ago or so.  It applies on top of the three changes listed immediately
above this paragraph.

At this point, it has been tested on

	nVidia Corporation CK804 Ethernet Controller (rev a3)

but I'm definitely interested in other testing, particularly performance
numbers (wire speed? or below?), CPU usage and memory usage info.  Most
particularly CPU usage info.  And maybe interrupt count with/without
this patchset, too.

If you send feedback, 'dmesg' and 'lspci' output is helpful.  The
current forcedeth driver in upstream (0.61) prints out useful diagnostic
information about your chip at bootup.



^ permalink raw reply	[flat|nested] 8+ messages in thread

* [PATCH 1/6] forcedeth: internal simplifications; changelog removal
  2007-10-17  5:52 [PATCH 0/6] forcedeth interrupt and task overhaul, v2 Jeff Garzik
@ 2007-10-17  5:53 ` Jeff Garzik
  2007-10-17  5:53 ` [PATCH 2/6] forcedeth: timer overhaul Jeff Garzik
                   ` (5 subsequent siblings)
  6 siblings, 0 replies; 8+ messages in thread
From: Jeff Garzik @ 2007-10-17  5:53 UTC (permalink / raw)
  To: netdev, LKML; +Cc: Manfred Spraul, Ingo Molnar, Ayaz Abdulla


commit 0aeb1f867bc76029f599f73ac757a50f7641ccc5
Author: Jeff Garzik <jeff@garzik.org>
Date:   Tue Oct 16 01:40:30 2007 -0400

    [netdrvr] forcedeth: internal simplifications; changelog removal
    
    * remove changelog from source; its kept in git repository
    
    * consolidate descriptor version tests using nv_optimized()
    
    * consolidate NIC DMA start, stop and drain into
      nv_start_txrx(), nv_stop_txrx(), nv_drain_txrx()
    
    Signed-off-by: Jeff Garzik <jgarzik@redhat.com>

 drivers/net/forcedeth.c |  234 +++++++++++++++---------------------------------
 1 file changed, 74 insertions(+), 160 deletions(-)

0aeb1f867bc76029f599f73ac757a50f7641ccc5
diff --git a/drivers/net/forcedeth.c b/drivers/net/forcedeth.c
index cfbb7aa..2d518fc 100644
--- a/drivers/net/forcedeth.c
+++ b/drivers/net/forcedeth.c
@@ -29,90 +29,6 @@
  * along with this program; if not, write to the Free Software
  * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA  02111-1307  USA
  *
- * Changelog:
- * 	0.01: 05 Oct 2003: First release that compiles without warnings.
- * 	0.02: 05 Oct 2003: Fix bug for nv_drain_tx: do not try to free NULL skbs.
- * 			   Check all PCI BARs for the register window.
- * 			   udelay added to mii_rw.
- * 	0.03: 06 Oct 2003: Initialize dev->irq.
- * 	0.04: 07 Oct 2003: Initialize np->lock, reduce handled irqs, add printks.
- * 	0.05: 09 Oct 2003: printk removed again, irq status print tx_timeout.
- * 	0.06: 10 Oct 2003: MAC Address read updated, pff flag generation updated,
- * 			   irq mask updated
- * 	0.07: 14 Oct 2003: Further irq mask updates.
- * 	0.08: 20 Oct 2003: rx_desc.Length initialization added, nv_alloc_rx refill
- * 			   added into irq handler, NULL check for drain_ring.
- * 	0.09: 20 Oct 2003: Basic link speed irq implementation. Only handle the
- * 			   requested interrupt sources.
- * 	0.10: 20 Oct 2003: First cleanup for release.
- * 	0.11: 21 Oct 2003: hexdump for tx added, rx buffer sizes increased.
- * 			   MAC Address init fix, set_multicast cleanup.
- * 	0.12: 23 Oct 2003: Cleanups for release.
- * 	0.13: 25 Oct 2003: Limit for concurrent tx packets increased to 10.
- * 			   Set link speed correctly. start rx before starting
- * 			   tx (nv_start_rx sets the link speed).
- * 	0.14: 25 Oct 2003: Nic dependant irq mask.
- * 	0.15: 08 Nov 2003: fix smp deadlock with set_multicast_list during
- * 			   open.
- * 	0.16: 15 Nov 2003: include file cleanup for ppc64, rx buffer size
- * 			   increased to 1628 bytes.
- * 	0.17: 16 Nov 2003: undo rx buffer size increase. Substract 1 from
- * 			   the tx length.
- * 	0.18: 17 Nov 2003: fix oops due to late initialization of dev_stats
- * 	0.19: 29 Nov 2003: Handle RxNoBuf, detect & handle invalid mac
- * 			   addresses, really stop rx if already running
- * 			   in nv_start_rx, clean up a bit.
- * 	0.20: 07 Dec 2003: alloc fixes
- * 	0.21: 12 Jan 2004: additional alloc fix, nic polling fix.
- *	0.22: 19 Jan 2004: reprogram timer to a sane rate, avoid lockup
- *			   on close.
- *	0.23: 26 Jan 2004: various small cleanups
- *	0.24: 27 Feb 2004: make driver even less anonymous in backtraces
- *	0.25: 09 Mar 2004: wol support
- *	0.26: 03 Jun 2004: netdriver specific annotation, sparse-related fixes
- *	0.27: 19 Jun 2004: Gigabit support, new descriptor rings,
- *			   added CK804/MCP04 device IDs, code fixes
- *			   for registers, link status and other minor fixes.
- *	0.28: 21 Jun 2004: Big cleanup, making driver mostly endian safe
- *	0.29: 31 Aug 2004: Add backup timer for link change notification.
- *	0.30: 25 Sep 2004: rx checksum support for nf 250 Gb. Add rx reset
- *			   into nv_close, otherwise reenabling for wol can
- *			   cause DMA to kfree'd memory.
- *	0.31: 14 Nov 2004: ethtool support for getting/setting link
- *			   capabilities.
- *	0.32: 16 Apr 2005: RX_ERROR4 handling added.
- *	0.33: 16 May 2005: Support for MCP51 added.
- *	0.34: 18 Jun 2005: Add DEV_NEED_LINKTIMER to all nForce nics.
- *	0.35: 26 Jun 2005: Support for MCP55 added.
- *	0.36: 28 Jun 2005: Add jumbo frame support.
- *	0.37: 10 Jul 2005: Additional ethtool support, cleanup of pci id list
- *	0.38: 16 Jul 2005: tx irq rewrite: Use global flags instead of
- *			   per-packet flags.
- *	0.39: 18 Jul 2005: Add 64bit descriptor support.
- *	0.40: 19 Jul 2005: Add support for mac address change.
- *	0.41: 30 Jul 2005: Write back original MAC in nv_close instead
- *			   of nv_remove
- *	0.42: 06 Aug 2005: Fix lack of link speed initialization
- *			   in the second (and later) nv_open call
- *	0.43: 10 Aug 2005: Add support for tx checksum.
- *	0.44: 20 Aug 2005: Add support for scatter gather and segmentation.
- *	0.45: 18 Sep 2005: Remove nv_stop/start_rx from every link check
- *	0.46: 20 Oct 2005: Add irq optimization modes.
- *	0.47: 26 Oct 2005: Add phyaddr 0 in phy scan.
- *	0.48: 24 Dec 2005: Disable TSO, bugfix for pci_map_single
- *	0.49: 10 Dec 2005: Fix tso for large buffers.
- *	0.50: 20 Jan 2006: Add 8021pq tagging support.
- *	0.51: 20 Jan 2006: Add 64bit consistent memory allocation for rings.
- *	0.52: 20 Jan 2006: Add MSI/MSIX support.
- *	0.53: 19 Mar 2006: Fix init from low power mode and add hw reset.
- *	0.54: 21 Mar 2006: Fix spin locks for multi irqs and cleanup.
- *	0.55: 22 Mar 2006: Add flow control (pause frame).
- *	0.56: 22 Mar 2006: Additional ethtool config and moduleparam support.
- *	0.57: 14 May 2006: Mac address set in probe/remove and order corrections.
- *	0.58: 30 Oct 2006: Added support for sideband management unit.
- *	0.59: 30 Oct 2006: Added support for recoverable error.
- *	0.60: 20 Jan 2007: Code optimizations for rings, rx & tx data paths, and stats.
- *
  * Known bugs:
  * We suspect that on some hardware no TX done interrupts are generated.
  * This means recovery from netif_stop_queue only happens if the hw timer
@@ -123,11 +39,6 @@
  * DEV_NEED_TIMERIRQ will not harm you on sane hardware, only generating a few
  * superfluous timer interrupts from the nic.
  */
-#ifdef CONFIG_FORCEDETH_NAPI
-#define DRIVERNAPI "-NAPI"
-#else
-#define DRIVERNAPI
-#endif
 #define FORCEDETH_VERSION		"0.61"
 #define DRV_NAME			"forcedeth"
 
@@ -908,6 +819,13 @@ static inline u32 nv_descr_getlength_ex(struct ring_desc_ex *prd, u32 v)
 	return le32_to_cpu(prd->flaglen) & LEN_MASK_V2;
 }
 
+static bool nv_optimized(struct fe_priv *np)
+{
+	if (np->desc_ver == DESC_VER_1 || np->desc_ver == DESC_VER_2)
+		return false;
+	return true;
+}
+
 static int reg_delay(struct net_device *dev, int offset, u32 mask, u32 target,
 				int delay, int delaymax, const char *msg)
 {
@@ -934,7 +852,7 @@ static void setup_hw_rings(struct net_device *dev, int rxtx_flags)
 	struct fe_priv *np = get_nvpriv(dev);
 	u8 __iomem *base = get_hwbase(dev);
 
-	if (np->desc_ver == DESC_VER_1 || np->desc_ver == DESC_VER_2) {
+	if (!nv_optimized(np)) {
 		if (rxtx_flags & NV_SETUP_RX_RING) {
 			writel((u32) cpu_to_le64(np->ring_addr), base + NvRegRxRingPhysAddr);
 		}
@@ -957,7 +875,7 @@ static void free_rings(struct net_device *dev)
 {
 	struct fe_priv *np = get_nvpriv(dev);
 
-	if (np->desc_ver == DESC_VER_1 || np->desc_ver == DESC_VER_2) {
+	if (!nv_optimized(np)) {
 		if (np->rx_ring.orig)
 			pci_free_consistent(np->pci_dev, sizeof(struct ring_desc) * (np->rx_ring_size + np->tx_ring_size),
 					    np->rx_ring.orig, np->ring_addr);
@@ -1403,6 +1321,18 @@ static void nv_stop_tx(struct net_device *dev)
 		       base + NvRegTransmitPoll);
 }
 
+static void nv_start_rxtx(struct net_device *dev)
+{
+	nv_start_rx(dev);
+	nv_start_tx(dev);
+}
+
+static void nv_stop_rxtx(struct net_device *dev)
+{
+	nv_stop_rx(dev);
+	nv_stop_tx(dev);
+}
+
 static void nv_txrx_reset(struct net_device *dev)
 {
 	struct fe_priv *np = netdev_priv(dev);
@@ -1611,7 +1541,7 @@ static void nv_do_rx_refill(unsigned long data)
 	} else {
 		disable_irq(np->msi_x_entry[NV_MSI_X_VECTOR_RX].vector);
 	}
-	if (np->desc_ver == DESC_VER_1 || np->desc_ver == DESC_VER_2)
+	if (!nv_optimized(np))
 		retcode = nv_alloc_rx(dev);
 	else
 		retcode = nv_alloc_rx_optimized(dev);
@@ -1636,8 +1566,10 @@ static void nv_init_rx(struct net_device *dev)
 {
 	struct fe_priv *np = netdev_priv(dev);
 	int i;
+
 	np->get_rx = np->put_rx = np->first_rx = np->rx_ring;
-	if (np->desc_ver == DESC_VER_1 || np->desc_ver == DESC_VER_2)
+
+	if (!nv_optimized(np))
 		np->last_rx.orig = &np->rx_ring.orig[np->rx_ring_size-1];
 	else
 		np->last_rx.ex = &np->rx_ring.ex[np->rx_ring_size-1];
@@ -1645,7 +1577,7 @@ static void nv_init_rx(struct net_device *dev)
 	np->last_rx_ctx = &np->rx_skb[np->rx_ring_size-1];
 
 	for (i = 0; i < np->rx_ring_size; i++) {
-		if (np->desc_ver == DESC_VER_1 || np->desc_ver == DESC_VER_2) {
+		if (!nv_optimized(np)) {
 			np->rx_ring.orig[i].flaglen = 0;
 			np->rx_ring.orig[i].buf = 0;
 		} else {
@@ -1663,8 +1595,10 @@ static void nv_init_tx(struct net_device *dev)
 {
 	struct fe_priv *np = netdev_priv(dev);
 	int i;
+
 	np->get_tx = np->put_tx = np->first_tx = np->tx_ring;
-	if (np->desc_ver == DESC_VER_1 || np->desc_ver == DESC_VER_2)
+
+	if (!nv_optimized(np))
 		np->last_tx.orig = &np->tx_ring.orig[np->tx_ring_size-1];
 	else
 		np->last_tx.ex = &np->tx_ring.ex[np->tx_ring_size-1];
@@ -1672,7 +1606,7 @@ static void nv_init_tx(struct net_device *dev)
 	np->last_tx_ctx = &np->tx_skb[np->tx_ring_size-1];
 
 	for (i = 0; i < np->tx_ring_size; i++) {
-		if (np->desc_ver == DESC_VER_1 || np->desc_ver == DESC_VER_2) {
+		if (!nv_optimized(np)) {
 			np->tx_ring.orig[i].flaglen = 0;
 			np->tx_ring.orig[i].buf = 0;
 		} else {
@@ -1692,7 +1626,8 @@ static int nv_init_ring(struct net_device *dev)
 
 	nv_init_tx(dev);
 	nv_init_rx(dev);
-	if (np->desc_ver == DESC_VER_1 || np->desc_ver == DESC_VER_2)
+
+	if (!nv_optimized(np))
 		return nv_alloc_rx(dev);
 	else
 		return nv_alloc_rx_optimized(dev);
@@ -1723,7 +1658,7 @@ static void nv_drain_tx(struct net_device *dev)
 	unsigned int i;
 
 	for (i = 0; i < np->tx_ring_size; i++) {
-		if (np->desc_ver == DESC_VER_1 || np->desc_ver == DESC_VER_2) {
+		if (!nv_optimized(np)) {
 			np->tx_ring.orig[i].flaglen = 0;
 			np->tx_ring.orig[i].buf = 0;
 		} else {
@@ -1743,7 +1678,7 @@ static void nv_drain_rx(struct net_device *dev)
 	int i;
 
 	for (i = 0; i < np->rx_ring_size; i++) {
-		if (np->desc_ver == DESC_VER_1 || np->desc_ver == DESC_VER_2) {
+		if (!nv_optimized(np)) {
 			np->rx_ring.orig[i].flaglen = 0;
 			np->rx_ring.orig[i].buf = 0;
 		} else {
@@ -1764,7 +1699,7 @@ static void nv_drain_rx(struct net_device *dev)
 	}
 }
 
-static void drain_ring(struct net_device *dev)
+static void nv_drain_rxtx(struct net_device *dev)
 {
 	nv_drain_tx(dev);
 	nv_drain_rx(dev);
@@ -2155,7 +2090,7 @@ static void nv_tx_timeout(struct net_device *dev)
 		}
 		printk(KERN_INFO "%s: Dumping tx ring\n", dev->name);
 		for (i=0;i<np->tx_ring_size;i+= 4) {
-			if (np->desc_ver == DESC_VER_1 || np->desc_ver == DESC_VER_2) {
+			if (!nv_optimized(np)) {
 				printk(KERN_INFO "%03x: %08x %08x // %08x %08x // %08x %08x // %08x %08x\n",
 				       i,
 				       le32_to_cpu(np->tx_ring.orig[i].buf),
@@ -2191,7 +2126,7 @@ static void nv_tx_timeout(struct net_device *dev)
 	nv_stop_tx(dev);
 
 	/* 2) check that the packets were not sent already: */
-	if (np->desc_ver == DESC_VER_1 || np->desc_ver == DESC_VER_2)
+	if (!nv_optimized(np))
 		nv_tx_done(dev);
 	else
 		nv_tx_done_optimized(dev, np->tx_ring_size);
@@ -2566,12 +2501,10 @@ static int nv_change_mtu(struct net_device *dev, int new_mtu)
 		netif_tx_lock_bh(dev);
 		spin_lock(&np->lock);
 		/* stop engines */
-		nv_stop_rx(dev);
-		nv_stop_tx(dev);
+		nv_stop_rxtx(dev);
 		nv_txrx_reset(dev);
 		/* drain rx queue */
-		nv_drain_rx(dev);
-		nv_drain_tx(dev);
+		nv_drain_rxtx(dev);
 		/* reinit driver view of the rx queue */
 		set_bufsize(dev);
 		if (nv_init_ring(dev)) {
@@ -2588,8 +2521,7 @@ static int nv_change_mtu(struct net_device *dev, int new_mtu)
 		pci_push(base);
 
 		/* restart rx engine */
-		nv_start_rx(dev);
-		nv_start_tx(dev);
+		nv_start_rxtx(dev);
 		spin_unlock(&np->lock);
 		netif_tx_unlock_bh(dev);
 		nv_enable_irq(dev);
@@ -3259,7 +3191,7 @@ static int nv_napi_poll(struct napi_struct *napi, int budget)
 	unsigned long flags;
 	int pkts, retcode;
 
-	if (np->desc_ver == DESC_VER_1 || np->desc_ver == DESC_VER_2) {
+	if (!nv_optimized(np)) {
 		pkts = nv_rx_process(dev, budget);
 		retcode = nv_alloc_rx(dev);
 	} else {
@@ -3500,7 +3432,7 @@ static int nv_request_irq(struct net_device *dev, int intr_test)
 	if (intr_test) {
 		handler = nv_nic_irq_test;
 	} else {
-		if (np->desc_ver == DESC_VER_3)
+		if (nv_optimized(np))
 			handler = nv_nic_irq_optimized;
 		else
 			handler = nv_nic_irq;
@@ -3649,12 +3581,10 @@ static void nv_do_nic_poll(unsigned long data)
 			netif_tx_lock_bh(dev);
 			spin_lock(&np->lock);
 			/* stop engines */
-			nv_stop_rx(dev);
-			nv_stop_tx(dev);
+			nv_stop_rxtx(dev);
 			nv_txrx_reset(dev);
 			/* drain rx queue */
-			nv_drain_rx(dev);
-			nv_drain_tx(dev);
+			nv_drain_rxtx(dev);
 			/* reinit driver view of the rx queue */
 			set_bufsize(dev);
 			if (nv_init_ring(dev)) {
@@ -3671,8 +3601,7 @@ static void nv_do_nic_poll(unsigned long data)
 			pci_push(base);
 
 			/* restart rx engine */
-			nv_start_rx(dev);
-			nv_start_tx(dev);
+			nv_start_rxtx(dev);
 			spin_unlock(&np->lock);
 			netif_tx_unlock_bh(dev);
 		}
@@ -3684,7 +3613,7 @@ static void nv_do_nic_poll(unsigned long data)
 	pci_push(base);
 
 	if (!using_multi_irqs(dev)) {
-		if (np->desc_ver == DESC_VER_3)
+		if (nv_optimized(np))
 			nv_nic_irq_optimized(0, dev);
 		else
 			nv_nic_irq(0, dev);
@@ -3881,8 +3810,7 @@ static int nv_set_settings(struct net_device *dev, struct ethtool_cmd *ecmd)
 		netif_tx_lock_bh(dev);
 		spin_lock(&np->lock);
 		/* stop engines */
-		nv_stop_rx(dev);
-		nv_stop_tx(dev);
+		nv_stop_rxtx(dev);
 		spin_unlock(&np->lock);
 		netif_tx_unlock_bh(dev);
 	}
@@ -3988,8 +3916,7 @@ static int nv_set_settings(struct net_device *dev, struct ethtool_cmd *ecmd)
 	}
 
 	if (netif_running(dev)) {
-		nv_start_rx(dev);
-		nv_start_tx(dev);
+		nv_start_rxtx(dev);
 		nv_enable_irq(dev);
 	}
 
@@ -4032,8 +3959,7 @@ static int nv_nway_reset(struct net_device *dev)
 			netif_tx_lock_bh(dev);
 			spin_lock(&np->lock);
 			/* stop engines */
-			nv_stop_rx(dev);
-			nv_stop_tx(dev);
+			nv_stop_rxtx(dev);
 			spin_unlock(&np->lock);
 			netif_tx_unlock_bh(dev);
 			printk(KERN_INFO "%s: link down.\n", dev->name);
@@ -4053,8 +3979,7 @@ static int nv_nway_reset(struct net_device *dev)
 		}
 
 		if (netif_running(dev)) {
-			nv_start_rx(dev);
-			nv_start_tx(dev);
+			nv_start_rxtx(dev);
 			nv_enable_irq(dev);
 		}
 		ret = 0;
@@ -4111,7 +4036,7 @@ static int nv_set_ringparam(struct net_device *dev, struct ethtool_ringparam* ri
 	}
 
 	/* allocate new rings */
-	if (np->desc_ver == DESC_VER_1 || np->desc_ver == DESC_VER_2) {
+	if (!nv_optimized(np)) {
 		rxtx_ring = pci_alloc_consistent(np->pci_dev,
 					    sizeof(struct ring_desc) * (ring->rx_pending + ring->tx_pending),
 					    &ring_addr);
@@ -4124,7 +4049,7 @@ static int nv_set_ringparam(struct net_device *dev, struct ethtool_ringparam* ri
 	tx_skbuff = kmalloc(sizeof(struct nv_skb_map) * ring->tx_pending, GFP_KERNEL);
 	if (!rxtx_ring || !rx_skbuff || !tx_skbuff) {
 		/* fall back to old rings */
-		if (np->desc_ver == DESC_VER_1 || np->desc_ver == DESC_VER_2) {
+		if (!nv_optimized(np)) {
 			if (rxtx_ring)
 				pci_free_consistent(np->pci_dev, sizeof(struct ring_desc) * (ring->rx_pending + ring->tx_pending),
 						    rxtx_ring, ring_addr);
@@ -4145,12 +4070,10 @@ static int nv_set_ringparam(struct net_device *dev, struct ethtool_ringparam* ri
 		netif_tx_lock_bh(dev);
 		spin_lock(&np->lock);
 		/* stop engines */
-		nv_stop_rx(dev);
-		nv_stop_tx(dev);
+		nv_stop_rxtx(dev);
 		nv_txrx_reset(dev);
 		/* drain queues */
-		nv_drain_rx(dev);
-		nv_drain_tx(dev);
+		nv_drain_rxtx(dev);
 		/* delete queues */
 		free_rings(dev);
 	}
@@ -4158,7 +4081,8 @@ static int nv_set_ringparam(struct net_device *dev, struct ethtool_ringparam* ri
 	/* set new values */
 	np->rx_ring_size = ring->rx_pending;
 	np->tx_ring_size = ring->tx_pending;
-	if (np->desc_ver == DESC_VER_1 || np->desc_ver == DESC_VER_2) {
+
+	if (!nv_optimized(np)) {
 		np->rx_ring.orig = (struct ring_desc*)rxtx_ring;
 		np->tx_ring.orig = &np->rx_ring.orig[np->rx_ring_size];
 	} else {
@@ -4190,8 +4114,7 @@ static int nv_set_ringparam(struct net_device *dev, struct ethtool_ringparam* ri
 		pci_push(base);
 
 		/* restart engines */
-		nv_start_rx(dev);
-		nv_start_tx(dev);
+		nv_start_rxtx(dev);
 		spin_unlock(&np->lock);
 		netif_tx_unlock_bh(dev);
 		nv_enable_irq(dev);
@@ -4232,8 +4155,7 @@ static int nv_set_pauseparam(struct net_device *dev, struct ethtool_pauseparam*
 		netif_tx_lock_bh(dev);
 		spin_lock(&np->lock);
 		/* stop engines */
-		nv_stop_rx(dev);
-		nv_stop_tx(dev);
+		nv_stop_rxtx(dev);
 		spin_unlock(&np->lock);
 		netif_tx_unlock_bh(dev);
 	}
@@ -4274,8 +4196,7 @@ static int nv_set_pauseparam(struct net_device *dev, struct ethtool_pauseparam*
 	}
 
 	if (netif_running(dev)) {
-		nv_start_rx(dev);
-		nv_start_tx(dev);
+		nv_start_rxtx(dev);
 		nv_enable_irq(dev);
 	}
 	return 0;
@@ -4511,8 +4432,7 @@ static int nv_loopback_test(struct net_device *dev)
 	pci_push(base);
 
 	/* restart rx engine */
-	nv_start_rx(dev);
-	nv_start_tx(dev);
+	nv_start_rxtx(dev);
 
 	/* setup packet for tx */
 	pkt_len = ETH_DATA_LEN;
@@ -4530,7 +4450,7 @@ static int nv_loopback_test(struct net_device *dev)
 	for (i = 0; i < pkt_len; i++)
 		pkt_data[i] = (u8)(i & 0xff);
 
-	if (np->desc_ver == DESC_VER_1 || np->desc_ver == DESC_VER_2) {
+	if (!nv_optimized(np)) {
 		np->tx_ring.orig[0].buf = cpu_to_le32(test_dma_addr);
 		np->tx_ring.orig[0].flaglen = cpu_to_le32((pkt_len-1) | np->tx_flags | tx_flags_extra);
 	} else {
@@ -4544,7 +4464,7 @@ static int nv_loopback_test(struct net_device *dev)
 	msleep(500);
 
 	/* check for rx of the packet */
-	if (np->desc_ver == DESC_VER_1 || np->desc_ver == DESC_VER_2) {
+	if (!nv_optimized(np)) {
 		flags = le32_to_cpu(np->rx_ring.orig[0].flaglen);
 		len = nv_descr_getlength(&np->rx_ring.orig[0], np->desc_ver);
 
@@ -4590,12 +4510,10 @@ static int nv_loopback_test(struct net_device *dev)
 	dev_kfree_skb_any(tx_skb);
  out:
 	/* stop engines */
-	nv_stop_rx(dev);
-	nv_stop_tx(dev);
+	nv_stop_rxtx(dev);
 	nv_txrx_reset(dev);
 	/* drain rx queue */
-	nv_drain_rx(dev);
-	nv_drain_tx(dev);
+	nv_drain_rxtx(dev);
 
 	if (netif_running(dev)) {
 		writel(misc1_flags, base + NvRegMisc1);
@@ -4633,12 +4551,10 @@ static void nv_self_test(struct net_device *dev, struct ethtool_test *test, u64
 				writel(NVREG_IRQSTAT_MASK, base + NvRegMSIXIrqStatus);
 			}
 			/* stop engines */
-			nv_stop_rx(dev);
-			nv_stop_tx(dev);
+			nv_stop_rxtx(dev);
 			nv_txrx_reset(dev);
 			/* drain rx queue */
-			nv_drain_rx(dev);
-			nv_drain_tx(dev);
+			nv_drain_rxtx(dev);
 			spin_unlock_irq(&np->lock);
 			netif_tx_unlock_bh(dev);
 		}
@@ -4679,8 +4595,7 @@ static void nv_self_test(struct net_device *dev, struct ethtool_test *test, u64
 			writel(NVREG_TXRXCTL_KICK|np->txrxctl_bits, get_hwbase(dev) + NvRegTxRxControl);
 			pci_push(base);
 			/* restart rx engine */
-			nv_start_rx(dev);
-			nv_start_tx(dev);
+			nv_start_rxtx(dev);
 			netif_start_queue(dev);
 #ifdef CONFIG_FORCEDETH_NAPI
 			napi_enable(&np->napi);
@@ -4909,8 +4824,7 @@ static int nv_open(struct net_device *dev)
 	 * to init hw */
 	np->linkspeed = 0;
 	ret = nv_update_linkspeed(dev);
-	nv_start_rx(dev);
-	nv_start_tx(dev);
+	nv_start_rxtx(dev);
 	netif_start_queue(dev);
 #ifdef CONFIG_FORCEDETH_NAPI
 	napi_enable(&np->napi);
@@ -4933,7 +4847,7 @@ static int nv_open(struct net_device *dev)
 
 	return 0;
 out_drain:
-	drain_ring(dev);
+	nv_drain_rxtx(dev);
 	return ret;
 }
 
@@ -4956,8 +4870,7 @@ static int nv_close(struct net_device *dev)
 
 	netif_stop_queue(dev);
 	spin_lock_irq(&np->lock);
-	nv_stop_tx(dev);
-	nv_stop_rx(dev);
+	nv_stop_rxtx(dev);
 	nv_txrx_reset(dev);
 
 	/* disable interrupts on the nic or we will lock up */
@@ -4970,7 +4883,7 @@ static int nv_close(struct net_device *dev)
 
 	nv_free_irq(dev);
 
-	drain_ring(dev);
+	nv_drain_rxtx(dev);
 
 	if (np->wolenabled) {
 		writel(NVREG_PFF_ALWAYS|NVREG_PFF_MYADDR, base + NvRegPacketFilterFlags);
@@ -5128,7 +5041,7 @@ static int __devinit nv_probe(struct pci_dev *pci_dev, const struct pci_device_i
 	np->rx_ring_size = RX_RING_DEFAULT;
 	np->tx_ring_size = TX_RING_DEFAULT;
 
-	if (np->desc_ver == DESC_VER_1 || np->desc_ver == DESC_VER_2) {
+	if (!nv_optimized(np)) {
 		np->rx_ring.orig = pci_alloc_consistent(pci_dev,
 					sizeof(struct ring_desc) * (np->rx_ring_size + np->tx_ring_size),
 					&np->ring_addr);
@@ -5150,7 +5063,8 @@ static int __devinit nv_probe(struct pci_dev *pci_dev, const struct pci_device_i
 
 	dev->open = nv_open;
 	dev->stop = nv_close;
-	if (np->desc_ver == DESC_VER_1 || np->desc_ver == DESC_VER_2)
+
+	if (!nv_optimized(np))
 		dev->hard_start_xmit = nv_start_xmit;
 	else
 		dev->hard_start_xmit = nv_start_xmit_optimized;

^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [PATCH 2/6] forcedeth: timer overhaul
  2007-10-17  5:52 [PATCH 0/6] forcedeth interrupt and task overhaul, v2 Jeff Garzik
  2007-10-17  5:53 ` [PATCH 1/6] forcedeth: internal simplifications; changelog removal Jeff Garzik
@ 2007-10-17  5:53 ` Jeff Garzik
  2007-10-17  5:54 ` [PATCH 3/6] forcedeth: eliminate some duplicate irq handling code Jeff Garzik
                   ` (4 subsequent siblings)
  6 siblings, 0 replies; 8+ messages in thread
From: Jeff Garzik @ 2007-10-17  5:53 UTC (permalink / raw)
  To: netdev, LKML; +Cc: Manfred Spraul, Ingo Molnar, Ayaz Abdulla


commit 160511126b6be7f15da33f7cab7374b12cb59999
Author: Jeff Garzik <jeff@garzik.org>
Date:   Tue Oct 16 02:22:39 2007 -0400

    [netdrvr] forcedeth: timer overhaul
    
    * remove np->in_shutdown, it mirrors netif_running()
    
    * convert stats timer to delayed workqueue
    
    * retrieve hw stats inside spinlock
    
    * split out the 'stop' portions of nv_stop_rx() and nv_stop_rx(),
      to enable future calling of these operations without the reg_delay()
      that immediately follows.
    
    Signed-off-by: Jeff Garzik <jgarzik@redhat.com>

 drivers/net/forcedeth.c |  135 +++++++++++++++++++++++++++---------------------
 1 file changed, 77 insertions(+), 58 deletions(-)

160511126b6be7f15da33f7cab7374b12cb59999
diff --git a/drivers/net/forcedeth.c b/drivers/net/forcedeth.c
index 2d518fc..39ade56 100644
--- a/drivers/net/forcedeth.c
+++ b/drivers/net/forcedeth.c
@@ -664,7 +664,7 @@ struct fe_priv {
 	/* General data:
 	 * Locking: spin_lock(&np->lock); */
 	struct nv_ethtool_stats estats;
-	int in_shutdown;
+
 	u32 linkspeed;
 	int duplex;
 	int autoneg;
@@ -705,7 +705,7 @@ struct fe_priv {
 	unsigned int pkt_limit;
 	struct timer_list oom_kick;
 	struct timer_list nic_poll;
-	struct timer_list stats_poll;
+	struct delayed_work stats_task;
 	u32 nic_poll_irq;
 	int rx_ring_size;
 
@@ -1264,18 +1264,28 @@ static void nv_start_rx(struct net_device *dev)
 	pci_push(base);
 }
 
-static void nv_stop_rx(struct net_device *dev)
+static void __nv_stop_rx(struct fe_priv *np)
 {
-	struct fe_priv *np = netdev_priv(dev);
-	u8 __iomem *base = get_hwbase(dev);
+	u8 __iomem *base = np->base;
 	u32 rx_ctrl = readl(base + NvRegReceiverControl);
 
-	dprintk(KERN_DEBUG "%s: nv_stop_rx\n", dev->name);
+	dprintk(KERN_DEBUG "%s: __nv_stop_rx\n", dev->name);
 	if (!np->mac_in_use)
 		rx_ctrl &= ~NVREG_RCVCTL_START;
 	else
 		rx_ctrl |= NVREG_RCVCTL_RX_PATH_EN;
 	writel(rx_ctrl, base + NvRegReceiverControl);
+}
+
+static void nv_stop_rx(struct net_device *dev)
+{
+	struct fe_priv *np = netdev_priv(dev);
+	u8 __iomem *base = get_hwbase(dev);
+
+	dprintk(KERN_DEBUG "%s: nv_stop_rx\n", dev->name);
+
+	__nv_stop_rx(np);
+
 	reg_delay(dev, NvRegReceiverStatus, NVREG_RCVSTAT_BUSY, 0,
 			NV_RXSTOP_DELAY1, NV_RXSTOP_DELAY1MAX,
 			KERN_INFO "nv_stop_rx: ReceiverStatus remained busy");
@@ -1299,18 +1309,29 @@ static void nv_start_tx(struct net_device *dev)
 	pci_push(base);
 }
 
-static void nv_stop_tx(struct net_device *dev)
+static void __nv_stop_tx(struct fe_priv *np)
 {
-	struct fe_priv *np = netdev_priv(dev);
-	u8 __iomem *base = get_hwbase(dev);
+	u8 __iomem *base = np->base;
 	u32 tx_ctrl = readl(base + NvRegTransmitterControl);
 
-	dprintk(KERN_DEBUG "%s: nv_stop_tx\n", dev->name);
+	dprintk(KERN_DEBUG "%s: __nv_stop_tx\n", dev->name);
+
 	if (!np->mac_in_use)
 		tx_ctrl &= ~NVREG_XMITCTL_START;
 	else
 		tx_ctrl |= NVREG_XMITCTL_TX_PATH_EN;
 	writel(tx_ctrl, base + NvRegTransmitterControl);
+}
+
+static void nv_stop_tx(struct net_device *dev)
+{
+	struct fe_priv *np = netdev_priv(dev);
+	u8 __iomem *base = get_hwbase(dev);
+
+	dprintk(KERN_DEBUG "%s: nv_stop_tx\n", dev->name);
+
+	__nv_stop_tx(np);
+
 	reg_delay(dev, NvRegTransmitterStatus, NVREG_XMITSTAT_BUSY, 0,
 			NV_TXSTOP_DELAY1, NV_TXSTOP_DELAY1MAX,
 			KERN_INFO "nv_stop_tx: TransmitterStatus remained busy");
@@ -1364,10 +1385,9 @@ static void nv_mac_reset(struct net_device *dev)
 	pci_push(base);
 }
 
-static void nv_get_hw_stats(struct net_device *dev)
+static void __nv_get_hw_stats(struct fe_priv *np)
 {
-	struct fe_priv *np = netdev_priv(dev);
-	u8 __iomem *base = get_hwbase(dev);
+	u8 __iomem *base = np->base;
 
 	np->estats.tx_bytes += readl(base + NvRegTxCnt);
 	np->estats.tx_zero_rexmt += readl(base + NvRegTxZeroReXmt);
@@ -1419,6 +1439,15 @@ static void nv_get_hw_stats(struct net_device *dev)
 	}
 }
 
+static void nv_get_hw_stats(struct fe_priv *np)
+{
+	unsigned long flags;
+
+	spin_lock_irqsave(&np->lock, flags);
+	__nv_get_hw_stats(np);
+	spin_unlock_irqrestore(&np->lock, flags);
+}
+
 /*
  * nv_get_stats: dev->get_stats function
  * Get latest stats value from the nic.
@@ -1431,7 +1460,7 @@ static struct net_device_stats *nv_get_stats(struct net_device *dev)
 
 	/* If the nic supports hw counters then retrieve latest values */
 	if (np->driver_data & (DEV_HAS_STATISTICS_V1|DEV_HAS_STATISTICS_V2)) {
-		nv_get_hw_stats(dev);
+		nv_get_hw_stats(np);
 
 		/* copy to net_device stats */
 		dev->stats.tx_bytes = np->estats.tx_bytes;
@@ -1547,7 +1576,7 @@ static void nv_do_rx_refill(unsigned long data)
 		retcode = nv_alloc_rx_optimized(dev);
 	if (retcode) {
 		spin_lock_irq(&np->lock);
-		if (!np->in_shutdown)
+		if (netif_running(dev))
 			mod_timer(&np->oom_kick, jiffies + OOM_REFILL);
 		spin_unlock_irq(&np->lock);
 	}
@@ -2507,10 +2536,9 @@ static int nv_change_mtu(struct net_device *dev, int new_mtu)
 		nv_drain_rxtx(dev);
 		/* reinit driver view of the rx queue */
 		set_bufsize(dev);
-		if (nv_init_ring(dev)) {
-			if (!np->in_shutdown)
-				mod_timer(&np->oom_kick, jiffies + OOM_REFILL);
-		}
+		if (nv_init_ring(dev))
+			mod_timer(&np->oom_kick, jiffies + OOM_REFILL);
+
 		/* reinit nic view of the rx queue */
 		writel(np->rx_buf_sz, base + NvRegOffloadConfig);
 		setup_hw_rings(dev, NV_SETUP_RX_RING | NV_SETUP_TX_RING);
@@ -2953,7 +2981,7 @@ static irqreturn_t nv_nic_irq(int foo, void *data)
 		if (nv_rx_process(dev, RX_WORK_PER_LOOP)) {
 			if (unlikely(nv_alloc_rx(dev))) {
 				spin_lock(&np->lock);
-				if (!np->in_shutdown)
+				if (netif_running(dev))
 					mod_timer(&np->oom_kick, jiffies + OOM_REFILL);
 				spin_unlock(&np->lock);
 			}
@@ -2987,7 +3015,7 @@ static irqreturn_t nv_nic_irq(int foo, void *data)
 				writel(np->irqmask, base + NvRegIrqMask);
 			pci_push(base);
 
-			if (!np->in_shutdown) {
+			if (netif_running(dev)) {
 				np->nic_poll_irq = np->irqmask;
 				np->recover_error = 1;
 				mod_timer(&np->nic_poll, jiffies + POLL_WAIT);
@@ -3004,7 +3032,7 @@ static irqreturn_t nv_nic_irq(int foo, void *data)
 				writel(np->irqmask, base + NvRegIrqMask);
 			pci_push(base);
 
-			if (!np->in_shutdown) {
+			if (netif_running(dev)) {
 				np->nic_poll_irq = np->irqmask;
 				mod_timer(&np->nic_poll, jiffies + POLL_WAIT);
 			}
@@ -3068,7 +3096,7 @@ static irqreturn_t nv_nic_irq_optimized(int foo, void *data)
 		if (nv_rx_process_optimized(dev, RX_WORK_PER_LOOP)) {
 			if (unlikely(nv_alloc_rx_optimized(dev))) {
 				spin_lock(&np->lock);
-				if (!np->in_shutdown)
+				if (netif_running(dev))
 					mod_timer(&np->oom_kick, jiffies + OOM_REFILL);
 				spin_unlock(&np->lock);
 			}
@@ -3102,7 +3130,7 @@ static irqreturn_t nv_nic_irq_optimized(int foo, void *data)
 				writel(np->irqmask, base + NvRegIrqMask);
 			pci_push(base);
 
-			if (!np->in_shutdown) {
+			if (netif_running(dev)) {
 				np->nic_poll_irq = np->irqmask;
 				np->recover_error = 1;
 				mod_timer(&np->nic_poll, jiffies + POLL_WAIT);
@@ -3120,7 +3148,7 @@ static irqreturn_t nv_nic_irq_optimized(int foo, void *data)
 				writel(np->irqmask, base + NvRegIrqMask);
 			pci_push(base);
 
-			if (!np->in_shutdown) {
+			if (netif_running(dev)) {
 				np->nic_poll_irq = np->irqmask;
 				mod_timer(&np->nic_poll, jiffies + POLL_WAIT);
 			}
@@ -3167,7 +3195,7 @@ static irqreturn_t nv_nic_irq_tx(int foo, void *data)
 			writel(NVREG_IRQ_TX_ALL, base + NvRegIrqMask);
 			pci_push(base);
 
-			if (!np->in_shutdown) {
+			if (netif_running(dev)) {
 				np->nic_poll_irq |= NVREG_IRQ_TX_ALL;
 				mod_timer(&np->nic_poll, jiffies + POLL_WAIT);
 			}
@@ -3201,7 +3229,7 @@ static int nv_napi_poll(struct napi_struct *napi, int budget)
 
 	if (retcode) {
 		spin_lock_irqsave(&np->lock, flags);
-		if (!np->in_shutdown)
+		if (netif_running(dev))
 			mod_timer(&np->oom_kick, jiffies + OOM_REFILL);
 		spin_unlock_irqrestore(&np->lock, flags);
 	}
@@ -3265,7 +3293,7 @@ static irqreturn_t nv_nic_irq_rx(int foo, void *data)
 		if (nv_rx_process_optimized(dev, RX_WORK_PER_LOOP)) {
 			if (unlikely(nv_alloc_rx_optimized(dev))) {
 				spin_lock_irqsave(&np->lock, flags);
-				if (!np->in_shutdown)
+				if (netif_running(dev))
 					mod_timer(&np->oom_kick, jiffies + OOM_REFILL);
 				spin_unlock_irqrestore(&np->lock, flags);
 			}
@@ -3277,7 +3305,7 @@ static irqreturn_t nv_nic_irq_rx(int foo, void *data)
 			writel(NVREG_IRQ_RX_ALL, base + NvRegIrqMask);
 			pci_push(base);
 
-			if (!np->in_shutdown) {
+			if (netif_running(dev)) {
 				np->nic_poll_irq |= NVREG_IRQ_RX_ALL;
 				mod_timer(&np->nic_poll, jiffies + POLL_WAIT);
 			}
@@ -3332,7 +3360,7 @@ static irqreturn_t nv_nic_irq_other(int foo, void *data)
 			writel(NVREG_IRQ_OTHER, base + NvRegIrqMask);
 			pci_push(base);
 
-			if (!np->in_shutdown) {
+			if (netif_running(dev)) {
 				np->nic_poll_irq |= NVREG_IRQ_OTHER;
 				np->recover_error = 1;
 				mod_timer(&np->nic_poll, jiffies + POLL_WAIT);
@@ -3350,7 +3378,7 @@ static irqreturn_t nv_nic_irq_other(int foo, void *data)
 			writel(NVREG_IRQ_OTHER, base + NvRegIrqMask);
 			pci_push(base);
 
-			if (!np->in_shutdown) {
+			if (netif_running(dev)) {
 				np->nic_poll_irq |= NVREG_IRQ_OTHER;
 				mod_timer(&np->nic_poll, jiffies + POLL_WAIT);
 			}
@@ -3587,10 +3615,9 @@ static void nv_do_nic_poll(unsigned long data)
 			nv_drain_rxtx(dev);
 			/* reinit driver view of the rx queue */
 			set_bufsize(dev);
-			if (nv_init_ring(dev)) {
-				if (!np->in_shutdown)
-					mod_timer(&np->oom_kick, jiffies + OOM_REFILL);
-			}
+			if (nv_init_ring(dev))
+				mod_timer(&np->oom_kick, jiffies + OOM_REFILL);
+
 			/* reinit nic view of the rx queue */
 			writel(np->rx_buf_sz, base + NvRegOffloadConfig);
 			setup_hw_rings(dev, NV_SETUP_RX_RING | NV_SETUP_TX_RING);
@@ -3644,15 +3671,14 @@ static void nv_poll_controller(struct net_device *dev)
 }
 #endif
 
-static void nv_do_stats_poll(unsigned long data)
+static void nv_stats_task(struct work_struct *work)
 {
-	struct net_device *dev = (struct net_device *) data;
-	struct fe_priv *np = netdev_priv(dev);
+	struct fe_priv *np =
+		container_of(work, struct fe_priv, stats_task.work);
 
-	nv_get_hw_stats(dev);
+	nv_get_hw_stats(np);
 
-	if (!np->in_shutdown)
-		mod_timer(&np->stats_poll, jiffies + STATS_INTERVAL);
+	schedule_delayed_work(&np->stats_task, STATS_INTERVAL);
 }
 
 static void nv_get_drvinfo(struct net_device *dev, struct ethtool_drvinfo *info)
@@ -4099,10 +4125,8 @@ static int nv_set_ringparam(struct net_device *dev, struct ethtool_ringparam* ri
 	if (netif_running(dev)) {
 		/* reinit driver view of the queues */
 		set_bufsize(dev);
-		if (nv_init_ring(dev)) {
-			if (!np->in_shutdown)
-				mod_timer(&np->oom_kick, jiffies + OOM_REFILL);
-		}
+		if (nv_init_ring(dev))
+			mod_timer(&np->oom_kick, jiffies + OOM_REFILL);
 
 		/* reinit nic view of the queues */
 		writel(np->rx_buf_sz, base + NvRegOffloadConfig);
@@ -4283,7 +4307,7 @@ static void nv_get_ethtool_stats(struct net_device *dev, struct ethtool_stats *e
 	struct fe_priv *np = netdev_priv(dev);
 
 	/* update stats */
-	nv_do_stats_poll((unsigned long)dev);
+	nv_get_hw_stats(np);
 
 	memcpy(buffer, &np->estats, nv_get_sset_count(dev, ETH_SS_STATS)*sizeof(u64));
 }
@@ -4582,10 +4606,9 @@ static void nv_self_test(struct net_device *dev, struct ethtool_test *test, u64
 		if (netif_running(dev)) {
 			/* reinit driver view of the rx queue */
 			set_bufsize(dev);
-			if (nv_init_ring(dev)) {
-				if (!np->in_shutdown)
-					mod_timer(&np->oom_kick, jiffies + OOM_REFILL);
-			}
+			if (nv_init_ring(dev))
+				mod_timer(&np->oom_kick, jiffies + OOM_REFILL);
+
 			/* reinit nic view of the rx queue */
 			writel(np->rx_buf_sz, base + NvRegOffloadConfig);
 			setup_hw_rings(dev, NV_SETUP_RX_RING | NV_SETUP_TX_RING);
@@ -4734,8 +4757,6 @@ static int nv_open(struct net_device *dev)
 	nv_txrx_reset(dev);
 	writel(0, base + NvRegUnknownSetupReg6);
 
-	np->in_shutdown = 0;
-
 	/* give hw rings */
 	setup_hw_rings(dev, NV_SETUP_RX_RING | NV_SETUP_TX_RING);
 	writel( ((np->rx_ring_size-1) << NVREG_RINGSZ_RXSHIFT) + ((np->tx_ring_size-1) << NVREG_RINGSZ_TXSHIFT),
@@ -4841,7 +4862,7 @@ static int nv_open(struct net_device *dev)
 
 	/* start statistics timer */
 	if (np->driver_data & (DEV_HAS_STATISTICS_V1|DEV_HAS_STATISTICS_V2))
-		mod_timer(&np->stats_poll, jiffies + STATS_INTERVAL);
+		schedule_delayed_work(&np->stats_task, STATS_INTERVAL);
 
 	spin_unlock_irq(&np->lock);
 
@@ -4857,8 +4878,8 @@ static int nv_close(struct net_device *dev)
 	u8 __iomem *base;
 
 	spin_lock_irq(&np->lock);
-	np->in_shutdown = 1;
 	spin_unlock_irq(&np->lock);
+
 #ifdef CONFIG_FORCEDETH_NAPI
 	napi_disable(&np->napi);
 #endif
@@ -4866,7 +4887,7 @@ static int nv_close(struct net_device *dev)
 
 	del_timer_sync(&np->oom_kick);
 	del_timer_sync(&np->nic_poll);
-	del_timer_sync(&np->stats_poll);
+	cancel_delayed_work_sync(&np->stats_task);
 
 	netif_stop_queue(dev);
 	spin_lock_irq(&np->lock);
@@ -4929,9 +4950,7 @@ static int __devinit nv_probe(struct pci_dev *pci_dev, const struct pci_device_i
 	init_timer(&np->nic_poll);
 	np->nic_poll.data = (unsigned long) dev;
 	np->nic_poll.function = &nv_do_nic_poll;	/* timer handler */
-	init_timer(&np->stats_poll);
-	np->stats_poll.data = (unsigned long) dev;
-	np->stats_poll.function = &nv_do_stats_poll;	/* timer handler */
+	INIT_DELAYED_WORK(&np->stats_task, nv_stats_task);
 
 	err = pci_enable_device(pci_dev);
 	if (err)

^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [PATCH 3/6] forcedeth: eliminate some duplicate irq handling code
  2007-10-17  5:52 [PATCH 0/6] forcedeth interrupt and task overhaul, v2 Jeff Garzik
  2007-10-17  5:53 ` [PATCH 1/6] forcedeth: internal simplifications; changelog removal Jeff Garzik
  2007-10-17  5:53 ` [PATCH 2/6] forcedeth: timer overhaul Jeff Garzik
@ 2007-10-17  5:54 ` Jeff Garzik
  2007-10-17  5:54 ` [PATCH 4/6] forcedeth: unconditionally enable NAPI Jeff Garzik
                   ` (3 subsequent siblings)
  6 siblings, 0 replies; 8+ messages in thread
From: Jeff Garzik @ 2007-10-17  5:54 UTC (permalink / raw)
  To: netdev, LKML; +Cc: Manfred Spraul, Ingo Molnar, Ayaz Abdulla


commit c6ad879c65e6f91c7f61b86936e2ea39b16711da
Author: Jeff Garzik <jeff@garzik.org>
Date:   Tue Oct 16 11:43:27 2007 -0400

    [netdrvr] forcedeth: eliminate some duplicate irq handling code
    
    * nv_nic_irq_optimized() is the exactly same as nv_nic_irq(), save
      for three function calls.  Consolidate together into a single function
      __nv_nic_irq().
    
    * remove pointless casts from void* in other irq handling code
    
    Signed-off-by: Jeff Garzik <jgarzik@redhat.com>

 drivers/net/forcedeth.c |  167 ++++++++++--------------------------------------
 1 file changed, 38 insertions(+), 129 deletions(-)

c6ad879c65e6f91c7f61b86936e2ea39b16711da
diff --git a/drivers/net/forcedeth.c b/drivers/net/forcedeth.c
index 39ade56..a4baad7 100644
--- a/drivers/net/forcedeth.c
+++ b/drivers/net/forcedeth.c
@@ -2937,15 +2937,14 @@ 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(int foo, void *data)
+static irqreturn_t __nv_nic_irq(struct net_device *dev, bool optimized)
 {
-	struct net_device *dev = (struct net_device *) data;
 	struct fe_priv *np = netdev_priv(dev);
-	u8 __iomem *base = get_hwbase(dev);
+	u8 __iomem *base = np->base;
 	u32 events;
 	int i;
 
-	dprintk(KERN_DEBUG "%s: nv_nic_irq\n", dev->name);
+	dprintk(KERN_DEBUG "%s: __nv_nic_irq\n", dev->name);
 
 	for (i=0; ; i++) {
 		if (!(np->msi_flags & NV_MSI_X_ENABLED)) {
@@ -2960,7 +2959,10 @@ static irqreturn_t nv_nic_irq(int foo, void *data)
 			break;
 
 		spin_lock(&np->lock);
-		nv_tx_done(dev);
+		if (optimized)
+			nv_tx_done_optimized(dev, TX_WORK_PER_LOOP);
+		else
+			nv_tx_done(dev);
 		spin_unlock(&np->lock);
 
 #ifdef CONFIG_FORCEDETH_NAPI
@@ -2978,12 +2980,23 @@ static irqreturn_t nv_nic_irq(int foo, void *data)
 			spin_unlock(&np->lock);
 		}
 #else
-		if (nv_rx_process(dev, RX_WORK_PER_LOOP)) {
-			if (unlikely(nv_alloc_rx(dev))) {
-				spin_lock(&np->lock);
-				if (netif_running(dev))
-					mod_timer(&np->oom_kick, jiffies + OOM_REFILL);
-				spin_unlock(&np->lock);
+		if (optimized) {
+			if (nv_rx_process_optimized(dev, RX_WORK_PER_LOOP)) {
+				if (unlikely(nv_alloc_rx_optimized(dev))) {
+					spin_lock(&np->lock);
+					if (netif_running(dev))
+						mod_timer(&np->oom_kick, jiffies + OOM_REFILL);
+					spin_unlock(&np->lock);
+				}
+			}
+		} else {
+			if (nv_rx_process(dev, RX_WORK_PER_LOOP)) {
+				if (unlikely(nv_alloc_rx(dev))) {
+					spin_lock(&np->lock);
+					if (netif_running(dev))
+						mod_timer(&np->oom_kick, jiffies + OOM_REFILL);
+					spin_unlock(&np->lock);
+				}
 			}
 		}
 #endif
@@ -3042,130 +3055,26 @@ static irqreturn_t nv_nic_irq(int foo, void *data)
 		}
 
 	}
-	dprintk(KERN_DEBUG "%s: nv_nic_irq completed\n", dev->name);
+	dprintk(KERN_DEBUG "%s: __nv_nic_irq completed\n", dev->name);
 
 	return IRQ_RETVAL(i);
 }
 
-/**
- * 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 irqreturn_t nv_nic_irq(int foo, void *data)
 {
-	struct net_device *dev = (struct net_device *) data;
-	struct fe_priv *np = netdev_priv(dev);
-	u8 __iomem *base = get_hwbase(dev);
-	u32 events;
-	int i;
-
-	dprintk(KERN_DEBUG "%s: nv_nic_irq_optimized\n", dev->name);
-
-	for (i=0; ; i++) {
-		if (!(np->msi_flags & NV_MSI_X_ENABLED)) {
-			events = readl(base + NvRegIrqStatus) & NVREG_IRQSTAT_MASK;
-			writel(NVREG_IRQSTAT_MASK, base + NvRegIrqStatus);
-		} else {
-			events = readl(base + NvRegMSIXIrqStatus) & NVREG_IRQSTAT_MASK;
-			writel(NVREG_IRQSTAT_MASK, base + NvRegMSIXIrqStatus);
-		}
-		dprintk(KERN_DEBUG "%s: irq: %08x\n", dev->name, events);
-		if (!(events & np->irqmask))
-			break;
-
-		spin_lock(&np->lock);
-		nv_tx_done_optimized(dev, TX_WORK_PER_LOOP);
-		spin_unlock(&np->lock);
-
-#ifdef CONFIG_FORCEDETH_NAPI
-		if (events & NVREG_IRQ_RX_ALL) {
-			netif_rx_schedule(dev, &np->napi);
-
-			/* Disable furthur receive irq's */
-			spin_lock(&np->lock);
-			np->irqmask &= ~NVREG_IRQ_RX_ALL;
-
-			if (np->msi_flags & NV_MSI_X_ENABLED)
-				writel(NVREG_IRQ_RX_ALL, base + NvRegIrqMask);
-			else
-				writel(np->irqmask, base + NvRegIrqMask);
-			spin_unlock(&np->lock);
-		}
-#else
-		if (nv_rx_process_optimized(dev, RX_WORK_PER_LOOP)) {
-			if (unlikely(nv_alloc_rx_optimized(dev))) {
-				spin_lock(&np->lock);
-				if (netif_running(dev))
-					mod_timer(&np->oom_kick, jiffies + OOM_REFILL);
-				spin_unlock(&np->lock);
-			}
-		}
-#endif
-		if (unlikely(events & NVREG_IRQ_LINK)) {
-			spin_lock(&np->lock);
-			nv_link_irq(dev);
-			spin_unlock(&np->lock);
-		}
-		if (unlikely(np->need_linktimer && time_after(jiffies, np->link_timeout))) {
-			spin_lock(&np->lock);
-			nv_linkchange(dev);
-			spin_unlock(&np->lock);
-			np->link_timeout = jiffies + LINK_TIMEOUT;
-		}
-		if (unlikely(events & (NVREG_IRQ_TX_ERR))) {
-			dprintk(KERN_DEBUG "%s: received irq with events 0x%x. Probably TX fail.\n",
-						dev->name, events);
-		}
-		if (unlikely(events & (NVREG_IRQ_UNKNOWN))) {
-			printk(KERN_DEBUG "%s: received irq with unknown events 0x%x. Please report\n",
-						dev->name, events);
-		}
-		if (unlikely(events & NVREG_IRQ_RECOVER_ERROR)) {
-			spin_lock(&np->lock);
-			/* disable interrupts on the nic */
-			if (!(np->msi_flags & NV_MSI_X_ENABLED))
-				writel(0, base + NvRegIrqMask);
-			else
-				writel(np->irqmask, base + NvRegIrqMask);
-			pci_push(base);
-
-			if (netif_running(dev)) {
-				np->nic_poll_irq = np->irqmask;
-				np->recover_error = 1;
-				mod_timer(&np->nic_poll, jiffies + POLL_WAIT);
-			}
-			spin_unlock(&np->lock);
-			break;
-		}
-
-		if (unlikely(i > max_interrupt_work)) {
-			spin_lock(&np->lock);
-			/* disable interrupts on the nic */
-			if (!(np->msi_flags & NV_MSI_X_ENABLED))
-				writel(0, base + NvRegIrqMask);
-			else
-				writel(np->irqmask, base + NvRegIrqMask);
-			pci_push(base);
-
-			if (netif_running(dev)) {
-				np->nic_poll_irq = np->irqmask;
-				mod_timer(&np->nic_poll, jiffies + POLL_WAIT);
-			}
-			spin_unlock(&np->lock);
-			printk(KERN_DEBUG "%s: too many iterations (%d) in nv_nic_irq.\n", dev->name, i);
-			break;
-		}
-
-	}
-	dprintk(KERN_DEBUG "%s: nv_nic_irq_optimized completed\n", dev->name);
+	struct net_device *dev = data;
+	return __nv_nic_irq(dev, false);
+}
 
-	return IRQ_RETVAL(i);
+static irqreturn_t nv_nic_irq_optimized(int foo, void *data)
+{
+	struct net_device *dev = data;
+	return __nv_nic_irq(dev, true);
 }
 
 static irqreturn_t nv_nic_irq_tx(int foo, void *data)
 {
-	struct net_device *dev = (struct net_device *) data;
+	struct net_device *dev = data;
 	struct fe_priv *np = netdev_priv(dev);
 	u8 __iomem *base = get_hwbase(dev);
 	u32 events;
@@ -3255,7 +3164,7 @@ static int nv_napi_poll(struct napi_struct *napi, int budget)
 #ifdef CONFIG_FORCEDETH_NAPI
 static irqreturn_t nv_nic_irq_rx(int foo, void *data)
 {
-	struct net_device *dev = (struct net_device *) data;
+	struct net_device *dev = data;
 	struct fe_priv *np = netdev_priv(dev);
 	u8 __iomem *base = get_hwbase(dev);
 	u32 events;
@@ -3274,7 +3183,7 @@ static irqreturn_t nv_nic_irq_rx(int foo, void *data)
 #else
 static irqreturn_t nv_nic_irq_rx(int foo, void *data)
 {
-	struct net_device *dev = (struct net_device *) data;
+	struct net_device *dev = data;
 	struct fe_priv *np = netdev_priv(dev);
 	u8 __iomem *base = get_hwbase(dev);
 	u32 events;
@@ -3322,7 +3231,7 @@ static irqreturn_t nv_nic_irq_rx(int foo, void *data)
 
 static irqreturn_t nv_nic_irq_other(int foo, void *data)
 {
-	struct net_device *dev = (struct net_device *) data;
+	struct net_device *dev = data;
 	struct fe_priv *np = netdev_priv(dev);
 	u8 __iomem *base = get_hwbase(dev);
 	u32 events;
@@ -3395,7 +3304,7 @@ static irqreturn_t nv_nic_irq_other(int foo, void *data)
 
 static irqreturn_t nv_nic_irq_test(int foo, void *data)
 {
-	struct net_device *dev = (struct net_device *) data;
+	struct net_device *dev = data;
 	struct fe_priv *np = netdev_priv(dev);
 	u8 __iomem *base = get_hwbase(dev);
 	u32 events;

^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [PATCH 4/6] forcedeth: unconditionally enable NAPI
  2007-10-17  5:52 [PATCH 0/6] forcedeth interrupt and task overhaul, v2 Jeff Garzik
                   ` (2 preceding siblings ...)
  2007-10-17  5:54 ` [PATCH 3/6] forcedeth: eliminate some duplicate irq handling code Jeff Garzik
@ 2007-10-17  5:54 ` Jeff Garzik
  2007-10-17  5:54 ` [PATCH 5/6] forcedeth: use NAPI for TX completion Jeff Garzik
                   ` (2 subsequent siblings)
  6 siblings, 0 replies; 8+ messages in thread
From: Jeff Garzik @ 2007-10-17  5:54 UTC (permalink / raw)
  To: netdev, LKML; +Cc: Manfred Spraul, Ingo Molnar, Ayaz Abdulla


commit 8f61debaeb334bce0ccba1a1384d549a377c1e8e
Author: Jeff Garzik <jeff@garzik.org>
Date:   Tue Oct 16 12:55:08 2007 -0400

    [netdrvr] forcedeth: unconditionally enable NAPI
    
    Remove all !CONFIG_FORCEDETH_NAPI code, and the Kconfig option,
    enabling NAPI unconditionally.
    
    The NIC driver recreates a lot of this functionality manually.
    
    Signed-off-by: Jeff Garzik <jgarzik@redhat.com>

 drivers/net/Kconfig     |   17 -----
 drivers/net/forcedeth.c |  143 ++----------------------------------------------
 2 files changed, 7 insertions(+), 153 deletions(-)

8f61debaeb334bce0ccba1a1384d549a377c1e8e
diff --git a/drivers/net/Kconfig b/drivers/net/Kconfig
index 83d52c8..eafdd58 100644
--- a/drivers/net/Kconfig
+++ b/drivers/net/Kconfig
@@ -1430,23 +1430,6 @@ config FORCEDETH
 	  <file:Documentation/networking/net-modules.txt>.  The module will be
 	  called forcedeth.
 
-config FORCEDETH_NAPI
-	bool "Use Rx and Tx Polling (NAPI) (EXPERIMENTAL)"
-	depends on FORCEDETH && EXPERIMENTAL
-	help
-	  NAPI is a new driver API designed to reduce CPU and interrupt load
-	  when the driver is receiving lots of packets from the card. It is
-	  still somewhat experimental and thus not yet enabled by default.
-
-	  If your estimated Rx load is 10kpps or more, or if the card will be
-	  deployed on potentially unfriendly networks (e.g. in a firewall),
-	  then say Y here.
-
-	  See <file:Documentation/networking/NAPI_HOWTO.txt> for more
-	  information.
-
-	  If in doubt, say N.
-
 config CS89x0
 	tristate "CS89x0 support"
 	depends on NET_PCI && (ISA || MACH_IXDP2351 || ARCH_IXDP2X01 || ARCH_PNX010X)
diff --git a/drivers/net/forcedeth.c b/drivers/net/forcedeth.c
index a4baad7..32a8893 100644
--- a/drivers/net/forcedeth.c
+++ b/drivers/net/forcedeth.c
@@ -1546,7 +1546,6 @@ static int nv_alloc_rx_optimized(struct net_device *dev)
 }
 
 /* If rx bufs are exhausted called after 50ms to attempt to refresh */
-#ifdef CONFIG_FORCEDETH_NAPI
 static void nv_do_rx_refill(unsigned long data)
 {
 	struct net_device *dev = (struct net_device *) data;
@@ -1555,41 +1554,6 @@ static void nv_do_rx_refill(unsigned long data)
 	/* Just reschedule NAPI rx processing */
 	netif_rx_schedule(dev, &np->napi);
 }
-#else
-static void nv_do_rx_refill(unsigned long data)
-{
-	struct net_device *dev = (struct net_device *) data;
-	struct fe_priv *np = netdev_priv(dev);
-	int retcode;
-
-	if (!using_multi_irqs(dev)) {
-		if (np->msi_flags & NV_MSI_X_ENABLED)
-			disable_irq(np->msi_x_entry[NV_MSI_X_VECTOR_ALL].vector);
-		else
-			disable_irq(dev->irq);
-	} else {
-		disable_irq(np->msi_x_entry[NV_MSI_X_VECTOR_RX].vector);
-	}
-	if (!nv_optimized(np))
-		retcode = nv_alloc_rx(dev);
-	else
-		retcode = nv_alloc_rx_optimized(dev);
-	if (retcode) {
-		spin_lock_irq(&np->lock);
-		if (netif_running(dev))
-			mod_timer(&np->oom_kick, jiffies + OOM_REFILL);
-		spin_unlock_irq(&np->lock);
-	}
-	if (!using_multi_irqs(dev)) {
-		if (np->msi_flags & NV_MSI_X_ENABLED)
-			enable_irq(np->msi_x_entry[NV_MSI_X_VECTOR_ALL].vector);
-		else
-			enable_irq(dev->irq);
-	} else {
-		enable_irq(np->msi_x_entry[NV_MSI_X_VECTOR_RX].vector);
-	}
-}
-#endif
 
 static void nv_init_rx(struct net_device *dev)
 {
@@ -2347,11 +2311,7 @@ static int nv_rx_process(struct net_device *dev, int limit)
 		skb->protocol = eth_type_trans(skb, dev);
 		dprintk(KERN_DEBUG "%s: nv_rx_process: %d bytes, proto %d accepted.\n",
 					dev->name, len, skb->protocol);
-#ifdef CONFIG_FORCEDETH_NAPI
 		netif_receive_skb(skb);
-#else
-		netif_rx(skb);
-#endif
 		dev->last_rx = jiffies;
 		dev->stats.rx_packets++;
 		dev->stats.rx_bytes += len;
@@ -2446,27 +2406,14 @@ static int nv_rx_process_optimized(struct net_device *dev, int limit)
 				dev->name, len, skb->protocol);
 
 			if (likely(!np->vlangrp)) {
-#ifdef CONFIG_FORCEDETH_NAPI
 				netif_receive_skb(skb);
-#else
-				netif_rx(skb);
-#endif
 			} else {
 				vlanflags = le32_to_cpu(np->get_rx.ex->buflow);
 				if (vlanflags & NV_RX3_VLAN_TAG_PRESENT) {
-#ifdef CONFIG_FORCEDETH_NAPI
 					vlan_hwaccel_receive_skb(skb, np->vlangrp,
 								 vlanflags & NV_RX3_VLAN_TAG_MASK);
-#else
-					vlan_hwaccel_rx(skb, np->vlangrp,
-							vlanflags & NV_RX3_VLAN_TAG_MASK);
-#endif
 				} else {
-#ifdef CONFIG_FORCEDETH_NAPI
 					netif_receive_skb(skb);
-#else
-					netif_rx(skb);
-#endif
 				}
 			}
 
@@ -2965,7 +2912,6 @@ static irqreturn_t __nv_nic_irq(struct net_device *dev, bool optimized)
 			nv_tx_done(dev);
 		spin_unlock(&np->lock);
 
-#ifdef CONFIG_FORCEDETH_NAPI
 		if (events & NVREG_IRQ_RX_ALL) {
 			netif_rx_schedule(dev, &np->napi);
 
@@ -2979,27 +2925,7 @@ static irqreturn_t __nv_nic_irq(struct net_device *dev, bool optimized)
 				writel(np->irqmask, base + NvRegIrqMask);
 			spin_unlock(&np->lock);
 		}
-#else
-		if (optimized) {
-			if (nv_rx_process_optimized(dev, RX_WORK_PER_LOOP)) {
-				if (unlikely(nv_alloc_rx_optimized(dev))) {
-					spin_lock(&np->lock);
-					if (netif_running(dev))
-						mod_timer(&np->oom_kick, jiffies + OOM_REFILL);
-					spin_unlock(&np->lock);
-				}
-			}
-		} else {
-			if (nv_rx_process(dev, RX_WORK_PER_LOOP)) {
-				if (unlikely(nv_alloc_rx(dev))) {
-					spin_lock(&np->lock);
-					if (netif_running(dev))
-						mod_timer(&np->oom_kick, jiffies + OOM_REFILL);
-					spin_unlock(&np->lock);
-				}
-			}
-		}
-#endif
+
 		if (unlikely(events & NVREG_IRQ_LINK)) {
 			spin_lock(&np->lock);
 			nv_link_irq(dev);
@@ -3119,7 +3045,6 @@ static irqreturn_t nv_nic_irq_tx(int foo, void *data)
 	return IRQ_RETVAL(i);
 }
 
-#ifdef CONFIG_FORCEDETH_NAPI
 static int nv_napi_poll(struct napi_struct *napi, int budget)
 {
 	struct fe_priv *np = container_of(napi, struct fe_priv, napi);
@@ -3159,9 +3084,7 @@ static int nv_napi_poll(struct napi_struct *napi, int budget)
 	}
 	return pkts;
 }
-#endif
 
-#ifdef CONFIG_FORCEDETH_NAPI
 static irqreturn_t nv_nic_irq_rx(int foo, void *data)
 {
 	struct net_device *dev = data;
@@ -3180,54 +3103,6 @@ static irqreturn_t nv_nic_irq_rx(int foo, void *data)
 	}
 	return IRQ_HANDLED;
 }
-#else
-static irqreturn_t nv_nic_irq_rx(int foo, void *data)
-{
-	struct net_device *dev = data;
-	struct fe_priv *np = netdev_priv(dev);
-	u8 __iomem *base = get_hwbase(dev);
-	u32 events;
-	int i;
-	unsigned long flags;
-
-	dprintk(KERN_DEBUG "%s: nv_nic_irq_rx\n", dev->name);
-
-	for (i=0; ; i++) {
-		events = readl(base + NvRegMSIXIrqStatus) & NVREG_IRQ_RX_ALL;
-		writel(NVREG_IRQ_RX_ALL, base + NvRegMSIXIrqStatus);
-		dprintk(KERN_DEBUG "%s: rx irq: %08x\n", dev->name, events);
-		if (!(events & np->irqmask))
-			break;
-
-		if (nv_rx_process_optimized(dev, RX_WORK_PER_LOOP)) {
-			if (unlikely(nv_alloc_rx_optimized(dev))) {
-				spin_lock_irqsave(&np->lock, flags);
-				if (netif_running(dev))
-					mod_timer(&np->oom_kick, jiffies + OOM_REFILL);
-				spin_unlock_irqrestore(&np->lock, flags);
-			}
-		}
-
-		if (unlikely(i > max_interrupt_work)) {
-			spin_lock_irqsave(&np->lock, flags);
-			/* disable interrupts on the nic */
-			writel(NVREG_IRQ_RX_ALL, base + NvRegIrqMask);
-			pci_push(base);
-
-			if (netif_running(dev)) {
-				np->nic_poll_irq |= NVREG_IRQ_RX_ALL;
-				mod_timer(&np->nic_poll, jiffies + POLL_WAIT);
-			}
-			spin_unlock_irqrestore(&np->lock, flags);
-			printk(KERN_DEBUG "%s: too many iterations (%d) in nv_nic_irq_rx.\n", dev->name, i);
-			break;
-		}
-	}
-	dprintk(KERN_DEBUG "%s: nv_nic_irq_rx completed\n", dev->name);
-
-	return IRQ_RETVAL(i);
-}
-#endif
 
 static irqreturn_t nv_nic_irq_other(int foo, void *data)
 {
@@ -4472,9 +4347,9 @@ static void nv_self_test(struct net_device *dev, struct ethtool_test *test, u64
 	if (test->flags & ETH_TEST_FL_OFFLINE) {
 		if (netif_running(dev)) {
 			netif_stop_queue(dev);
-#ifdef CONFIG_FORCEDETH_NAPI
+
 			napi_disable(&np->napi);
-#endif
+
 			netif_tx_lock_bh(dev);
 			spin_lock_irq(&np->lock);
 			nv_disable_hw_interrupts(dev, np->irqmask);
@@ -4529,9 +4404,9 @@ static void nv_self_test(struct net_device *dev, struct ethtool_test *test, u64
 			/* restart rx engine */
 			nv_start_rxtx(dev);
 			netif_start_queue(dev);
-#ifdef CONFIG_FORCEDETH_NAPI
+
 			napi_enable(&np->napi);
-#endif
+
 			nv_enable_hw_interrupts(dev, np->irqmask);
 		}
 	}
@@ -4756,9 +4631,7 @@ static int nv_open(struct net_device *dev)
 	ret = nv_update_linkspeed(dev);
 	nv_start_rxtx(dev);
 	netif_start_queue(dev);
-#ifdef CONFIG_FORCEDETH_NAPI
 	napi_enable(&np->napi);
-#endif
 
 	if (ret) {
 		netif_carrier_on(dev);
@@ -4789,9 +4662,7 @@ static int nv_close(struct net_device *dev)
 	spin_lock_irq(&np->lock);
 	spin_unlock_irq(&np->lock);
 
-#ifdef CONFIG_FORCEDETH_NAPI
 	napi_disable(&np->napi);
-#endif
 	synchronize_irq(dev->irq);
 
 	del_timer_sync(&np->oom_kick);
@@ -5003,9 +4874,9 @@ static int __devinit nv_probe(struct pci_dev *pci_dev, const struct pci_device_i
 #ifdef CONFIG_NET_POLL_CONTROLLER
 	dev->poll_controller = nv_poll_controller;
 #endif
-#ifdef CONFIG_FORCEDETH_NAPI
+
 	netif_napi_add(dev, &np->napi, nv_napi_poll, RX_WORK_PER_LOOP);
-#endif
+
 	SET_ETHTOOL_OPS(dev, &ops);
 	dev->tx_timeout = nv_tx_timeout;
 	dev->watchdog_timeo = NV_WATCHDOG_TIMEO;

^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [PATCH 5/6] forcedeth: use NAPI for TX completion
  2007-10-17  5:52 [PATCH 0/6] forcedeth interrupt and task overhaul, v2 Jeff Garzik
                   ` (3 preceding siblings ...)
  2007-10-17  5:54 ` [PATCH 4/6] forcedeth: unconditionally enable NAPI Jeff Garzik
@ 2007-10-17  5:54 ` Jeff Garzik
  2007-10-17  5:55 ` [PATCH 6/6] [netdrvr] interrupt handling overhaul Jeff Garzik
  2007-10-17  5:59 ` [PATCH 0/6] forcedeth interrupt and task overhaul, v2 Jeff Garzik
  6 siblings, 0 replies; 8+ messages in thread
From: Jeff Garzik @ 2007-10-17  5:54 UTC (permalink / raw)
  To: netdev, LKML; +Cc: Manfred Spraul, Ingo Molnar, Ayaz Abdulla


commit a7c00e796597b797ceac3c18e8b85c124196c5ab
Author: Jeff Garzik <jeff@garzik.org>
Date:   Tue Oct 16 17:33:19 2007 -0400

    [netdrvr] forcedeth: use NAPI for TX completion
    
    A hand-rolled TX poll & work limit system was already in place, so it
    was easy to convert the TX path to use NAPI.
    
    This simplifies the code, and enables future improvements and
    simplifications.
    
    Signed-off-by: Jeff Garzik <jgarzik@redhat.com>

 drivers/net/forcedeth.c |  170 +++++++++++++++++++++++++-----------------------
 1 file changed, 90 insertions(+), 80 deletions(-)

a7c00e796597b797ceac3c18e8b85c124196c5ab
diff --git a/drivers/net/forcedeth.c b/drivers/net/forcedeth.c
index 32a8893..81fe016 100644
--- a/drivers/net/forcedeth.c
+++ b/drivers/net/forcedeth.c
@@ -660,6 +660,7 @@ struct fe_priv {
 
 	struct net_device *dev;
 	struct napi_struct napi;
+	struct napi_struct tx_napi;
 
 	/* General data:
 	 * Locking: spin_lock(&np->lock); */
@@ -725,7 +726,6 @@ struct fe_priv {
 	union ring_type tx_ring;
 	u32 tx_flags;
 	int tx_ring_size;
-	int tx_stop;
 
 	/* vlan fields */
 	struct vlan_group *vlangrp;
@@ -1732,10 +1732,7 @@ static int nv_start_xmit(struct sk_buff *skb, struct net_device *dev)
 
 	empty_slots = nv_get_empty_tx_slots(np);
 	if (unlikely(empty_slots <= entries)) {
-		spin_lock_irq(&np->lock);
 		netif_stop_queue(dev);
-		np->tx_stop = 1;
-		spin_unlock_irq(&np->lock);
 		return NETDEV_TX_BUSY;
 	}
 
@@ -1848,10 +1845,7 @@ static int nv_start_xmit_optimized(struct sk_buff *skb, struct net_device *dev)
 
 	empty_slots = nv_get_empty_tx_slots(np);
 	if (unlikely(empty_slots <= entries)) {
-		spin_lock_irq(&np->lock);
 		netif_stop_queue(dev);
-		np->tx_stop = 1;
-		spin_unlock_irq(&np->lock);
 		return NETDEV_TX_BUSY;
 	}
 
@@ -1956,14 +1950,15 @@ static int nv_start_xmit_optimized(struct sk_buff *skb, struct net_device *dev)
  *
  * Caller must own np->lock.
  */
-static void nv_tx_done(struct net_device *dev)
+static int nv_tx_done(struct net_device *dev, int limit)
 {
 	struct fe_priv *np = netdev_priv(dev);
 	u32 flags;
-	struct ring_desc* orig_get_tx = np->get_tx.orig;
+	int tx_work = 0;
 
 	while ((np->get_tx.orig != np->put_tx.orig) &&
-	       !((flags = le32_to_cpu(np->get_tx.orig->flaglen)) & NV_TX_VALID)) {
+	       !((flags = le32_to_cpu(np->get_tx.orig->flaglen)) & NV_TX_VALID) &&
+	       (tx_work < limit)) {
 
 		dprintk(KERN_DEBUG "%s: nv_tx_done: flags 0x%x.\n",
 					dev->name, flags);
@@ -2008,22 +2003,25 @@ static void nv_tx_done(struct net_device *dev)
 			np->get_tx.orig = np->first_tx.orig;
 		if (unlikely(np->get_tx_ctx++ == np->last_tx_ctx))
 			np->get_tx_ctx = np->first_tx_ctx;
+
+		tx_work++;
 	}
-	if (unlikely((np->tx_stop == 1) && (np->get_tx.orig != orig_get_tx))) {
-		np->tx_stop = 0;
+
+	if (tx_work)
 		netif_wake_queue(dev);
-	}
+
+	return tx_work;
 }
 
-static void nv_tx_done_optimized(struct net_device *dev, int limit)
+static int nv_tx_done_optimized(struct net_device *dev, int limit)
 {
 	struct fe_priv *np = netdev_priv(dev);
 	u32 flags;
-	struct ring_desc_ex* orig_get_tx = np->get_tx.ex;
+	int tx_work = 0;
 
 	while ((np->get_tx.ex != np->put_tx.ex) &&
 	       !((flags = le32_to_cpu(np->get_tx.ex->flaglen)) & NV_TX_VALID) &&
-	       (limit-- > 0)) {
+	       (tx_work < limit)) {
 
 		dprintk(KERN_DEBUG "%s: nv_tx_done_optimized: flags 0x%x.\n",
 					dev->name, flags);
@@ -2043,11 +2041,14 @@ static void nv_tx_done_optimized(struct net_device *dev, int limit)
 			np->get_tx.ex = np->first_tx.ex;
 		if (unlikely(np->get_tx_ctx++ == np->last_tx_ctx))
 			np->get_tx_ctx = np->first_tx_ctx;
+
+		tx_work++;
 	}
-	if (unlikely((np->tx_stop == 1) && (np->get_tx.ex != orig_get_tx))) {
-		np->tx_stop = 0;
+
+	if (tx_work)
 		netif_wake_queue(dev);
-	}
+
+	return tx_work;
 }
 
 /*
@@ -2120,7 +2121,7 @@ static void nv_tx_timeout(struct net_device *dev)
 
 	/* 2) check that the packets were not sent already: */
 	if (!nv_optimized(np))
-		nv_tx_done(dev);
+		nv_tx_done(dev, np->tx_ring_size);
 	else
 		nv_tx_done_optimized(dev, np->tx_ring_size);
 
@@ -2888,7 +2889,7 @@ static irqreturn_t __nv_nic_irq(struct net_device *dev, bool optimized)
 {
 	struct fe_priv *np = netdev_priv(dev);
 	u8 __iomem *base = np->base;
-	u32 events;
+	u32 events, updmask = 0;
 	int i;
 
 	dprintk(KERN_DEBUG "%s: __nv_nic_irq\n", dev->name);
@@ -2905,22 +2906,22 @@ static irqreturn_t __nv_nic_irq(struct net_device *dev, bool optimized)
 		if (!(events & np->irqmask))
 			break;
 
-		spin_lock(&np->lock);
-		if (optimized)
-			nv_tx_done_optimized(dev, TX_WORK_PER_LOOP);
-		else
-			nv_tx_done(dev);
-		spin_unlock(&np->lock);
-
 		if (events & NVREG_IRQ_RX_ALL) {
 			netif_rx_schedule(dev, &np->napi);
+			updmask |= NVREG_IRQ_RX_ALL;
+		}
+
+		if (events & NVREG_IRQ_TX_ALL) {
+			netif_rx_schedule(dev, &np->tx_napi);
+			updmask |= NVREG_IRQ_TX_ALL;
+		}
 
-			/* Disable furthur receive irq's */
+		if (updmask) {
 			spin_lock(&np->lock);
-			np->irqmask &= ~NVREG_IRQ_RX_ALL;
+			np->irqmask &= ~updmask;
 
 			if (np->msi_flags & NV_MSI_X_ENABLED)
-				writel(NVREG_IRQ_RX_ALL, base + NvRegIrqMask);
+				writel(updmask, base + NvRegIrqMask);
 			else
 				writel(np->irqmask, base + NvRegIrqMask);
 			spin_unlock(&np->lock);
@@ -2998,58 +2999,11 @@ static irqreturn_t nv_nic_irq_optimized(int foo, void *data)
 	return __nv_nic_irq(dev, true);
 }
 
-static irqreturn_t nv_nic_irq_tx(int foo, void *data)
-{
-	struct net_device *dev = data;
-	struct fe_priv *np = netdev_priv(dev);
-	u8 __iomem *base = get_hwbase(dev);
-	u32 events;
-	int i;
-	unsigned long flags;
-
-	dprintk(KERN_DEBUG "%s: nv_nic_irq_tx\n", dev->name);
-
-	for (i=0; ; i++) {
-		events = readl(base + NvRegMSIXIrqStatus) & NVREG_IRQ_TX_ALL;
-		writel(NVREG_IRQ_TX_ALL, base + NvRegMSIXIrqStatus);
-		dprintk(KERN_DEBUG "%s: tx irq: %08x\n", dev->name, events);
-		if (!(events & np->irqmask))
-			break;
-
-		spin_lock_irqsave(&np->lock, flags);
-		nv_tx_done_optimized(dev, TX_WORK_PER_LOOP);
-		spin_unlock_irqrestore(&np->lock, flags);
-
-		if (unlikely(events & (NVREG_IRQ_TX_ERR))) {
-			dprintk(KERN_DEBUG "%s: received irq with events 0x%x. Probably TX fail.\n",
-						dev->name, events);
-		}
-		if (unlikely(i > max_interrupt_work)) {
-			spin_lock_irqsave(&np->lock, flags);
-			/* disable interrupts on the nic */
-			writel(NVREG_IRQ_TX_ALL, base + NvRegIrqMask);
-			pci_push(base);
-
-			if (netif_running(dev)) {
-				np->nic_poll_irq |= NVREG_IRQ_TX_ALL;
-				mod_timer(&np->nic_poll, jiffies + POLL_WAIT);
-			}
-			spin_unlock_irqrestore(&np->lock, flags);
-			printk(KERN_DEBUG "%s: too many iterations (%d) in nv_nic_irq_tx.\n", dev->name, i);
-			break;
-		}
-
-	}
-	dprintk(KERN_DEBUG "%s: nv_nic_irq_tx completed\n", dev->name);
-
-	return IRQ_RETVAL(i);
-}
-
 static int nv_napi_poll(struct napi_struct *napi, int budget)
 {
 	struct fe_priv *np = container_of(napi, struct fe_priv, napi);
 	struct net_device *dev = np->dev;
-	u8 __iomem *base = get_hwbase(dev);
+	u8 __iomem *base = np->base;
 	unsigned long flags;
 	int pkts, retcode;
 
@@ -3089,7 +3043,7 @@ static irqreturn_t nv_nic_irq_rx(int foo, void *data)
 {
 	struct net_device *dev = data;
 	struct fe_priv *np = netdev_priv(dev);
-	u8 __iomem *base = get_hwbase(dev);
+	u8 __iomem *base = np->base;
 	u32 events;
 
 	events = readl(base + NvRegMSIXIrqStatus) & NVREG_IRQ_RX_ALL;
@@ -3104,6 +3058,57 @@ static irqreturn_t nv_nic_irq_rx(int foo, void *data)
 	return IRQ_HANDLED;
 }
 
+static irqreturn_t nv_nic_irq_tx(int foo, void *data)
+{
+	struct net_device *dev = data;
+	struct fe_priv *np = netdev_priv(dev);
+	u8 __iomem *base = np->base;
+	u32 events;
+
+	events = readl(base + NvRegMSIXIrqStatus) & NVREG_IRQ_TX_ALL;
+	writel(NVREG_IRQ_TX_ALL, base + NvRegMSIXIrqStatus);
+
+	if (events) {
+		netif_rx_schedule(dev, &np->tx_napi);
+		/* disable receive interrupts on the nic */
+		writel(NVREG_IRQ_TX_ALL, base + NvRegIrqMask);
+		pci_push(base);
+	}
+	return IRQ_HANDLED;
+}
+
+static int nv_napi_tx_poll(struct napi_struct *napi, int budget)
+{
+	struct fe_priv *np = container_of(napi, struct fe_priv, tx_napi);
+	struct net_device *dev = np->dev;
+	u8 __iomem *base = np->base;
+	unsigned long flags;
+	int pkts;
+
+	spin_lock_irqsave(&np->lock, flags);
+
+	if (nv_optimized(np))
+		pkts = nv_tx_done_optimized(dev, budget);
+	else
+		pkts = nv_tx_done(dev, budget);
+
+	if (pkts < budget) {
+		/* re-enable receive interrupts */
+
+		__netif_rx_complete(dev, napi);
+
+		np->irqmask |= NVREG_IRQ_RX_ALL;
+		if (np->msi_flags & NV_MSI_X_ENABLED)
+			writel(NVREG_IRQ_RX_ALL, base + NvRegIrqMask);
+		else
+			writel(np->irqmask, base + NvRegIrqMask);
+	}
+
+	spin_unlock_irqrestore(&np->lock, flags);
+
+	return pkts;
+}
+
 static irqreturn_t nv_nic_irq_other(int foo, void *data)
 {
 	struct net_device *dev = data;
@@ -4348,6 +4353,7 @@ static void nv_self_test(struct net_device *dev, struct ethtool_test *test, u64
 		if (netif_running(dev)) {
 			netif_stop_queue(dev);
 
+			napi_disable(&np->tx_napi);
 			napi_disable(&np->napi);
 
 			netif_tx_lock_bh(dev);
@@ -4406,6 +4412,7 @@ static void nv_self_test(struct net_device *dev, struct ethtool_test *test, u64
 			netif_start_queue(dev);
 
 			napi_enable(&np->napi);
+			napi_enable(&np->tx_napi);
 
 			nv_enable_hw_interrupts(dev, np->irqmask);
 		}
@@ -4632,6 +4639,7 @@ static int nv_open(struct net_device *dev)
 	nv_start_rxtx(dev);
 	netif_start_queue(dev);
 	napi_enable(&np->napi);
+	napi_enable(&np->tx_napi);
 
 	if (ret) {
 		netif_carrier_on(dev);
@@ -4662,6 +4670,7 @@ static int nv_close(struct net_device *dev)
 	spin_lock_irq(&np->lock);
 	spin_unlock_irq(&np->lock);
 
+	napi_disable(&np->tx_napi);
 	napi_disable(&np->napi);
 	synchronize_irq(dev->irq);
 
@@ -4876,6 +4885,7 @@ static int __devinit nv_probe(struct pci_dev *pci_dev, const struct pci_device_i
 #endif
 
 	netif_napi_add(dev, &np->napi, nv_napi_poll, RX_WORK_PER_LOOP);
+	netif_napi_add(dev, &np->tx_napi, nv_napi_tx_poll, TX_WORK_PER_LOOP);
 
 	SET_ETHTOOL_OPS(dev, &ops);
 	dev->tx_timeout = nv_tx_timeout;

^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [PATCH 6/6] [netdrvr] interrupt handling overhaul
  2007-10-17  5:52 [PATCH 0/6] forcedeth interrupt and task overhaul, v2 Jeff Garzik
                   ` (4 preceding siblings ...)
  2007-10-17  5:54 ` [PATCH 5/6] forcedeth: use NAPI for TX completion Jeff Garzik
@ 2007-10-17  5:55 ` Jeff Garzik
  2007-10-17  5:59 ` [PATCH 0/6] forcedeth interrupt and task overhaul, v2 Jeff Garzik
  6 siblings, 0 replies; 8+ messages in thread
From: Jeff Garzik @ 2007-10-17  5:55 UTC (permalink / raw)
  To: netdev, LKML; +Cc: Manfred Spraul, Ingo Molnar, Ayaz Abdulla


commit 4f97856cd73ad3ccee06f1856c60cb1ed8f44ceb
Author: Jeff Garzik <jeff@garzik.org>
Date:   Tue Oct 16 19:48:15 2007 -0400

    [netdrvr] interrupt handling overhaul
    
    * eliminate the work loops in the interrupt handlers.  they are no
      longer needed, now that NAPI and other asynchronous tasks handle our
      work (and can be independently controlled by means other than
      disable_irq or spin_lock_irqsave).
    
    * make ->poll_controller call interrupt handler directly
    
    * now that nv_do_nic_poll()'s only users are those that request
      recovery, we can rename it to nv_reset_task(), and convert it from a
      timer-context function to a workqueue.
    
    * ->tx_timeout TX reset is replaced by requesting recovery
    
    * in the main irq handler (INTx or MSI), the spinlock is always
      taken in the common case: TX-or-RX work available.  take the lock once
      for any work, near the beginning of the irq handler (after status is
      read and cleared).
    
      This accomplishes the goal of getting a np->irqmask access inside the
      lock, simplifies the code, and enables better control over irq
      masking.
    
    * no need to take lock, simply to schedule timer
    
    * MSI-X RX/TX irq handling micro-optimization
    
    * use IRQ_NONE/IRQ_HANDLED in nv_nic_irq_test(), rather than the less
      readable versions of the same values.
    
    * in nv_close(), make sure to cancel the workqueue tasks first thing,
      to ensure they (mainly the reset task) will not race with interface
      shutdown.
    
    * move now-pointless tests of np->recover_error and netif_running()
      in nv_reset_task(), resulting in a large de-indent of existing code.
    
    * eliminate now-useless max_interrupt_work module option.  though we
      might want to consider permitting tuning of NAPI weights via ethtool,
      in the future.
    
    Signed-off-by: Jeff Garzik <jgarzik@redhat.com>

 drivers/net/forcedeth.c |  385 +++++++++++++++++++-----------------------------
 1 file changed, 158 insertions(+), 227 deletions(-)

4f97856cd73ad3ccee06f1856c60cb1ed8f44ceb
diff --git a/drivers/net/forcedeth.c b/drivers/net/forcedeth.c
index 81fe016..8af9a96 100644
--- a/drivers/net/forcedeth.c
+++ b/drivers/net/forcedeth.c
@@ -58,6 +58,7 @@
 #include <linux/init.h>
 #include <linux/if_vlan.h>
 #include <linux/dma-mapping.h>
+#include <linux/workqueue.h>
 
 #include <asm/irq.h>
 #include <asm/io.h>
@@ -676,7 +677,6 @@ struct fe_priv {
 	unsigned int phy_model;
 	u16 gigabit;
 	int intr_test;
-	int recover_error;
 
 	/* General data: RO fields */
 	dma_addr_t ring_addr;
@@ -701,14 +701,14 @@ struct fe_priv {
 	struct nv_skb_map *first_rx_ctx, *last_rx_ctx;
 	struct nv_skb_map *rx_skb;
 
-	union ring_type rx_ring;
-	unsigned int rx_buf_sz;
-	unsigned int pkt_limit;
-	struct timer_list oom_kick;
-	struct timer_list nic_poll;
-	struct delayed_work stats_task;
-	u32 nic_poll_irq;
-	int rx_ring_size;
+	union ring_type		rx_ring;
+	unsigned int		rx_buf_sz;
+	unsigned int		pkt_limit;
+	struct timer_list	oom_kick;
+	struct work_struct	reset_task;
+	struct delayed_work	stats_task;
+	u32			nic_poll_irq;
+	int			rx_ring_size;
 
 	/* media detection workaround.
 	 * Locking: Within irq hander or disable_irq+spin_lock(&np->lock);
@@ -739,12 +739,6 @@ struct fe_priv {
 };
 
 /*
- * Maximum number of loops until we assume that a bit in the irq mask
- * is stuck. Overridable with module param.
- */
-static int max_interrupt_work = 5;
-
-/*
  * Optimization can be either throuput mode or cpu mode
  *
  * Throughput Mode: Every tx and rx packet will generate an interrupt.
@@ -2116,27 +2110,9 @@ static void nv_tx_timeout(struct net_device *dev)
 
 	spin_lock_irq(&np->lock);
 
-	/* 1) stop tx engine */
-	nv_stop_tx(dev);
-
-	/* 2) check that the packets were not sent already: */
-	if (!nv_optimized(np))
-		nv_tx_done(dev, np->tx_ring_size);
-	else
-		nv_tx_done_optimized(dev, np->tx_ring_size);
-
-	/* 3) if there are dead entries: clear everything */
-	if (np->get_tx_ctx != np->put_tx_ctx) {
-		printk(KERN_DEBUG "%s: tx_timeout: dead entries!\n", dev->name);
-		nv_drain_tx(dev);
-		nv_init_tx(dev);
-		setup_hw_rings(dev, NV_SETUP_TX_RING);
-	}
-
-	netif_wake_queue(dev);
+	np->nic_poll_irq = np->irqmask;
+	schedule_work(&np->reset_task);
 
-	/* 4) restart tx engine */
-	nv_start_tx(dev);
 	spin_unlock_irq(&np->lock);
 }
 
@@ -2890,101 +2866,82 @@ static irqreturn_t __nv_nic_irq(struct net_device *dev, bool optimized)
 	struct fe_priv *np = netdev_priv(dev);
 	u8 __iomem *base = np->base;
 	u32 events, updmask = 0;
-	int i;
+	bool msix = np->msi_flags & NV_MSI_X_ENABLED;
 
 	dprintk(KERN_DEBUG "%s: __nv_nic_irq\n", dev->name);
 
-	for (i=0; ; i++) {
-		if (!(np->msi_flags & NV_MSI_X_ENABLED)) {
-			events = readl(base + NvRegIrqStatus) & NVREG_IRQSTAT_MASK;
-			writel(NVREG_IRQSTAT_MASK, base + NvRegIrqStatus);
-		} else {
-			events = readl(base + NvRegMSIXIrqStatus) & NVREG_IRQSTAT_MASK;
-			writel(NVREG_IRQSTAT_MASK, base + NvRegMSIXIrqStatus);
-		}
-		dprintk(KERN_DEBUG "%s: irq: %08x\n", dev->name, events);
-		if (!(events & np->irqmask))
-			break;
+	if (!msix) {
+		events = readl(base + NvRegIrqStatus) & NVREG_IRQSTAT_MASK;
+		writel(NVREG_IRQSTAT_MASK, base + NvRegIrqStatus);
+	} else {
+		events = readl(base + NvRegMSIXIrqStatus) & NVREG_IRQSTAT_MASK;
+		writel(NVREG_IRQSTAT_MASK, base + NvRegMSIXIrqStatus);
+	}
 
-		if (events & NVREG_IRQ_RX_ALL) {
-			netif_rx_schedule(dev, &np->napi);
-			updmask |= NVREG_IRQ_RX_ALL;
-		}
+	dprintk(KERN_DEBUG "%s: irq: %08x\n", dev->name, events);
 
-		if (events & NVREG_IRQ_TX_ALL) {
-			netif_rx_schedule(dev, &np->tx_napi);
-			updmask |= NVREG_IRQ_TX_ALL;
-		}
+	if (!events)
+		return IRQ_NONE;
 
-		if (updmask) {
-			spin_lock(&np->lock);
-			np->irqmask &= ~updmask;
+	spin_lock(&np->lock);
 
-			if (np->msi_flags & NV_MSI_X_ENABLED)
-				writel(updmask, base + NvRegIrqMask);
-			else
-				writel(np->irqmask, base + NvRegIrqMask);
-			spin_unlock(&np->lock);
-		}
+	if (!(events & np->irqmask))
+		goto out;
 
-		if (unlikely(events & NVREG_IRQ_LINK)) {
-			spin_lock(&np->lock);
-			nv_link_irq(dev);
-			spin_unlock(&np->lock);
-		}
-		if (unlikely(np->need_linktimer && time_after(jiffies, np->link_timeout))) {
-			spin_lock(&np->lock);
-			nv_linkchange(dev);
-			spin_unlock(&np->lock);
-			np->link_timeout = jiffies + LINK_TIMEOUT;
-		}
-		if (unlikely(events & (NVREG_IRQ_TX_ERR))) {
-			dprintk(KERN_DEBUG "%s: received irq with events 0x%x. Probably TX fail.\n",
-						dev->name, events);
-		}
-		if (unlikely(events & (NVREG_IRQ_UNKNOWN))) {
-			printk(KERN_DEBUG "%s: received irq with unknown events 0x%x. Please report\n",
-						dev->name, events);
-		}
-		if (unlikely(events & NVREG_IRQ_RECOVER_ERROR)) {
-			spin_lock(&np->lock);
-			/* disable interrupts on the nic */
-			if (!(np->msi_flags & NV_MSI_X_ENABLED))
-				writel(0, base + NvRegIrqMask);
-			else
-				writel(np->irqmask, base + NvRegIrqMask);
-			pci_push(base);
+	if (events & NVREG_IRQ_RX_ALL) {
+		netif_rx_schedule(dev, &np->napi);
+		updmask |= NVREG_IRQ_RX_ALL;
+	}
 
-			if (netif_running(dev)) {
-				np->nic_poll_irq = np->irqmask;
-				np->recover_error = 1;
-				mod_timer(&np->nic_poll, jiffies + POLL_WAIT);
-			}
-			spin_unlock(&np->lock);
-			break;
-		}
-		if (unlikely(i > max_interrupt_work)) {
-			spin_lock(&np->lock);
-			/* disable interrupts on the nic */
-			if (!(np->msi_flags & NV_MSI_X_ENABLED))
-				writel(0, base + NvRegIrqMask);
-			else
-				writel(np->irqmask, base + NvRegIrqMask);
-			pci_push(base);
+	if (events & NVREG_IRQ_TX_ALL) {
+		netif_rx_schedule(dev, &np->tx_napi);
+		updmask |= NVREG_IRQ_TX_ALL;
+	}
 
-			if (netif_running(dev)) {
-				np->nic_poll_irq = np->irqmask;
-				mod_timer(&np->nic_poll, jiffies + POLL_WAIT);
-			}
-			spin_unlock(&np->lock);
-			printk(KERN_DEBUG "%s: too many iterations (%d) in nv_nic_irq.\n", dev->name, i);
-			break;
-		}
+	if (updmask) {
+		np->irqmask &= ~updmask;
+
+		if (!msix)
+			writel(updmask, base + NvRegIrqMask);
+		else
+			writel(np->irqmask, base + NvRegIrqMask);
+	}
+
+	if (unlikely(events & NVREG_IRQ_LINK))
+		nv_link_irq(dev);
+
+	if (unlikely(np->need_linktimer && time_after(jiffies, np->link_timeout))) {
+		nv_linkchange(dev);
+		np->link_timeout = jiffies + LINK_TIMEOUT;
+	}
+	if (unlikely(events & (NVREG_IRQ_TX_ERR))) {
+		dprintk(KERN_DEBUG "%s: received irq with events 0x%x. Probably TX fail.\n",
+					dev->name, events);
+	}
+	if (unlikely(events & (NVREG_IRQ_UNKNOWN))) {
+		printk(KERN_DEBUG "%s: received irq with unknown events 0x%x. Please report\n",
+					dev->name, events);
+	}
+	if (unlikely(events & NVREG_IRQ_RECOVER_ERROR)) {
+		/* disable interrupts on the nic */
+		if (!msix)
+			writel(0, base + NvRegIrqMask);
+		else
+			writel(np->irqmask, base + NvRegIrqMask);
+		pci_push(base);
 
+		if (netif_running(dev)) {
+			np->nic_poll_irq = np->irqmask;
+			schedule_work(&np->reset_task);
+		}
 	}
+
+out:
+	spin_unlock(&np->lock);
+
 	dprintk(KERN_DEBUG "%s: __nv_nic_irq completed\n", dev->name);
 
-	return IRQ_RETVAL(i);
+	return IRQ_HANDLED;
 }
 
 static irqreturn_t nv_nic_irq(int foo, void *data)
@@ -3015,12 +2972,8 @@ static int nv_napi_poll(struct napi_struct *napi, int budget)
 		retcode = nv_alloc_rx_optimized(dev);
 	}
 
-	if (retcode) {
-		spin_lock_irqsave(&np->lock, flags);
-		if (netif_running(dev))
-			mod_timer(&np->oom_kick, jiffies + OOM_REFILL);
-		spin_unlock_irqrestore(&np->lock, flags);
-	}
+	if (retcode && netif_running(dev))
+		mod_timer(&np->oom_kick, jiffies + OOM_REFILL);
 
 	if (pkts < budget) {
 		/* re-enable receive interrupts */
@@ -3047,14 +3000,16 @@ static irqreturn_t nv_nic_irq_rx(int foo, void *data)
 	u32 events;
 
 	events = readl(base + NvRegMSIXIrqStatus) & NVREG_IRQ_RX_ALL;
-	writel(NVREG_IRQ_RX_ALL, base + NvRegMSIXIrqStatus);
 
-	if (events) {
+	if (likely(events)) {
+		writel(NVREG_IRQ_RX_ALL, base + NvRegMSIXIrqStatus);
+
 		netif_rx_schedule(dev, &np->napi);
 		/* disable receive interrupts on the nic */
 		writel(NVREG_IRQ_RX_ALL, base + NvRegIrqMask);
 		pci_push(base);
 	}
+
 	return IRQ_HANDLED;
 }
 
@@ -3066,14 +3021,16 @@ static irqreturn_t nv_nic_irq_tx(int foo, void *data)
 	u32 events;
 
 	events = readl(base + NvRegMSIXIrqStatus) & NVREG_IRQ_TX_ALL;
-	writel(NVREG_IRQ_TX_ALL, base + NvRegMSIXIrqStatus);
 
-	if (events) {
+	if (likely(events)) {
+		writel(NVREG_IRQ_TX_ALL, base + NvRegMSIXIrqStatus);
+
 		netif_rx_schedule(dev, &np->tx_napi);
 		/* disable receive interrupts on the nic */
 		writel(NVREG_IRQ_TX_ALL, base + NvRegIrqMask);
 		pci_push(base);
 	}
+
 	return IRQ_HANDLED;
 }
 
@@ -3115,71 +3072,46 @@ static irqreturn_t nv_nic_irq_other(int foo, void *data)
 	struct fe_priv *np = netdev_priv(dev);
 	u8 __iomem *base = get_hwbase(dev);
 	u32 events;
-	int i;
-	unsigned long flags;
 
 	dprintk(KERN_DEBUG "%s: nv_nic_irq_other\n", dev->name);
 
-	for (i=0; ; i++) {
-		events = readl(base + NvRegMSIXIrqStatus) & NVREG_IRQ_OTHER;
-		writel(NVREG_IRQ_OTHER, base + NvRegMSIXIrqStatus);
-		dprintk(KERN_DEBUG "%s: irq: %08x\n", dev->name, events);
-		if (!(events & np->irqmask))
-			break;
+	spin_lock(&np->lock);
 
-		/* check tx in case we reached max loop limit in tx isr */
-		spin_lock_irqsave(&np->lock, flags);
-		nv_tx_done_optimized(dev, TX_WORK_PER_LOOP);
-		spin_unlock_irqrestore(&np->lock, flags);
+	events = readl(base + NvRegMSIXIrqStatus) & NVREG_IRQ_OTHER;
+	writel(NVREG_IRQ_OTHER, base + NvRegMSIXIrqStatus);
 
-		if (events & NVREG_IRQ_LINK) {
-			spin_lock_irqsave(&np->lock, flags);
-			nv_link_irq(dev);
-			spin_unlock_irqrestore(&np->lock, flags);
-		}
-		if (np->need_linktimer && time_after(jiffies, np->link_timeout)) {
-			spin_lock_irqsave(&np->lock, flags);
-			nv_linkchange(dev);
-			spin_unlock_irqrestore(&np->lock, flags);
-			np->link_timeout = jiffies + LINK_TIMEOUT;
-		}
-		if (events & NVREG_IRQ_RECOVER_ERROR) {
-			spin_lock_irq(&np->lock);
-			/* disable interrupts on the nic */
-			writel(NVREG_IRQ_OTHER, base + NvRegIrqMask);
-			pci_push(base);
+	dprintk(KERN_DEBUG "%s: irq: %08x\n", dev->name, events);
+	if (!(events & np->irqmask))
+		goto out;
 
-			if (netif_running(dev)) {
-				np->nic_poll_irq |= NVREG_IRQ_OTHER;
-				np->recover_error = 1;
-				mod_timer(&np->nic_poll, jiffies + POLL_WAIT);
-			}
-			spin_unlock_irq(&np->lock);
-			break;
-		}
-		if (events & (NVREG_IRQ_UNKNOWN)) {
-			printk(KERN_DEBUG "%s: received irq with unknown events 0x%x. Please report\n",
-						dev->name, events);
-		}
-		if (unlikely(i > max_interrupt_work)) {
-			spin_lock_irqsave(&np->lock, flags);
-			/* disable interrupts on the nic */
-			writel(NVREG_IRQ_OTHER, base + NvRegIrqMask);
-			pci_push(base);
+	if (events & NVREG_IRQ_LINK)
+		nv_link_irq(dev);
 
-			if (netif_running(dev)) {
-				np->nic_poll_irq |= NVREG_IRQ_OTHER;
-				mod_timer(&np->nic_poll, jiffies + POLL_WAIT);
-			}
-			spin_unlock_irqrestore(&np->lock, flags);
-			printk(KERN_DEBUG "%s: too many iterations (%d) in nv_nic_irq_other.\n", dev->name, i);
-			break;
-		}
+	if (np->need_linktimer && time_after(jiffies, np->link_timeout)) {
+		nv_linkchange(dev);
+		np->link_timeout = jiffies + LINK_TIMEOUT;
+	}
+	if (events & NVREG_IRQ_RECOVER_ERROR) {
+		/* disable interrupts on the nic */
+		writel(NVREG_IRQ_OTHER, base + NvRegIrqMask);
+		pci_push(base);
 
+		if (netif_running(dev)) {
+			np->nic_poll_irq |= NVREG_IRQ_OTHER;
+			schedule_work(&np->reset_task);
+		}
+	}
+	if (events & (NVREG_IRQ_UNKNOWN)) {
+		printk(KERN_DEBUG "%s: received irq with unknown events 0x%x. Please report\n",
+					dev->name, events);
 	}
+
+out:
+	spin_unlock(&np->lock);
+
 	dprintk(KERN_DEBUG "%s: nv_nic_irq_other completed\n", dev->name);
 
-	return IRQ_RETVAL(i);
+	return IRQ_HANDLED;
 }
 
 static irqreturn_t nv_nic_irq_test(int foo, void *data)
@@ -3201,7 +3133,7 @@ static irqreturn_t nv_nic_irq_test(int foo, void *data)
 	pci_push(base);
 	dprintk(KERN_DEBUG "%s: irq: %08x\n", dev->name, events);
 	if (!(events & NVREG_IRQ_TIMER))
-		return IRQ_RETVAL(0);
+		return IRQ_NONE;
 
 	spin_lock(&np->lock);
 	np->intr_test = 1;
@@ -3209,7 +3141,7 @@ static irqreturn_t nv_nic_irq_test(int foo, void *data)
 
 	dprintk(KERN_DEBUG "%s: nv_nic_irq_test completed\n", dev->name);
 
-	return IRQ_RETVAL(1);
+	return IRQ_HANDLED;
 }
 
 static void set_msix_vector_map(struct net_device *dev, u32 vector, u32 irqmask)
@@ -3356,11 +3288,11 @@ static void nv_free_irq(struct net_device *dev)
 	}
 }
 
-static void nv_do_nic_poll(unsigned long data)
+static void nv_reset_task(struct work_struct *work)
 {
-	struct net_device *dev = (struct net_device *) data;
-	struct fe_priv *np = netdev_priv(dev);
-	u8 __iomem *base = get_hwbase(dev);
+	struct fe_priv *np = container_of(work, struct fe_priv, reset_task);
+	struct net_device *dev = np->dev;
+	u8 __iomem *base = np->base;
 	u32 mask = 0;
 
 	/*
@@ -3389,39 +3321,38 @@ static void nv_do_nic_poll(unsigned long data)
 			mask |= NVREG_IRQ_OTHER;
 		}
 	}
+
+	dev_printk(KERN_INFO, &np->pci_dev->dev, "resetting MAC "
+		   "(pimask %x, mask %x)\n",
+		   np->nic_poll_irq, mask);
+
 	np->nic_poll_irq = 0;
 
-	if (np->recover_error) {
-		np->recover_error = 0;
-		printk(KERN_INFO "forcedeth: MAC in recoverable error state\n");
-		if (netif_running(dev)) {
-			netif_tx_lock_bh(dev);
-			spin_lock(&np->lock);
-			/* stop engines */
-			nv_stop_rxtx(dev);
-			nv_txrx_reset(dev);
-			/* drain rx queue */
-			nv_drain_rxtx(dev);
-			/* reinit driver view of the rx queue */
-			set_bufsize(dev);
-			if (nv_init_ring(dev))
-				mod_timer(&np->oom_kick, jiffies + OOM_REFILL);
+	netif_tx_lock_bh(dev);
+	spin_lock(&np->lock);
+	/* stop engines */
+	nv_stop_rxtx(dev);
+	nv_txrx_reset(dev);
+	/* drain rx queue */
+	nv_drain_rxtx(dev);
+	/* reinit driver view of the rx queue */
+	set_bufsize(dev);
+	if (nv_init_ring(dev))
+		mod_timer(&np->oom_kick, jiffies + OOM_REFILL);
 
-			/* reinit nic view of the rx queue */
-			writel(np->rx_buf_sz, base + NvRegOffloadConfig);
-			setup_hw_rings(dev, NV_SETUP_RX_RING | NV_SETUP_TX_RING);
-			writel( ((np->rx_ring_size-1) << NVREG_RINGSZ_RXSHIFT) + ((np->tx_ring_size-1) << NVREG_RINGSZ_TXSHIFT),
-				base + NvRegRingSizes);
-			pci_push(base);
-			writel(NVREG_TXRXCTL_KICK|np->txrxctl_bits, get_hwbase(dev) + NvRegTxRxControl);
-			pci_push(base);
+	/* reinit nic view of the rx queue */
+	writel(np->rx_buf_sz, base + NvRegOffloadConfig);
+	setup_hw_rings(dev, NV_SETUP_RX_RING | NV_SETUP_TX_RING);
+	writel( ((np->rx_ring_size-1) << NVREG_RINGSZ_RXSHIFT) + ((np->tx_ring_size-1) << NVREG_RINGSZ_TXSHIFT),
+		base + NvRegRingSizes);
+	pci_push(base);
+	writel(NVREG_TXRXCTL_KICK|np->txrxctl_bits, get_hwbase(dev) + NvRegTxRxControl);
+	pci_push(base);
 
-			/* restart rx engine */
-			nv_start_rxtx(dev);
-			spin_unlock(&np->lock);
-			netif_tx_unlock_bh(dev);
-		}
-	}
+	/* restart rx engine */
+	nv_start_rxtx(dev);
+	spin_unlock(&np->lock);
+	netif_tx_unlock_bh(dev);
 
 	/* FIXME: Do we need synchronize_irq(dev->irq) here? */
 
@@ -3456,7 +3387,13 @@ static void nv_do_nic_poll(unsigned long data)
 #ifdef CONFIG_NET_POLL_CONTROLLER
 static void nv_poll_controller(struct net_device *dev)
 {
-	nv_do_nic_poll((unsigned long) dev);
+	struct fe_priv *np = netdev_priv(dev);
+	bool optimized = nv_optimized(np);
+	unsigned long flags;
+
+	local_irq_save(flags);
+	__nv_nic_irq(dev, optimized);
+	local_irq_restore(flags);
 }
 #endif
 
@@ -4667,18 +4604,16 @@ static int nv_close(struct net_device *dev)
 	struct fe_priv *np = netdev_priv(dev);
 	u8 __iomem *base;
 
-	spin_lock_irq(&np->lock);
-	spin_unlock_irq(&np->lock);
+	cancel_work_sync(&np->reset_task);
+	cancel_delayed_work_sync(&np->stats_task);
 
 	napi_disable(&np->tx_napi);
 	napi_disable(&np->napi);
-	synchronize_irq(dev->irq);
 
 	del_timer_sync(&np->oom_kick);
-	del_timer_sync(&np->nic_poll);
-	cancel_delayed_work_sync(&np->stats_task);
 
-	netif_stop_queue(dev);
+	netif_tx_disable(dev);
+
 	spin_lock_irq(&np->lock);
 	nv_stop_rxtx(dev);
 	nv_txrx_reset(dev);
@@ -4736,9 +4671,7 @@ static int __devinit nv_probe(struct pci_dev *pci_dev, const struct pci_device_i
 	init_timer(&np->oom_kick);
 	np->oom_kick.data = (unsigned long) dev;
 	np->oom_kick.function = &nv_do_rx_refill;	/* timer handler */
-	init_timer(&np->nic_poll);
-	np->nic_poll.data = (unsigned long) dev;
-	np->nic_poll.function = &nv_do_nic_poll;	/* timer handler */
+	INIT_WORK(&np->reset_task, nv_reset_task);
 	INIT_DELAYED_WORK(&np->stats_task, nv_stats_task);
 
 	err = pci_enable_device(pci_dev);
@@ -5337,8 +5270,6 @@ static void __exit exit_nic(void)
 	pci_unregister_driver(&driver);
 }
 
-module_param(max_interrupt_work, int, 0);
-MODULE_PARM_DESC(max_interrupt_work, "forcedeth maximum events handled per interrupt");
 module_param(optimization_mode, int, 0);
 MODULE_PARM_DESC(optimization_mode, "In throughput mode (0), every tx & rx packet will generate an interrupt. In CPU mode (1), interrupts are controlled by a timer.");
 module_param(poll_interval, int, 0);

^ permalink raw reply related	[flat|nested] 8+ messages in thread

* Re: [PATCH 0/6] forcedeth interrupt and task overhaul, v2
  2007-10-17  5:52 [PATCH 0/6] forcedeth interrupt and task overhaul, v2 Jeff Garzik
                   ` (5 preceding siblings ...)
  2007-10-17  5:55 ` [PATCH 6/6] [netdrvr] interrupt handling overhaul Jeff Garzik
@ 2007-10-17  5:59 ` Jeff Garzik
  6 siblings, 0 replies; 8+ messages in thread
From: Jeff Garzik @ 2007-10-17  5:59 UTC (permalink / raw)
  To: netdev, LKML; +Cc: Manfred Spraul, Ingo Molnar, Ayaz Abdulla

Jeff Garzik wrote:
> These six changes can be found in the 'fe' branch of
> git://git.kernel.org/pub/scm/linux/kernel/git/jgarzik/netdev-2.6.git

Note the change from the previous 'fe-lock' branch, which is now left 
as-is on kernel.org, but not being updated.



^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2007-10-17  5:59 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2007-10-17  5:52 [PATCH 0/6] forcedeth interrupt and task overhaul, v2 Jeff Garzik
2007-10-17  5:53 ` [PATCH 1/6] forcedeth: internal simplifications; changelog removal Jeff Garzik
2007-10-17  5:53 ` [PATCH 2/6] forcedeth: timer overhaul Jeff Garzik
2007-10-17  5:54 ` [PATCH 3/6] forcedeth: eliminate some duplicate irq handling code Jeff Garzik
2007-10-17  5:54 ` [PATCH 4/6] forcedeth: unconditionally enable NAPI Jeff Garzik
2007-10-17  5:54 ` [PATCH 5/6] forcedeth: use NAPI for TX completion Jeff Garzik
2007-10-17  5:55 ` [PATCH 6/6] [netdrvr] interrupt handling overhaul Jeff Garzik
2007-10-17  5:59 ` [PATCH 0/6] forcedeth interrupt and task overhaul, v2 Jeff Garzik

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