| Message ID | 494196AA.6080002@cosmosbay.com |
|---|---|
| State | Not Applicable, archived |
| Delegated to: | David Miller |
| Headers | show |
On Thu, Dec 11, 2008 at 11:39:38PM +0100, Eric Dumazet wrote: > Sockets, pipes and anonymous fds have interesting properties. > > Like other files, they use a dentry and an inode. > > But dentries for these kind of files are not hashed into dcache, > since there is no way someone can lookup such a file in the vfs tree. > (/proc/{pid}/fd/{number} uses a different mechanism) > > Still, allocating and freeing such dentries are expensive processes, > because we currently take dcache_lock inside d_alloc(), d_instantiate(), > and dput(). This lock is very contended on SMP machines. > > This patch defines a new DCACHE_SINGLE flag, to mark a dentry as > a single one (for sockets, pipes, anonymous fd), and a new > d_alloc_single(const struct qstr *name, struct inode *inode) > method, called by the three subsystems. > > Internally, dput() can take a fast path to dput_single() for > SINGLE dentries. No more atomic_dec_and_lock() > for such dentries. > > > Differences betwen an SINGLE dentry and a normal one are : > > 1) SINGLE dentry has the DCACHE_SINGLE flag > 2) SINGLE dentry's parent is itself (DCACHE_DISCONNECTED) > This to avoid taking a reference on sb 'root' dentry, shared > by too many dentries. > 3) They are not hashed into global hash table (DCACHE_UNHASHED) > 4) Their d_alias list is empty > > ("socketallocbench -n 8" bench result : from 25s to 19.9s) Acked-by: Paul E. McKenney <paulmck@linux.vnet.ibm.com> > Signed-off-by: Eric Dumazet <dada1@cosmosbay.com> > --- > fs/anon_inodes.c | 16 ------------ > fs/dcache.c | 51 +++++++++++++++++++++++++++++++++++++++ > fs/pipe.c | 23 +---------------- > include/linux/dcache.h | 9 ++++++ > net/socket.c | 24 +----------------- > 5 files changed, 65 insertions(+), 58 deletions(-) > > diff --git a/fs/anon_inodes.c b/fs/anon_inodes.c > index 3662dd4..8bf83cb 100644 > --- a/fs/anon_inodes.c > +++ b/fs/anon_inodes.c > @@ -33,23 +33,12 @@ static int anon_inodefs_get_sb(struct file_system_type *fs_type, int flags, > mnt); > } > > -static int anon_inodefs_delete_dentry(struct dentry *dentry) > -{ > - /* > - * We faked vfs to believe the dentry was hashed when we created it. > - * Now we restore the flag so that dput() will work correctly. > - */ > - dentry->d_flags |= DCACHE_UNHASHED; > - return 1; > -} > - > static struct file_system_type anon_inode_fs_type = { > .name = "anon_inodefs", > .get_sb = anon_inodefs_get_sb, > .kill_sb = kill_anon_super, > }; > static struct dentry_operations anon_inodefs_dentry_operations = { > - .d_delete = anon_inodefs_delete_dentry, > }; > > /** > @@ -92,7 +81,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_single(&this, anon_inode_inode); > if (!dentry) > goto err_put_unused_fd; > > @@ -104,9 +93,6 @@ int anon_inode_getfd(const char *name, const struct file_operations *fops, > atomic_inc(&anon_inode_inode->i_count); > > dentry->d_op = &anon_inodefs_dentry_operations; > - /* Do not publish this dentry inside the global dentry hash table */ > - dentry->d_flags &= ~DCACHE_UNHASHED; > - d_instantiate(dentry, anon_inode_inode); > > error = -ENFILE; > file = alloc_file(anon_inode_mnt, dentry, > diff --git a/fs/dcache.c b/fs/dcache.c > index f463a81..af3bfb3 100644 > --- a/fs/dcache.c > +++ b/fs/dcache.c > @@ -219,6 +219,23 @@ static struct dentry *d_kill(struct dentry *dentry) > */ > > /* > + * special version of dput() for pipes/sockets/anon. > + * These dentries are not present in hash table, we can avoid > + * taking/dirtying dcache_lock > + */ > +static void dput_single(struct dentry *dentry) > +{ > + struct inode *inode; > + > + if (!atomic_dec_and_test(&dentry->d_count)) > + return; > + inode = dentry->d_inode; > + if (inode) > + iput(inode); > + d_free(dentry); > +} > + > +/* > * dput - release a dentry > * @dentry: dentry to release > * > @@ -234,6 +251,11 @@ void dput(struct dentry *dentry) > { > if (!dentry) > return; > + /* > + * single dentries (sockets/pipes/anon) fast path > + */ > + if (dentry->d_flags & DCACHE_SINGLE) > + return dput_single(dentry); > > repeat: > if (atomic_read(&dentry->d_count) == 1) > @@ -1119,6 +1141,35 @@ struct dentry * d_alloc_root(struct inode * root_inode) > return res; > } > > +/** > + * d_alloc_single - allocate SINGLE dentry > + * @name: dentry name, given in a qstr structure > + * @inode: inode to allocate the dentry for > + * > + * Allocate an SINGLE dentry for the inode given. The inode is > + * instantiated and returned. %NULL is returned if there is insufficient > + * memory. > + * - SINGLE dentries have themselves as a parent. > + * - SINGLE dentries are not hashed into global hash table > + * - their d_alias list is empty > + */ > +struct dentry *d_alloc_single(const struct qstr *name, struct inode *inode) > +{ > + struct dentry *entry; > + > + entry = d_alloc(NULL, name); > + if (entry) { > + entry->d_sb = inode->i_sb; > + entry->d_parent = entry; > + entry->d_flags |= DCACHE_SINGLE | DCACHE_DISCONNECTED; > + entry->d_inode = inode; > + fsnotify_d_instantiate(entry, inode); > + security_d_instantiate(entry, inode); > + } > + return entry; > +} > + > + > static inline struct hlist_head *d_hash(struct dentry *parent, > unsigned long hash) > { > diff --git a/fs/pipe.c b/fs/pipe.c > index 7aea8b8..4de6dd5 100644 > --- a/fs/pipe.c > +++ b/fs/pipe.c > @@ -849,17 +849,6 @@ void free_pipe_info(struct inode *inode) > } > > static struct vfsmount *pipe_mnt __read_mostly; > -static int pipefs_delete_dentry(struct dentry *dentry) > -{ > - /* > - * At creation time, we pretended this dentry was hashed > - * (by clearing DCACHE_UNHASHED bit in d_flags) > - * At delete time, we restore the truth : not hashed. > - * (so that dput() can proceed correctly) > - */ > - dentry->d_flags |= DCACHE_UNHASHED; > - return 0; > -} > > /* > * pipefs_dname() is called from d_path(). > @@ -871,7 +860,6 @@ static char *pipefs_dname(struct dentry *dentry, char *buffer, int buflen) > } > > static struct dentry_operations pipefs_dentry_operations = { > - .d_delete = pipefs_delete_dentry, > .d_dname = pipefs_dname, > }; > > @@ -918,7 +906,7 @@ struct file *create_write_pipe(int flags) > struct inode *inode; > struct file *f; > struct dentry *dentry; > - struct qstr name = { .name = "" }; > + static const struct qstr name = { .name = "" }; > > err = -ENFILE; > inode = get_pipe_inode(); > @@ -926,18 +914,11 @@ struct file *create_write_pipe(int flags) > goto err; > > err = -ENOMEM; > - dentry = d_alloc(pipe_mnt->mnt_sb->s_root, &name); > + dentry = d_alloc_single(&name, inode); > if (!dentry) > goto err_inode; > > dentry->d_op = &pipefs_dentry_operations; > - /* > - * We dont want to publish this dentry into global dentry hash table. > - * We pretend dentry is already hashed, by unsetting DCACHE_UNHASHED > - * This permits a working /proc/$pid/fd/XXX on pipes > - */ > - dentry->d_flags &= ~DCACHE_UNHASHED; > - d_instantiate(dentry, inode); > > err = -ENFILE; > f = alloc_file(pipe_mnt, dentry, FMODE_WRITE, &write_pipefifo_fops); > diff --git a/include/linux/dcache.h b/include/linux/dcache.h > index a37359d..ca8d269 100644 > --- a/include/linux/dcache.h > +++ b/include/linux/dcache.h > @@ -176,6 +176,14 @@ d_iput: no no no yes > #define DCACHE_UNHASHED 0x0010 > > #define DCACHE_INOTIFY_PARENT_WATCHED 0x0020 /* Parent inode is watched */ > +#define DCACHE_SINGLE 0x0040 > + /* > + * socket, pipe or anonymous fd dentry > + * - SINGLE dentries have themselves as a parent. > + * - SINGLE dentries are not hashed into global hash table > + * - Their d_alias list is empty > + * - They dont need dcache_lock synchronization > + */ > > extern spinlock_t dcache_lock; > extern seqlock_t rename_lock; > @@ -235,6 +243,7 @@ extern void shrink_dcache_sb(struct super_block *); > extern void shrink_dcache_parent(struct dentry *); > extern void shrink_dcache_for_umount(struct super_block *); > extern int d_invalidate(struct dentry *); > +extern struct dentry *d_alloc_single(const struct qstr *, struct inode *); > > /* only used at mount-time */ > extern struct dentry * d_alloc_root(struct inode *); > diff --git a/net/socket.c b/net/socket.c > index 92764d8..353c928 100644 > --- a/net/socket.c > +++ b/net/socket.c > @@ -308,18 +308,6 @@ static struct file_system_type sock_fs_type = { > .kill_sb = kill_anon_super, > }; > > -static int sockfs_delete_dentry(struct dentry *dentry) > -{ > - /* > - * At creation time, we pretended this dentry was hashed > - * (by clearing DCACHE_UNHASHED bit in d_flags) > - * At delete time, we restore the truth : not hashed. > - * (so that dput() can proceed correctly) > - */ > - dentry->d_flags |= DCACHE_UNHASHED; > - return 0; > -} > - > /* > * sockfs_dname() is called from d_path(). > */ > @@ -330,7 +318,6 @@ static char *sockfs_dname(struct dentry *dentry, char *buffer, int buflen) > } > > static struct dentry_operations sockfs_dentry_operations = { > - .d_delete = sockfs_delete_dentry, > .d_dname = sockfs_dname, > }; > > @@ -372,20 +359,13 @@ static int sock_alloc_fd(struct file **filep, int flags) > static int sock_attach_fd(struct socket *sock, struct file *file, int flags) > { > struct dentry *dentry; > - struct qstr name = { .name = "" }; > + static const struct qstr name = { .name = "" }; > > - dentry = d_alloc(sock_mnt->mnt_sb->s_root, &name); > + dentry = d_alloc_single(&name, SOCK_INODE(sock)); > if (unlikely(!dentry)) > return -ENOMEM; > > dentry->d_op = &sockfs_dentry_operations; > - /* > - * We dont want to push this dentry into global dentry hash table. > - * We pretend dentry is already hashed, by unsetting DCACHE_UNHASHED > - * This permits a working /proc/$pid/fd/XXX on sockets > - */ > - dentry->d_flags &= ~DCACHE_UNHASHED; > - d_instantiate(dentry, SOCK_INODE(sock)); > > sock->file = file; > init_file(file, sock_mnt, dentry, FMODE_READ | FMODE_WRITE, > -- > To unsubscribe from this list: send the line "unsubscribe netdev" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/fs/anon_inodes.c b/fs/anon_inodes.c index 3662dd4..8bf83cb 100644 --- a/fs/anon_inodes.c +++ b/fs/anon_inodes.c @@ -33,23 +33,12 @@ static int anon_inodefs_get_sb(struct file_system_type *fs_type, int flags, mnt); } -static int anon_inodefs_delete_dentry(struct dentry *dentry) -{ - /* - * We faked vfs to believe the dentry was hashed when we created it. - * Now we restore the flag so that dput() will work correctly. - */ - dentry->d_flags |= DCACHE_UNHASHED; - return 1; -} - static struct file_system_type anon_inode_fs_type = { .name = "anon_inodefs", .get_sb = anon_inodefs_get_sb, .kill_sb = kill_anon_super, }; static struct dentry_operations anon_inodefs_dentry_operations = { - .d_delete = anon_inodefs_delete_dentry, }; /** @@ -92,7 +81,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_single(&this, anon_inode_inode); if (!dentry) goto err_put_unused_fd; @@ -104,9 +93,6 @@ int anon_inode_getfd(const char *name, const struct file_operations *fops, atomic_inc(&anon_inode_inode->i_count); dentry->d_op = &anon_inodefs_dentry_operations; - /* Do not publish this dentry inside the global dentry hash table */ - dentry->d_flags &= ~DCACHE_UNHASHED; - d_instantiate(dentry, anon_inode_inode); error = -ENFILE; file = alloc_file(anon_inode_mnt, dentry, diff --git a/fs/dcache.c b/fs/dcache.c index f463a81..af3bfb3 100644 --- a/fs/dcache.c +++ b/fs/dcache.c @@ -219,6 +219,23 @@ static struct dentry *d_kill(struct dentry *dentry) */ /* + * special version of dput() for pipes/sockets/anon. + * These dentries are not present in hash table, we can avoid + * taking/dirtying dcache_lock + */ +static void dput_single(struct dentry *dentry) +{ + struct inode *inode; + + if (!atomic_dec_and_test(&dentry->d_count)) + return; + inode = dentry->d_inode; + if (inode) + iput(inode); + d_free(dentry); +} + +/* * dput - release a dentry * @dentry: dentry to release * @@ -234,6 +251,11 @@ void dput(struct dentry *dentry) { if (!dentry) return; + /* + * single dentries (sockets/pipes/anon) fast path + */ + if (dentry->d_flags & DCACHE_SINGLE) + return dput_single(dentry); repeat: if (atomic_read(&dentry->d_count) == 1) @@ -1119,6 +1141,35 @@ struct dentry * d_alloc_root(struct inode * root_inode) return res; } +/** + * d_alloc_single - allocate SINGLE dentry + * @name: dentry name, given in a qstr structure + * @inode: inode to allocate the dentry for + * + * Allocate an SINGLE dentry for the inode given. The inode is + * instantiated and returned. %NULL is returned if there is insufficient + * memory. + * - SINGLE dentries have themselves as a parent. + * - SINGLE dentries are not hashed into global hash table + * - their d_alias list is empty + */ +struct dentry *d_alloc_single(const struct qstr *name, struct inode *inode) +{ + struct dentry *entry; + + entry = d_alloc(NULL, name); + if (entry) { + entry->d_sb = inode->i_sb; + entry->d_parent = entry; + entry->d_flags |= DCACHE_SINGLE | DCACHE_DISCONNECTED; + entry->d_inode = inode; + fsnotify_d_instantiate(entry, inode); + security_d_instantiate(entry, inode); + } + return entry; +} + + static inline struct hlist_head *d_hash(struct dentry *parent, unsigned long hash) { diff --git a/fs/pipe.c b/fs/pipe.c index 7aea8b8..4de6dd5 100644 --- a/fs/pipe.c +++ b/fs/pipe.c @@ -849,17 +849,6 @@ void free_pipe_info(struct inode *inode) } static struct vfsmount *pipe_mnt __read_mostly; -static int pipefs_delete_dentry(struct dentry *dentry) -{ - /* - * At creation time, we pretended this dentry was hashed - * (by clearing DCACHE_UNHASHED bit in d_flags) - * At delete time, we restore the truth : not hashed. - * (so that dput() can proceed correctly) - */ - dentry->d_flags |= DCACHE_UNHASHED; - return 0; -} /* * pipefs_dname() is called from d_path(). @@ -871,7 +860,6 @@ static char *pipefs_dname(struct dentry *dentry, char *buffer, int buflen) } static struct dentry_operations pipefs_dentry_operations = { - .d_delete = pipefs_delete_dentry, .d_dname = pipefs_dname, }; @@ -918,7 +906,7 @@ struct file *create_write_pipe(int flags) struct inode *inode; struct file *f; struct dentry *dentry; - struct qstr name = { .name = "" }; + static const struct qstr name = { .name = "" }; err = -ENFILE; inode = get_pipe_inode(); @@ -926,18 +914,11 @@ struct file *create_write_pipe(int flags) goto err; err = -ENOMEM; - dentry = d_alloc(pipe_mnt->mnt_sb->s_root, &name); + dentry = d_alloc_single(&name, inode); if (!dentry) goto err_inode; dentry->d_op = &pipefs_dentry_operations; - /* - * We dont want to publish this dentry into global dentry hash table. - * We pretend dentry is already hashed, by unsetting DCACHE_UNHASHED - * This permits a working /proc/$pid/fd/XXX on pipes - */ - dentry->d_flags &= ~DCACHE_UNHASHED; - d_instantiate(dentry, inode); err = -ENFILE; f = alloc_file(pipe_mnt, dentry, FMODE_WRITE, &write_pipefifo_fops); diff --git a/include/linux/dcache.h b/include/linux/dcache.h index a37359d..ca8d269 100644 --- a/include/linux/dcache.h +++ b/include/linux/dcache.h @@ -176,6 +176,14 @@ d_iput: no no no yes #define DCACHE_UNHASHED 0x0010 #define DCACHE_INOTIFY_PARENT_WATCHED 0x0020 /* Parent inode is watched */ +#define DCACHE_SINGLE 0x0040 + /* + * socket, pipe or anonymous fd dentry + * - SINGLE dentries have themselves as a parent. + * - SINGLE dentries are not hashed into global hash table + * - Their d_alias list is empty + * - They dont need dcache_lock synchronization + */ extern spinlock_t dcache_lock; extern seqlock_t rename_lock; @@ -235,6 +243,7 @@ extern void shrink_dcache_sb(struct super_block *); extern void shrink_dcache_parent(struct dentry *); extern void shrink_dcache_for_umount(struct super_block *); extern int d_invalidate(struct dentry *); +extern struct dentry *d_alloc_single(const struct qstr *, struct inode *); /* only used at mount-time */ extern struct dentry * d_alloc_root(struct inode *); diff --git a/net/socket.c b/net/socket.c index 92764d8..353c928 100644 --- a/net/socket.c +++ b/net/socket.c @@ -308,18 +308,6 @@ static struct file_system_type sock_fs_type = { .kill_sb = kill_anon_super, }; -static int sockfs_delete_dentry(struct dentry *dentry) -{ - /* - * At creation time, we pretended this dentry was hashed - * (by clearing DCACHE_UNHASHED bit in d_flags) - * At delete time, we restore the truth : not hashed. - * (so that dput() can proceed correctly) - */ - dentry->d_flags |= DCACHE_UNHASHED; - return 0; -} - /* * sockfs_dname() is called from d_path(). */ @@ -330,7 +318,6 @@ static char *sockfs_dname(struct dentry *dentry, char *buffer, int buflen) } static struct dentry_operations sockfs_dentry_operations = { - .d_delete = sockfs_delete_dentry, .d_dname = sockfs_dname, }; @@ -372,20 +359,13 @@ static int sock_alloc_fd(struct file **filep, int flags) static int sock_attach_fd(struct socket *sock, struct file *file, int flags) { struct dentry *dentry; - struct qstr name = { .name = "" }; + static const struct qstr name = { .name = "" }; - dentry = d_alloc(sock_mnt->mnt_sb->s_root, &name); + dentry = d_alloc_single(&name, SOCK_INODE(sock)); if (unlikely(!dentry)) return -ENOMEM; dentry->d_op = &sockfs_dentry_operations; - /* - * We dont want to push this dentry into global dentry hash table. - * We pretend dentry is already hashed, by unsetting DCACHE_UNHASHED - * This permits a working /proc/$pid/fd/XXX on sockets - */ - dentry->d_flags &= ~DCACHE_UNHASHED; - d_instantiate(dentry, SOCK_INODE(sock)); sock->file = file; init_file(file, sock_mnt, dentry, FMODE_READ | FMODE_WRITE,
Sockets, pipes and anonymous fds have interesting properties. Like other files, they use a dentry and an inode. But dentries for these kind of files are not hashed into dcache, since there is no way someone can lookup such a file in the vfs tree. (/proc/{pid}/fd/{number} uses a different mechanism) Still, allocating and freeing such dentries are expensive processes, because we currently take dcache_lock inside d_alloc(), d_instantiate(), and dput(). This lock is very contended on SMP machines. This patch defines a new DCACHE_SINGLE flag, to mark a dentry as a single one (for sockets, pipes, anonymous fd), and a new d_alloc_single(const struct qstr *name, struct inode *inode) method, called by the three subsystems. Internally, dput() can take a fast path to dput_single() for SINGLE dentries. No more atomic_dec_and_lock() for such dentries. Differences betwen an SINGLE dentry and a normal one are : 1) SINGLE dentry has the DCACHE_SINGLE flag 2) SINGLE dentry's parent is itself (DCACHE_DISCONNECTED) This to avoid taking a reference on sb 'root' dentry, shared by too many dentries. 3) They are not hashed into global hash table (DCACHE_UNHASHED) 4) Their d_alias list is empty ("socketallocbench -n 8" bench result : from 25s to 19.9s) Signed-off-by: Eric Dumazet <dada1@cosmosbay.com> --- fs/anon_inodes.c | 16 ------------ fs/dcache.c | 51 +++++++++++++++++++++++++++++++++++++++ fs/pipe.c | 23 +---------------- include/linux/dcache.h | 9 ++++++ net/socket.c | 24 +----------------- 5 files changed, 65 insertions(+), 58 deletions(-) -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html