Re: PATCH SB1250 NAPI support

["Tom Rix" <trix@specifix.com>]

[-- Attachment #1: Type: text/plain, Size: 1394 bytes --]

Here is a revised patch that does the interupt masking earlier.  Since  
resetting the mask is done now in 3 places, I created a macro to handle  
it.  This patch has been tested on the linux-mips kernel from 6/29/06 on a  
sb1250.
Tom

On Thu, 29 Jun 2006 15:01:07 -0500, Francois Romieu <romieu@fr.zoreil.com>  
wrote:

> Tom Rix <trix@specifix.com> :
> [...]
> diff -rup a/drivers/net/sb1250-mac.c b/drivers/net/sb1250-mac.c
> --- a/drivers/net/sb1250-mac.c	2006-03-09 04:25:41.000000000 -0600
> +++ b/drivers/net/sb1250-mac.c	2006-03-09 05:30:52.000000000 -0600
> [...]
> @@ -2079,13 +2095,31 @@ static irqreturn_t sbmac_intr(int irq,vo
>  		 * Transmits on channel 0
>  		 */
> +#if defined(CONFIG_SBMAC_NAPI)
>  		if (isr & (M_MAC_INT_CHANNEL << S_MAC_TX_CH0)) {
> -			sbdma_tx_process(sc,&(sc->sbm_txdma));
> +			sbdma_tx_process(sc,&(sc->sbm_txdma), 0);
>  		}
> 		/*
>  		 * Receives on channel 0
>  		 */
> +		if (isr & (M_MAC_INT_CHANNEL << S_MAC_RX_CH0)) {
> +			if (netif_rx_schedule_prep(dev))
> +			{
>
> An irq could appear here. I am skeptical that it is safe to write
> the irq mask register so late.
>
> One should probably consider a break in the enclosing "for" loop too.
>
>
> +				__raw_writeq(0, sc->sbm_imr);
> +				__netif_rx_schedule(dev);
> +			}
> +			else
> +			{
>
> } else {, please.
>



-- 
Using Opera's revolutionary e-mail client: http://www.opera.com/mail/

[-- Attachment #2: mips-sb1250-mac-NAPI-5.patch --]
[-- Type: application/octet-stream, Size: 11138 bytes --]

--- a/drivers/net/Kconfig	2006-07-01 06:29:11.000000000 -0500
+++ b/drivers/net/Kconfig	2006-07-01 06:28:58.000000000 -0500
@@ -2013,10 +2013,6 @@ config R8169_NAPI
 
 	  If in doubt, say N.
 
-config NET_SB1250_MAC
-	tristate "SB1250 Ethernet support"
-	depends on SIBYTE_SB1xxx_SOC
-
 config R8169_VLAN
 	bool "VLAN support"
 	depends on R8169 && VLAN_8021Q
@@ -2026,6 +2022,27 @@ config R8169_VLAN
 	  
 	  If in doubt, say Y.
 
+config NET_SB1250_MAC
+	tristate "SB1250 Ethernet support"
+	depends on SIBYTE_SB1xxx_SOC
+
+config SBMAC_NAPI
+	bool "Use Rx Polling (NAPI) (EXPERIMENTAL)"
+	depends on NET_SB1250_MAC && 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 y.
+
 config SIS190
 	tristate "SiS190/SiS191 gigabit ethernet support"
 	depends on PCI
--- a/drivers/net/sb1250-mac.c	2006-07-01 06:29:27.000000000 -0500
+++ b/drivers/net/sb1250-mac.c	2006-07-01 06:29:18.000000000 -0500
@@ -164,6 +164,25 @@ typedef enum { sbmac_state_uninit, sbmac
 #define ENET_PACKET_SIZE	1518
 /*#define ENET_PACKET_SIZE	9216 */
 
+
+#ifdef CONFIG_SBMAC_COALESCE
+/*
+ * Accept any TX interrupt and EOP count/timer RX interrupts on ch 0
+ */
+#define SET_INTR_MASK(S) \
+__raw_writeq(((M_MAC_INT_EOP_COUNT | M_MAC_INT_EOP_TIMER) << S_MAC_TX_CH0) | \
+	     ((M_MAC_INT_EOP_COUNT | M_MAC_INT_EOP_TIMER) << S_MAC_RX_CH0), \
+	     (S)->sbm_imr);
+#else
+/*
+ * Accept any kind of interrupt on TX and RX DMA channel 0
+ */
+#define SET_INTR_MASK(S) \
+__raw_writeq((M_MAC_INT_CHANNEL << S_MAC_TX_CH0) | \
+             (M_MAC_INT_CHANNEL << S_MAC_RX_CH0), (S)->sbm_imr);
+#endif
+
+
 /**********************************************************************
  *  DMA Descriptor structure
  ********************************************************************* */
@@ -205,6 +224,7 @@ typedef struct sbmacdma_s {
 	 * This stuff is for maintenance of the ring
 	 */
 
+	sbdmadscr_t     *sbdma_dscrtable_unaligned; 
 	sbdmadscr_t     *sbdma_dscrtable;	/* base of descriptor table */
 	sbdmadscr_t     *sbdma_dscrtable_end; /* end of descriptor table */
 
@@ -287,8 +307,8 @@ static int sbdma_add_rcvbuffer(sbmacdma_
 static int sbdma_add_txbuffer(sbmacdma_t *d,struct sk_buff *m);
 static void sbdma_emptyring(sbmacdma_t *d);
 static void sbdma_fillring(sbmacdma_t *d);
-static void sbdma_rx_process(struct sbmac_softc *sc,sbmacdma_t *d);
-static void sbdma_tx_process(struct sbmac_softc *sc,sbmacdma_t *d);
+static int sbdma_rx_process(struct sbmac_softc *sc,sbmacdma_t *d, int poll);
+static void sbdma_tx_process(struct sbmac_softc *sc,sbmacdma_t *d, int poll);
 static int sbmac_initctx(struct sbmac_softc *s);
 static void sbmac_channel_start(struct sbmac_softc *s);
 static void sbmac_channel_stop(struct sbmac_softc *s);
@@ -309,6 +329,10 @@ static struct net_device_stats *sbmac_ge
 static void sbmac_set_rx_mode(struct net_device *dev);
 static int sbmac_mii_ioctl(struct net_device *dev, struct ifreq *rq, int cmd);
 static int sbmac_close(struct net_device *dev);
+#if defined (CONFIG_SBMAC_NAPI)
+static int sbmac_poll(struct net_device *poll_dev, int *budget);
+#endif
+
 static int sbmac_mii_poll(struct sbmac_softc *s,int noisy);
 static int sbmac_mii_probe(struct net_device *dev);
 
@@ -736,7 +760,7 @@ static void sbdma_initctx(sbmacdma_t *d,
 
 	d->sbdma_maxdescr = maxdescr;
 
-	d->sbdma_dscrtable = (sbdmadscr_t *)
+	d->sbdma_dscrtable_unaligned = d->sbdma_dscrtable = (sbdmadscr_t *)
 		kmalloc((d->sbdma_maxdescr+1)*sizeof(sbdmadscr_t), GFP_KERNEL);
 
 	/*
@@ -778,7 +802,6 @@ static void sbdma_initctx(sbmacdma_t *d,
 		d->sbdma_int_timeout = 0;
 	}
 #endif
-
 }
 
 /**********************************************************************
@@ -1146,13 +1169,14 @@ static void sbdma_fillring(sbmacdma_t *d
  *  	   nothing
  ********************************************************************* */
 
-static void sbdma_rx_process(struct sbmac_softc *sc,sbmacdma_t *d)
+static int sbdma_rx_process(struct sbmac_softc *sc,sbmacdma_t *d, int poll)
 {
 	int curidx;
 	int hwidx;
 	sbdmadscr_t *dsc;
 	struct sk_buff *sb;
 	int len;
+	int work_done = 0;
 
 	for (;;) {
 		/*
@@ -1230,8 +1254,15 @@ static void sbdma_rx_process(struct sbma
 						sb->ip_summed = CHECKSUM_NONE;
 					}
 				}
-
+				
+#if defined(CONFIG_SBMAC_NAPI)
+				if (0 == poll)
+					netif_rx(sb);
+				else
+					netif_receive_skb(sb);
+#else
 				netif_rx(sb);
+#endif				
 			}
 		} else {
 			/*
@@ -1248,8 +1279,9 @@ static void sbdma_rx_process(struct sbma
 		 */
 
 		d->sbdma_remptr = SBDMA_NEXTBUF(d,sbdma_remptr);
-
+		work_done++;
 	}
+	return work_done;
 }
 
 
@@ -1271,15 +1303,22 @@ static void sbdma_rx_process(struct sbma
  *  	   nothing
  ********************************************************************* */
 
-static void sbdma_tx_process(struct sbmac_softc *sc,sbmacdma_t *d)
+static void sbdma_tx_process(struct sbmac_softc *sc,sbmacdma_t *d, int poll)
 {
 	int curidx;
 	int hwidx;
 	sbdmadscr_t *dsc;
 	struct sk_buff *sb;
 	unsigned long flags;
+	int packets_handled = 0;
 
 	spin_lock_irqsave(&(sc->sbm_lock), flags);
+	
+	if (d->sbdma_remptr == d->sbdma_addptr)
+		goto end_unlock;
+
+	hwidx = (int) (((__raw_readq(d->sbdma_curdscr) & M_DMA_CURDSCR_ADDR) -
+			d->sbdma_dscrtable_phys) / sizeof(sbdmadscr_t));
 
 	for (;;) {
 		/*
@@ -1294,15 +1333,12 @@ static void sbdma_tx_process(struct sbma
 		 */
 
 		curidx = d->sbdma_remptr - d->sbdma_dscrtable;
-		hwidx = (int) (((__raw_readq(d->sbdma_curdscr) & M_DMA_CURDSCR_ADDR) -
-				d->sbdma_dscrtable_phys) / sizeof(sbdmadscr_t));
 
 		/*
 		 * If they're the same, that means we've processed all
 		 * of the descriptors up to (but not including) the one that
 		 * the hardware is working on right now.
 		 */
-
 		if (curidx == hwidx)
 			break;
 
@@ -1333,6 +1369,7 @@ static void sbdma_tx_process(struct sbma
 
 		d->sbdma_remptr = SBDMA_NEXTBUF(d,sbdma_remptr);
 
+		packets_handled++;
 	}
 
 	/*
@@ -1340,15 +1377,15 @@ static void sbdma_tx_process(struct sbma
 	 * Other drivers seem to do this when we reach a low
 	 * watermark on the transmit queue.
 	 */
+	
+	if (packets_handled)
+		netif_wake_queue(d->sbdma_eth->sbm_dev);
 
-	netif_wake_queue(d->sbdma_eth->sbm_dev);
-
+ end_unlock:
 	spin_unlock_irqrestore(&(sc->sbm_lock), flags);
 
 }
 
-
-
 /**********************************************************************
  *  SBMAC_INITCTX(s)
  *
@@ -1392,7 +1429,6 @@ static int sbmac_initctx(struct sbmac_so
 	 * Initialize the DMA channels.  Right now, only one per MAC is used
 	 * Note: Only do this _once_, as it allocates memory from the kernel!
 	 */
-
 	sbdma_initctx(&(s->sbm_txdma),s,0,DMA_TX,SBMAC_MAX_TXDESCR);
 	sbdma_initctx(&(s->sbm_rxdma),s,0,DMA_RX,SBMAC_MAX_RXDESCR);
 
@@ -1416,9 +1452,9 @@ static int sbmac_initctx(struct sbmac_so
 
 static void sbdma_uninitctx(struct sbmacdma_s *d)
 {
-	if (d->sbdma_dscrtable) {
-		kfree(d->sbdma_dscrtable);
-		d->sbdma_dscrtable = NULL;
+	if (d->sbdma_dscrtable_unaligned) {
+		kfree(d->sbdma_dscrtable_unaligned);
+		d->sbdma_dscrtable_unaligned = d->sbdma_dscrtable = NULL;
 	}
 
 	if (d->sbdma_ctxtable) {
@@ -1605,29 +1641,16 @@ static void sbmac_channel_start(struct s
 
 #if defined(CONFIG_SIBYTE_BCM1x55) || defined(CONFIG_SIBYTE_BCM1x80)
 	__raw_writeq(M_MAC_RXDMA_EN0 |
-		       M_MAC_TXDMA_EN0, s->sbm_macenable);
+		     M_MAC_TXDMA_EN0, s->sbm_macenable);
 #elif defined(CONFIG_SIBYTE_SB1250) || defined(CONFIG_SIBYTE_BCM112X)
 	__raw_writeq(M_MAC_RXDMA_EN0 |
-		       M_MAC_TXDMA_EN0 |
-		       M_MAC_RX_ENABLE |
-		       M_MAC_TX_ENABLE, s->sbm_macenable);
+		     M_MAC_TXDMA_EN0 |
+		     M_MAC_RX_ENABLE |
+		     M_MAC_TX_ENABLE, s->sbm_macenable);
 #else
 #error invalid SiByte MAC configuation
 #endif
-
-#ifdef CONFIG_SBMAC_COALESCE
-	/*
-	 * Accept any TX interrupt and EOP count/timer RX interrupts on ch 0
-	 */
-	__raw_writeq(((M_MAC_INT_EOP_COUNT | M_MAC_INT_EOP_TIMER) << S_MAC_TX_CH0) |
-		       ((M_MAC_INT_EOP_COUNT | M_MAC_INT_EOP_TIMER) << S_MAC_RX_CH0), s->sbm_imr);
-#else
-	/*
-	 * Accept any kind of interrupt on TX and RX DMA channel 0
-	 */
-	__raw_writeq((M_MAC_INT_CHANNEL << S_MAC_TX_CH0) |
-		       (M_MAC_INT_CHANNEL << S_MAC_RX_CH0), s->sbm_imr);
-#endif
+	SET_INTR_MASK(s);
 
 	/*
 	 * Enable receiving unicasts and broadcasts
@@ -2063,7 +2086,6 @@ static irqreturn_t sbmac_intr(int irq,vo
 		 * Read the ISR (this clears the bits in the real
 		 * register, except for counter addr)
 		 */
-
 		isr = __raw_readq(sc->sbm_isr) & ~M_MAC_COUNTER_ADDR;
 
 		if (isr == 0)
@@ -2075,13 +2097,33 @@ static irqreturn_t sbmac_intr(int irq,vo
 		 * Transmits on channel 0
 		 */
 
+#if defined(CONFIG_SBMAC_NAPI)
+		if (isr & (M_MAC_INT_CHANNEL << S_MAC_RX_CH0)) {
+			__raw_writeq(0, sc->sbm_imr);
+		}
+
 		if (isr & (M_MAC_INT_CHANNEL << S_MAC_TX_CH0)) {
-			sbdma_tx_process(sc,&(sc->sbm_txdma));
+			sbdma_tx_process(sc,&(sc->sbm_txdma), 0);
 		}
 
 		/*
 		 * Receives on channel 0
 		 */
+		if (isr & (M_MAC_INT_CHANNEL << S_MAC_RX_CH0)) {
+			if (netif_rx_schedule_prep(dev))
+			{
+				__netif_rx_schedule(dev);
+			} else {
+				SET_INTR_MASK(sc);
+				sbdma_rx_process(sc,&(sc->sbm_rxdma), 0);
+			}
+		}
+#else
+		/* Non NAPI */
+
+		if (isr & (M_MAC_INT_CHANNEL << S_MAC_TX_CH0)) {
+			sbdma_tx_process(sc,&(sc->sbm_txdma), 0);
+		}
 
 		/*
 		 * It's important to test all the bits (or at least the
@@ -2101,8 +2143,9 @@ static irqreturn_t sbmac_intr(int irq,vo
 
 
 		if (isr & (M_MAC_INT_CHANNEL << S_MAC_RX_CH0)) {
-			sbdma_rx_process(sc,&(sc->sbm_rxdma));
+			sbdma_rx_process(sc,&(sc->sbm_rxdma), 0);
 		}
+#endif
 	}
 	return IRQ_RETVAL(handled);
 }
@@ -2401,7 +2444,10 @@ static int sbmac_init(struct net_device 
 	dev->do_ioctl           = sbmac_mii_ioctl;
 	dev->tx_timeout         = sbmac_tx_timeout;
 	dev->watchdog_timeo     = TX_TIMEOUT;
-
+#if defined(CONFIG_SBMAC_NAPI)
+	dev->poll               = sbmac_poll;
+	dev->weight             = 16;
+#endif
 	dev->change_mtu         = sb1250_change_mtu;
 
 	/* This is needed for PASS2 for Rx H/W checksum feature */
@@ -2809,7 +2855,29 @@ static int sbmac_close(struct net_device
 	return 0;
 }
 
+#if defined(CONFIG_SBMAC_NAPI)
+static int sbmac_poll(struct net_device *dev, int *budget)
+{
+	int work_to_do;
+	int work_done;
+	struct sbmac_softc *sc = netdev_priv(dev);
+
+	work_to_do = min(*budget, dev->quota);
+	work_done = sbdma_rx_process(sc,&(sc->sbm_rxdma), 1);
 
+	sbdma_tx_process(sc,&(sc->sbm_txdma), 1);
+
+	*budget -= work_done;
+	dev->quota -= work_done;
+
+	if (work_done < work_to_do) {
+		netif_rx_complete(dev);
+		SET_INTR_MASK(sc);
+	}
+
+	return (work_done >= work_to_do);
+}
+#endif
 
 #if defined(SBMAC_ETH0_HWADDR) || defined(SBMAC_ETH1_HWADDR) || defined(SBMAC_ETH2_HWADDR) || defined(SBMAC_ETH3_HWADDR)
 static void