lkml.org 
[lkml]   [2008]   [Nov]   [21]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    Patch in this message
    /
    Date
    From
    Subject[PATCH] fs: pipe/sockets/anon dentries should not have a parent
    Eric Dumazet a écrit :
    > David Miller a écrit :
    >> From: Eric Dumazet <dada1@cosmosbay.com>
    >> 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@cosmosbay.com>
    ---
    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(-)
    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;
    \
     
     \ /
      Last update: 2008-11-21 16:17    [W:5.782 / U:0.056 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site