public inbox for linux-xfs@vger.kernel.org
 help / color / mirror / Atom feed
From: Catherine Hoang <catherine.hoang@oracle.com>
To: stable@vger.kernel.org
Cc: linux-xfs@vger.kernel.org
Subject: [PATCH 6.6 19/21] xfs: fix freeing speculative preallocations for preallocated files
Date: Tue, 15 Oct 2024 17:11:24 -0700	[thread overview]
Message-ID: <20241016001126.3256-20-catherine.hoang@oracle.com> (raw)
In-Reply-To: <20241016001126.3256-1-catherine.hoang@oracle.com>

From: Christoph Hellwig <hch@lst.de>

commit 610b29161b0aa9feb59b78dc867553274f17fb01 upstream.

xfs_can_free_eofblocks returns false for files that have persistent
preallocations unless the force flag is passed and there are delayed
blocks.  This means it won't free delalloc reservations for files
with persistent preallocations unless the force flag is set, and it
will also free the persistent preallocations if the force flag is
set and the file happens to have delayed allocations.

Both of these are bad, so do away with the force flag and always free
only post-EOF delayed allocations for files with the XFS_DIFLAG_PREALLOC
or APPEND flags set.

Signed-off-by: Christoph Hellwig <hch@lst.de>
Reviewed-by: Darrick J. Wong <djwong@kernel.org>
Signed-off-by: Chandan Babu R <chandanbabu@kernel.org>
Signed-off-by: Catherine Hoang <catherine.hoang@oracle.com>
Acked-by: Darrick J. Wong <djwong@kernel.org>
---
 fs/xfs/xfs_bmap_util.c | 30 ++++++++++++++++++++++--------
 fs/xfs/xfs_bmap_util.h |  2 +-
 fs/xfs/xfs_icache.c    |  2 +-
 fs/xfs/xfs_inode.c     | 14 ++++----------
 4 files changed, 28 insertions(+), 20 deletions(-)

diff --git a/fs/xfs/xfs_bmap_util.c b/fs/xfs/xfs_bmap_util.c
index 4a7d1a1b67a3..f9d72d8e3c35 100644
--- a/fs/xfs/xfs_bmap_util.c
+++ b/fs/xfs/xfs_bmap_util.c
@@ -636,13 +636,11 @@ xfs_bmap_punch_delalloc_range(
 
 /*
  * Test whether it is appropriate to check an inode for and free post EOF
- * blocks. The 'force' parameter determines whether we should also consider
- * regular files that are marked preallocated or append-only.
+ * blocks.
  */
 bool
 xfs_can_free_eofblocks(
-	struct xfs_inode	*ip,
-	bool			force)
+	struct xfs_inode	*ip)
 {
 	struct xfs_bmbt_irec	imap;
 	struct xfs_mount	*mp = ip->i_mount;
@@ -676,11 +674,11 @@ xfs_can_free_eofblocks(
 		return false;
 
 	/*
-	 * Do not free real preallocated or append-only files unless the file
-	 * has delalloc blocks and we are forced to remove them.
+	 * Only free real extents for inodes with persistent preallocations or
+	 * the append-only flag.
 	 */
 	if (ip->i_diflags & (XFS_DIFLAG_PREALLOC | XFS_DIFLAG_APPEND))
-		if (!force || ip->i_delayed_blks == 0)
+		if (ip->i_delayed_blks == 0)
 			return false;
 
 	/*
@@ -734,6 +732,22 @@ xfs_free_eofblocks(
 	/* Wait on dio to ensure i_size has settled. */
 	inode_dio_wait(VFS_I(ip));
 
+	/*
+	 * For preallocated files only free delayed allocations.
+	 *
+	 * Note that this means we also leave speculative preallocations in
+	 * place for preallocated files.
+	 */
+	if (ip->i_diflags & (XFS_DIFLAG_PREALLOC | XFS_DIFLAG_APPEND)) {
+		if (ip->i_delayed_blks) {
+			xfs_bmap_punch_delalloc_range(ip,
+				round_up(XFS_ISIZE(ip), mp->m_sb.sb_blocksize),
+				LLONG_MAX);
+		}
+		xfs_inode_clear_eofblocks_tag(ip);
+		return 0;
+	}
+
 	error = xfs_trans_alloc(mp, &M_RES(mp)->tr_itruncate, 0, 0, 0, &tp);
 	if (error) {
 		ASSERT(xfs_is_shutdown(mp));
@@ -1048,7 +1062,7 @@ xfs_prepare_shift(
 	 * Trim eofblocks to avoid shifting uninitialized post-eof preallocation
 	 * into the accessible region of the file.
 	 */
-	if (xfs_can_free_eofblocks(ip, true)) {
+	if (xfs_can_free_eofblocks(ip)) {
 		error = xfs_free_eofblocks(ip);
 		if (error)
 			return error;
diff --git a/fs/xfs/xfs_bmap_util.h b/fs/xfs/xfs_bmap_util.h
index 6888078f5c31..1383019ccdb7 100644
--- a/fs/xfs/xfs_bmap_util.h
+++ b/fs/xfs/xfs_bmap_util.h
@@ -63,7 +63,7 @@ int	xfs_insert_file_space(struct xfs_inode *, xfs_off_t offset,
 				xfs_off_t len);
 
 /* EOF block manipulation functions */
-bool	xfs_can_free_eofblocks(struct xfs_inode *ip, bool force);
+bool	xfs_can_free_eofblocks(struct xfs_inode *ip);
 int	xfs_free_eofblocks(struct xfs_inode *ip);
 
 int	xfs_swap_extents(struct xfs_inode *ip, struct xfs_inode *tip,
diff --git a/fs/xfs/xfs_icache.c b/fs/xfs/xfs_icache.c
index db88f41c94c6..57a9f2317525 100644
--- a/fs/xfs/xfs_icache.c
+++ b/fs/xfs/xfs_icache.c
@@ -1149,7 +1149,7 @@ xfs_inode_free_eofblocks(
 	}
 	*lockflags |= XFS_IOLOCK_EXCL;
 
-	if (xfs_can_free_eofblocks(ip, false))
+	if (xfs_can_free_eofblocks(ip))
 		return xfs_free_eofblocks(ip);
 
 	/* inode could be preallocated or append-only */
diff --git a/fs/xfs/xfs_inode.c b/fs/xfs/xfs_inode.c
index 8bfde8fce6e2..7aa73855fab6 100644
--- a/fs/xfs/xfs_inode.c
+++ b/fs/xfs/xfs_inode.c
@@ -1469,7 +1469,7 @@ xfs_release(
 	if (!xfs_ilock_nowait(ip, XFS_IOLOCK_EXCL))
 		return 0;
 
-	if (xfs_can_free_eofblocks(ip, false)) {
+	if (xfs_can_free_eofblocks(ip)) {
 		/*
 		 * Check if the inode is being opened, written and closed
 		 * frequently and we have delayed allocation blocks outstanding
@@ -1685,15 +1685,13 @@ xfs_inode_needs_inactive(
 
 	/*
 	 * This file isn't being freed, so check if there are post-eof blocks
-	 * to free.  @force is true because we are evicting an inode from the
-	 * cache.  Post-eof blocks must be freed, lest we end up with broken
-	 * free space accounting.
+	 * to free.
 	 *
 	 * Note: don't bother with iolock here since lockdep complains about
 	 * acquiring it in reclaim context. We have the only reference to the
 	 * inode at this point anyways.
 	 */
-	return xfs_can_free_eofblocks(ip, true);
+	return xfs_can_free_eofblocks(ip);
 }
 
 /*
@@ -1741,15 +1739,11 @@ xfs_inactive(
 
 	if (VFS_I(ip)->i_nlink != 0) {
 		/*
-		 * force is true because we are evicting an inode from the
-		 * cache. Post-eof blocks must be freed, lest we end up with
-		 * broken free space accounting.
-		 *
 		 * Note: don't bother with iolock here since lockdep complains
 		 * about acquiring it in reclaim context. We have the only
 		 * reference to the inode at this point anyways.
 		 */
-		if (xfs_can_free_eofblocks(ip, true))
+		if (xfs_can_free_eofblocks(ip))
 			error = xfs_free_eofblocks(ip);
 
 		goto out;
-- 
2.39.3


  parent reply	other threads:[~2024-10-16  0:12 UTC|newest]

Thread overview: 23+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-10-16  0:11 [PATCH 6.6 00/21] xfs backports for 6.6.y (from 6.10) Catherine Hoang
2024-10-16  0:11 ` [PATCH 6.6 01/21] xfs: fix error returns from xfs_bmapi_write Catherine Hoang
2024-10-16  0:11 ` [PATCH 6.6 02/21] xfs: fix xfs_bmap_add_extent_delay_real for partial conversions Catherine Hoang
2024-10-16  0:11 ` [PATCH 6.6 03/21] xfs: remove a racy if_bytes check in xfs_reflink_end_cow_extent Catherine Hoang
2024-10-16  0:11 ` [PATCH 6.6 04/21] xfs: require XFS_SB_FEAT_INCOMPAT_LOG_XATTRS for attr log intent item recovery Catherine Hoang
2024-10-16  0:11 ` [PATCH 6.6 05/21] xfs: check opcode and iovec count match in xlog_recover_attri_commit_pass2 Catherine Hoang
2024-10-16  0:11 ` [PATCH 6.6 06/21] xfs: fix missing check for invalid attr flags Catherine Hoang
2024-10-16  0:11 ` [PATCH 6.6 07/21] xfs: check shortform attr entry flags specifically Catherine Hoang
2024-10-16  0:11 ` [PATCH 6.6 08/21] xfs: validate recovered name buffers when recovering xattr items Catherine Hoang
2024-10-16  0:11 ` [PATCH 6.6 09/21] xfs: enforce one namespace per attribute Catherine Hoang
2024-10-16  0:11 ` [PATCH 6.6 10/21] xfs: revert commit 44af6c7e59b12 Catherine Hoang
2024-10-16  0:11 ` [PATCH 6.6 11/21] xfs: use dontcache for grabbing inodes during scrub Catherine Hoang
2024-10-16  0:11 ` [PATCH 6.6 12/21] xfs: match lock mode in xfs_buffered_write_iomap_begin() Catherine Hoang
2024-10-16  0:11 ` [PATCH 6.6 13/21] xfs: make the seq argument to xfs_bmapi_convert_delalloc() optional Catherine Hoang
2024-10-16  0:11 ` [PATCH 6.6 14/21] xfs: make xfs_bmapi_convert_delalloc() to allocate the target offset Catherine Hoang
2024-10-16  0:11 ` [PATCH 6.6 15/21] xfs: convert delayed extents to unwritten when zeroing post eof blocks Catherine Hoang
2024-10-16  0:11 ` [PATCH 6.6 16/21] xfs: allow symlinks with short remote targets Catherine Hoang
2024-10-16  0:11 ` [PATCH 6.6 17/21] xfs: make sure sb_fdblocks is non-negative Catherine Hoang
2024-10-16  0:11 ` [PATCH 6.6 18/21] xfs: fix unlink vs cluster buffer instantiation race Catherine Hoang
2024-10-16  0:11 ` Catherine Hoang [this message]
2024-10-16  0:11 ` [PATCH 6.6 20/21] xfs: restrict when we try to align cow fork delalloc to cowextsz hints Catherine Hoang
2024-10-16  0:11 ` [PATCH 6.6 21/21] xfs: allow unlinked symlinks and dirs with zero size Catherine Hoang
2024-10-18  8:44 ` [PATCH 6.6 00/21] xfs backports for 6.6.y (from 6.10) Greg KH

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=20241016001126.3256-20-catherine.hoang@oracle.com \
    --to=catherine.hoang@oracle.com \
    --cc=linux-xfs@vger.kernel.org \
    --cc=stable@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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox