Why do fs outside regular namespace clear DCACHE_UNHASHED?

Why do fs outside regular namespace clear DCACHE_UNHASHED?

[Nick Piggin]

Please humour a relative vfs/procfs newbie... Why do filesystems insist that
a dentry must be !DCACHE_UNHASHED in order to work in /proc/pid/fd/? I can't
find the code that checks this, and pipefs fds for example _seems_ to work in
proc/pid with this patch applied...


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

Index: linux-2.6/fs/pipe.c
===================================================================
--- linux-2.6.orig/fs/pipe.c
+++ linux-2.6/fs/pipe.c
@@ -873,17 +873,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().
@@ -895,7 +884,6 @@ static char *pipefs_dname(struct dentry
 }
 
 static const struct dentry_operations pipefs_dentry_operations = {
-	.d_delete	= pipefs_delete_dentry,
 	.d_dname	= pipefs_dname,
 };
 
@@ -955,12 +943,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
@@ -307,18 +307,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().
  */
@@ -329,7 +317,6 @@ static char *sockfs_dname(struct dentry
 }
 
 static const struct dentry_operations sockfs_dentry_operations = {
-	.d_delete = sockfs_delete_dentry,
 	.d_dname  = sockfs_dname,
 };
 
@@ -378,12 +365,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
@@ -33,24 +33,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,
-};
 
 /**
  * anon_inode_getfd - creates a new file instance by hooking it up to an
@@ -106,9 +93,6 @@ int anon_inode_getfd(const char *name, c
 	 */
 	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: Why do fs outside regular namespace clear DCACHE_UNHASHED?

[Miklos Szeredi]

On Thu, 28 May 2009, Nick Piggin wrote:
> Please humour a relative vfs/procfs newbie... Why do filesystems insist that
> a dentry must be !DCACHE_UNHASHED in order to work in /proc/pid/fd/? I can't
> find the code that checks this, and pipefs fds for example _seems_ to work in
> proc/pid with this patch applied...

I suspect this is more of an optimization: the old code tries not to
burden the global dentry cache with all the anonymous pipes, sockets,
etc. since they won't ever be looked up from the cache.

Thanks,
Miklos

> 
> 
> ---
>  fs/anon_inodes.c |   16 ----------------
>  fs/pipe.c        |   18 ------------------
>  net/socket.c     |   19 -------------------
>  3 files changed, 53 deletions(-)
> 
> Index: linux-2.6/fs/pipe.c
> ===================================================================
> --- linux-2.6.orig/fs/pipe.c
> +++ linux-2.6/fs/pipe.c
> @@ -873,17 +873,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().
> @@ -895,7 +884,6 @@ static char *pipefs_dname(struct dentry
>  }
>  
>  static const struct dentry_operations pipefs_dentry_operations = {
> -	.d_delete	= pipefs_delete_dentry,
>  	.d_dname	= pipefs_dname,
>  };
>  
> @@ -955,12 +943,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
> @@ -307,18 +307,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().
>   */
> @@ -329,7 +317,6 @@ static char *sockfs_dname(struct dentry
>  }
>  
>  static const struct dentry_operations sockfs_dentry_operations = {
> -	.d_delete = sockfs_delete_dentry,
>  	.d_dname  = sockfs_dname,
>  };
>  
> @@ -378,12 +365,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
> @@ -33,24 +33,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,
> -};
>  
>  /**
>   * anon_inode_getfd - creates a new file instance by hooking it up to an
> @@ -106,9 +93,6 @@ int anon_inode_getfd(const char *name, c
>  	 */
>  	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;
> --
> To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

Re: Why do fs outside regular namespace clear DCACHE_UNHASHED?

[Nick Piggin]

On Thu, May 28, 2009 at 01:43:40PM +0200, Miklos Szeredi wrote:
> On Thu, 28 May 2009, Nick Piggin wrote:
> > Please humour a relative vfs/procfs newbie... Why do filesystems insist that
> > a dentry must be !DCACHE_UNHASHED in order to work in /proc/pid/fd/? I can't
> > find the code that checks this, and pipefs fds for example _seems_ to work in
> > proc/pid with this patch applied...
> 
> I suspect this is more of an optimization: the old code tries not to
> burden the global dentry cache with all the anonymous pipes, sockets,
> etc. since they won't ever be looked up from the cache.

But I also couldn't see that the flag causes the dcache to put the
entry in the hash or not...

Re: Why do fs outside regular namespace clear DCACHE_UNHASHED?

[Miklos Szeredi]

On Mon, 1 Jun 2009, Nick Piggin wrote:
> > I suspect this is more of an optimization: the old code tries not to
> > burden the global dentry cache with all the anonymous pipes, sockets,
> > etc. since they won't ever be looked up from the cache.
> 
> But I also couldn't see that the flag causes the dcache to put the
> entry in the hash or not...

Oh, right.  Even if that was the intent it hadn't been working for a
while now.  I aggree with your patch then.

Acked-by: Miklos Szeredi <mszeredi@suse.cz>

Thanks,
Miklos