All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Darrick J. Wong" <djwong@kernel.org>
To: djwong@kernel.org
Cc: Dave Chinner <dchinner@redhat.com>, linux-xfs@vger.kernel.org
Subject: [PATCH 2/3] xfs: don't free EOF blocks on read close
Date: Sun, 31 Dec 2023 14:00:28 -0800	[thread overview]
Message-ID: <170404854358.1769544.9714669653726216761.stgit@frogsfrogsfrogs> (raw)
In-Reply-To: <170404854320.1769544.582901935144092640.stgit@frogsfrogsfrogs>

From: Dave Chinner <dchinner@redhat.com>

When we have a workload that does open/read/close in parallel with other
allocation, the file becomes rapidly fragmented. This is due to close()
calling xfs_release() and removing the speculative preallocation beyond
EOF.

The existing open/*/close heuristic in xfs_release() does not catch this
as a sync writer does not leave delayed allocation blocks allocated on
the inode for later writeback that can be detected in xfs_release() and
hence XFS_IDIRTY_RELEASE never gets set.

In xfs_file_release(), we know more about the released file context, and
so we need to communicate some of the details to xfs_release() so it can
do the right thing here and skip EOF block truncation. This defers the
EOF block cleanup for synchronous write contexts to the background EOF
block cleaner which will clean up within a few minutes.

Before:

Test 1: sync write fragmentation counts

/mnt/scratch/file.0: 919
/mnt/scratch/file.1: 916
/mnt/scratch/file.2: 919
/mnt/scratch/file.3: 920
/mnt/scratch/file.4: 920
/mnt/scratch/file.5: 921
/mnt/scratch/file.6: 916
/mnt/scratch/file.7: 918

After:

Test 1: sync write fragmentation counts

/mnt/scratch/file.0: 24
/mnt/scratch/file.1: 24
/mnt/scratch/file.2: 11
/mnt/scratch/file.3: 24
/mnt/scratch/file.4: 3
/mnt/scratch/file.5: 24
/mnt/scratch/file.6: 24
/mnt/scratch/file.7: 23

Signed-off-by: Dave Chinner <dchinner@redhat.com>
[darrick: wordsmithing, fix commit message]
Signed-off-by: Darrick J. Wong <djwong@kernel.org>
Reviewed-by: Darrick J. Wong <djwong@kernel.org>
---
 fs/xfs/xfs_file.c  |   14 ++++++++++++--
 fs/xfs/xfs_inode.c |    9 +++++----
 fs/xfs/xfs_inode.h |    2 +-
 3 files changed, 18 insertions(+), 7 deletions(-)


diff --git a/fs/xfs/xfs_file.c b/fs/xfs/xfs_file.c
index ebdda286cb2a2..f2dd4daaa4e24 100644
--- a/fs/xfs/xfs_file.c
+++ b/fs/xfs/xfs_file.c
@@ -1367,12 +1367,22 @@ xfs_dir_open(
 	return error;
 }
 
+/*
+ * When we release the file, we don't want it to trim EOF blocks if it is a
+ * readonly context.  This avoids open/read/close workloads from removing
+ * EOF blocks that other writers depend upon to reduce fragmentation.
+ */
 STATIC int
 xfs_file_release(
 	struct inode	*inode,
-	struct file	*filp)
+	struct file	*file)
 {
-	return xfs_release(XFS_I(inode));
+	bool		free_eof_blocks = true;
+
+	if ((file->f_mode & (FMODE_WRITE | FMODE_READ)) == FMODE_READ)
+		free_eof_blocks = false;
+
+	return xfs_release(XFS_I(inode), free_eof_blocks);
 }
 
 STATIC int
diff --git a/fs/xfs/xfs_inode.c b/fs/xfs/xfs_inode.c
index dc0710661013f..3408804bee9b2 100644
--- a/fs/xfs/xfs_inode.c
+++ b/fs/xfs/xfs_inode.c
@@ -1137,10 +1137,11 @@ xfs_itruncate_extents_flags(
 
 int
 xfs_release(
-	xfs_inode_t	*ip)
+	struct xfs_inode	*ip,
+	bool			want_free_eofblocks)
 {
-	xfs_mount_t	*mp = ip->i_mount;
-	int		error = 0;
+	struct xfs_mount	*mp = ip->i_mount;
+	int			error = 0;
 
 	if (!S_ISREG(VFS_I(ip)->i_mode) || (VFS_I(ip)->i_mode == 0))
 		return 0;
@@ -1182,7 +1183,7 @@ xfs_release(
 	 * 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))
+	if (!want_free_eofblocks || !xfs_ilock_nowait(ip, XFS_IOLOCK_EXCL))
 		return 0;
 
 	if (xfs_can_free_eofblocks(ip, false)) {
diff --git a/fs/xfs/xfs_inode.h b/fs/xfs/xfs_inode.h
index df8197fe4cb82..2779a353b4618 100644
--- a/fs/xfs/xfs_inode.h
+++ b/fs/xfs/xfs_inode.h
@@ -525,7 +525,7 @@ 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_release(struct xfs_inode *ip, bool want_free_eofblocks);
 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);


  parent reply	other threads:[~2023-12-31 22:00 UTC|newest]

Thread overview: 49+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-12-31 18:25 [NYE PATCHRIVER 4/4] xfs: freespace defrag for online shrink Darrick J. Wong
2023-12-31 19:38 ` [PATCHSET 1/5] xfs: improve post-close eofblocks gc behavior Darrick J. Wong
2023-12-31 22:00   ` [PATCH 1/3] xfs: only free posteof blocks on first close Darrick J. Wong
2023-12-31 22:00   ` Darrick J. Wong [this message]
2023-12-31 22:00   ` [PATCH 3/3] xfs: Don't free EOF blocks on close when extent size hints are set Darrick J. Wong
2023-12-31 19:38 ` [PATCHSET RFC 2/5] xfs: noalloc allocation groups Darrick J. Wong
2023-12-31 22:00   ` [PATCH 1/5] xfs: track deferred ops statistics Darrick J. Wong
2023-12-31 22:01   ` [PATCH 2/5] xfs: whine to dmesg when we encounter errors Darrick J. Wong
2023-12-31 22:01   ` [PATCH 3/5] xfs: create a noalloc mode for allocation groups Darrick J. Wong
2023-12-31 22:01   ` [PATCH 4/5] xfs: enable userspace to hide an AG from allocation Darrick J. Wong
2023-12-31 22:02   ` [PATCH 5/5] xfs: apply noalloc mode to inode allocations too Darrick J. Wong
2023-12-31 19:39 ` [PATCHSET 3/5] xfs: report refcount information to userspace Darrick J. Wong
2023-12-31 22:02   ` [PATCH 1/1] xfs: export reference count " Darrick J. Wong
2023-12-31 19:39 ` [PATCHSET 4/5] xfs: defragment free space Darrick J. Wong
2023-12-31 22:02   ` [PATCH 1/2] xfs: capture the offset and length in fallocate tracepoints Darrick J. Wong
2023-12-31 22:02   ` [PATCH 2/2] xfs: add an ioctl to map free space into a file Darrick J. Wong
2023-12-31 19:39 ` [PATCHSET v2 5/5] xfs: aligned file data extent mappings Darrick J. Wong
2023-12-31 22:03   ` [PATCH 1/4] xfs: create a new inode flag to require extsize alignment of file data space Darrick J. Wong
2023-12-31 22:03   ` [PATCH 2/4] xfs: make file data allocations observe the 'forcealign' flag Darrick J. Wong
2024-01-16  9:26     ` John Garry
2023-12-31 22:03   ` [PATCH 3/4] xfs: support reflink with force align enabled Darrick J. Wong
2023-12-31 22:03   ` [PATCH 4/4] xfs: enable file data force-align feature Darrick J. Wong
2023-12-31 19:56 ` [PATCHSET RFC 1/3] xfsprogs: noalloc allocation groups Darrick J. Wong
2023-12-27 13:38   ` [PATCH 1/5] xfs: track deferred ops statistics Darrick J. Wong
2023-12-27 13:38   ` [PATCH 2/5] xfs: create a noalloc mode for allocation groups Darrick J. Wong
2023-12-27 13:38   ` [PATCH 3/5] xfs: enable userspace to hide an AG from allocation Darrick J. Wong
2023-12-27 13:39   ` [PATCH 4/5] xfs: apply noalloc mode to inode allocations too Darrick J. Wong
2023-12-27 13:39   ` [PATCH 5/5] xfs_io: enhance the aginfo command to control the noalloc flag Darrick J. Wong
2023-12-31 19:56 ` [PATCHSET 2/3] xfsprogs: report refcount information to userspace Darrick J. Wong
2023-12-27 13:39   ` [PATCH 1/2] xfs: export reference count " Darrick J. Wong
2023-12-27 13:39   ` [PATCH 2/2] xfs_io: dump reference count information Darrick J. Wong
2023-12-31 19:56 ` [PATCHSET 3/3] xfsprogs: defragment free space Darrick J. Wong
2023-12-27 13:40   ` [PATCH 01/10] xfs: add an ioctl to map free space into a file Darrick J. Wong
2023-12-27 13:40   ` [PATCH 02/10] xfs_io: support using XFS_IOC_MAP_FREESP to map free space Darrick J. Wong
2023-12-27 13:40   ` [PATCH 03/10] xfs_db: get and put blocks on the AGFL Darrick J. Wong
2023-12-27 13:41   ` [PATCH 04/10] xfs_spaceman: implement clearing free space Darrick J. Wong
2023-12-27 13:41   ` [PATCH 05/10] spaceman: physically move a regular inode Darrick J. Wong
2023-12-27 13:41   ` [PATCH 06/10] spaceman: find owners of space in an AG Darrick J. Wong
2023-12-27 13:41   ` [PATCH 07/10] xfs_spaceman: wrap radix tree accesses in find_owner.c Darrick J. Wong
2023-12-27 13:42   ` [PATCH 08/10] xfs_spaceman: port relocation structure to 32-bit systems Darrick J. Wong
2023-12-27 13:42   ` [PATCH 09/10] spaceman: relocate the contents of an AG Darrick J. Wong
2023-12-27 13:42   ` [PATCH 10/10] spaceman: move inodes with hardlinks Darrick J. Wong
2023-12-31 20:02 ` [PATCHSET 1/2] fstests: functional test for refcount reporting Darrick J. Wong
2023-12-27 14:06   ` [PATCH 1/2] xfs/122: update for the getfsrefs ioctl Darrick J. Wong
2023-12-27 14:06   ` [PATCH 2/2] xfs: test output of new FSREFCOUNTS ioctl Darrick J. Wong
2023-12-31 20:02 ` [PATCHSET 2/2] fstests: defragment free space Darrick J. Wong
2023-12-27 14:06   ` [PATCH 1/2] xfs/122: update for XFS_IOC_MAP_FREESP Darrick J. Wong
2023-12-27 14:07   ` [PATCH 2/2] xfs: test clearing of free space Darrick J. Wong
  -- strict thread matches above, loose matches on Subject: below --
2022-12-30 22:19 [PATCHSET 0/3] xfs: improve post-close eofblocks gc behavior Darrick J. Wong
2022-12-30 22:19 ` [PATCH 2/3] xfs: don't free EOF blocks on read close Darrick J. Wong

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=170404854358.1769544.9714669653726216761.stgit@frogsfrogsfrogs \
    --to=djwong@kernel.org \
    --cc=dchinner@redhat.com \
    --cc=linux-xfs@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.