lkml.org 
[lkml]   [2009]   [Jan]   [16]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
Patch in this message
/
From
SubjectRe: [Ecryptfs-devel] [PATCH] ecryptfs: some inode attrs, and a question
Date

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


\
 
 \ /
  Last update: 2009-01-16 09:07    [W:0.052 / U:0.260 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site