Messages in this thread Patch in this message | | | Date | Mon, 4 Sep 2006 17:54:19 -0500 | From | Michael Halcrow <> | Subject | Re: [PATCH 4/4] eCryptfs: ino_t to u64 for filldir |
| |
On Thu, Aug 31, 2006 at 11:30:31AM +0100, David Howells wrote: > Michael Halcrow <mhalcrow@us.ibm.com> wrote: > > + inode = iget5_locked(sb, lower_inode->i_ino, ecryptfs_inode_test, > > + ecryptfs_inode_set, lower_inode); > > The second argument of iget5_locked() is a hash value. I would use > lower_inode not lower_inode->i_ino as the former is fundamental to > your search and the latter irrelevant.
Is it safe to assume that a pointer will always be equal to or smaller than an unsigned long for all architectures? Casting a pointer to an unsigned long, in general, makes me a bit uncomfortable.
> > + inode->i_ino = lower_inode->i_ino; > > + if (inode->i_state & I_NEW) { > > + ecryptfs_init_inode(inode); > > + unlock_new_inode(inode); > > + } > > Shouldn't the setting of i_ino be inside the if-statement? > > You should set the lower inode pointer in ecryptfs_inode_set() so > that the ecryptfs inode is linked to the lower inode whilst > inode_lock is held (see get_new_inode()). You could also set i_ino > there too. Consider this bit of pseudocode: > > int ecryptfs_inode_set(struct inode *inode, void *lower_inode) > { > ecryptfs_set_lower_inode(inode, lower_inode); > inode->i_ino = lower_inode->i_ino; > return 0; > }
Dave Chinner told me that XFS uses 32-bit inode numbers on 32-bit machines, so I imagine that this patch really is only helpful for NFS.
How does this look?
---
Modify inode number generation to account for differences in the inode number data type sizes in lower filesystems.
Signed-off-by: Michael Halcrow <mhalcrow@us.ibm.com>
---
fs/ecryptfs/ecryptfs_kernel.h | 3 +++ fs/ecryptfs/inode.c | 23 +++++++++++++++++++++++ fs/ecryptfs/main.c | 16 +++++++--------- fs/ecryptfs/super.c | 12 ++++-------- 4 files changed, 37 insertions(+), 17 deletions(-)
62dee692ac7e88898f329e389ebd3178fce428a3 diff --git a/fs/ecryptfs/ecryptfs_kernel.h b/fs/ecryptfs/ecryptfs_kernel.h index 349ce2a..860af16 100644 --- a/fs/ecryptfs/ecryptfs_kernel.h +++ b/fs/ecryptfs/ecryptfs_kernel.h @@ -474,5 +474,8 @@ int ecryptfs_truncate(struct dentry *den int ecryptfs_process_cipher(struct crypto_tfm **tfm, struct crypto_tfm **key_tfm, char *cipher_name, size_t key_size); +int ecryptfs_inode_test(struct inode *inode, void *candidate_lower_inode); +int ecryptfs_inode_set(struct inode *inode, void *ignored); +void ecryptfs_init_inode(struct inode *inode); #endif /* #ifndef ECRYPTFS_KERNEL_H */ diff --git a/fs/ecryptfs/inode.c b/fs/ecryptfs/inode.c index d8659ff..642a260 100644 --- a/fs/ecryptfs/inode.c +++ b/fs/ecryptfs/inode.c @@ -1024,6 +1024,29 @@ out: return rc; } +int ecryptfs_inode_test(struct inode *inode, void *candidate_lower_inode) +{ + if (ecryptfs_inode_to_private(inode) && + (ecryptfs_inode_to_lower(inode) + == (struct inode *)candidate_lower_inode)) + return 1; + else + return 0; +} + +int ecryptfs_inode_set(struct inode *inode, void *lower_inode) +{ + /* This is where we setup the self-reference in the vfs_inode's + * i_private. That way we don't have to walk the list again. */ + ecryptfs_set_inode_private(inode, + container_of(inode, + struct ecryptfs_inode_info, + vfs_inode)); + ecryptfs_set_inode_lower(inode, igrab((struct inode *)lower_inode)); + inode->i_ino = ((struct inode *)lower_inode)->i_ino; + return 0; +} + struct inode_operations ecryptfs_symlink_iops = { .readlink = ecryptfs_readlink, .follow_link = ecryptfs_follow_link, diff --git a/fs/ecryptfs/main.c b/fs/ecryptfs/main.c index f7ea912..83e920c 100644 --- a/fs/ecryptfs/main.c +++ b/fs/ecryptfs/main.c @@ -68,9 +68,6 @@ void __ecryptfs_printk(const char *fmt, * * Interposes upper and lower dentries. * - * This function will allocate an ecryptfs_inode through the call to - * iget(sb, lower_inode->i_ino). - * * Returns zero on success; non-zero otherwise */ int ecryptfs_interpose(struct dentry *lower_dentry, struct dentry *dentry, @@ -85,22 +82,23 @@ int ecryptfs_interpose(struct dentry *lo rc = -EXDEV; goto out; } - inode = iget(sb, lower_inode->i_ino); + inode = iget5_locked(sb, (unsigned long)lower_inode, + ecryptfs_inode_test, ecryptfs_inode_set, + lower_inode); if (!inode) { rc = -EACCES; goto out; } - /* This check is required here because if we failed to allocated the - * required space for an inode_info_cache struct, then the only way - * we know we failed, is by the pointer being NULL */ + if (inode->i_state & I_NEW) { + ecryptfs_init_inode(inode); + unlock_new_inode(inode); + } if (!ecryptfs_inode_to_private(inode)) { ecryptfs_printk(KERN_ERR, "Out of memory. Failure to " "allocate memory in ecryptfs_read_inode.\n"); rc = -ENOMEM; goto out; } - if (!ecryptfs_inode_to_lower(inode)) - ecryptfs_set_inode_lower(inode, igrab(lower_inode)); if (S_ISLNK(lower_inode->i_mode)) inode->i_op = &ecryptfs_symlink_iops; else if (S_ISDIR(lower_inode->i_mode)) diff --git a/fs/ecryptfs/super.c b/fs/ecryptfs/super.c index f4f06ea..f2bfac3 100644 --- a/fs/ecryptfs/super.c +++ b/fs/ecryptfs/super.c @@ -78,25 +78,21 @@ static void ecryptfs_destroy_inode(struc } /** - * ecryptfs_read_inode + * ecryptfs_init_inode * @inode: The ecryptfs inode * * Set up the ecryptfs inode. */ -static void ecryptfs_read_inode(struct inode *inode) +void ecryptfs_init_inode(struct inode *inode) { - /* This is where we setup the self-reference in the vfs_inode's - * i_private. That way we don't have to walk the list again. */ - ecryptfs_set_inode_private(inode, - list_entry(inode, struct ecryptfs_inode_info, - vfs_inode)); - ecryptfs_set_inode_lower(inode, NULL); inode->i_version++; inode->i_op = &ecryptfs_main_iops; inode->i_fop = &ecryptfs_main_fops; inode->i_mapping->a_ops = &ecryptfs_aops; } +static void ecryptfs_read_inode(struct inode *inode) { } + /** * ecryptfs_put_super * @sb: Pointer to the ecryptfs super block -- 1.3.3 - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
| |