All of lore.kernel.org
 help / color / mirror / Atom feed
From: Eran Liberty <liberty@extricom.com>
To: David Miller <davem@davemloft.net>
Cc: galak@kernel.crashing.org, LKML <linux-kernel@vger.kernel.org>
Subject: [PATCH] gainfar.c : code cleanup
Date: Tue, 29 Jun 2010 19:20:02 +0300	[thread overview]
Message-ID: <4C2A1D32.1040501@extricom.com> (raw)
In-Reply-To: <4C2A145D.1000702@extricom.com>

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

This patch relates to "[PATCH] gainfar.c : skb_over_panic 
(kernel-2.6.32.15)"

While in 2.6.32.15 it actually fixed a bug here it merely cleans up the 
previous attempts to fix the bug with a more coherent code.

Currently before queuing skb into the rx_recycle it is 
"un-skb_reserve"-ed so when taken out in gfar_new_skb() it wont be 
reserved twice.

This patch makes sure the alignment skb_reserve is done once, upon 
allocating the skb and
not when taken out of the rx_recycle pool. Eliminating the need to undo 
anything before queue skb back to the pool.

NOTE: This patch will compile and is fairly straight forward  but I do 
not have environment to test it as I did with the 2.6.32.15 fix.

-- Liberty

Signed-off-by: Eran Liberty <liberty@extricom.com>


[-- Attachment #2: gianfar_skb_over_panic-linus.patch --]
[-- Type: text/x-diff, Size: 3025 bytes --]

---
 drivers/net/gianfar.c |   54 ++++++++++++++++++++++++------------------------
 1 files changed, 27 insertions(+), 27 deletions(-)

diff --git a/drivers/net/gianfar.c b/drivers/net/gianfar.c
index 28b53d1..334c8b5 100644
--- a/drivers/net/gianfar.c
+++ b/drivers/net/gianfar.c
@@ -2342,6 +2342,15 @@ static void gfar_timeout(struct net_device *dev)
 	schedule_work(&priv->reset_task);
 }
 
+static void gfar_align_skb(struct sk_buff *skb)
+{
+	/* We need the data buffer to be aligned properly.  We will reserve
+	 * as many bytes as needed to align the data properly
+	 */
+	skb_reserve(skb, RXBUF_ALIGNMENT -
+		(((unsigned long) skb->data) & (RXBUF_ALIGNMENT - 1)));
+}
+
 /* Interrupt Handler for Transmit complete */
 static int gfar_clean_tx_ring(struct gfar_priv_tx_q *tx_queue)
 {
@@ -2426,9 +2435,10 @@ static int gfar_clean_tx_ring(struct gfar_priv_tx_q *tx_queue)
 		 */
 		if (skb_queue_len(&priv->rx_recycle) < rx_queue->rx_ring_size &&
 				skb_recycle_check(skb, priv->rx_buffer_size +
-					RXBUF_ALIGNMENT))
+					RXBUF_ALIGNMENT)) {
+			gfar_align_skb(skb);
 			__skb_queue_head(&priv->rx_recycle, skb);
-		else
+		} else
 			dev_kfree_skb_any(skb);
 
 		tx_queue->tx_skbuff[skb_dirtytx] = NULL;
@@ -2491,29 +2501,28 @@ static void gfar_new_rxbdp(struct gfar_priv_rx_q *rx_queue, struct rxbd8 *bdp,
 	gfar_init_rxbdp(rx_queue, bdp, buf);
 }
 
-
-struct sk_buff * gfar_new_skb(struct net_device *dev)
+static struct sk_buff * gfar_alloc_skb(struct net_device *dev)
 {
-	unsigned int alignamount;
 	struct gfar_private *priv = netdev_priv(dev);
 	struct sk_buff *skb = NULL;
 
-	skb = __skb_dequeue(&priv->rx_recycle);
-	if (!skb)
-		skb = netdev_alloc_skb(dev,
-				priv->rx_buffer_size + RXBUF_ALIGNMENT);
-
+	skb = netdev_alloc_skb(dev, priv->rx_buffer_size + RXBUF_ALIGNMENT);
 	if (!skb)
 		return NULL;
 
-	alignamount = RXBUF_ALIGNMENT -
-		(((unsigned long) skb->data) & (RXBUF_ALIGNMENT - 1));
+	gfar_align_skb(skb);
 
-	/* We need the data buffer to be aligned properly.  We will reserve
-	 * as many bytes as needed to align the data properly
-	 */
-	skb_reserve(skb, alignamount);
-	GFAR_CB(skb)->alignamount = alignamount;
+	return skb;
+}
+
+struct sk_buff * gfar_new_skb(struct net_device *dev)
+{
+	struct gfar_private *priv = netdev_priv(dev);
+	struct sk_buff *skb = NULL;
+
+	skb = __skb_dequeue(&priv->rx_recycle);
+	if (!skb)
+		skb = gfar_alloc_skb(dev);
 
 	return skb;
 }
@@ -2666,17 +2675,8 @@ int gfar_clean_rx_ring(struct gfar_priv_rx_q *rx_queue, int rx_work_limit)
 
 			if (unlikely(!newskb))
 				newskb = skb;
-			else if (skb) {
-				/*
-				 * We need to un-reserve() the skb to what it
-				 * was before gfar_new_skb() re-aligned
-				 * it to an RXBUF_ALIGNMENT boundary
-				 * before we put the skb back on the
-				 * recycle list.
-				 */
-				skb_reserve(skb, -GFAR_CB(skb)->alignamount);
+			else if (skb) 
 				__skb_queue_head(&priv->rx_recycle, skb);
-			}
 		} else {
 			/* Increment the number of packets */
 			rx_queue->stats.rx_packets++;
-- 
1.5.6.5


  reply	other threads:[~2010-06-29 16:17 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2010-06-24  9:53 [PATCH] gainfar.c : skb_over_panic (kernel-2.6.32.15) Eran Liberty
2010-06-24 21:52 ` David Miller
2010-06-28  7:57   ` Eran Liberty
2010-06-28 18:33     ` David Miller
2010-06-29 15:42       ` Eran Liberty
2010-06-29 16:20         ` Eran Liberty [this message]
2010-09-29  0:35         ` emin ak
2010-10-03  9:32           ` Eran Liberty
2010-10-03 10:54             ` emin ak
2010-10-03 14:48               ` Eran Liberty

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=4C2A1D32.1040501@extricom.com \
    --to=liberty@extricom.com \
    --cc=davem@davemloft.net \
    --cc=galak@kernel.crashing.org \
    --cc=linux-kernel@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.