netdev - [PATCH] net: make sure struct dst_entry refcount is aligned on 64 bytes

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Fri, 14 Nov 2008 11:47:01 +0100
From:	Eric Dumazet <dada1@...mosbay.com>
To:	David Miller <davem@...emloft.net>
CC:	Alexey Dobriyan <adobriyan@...il.com>, netdev@...r.kernel.org,
	shemminger@...tta.com,
	"Zhang, Yanmin" <yanmin_zhang@...ux.intel.com>
Subject: [PATCH] net: make sure struct dst_entry refcount is aligned on 64
 bytes

Alexey Dobriyan a écrit :
> On Fri, Nov 14, 2008 at 10:04:24AM +0100, Eric Dumazet wrote:
>> David Miller a écrit :
>>> From: Eric Dumazet <dada1@...mosbay.com>
>>> Date: Fri, 14 Nov 2008 09:09:31 +0100
>>>
>>>> During tbench/oprofile sessions, I found that dst_release() was in third position.
>>>  ...
>>>> Instead of first checking the refcount value, then decrement it,
>>>> we use atomic_dec_return() to help CPU to make the right memory transaction
>>>> (ie getting the cache line in exclusive mode)
>>>  ...
>>>> Signed-off-by: Eric Dumazet <dada1@...mosbay.com>
>>> This looks great, applied, thanks Eric.
>>>
>> Thanks David
>>
>>
>> I think I understood some regressions here on 32bits 
>>
>> offsetof(struct dst_entry, __refcnt) is 0x7c again !!!
>>
>> This is really really bad for performance
>>
>> I believe this comes from a patch from Alexey Dobriyan
>> (commit def8b4faff5ca349beafbbfeb2c51f3602a6ef3a
>> net: reduce structures when XFRM=n)
> 
> Ick.

Well, your patch is a good thing, we only need to make adjustments.

> 
>> This kills effort from Zhang Yanmin (and me...)
>>
>> (commit f1dd9c379cac7d5a76259e7dffcd5f8edc697d17
>> [NET]: Fix tbench regression in 2.6.25-rc1)
>>
>>
>> Really we must find something so that this damned __refcnt is starting at 0x80
> 
> Make it last member?

Yes, it will help tbench, but not machines that stress IP route cache

(dst_use() must dirty the three fields "refcnt, __use , lastuse" )

Also, 'next' pointer should be in the same cache line, to speedup route
cache lookups.

Next problem is that offsets depend on architecture being 32 or 64 bits.

On 64bit, offsetof(struct dst_entry, __refcnt) is 0xb0 : not very good...


[PATCH] net: make sure struct dst_entry refcount is aligned on 64 bytes

As found in the past (commit f1dd9c379cac7d5a76259e7dffcd5f8edc697d17
[NET]: Fix tbench regression in 2.6.25-rc1), it is really
important that struct dst_entry refcount is aligned on a cache line.

We cannot use __atribute((aligned)), so manually pad the structure
for 32 and 64 bit arches.

for 32bit : offsetof(truct dst_entry, __refcnt) is 0x80
for 64bit : offsetof(truct dst_entry, __refcnt) is 0xc0

As it is not possible to guess at compile time cache line size,
we use a generic value of 64 bytes, that satisfies many current arches.
(Using 128 bytes alignment on 64bit arches would waste 64 bytes)

Add a BUILD_BUG_ON to catch future updates to "struct dst_entry" dont
break this alignment.

"tbench 8" is 4.4 % faster on a dual quad core (HP BL460c G1), Intel E5450 @3.00GHz
(2350 MB/s instead of 2250 MB/s)

Signed-off-by: Eric Dumazet <dada1@...mosbay.com>
---
 include/net/dst.h |   19 +++++++++++++++++++
 1 files changed, 19 insertions(+)

View attachment "dst_align.patch" of type "text/plain" (1128 bytes)