Re: [PATCH 3/5] Inode: Allow external initialisers

Lachlan McIlroy
2008-10-14T06:00:40Z

Dave, this is modifying files outside fs/xfs. Who has reviewed this patch?

Dave Chinner wrote:
> To allow XFS to combine the XFS and linux inodes into a single
> structure, we need to drive inode lookup from the XFS inode cache,
> not the generic inode cache. This means that we need initialise a
> struct inode from a context outside alloc_inode() as it is no longer
> used by XFS.
>
> Factor and export the struct inode initialisation code from
> alloc_inode() to inode_init_always() as a counterpart to
> inode_init_once(). i.e. we have to call this init function for each
> inode instantiation (always), as opposed inode_init_once() which is
> only called on slab object instantiation (once).
>
> Signed-off-by: Dave Chinner <david@fromorbit.com>
> ---
> fs/inode.c | 152 +++++++++++++++++++++++++++++-----------------------
> include/linux/fs.h | 1 +
> 2 files changed, 85 insertions(+), 68 deletions(-)
>
> diff --git a/fs/inode.c b/fs/inode.c
> index 0487ddb..e7ee999 100644
> --- a/fs/inode.c
> +++ b/fs/inode.c
> @@ -108,84 +108,100 @@ static void wake_up_inode(struct inode *inode)
> wake_up_bit(&inode->i_state, __I_LOCK);
> }
>
> -static struct inode *alloc_inode(struct super_block *sb)
> +/**
> + * inode_init_always - perform inode structure intialisation
> + * @sb - superblock inode belongs to.
> + * @inode - inode to initialise
> + *
> + * These are initializations that need to be done on every inode
> + * allocation as the fields are not initialised by slab allocation.
> + */
> +struct inode *inode_init_always(struct super_block *sb, struct inode *inode)
> {
> static const struct address_space_operations empty_aops;
> static struct inode_operations empty_iops;
> static const struct file_operations empty_fops;
> - struct inode *inode;
> -
> - if (sb->s_op->alloc_inode)
> - inode = sb->s_op->alloc_inode(sb);
> - else
> - inode = (struct inode *) kmem_cache_alloc(inode_cachep, GFP_KERNEL);
>
> - if (inode) {
> - struct address_space * const mapping = &inode->i_data;
> -
> - inode->i_sb = sb;
> - inode->i_blkbits = sb->s_blocksize_bits;
> - inode->i_flags = 0;
> - atomic_set(&inode->i_count, 1);
> - inode->i_op = &empty_iops;
> - inode->i_fop = &empty_fops;
> - inode->i_nlink = 1;
> - atomic_set(&inode->i_writecount, 0);
> - inode->i_size = 0;
> - inode->i_blocks = 0;
> - inode->i_bytes = 0;
> - inode->i_generation = 0;
> + struct address_space * const mapping = &inode->i_data;
> +
> + inode->i_sb = sb;
> + inode->i_blkbits = sb->s_blocksize_bits;
> + inode->i_flags = 0;
> + atomic_set(&inode->i_count, 1);
> + inode->i_op = &empty_iops;
> + inode->i_fop = &empty_fops;
> + inode->i_nlink = 1;
> + atomic_set(&inode->i_writecount, 0);
> + inode->i_size = 0;
> + inode->i_blocks = 0;
> + inode->i_bytes = 0;
> + inode->i_generation = 0;
> #ifdef CONFIG_QUOTA
> - memset(&inode->i_dquot, 0, sizeof(inode->i_dquot));
> + memset(&inode->i_dquot, 0, sizeof(inode->i_dquot));
> #endif
> - inode->i_pipe = NULL;
> - inode->i_bdev = NULL;
> - inode->i_cdev = NULL;
> - inode->i_rdev = 0;
> - inode->dirtied_when = 0;
> - if (security_inode_alloc(inode)) {
> - if (inode->i_sb->s_op->destroy_inode)
> - inode->i_sb->s_op->destroy_inode(inode);
> - else
> - kmem_cache_free(inode_cachep, (inode));
> - return NULL;
> - }
> + inode->i_pipe = NULL;
> + inode->i_bdev = NULL;
> + inode->i_cdev = NULL;
> + inode->i_rdev = 0;
> + inode->dirtied_when = 0;
> + if (security_inode_alloc(inode)) {
> + if (inode->i_sb->s_op->destroy_inode)
> + inode->i_sb->s_op->destroy_inode(inode);
> + else
> + kmem_cache_free(inode_cachep, (inode));
> + return NULL;
> + }
>
> - spin_lock_init(&inode->i_lock);
> - lockdep_set_class(&inode->i_lock, &sb->s_type->i_lock_key);
> + spin_lock_init(&inode->i_lock);
> + lockdep_set_class(&inode->i_lock, &sb->s_type->i_lock_key);
>
> - mutex_init(&inode->i_mutex);
> - lockdep_set_class(&inode->i_mutex, &sb->s_type->i_mutex_key);
> + mutex_init(&inode->i_mutex);
> + lockdep_set_class(&inode->i_mutex, &sb->s_type->i_mutex_key);
>
> - init_rwsem(&inode->i_alloc_sem);
> - lockdep_set_class(&inode->i_alloc_sem, &sb->s_type->i_alloc_sem_key);
> + init_rwsem(&inode->i_alloc_sem);
> + lockdep_set_class(&inode->i_alloc_sem, &sb->s_type->i_alloc_sem_key);
>
> - mapping->a_ops = &empty_aops;
> - mapping->host = inode;
> - mapping->flags = 0;
> - mapping_set_gfp_mask(mapping, GFP_HIGHUSER_PAGECACHE);
> - mapping->assoc_mapping = NULL;
> - mapping->backing_dev_info = &default_backing_dev_info;
> - mapping->writeback_index = 0;
> + mapping->a_ops = &empty_aops;
> + mapping->host = inode;
> + mapping->flags = 0;
> + mapping_set_gfp_mask(mapping, GFP_HIGHUSER_PAGECACHE);
> + mapping->assoc_mapping = NULL;
> + mapping->backing_dev_info = &default_backing_dev_info;
> + mapping->writeback_index = 0;
>
> - /*
> - * If the block_device provides a backing_dev_info for client
> - * inodes then use that. Otherwise the inode share the bdev's
> - * backing_dev_info.
> - */
> - if (sb->s_bdev) {
> - struct backing_dev_info *bdi;
> + /*
> + * If the block_device provides a backing_dev_info for client
> + * inodes then use that. Otherwise the inode share the bdev's
> + * backing_dev_info.
> + */
> + if (sb->s_bdev) {
> + struct backing_dev_info *bdi;
>
> - bdi = sb->s_bdev->bd_inode_backing_dev_info;
> - if (!bdi)
> - bdi = sb->s_bdev->bd_inode->i_mapping->backing_dev_info;
> - mapping->backing_dev_info = bdi;
> - }
> - inode->i_private = NULL;
> - inode->i_mapping = mapping;
> + bdi = sb->s_bdev->bd_inode_backing_dev_info;
> + if (!bdi)
> + bdi = sb->s_bdev->bd_inode->i_mapping->backing_dev_info;
> + mapping->backing_dev_info = bdi;
> }
> + inode->i_private = NULL;
> + inode->i_mapping = mapping;
> +
> return inode;
> }
> +EXPORT_SYMBOL(inode_init_always);
> +
> +static struct inode *alloc_inode(struct super_block *sb)
> +{
> + struct inode *inode;
> +
> + if (sb->s_op->alloc_inode)
> + inode = sb->s_op->alloc_inode(sb);
> + else
> + inode = kmem_cache_alloc(inode_cachep, GFP_KERNEL);
> +
> + if (inode)
> + return inode_init_always(sb, inode);
> + return NULL;
> +}
>
> void destroy_inode(struct inode *inode)
> {
> diff --git a/include/linux/fs.h b/include/linux/fs.h
> index 580b513..ce55983 100644
> --- a/include/linux/fs.h
> +++ b/include/linux/fs.h
> @@ -1811,6 +1811,7 @@ extern loff_t default_llseek(struct file *file, loff_t offset, int origin);
>
> extern loff_t vfs_llseek(struct file *file, loff_t offset, int origin);
>
> +extern struct inode * inode_init_always(struct super_block *, struct inode *);
> extern void inode_init_once(struct inode *);
> extern void iput(struct inode *);
> extern struct inode * igrab(struct inode *);