Re: [PATCH RFC] netfilter: cacheline aligning in struct netns_ct
Thanks Jesper and David for reviewing and replying.
Great commitment of Jesper for moving above three hlist_nulls_heads to
percpu structure. Sorry to miss the newest advances in nf-next.
And arbitrarily cache-line aligning structure members is not a good idea
for large cache lines.
Still I have an idea that could we pack the seldom changed members
(nf_ct_proto included) to a structure and we allocated it from a
cache aligned slub when the structure "net" is initialized.
Zhouyi
On Wed, Mar 12, 2014 at 5:36 PM, David Laight wrote:
> From: Zhouyi Zhou
>> not frequently changing components should share same cachelines
>>
>> Signed-off-by: Zhouyi Zhou
>> ---
>> include/net/netns/conntrack.h | 12
>> 1 file changed, 8 insertions(+), 4 deletions(-)
>>
>> diff --git a/include/net/netns/conntrack.h b/include/net/netns/conntrack.h
>> index fbcc7fa..69d2d58 100644
>> --- a/include/net/netns/conntrack.h
>> +++ b/include/net/netns/conntrack.h
>> @@ -65,6 +65,12 @@ struct nf_ip_net {
>> struct netns_ct {
>> atomic_tcount;
>> unsigned intexpect_count;
>> + struct hlist_nulls_head unconfirmed;
>> + struct hlist_nulls_head dying;
>> + struct hlist_nulls_head tmpl;
>> +
>> + /* not frequently changing components should share same cachelines */
>> + struct nf_ip_netnf_ct_proto cacheline_aligned_in_smp;
>
> I'm not sure this has the effect you are trying to achieve.
> Adding an __attribute__((aligned(n))) to a structure member not only
> adds padding before the member, but also pads the member to a multiple
> of the specified size.
>
> With large cache lines you probably want neither.
> IIRC one of the ppc systems has something like 256 byte cache lines
> (and if it doesn't now, it might have soon).
> So arbitrarily cache-line aligning structure members may not be a good idea.
>
> In any case reducing the number of cache lines accessed, and the number
> dirtied is probably more important than putting 'readonly' data in its
> own cache lines.
>
> David
>
>
>
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
RE: [PATCH RFC] netfilter: cacheline aligning in struct netns_ct
From: Zhouyi Zhou
> not frequently changing components should share same cachelines
>
> Signed-off-by: Zhouyi Zhou
> ---
> include/net/netns/conntrack.h | 12
> 1 file changed, 8 insertions(+), 4 deletions(-)
>
> diff --git a/include/net/netns/conntrack.h b/include/net/netns/conntrack.h
> index fbcc7fa..69d2d58 100644
> --- a/include/net/netns/conntrack.h
> +++ b/include/net/netns/conntrack.h
> @@ -65,6 +65,12 @@ struct nf_ip_net {
> struct netns_ct {
> atomic_tcount;
> unsigned intexpect_count;
> + struct hlist_nulls_head unconfirmed;
> + struct hlist_nulls_head dying;
> + struct hlist_nulls_head tmpl;
> +
> + /* not frequently changing components should share same cachelines */
> + struct nf_ip_netnf_ct_proto cacheline_aligned_in_smp;
I'm not sure this has the effect you are trying to achieve.
Adding an __attribute__((aligned(n))) to a structure member not only
adds padding before the member, but also pads the member to a multiple
of the specified size.
With large cache lines you probably want neither.
IIRC one of the ppc systems has something like 256 byte cache lines
(and if it doesn't now, it might have soon).
So arbitrarily cache-line aligning structure members may not be a good idea.
In any case reducing the number of cache lines accessed, and the number
dirtied is probably more important than putting 'readonly' data in its
own cache lines.
David
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH RFC] netfilter: cacheline aligning in struct netns_ct
On Wed, 12 Mar 2014 15:56:34 +0800
Zhouyi Zhou wrote:
> not frequently changing components should share same cachelines
>
> Signed-off-by: Zhouyi Zhou
> ---
> include/net/netns/conntrack.h | 12
> 1 file changed, 8 insertions(+), 4 deletions(-)
NACK because I have already moved these "special" lists in commit:
https://git.kernel.org/cgit/linux/kernel/git/pablo/nf-next.git/commit/?id=b7779d06f9950e14a008a2de970b44233fe49c86
Next remember that this struct netns_ct, is inlined in struct net
(include/net/net_namespace.h). Thus, the alignment is not quite as you
might expect...
> diff --git a/include/net/netns/conntrack.h b/include/net/netns/conntrack.h
> index fbcc7fa..69d2d58 100644
> --- a/include/net/netns/conntrack.h
> +++ b/include/net/netns/conntrack.h
> @@ -65,6 +65,12 @@ struct nf_ip_net {
> struct netns_ct {
> atomic_tcount;
> unsigned intexpect_count;
> + struct hlist_nulls_head unconfirmed;
> + struct hlist_nulls_head dying;
> + struct hlist_nulls_head tmpl;
> +
> + /* not frequently changing components should share same cachelines */
> + struct nf_ip_netnf_ct_proto cacheline_aligned_in_smp;
> #ifdef CONFIG_SYSCTL
> struct ctl_table_header *sysctl_header;
> struct ctl_table_header *acct_sysctl_header;
> @@ -86,13 +92,11 @@ struct netns_ct {
> struct kmem_cache *nf_conntrack_cachep;
> struct hlist_nulls_head *hash;
> struct hlist_head *expect_hash;
> - struct hlist_nulls_head unconfirmed;
> - struct hlist_nulls_head dying;
> - struct hlist_nulls_head tmpl;
> +
> struct ip_conntrack_stat __percpu *stat;
> struct nf_ct_event_notifier __rcu *nf_conntrack_event_cb;
> struct nf_exp_event_notifier __rcu *nf_expect_event_cb;
> - struct nf_ip_netnf_ct_proto;
> +
> #if defined(CONFIG_NF_CONNTRACK_LABELS)
> unsigned intlabels_used;
> u8 label_words;
--
Best regards,
Jesper Dangaard Brouer
MSc.CS, Sr. Network Kernel Developer at Red Hat
Author of http://www.iptv-analyzer.org
LinkedIn: http://www.linkedin.com/in/brouer
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH RFC] netfilter: cacheline aligning in struct netns_ct
On Wed, 12 Mar 2014 15:56:34 +0800
Zhouyi Zhou zhouzho...@gmail.com wrote:
not frequently changing components should share same cachelines
Signed-off-by: Zhouyi Zhou yizhouz...@ict.ac.cn
---
include/net/netns/conntrack.h | 12
1 file changed, 8 insertions(+), 4 deletions(-)
NACK because I have already moved these special lists in commit:
https://git.kernel.org/cgit/linux/kernel/git/pablo/nf-next.git/commit/?id=b7779d06f9950e14a008a2de970b44233fe49c86
Next remember that this struct netns_ct, is inlined in struct net
(include/net/net_namespace.h). Thus, the alignment is not quite as you
might expect...
diff --git a/include/net/netns/conntrack.h b/include/net/netns/conntrack.h
index fbcc7fa..69d2d58 100644
--- a/include/net/netns/conntrack.h
+++ b/include/net/netns/conntrack.h
@@ -65,6 +65,12 @@ struct nf_ip_net {
struct netns_ct {
atomic_tcount;
unsigned intexpect_count;
+ struct hlist_nulls_head unconfirmed;
+ struct hlist_nulls_head dying;
+ struct hlist_nulls_head tmpl;
+
+ /* not frequently changing components should share same cachelines */
+ struct nf_ip_netnf_ct_proto cacheline_aligned_in_smp;
#ifdef CONFIG_SYSCTL
struct ctl_table_header *sysctl_header;
struct ctl_table_header *acct_sysctl_header;
@@ -86,13 +92,11 @@ struct netns_ct {
struct kmem_cache *nf_conntrack_cachep;
struct hlist_nulls_head *hash;
struct hlist_head *expect_hash;
- struct hlist_nulls_head unconfirmed;
- struct hlist_nulls_head dying;
- struct hlist_nulls_head tmpl;
+
struct ip_conntrack_stat __percpu *stat;
struct nf_ct_event_notifier __rcu *nf_conntrack_event_cb;
struct nf_exp_event_notifier __rcu *nf_expect_event_cb;
- struct nf_ip_netnf_ct_proto;
+
#if defined(CONFIG_NF_CONNTRACK_LABELS)
unsigned intlabels_used;
u8 label_words;
--
Best regards,
Jesper Dangaard Brouer
MSc.CS, Sr. Network Kernel Developer at Red Hat
Author of http://www.iptv-analyzer.org
LinkedIn: http://www.linkedin.com/in/brouer
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
RE: [PATCH RFC] netfilter: cacheline aligning in struct netns_ct
From: Zhouyi Zhou
not frequently changing components should share same cachelines
Signed-off-by: Zhouyi Zhou yizhouz...@ict.ac.cn
---
include/net/netns/conntrack.h | 12
1 file changed, 8 insertions(+), 4 deletions(-)
diff --git a/include/net/netns/conntrack.h b/include/net/netns/conntrack.h
index fbcc7fa..69d2d58 100644
--- a/include/net/netns/conntrack.h
+++ b/include/net/netns/conntrack.h
@@ -65,6 +65,12 @@ struct nf_ip_net {
struct netns_ct {
atomic_tcount;
unsigned intexpect_count;
+ struct hlist_nulls_head unconfirmed;
+ struct hlist_nulls_head dying;
+ struct hlist_nulls_head tmpl;
+
+ /* not frequently changing components should share same cachelines */
+ struct nf_ip_netnf_ct_proto cacheline_aligned_in_smp;
I'm not sure this has the effect you are trying to achieve.
Adding an __attribute__((aligned(n))) to a structure member not only
adds padding before the member, but also pads the member to a multiple
of the specified size.
With large cache lines you probably want neither.
IIRC one of the ppc systems has something like 256 byte cache lines
(and if it doesn't now, it might have soon).
So arbitrarily cache-line aligning structure members may not be a good idea.
In any case reducing the number of cache lines accessed, and the number
dirtied is probably more important than putting 'readonly' data in its
own cache lines.
David
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH RFC] netfilter: cacheline aligning in struct netns_ct
Thanks Jesper and David for reviewing and replying.
Great commitment of Jesper for moving above three hlist_nulls_heads to
percpu structure. Sorry to miss the newest advances in nf-next.
And arbitrarily cache-line aligning structure members is not a good idea
for large cache lines.
Still I have an idea that could we pack the seldom changed members
(nf_ct_proto included) to a structure and we allocated it from a
cache aligned slub when the structure net is initialized.
Zhouyi
On Wed, Mar 12, 2014 at 5:36 PM, David Laight david.lai...@aculab.com wrote:
From: Zhouyi Zhou
not frequently changing components should share same cachelines
Signed-off-by: Zhouyi Zhou yizhouz...@ict.ac.cn
---
include/net/netns/conntrack.h | 12
1 file changed, 8 insertions(+), 4 deletions(-)
diff --git a/include/net/netns/conntrack.h b/include/net/netns/conntrack.h
index fbcc7fa..69d2d58 100644
--- a/include/net/netns/conntrack.h
+++ b/include/net/netns/conntrack.h
@@ -65,6 +65,12 @@ struct nf_ip_net {
struct netns_ct {
atomic_tcount;
unsigned intexpect_count;
+ struct hlist_nulls_head unconfirmed;
+ struct hlist_nulls_head dying;
+ struct hlist_nulls_head tmpl;
+
+ /* not frequently changing components should share same cachelines */
+ struct nf_ip_netnf_ct_proto cacheline_aligned_in_smp;
I'm not sure this has the effect you are trying to achieve.
Adding an __attribute__((aligned(n))) to a structure member not only
adds padding before the member, but also pads the member to a multiple
of the specified size.
With large cache lines you probably want neither.
IIRC one of the ppc systems has something like 256 byte cache lines
(and if it doesn't now, it might have soon).
So arbitrarily cache-line aligning structure members may not be a good idea.
In any case reducing the number of cache lines accessed, and the number
dirtied is probably more important than putting 'readonly' data in its
own cache lines.
David
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
