Re: [PATCH 02/11] inode: add IOP_NOTHASHED to avoid inode hash lock in evict

[Jan Kara <jack@suse.cz>]

On Wed 31-07-13 14:15:41, Dave Chinner wrote:
> From: Dave Chinner <dchinner@redhat.com>
> 
> Some filesystems don't use the VFS inode hash and fake the fact they
> are hashed so that all the writeback code works correctly. However,
> this means the evict() path still tries to remove the inode from the
> hash, meaning that the inode_hash_lock() needs to be taken
> unnecessarily. Hence under certain workloads the inode_hash_lock can
> be contended even if the inode is never actually hashed.
> 
> To avoid this, add an inode opflag to allow inode_hash_remove() to
> avoid taking the hash lock on inodes have never actually been
> hashed.
> 
> Signed-off-by: Dave Chinner <dchinner@redhat.com>
> ---
>  fs/xfs/xfs_iops.c  | 2 ++
>  include/linux/fs.h | 3 ++-
>  2 files changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/fs/xfs/xfs_iops.c b/fs/xfs/xfs_iops.c
> index 96dda62..68a8264 100644
> --- a/fs/xfs/xfs_iops.c
> +++ b/fs/xfs/xfs_iops.c
> @@ -1168,8 +1168,10 @@ xfs_setup_inode(
>  	inode->i_state = I_NEW;
>  
>  	inode_sb_list_add(inode);
> +
>  	/* make the inode look hashed for the writeback code */
>  	hlist_add_fake(&inode->i_hash);
> +	inode->i_opflags |= IOP_NOTHASHED;
>  
>  	inode->i_mode	= ip->i_d.di_mode;
>  	set_nlink(inode, ip->i_d.di_nlink);
> diff --git a/include/linux/fs.h b/include/linux/fs.h
> index b09ddc0..51cf6ed 100644
> --- a/include/linux/fs.h
> +++ b/include/linux/fs.h
> @@ -515,6 +515,7 @@ struct posix_acl;
>  #define IOP_FASTPERM	0x0001
>  #define IOP_LOOKUP	0x0002
>  #define IOP_NOFOLLOW	0x0004
> +#define IOP_NOTHASHED	0x0008	/* inode never hashed, avoid unhashing */
>  
>  /*
>   * Keep mostly read-only and often accessed (especially for
> @@ -2371,7 +2372,7 @@ static inline void insert_inode_hash(struct inode *inode)
>  extern void __remove_inode_hash(struct inode *);
>  static inline void remove_inode_hash(struct inode *inode)
>  {
> -	if (!inode_unhashed(inode))
> +	if (!((inode->i_opflags & IOP_NOTHASHED) || inode_unhashed(inode)))
  It's somewhat easier for me to read if the condition if written as:
	if (!(inode->i_opflags & IOP_NOTHASHED) && !inode_unhashed(inode))
  But whatever... The patch looks good. You can add:
Reviewed-by: Jan Kara <jack@suse.cz>

>  		__remove_inode_hash(inode);
>  }

								Honza
-- 
Jan Kara <jack@suse.cz>
SUSE Labs, CR