Re: [PATCH 02/10] xfs: remove the i_mode check in xfs_release

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

On Sun, Jun 23, 2024 at 07:34:47AM +0200, Christoph Hellwig wrote:
> xfs_release is only called from xfs_file_release, which is wired up as
> the f_op->release handler for regular files only.
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> ---
>  fs/xfs/xfs_inode.c | 3 ---
>  1 file changed, 3 deletions(-)
> 
> diff --git a/fs/xfs/xfs_inode.c b/fs/xfs/xfs_inode.c
> index 38f946e3be2da3..9a9340aebe9d8a 100644
> --- a/fs/xfs/xfs_inode.c
> +++ b/fs/xfs/xfs_inode.c
> @@ -1552,9 +1552,6 @@ xfs_release(
>  	xfs_mount_t	*mp = ip->i_mount;
>  	int		error = 0;
>  
> -	if (!S_ISREG(VFS_I(ip)->i_mode) || (VFS_I(ip)->i_mode == 0))

How would we encounter !i_mode regular files being released?

If an open file's link count is incorrectly low, it can't get freed
until after all the open file descriptors have been released, right?
Or is there some other vector for this?

I'm wondering if this ought to be:

	if (XFS_IS_CORRUPT(mp, !VFS_I(ip)->i_mode)) {
		xfs_inode_mark_sick(ip);
		return -EFSCORRUPTED;
	}

--D

> -		return 0;
> -
>  	/* If this is a read-only mount, don't do this (would generate I/O) */
>  	if (xfs_is_readonly(mp))
>  		return 0;
> -- 
> 2.43.0
> 
>