On Sun Dec 06, 2009 at 06:05:30PM -0500, Erez Zadok (ezk@xxxxxxxxxxxxx) 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@xxxxxxxxxxxxx> > Signed-off-by: Shrikar archak <shrikar84@xxxxxxxxx> > Signed-off-by: Erez Zadok <ezk@xxxxxxxxxxxxx> > 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; > } > -- To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
