Re: [PATCH] ecryptfs: remove unnecessary d_drop calls in ecryptfs_link

[Tyler Hicks <tyhicks@linux.vnet.ibm.com>]

On Sun Dec 06, 2009 at 06:05:30PM -0500, Erez Zadok (ezk@cs.sunysb.edu) was quoted:
> Tyler/Dustin,
> 
> We found three unnecessary calls to d_drop inside ecryptfs_link().  They are
> called unconditionally whether ->link succeeds or not.  They are unnecessary
> because they often remove valid objects from the dcache, which forces these
> objects to be re-looked up the next time they're needed.  Presumably the
> user process which called link(2) is likely to use those newly created
> hardlinks right afterwards.  So removing them from the dcache hurts
> performance.
> 
> We've tested the following patch in ecryptfs by successfully hard-linking a
> few files, looking them up, deleting them, etc.  Nothing too intensive
> thou.n However, we don't have those d_drop calls in unionfs, and it's been
> working fine for years.  Moreover, we've tested with and without those three
> d_drop calls in our wrapfs "pass-through" stackable file system with the
> full ltp/racer/etc. suites and all's well.  Therefore, we're reasonably
> certain that this patch is good.  Bug discovered by Aseem Rastogi during
> code review.
> 
> Still, perhaps someone can recall some history and how come these d_drop
> calls showed up in ecryptfs_link()?

git blame points out commits 45ec4aba and ae56fb16, but you aren't the
first person to point out these seemingly pointless d_drop's.  I think
I'll go ahead and commit this to the next branch and give it plenty of
testing.

Applied to
git://git.kernel.org/pub/scm/linux/kernel/git/ecryptfs/ecryptfs-2.6.git#next

> 
> Cheers,
> Erez.
> 
> ------------------------------------------------------------------------------
> 
> ecryptfs: remove unnecessary d_drop calls in wrapfs_link
> 
> Unnecessary because it would unhash perfectly valid dentries, causing them
> to have to be re-looked up the next time they're needed, which presumably is
> right after.
> 
> Signed-off-by: Aseem Rastogi <arastogi@cs.sunysb.edu>
> Signed-off-by: Shrikar archak <shrikar84@gmail.com>
> Signed-off-by: Erez Zadok <ezk@cs.sunysb.edu>
> diff --git a/fs/ecryptfs/inode.c b/fs/ecryptfs/inode.c
> index 186d744..2dc0bdb 100644
> --- a/fs/ecryptfs/inode.c
> +++ b/fs/ecryptfs/inode.c
> @@ -463,9 +463,6 @@ out_lock:
>  	unlock_dir(lower_dir_dentry);
>  	dput(lower_new_dentry);
>  	dput(lower_old_dentry);
> -	d_drop(lower_old_dentry);
> -	d_drop(new_dentry);
> -	d_drop(old_dentry);
>  	return rc;
>  }
>