From mboxrd@z Thu Jan 1 00:00:00 1970 From: Tyler Hicks Subject: Re: [PATCH] ecryptfs: remove unnecessary d_drop calls in ecryptfs_link Date: Mon, 7 Dec 2009 16:32:05 -0600 Message-ID: <20091207223204.GC9807@fedora-virt> References: <200912062305.nB6N5UiG029402@agora.fsl.cs.sunysb.edu> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Cc: Dustin Kirkland , Saumitra Bhanage , Andrew Morton , viro@zeniv.linux.org.uk, linux-fsdevel@vger.kernel.org To: Erez Zadok Return-path: Received: from e39.co.us.ibm.com ([32.97.110.160]:40206 "EHLO e39.co.us.ibm.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S933422AbZLGWcE (ORCPT ); Mon, 7 Dec 2009 17:32:04 -0500 Received: from d03relay02.boulder.ibm.com (d03relay02.boulder.ibm.com [9.17.195.227]) by e39.co.us.ibm.com (8.14.3/8.13.1) with ESMTP id nB7MPZtW019151 for ; Mon, 7 Dec 2009 15:25:35 -0700 Received: from d03av04.boulder.ibm.com (d03av04.boulder.ibm.com [9.17.195.170]) by d03relay02.boulder.ibm.com (8.13.8/8.13.8/NCO v9.1) with ESMTP id nB7MW6gD063438 for ; Mon, 7 Dec 2009 15:32:06 -0700 Received: from d03av04.boulder.ibm.com (loopback [127.0.0.1]) by d03av04.boulder.ibm.com (8.14.3/8.13.1/NCO v10.0 AVout) with ESMTP id nB7MW5De011196 for ; Mon, 7 Dec 2009 15:32:05 -0700 Content-Disposition: inline In-Reply-To: <200912062305.nB6N5UiG029402@agora.fsl.cs.sunysb.edu> Sender: linux-fsdevel-owner@vger.kernel.org List-ID: 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 > Signed-off-by: Shrikar archak > Signed-off-by: Erez Zadok > 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; > } >