Re: [PATCH 04/10] xfs: don't bother returning errors from xfs_file_release

["Darrick J. Wong" <djwong@kernel.org>]

On Sun, Jun 23, 2024 at 07:34:49AM +0200, Christoph Hellwig wrote:
> While ->release returns int, the only caller ignores the return value.
> As we're only doing cleanup work there isn't much of a point in
> return a value to start with, so just document the situation instead.
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> ---
>  fs/xfs/xfs_file.c | 18 +++++++++---------
>  1 file changed, 9 insertions(+), 9 deletions(-)
> 
> diff --git a/fs/xfs/xfs_file.c b/fs/xfs/xfs_file.c
> index d39d0ea522d1c2..7b91cbab80da55 100644
> --- a/fs/xfs/xfs_file.c
> +++ b/fs/xfs/xfs_file.c
> @@ -1186,6 +1186,10 @@ xfs_dir_open(
>  	return error;
>  }
>  
> +/*
> + * Don't bother propagating errors.  We're just doing cleanup, and the caller
> + * ignores the return value anyway.

Shouldn't we drop the int return from the function declaration, then?

(Is that also a cleanup that's you're working on?)

--D

> + */
>  STATIC int
>  xfs_file_release(
>  	struct inode		*inode,
> @@ -1193,7 +1197,6 @@ xfs_file_release(
>  {
>  	struct xfs_inode	*ip = XFS_I(inode);
>  	struct xfs_mount	*mp = ip->i_mount;
> -	int			error;
>  
>  	/* If this is a read-only mount, don't generate I/O */
>  	if (xfs_is_readonly(mp))
> @@ -1211,11 +1214,8 @@ xfs_file_release(
>  	if (!xfs_is_shutdown(mp) &&
>  	    xfs_iflags_test_and_clear(ip, XFS_ITRUNCATED)) {
>  		xfs_iflags_clear(ip, XFS_IDIRTY_RELEASE);
> -		if (ip->i_delayed_blks > 0) {
> -			error = filemap_flush(inode->i_mapping);
> -			if (error)
> -				return error;
> -		}
> +		if (ip->i_delayed_blks > 0)
> +			filemap_flush(inode->i_mapping);
>  	}
>  
>  	/*
> @@ -1249,14 +1249,14 @@ xfs_file_release(
>  			 * dirty close we will still remove the speculative
>  			 * allocation, but after that we will leave it in place.
>  			 */
> -			error = xfs_free_eofblocks(ip);
> -			if (!error && ip->i_delayed_blks)
> +			xfs_free_eofblocks(ip);
> +			if (ip->i_delayed_blks)
>  				xfs_iflags_set(ip, XFS_IDIRTY_RELEASE);
>  		}
>  		xfs_iunlock(ip, XFS_IOLOCK_EXCL);
>  	}
>  
> -	return error;
> +	return 0;
>  }
>  
>  STATIC int
> -- 
> 2.43.0
> 
>