diff mbox series

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

Message ID 20240623053532.857496-3-hch@lst.de (mailing list archive)
State New
Headers show
Series [01/10] xfs: fix freeing speculative preallocations for preallocated files | expand

Commit Message

Christoph Hellwig June 23, 2024, 5:34 a.m. UTC 
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(-)

Comments

Darrick J. Wong June 24, 2024, 3:34 p.m. UTC | #1 
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
> 
>
Christoph Hellwig June 24, 2024, 3:50 p.m. UTC | #2 
On Mon, Jun 24, 2024 at 08:34:59AM -0700, Darrick J. Wong wrote:
> > -	if (!S_ISREG(VFS_I(ip)->i_mode) || (VFS_I(ip)->i_mode == 0))
> 
> How would we encounter !i_mode regular files being released?

We can't.  If that code ever made any sense than in ancient pre-history
in IRIX.

> 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?

No.

> 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;
> 	}

I wouldn't even bother with that.
diff mbox series

Patch

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