Message ID | 4D6CA860.3020409@cn.fujitsu.com |
---|---|
State | Not Applicable, archived |
Delegated to: | David Miller |
Headers | show |
From: Lai Jiangshan <laijs@cn.fujitsu.com> Date: Tue, 01 Mar 2011 16:03:44 +0800 > > struct dst_entry assumes the size of struct rcu_head as 2 * sizeof(long) > and manually adds pads for aligning for "__refcnt". > > When the size of struct rcu_head is changed, these manual padding > is wrong. Use __attribute__((aligned (64))) instead. > > Signed-off-by: Lai Jiangshan <laijs@cn.fujitsu.com> We don't want to use the align if it's going to waste lots of space. Instead we want to rearrange the structure so that the alignment comes more cheaply. -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Le mardi 01 mars 2011 à 16:03 +0800, Lai Jiangshan a écrit : > struct dst_entry assumes the size of struct rcu_head as 2 * sizeof(long) > and manually adds pads for aligning for "__refcnt". > > When the size of struct rcu_head is changed, these manual padding > is wrong. Use __attribute__((aligned (64))) instead. > > Signed-off-by: Lai Jiangshan <laijs@cn.fujitsu.com> > --- > diff --git a/include/net/dst.h b/include/net/dst.h > index 93b0310..4ef6c4a 100644 > --- a/include/net/dst.h > +++ b/include/net/dst.h > @@ -62,8 +62,6 @@ struct dst_entry { > struct hh_cache *hh; > #ifdef CONFIG_XFRM > struct xfrm_state *xfrm; > -#else > - void *__pad1; > #endif > int (*input)(struct sk_buff*); > int (*output)(struct sk_buff*); > @@ -74,23 +72,18 @@ struct dst_entry { > > #ifdef CONFIG_NET_CLS_ROUTE > __u32 tclassid; > -#else > - __u32 __pad2; > #endif > > > /* > * Align __refcnt to a 64 bytes alignment > * (L1_CACHE_SIZE would be too much) > - */ > -#ifdef CONFIG_64BIT > - long __pad_to_align_refcnt[1]; > -#endif > - /* > + * > * __refcnt wants to be on a different cache line from > * input/output/ops or performance tanks badly > */ > - atomic_t __refcnt; /* client references */ > + atomic_t __refcnt /* client references */ > + __attribute__((aligned (64))); > int __use; > unsigned long lastuse; > union { If struct rcu_head is bigger, this is for debugging purposes, so we dont care about performance, and can avoid wasting ~64 bytes. Some machines still have about 2.000.000 active dst entries : the convoluted checks we added in include/net/dst.h are here to make sure we dont have huge holes in the dst structure. (This might change when/if IP route cache is gone) -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Le mardi 01 mars 2011 à 00:16 -0800, David Miller a écrit : > From: Lai Jiangshan <laijs@cn.fujitsu.com> > Date: Tue, 01 Mar 2011 16:03:44 +0800 > > > > > struct dst_entry assumes the size of struct rcu_head as 2 * sizeof(long) > > and manually adds pads for aligning for "__refcnt". > > > > When the size of struct rcu_head is changed, these manual padding > > is wrong. Use __attribute__((aligned (64))) instead. > > > > Signed-off-by: Lai Jiangshan <laijs@cn.fujitsu.com> > > We don't want to use the align if it's going to waste lots of space. > > Instead we want to rearrange the structure so that the alignment comes > more cheaply. Oh well, I should have read your answer before sending mine :) -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/include/net/dst.h b/include/net/dst.h index 93b0310..4ef6c4a 100644 --- a/include/net/dst.h +++ b/include/net/dst.h @@ -62,8 +62,6 @@ struct dst_entry { struct hh_cache *hh; #ifdef CONFIG_XFRM struct xfrm_state *xfrm; -#else - void *__pad1; #endif int (*input)(struct sk_buff*); int (*output)(struct sk_buff*); @@ -74,23 +72,18 @@ struct dst_entry { #ifdef CONFIG_NET_CLS_ROUTE __u32 tclassid; -#else - __u32 __pad2; #endif /* * Align __refcnt to a 64 bytes alignment * (L1_CACHE_SIZE would be too much) - */ -#ifdef CONFIG_64BIT - long __pad_to_align_refcnt[1]; -#endif - /* + * * __refcnt wants to be on a different cache line from * input/output/ops or performance tanks badly */ - atomic_t __refcnt; /* client references */ + atomic_t __refcnt /* client references */ + __attribute__((aligned (64))); int __use; unsigned long lastuse; union {
struct dst_entry assumes the size of struct rcu_head as 2 * sizeof(long) and manually adds pads for aligning for "__refcnt". When the size of struct rcu_head is changed, these manual padding is wrong. Use __attribute__((aligned (64))) instead. Signed-off-by: Lai Jiangshan <laijs@cn.fujitsu.com> --- -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html