Re: [Ecryptfs-devel] [PATCH] ecryptfs: some inode attrs, and a question

[hooanon05@yahoo.co.jp]

Tyler Hicks:
> >> - remove the unnecessary d_drop()s in ecryptfs_link().
> 
> I tend to agree that they look unnecessary, but one of them was added
> for cifs (see ae56fb16) and the other two seem to be intentional (see
> 45ec4aba) as well.  I'm working on adding support for networked
> filesystems, I'll keep these d_drop()s in mind and see if we can drop
> them. :)

commit ae56fb16337c882c52806508f93ead4034004c7a
    When CIFS is the lower filesystem, the old lower dentry needs to be explicitly
    dropped from inside eCryptfs to force a revalidate.  In addition, when CIFS is
    the lower filesystem, the inode attributes need to be copied back up from the
    lower inode to the eCryptfs inode on an eCryptfs revalidate.

Even with this commit, fstat(2) still returns incorrect attributes,
doesn't it? Because fstat(2) doesn't involve revalidate.
{
	fd = open(fileA);
	link(fileA, fileB);
	fstat(fd);
}


commit 45ec4ababe999cb95f9c0cad03b2689cb0b77a2b
    Fix the use of dget/dput calls to balance out on the lower filesystem.
	:::
diff --git a/fs/ecryptfs/inode.c b/fs/ecryptfs/inode.c
index 2f2c6cf..ff4865d 100644
--- a/fs/ecryptfs/inode.c
+++ b/fs/ecryptfs/inode.c
	:::
@@ -475,8 +470,8 @@ out_lock:
 	unlock_dir(lower_dir_dentry);
 	dput(lower_new_dentry);
 	dput(lower_old_dentry);
-	if (!new_dentry->d_inode)
-		d_drop(new_dentry);
+	d_drop(new_dentry);
+	d_drop(old_dentry);
 	return rc;
 }
 
Are these two d_drop()s really intentional?
I don't understand why they are necessary after the success of link.
It might be the same to the commit ae56fb16 (see above), ie. wanting to
force re-lookup and get the latest inode attributes. (Just a guess)
Actually they are maintained in ecryptfs_link() correctly...


J. R. Okajima