lkml.org 
[lkml]   [2010]   [Nov]   [8]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    From
    Date
    SubjectRE: Re:[PATCH v14 06/17] Use callback to deal with skb_release_data() specially.
    >-----Original Message-----
    >From: Eric Dumazet [mailto:eric.dumazet@gmail.com]
    >Sent: Monday, November 08, 2010 4:25 PM
    >To: Xin, Xiaohui
    >Cc: netdev@vger.kernel.org; kvm@vger.kernel.org; linux-kernel@vger.kernel.org;
    >mst@redhat.com; mingo@elte.hu; davem@davemloft.net; herbert@gondor.apana.org.au;
    >jdike@linux.intel.com
    >Subject: Re: Re:[PATCH v14 06/17] Use callback to deal with skb_release_data() specially.
    >
    >Le lundi 08 novembre 2010 à 16:03 +0800, xiaohui.xin@intel.com a écrit :
    >> From: Xin Xiaohui <xiaohui.xin@intel.com>
    >>
    >> >> Hmm, I suggest you read the comment two lines above.
    >> >>
    >> >> If destructor_arg is now cleared each time we allocate a new skb, then,
    >> >> please move it before dataref in shinfo structure, so that the following
    >> >> memset() does the job efficiently...
    >> >
    >> >
    >> >Something like :
    >> >
    >> >diff --git a/include/linux/skbuff.h b/include/linux/skbuff.h
    >> >index e6ba898..2dca504 100644
    >> >--- a/include/linux/skbuff.h
    >> >+++ b/include/linux/skbuff.h
    >> >@@ -195,6 +195,9 @@ struct skb_shared_info {
    >> > __be32 ip6_frag_id;
    >> > __u8 tx_flags;
    >> > struct sk_buff *frag_list;
    >> >+ /* Intermediate layers must ensure that destructor_arg
    >> >+ * remains valid until skb destructor */
    >> >+ void *destructor_arg;
    >> > struct skb_shared_hwtstamps hwtstamps;
    >> >
    >> > /*
    >> >@@ -202,9 +205,6 @@ struct skb_shared_info {
    >> > */
    >> > atomic_t dataref;
    >> >
    >> >- /* Intermediate layers must ensure that destructor_arg
    >> >- * remains valid until skb destructor */
    >> >- void * destructor_arg;
    >> > /* must be last field, see pskb_expand_head() */
    >> > skb_frag_t frags[MAX_SKB_FRAGS];
    >> > };
    >> >
    >> >
    >>
    >> Will that affect the cache line?
    >
    >What do you mean ?
    >
    >> Or, we can move the line to clear destructor_arg to the end of __alloc_skb().
    >> It looks like as the following, which one do you prefer?
    >>
    >> Thanks
    >> Xiaohui
    >>
    >> ---
    >> net/core/skbuff.c | 8 ++++++++
    >> 1 files changed, 8 insertions(+), 0 deletions(-)
    >>
    >> diff --git a/net/core/skbuff.c b/net/core/skbuff.c
    >> index c83b421..df852f2 100644
    >> --- a/net/core/skbuff.c
    >> +++ b/net/core/skbuff.c
    >> @@ -224,6 +224,7 @@ struct sk_buff *__alloc_skb(unsigned int size, gfp_t gfp_mask,
    >>
    >> child->fclone = SKB_FCLONE_UNAVAILABLE;
    >> }
    >> + shinfo->destructor_arg = NULL;
    >> out:
    >> return skb;
    >> nodata:
    >
    >I dont understand why you want to do this.
    >
    >This adds an instruction, makes code bigger, and no obvious gain for me,
    >at memory transactions side.
    >
    >If integrated in the existing memset(), cost is an extra iteration to
    >perform the clear of this field.
    >
    Ok. Thanks for this explanation and will update with your solution.

    Thanks
    Xiaohui


    --
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

    \
     
     \ /
      Last update: 2010-11-08 09:41    [W:0.030 / U:212.404 seconds]
    ©2003-2016 Jasper Spaans. hosted at Digital OceanAdvertise on this site