Re: [PATCH 3/4] add __destroy_inode

From: Eric Sandeen <sandeen@sandeen.net>
To: Christoph Hellwig <hch@infradead.org>
Cc: linux-fsdevel@vger.kernel.org, xfs@oss.sgi.com
Subject: Re: [PATCH 3/4] add __destroy_inode
Date: Thu, 06 Aug 2009 17:56:53 -0500	[thread overview]
Message-ID: <4A7B5FB5.2030706@sandeen.net> (raw)
In-Reply-To: <20090804141836.606829000@bombadil.infradead.org>

Christoph Hellwig wrote:

> When we want to tear down an inode that lost the add to the cache race
> in XFS we must not call into ->destroy_inode because that would delete
> the inode that won the race from the inode cache radix tree.
>
> This patch provides the __destroy_inode helper needed to fix this,
> the actual fix will be in th next patch.
>
> Signed-off-by: Christoph Hellwig <hch@lst.de>
>
Looks fine to me logically, though having __destroy_inode do everything
-but- ->destroy_inode is a little funky semantically ... maybe
__free_inode?  Not a huge deal.

Reviewed-by: Eric Sandeen <sandeen@sandeen.net>

> Index: linux-2.6/fs/inode.c
> ===================================================================
> --- linux-2.6.orig/fs/inode.c	2009-08-04 16:01:18.766801320 +0200
> +++ linux-2.6/fs/inode.c	2009-08-04 16:01:37.281556243 +0200
> @@ -228,7 +228,7 @@ static struct inode *alloc_inode(struct 
>  	return inode;
>  }
>  
> -void destroy_inode(struct inode *inode)
> +void __destroy_inode(struct inode *inode)
>  {
>  	BUG_ON(inode_has_buffers(inode));
>  	ima_inode_free(inode);
> @@ -240,13 +240,17 @@ void destroy_inode(struct inode *inode)
>  	if (inode->i_default_acl && inode->i_default_acl != ACL_NOT_CACHED)
>  		posix_acl_release(inode->i_default_acl);
>  #endif
> +}
> +EXPORT_SYMBOL(__destroy_inode);
> +
> +void destroy_inode(struct inode *inode)
> +{
> +	__destroy_inode(inode);
>  	if (inode->i_sb->s_op->destroy_inode)
>  		inode->i_sb->s_op->destroy_inode(inode);
>  	else
>  		kmem_cache_free(inode_cachep, (inode));
>  }
> -EXPORT_SYMBOL(destroy_inode);
> -
>  
>  /*
>   * These are initializations that only need to be done
> Index: linux-2.6/fs/xfs/xfs_inode.h
> ===================================================================
> --- linux-2.6.orig/fs/xfs/xfs_inode.h	2009-08-04 15:59:38.705782128 +0200
> +++ linux-2.6/fs/xfs/xfs_inode.h	2009-08-04 16:01:20.157556334 +0200
> @@ -322,8 +322,11 @@ static inline struct inode *VFS_I(struct
>   */
>  static inline void xfs_destroy_inode(struct xfs_inode *ip)
>  {
> -	make_bad_inode(VFS_I(ip));
> -	return destroy_inode(VFS_I(ip));
> +	struct inode *inode = VFS_I(ip);
> +
> +	make_bad_inode(inode);
> +	__destroy_inode(inode);
> +	inode->i_sb->s_op->destroy_inode(inode);
>  }
>  
>  /*
> Index: linux-2.6/include/linux/fs.h
> ===================================================================
> --- linux-2.6.orig/include/linux/fs.h	2009-08-04 16:01:18.770804693 +0200
> +++ linux-2.6/include/linux/fs.h	2009-08-04 16:01:20.159539128 +0200
> @@ -2163,6 +2163,7 @@ extern void __iget(struct inode * inode)
>  extern void iget_failed(struct inode *);
>  extern void clear_inode(struct inode *);
>  extern void destroy_inode(struct inode *);
> +extern void __destroy_inode(struct inode *);
>  extern struct inode *new_inode(struct super_block *);
>  extern int should_remove_suid(struct dentry *);
>  extern int file_remove_suid(struct file *);
>
> _______________________________________________
> xfs mailing list
> xfs@oss.sgi.com
> http://oss.sgi.com/mailman/listinfo/xfs
>

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs