lkml.org 
[lkml]   [2006]   [Sep]   [5]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    Patch in this message
    /
    Date
    From
    SubjectRe: [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/

    \
     
     \ /
      Last update: 2006-09-06 02:35    [W:3.748 / U:0.268 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site