[PATCH] Remove rx_copybreak, act as if it's 0

Remove a dubious optimization in the rx path.  For short (shorter than
rx_copybreak, 200 bytes by default) unfragmented packets, a new, shorter
skb would be allocated, and the data would be copied into it, allowing
the original skb to be recycled for another incoming packet.

For a good memory allocator, allocating an skb of any size should not
depend on the skb size as long at it remains shorter than a page.  At
least, the difference would be comparable to the price of copying the
data.

It's also likely that the newly allocated full-sized skb would actually
use the memory left by consumption of the previous skb.  Using variable
size skbs could increase memory fragmentation and time to find another
full-sized skb when it's finally needed.

And last but not least, correctness and maintainability of the driver is
more important than a marginal speed increase in the code, especially if
no waiting for hardware is involved.  Having two ways to do one thing
increases complexity of the driver and the risk of breakage.

Avoiding the optimization also allows to share more code between the
fragmented and non-fragmented cases.

Signed-off-by: Pavel Roskin <proski@gnu.org>

1 files changed, 12 insertions, 25 deletions

diff --git a/at76_usb.c b/at76_usb.c
index f7dd37c..b8ae492 100644
--- a/at76_usb.c
+++ b/at76_usb.c
@@ -171,7 +171,6 @@ static struct usb_device_id dev_table[] = {
 MODULE_DEVICE_TABLE(usb, dev_table);
 
 /* Module parameters */
-static int rx_copybreak = 200;
 static int scan_min_time = 10;
 static int scan_max_time = 120;
 
@@ -5374,9 +5373,8 @@ static void at76_ieee80211_to_eth(struct sk_buff *skb, int iw_mode)
    or it was the last of a fragment set a skb containing the whole packet
    is returned for further processing. Otherwise we get NULL and are
    done and the packet is either stored inside the fragment buffer
-   or thrown away. The check for rx_copybreak is moved here.
-   Every returned skb starts with the ieee802_11 header and contains
-   _no_ FCS at the end */
+   or thrown away.  Every returned skb starts with the ieee802_11 header
+   and contains _no_ FCS at the end */
 static struct sk_buff *at76_check_for_rx_frags(struct at76_priv *priv)
 {
 	struct sk_buff *skb = priv->rx_skb;
@@ -5421,30 +5419,13 @@ static struct sk_buff *at76_check_for_rx_frags(struct at76_priv *priv)
 		return NULL;
 	}
 
-	if (fragnr == 0 && !(frame_ctl & IEEE80211_FCTL_MOREFRAGS)) {
-		/* unfragmented packet received */
-		if (length < rx_copybreak
-		    && (skb = dev_alloc_skb(length)) != NULL) {
-			memcpy(skb_put(skb, length),
-			       priv->rx_skb->data + AT76_RX_HDRLEN, length);
-		} else {
-			skb_pull(skb, AT76_RX_HDRLEN);
-			skb_trim(skb, length);
-			/* Use a new skb for the next receive */
-			priv->rx_skb = NULL;
-		}
-
-		at76_dbg(DBG_RX_FRAGS, "%s: unfragmented", priv->netdev->name);
-
-		return skb;
-	}
+	at76_assert(length > AT76_RX_HDRLEN);
 
 	/* remove the at76_rx_buffer header - we don't need it anymore */
 	/* we need the IEEE802.11 header (for the addresses) if this packet
 	   is the first of a chain */
-
-	at76_assert(length > AT76_RX_HDRLEN);
 	skb_pull(skb, AT76_RX_HDRLEN);
+
 	/* remove FCS at end */
 	skb_trim(skb, length);
 
@@ -5453,6 +5434,14 @@ static struct sk_buff *at76_check_for_rx_frags(struct at76_priv *priv)
 		 skb->head, skb->data, skb_tail_pointer(skb),
 		 skb_end_pointer(skb), skb->len, data, data_len);
 
+	if (fragnr == 0 && !(frame_ctl & IEEE80211_FCTL_MOREFRAGS)) {
+		/* unfragmented packet received */
+		/* Use a new skb for the next receive */
+		priv->rx_skb = NULL;
+		at76_dbg(DBG_RX_FRAGS, "%s: unfragmented", priv->netdev->name);
+		return skb;
+	}
+
 	/* look if we've got a chain for the sender address.
 	   afterwards optr points to first free or the oldest entry,
 	   or, if i < NR_RX_DATA_BUF, bptr points to the entry for the
@@ -6093,8 +6082,6 @@ static void __exit at76_mod_exit(void)
 
 module_param_named(debug, at76_debug, int, 0600);
 MODULE_PARM_DESC(debug, "Debugging level");
-module_param(rx_copybreak, int, 0400);
-MODULE_PARM_DESC(rx_copybreak, "rx packet copy threshold");
 
 module_init(at76_mod_init);
 module_exit(at76_mod_exit);