On 03/01/2011 05:20 PM, Eric Dumazet wrote:
> Le mardi 01 mars 2011 Ã 16:53 +0800, Lai Jiangshan a Ãcrit :
>> On 03/01/2011 04:16 PM, David Miller wrote:
>>> From: Lai Jiangshan <laijs@xxxxxxxxxxxxxx>
>>> 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@xxxxxxxxxxxxxx>
>>>
>>> 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.
>>
>> Subject: [PATCH 4/4 V2] net,rcu: don't assume the size of struct rcu_head
>>
>> 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
>> are hardly suit for the changes. So we rearrange the structure,
>> and move the seldom access rcu_head to the end of the structure.
>>
>> Signed-off-by: Lai Jiangshan <laijs@xxxxxxxxxxxxxx>
>> ---
>>
>> diff --git a/include/net/dst.h b/include/net/dst.h
>> index 93b0310..d8c5296 100644
>> --- a/include/net/dst.h
>> +++ b/include/net/dst.h
>> @@ -37,7 +37,6 @@
>> struct sk_buff;
>>
>> struct dst_entry {
>> - struct rcu_head rcu_head;
>> struct dst_entry *child;
>> struct net_device *dev;
>> short error;
>> @@ -78,6 +77,13 @@ struct dst_entry {
>> __u32 __pad2;
>> #endif
>>
>> + unsigned long lastuse;
>> + union {
>> + struct dst_entry *next;
>> + struct rtable __rcu *rt_next;
>> + struct rt6_info *rt6_next;
>> + struct dn_route __rcu *dn_next;
>> + };
>>
>> /*
>> * Align __refcnt to a 64 bytes alignment
>> @@ -92,13 +98,7 @@ struct dst_entry {
>> */
>> atomic_t __refcnt; /* client references */
>> int __use;
>> - unsigned long lastuse;
>> - union {
>> - struct dst_entry *next;
>> - struct rtable __rcu *rt_next;
>> - struct rt6_info *rt6_next;
>> - struct dn_route __rcu *dn_next;
>> - };
>> + struct rcu_head rcu_head;
>> };
>>
>> #ifdef __KERNEL__
>
> Nope...
>
> "lastuse" and "next" must be in this place, or this introduce false
> sharing we wanted to avoid in the past.
>
> I suggest you leave this code as is, we will address the problem when
> rcu_head changes (assuming we can test a CONFIG_RCU_HEAD_DEBUG or
> something)
>
> First part of "struct dst_entry" is mostly read, while part beginning
> after refcnt is often written.
>
Is it the cause of false sharing? I thought that all are rare write(except __refcnt)
since it is protected by RCU.
Do you allow me just move the seldom access rcu_head to the end of the structure
and add pads before __refcnt? I guess it increases about 3% the size of dst_entry.
I accept that I leave this code as is, when I change rcu_head I will notify you.
Thanks,
Lai
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@xxxxxxxxxx For more info on Linux MM,
see: http://www.linux-mm.org/ .
Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/
Don't email: <a href