linux-kernel - [PATCH] fs: pipe/sockets/anon dentries should have themselves as parent

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, 21 Nov 2008 18:58:29 +0100
From:	Eric Dumazet <dada1@...mosbay.com>
To:	Christoph Hellwig <hch@...radead.org>
CC:	David Miller <davem@...emloft.net>, mingo@...e.hu,
	cl@...ux-foundation.org, rjw@...k.pl, linux-kernel@...r.kernel.org,
	kernel-testers@...r.kernel.org, efault@....de,
	a.p.zijlstra@...llo.nl, Linux Netdev List <netdev@...r.kernel.org>,
	viro@...iv.linux.org.uk, linux-fsdevel@...r.kernel.org
Subject: [PATCH] fs: pipe/sockets/anon dentries should have themselves as
 parent

Christoph Hellwig a écrit :
> On Fri, Nov 21, 2008 at 04:13:38PM +0100, Eric Dumazet wrote:
>> [PATCH] fs: pipe/sockets/anon dentries should not have a parent
>>
>> Linking pipe/sockets/anon dentries to one root 'parent' has no functional
>> impact at all, but a scalability one.
>>
>> We can avoid touching a cache line at allocation stage (inside d_alloc(), no need
>> to touch root->d_count), but also at freeing time (in d_kill, decrementing d_count)
>> We avoid an expensive atomic_dec_and_lock() call on the root dentry.
>>
>> If we correct dnotify_parent() and inotify_d_instantiate() to take into account
>> a NULL d_parent, we can call d_alloc() with a NULL parent instead of root dentry.
> 
> Sorry folks, but a NULL d_parent is a no-go from the VFS perspective,
> but you can set d_parent to the dentry itself which is the magic used
> for root of tree dentries.  They should also be marked
> DCACHE_DISCONNECTED to make sure this is not unexpected.
> 
> And this kind of stuff really needs to go through -fsdevel.

Thanks Christoph for your review, sorry for fsdevel being forgotten.

d_alloc_root() is not an option here, since we also want such dentries
to be unhashed. So here is a second version, with the introduction
of a new helper, d_alloc_unhashed(), to be used by pipes, sockets and anon

I got even better numbers, probably because dnotify/inotify dont have
the NULL d_parent test anymore.

[PATCH] fs: pipe/sockets/anon dentries should have themselves as parent


Linking pipe/sockets/anon dentries to one root 'parent' has no functional
impact at all, but a scalability one.

We can avoid touching a cache line at allocation stage (inside d_alloc(), no need
to touch root->d_count), but also at freeing time (in d_kill, decrementing d_count)
We avoid an expensive atomic_dec_and_lock() call on the root dentry.

We add d_alloc_unhashed(const char *name, struct inode *inode) helper
to be used by pipes/socket/anon. This function is about the same as
d_alloc_root() but for unhashed entries.

Before patch, time to run 8 *  1 million of close(socket()) calls on 8 CPUS was :

real    0m27.496s
user    0m0.657s
sys     3m39.092s

After patch :

real    0m23.843s
user    0m0.616s
sys     3m9.732s


Old oprofile :
CPU: Core 2, speed 3000.11 MHz (estimated)
Counted CPU_CLK_UNHALTED events (Clock cycles when not halted) with a unit mask of 0x00 (Unhalted core cycles) count 100000
samples  cum. samples  %        cum. %     symbol name
164257   164257        11.0245  11.0245    init_file
155488   319745        10.4359  21.4604    d_alloc
151887   471632        10.1942  31.6547    _atomic_dec_and_lock
91620    563252         6.1493  37.8039    inet_create
74245    637497         4.9831  42.7871    kmem_cache_alloc
46702    684199         3.1345  45.9216    dentry_iput
46186    730385         3.0999  49.0215    tcp_close
42824    773209         2.8742  51.8957    kmem_cache_free
37275    810484         2.5018  54.3975    wake_up_inode
36553    847037         2.4533  56.8508    tcp_v4_init_sock
35661    882698         2.3935  59.2443    inotify_d_instantiate
32998    915696         2.2147  61.4590    sysenter_past_esp
31442    947138         2.1103  63.5693    d_instantiate
31303    978441         2.1010  65.6703    generic_forget_inode
27533    1005974        1.8479  67.5183    vfs_dq_drop
24237    1030211        1.6267  69.1450    sock_attach_fd
19290    1049501        1.2947  70.4397    __copy_from_user_ll


New oprofile :
CPU: Core 2, speed 3000.11 MHz (estimated)
Counted CPU_CLK_UNHALTED events (Clock cycles when not halted) with a unit mask of 0x00 (Unhalted core cycles) count 100000
samples  cum. samples  %        cum. %     symbol name
148703   148703        10.8581  10.8581    inet_create
116680   265383         8.5198  19.3779    new_inode
108912   374295         7.9526  27.3306    init_file
82911    457206         6.0541  33.3846    kmem_cache_alloc
65690    522896         4.7966  38.1812    wake_up_inode
53286    576182         3.8909  42.0721    _atomic_dec_and_lock
43814    619996         3.1992  45.2713    generic_forget_inode
41993    661989         3.0663  48.3376    d_alloc
41244    703233         3.0116  51.3492    kmem_cache_free
39244    742477         2.8655  54.2148    tcp_v4_init_sock
37402    779879         2.7310  56.9458    tcp_close
33336    813215         2.4342  59.3800    sysenter_past_esp
28596    841811         2.0880  61.4680    inode_has_buffers
25769    867580         1.8816  63.3496    d_kill
22606    890186         1.6507  65.0003    dentry_iput
20224    910410         1.4767  66.4770    vfs_dq_drop
19800    930210         1.4458  67.9228    __copy_from_user_ll

Signed-off-by: Eric Dumazet <dada1@...mosbay.com>
---
 fs/anon_inodes.c       |    9 +--------
 fs/dcache.c            |   31 +++++++++++++++++++++++++++++++
 fs/pipe.c              |   10 +---------
 include/linux/dcache.h |    1 +
 net/socket.c           |   10 +---------
 5 files changed, 35 insertions(+), 26 deletions(-)

View attachment "d_alloc_unhashed.patch" of type "text/plain" (4729 bytes)