public inbox for netdev@vger.kernel.org 
 help / color / mirror / Atom feed
From: Eric Dumazet <dada1-fPLkHRcR87vqlBn2x/YWAg@public•gmane.org>
To: David Miller <davem-fT/PcQaiUtIeIZ0/mPfg9Q@public•gmane.org>,
	mingo-X9Un+BFzKDI@public•gmane.org
Cc: cl-de/tnXTf+JLsfHDXvbKv3WD2FQJk+8+b@public•gmane.org,
	rjw-KKrjLPT3xs0@public•gmane.org,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA@public•gmane.org,
	kernel-testers-u79uwXL29TY76Z2rM5mHXA@public•gmane.org,
	efault-Mmb7MZpHnFY@public•gmane.org,
	a.p.zijlstra-/NLkJaSkS4VmR6Xm/wNWPw@public•gmane.org,
	Linux Netdev List
	<netdev-u79uwXL29TY76Z2rM5mHXA@public•gmane.org>
Subject: [PATCH] fs: pipe/sockets/anon dentries should not have a parent
Date: Fri, 21 Nov 2008 16:13:38 +0100	[thread overview]
Message-ID: <4926D022.5060008@cosmosbay.com> (raw)
In-Reply-To: <4926AEDB.10007-fPLkHRcR87vqlBn2x/YWAg@public.gmane.org>

[-- Attachment #1: Type: text/plain, Size: 5720 bytes --]

Eric Dumazet a écrit :
> David Miller a écrit :
>> From: Eric Dumazet <dada1-fPLkHRcR87vqlBn2x/YWAg@public•gmane.org>
>> Date: Fri, 21 Nov 2008 09:51:32 +0100
>>
>>> Now, I wish sockets and pipes not going through dcache, not tbench 
>>> affair
>>> of course but real workloads...
>>>
>>> running 8 processes on a 8 way machine doing a
>>> for (;;)
>>>     close(socket(AF_INET, SOCK_STREAM, 0));
>>>
>>> is slow as hell, we hit so many contended cache lines ...
>>>
>>> ticket spin locks are slower in this case (dcache_lock for example
>>> is taken twice when we allocate a socket(), once in d_alloc(), 
>>> another one
>>> in d_instantiate())
>>
>> As you of course know, this used to be a ton worse.  At least now
>> these things are unhashed. :)
> 
> Well, this is dust compared to what we currently have.
> 
> To allocate a socket we :
> 0) Do the usual file manipulation (pretty scalable these days)
>   (but recent drop_file_write_access() and co slow down a bit)
> 1) allocate an inode with new_inode()
>    This function :
>     - locks inode_lock,
>     - dirties nr_inodes counter
>     - dirties inode_in_use list  (for sockets, I doubt it is usefull)
>     - dirties superblock s_inodes.
>     - dirties last_ino counter
> All these are in different cache lines of course.
> 2) allocate a dentry
>   d_alloc() takes dcache_lock,
>   insert dentry on its parent list (dirtying sock_mnt->mnt_sb->s_root)
>   dirties nr_dentry
> 3) d_instantiate() dentry  (dcache_lock taken again)
> 4) init_file() -> atomic_inc on sock_mnt->refcount (in case we want to 
> umount this vfs ...)
> 
> 
> 
> At close() time, we must undo the things. Its even more expensive because
> of the _atomic_dec_and_lock() that stress a lot, and because of two 
> cache lines that are touched when an element is deleted from a list.
> 
> for (i = 0; i < 1000*1000; i++)
>     close(socket(socket(AF_INET, SOCK_STREAM, 0));
> 
> Cost if run one one cpu :
> 
> real    0m1.561s
> user    0m0.092s
> sys     0m1.469s
> 
> If run on 8 CPUS :
> 
> real    0m27.496s
> user    0m0.657s
> sys     3m39.092s
> 
> 

[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.

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

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

After patch :

real    0m23.997s
user    0m0.682s
sys     3m11.193s


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.24 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
147287   147287        10.3984  10.3984    new_inode
144884   292171        10.2287  20.6271    inet_create
93670    385841         6.6131  27.2402    init_file
89852    475693         6.3435  33.5837    wake_up_inode
80910    556603         5.7122  39.2959    kmem_cache_alloc
53588    610191         3.7833  43.0792    _atomic_dec_and_lock
44341    654532         3.1305  46.2096    generic_forget_inode
38710    693242         2.7329  48.9425    kmem_cache_free
37605    730847         2.6549  51.5974    tcp_v4_init_sock
37228    768075         2.6283  54.2257    d_alloc
34085    802160         2.4064  56.6321    tcp_close
32550    834710         2.2980  58.9301    sysenter_past_esp
25931    860641         1.8307  60.7608    vfs_dq_drop
24458    885099         1.7267  62.4875    d_kill
22015    907114         1.5542  64.0418    dentry_iput
18877    925991         1.3327  65.3745    __copy_from_user_ll
17873    943864         1.2618  66.6363    mwait_idle

Signed-off-by: Eric Dumazet <dada1-fPLkHRcR87vqlBn2x/YWAg@public•gmane.org>
---
 fs/anon_inodes.c |    2 +-
 fs/dnotify.c     |    2 +-
 fs/inotify.c     |    2 +-
 fs/pipe.c        |    2 +-
 net/socket.c     |    2 +-
 5 files changed, 5 insertions(+), 5 deletions(-)

[-- Attachment #2: null_parent.patch --]
[-- Type: text/plain, Size: 2076 bytes --]

diff --git a/fs/anon_inodes.c b/fs/anon_inodes.c
index 3662dd4..22cce87 100644
--- a/fs/anon_inodes.c
+++ b/fs/anon_inodes.c
@@ -92,7 +92,7 @@ int anon_inode_getfd(const char *name, const struct file_operations *fops,
 	this.name = name;
 	this.len = strlen(name);
 	this.hash = 0;
-	dentry = d_alloc(anon_inode_mnt->mnt_sb->s_root, &this);
+	dentry = d_alloc(NULL, &this);
 	if (!dentry)
 		goto err_put_unused_fd;
 
diff --git a/fs/dnotify.c b/fs/dnotify.c
index 676073b..66066a3 100644
--- a/fs/dnotify.c
+++ b/fs/dnotify.c
@@ -173,7 +173,7 @@ void dnotify_parent(struct dentry *dentry, unsigned long event)
 
 	spin_lock(&dentry->d_lock);
 	parent = dentry->d_parent;
-	if (parent->d_inode->i_dnotify_mask & event) {
+	if (parent && parent->d_inode->i_dnotify_mask & event) {
 		dget(parent);
 		spin_unlock(&dentry->d_lock);
 		__inode_dir_notify(parent->d_inode, event);
diff --git a/fs/inotify.c b/fs/inotify.c
index 7bbed1b..9f051bb 100644
--- a/fs/inotify.c
+++ b/fs/inotify.c
@@ -270,7 +270,7 @@ void inotify_d_instantiate(struct dentry *entry, struct inode *inode)
 
 	spin_lock(&entry->d_lock);
 	parent = entry->d_parent;
-	if (parent->d_inode && inotify_inode_watched(parent->d_inode))
+	if (parent && parent->d_inode && inotify_inode_watched(parent->d_inode))
 		entry->d_flags |= DCACHE_INOTIFY_PARENT_WATCHED;
 	spin_unlock(&entry->d_lock);
 }
diff --git a/fs/pipe.c b/fs/pipe.c
index 7aea8b8..4b961bc 100644
--- a/fs/pipe.c
+++ b/fs/pipe.c
@@ -926,7 +926,7 @@ struct file *create_write_pipe(int flags)
 		goto err;
 
 	err = -ENOMEM;
-	dentry = d_alloc(pipe_mnt->mnt_sb->s_root, &name);
+	dentry = d_alloc(NULL, &name);
 	if (!dentry)
 		goto err_inode;
 
diff --git a/net/socket.c b/net/socket.c
index e9d65ea..b84de7d 100644
--- a/net/socket.c
+++ b/net/socket.c
@@ -373,7 +373,7 @@ static int sock_attach_fd(struct socket *sock, struct file *file, int flags)
 	struct dentry *dentry;
 	struct qstr name = { .name = "" };
 
-	dentry = d_alloc(sock_mnt->mnt_sb->s_root, &name);
+	dentry = d_alloc(NULL, &name);
 	if (unlikely(!dentry))
 		return -ENOMEM;
 

       reply	other threads:[~2008-11-21 15:13 UTC|newest]

Thread overview: 75+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <Pine.LNX.4.64.0811201727070.9089@quilx.com>
     [not found] ` <20081121083044.GL16242@elte.hu>
     [not found]   ` <49267694.1030506@cosmosbay.com>
     [not found]     ` <20081121.010508.40225532.davem@davemloft.net>
     [not found]       ` <4926AEDB.10007@cosmosbay.com>
     [not found]         ` <4926AEDB.10007-fPLkHRcR87vqlBn2x/YWAg@public.gmane.org>
2008-11-21 15:13           ` Eric Dumazet [this message]
     [not found]             ` <4926D022.5060008-fPLkHRcR87vqlBn2x/YWAg@public.gmane.org>
2008-11-21 15:21               ` [PATCH] fs: pipe/sockets/anon dentries should not have a parent Ingo Molnar
     [not found]                 ` <20081121152148.GA20388-X9Un+BFzKDI@public.gmane.org>
2008-11-21 15:28                   ` Eric Dumazet
     [not found]                     ` <4926D39D.9050603-fPLkHRcR87vqlBn2x/YWAg@public.gmane.org>
2008-11-21 15:34                       ` Ingo Molnar
2008-11-26 23:27                         ` [PATCH 0/6] fs: Scalability of sockets/pipes allocation/deallocation on SMP Eric Dumazet
2008-11-27  9:39                           ` Christoph Hellwig
2008-11-28 18:03                           ` Ingo Molnar
     [not found]                             ` <20081128180318.GL10487-X9Un+BFzKDI@public.gmane.org>
2008-11-28 18:47                               ` Peter Zijlstra
2008-11-29  6:38                                 ` Christoph Hellwig
     [not found]                                   ` <20081129063816.GA869-wEGCiKHe2LqWVfeAwA7xHQ@public.gmane.org>
2008-11-29  8:07                                     ` Eric Dumazet
2008-11-29  8:43                           ` [PATCH v2 0/5] " Eric Dumazet
2008-12-11 22:38                             ` [PATCH v3 0/7] " Eric Dumazet
2008-12-11 22:38                             ` [PATCH v3 1/7] fs: Use a percpu_counter to track nr_dentry Eric Dumazet
2007-07-24  1:24                               ` Nick Piggin
     [not found]                               ` <49419680.8010409-fPLkHRcR87vqlBn2x/YWAg@public.gmane.org>
2008-12-16 21:04                                 ` Paul E. McKenney
2008-12-11 22:39                             ` [PATCH v3 2/7] fs: Use a percpu_counter to track nr_inodes Eric Dumazet
     [not found]                               ` <4941968E.3020201-fPLkHRcR87vqlBn2x/YWAg@public.gmane.org>
2007-07-24  1:30                                 ` Nick Piggin
     [not found]                                   ` <200707241130.56767.nickpiggin-/E1597aS9LT0CCvOHzKKcA@public.gmane.org>
2008-12-12  5:11                                     ` Eric Dumazet
2008-12-16 21:10                                 ` Paul E. McKenney
2008-12-11 22:39                             ` [PATCH v3 3/7] fs: Introduce a per_cpu last_ino allocator Eric Dumazet
2007-07-24  1:34                               ` Nick Piggin
2008-12-16 21:26                               ` Paul E. McKenney
2008-12-11 22:39                             ` [PATCH v3 4/7] fs: Introduce SINGLE dentries for pipes, socket, anon fd Eric Dumazet
     [not found]                               ` <494196AA.6080002-fPLkHRcR87vqlBn2x/YWAg@public.gmane.org>
2008-12-16 21:40                                 ` Paul E. McKenney
2008-12-11 22:40                             ` [PATCH v3 5/7] fs: new_inode_single() and iput_single() Eric Dumazet
2008-12-16 21:41                               ` Paul E. McKenney
     [not found]                             ` <493100B0.6090104-fPLkHRcR87vqlBn2x/YWAg@public.gmane.org>
2008-12-11 22:40                               ` [PATCH v3 6/7] fs: struct file move from call_rcu() to SLAB_DESTROY_BY_RCU Eric Dumazet
2007-07-24  1:13                                 ` Nick Piggin
2008-12-12  2:50                                   ` Nick Piggin
2008-12-12  4:45                                   ` Eric Dumazet
     [not found]                                     ` <4941EC65.5040903-fPLkHRcR87vqlBn2x/YWAg@public.gmane.org>
2008-12-12 16:48                                       ` Eric Dumazet
     [not found]                                         ` <494295C6.2020906-fPLkHRcR87vqlBn2x/YWAg@public.gmane.org>
2008-12-13  2:07                                           ` Christoph Lameter
     [not found]                                             ` <Pine.LNX.4.64.0812121958470.15781-dRBSpnHQED8AvxtiuMwx3w@public.gmane.org>
2008-12-17 20:25                                               ` Eric Dumazet
2008-12-13  1:41                                       ` Christoph Lameter
2008-12-11 22:41                             ` [PATCH v3 7/7] fs: MS_NOREFCOUNT Eric Dumazet
     [not found]                           ` <492DDB6A.8090806-fPLkHRcR87vqlBn2x/YWAg@public.gmane.org>
2008-11-27  1:37                             ` [PATCH 0/6] fs: Scalability of sockets/pipes allocation/deallocation on SMP Christoph Lameter
     [not found]                               ` <Pine.LNX.4.64.0811261935330.31159-dRBSpnHQED8AvxtiuMwx3w@public.gmane.org>
2008-11-27  6:27                                 ` Eric Dumazet
     [not found]                                   ` <492E3DEF.8030602-fPLkHRcR87vqlBn2x/YWAg@public.gmane.org>
2008-11-27 14:44                                     ` Christoph Lameter
2008-11-29  8:43                             ` [PATCH v2 1/5] fs: Use a percpu_counter to track nr_dentry Eric Dumazet
2008-11-29  8:43                             ` [PATCH v2 2/5] fs: Use a percpu_counter to track nr_inodes Eric Dumazet
2008-11-29  8:44                             ` [PATCH v2 4/5] fs: Introduce SINGLE dentries for pipes, socket, anon fd Eric Dumazet
     [not found]                               ` <493100E7.3030907-fPLkHRcR87vqlBn2x/YWAg@public.gmane.org>
2008-11-29 10:38                                 ` Jörn Engel
     [not found]                                   ` <20081129103836.GA11959-PCqxUs/MD9bYtjvyW6yDsg@public.gmane.org>
2008-11-29 11:14                                     ` Eric Dumazet
2008-11-29  8:45                             ` [PATCH v2 5/5] fs: new_inode_single() and iput_single() Eric Dumazet
2008-11-29 11:14                               ` Jörn Engel
2008-11-29  8:44                           ` [PATCH v2 3/5] fs: Introduce a per_cpu last_ino allocator Eric Dumazet
2008-11-26 23:32                         ` [PATCH 3/6] " Eric Dumazet
     [not found]                           ` <492DDC88.2050305-fPLkHRcR87vqlBn2x/YWAg@public.gmane.org>
2008-11-27  9:46                             ` Christoph Hellwig
     [not found]                         ` <20081121153453.GA23713-X9Un+BFzKDI@public.gmane.org>
2008-11-26 23:30                           ` [PATCH 1/6] fs: Introduce a per_cpu nr_dentry Eric Dumazet
     [not found]                             ` <492DDC0B.8060804-fPLkHRcR87vqlBn2x/YWAg@public.gmane.org>
2008-11-27  9:41                               ` Christoph Hellwig
2008-11-26 23:32                           ` [PATCH 4/6] fs: Introduce a per_cpu nr_inodes Eric Dumazet
2008-11-27  9:32                             ` Peter Zijlstra
2008-11-27  9:39                               ` Peter Zijlstra
2008-11-27  9:48                                 ` Christoph Hellwig
2008-11-27 10:01                               ` Eric Dumazet
2008-11-27 10:07                               ` Andi Kleen
2008-11-27 14:46                               ` Christoph Lameter
2008-11-26 23:32                           ` [PATCH 5/6] fs: Introduce special inodes Eric Dumazet
     [not found]                             ` <492DDC99.5060106-fPLkHRcR87vqlBn2x/YWAg@public.gmane.org>
2008-11-27  8:20                               ` David Miller
2008-11-26 23:32                         ` [PATCH 6/6] fs: Introduce kern_mount_special() to mount special vfs Eric Dumazet
     [not found]                           ` <492DDCAB.1070204-fPLkHRcR87vqlBn2x/YWAg@public.gmane.org>
2008-11-27  8:21                             ` David Miller
2008-11-28  9:26                             ` Al Viro
     [not found]                               ` <20081128092604.GL28946-3bDd1+5oDREiFSDQTTA3OLVCufUGDwFn@public.gmane.org>
2008-11-28  9:34                                 ` Al Viro
2008-11-28 18:02                                 ` Ingo Molnar
2008-11-28 18:58                                   ` Ingo Molnar
     [not found]                                   ` <20081128180220.GK10487-X9Un+BFzKDI@public.gmane.org>
2008-11-28 22:20                                     ` Eric Dumazet
2008-11-28 22:37                               ` Eric Dumazet
2008-11-28 22:43                                 ` Eric Dumazet
2008-11-27  9:53                           ` Christoph Hellwig
     [not found]                             ` <20081127095321.GE13860-wEGCiKHe2LqWVfeAwA7xHQ@public.gmane.org>
2008-11-27 10:04                               ` Eric Dumazet
     [not found]                                 ` <492E70B6.70108-fPLkHRcR87vqlBn2x/YWAg@public.gmane.org>
2008-11-27 10:10                                   ` Christoph Hellwig
2008-11-21 15:36             ` [PATCH] fs: pipe/sockets/anon dentries should not have a parent Christoph Hellwig
2008-11-21 17:58               ` [PATCH] fs: pipe/sockets/anon dentries should have themselves as parent Eric Dumazet
     [not found]                 ` <4926F6C5.9030108-fPLkHRcR87vqlBn2x/YWAg@public.gmane.org>
2008-11-21 18:43                   ` Matthew Wilcox
2008-11-23  3:53                     ` Eric Dumazet

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=4926D022.5060008@cosmosbay.com \
    --to=dada1-fplkhrcr87vqlbn2x/ywag@public$(echo .)gmane.org \
    --cc=a.p.zijlstra-/NLkJaSkS4VmR6Xm/wNWPw@public$(echo .)gmane.org \
    --cc=cl-de/tnXTf+JLsfHDXvbKv3WD2FQJk+8+b@public$(echo .)gmane.org \
    --cc=davem-fT/PcQaiUtIeIZ0/mPfg9Q@public$(echo .)gmane.org \
    --cc=efault-Mmb7MZpHnFY@public$(echo .)gmane.org \
    --cc=kernel-testers-u79uwXL29TY76Z2rM5mHXA@public$(echo .)gmane.org \
    --cc=linux-kernel-u79uwXL29TY76Z2rM5mHXA@public$(echo .)gmane.org \
    --cc=mingo-X9Un+BFzKDI@public$(echo .)gmane.org \
    --cc=netdev-u79uwXL29TY76Z2rM5mHXA@public$(echo .)gmane.org \
    --cc=rjw-KKrjLPT3xs0@public$(echo .)gmane.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox