On Thu, Jul 24, 2014 at 10:25:13PM +0530, Varka Bhadram wrote:
>
> On Thursday 24 July 2014 10:04 PM, Alexander Aring wrote:
> >On Thu, Jul 24, 2014 at 05:50:03PM +0530, Varka Bhadram wrote:
> >>Hi Alex,
> >>
> >>On 07/24/2014 05:40 PM, Alexander Aring wrote:
> >>>Varka,
> >>>
> >>>On Thu, Jul 24, 2014 at 05:16:39PM +0530, varkabhad...@gmail.com wrote:
> >>>>From: Varka Bhadram <var...@cdac.in>
> >>>>
> >>>>This patch replace the kfree(skb) with dev_kfree_skb() which is
> >>>s/kfree(skb)/kfree_skb(skb)
> >>>
> >>>>used on tx error path. And also use sizeof(*work) instead of
> >>>>sizeof(struct xmit_work)
> >>>>
> >>>>Signed-off-by: Varka Bhadram <var...@cdac.in>
> >>>>---
> >>>> net/mac802154/tx.c | 9 ++++-----
> >>>> 1 file changed, 4 insertions(+), 5 deletions(-)
> >>>>
> >>>>diff --git a/net/mac802154/tx.c b/net/mac802154/tx.c
> >>>>index 8124353..d05d6ea 100644
> >>>>--- a/net/mac802154/tx.c
> >>>>+++ b/net/mac802154/tx.c
> >>>>@@ -89,7 +89,7 @@ netdev_tx_t mac802154_tx(struct mac802154_priv *priv,
> >>>>struct sk_buff *skb,
> >>>> if (!(priv->phy->channels_supported[page] & (1 << chan))) {
> >>>> WARN_ON(1);
> >>>>- kfree_skb(skb);
> >>>>+ dev_kfree_skb(skb);
> >>>> return NETDEV_TX_OK;
> >>>> }
> >>>>@@ -104,13 +104,13 @@ netdev_tx_t mac802154_tx(struct mac802154_priv
> >>>>*priv, struct sk_buff *skb,
> >>>> }
> >>>> if (skb_cow_head(skb, priv->hw.extra_tx_headroom)) {
> >>>>- kfree_skb(skb);
> >>>>+ dev_kfree_skb(skb);
> >>Is it acceptable here..?
> >No on all error paths we need to use kfree_skb.
> >
> >Look commit 92a2ec72a7dbb84f4b614c9b72880d86db69475f. Actually at this
> >branch you reverting this commit.
> >
> >>>> return NETDEV_TX_OK;
> >>>> }
> >>>>- work = kzalloc(sizeof(struct xmit_work), GFP_ATOMIC);
> >>>>+ work = kzalloc(sizeof(*work), GFP_ATOMIC);
> >>>> if (!work) {
> >>>>- kfree_skb(skb);
> >>>>+ dev_kfree_skb(skb);
> >>>> return NETDEV_TX_BUSY;
> >>>> }
> >>>Sorry this is wrong. dev_kfree_skb is the same like consume_skb.
> >>>
> >>>Look for the documentation:
> >>>
> >>>"Drop a ref to the buffer and free it if the usage count has hit zero
> >>> Functions identically to kfree_skb, but kfree_skb assumes that the frame
> >>> is being dropped after a failure and notes that"
> >>This type of functionality use on IEEE-80211 subsystem also.. Every where
> >>on Tx
> >>error path subsystem uses dev_kfree_skb(skb);
> >>
> >mhh, maybe we could clear this. Can you give a link with an example
> >where they do this?
>
> Please see: http://lxr.free-electrons.com/source/net/mac80211/tx.c#L2218
> <http://lxr.free-electrons.com/source/net/mac80211/tx.c#L2218>
I think this is because "... but kfree_skb assumes that the frame is
being dropped after a failure and notes that". It's dropped afterwards,
then we need kfree_skb.
But ieee80211_subif_start_xmit the skb is not dropped afterwards. The function
comment is:
Returns: 0 on success (and frees skb in this case) or 1 on failure (skb will
not be freed, and caller is responsible for either retrying later or freeing
skb).
In this case the skb is not dropped, it's laying around and the caller can do a
retry or free the skb, then it's dropped.
And I compared the two functions, it looks almost the same except the trace
calls
which is for some function tracing, I think.
- Alex
------------------------------------------------------------------------------
Want fast and easy access to all the code in your enterprise? Index and
search up to 200,000 lines of code with a free copy of Black Duck
Code Sight - the same software that powers the world's largest code
search on Ohloh, the Black Duck Open Hub! Try it now.
http://p.sf.net/sfu/bds
_______________________________________________
Linux-zigbee-devel mailing list
Linux-zigbee-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-zigbee-devel
