[PATCH 0/3] chelsio driver changes

[PATCH 0/3] chelsio driver changes

[Stephen Hemminger]

One error path bug fix, and a couple of changes to improve
receive performance.  The performance is now up to 5+ Gbits/sec
which is about the limit of the memory/bus on the test systems.

[PATCH 1/3] chelsio: fix error path

[Stephen Hemminger]

[-- Attachment #1: chelsio-coverity.patch --]
[-- Type: text/plain, Size: 625 bytes --]

Fix handling of allocation failure.

Signed-off-by: Stephen Hemminger <shemminger@osdl.org>

--- linux-2.6.20-rc1.orig/drivers/net/chelsio/my3126.c
+++ linux-2.6.20-rc1/drivers/net/chelsio/my3126.c
@@ -170,13 +170,14 @@ static struct cphy *my3126_phy_create(ad
 {
 	struct cphy *cphy = kzalloc(sizeof (*cphy), GFP_KERNEL);
 
-	if (cphy)
-		cphy_init(cphy, adapter, phy_addr, &my3126_ops, mdio_ops);
+	if (!cphy)
+		return NULL;
 
+	cphy_init(cphy, adapter, phy_addr, &my3126_ops, mdio_ops);
 	INIT_DELAYED_WORK(&cphy->phy_update, my3216_poll);
 	cphy->bmsr = 0;
 
-	return (cphy);
+	return cphy;
 }
 
 /* Chip Reset */

-- 

[PATCH 2/3] chelsio: NAPI speed improvement

[Stephen Hemminger]

[-- Attachment #1: chelsio-napi2.patch --]
[-- Type: text/plain, Size: 4598 bytes --]

Speedup and cleanup the receive processing by eliminating the
mmio read and a lock round trip.

Signed-off-by: Stephen Hemminger <shemminger@osdl.org>

---
 drivers/net/chelsio/sge.c |   82 ++++++++++++++++++++--------------------------
 1 file changed, 37 insertions(+), 45 deletions(-)

--- linux-2.6.20-rc1.orig/drivers/net/chelsio/sge.c
+++ linux-2.6.20-rc1/drivers/net/chelsio/sge.c
@@ -1576,6 +1576,14 @@ static int process_responses(struct adap
 	return budget;
 }
 
+static inline int responses_pending(const struct adapter *adapter)
+{
+	const struct respQ *Q = &adapter->sge->respQ;
+	const struct respQ_e *e = &Q->entries[Q->cidx];
+
+	return (e->GenerationBit == Q->genbit);
+}
+
 #ifdef CONFIG_CHELSIO_T1_NAPI
 /*
  * A simpler version of process_responses() that handles only pure (i.e.,
@@ -1585,13 +1593,16 @@ static int process_responses(struct adap
  * which the caller must ensure is a valid pure response.  Returns 1 if it
  * encounters a valid data-carrying response, 0 otherwise.
  */
-static int process_pure_responses(struct adapter *adapter, struct respQ_e *e)
+static int process_pure_responses(struct adapter *adapter)
 {
 	struct sge *sge = adapter->sge;
 	struct respQ *q = &sge->respQ;
+	struct respQ_e *e = &q->entries[q->cidx];
 	unsigned int flags = 0;
 	unsigned int cmdq_processed[SGE_CMDQ_N] = {0, 0};
 
+	if (e->DataValid)
+		return 1;
 	do {
 		flags |= e->Qsleeping;
 
@@ -1627,22 +1638,17 @@ static int process_pure_responses(struct
 int t1_poll(struct net_device *dev, int *budget)
 {
 	struct adapter *adapter = dev->priv;
-	int effective_budget = min(*budget, dev->quota);
-	int work_done = process_responses(adapter, effective_budget);
+	int work_done;
 
+	work_done = process_responses(adapter, min(*budget, dev->quota));
 	*budget -= work_done;
 	dev->quota -= work_done;
 
-	if (work_done >= effective_budget)
+	if (unlikely(responses_pending(adapter)))
 		return 1;
 
- 	spin_lock_irq(&adapter->async_lock);
-	__netif_rx_complete(dev);
+	netif_rx_complete(dev);
 	writel(adapter->sge->respQ.cidx, adapter->regs + A_SG_SLEEPING);
-	writel(adapter->slow_intr_mask | F_PL_INTR_SGE_DATA,
-	       adapter->regs + A_PL_ENABLE);
- 	spin_unlock_irq(&adapter->async_lock);
-
 	return 0;
 }
 
@@ -1652,44 +1658,34 @@ int t1_poll(struct net_device *dev, int 
 irqreturn_t t1_interrupt(int irq, void *data)
 {
 	struct adapter *adapter = data;
- 	struct net_device *dev = adapter->sge->netdev;
 	struct sge *sge = adapter->sge;
- 	u32 cause;
-	int handled = 0;
+	int handled;
 
-	cause = readl(adapter->regs + A_PL_CAUSE);
-	if (cause == 0 || cause == ~0)
-		return IRQ_NONE;
+	if (likely(responses_pending(adapter))) {
+		struct net_device *dev = sge->netdev;
 
-	spin_lock(&adapter->async_lock);
- 	if (cause & F_PL_INTR_SGE_DATA) {
-		struct respQ *q = &adapter->sge->respQ;
-		struct respQ_e *e = &q->entries[q->cidx];
-
- 		handled = 1;
- 		writel(F_PL_INTR_SGE_DATA, adapter->regs + A_PL_CAUSE);
-
-		if (e->GenerationBit == q->genbit &&
-		    __netif_rx_schedule_prep(dev)) {
-			if (e->DataValid || process_pure_responses(adapter, e)) {
-				/* mask off data IRQ */
-				writel(adapter->slow_intr_mask,
-				       adapter->regs + A_PL_ENABLE);
-				__netif_rx_schedule(sge->netdev);
-				goto unlock;
-			}
-			/* no data, no NAPI needed */
-			netif_poll_enable(dev);
+		writel(F_PL_INTR_SGE_DATA, adapter->regs + A_PL_CAUSE);
 
+		if (__netif_rx_schedule_prep(dev)) {
+			if (process_pure_responses(adapter)) {
+				__netif_rx_schedule(dev);
+			} else {
+				/* no data, no NAPI needed */
+				writel(sge->respQ.cidx,
+				       adapter->regs + A_SG_SLEEPING);
+				netif_poll_enable(dev);	/* undo schedule_prep */
+			}
 		}
-		writel(q->cidx, adapter->regs + A_SG_SLEEPING);
-	} else
-		handled = t1_slow_intr_handler(adapter);
+		return IRQ_HANDLED;
+	}
+
+	spin_lock(&adapter->async_lock);
+	handled = t1_slow_intr_handler(adapter);
+	spin_unlock(&adapter->async_lock);
 
 	if (!handled)
 		sge->stats.unhandled_irqs++;
-unlock:
-	spin_unlock(&adapter->async_lock);
+
 	return IRQ_RETVAL(handled != 0);
 }
 
@@ -1712,17 +1708,13 @@ unlock:
 irqreturn_t t1_interrupt(int irq, void *cookie)
 {
 	int work_done;
-	struct respQ_e *e;
 	struct adapter *adapter = cookie;
-	struct respQ *Q = &adapter->sge->respQ;
 
 	spin_lock(&adapter->async_lock);
-	e = &Q->entries[Q->cidx];
-	prefetch(e);
 
 	writel(F_PL_INTR_SGE_DATA, adapter->regs + A_PL_CAUSE);
 
-	if (likely(e->GenerationBit == Q->genbit))
+	if (likely(responses_pending(adapter))
 		work_done = process_responses(adapter, -1);
 	else
 		work_done = t1_slow_intr_handler(adapter);

-- 

[PATCH 3/3] chelsio: more receive cleanup

[Stephen Hemminger]

[-- Attachment #1: chelsio-prefetch --]
[-- Type: text/plain, Size: 6939 bytes --]

Cleanup receive processing some more:
   * do the reserve padding of skb during setup
   * don't pass constants to get_packet
   * do smart prefetch of skb
   * make copybreak a module parameter

Signed-off-by: Stephen Hemminger <shemminger@osdl.org>

---
 drivers/net/chelsio/sge.c |   81 +++++++++++++++++++++++-----------------------
 1 file changed, 42 insertions(+), 39 deletions(-)

--- linux-2.6.20-rc1.orig/drivers/net/chelsio/sge.c
+++ linux-2.6.20-rc1/drivers/net/chelsio/sge.c
@@ -71,12 +71,9 @@
 #define SGE_FREEL_REFILL_THRESH	16
 #define SGE_RESPQ_E_N		1024
 #define SGE_INTRTIMER_NRES	1000
-#define SGE_RX_COPY_THRES	256
 #define SGE_RX_SM_BUF_SIZE	1536
 #define SGE_TX_DESC_MAX_PLEN	16384
 
-# define SGE_RX_DROP_THRES 2
-
 #define SGE_RESPQ_REPLENISH_THRES (SGE_RESPQ_E_N / 4)
 
 /*
@@ -862,6 +859,8 @@ static void refill_free_list(struct sge 
 		skb_reserve(skb, q->dma_offset);
 		mapping = pci_map_single(pdev, skb->data, dma_len,
 					 PCI_DMA_FROMDEVICE);
+		skb_reserve(skb, sge->rx_pkt_pad);
+
 		ce->skb = skb;
 		pci_unmap_addr_set(ce, dma_addr, mapping);
 		pci_unmap_len_set(ce, dma_len, dma_len);
@@ -1041,6 +1040,10 @@ static void recycle_fl_buf(struct freelQ
 	}
 }
 
+static int copybreak __read_mostly = 256;
+module_param(copybreak, int, 0);
+MODULE_PARM_DESC(copybreak, "Receive copy threshold");
+
 /**
  *	get_packet - return the next ingress packet buffer
  *	@pdev: the PCI device that received the packet
@@ -1059,37 +1062,33 @@ static void recycle_fl_buf(struct freelQ
  *	threshold and the packet is too big to copy, or (b) the packet should
  *	be copied but there is no memory for the copy.
  */
-static inline struct sk_buff *get_packet(struct pci_dev *pdev,
-					 struct freelQ *fl, unsigned int len,
-					 int dma_pad, int skb_pad,
-					 unsigned int copy_thres,
-					 unsigned int drop_thres)
+static inline struct sk_buff *get_packet(struct pci_dev *pdev, struct freelQ *fl,
+					 unsigned int len)
 {
 	struct sk_buff *skb;
-	struct freelQ_ce *ce = &fl->centries[fl->cidx];
+	const struct freelQ_ce *ce = &fl->centries[fl->cidx];
+
+	if (len < copybreak) {
+		skb = alloc_skb(len + 2, GFP_ATOMIC);
+		if (!skb)
+			goto use_orig_buf;
 
-	if (len < copy_thres) {
-		skb = alloc_skb(len + skb_pad, GFP_ATOMIC);
-		if (likely(skb != NULL)) {
-			skb_reserve(skb, skb_pad);
-			skb_put(skb, len);
-			pci_dma_sync_single_for_cpu(pdev,
+		skb_reserve(skb, 2);	/* align IP header */
+		skb_put(skb, len);
+		pci_dma_sync_single_for_cpu(pdev,
 					    pci_unmap_addr(ce, dma_addr),
  					    pci_unmap_len(ce, dma_len),
 					    PCI_DMA_FROMDEVICE);
-			memcpy(skb->data, ce->skb->data + dma_pad, len);
-			pci_dma_sync_single_for_device(pdev,
+		memcpy(skb->data, ce->skb->data, len);
+		pci_dma_sync_single_for_device(pdev,
 					    pci_unmap_addr(ce, dma_addr),
  					    pci_unmap_len(ce, dma_len),
 					    PCI_DMA_FROMDEVICE);
-		} else if (!drop_thres)
-			goto use_orig_buf;
-
 		recycle_fl_buf(fl, fl->cidx);
 		return skb;
 	}
 
-	if (fl->credits < drop_thres) {
+	if (fl->credits < 2) {
 		recycle_fl_buf(fl, fl->cidx);
 		return NULL;
 	}
@@ -1098,7 +1097,8 @@ use_orig_buf:
 	pci_unmap_single(pdev, pci_unmap_addr(ce, dma_addr),
 			 pci_unmap_len(ce, dma_len), PCI_DMA_FROMDEVICE);
 	skb = ce->skb;
-	skb_reserve(skb, dma_pad);
+	prefetch(skb->data);
+
 	skb_put(skb, len);
 	return skb;
 }
@@ -1375,27 +1375,25 @@ static void restart_sched(unsigned long 
  *
  *	Process an ingress ethernet pakcet and deliver it to the stack.
  */
-static int sge_rx(struct sge *sge, struct freelQ *fl, unsigned int len)
+static void sge_rx(struct sge *sge, struct freelQ *fl, unsigned int len)
 {
 	struct sk_buff *skb;
-	struct cpl_rx_pkt *p;
+	const struct cpl_rx_pkt *p;
 	struct adapter *adapter = sge->adapter;
 	struct sge_port_stats *st;
 
-	skb = get_packet(adapter->pdev, fl, len - sge->rx_pkt_pad,
-			 sge->rx_pkt_pad, 2, SGE_RX_COPY_THRES,
-			 SGE_RX_DROP_THRES);
+	skb = get_packet(adapter->pdev, fl, len - sge->rx_pkt_pad);
 	if (unlikely(!skb)) {
 		sge->stats.rx_drops++;
-		return 0;
+		return;
 	}
 
-	p = (struct cpl_rx_pkt *)skb->data;
-	skb_pull(skb, sizeof(*p));
+	p = (const struct cpl_rx_pkt *) skb->data;
 	if (p->iff >= adapter->params.nports) {
 		kfree_skb(skb);
-		return 0;
+		return;
 	}
+	__skb_pull(skb, sizeof(*p));
 
 	skb->dev = adapter->port[p->iff].dev;
 	skb->dev->last_rx = jiffies;
@@ -1427,7 +1425,6 @@ static int sge_rx(struct sge *sge, struc
 		netif_rx(skb);
 #endif
 	}
-	return 0;
 }
 
 /*
@@ -1510,12 +1507,11 @@ static int process_responses(struct adap
 	struct sge *sge = adapter->sge;
 	struct respQ *q = &sge->respQ;
 	struct respQ_e *e = &q->entries[q->cidx];
-	int budget_left = budget;
+	int done = 0;
 	unsigned int flags = 0;
 	unsigned int cmdq_processed[SGE_CMDQ_N] = {0, 0};
 	
-
-	while (likely(budget_left && e->GenerationBit == q->genbit)) {
+	while (done < budget && e->GenerationBit == q->genbit) {
 		flags |= e->Qsleeping;
 		
 		cmdq_processed[0] += e->Cmdq0CreditReturn;
@@ -1525,14 +1521,16 @@ static int process_responses(struct adap
 		 * ping-pong of TX state information on MP where the sender
 		 * might run on a different CPU than this function...
 		 */
-		if (unlikely(flags & F_CMDQ0_ENABLE || cmdq_processed[0] > 64)) {
+		if (unlikely((flags & F_CMDQ0_ENABLE) || cmdq_processed[0] > 64)) {
 			flags = update_tx_info(adapter, flags, cmdq_processed[0]);
 			cmdq_processed[0] = 0;
 		}
+
 		if (unlikely(cmdq_processed[1] > 16)) {
 			sge->cmdQ[1].processed += cmdq_processed[1];
 			cmdq_processed[1] = 0;
 		}
+
 		if (likely(e->DataValid)) {
 			struct freelQ *fl = &sge->freelQ[e->FreelistQid];
 
@@ -1542,12 +1540,16 @@ static int process_responses(struct adap
 			else
 				sge_rx(sge, fl, e->BufferLength);
 
+			++done;
+
 			/*
 			 * Note: this depends on each packet consuming a
 			 * single free-list buffer; cf. the BUG above.
 			 */
 			if (++fl->cidx == fl->size)
 				fl->cidx = 0;
+			prefetch(fl->centries[fl->cidx].skb);
+
 			if (unlikely(--fl->credits <
 				     fl->size - SGE_FREEL_REFILL_THRESH))
 				refill_free_list(sge, fl);
@@ -1566,14 +1568,12 @@ static int process_responses(struct adap
 			writel(q->credits, adapter->regs + A_SG_RSPQUEUECREDIT);
 			q->credits = 0;
 		}
-		--budget_left;
 	}
 
 	flags = update_tx_info(adapter, flags, cmdq_processed[0]); 
 	sge->cmdQ[1].processed += cmdq_processed[1];
 
-	budget -= budget_left;
-	return budget;
+	return done;
 }
 
 static inline int responses_pending(const struct adapter *adapter)
@@ -1598,11 +1598,14 @@ static int process_pure_responses(struct
 	struct sge *sge = adapter->sge;
 	struct respQ *q = &sge->respQ;
 	struct respQ_e *e = &q->entries[q->cidx];
+	const struct freelQ *fl = &sge->freelQ[e->FreelistQid];
 	unsigned int flags = 0;
 	unsigned int cmdq_processed[SGE_CMDQ_N] = {0, 0};
 
+	prefetch(fl->centries[fl->cidx].skb);
 	if (e->DataValid)
 		return 1;
+
 	do {
 		flags |= e->Qsleeping;
 

-- 

Re: [PATCH 3/3] chelsio: more receive cleanup

[Francois Romieu]

Stephen Hemminger <shemminger@osdl.org> :
[...]
> --- linux-2.6.20-rc1.orig/drivers/net/chelsio/sge.c
> +++ linux-2.6.20-rc1/drivers/net/chelsio/sge.c
[...]
> @@ -1059,37 +1062,33 @@ static void recycle_fl_buf(struct freelQ
>   *	threshold and the packet is too big to copy, or (b) the packet should
>   *	be copied but there is no memory for the copy.
>   */
> -static inline struct sk_buff *get_packet(struct pci_dev *pdev,
> -					 struct freelQ *fl, unsigned int len,
> -					 int dma_pad, int skb_pad,
> -					 unsigned int copy_thres,
> -					 unsigned int drop_thres)
> +static inline struct sk_buff *get_packet(struct pci_dev *pdev, struct freelQ *fl,
> +					 unsigned int len)
>  {
>  	struct sk_buff *skb;
> -	struct freelQ_ce *ce = &fl->centries[fl->cidx];
> +	const struct freelQ_ce *ce = &fl->centries[fl->cidx];
> +
> +	if (len < copybreak) {

If you are into cleanups, maybe add likely/unlikely (and remove the inline) ?

> +		skb = alloc_skb(len + 2, GFP_ATOMIC);
> +		if (!skb)
> +			goto use_orig_buf;
>  
> -	if (len < copy_thres) {
> -		skb = alloc_skb(len + skb_pad, GFP_ATOMIC);
> -		if (likely(skb != NULL)) {
> -			skb_reserve(skb, skb_pad);
> -			skb_put(skb, len);
> -			pci_dma_sync_single_for_cpu(pdev,
> +		skb_reserve(skb, 2);	/* align IP header */

s/2/NET_IP_ALIGN/

Btw, since the driver supports netpoll, its blind enabling of interrupts
in t1_poll() seems old-fashoined (see thread starting at
http://lkml.org/lkml/2006/12/12/86)

-- 
Ueimor

Re: [PATCH 3/3] chelsio: more receive cleanup

[Stephen Hemminger]

On Fri, 15 Dec 2006 21:10:19 +0100
Francois Romieu <romieu@fr.zoreil.com> wrote:

> Stephen Hemminger <shemminger@osdl.org> :
> [...]
> > --- linux-2.6.20-rc1.orig/drivers/net/chelsio/sge.c
> > +++ linux-2.6.20-rc1/drivers/net/chelsio/sge.c
> [...]
> > @@ -1059,37 +1062,33 @@ static void recycle_fl_buf(struct freelQ
> >   *	threshold and the packet is too big to copy, or (b) the packet should
> >   *	be copied but there is no memory for the copy.
> >   */
> > -static inline struct sk_buff *get_packet(struct pci_dev *pdev,
> > -					 struct freelQ *fl, unsigned int len,
> > -					 int dma_pad, int skb_pad,
> > -					 unsigned int copy_thres,
> > -					 unsigned int drop_thres)
> > +static inline struct sk_buff *get_packet(struct pci_dev *pdev, struct freelQ *fl,
> > +					 unsigned int len)
> >  {
> >  	struct sk_buff *skb;
> > -	struct freelQ_ce *ce = &fl->centries[fl->cidx];
> > +	const struct freelQ_ce *ce = &fl->centries[fl->cidx];
> > +
> > +	if (len < copybreak) {
> 
> If you are into cleanups, maybe add likely/unlikely (and remove the inline) ?

Why, you can't predict copybreak on off

> 
> > +		skb = alloc_skb(len + 2, GFP_ATOMIC);
> > +		if (!skb)
> > +			goto use_orig_buf;
> >  
> > -	if (len < copy_thres) {
> > -		skb = alloc_skb(len + skb_pad, GFP_ATOMIC);
> > -		if (likely(skb != NULL)) {
> > -			skb_reserve(skb, skb_pad);
> > -			skb_put(skb, len);
> > -			pci_dma_sync_single_for_cpu(pdev,
> > +		skb_reserve(skb, 2);	/* align IP header */
> 
> s/2/NET_IP_ALIGN/

This is wrong, NET_IP_ALIGN is for DMA buffers only. It is defined
as 0 for those platforms where DMA alignment is important. In this
case we are copying, so we want to force 2.

> Btw, since the driver supports netpoll, its blind enabling of interrupts
> in t1_poll() seems old-fashoined (see thread starting at
> http://lkml.org/lkml/2006/12/12/86)
> 


-- 
Stephen Hemminger <shemminger@osdl.org>

Re: [PATCH 3/3] chelsio: more receive cleanup

[Francois Romieu]

Stephen Hemminger <shemminger@osdl.org> :
> Francois Romieu <romieu@fr.zoreil.com> wrote:
[...]
> Why, you can't predict copybreak on off

Plain gut feeling. I'd rather spend extra cycles on small packets (whose
allocation is already optimized) and keep the normal packets less expensive
on a 10Gb/s network card.

[...]
> This is wrong, NET_IP_ALIGN is for DMA buffers only. It is defined
> as 0 for those platforms where DMA alignment is important. In this
> case we are copying, so we want to force 2.

Ok, ok.

-- 
Ueimor

Re: [PATCH 3/3] chelsio: more receive cleanup

[Stephen Hemminger]

On Sat, 16 Dec 2006 01:26:04 +0100
Francois Romieu <romieu@fr.zoreil.com> wrote:

> Stephen Hemminger <shemminger@osdl.org> :
> > Francois Romieu <romieu@fr.zoreil.com> wrote:
> [...]
> > Why, you can't predict copybreak on off
> 
> Plain gut feeling. I'd rather spend extra cycles on small packets (whose
> allocation is already optimized) and keep the normal packets less expensive
> on a 10Gb/s network card.
>

Likely/unlikely is an optimization that doesn't make a big difference and
I would rather the compiler choose unless it is something obvious like
an error path or external interrupt (like PHY).

There was some discussion on LKML, which I mostly forgot, that hinted that
on some architectures using likely/unlikely incorrectly could have a major
hit.


-- 
Stephen Hemminger <shemminger@osdl.org>

Re: [PATCH 0/3] chelsio driver changes

[Jeff Garzik]

Stephen Hemminger wrote:
> One error path bug fix, and a couple of changes to improve
> receive performance.  The performance is now up to 5+ Gbits/sec
> which is about the limit of the memory/bus on the test systems.

I request a rediff+resend against netdev-2.6.git#upstream, giving 
sufficient time [hours :(] for it to mirror out.

	Jeff

[PATCH 1/3] chelsio: error path fix

[Stephen Hemminger]

Fix handling of allocation failure.

Signed-off-by: Stephen Hemminger <shemminger@osdl.org>

--- netdev-2.6.orig/drivers/net/chelsio/my3126.c
+++ netdev-2.6/drivers/net/chelsio/my3126.c
@@ -170,9 +170,10 @@ static struct cphy *my3126_phy_create(ad
 {
 	struct cphy *cphy = kzalloc(sizeof (*cphy), GFP_KERNEL);
 
-	if (cphy)
-		cphy_init(cphy, adapter, phy_addr, &my3126_ops, mdio_ops);
+	if (!cphy)
+		return NULL;
 
+	cphy_init(cphy, adapter, phy_addr, &my3126_ops, mdio_ops);
 	INIT_DELAYED_WORK(&cphy->phy_update, my3216_poll);
 	cphy->bmsr = 0;
 

[PATCH 3/3] chelsio: more rx speedup

[Stephen Hemminger]

Cleanup receive processing some more:
   * do the reserve padding of skb during setup
   * don't pass constants to get_packet
   * do smart prefetch of skb
   * make copybreak a module parameter

Signed-off-by: Stephen Hemminger <shemminger@osdl.org>

---
 drivers/net/chelsio/sge.c |   87 +++++++++++++++++++++++-----------------------
 1 file changed, 45 insertions(+), 42 deletions(-)

--- netdev-2.6.orig/drivers/net/chelsio/sge.c
+++ netdev-2.6/drivers/net/chelsio/sge.c
@@ -71,12 +71,9 @@
 #define SGE_FREEL_REFILL_THRESH	16
 #define SGE_RESPQ_E_N		1024
 #define SGE_INTRTIMER_NRES	1000
-#define SGE_RX_COPY_THRES	256
 #define SGE_RX_SM_BUF_SIZE	1536
 #define SGE_TX_DESC_MAX_PLEN	16384
 
-# define SGE_RX_DROP_THRES 2
-
 #define SGE_RESPQ_REPLENISH_THRES (SGE_RESPQ_E_N / 4)
 
 /*
@@ -846,6 +843,8 @@ static void refill_free_list(struct sge 
 		skb_reserve(skb, q->dma_offset);
 		mapping = pci_map_single(pdev, skb->data, dma_len,
 					 PCI_DMA_FROMDEVICE);
+		skb_reserve(skb, sge->rx_pkt_pad);
+
 		ce->skb = skb;
 		pci_unmap_addr_set(ce, dma_addr, mapping);
 		pci_unmap_len_set(ce, dma_len, dma_len);
@@ -1024,6 +1023,10 @@ static void recycle_fl_buf(struct freelQ
 	}
 }
 
+static int copybreak __read_mostly = 256;
+module_param(copybreak, int, 0);
+MODULE_PARM_DESC(copybreak, "Receive copy threshold");
+
 /**
  *	get_packet - return the next ingress packet buffer
  *	@pdev: the PCI device that received the packet
@@ -1043,45 +1046,42 @@ static void recycle_fl_buf(struct freelQ
  *	be copied but there is no memory for the copy.
  */
 static inline struct sk_buff *get_packet(struct pci_dev *pdev,
-					 struct freelQ *fl, unsigned int len,
-					 int dma_pad, int skb_pad,
-					 unsigned int copy_thres,
-					 unsigned int drop_thres)
+					 struct freelQ *fl, unsigned int len)
 {
 	struct sk_buff *skb;
-	struct freelQ_ce *ce = &fl->centries[fl->cidx];
+	const struct freelQ_ce *ce = &fl->centries[fl->cidx];
 
-	if (len < copy_thres) {
-		skb = alloc_skb(len + skb_pad, GFP_ATOMIC);
-		if (likely(skb != NULL)) {
-			skb_reserve(skb, skb_pad);
-			skb_put(skb, len);
-			pci_dma_sync_single_for_cpu(pdev,
-					    pci_unmap_addr(ce, dma_addr),
-					    pci_unmap_len(ce, dma_len),
-					    PCI_DMA_FROMDEVICE);
-			memcpy(skb->data, ce->skb->data + dma_pad, len);
-			pci_dma_sync_single_for_device(pdev,
+	if (len < copybreak) {
+		skb = alloc_skb(len + 2, GFP_ATOMIC);
+		if (!skb)
+			goto use_orig_buf;
+
+		skb_reserve(skb, 2);	/* align IP header */
+		skb_put(skb, len);
+		pci_dma_sync_single_for_cpu(pdev,
 					    pci_unmap_addr(ce, dma_addr),
 					    pci_unmap_len(ce, dma_len),
 					    PCI_DMA_FROMDEVICE);
-		} else if (!drop_thres)
-			goto use_orig_buf;
-
+		memcpy(skb->data, ce->skb->data, len);
+		pci_dma_sync_single_for_device(pdev,
+					       pci_unmap_addr(ce, dma_addr),
+					       pci_unmap_len(ce, dma_len),
+					       PCI_DMA_FROMDEVICE);
 		recycle_fl_buf(fl, fl->cidx);
 		return skb;
 	}
 
-	if (fl->credits < drop_thres) {
+use_orig_buf:
+	if (fl->credits < 2) {
 		recycle_fl_buf(fl, fl->cidx);
 		return NULL;
 	}
 
-use_orig_buf:
 	pci_unmap_single(pdev, pci_unmap_addr(ce, dma_addr),
 			 pci_unmap_len(ce, dma_len), PCI_DMA_FROMDEVICE);
 	skb = ce->skb;
-	skb_reserve(skb, dma_pad);
+	prefetch(skb->data);
+
 	skb_put(skb, len);
 	return skb;
 }
@@ -1359,27 +1359,25 @@ static void restart_sched(unsigned long 
  *
  *	Process an ingress ethernet pakcet and deliver it to the stack.
  */
-static int sge_rx(struct sge *sge, struct freelQ *fl, unsigned int len)
+static void sge_rx(struct sge *sge, struct freelQ *fl, unsigned int len)
 {
 	struct sk_buff *skb;
-	struct cpl_rx_pkt *p;
+	const struct cpl_rx_pkt *p;
 	struct adapter *adapter = sge->adapter;
 	struct sge_port_stats *st;
 
-	skb = get_packet(adapter->pdev, fl, len - sge->rx_pkt_pad,
-			 sge->rx_pkt_pad, 2, SGE_RX_COPY_THRES,
-			 SGE_RX_DROP_THRES);
+	skb = get_packet(adapter->pdev, fl, len - sge->rx_pkt_pad);
 	if (unlikely(!skb)) {
 		sge->stats.rx_drops++;
-		return 0;
+		return;
 	}
 
-	p = (struct cpl_rx_pkt *)skb->data;
-	skb_pull(skb, sizeof(*p));
+	p = (const struct cpl_rx_pkt *) skb->data;
 	if (p->iff >= adapter->params.nports) {
 		kfree_skb(skb);
-		return 0;
+		return;
 	}
+	__skb_pull(skb, sizeof(*p));
 
 	skb->dev = adapter->port[p->iff].dev;
 	skb->dev->last_rx = jiffies;
@@ -1411,7 +1409,6 @@ static int sge_rx(struct sge *sge, struc
 		netif_rx(skb);
 #endif
 	}
-	return 0;
 }
 
 /*
@@ -1493,12 +1490,11 @@ static int process_responses(struct adap
 	struct sge *sge = adapter->sge;
 	struct respQ *q = &sge->respQ;
 	struct respQ_e *e = &q->entries[q->cidx];
-	int budget_left = budget;
+	int done = 0;
 	unsigned int flags = 0;
 	unsigned int cmdq_processed[SGE_CMDQ_N] = {0, 0};
 
-
-	while (likely(budget_left && e->GenerationBit == q->genbit)) {
+	while (done < budget && e->GenerationBit == q->genbit) {
 		flags |= e->Qsleeping;
 
 		cmdq_processed[0] += e->Cmdq0CreditReturn;
@@ -1508,14 +1504,16 @@ static int process_responses(struct adap
 		 * ping-pong of TX state information on MP where the sender
 		 * might run on a different CPU than this function...
 		 */
-		if (unlikely(flags & F_CMDQ0_ENABLE || cmdq_processed[0] > 64)) {
+		if (unlikely((flags & F_CMDQ0_ENABLE) || cmdq_processed[0] > 64)) {
 			flags = update_tx_info(adapter, flags, cmdq_processed[0]);
 			cmdq_processed[0] = 0;
 		}
+
 		if (unlikely(cmdq_processed[1] > 16)) {
 			sge->cmdQ[1].processed += cmdq_processed[1];
 			cmdq_processed[1] = 0;
 		}
+
 		if (likely(e->DataValid)) {
 			struct freelQ *fl = &sge->freelQ[e->FreelistQid];
 
@@ -1525,12 +1523,16 @@ static int process_responses(struct adap
 			else
 				sge_rx(sge, fl, e->BufferLength);
 
+			++done;
+
 			/*
 			 * Note: this depends on each packet consuming a
 			 * single free-list buffer; cf. the BUG above.
 			 */
 			if (++fl->cidx == fl->size)
 				fl->cidx = 0;
+			prefetch(fl->centries[fl->cidx].skb);
+
 			if (unlikely(--fl->credits <
 				     fl->size - SGE_FREEL_REFILL_THRESH))
 				refill_free_list(sge, fl);
@@ -1549,14 +1551,12 @@ static int process_responses(struct adap
 			writel(q->credits, adapter->regs + A_SG_RSPQUEUECREDIT);
 			q->credits = 0;
 		}
-		--budget_left;
 	}
 
 	flags = update_tx_info(adapter, flags, cmdq_processed[0]);
 	sge->cmdQ[1].processed += cmdq_processed[1];
 
-	budget -= budget_left;
-	return budget;
+	return done;
 }
 
 static inline int responses_pending(const struct adapter *adapter)
@@ -1581,11 +1581,14 @@ static int process_pure_responses(struct
 	struct sge *sge = adapter->sge;
 	struct respQ *q = &sge->respQ;
 	struct respQ_e *e = &q->entries[q->cidx];
+	const struct freelQ *fl = &sge->freelQ[e->FreelistQid];
 	unsigned int flags = 0;
 	unsigned int cmdq_processed[SGE_CMDQ_N] = {0, 0};
 
+	prefetch(fl->centries[fl->cidx].skb);
 	if (e->DataValid)
 		return 1;
+
 	do {
 		flags |= e->Qsleeping;
 

[PATCH 2/3] chelsio: NAPI speed improvement

[Stephen Hemminger]

Speedup and cleanup the receive processing by eliminating the
mmio read and a lock round trip.

Signed-off-by: Stephen Hemminger <shemminger@osdl.org>

---
 drivers/net/chelsio/sge.c |   77 ++++++++++++++++++++--------------------------
 1 file changed, 35 insertions(+), 42 deletions(-)

--- netdev-2.6.orig/drivers/net/chelsio/sge.c
+++ netdev-2.6/drivers/net/chelsio/sge.c
@@ -1559,6 +1559,14 @@ static int process_responses(struct adap
 	return budget;
 }
 
+static inline int responses_pending(const struct adapter *adapter)
+{
+	const struct respQ *Q = &adapter->sge->respQ;
+	const struct respQ_e *e = &Q->entries[Q->cidx];
+
+	return (e->GenerationBit == Q->genbit);
+}
+
 #ifdef CONFIG_CHELSIO_T1_NAPI
 /*
  * A simpler version of process_responses() that handles only pure (i.e.,
@@ -1568,13 +1576,16 @@ static int process_responses(struct adap
  * which the caller must ensure is a valid pure response.  Returns 1 if it
  * encounters a valid data-carrying response, 0 otherwise.
  */
-static int process_pure_responses(struct adapter *adapter, struct respQ_e *e)
+static int process_pure_responses(struct adapter *adapter)
 {
 	struct sge *sge = adapter->sge;
 	struct respQ *q = &sge->respQ;
+	struct respQ_e *e = &q->entries[q->cidx];
 	unsigned int flags = 0;
 	unsigned int cmdq_processed[SGE_CMDQ_N] = {0, 0};
 
+	if (e->DataValid)
+		return 1;
 	do {
 		flags |= e->Qsleeping;
 
@@ -1610,23 +1621,20 @@ static int process_pure_responses(struct
 int t1_poll(struct net_device *dev, int *budget)
 {
 	struct adapter *adapter = dev->priv;
-	int effective_budget = min(*budget, dev->quota);
-	int work_done = process_responses(adapter, effective_budget);
+	int work_done;
 
+	work_done = process_responses(adapter, min(*budget, dev->quota));
 	*budget -= work_done;
 	dev->quota -= work_done;
 
-	if (work_done >= effective_budget)
+	if (unlikely(responses_pending(adapter)))
 		return 1;
 
-	spin_lock_irq(&adapter->async_lock);
-	__netif_rx_complete(dev);
+	netif_rx_complete(dev);
 	writel(adapter->sge->respQ.cidx, adapter->regs + A_SG_SLEEPING);
-	writel(adapter->slow_intr_mask | F_PL_INTR_SGE_DATA,
-	       adapter->regs + A_PL_ENABLE);
-	spin_unlock_irq(&adapter->async_lock);
 
 	return 0;
+
 }
 
 /*
@@ -1635,44 +1643,33 @@ int t1_poll(struct net_device *dev, int 
 irqreturn_t t1_interrupt(int irq, void *data)
 {
 	struct adapter *adapter = data;
-	struct net_device *dev = adapter->sge->netdev;
 	struct sge *sge = adapter->sge;
-	u32 cause;
-	int handled = 0;
+	int handled;
 
-	cause = readl(adapter->regs + A_PL_CAUSE);
-	if (cause == 0 || cause == ~0)
-		return IRQ_NONE;
-
-	spin_lock(&adapter->async_lock);
-	if (cause & F_PL_INTR_SGE_DATA) {
-		struct respQ *q = &adapter->sge->respQ;
-		struct respQ_e *e = &q->entries[q->cidx];
+	if (likely(responses_pending(adapter))) {
+		struct net_device *dev = sge->netdev;
 
-		handled = 1;
 		writel(F_PL_INTR_SGE_DATA, adapter->regs + A_PL_CAUSE);
 
-		if (e->GenerationBit == q->genbit &&
-		    __netif_rx_schedule_prep(dev)) {
-			if (e->DataValid || process_pure_responses(adapter, e)) {
-				/* mask off data IRQ */
-				writel(adapter->slow_intr_mask,
-				       adapter->regs + A_PL_ENABLE);
-				__netif_rx_schedule(sge->netdev);
-				goto unlock;
+		if (__netif_rx_schedule_prep(dev)) {
+			if (process_pure_responses(adapter))
+				__netif_rx_schedule(dev);
+			else {
+				/* no data, no NAPI needed */
+				writel(sge->respQ.cidx, adapter->regs + A_SG_SLEEPING);
+				netif_poll_enable(dev);	/* undo schedule_prep */
 			}
-			/* no data, no NAPI needed */
-			netif_poll_enable(dev);
-
 		}
-		writel(q->cidx, adapter->regs + A_SG_SLEEPING);
-	} else
-		handled = t1_slow_intr_handler(adapter);
+		return IRQ_HANDLED;
+	}
+
+	spin_lock(&adapter->async_lock);
+	handled = t1_slow_intr_handler(adapter);
+	spin_unlock(&adapter->async_lock);
 
 	if (!handled)
 		sge->stats.unhandled_irqs++;
-unlock:
-	spin_unlock(&adapter->async_lock);
+
 	return IRQ_RETVAL(handled != 0);
 }
 
@@ -1695,17 +1692,13 @@ unlock:
 irqreturn_t t1_interrupt(int irq, void *cookie)
 {
 	int work_done;
-	struct respQ_e *e;
 	struct adapter *adapter = cookie;
-	struct respQ *Q = &adapter->sge->respQ;
 
 	spin_lock(&adapter->async_lock);
-	e = &Q->entries[Q->cidx];
-	prefetch(e);
 
 	writel(F_PL_INTR_SGE_DATA, adapter->regs + A_PL_CAUSE);
 
-	if (likely(e->GenerationBit == Q->genbit))
+	if (likely(responses_pending(adapter))
 		work_done = process_responses(adapter, -1);
 	else
 		work_done = t1_slow_intr_handler(adapter);

Re: [PATCH 3/3] chelsio: more rx speedup

[Ingo Oeser]

Hi Stephen,

Stephen Hemminger schrieb:
> --- netdev-2.6.orig/drivers/net/chelsio/sge.c
> +++ netdev-2.6/drivers/net/chelsio/sge.c
[...]
> @@ -1043,45 +1046,42 @@ static void recycle_fl_buf(struct freelQ
>   *	be copied but there is no memory for the copy.
>   */
>  static inline struct sk_buff *get_packet(struct pci_dev *pdev,
> -					 struct freelQ *fl, unsigned int len,
> -					 int dma_pad, int skb_pad,
> -					 unsigned int copy_thres,
> -					 unsigned int drop_thres)
> +					 struct freelQ *fl, unsigned int len)
>  {
>  	struct sk_buff *skb;
> -	struct freelQ_ce *ce = &fl->centries[fl->cidx];
> +	const struct freelQ_ce *ce = &fl->centries[fl->cidx];
>  
> -	if (len < copy_thres) {
> -		skb = alloc_skb(len + skb_pad, GFP_ATOMIC);
> -		if (likely(skb != NULL)) {
> -			skb_reserve(skb, skb_pad);
> -			skb_put(skb, len);
> -			pci_dma_sync_single_for_cpu(pdev,
> -					    pci_unmap_addr(ce, dma_addr),
> -					    pci_unmap_len(ce, dma_len),
> -					    PCI_DMA_FROMDEVICE);
> -			memcpy(skb->data, ce->skb->data + dma_pad, len);
> -			pci_dma_sync_single_for_device(pdev,
> +	if (len < copybreak) {
> +		skb = alloc_skb(len + 2, GFP_ATOMIC);
> +		if (!skb)
> +			goto use_orig_buf;
> +
> +		skb_reserve(skb, 2);	/* align IP header */

Please use NET_IP_ALIGN here:

+		skb = alloc_skb(len + NET_IP_ALIGN, GFP_ATOMIC);
+		if (!skb)
+			goto use_orig_buf;
+
+		skb_reserve(skb, NET_IP_ALIGN);

> +		skb_put(skb, len);
> +		pci_dma_sync_single_for_cpu(pdev,
>  					    pci_unmap_addr(ce, dma_addr),
>  					    pci_unmap_len(ce, dma_len),
>  					    PCI_DMA_FROMDEVICE);
> -		} else if (!drop_thres)
> -			goto use_orig_buf;
> -
> +		memcpy(skb->data, ce->skb->data, len);
> +		pci_dma_sync_single_for_device(pdev,
> +					       pci_unmap_addr(ce, dma_addr),
> +					       pci_unmap_len(ce, dma_len),
> +					       PCI_DMA_FROMDEVICE);
>  		recycle_fl_buf(fl, fl->cidx);
>  		return skb;
>  	}
>  
> -	if (fl->credits < drop_thres) {
> +use_orig_buf:
> +	if (fl->credits < 2) {

Why 2? What does this magic number mean?


Regards

Ingo Oeser

Re: [PATCH 3/3] chelsio: more rx speedup

[Stephen Hemminger]

On Tue, 9 Jan 2007 09:42:03 +0100
Ingo Oeser <netdev@axxeo.de> wrote:

> Hi Stephen,
> 
> Stephen Hemminger schrieb:
> > --- netdev-2.6.orig/drivers/net/chelsio/sge.c
> > +++ netdev-2.6/drivers/net/chelsio/sge.c
> [...]
> > @@ -1043,45 +1046,42 @@ static void recycle_fl_buf(struct freelQ
> >   *	be copied but there is no memory for the copy.
> >   */
> >  static inline struct sk_buff *get_packet(struct pci_dev *pdev,
> > -					 struct freelQ *fl, unsigned int len,
> > -					 int dma_pad, int skb_pad,
> > -					 unsigned int copy_thres,
> > -					 unsigned int drop_thres)
> > +					 struct freelQ *fl, unsigned int len)
> >  {
> >  	struct sk_buff *skb;
> > -	struct freelQ_ce *ce = &fl->centries[fl->cidx];
> > +	const struct freelQ_ce *ce = &fl->centries[fl->cidx];
> >  
> > -	if (len < copy_thres) {
> > -		skb = alloc_skb(len + skb_pad, GFP_ATOMIC);
> > -		if (likely(skb != NULL)) {
> > -			skb_reserve(skb, skb_pad);
> > -			skb_put(skb, len);
> > -			pci_dma_sync_single_for_cpu(pdev,
> > -					    pci_unmap_addr(ce, dma_addr),
> > -					    pci_unmap_len(ce, dma_len),
> > -					    PCI_DMA_FROMDEVICE);
> > -			memcpy(skb->data, ce->skb->data + dma_pad, len);
> > -			pci_dma_sync_single_for_device(pdev,
> > +	if (len < copybreak) {
> > +		skb = alloc_skb(len + 2, GFP_ATOMIC);
> > +		if (!skb)
> > +			goto use_orig_buf;
> > +
> > +		skb_reserve(skb, 2);	/* align IP header */
> 
> Please use NET_IP_ALIGN here:

Wrong, NET_IP_ALIGN is intended to deal with platforms where alignment of DMA is more
important of alignment of structures.  Therefore if data is copied, it should
always be 2.

> 
> +		skb = alloc_skb(len + NET_IP_ALIGN, GFP_ATOMIC);
> +		if (!skb)
> +			goto use_orig_buf;
> +
> +		skb_reserve(skb, NET_IP_ALIGN);
> 
> > +		skb_put(skb, len);
> > +		pci_dma_sync_single_for_cpu(pdev,
> >  					    pci_unmap_addr(ce, dma_addr),
> >  					    pci_unmap_len(ce, dma_len),
> >  					    PCI_DMA_FROMDEVICE);
> > -		} else if (!drop_thres)
> > -			goto use_orig_buf;
> > -
> > +		memcpy(skb->data, ce->skb->data, len);
> > +		pci_dma_sync_single_for_device(pdev,
> > +					       pci_unmap_addr(ce, dma_addr),
> > +					       pci_unmap_len(ce, dma_len),
> > +					       PCI_DMA_FROMDEVICE);
> >  		recycle_fl_buf(fl, fl->cidx);
> >  		return skb;
> >  	}
> >  
> > -	if (fl->credits < drop_thres) {
> > +use_orig_buf:
> > +	if (fl->credits < 2) {
> 
> Why 2? What does this magic number mean?

No idea, it was there in the original. (as a parameter).

-- 
Stephen Hemminger <shemminger@osdl.org>

Re: [PATCH 3/3] chelsio: more rx speedup

[Divy Le Ray]

Stephen Hemminger wrote:
> On Tue, 9 Jan 2007 09:42:03 +0100
> Ingo Oeser <netdev@axxeo.de> wrote:
>
>   
>> Hi Stephen,
>>
>> Stephen Hemminger schrieb:
>>     
>>> --- netdev-2.6.orig/drivers/net/chelsio/sge.c
>>> +++ netdev-2.6/drivers/net/chelsio/sge.c
>>>       
>> [...]
>>     
>>> @@ -1043,45 +1046,42 @@ static void recycle_fl_buf(struct freelQ
>>>   *	be copied but there is no memory for the copy.
>>>   */
>>>  static inline struct sk_buff *get_packet(struct pci_dev *pdev,
>>> -					 struct freelQ *fl, unsigned int len,
>>> -					 int dma_pad, int skb_pad,
>>> -					 unsigned int copy_thres,
>>> -					 unsigned int drop_thres)
>>> +					 struct freelQ *fl, unsigned int len)
>>>  {
>>>  	struct sk_buff *skb;
>>> -	struct freelQ_ce *ce = &fl->centries[fl->cidx];
>>> +	const struct freelQ_ce *ce = &fl->centries[fl->cidx];
>>>  
>>> -	if (len < copy_thres) {
>>> -		skb = alloc_skb(len + skb_pad, GFP_ATOMIC);
>>> -		if (likely(skb != NULL)) {
>>> -			skb_reserve(skb, skb_pad);
>>> -			skb_put(skb, len);
>>> -			pci_dma_sync_single_for_cpu(pdev,
>>> -					    pci_unmap_addr(ce, dma_addr),
>>> -					    pci_unmap_len(ce, dma_len),
>>> -					    PCI_DMA_FROMDEVICE);
>>> -			memcpy(skb->data, ce->skb->data + dma_pad, len);
>>> -			pci_dma_sync_single_for_device(pdev,
>>> +	if (len < copybreak) {
>>> +		skb = alloc_skb(len + 2, GFP_ATOMIC);
>>> +		if (!skb)
>>> +			goto use_orig_buf;
>>> +
>>> +		skb_reserve(skb, 2);	/* align IP header */
>>>       
>> Please use NET_IP_ALIGN here:
>>     
>
> Wrong, NET_IP_ALIGN is intended to deal with platforms where alignment of DMA is more
> important of alignment of structures.  Therefore if data is copied, it should
> always be 2.
>
>   
>> +		skb = alloc_skb(len + NET_IP_ALIGN, GFP_ATOMIC);
>> +		if (!skb)
>> +			goto use_orig_buf;
>> +
>> +		skb_reserve(skb, NET_IP_ALIGN);
>>
>>     
>>> +		skb_put(skb, len);
>>> +		pci_dma_sync_single_for_cpu(pdev,
>>>  					    pci_unmap_addr(ce, dma_addr),
>>>  					    pci_unmap_len(ce, dma_len),
>>>  					    PCI_DMA_FROMDEVICE);
>>> -		} else if (!drop_thres)
>>> -			goto use_orig_buf;
>>> -
>>> +		memcpy(skb->data, ce->skb->data, len);
>>> +		pci_dma_sync_single_for_device(pdev,
>>> +					       pci_unmap_addr(ce, dma_addr),
>>> +					       pci_unmap_len(ce, dma_len),
>>> +					       PCI_DMA_FROMDEVICE);
>>>  		recycle_fl_buf(fl, fl->cidx);
>>>  		return skb;
>>>  	}
>>>  
>>> -	if (fl->credits < drop_thres) {
>>> +use_orig_buf:
>>> +	if (fl->credits < 2) {
>>>       
>> Why 2? What does this magic number mean?
>>     
>
> No idea, it was there in the original. (as a parameter).
>
>   
The T2 HW behaves nicely when it is guaranteed to have 2 available 
entries in the rx free list.

Cheers,
Divy

Re: [PATCH 3/3] chelsio: more rx speedup

[Ingo Oeser]

Stephen Hemminger schrieb:
> On Tue, 9 Jan 2007 09:42:03 +0100
> Ingo Oeser <netdev@axxeo.de> wrote:
> > Stephen Hemminger schrieb:
> > > --- netdev-2.6.orig/drivers/net/chelsio/sge.c
> > > +++ netdev-2.6/drivers/net/chelsio/sge.c
> > Please use NET_IP_ALIGN here:
> 
> Wrong, NET_IP_ALIGN is intended to deal with platforms where alignment of DMA is more
> important of alignment of structures.  Therefore if data is copied, it should
> always be 2.

Ah! Thanks for clearing this up. These magic numbers always make my head spin
so I tried to come up with sth. useful :-)

If this pattern is present in many drivers, we might have a define or helper for this.


Regards

Ingo Oeser

Re: [PATCH 3/3] chelsio: more rx speedup

[Ingo Oeser]

Divy Le Ray schrieb:
> Stephen Hemminger wrote:
> > On Tue, 9 Jan 2007 09:42:03 +0100
> > Ingo Oeser <netdev@axxeo.de> wrote:
> >> Stephen Hemminger schrieb:
> >>> -	if (fl->credits < drop_thres) {
> >>> +use_orig_buf:
> >>> +	if (fl->credits < 2) {
> >>>       
> >> Why 2? What does this magic number mean?
> > No idea, it was there in the original. (as a parameter).
> The T2 HW behaves nicely when it is guaranteed to have 2 available 
> entries in the rx free list.

Can we have a #define for this?

That would help people understand this issue more.
And don't be shy about errata, all hardware and software out there has 
them like the humans that made them :-)

Regards

Ingo Oeser

Re: [PATCH 2/3] chelsio: NAPI speed improvement

[Jeff Garzik]

Stephen Hemminger wrote:
> Speedup and cleanup the receive processing by eliminating the
> mmio read and a lock round trip.
> 
> Signed-off-by: Stephen Hemminger <shemminger@osdl.org>

applied

Re: [PATCH 3/3] chelsio: more rx speedup

[Jeff Garzik]

Stephen Hemminger wrote:
> Cleanup receive processing some more:
>    * do the reserve padding of skb during setup
>    * don't pass constants to get_packet
>    * do smart prefetch of skb
>    * make copybreak a module parameter
> 
> Signed-off-by: Stephen Hemminger <shemminger@osdl.org>

applied