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()? 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
