Re: Re:[PATCH v14 06/17] Use callback to deal with skb_release_data() specially.
Le lundi 08 novembre 2010 à 16:03 +0800, xiaohui@intel.com a écrit :
From: Xin Xiaohui xiaohui@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;
__u8tx_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_tdataref;
-/* 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.
--
To unsubscribe from this list: send the line unsubscribe kvm in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: Re:[PATCH v14 06/17] Use callback to deal with skb_release_data() specially.
-Original Message-
From: Eric Dumazet [mailto:eric.duma...@gmail.com]
Sent: Monday, November 08, 2010 4:25 PM
To: Xin, Xiaohui
Cc: net...@vger.kernel.org; kvm@vger.kernel.org; linux-ker...@vger.kernel.org;
m...@redhat.com; mi...@elte.hu; da...@davemloft.net;
herb...@gondor.apana.org.au;
jd...@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@intel.com a écrit :
From: Xin Xiaohui xiaohui@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;
__u8tx_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_tdataref;
- /* 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 kvm in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re:[PATCH v14 06/17] Use callback to deal with skb_release_data() specially.
From: Xin Xiaohui xiaohui@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;
__u8tx_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_tdataref;
- /* 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?
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:
@@ -343,6 +344,13 @@ static void skb_release_data(struct sk_buff *skb)
if (skb_has_frags(skb))
skb_drop_fraglist(skb);
+ if (skb-dev dev_is_mpassthru(skb-dev)) {
+ struct skb_ext_page *ext_page =
+ skb_shinfo(skb)-destructor_arg;
+ if (ext_page ext_page-dtor)
+ ext_page-dtor(ext_page);
+ }
+
kfree(skb-head);
}
}
--
1.7.3
--
To unsubscribe from this list: send the line unsubscribe kvm in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v14 06/17] Use callback to deal with skb_release_data() specially.
Le jeudi 04 novembre 2010 à 17:05 +0800, xiaohui@intel.com a écrit :
From: Xin Xiaohui xiaohui@intel.com
If buffer is external, then use the callback to destruct
buffers.
Signed-off-by: Xin Xiaohui xiaohui@intel.com
Signed-off-by: Zhao Yu yzhao81...@gmail.com
Reviewed-by: Jeff Dike jd...@linux.intel.com
---
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..5e6d69c 100644
--- a/net/core/skbuff.c
+++ b/net/core/skbuff.c
@@ -210,6 +210,7 @@ struct sk_buff *__alloc_skb(unsigned int size, gfp_t
gfp_mask,
/* make sure we initialize shinfo sequentially */
shinfo = skb_shinfo(skb);
+ shinfo-destructor_arg = NULL;
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...
memset(shinfo, 0, offsetof(struct skb_shared_info, dataref));
atomic_set(shinfo-dataref, 1);
@@ -343,6 +344,13 @@ static void skb_release_data(struct sk_buff *skb)
if (skb_has_frags(skb))
skb_drop_fraglist(skb);
+ if (skb-dev dev_is_mpassthru(skb-dev)) {
+ struct skb_ext_page *ext_page =
+ skb_shinfo(skb)-destructor_arg;
+ if (ext_page ext_page-dtor)
+ ext_page-dtor(ext_page);
+ }
+
kfree(skb-head);
}
}
--
To unsubscribe from this list: send the line unsubscribe kvm in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v14 06/17] Use callback to deal with skb_release_data() specially.
Le jeudi 04 novembre 2010 à 10:04 +0100, Eric Dumazet a écrit :
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;
__u8tx_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_tdataref;
- /* 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];
};
--
To unsubscribe from this list: send the line unsubscribe kvm in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
