[patch 5/9] fs: remove unneeded DCACHE_UNHASHED tricks

[patch 5/9] fs: remove unneeded DCACHE_UNHASHED tricks

[akpm]

From: Nick Piggin <npiggin@suse.de>

Filesystems outside the regular namespace do not have to clear
DCACHE_UNHASHED in order to have a working /proc/$pid/fd/XXX.  Nothing in
proc prevents the fd link from being used if its dentry is not in the
hash.

Also, it does not get put into the dcache hash if DCACHE_UNHASHED is
clear; that depends on the filesystem calling d_add or d_rehash.

So delete the misleading comments and needless code.

Acked-by: Miklos Szeredi <mszeredi@suse.cz>
Signed-off-by: Nick Piggin <npiggin@suse.de>
Cc: Davide Libenzi <davidel@xmailserver.org>
Cc: "David S. Miller" <davem@davemloft.net>
Cc: Jens Axboe <jens.axboe@oracle.com>
Cc: Christoph Hellwig <hch@lst.de>
Cc: Al Viro <viro@zeniv.linux.org.uk>
Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
---

 fs/anon_inodes.c |   16 ----------------
 fs/pipe.c        |   18 ------------------
 net/socket.c     |   19 -------------------
 3 files changed, 53 deletions(-)

diff -puN fs/anon_inodes.c~fs-remove-unneeded-dcache_unhashed-tricks fs/anon_inodes.c
--- a/fs/anon_inodes.c~fs-remove-unneeded-dcache_unhashed-tricks
+++ a/fs/anon_inodes.c
@@ -35,24 +35,11 @@ static int anon_inodefs_get_sb(struct fi
 			     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 const struct dentry_operations anon_inodefs_dentry_operations = {
-	.d_delete	= anon_inodefs_delete_dentry,
-};
 
 /*
  * nop .set_page_dirty method so that people can use .page_mkwrite on
@@ -117,9 +104,6 @@ struct file *anon_inode_getfile(const ch
 	 */
 	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;
diff -puN fs/pipe.c~fs-remove-unneeded-dcache_unhashed-tricks fs/pipe.c
--- a/fs/pipe.c~fs-remove-unneeded-dcache_unhashed-tricks
+++ a/fs/pipe.c
@@ -906,17 +906,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().
@@ -928,7 +917,6 @@ static char *pipefs_dname(struct dentry 
 }
 
 static const struct dentry_operations pipefs_dentry_operations = {
-	.d_delete	= pipefs_delete_dentry,
 	.d_dname	= pipefs_dname,
 };
 
@@ -988,12 +976,6 @@ struct file *create_write_pipe(int flags
 		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;
diff -puN net/socket.c~fs-remove-unneeded-dcache_unhashed-tricks net/socket.c
--- a/net/socket.c~fs-remove-unneeded-dcache_unhashed-tricks
+++ a/net/socket.c
@@ -312,18 +312,6 @@ static struct file_system_type sock_fs_t
 	.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().
  */
@@ -334,7 +322,6 @@ static char *sockfs_dname(struct dentry 
 }
 
 static const struct dentry_operations sockfs_dentry_operations = {
-	.d_delete = sockfs_delete_dentry,
 	.d_dname  = sockfs_dname,
 };
 
@@ -383,12 +370,6 @@ static int sock_attach_fd(struct socket 
 		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;
_

Re: [patch 5/9] fs: remove unneeded DCACHE_UNHASHED tricks

[Nick Piggin]

On Tue, Nov 17, 2009 at 02:56:30PM -0800, Andrew Morton wrote:
> From: Nick Piggin <npiggin@suse.de>
> 
> Filesystems outside the regular namespace do not have to clear
> DCACHE_UNHASHED in order to have a working /proc/$pid/fd/XXX.  Nothing in
> proc prevents the fd link from being used if its dentry is not in the
> hash.
> 
> Also, it does not get put into the dcache hash if DCACHE_UNHASHED is
> clear; that depends on the filesystem calling d_add or d_rehash.
> 
> So delete the misleading comments and needless code.

Thanks for carrying this for me Andrew, I'm sorry it slipped my mind
that you had it in your tree -- it just requires a small update to fix
the /proc/pid/fd link path name for anonfs.

--
fs: anon_inodes implement dname

Add a d_dname method for anon_inodes filesystem, the same way pipefs and sockfs
pseudo filesystems. This allows us to remove the DCACHE_UNHASHED hack from
anon_inodes.c (see next patch).

Signed-off-by: Nick Piggin <npiggin@suse.de>
---
 fs/anon_inodes.c |   10 ++++++++++
 1 file changed, 10 insertions(+)

Index: linux-2.6/fs/anon_inodes.c
===================================================================
--- linux-2.6.orig/fs/anon_inodes.c
+++ linux-2.6/fs/anon_inodes.c
@@ -45,6 +45,15 @@ static int anon_inodefs_delete_dentry(st
 	return 1;
 }
 
+/*
+ * anon_inodefs_dname() is called from d_path().
+ */
+static char *anon_inodefs_dname(struct dentry *dentry, char *buffer, int buflen)
+{
+	return dynamic_dname(dentry, buffer, buflen, "anon_inode:%s",
+				dentry->d_name.name, dentry->d_inode->i_ino);
+}
+
 static struct file_system_type anon_inode_fs_type = {
 	.name		= "anon_inodefs",
 	.get_sb		= anon_inodefs_get_sb,
@@ -52,6 +61,7 @@ static struct file_system_type anon_inod
 };
 static const struct dentry_operations anon_inodefs_dentry_operations = {
 	.d_delete	= anon_inodefs_delete_dentry,
+	.d_dname	= anon_inodefs_dname,
 };
 
 /*

Re: [patch 5/9] fs: remove unneeded DCACHE_UNHASHED tricks

[Nick Piggin]

On Wed, Nov 18, 2009 at 05:29:14AM +0100, Nick Piggin wrote:
> On Tue, Nov 17, 2009 at 02:56:30PM -0800, Andrew Morton wrote:
> > From: Nick Piggin <npiggin@suse.de>
> > 
> > Filesystems outside the regular namespace do not have to clear
> > DCACHE_UNHASHED in order to have a working /proc/$pid/fd/XXX.  Nothing in
> > proc prevents the fd link from being used if its dentry is not in the
> > hash.
> > 
> > Also, it does not get put into the dcache hash if DCACHE_UNHASHED is
> > clear; that depends on the filesystem calling d_add or d_rehash.
> > 
> > So delete the misleading comments and needless code.
> 
> Thanks for carrying this for me Andrew, I'm sorry it slipped my mind
> that you had it in your tree -- it just requires a small update to fix
> the /proc/pid/fd link path name for anonfs.
> 
> --
> fs: anon_inodes implement dname

And then here is the rediffed DCACHE_UNHASHED patch.
--
fs: no games with DCACHE_UNHASHED

Filesystems outside the regular namespace do not have to clear DCACHE_UNHASHED
in order to have a working /proc/$pid/fd/XXX. Nothing in proc prevents the
fd link from being used if its dentry is not in the hash.

Also, it does not get put into the dcache hash if DCACHE_UNHASHED is clear;
that depends on the filesystem calling d_add or d_rehash.

So delete the misleading comments and needless code.

Acked-by: Miklos Szeredi <mszeredi@suse.cz>
Signed-off-by: Nick Piggin <npiggin@suse.de>
---
 fs/anon_inodes.c |   13 -------------
 fs/pipe.c        |   18 ------------------
 net/socket.c     |   19 -------------------
 3 files changed, 50 deletions(-)

Index: linux-2.6/fs/pipe.c
===================================================================
--- linux-2.6.orig/fs/pipe.c
+++ linux-2.6/fs/pipe.c
@@ -906,17 +906,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().
@@ -928,7 +917,6 @@ static char *pipefs_dname(struct dentry
 }
 
 static const struct dentry_operations pipefs_dentry_operations = {
-	.d_delete	= pipefs_delete_dentry,
 	.d_dname	= pipefs_dname,
 };
 
@@ -988,12 +976,6 @@ struct file *create_write_pipe(int flags
 		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;
Index: linux-2.6/net/socket.c
===================================================================
--- linux-2.6.orig/net/socket.c
+++ linux-2.6/net/socket.c
@@ -306,18 +306,6 @@ static struct file_system_type sock_fs_t
 	.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().
  */
@@ -328,7 +316,6 @@ static char *sockfs_dname(struct dentry
 }
 
 static const struct dentry_operations sockfs_dentry_operations = {
-	.d_delete = sockfs_delete_dentry,
 	.d_dname  = sockfs_dname,
 };
 
@@ -377,12 +364,6 @@ static int sock_attach_fd(struct socket
 		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;
Index: linux-2.6/fs/anon_inodes.c
===================================================================
--- linux-2.6.orig/fs/anon_inodes.c
+++ linux-2.6/fs/anon_inodes.c
@@ -35,16 +35,6 @@ static int anon_inodefs_get_sb(struct fi
 			     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;
-}
-
 /*
  * anon_inodefs_dname() is called from d_path().
  */
@@ -60,7 +50,6 @@ static struct file_system_type anon_inod
 	.kill_sb	= kill_anon_super,
 };
 static const struct dentry_operations anon_inodefs_dentry_operations = {
-	.d_delete	= anon_inodefs_delete_dentry,
 	.d_dname	= anon_inodefs_dname,
 };
 
@@ -128,8 +117,6 @@ struct file *anon_inode_getfile(const ch
 	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;

Re: [patch 5/9] fs: remove unneeded DCACHE_UNHASHED tricks

[OGAWA Hirofumi]

Nick Piggin <npiggin@suse.de> writes:

> +	return dynamic_dname(dentry, buffer, buflen, "anon_inode:%s",
> +				dentry->d_name.name, dentry->d_inode->i_ino);
> +}

This may be typo? The fmt has only "%s", but args are ->d_name.name and
->i_no.

If so, it sounds like we would need to add

   __attribute__ ((format(printf, ...)))

stuff to dynamic_dname().

Thanks.
-- 
OGAWA Hirofumi <hirofumi@mail.parknet.co.jp>