Re: [PATCH 08/12] xfs: fix inode ag walk predicate function return values

[Brian Foster <bfoster@redhat.com>]

On Thu, May 21, 2020 at 07:54:19PM -0700, Darrick J. Wong wrote:
> From: Darrick J. Wong <darrick.wong@oracle.com>
> 
> There are a number of predicate functions that help the incore inode
> walking code decide if we really want to apply the iteration function to
> the inode.  These are boolean decisions, so change the return types to
> boolean to match.
> 
> Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
> Reviewed-by: Christoph Hellwig <hch@lst.de>
> ---

Reviewed-by: Brian Foster <bfoster@redhat.com>

>  fs/xfs/xfs_icache.c |   39 ++++++++++++++++++++++-----------------
>  1 file changed, 22 insertions(+), 17 deletions(-)
> 
> 
> diff --git a/fs/xfs/xfs_icache.c b/fs/xfs/xfs_icache.c
> index 3a45ec948c1a..31d85cc4bd8b 100644
> --- a/fs/xfs/xfs_icache.c
> +++ b/fs/xfs/xfs_icache.c
> @@ -741,7 +741,12 @@ xfs_icache_inode_is_allocated(
>   */
>  #define XFS_LOOKUP_BATCH	32
>  
> -STATIC int
> +/*
> + * Decide if the given @ip is eligible to be a part of the inode walk, and
> + * grab it if so.  Returns true if it's ready to go or false if we should just
> + * ignore it.
> + */
> +STATIC bool
>  xfs_inode_ag_walk_grab(
>  	struct xfs_inode	*ip,
>  	int			flags)
> @@ -772,18 +777,18 @@ xfs_inode_ag_walk_grab(
>  
>  	/* nothing to sync during shutdown */
>  	if (XFS_FORCED_SHUTDOWN(ip->i_mount))
> -		return -EFSCORRUPTED;
> +		return false;
>  
>  	/* If we can't grab the inode, it must on it's way to reclaim. */
>  	if (!igrab(inode))
> -		return -ENOENT;
> +		return false;
>  
>  	/* inode is valid */
> -	return 0;
> +	return true;
>  
>  out_unlock_noent:
>  	spin_unlock(&ip->i_flags_lock);
> -	return -ENOENT;
> +	return false;
>  }
>  
>  STATIC int
> @@ -835,7 +840,7 @@ xfs_inode_ag_walk(
>  		for (i = 0; i < nr_found; i++) {
>  			struct xfs_inode *ip = batch[i];
>  
> -			if (done || xfs_inode_ag_walk_grab(ip, iter_flags))
> +			if (done || !xfs_inode_ag_walk_grab(ip, iter_flags))
>  				batch[i] = NULL;
>  
>  			/*
> @@ -1392,48 +1397,48 @@ xfs_reclaim_inodes_count(
>  	return reclaimable;
>  }
>  
> -STATIC int
> +STATIC bool
>  xfs_inode_match_id(
>  	struct xfs_inode	*ip,
>  	struct xfs_eofblocks	*eofb)
>  {
>  	if ((eofb->eof_flags & XFS_EOF_FLAGS_UID) &&
>  	    !uid_eq(VFS_I(ip)->i_uid, eofb->eof_uid))
> -		return 0;
> +		return false;
>  
>  	if ((eofb->eof_flags & XFS_EOF_FLAGS_GID) &&
>  	    !gid_eq(VFS_I(ip)->i_gid, eofb->eof_gid))
> -		return 0;
> +		return false;
>  
>  	if ((eofb->eof_flags & XFS_EOF_FLAGS_PRID) &&
>  	    ip->i_d.di_projid != eofb->eof_prid)
> -		return 0;
> +		return false;
>  
> -	return 1;
> +	return true;
>  }
>  
>  /*
>   * A union-based inode filtering algorithm. Process the inode if any of the
>   * criteria match. This is for global/internal scans only.
>   */
> -STATIC int
> +STATIC bool
>  xfs_inode_match_id_union(
>  	struct xfs_inode	*ip,
>  	struct xfs_eofblocks	*eofb)
>  {
>  	if ((eofb->eof_flags & XFS_EOF_FLAGS_UID) &&
>  	    uid_eq(VFS_I(ip)->i_uid, eofb->eof_uid))
> -		return 1;
> +		return true;
>  
>  	if ((eofb->eof_flags & XFS_EOF_FLAGS_GID) &&
>  	    gid_eq(VFS_I(ip)->i_gid, eofb->eof_gid))
> -		return 1;
> +		return true;
>  
>  	if ((eofb->eof_flags & XFS_EOF_FLAGS_PRID) &&
>  	    ip->i_d.di_projid == eofb->eof_prid)
> -		return 1;
> +		return true;
>  
> -	return 0;
> +	return false;
>  }
>  
>  /*
> @@ -1446,7 +1451,7 @@ xfs_inode_matches_eofb(
>  	struct xfs_inode	*ip,
>  	struct xfs_eofblocks	*eofb)
>  {
> -	int			match;
> +	bool			match;
>  
>  	if (!eofb)
>  		return true;
>