On Sun, Jun 23, 2024 at 07:34:48AM +0200, Christoph Hellwig wrote:
> Currently f_op->release is split in not very obvious ways. Fix that by
> folding xfs_release into xfs_file_release.
>
> Signed-off-by: Christoph Hellwig <hch@xxxxxx>
Pretty straightforward hoist, looks like
Reviewed-by: Darrick J. Wong <djwong@xxxxxxxxxx>
--D
> ---
> fs/xfs/xfs_file.c | 71 +++++++++++++++++++++++++++++++++++++++--
> fs/xfs/xfs_inode.c | 79 ----------------------------------------------
> fs/xfs/xfs_inode.h | 1 -
> 3 files changed, 68 insertions(+), 83 deletions(-)
>
> diff --git a/fs/xfs/xfs_file.c b/fs/xfs/xfs_file.c
> index b240ea5241dc9d..d39d0ea522d1c2 100644
> --- a/fs/xfs/xfs_file.c
> +++ b/fs/xfs/xfs_file.c
> @@ -1188,10 +1188,75 @@ xfs_dir_open(
>
> STATIC int
> xfs_file_release(
> - struct inode *inode,
> - struct file *filp)
> + struct inode *inode,
> + struct file *file)
> {
> - return xfs_release(XFS_I(inode));
> + 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))
> + return 0;
> +
> + /*
> + * If we previously truncated this file and removed old data in the
> + * process, we want to initiate "early" writeout on the last close.
> + * This is an attempt to combat the notorious NULL files problem which
> + * is particularly noticeable from a truncate down, buffered (re-)write
> + * (delalloc), followed by a crash. What we are effectively doing here
> + * is significantly reducing the time window where we'd otherwise be
> + * exposed to that problem.
> + */
> + 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;
> + }
> + }
> +
> + /*
> + * XFS aggressively preallocates post-EOF space to generate contiguous
> + * allocations for writers that append to the end of the file and we
> + * try to free these when an open file context is released.
> + *
> + * There is no point in freeing blocks here for open but unlinked files
> + * as they will be taken care of by the inactivation path soon.
> + *
> + * If we can't get the iolock just skip truncating the blocks past EOF
> + * because we could deadlock with the mmap_lock otherwise. We'll get
> + * another chance to drop them once the last reference to the inode is
> + * dropped, so we'll never leak blocks permanently.
> + */
> + if (inode->i_nlink && xfs_ilock_nowait(ip, XFS_IOLOCK_EXCL)) {
> + if (xfs_can_free_eofblocks(ip) &&
> + !xfs_iflags_test(ip, XFS_IDIRTY_RELEASE)) {
> + /*
> + * Check if the inode is being opened, written and
> + * closed frequently and we have delayed allocation
> + * blocks outstanding (e.g. streaming writes from the
> + * NFS server), truncating the blocks past EOF will
> + * cause fragmentation to occur.
> + *
> + * In this case don't do the truncation, but we have to
> + * be careful how we detect this case. Blocks beyond EOF
> + * show up as i_delayed_blks even when the inode is
> + * clean, so we need to truncate them away first before
> + * checking for a dirty release. Hence on the first
> + * 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_iflags_set(ip, XFS_IDIRTY_RELEASE);
> + }
> + xfs_iunlock(ip, XFS_IOLOCK_EXCL);
> + }
> +
> + return error;
> }
>
> STATIC int
> diff --git a/fs/xfs/xfs_inode.c b/fs/xfs/xfs_inode.c
> index 9a9340aebe9d8a..fe4906a08665ee 100644
> --- a/fs/xfs/xfs_inode.c
> +++ b/fs/xfs/xfs_inode.c
> @@ -1545,85 +1545,6 @@ xfs_itruncate_extents_flags(
> return error;
> }
>
> -int
> -xfs_release(
> - xfs_inode_t *ip)
> -{
> - xfs_mount_t *mp = ip->i_mount;
> - int error = 0;
> -
> - /* If this is a read-only mount, don't do this (would generate I/O) */
> - if (xfs_is_readonly(mp))
> - return 0;
> -
> - if (!xfs_is_shutdown(mp)) {
> - int truncated;
> -
> - /*
> - * If we previously truncated this file and removed old data
> - * in the process, we want to initiate "early" writeout on
> - * the last close. This is an attempt to combat the notorious
> - * NULL files problem which is particularly noticeable from a
> - * truncate down, buffered (re-)write (delalloc), followed by
> - * a crash. What we are effectively doing here is
> - * significantly reducing the time window where we'd otherwise
> - * be exposed to that problem.
> - */
> - truncated = xfs_iflags_test_and_clear(ip, XFS_ITRUNCATED);
> - if (truncated) {
> - xfs_iflags_clear(ip, XFS_IDIRTY_RELEASE);
> - if (ip->i_delayed_blks > 0) {
> - error = filemap_flush(VFS_I(ip)->i_mapping);
> - if (error)
> - return error;
> - }
> - }
> - }
> -
> - if (VFS_I(ip)->i_nlink == 0)
> - return 0;
> -
> - /*
> - * If we can't get the iolock just skip truncating the blocks past EOF
> - * because we could deadlock with the mmap_lock otherwise. We'll get
> - * another chance to drop them once the last reference to the inode is
> - * dropped, so we'll never leak blocks permanently.
> - */
> - if (!xfs_ilock_nowait(ip, XFS_IOLOCK_EXCL))
> - return 0;
> -
> - if (xfs_can_free_eofblocks(ip)) {
> - /*
> - * Check if the inode is being opened, written and closed
> - * frequently and we have delayed allocation blocks outstanding
> - * (e.g. streaming writes from the NFS server), truncating the
> - * blocks past EOF will cause fragmentation to occur.
> - *
> - * In this case don't do the truncation, but we have to be
> - * careful how we detect this case. Blocks beyond EOF show up as
> - * i_delayed_blks even when the inode is clean, so we need to
> - * truncate them away first before checking for a dirty release.
> - * Hence on the first dirty close we will still remove the
> - * speculative allocation, but after that we will leave it in
> - * place.
> - */
> - if (xfs_iflags_test(ip, XFS_IDIRTY_RELEASE))
> - goto out_unlock;
> -
> - error = xfs_free_eofblocks(ip);
> - if (error)
> - goto out_unlock;
> -
> - /* delalloc blocks after truncation means it really is dirty */
> - if (ip->i_delayed_blks)
> - xfs_iflags_set(ip, XFS_IDIRTY_RELEASE);
> - }
> -
> -out_unlock:
> - xfs_iunlock(ip, XFS_IOLOCK_EXCL);
> - return error;
> -}
> -
> /*
> * Mark all the buffers attached to this directory stale. In theory we should
> * never be freeing a directory with any blocks at all, but this covers the
> diff --git a/fs/xfs/xfs_inode.h b/fs/xfs/xfs_inode.h
> index 292b90b5f2ac84..ae9851226f9913 100644
> --- a/fs/xfs/xfs_inode.h
> +++ b/fs/xfs/xfs_inode.h
> @@ -513,7 +513,6 @@ enum layout_break_reason {
> #define XFS_INHERIT_GID(pip) \
> (xfs_has_grpid((pip)->i_mount) || (VFS_I(pip)->i_mode & S_ISGID))
>
> -int xfs_release(struct xfs_inode *ip);
> int xfs_inactive(struct xfs_inode *ip);
> int xfs_lookup(struct xfs_inode *dp, const struct xfs_name *name,
> struct xfs_inode **ipp, struct xfs_name *ci_name);
> --
> 2.43.0
>
>
