From: Eric Dumazet <eric.dumazet@gmail.com>
To: Paul Gortmaker <paul.gortmaker@windriver.com>
Cc: David Miller <davem@davemloft.net>,
netdev <netdev@vger.kernel.org>,
Manfred Rudigier <manfred.rudigier@omicron.at>,
Claudiu Manoil <claudiu.manoil@freescale.com>,
Jiajun Wu <b06378@freescale.com>,
Andy Fleming <afleming@freescale.com>
Subject: Re: [PATCH] gianfar: fix potential sk_wmem_alloc imbalance
Date: Sun, 08 Jul 2012 11:09:02 +0200 [thread overview]
Message-ID: <1341738542.3265.1406.camel@edumazet-glaptop> (raw)
In-Reply-To: <20120706180947.GI1817@windriver.com>
On Fri, 2012-07-06 at 14:09 -0400, Paul Gortmaker wrote:
> Aside from the one line change at driver init, is there more to it than
> that? More specifically, it currently does:
>
> fcb_length = GMAC_FCB_LEN;
>
> if (...timestamps...)
> fcb_length = GMAC_FCB_LEN + GMAC_TXPAL_LEN;
>
> if (... && (skb_headroom(skb) < fcb_length))
> ...
> skb_new = skb_realloc_headroom(skb, fcb_length);
>
> and I don't know the code well enough to know if setting the
> needed_headroom value _guarantees_ the above fcb_length comparison
> will always be false, and hence can be deleted. It kind of looks
> like it via LL_RESERVED_SPACE, but I'm not 100% sure...
This is not a guarantee but should take care of most cases.
So we should keep the test anyway.
And init needed_headroom to the largest room. Existing tests seems to
ignore vlan case and timestamping.
diff --git a/drivers/net/ethernet/freescale/gianfar.c b/drivers/net/ethernet/freescale/gianfar.c
index af16f9f..b4517b7 100644
--- a/drivers/net/ethernet/freescale/gianfar.c
+++ b/drivers/net/ethernet/freescale/gianfar.c
@@ -1084,9 +1084,7 @@ static int gfar_probe(struct platform_device *ofdev)
else
priv->padding = 0;
- if (dev->features & NETIF_F_IP_CSUM ||
- priv->device_flags & FSL_GIANFAR_DEV_HAS_TIMER)
- dev->needed_headroom = GMAC_FCB_LEN;
+ dev->needed_headroom = GMAC_FCB_LEN + GMAC_TXPAL_LEN;
/* Program the isrg regs only if number of grps > 1 */
if (priv->num_grps > 1) {
next prev parent reply other threads:[~2012-07-08 9:09 UTC|newest]
Thread overview: 6+ messages / expand[flat|nested] mbox.gz Atom feed top
2012-07-05 21:45 [PATCH] gianfar: fix potential sk_wmem_alloc imbalance Eric Dumazet
2012-07-06 18:09 ` Paul Gortmaker
2012-07-08 9:09 ` Eric Dumazet [this message]
2012-07-11 8:06 ` Claudiu Manoil
2012-07-09 21:38 ` Paul Gortmaker
2012-07-09 22:28 ` David Miller
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=1341738542.3265.1406.camel@edumazet-glaptop \
--to=eric.dumazet@gmail.com \
--cc=afleming@freescale.com \
--cc=b06378@freescale.com \
--cc=claudiu.manoil@freescale.com \
--cc=davem@davemloft.net \
--cc=manfred.rudigier@omicron.at \
--cc=netdev@vger.kernel.org \
--cc=paul.gortmaker@windriver.com \
/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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox