From: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
To: stable@vger.kernel.org
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
patches@lists.linux.dev, xfs-stable@lists.linux.dev,
Brian Foster <bfoster@redhat.com>,
"Darrick J. Wong" <djwong@kernel.org>,
Carlos Maiolino <cem@kernel.org>,
Catherine Hoang <catherine.hoang@oracle.com>
Subject: [PATCH 6.6 007/140] xfs: dont free cowblocks from under dirty pagecache on unshare
Date: Mon, 24 Feb 2025 15:33:26 +0100 [thread overview]
Message-ID: <20250224142603.295057663@linuxfoundation.org> (raw)
In-Reply-To: <20250224142602.998423469@linuxfoundation.org>
6.6-stable review patch. If anyone has any objections, please let me know.
------------------
From: Brian Foster <bfoster@redhat.com>
commit 4390f019ad7866c3791c3d768d2ff185d89e8ebe upstream.
fallocate unshare mode explicitly breaks extent sharing. When a
command completes, it checks the data fork for any remaining shared
extents to determine whether the reflink inode flag and COW fork
preallocation can be removed. This logic doesn't consider in-core
pagecache and I/O state, however, which means we can unsafely remove
COW fork blocks that are still needed under certain conditions.
For example, consider the following command sequence:
xfs_io -fc "pwrite 0 1k" -c "reflink <file> 0 256k 1k" \
-c "pwrite 0 32k" -c "funshare 0 1k" <file>
This allocates a data block at offset 0, shares it, and then
overwrites it with a larger buffered write. The overwrite triggers
COW fork preallocation, 32 blocks by default, which maps the entire
32k write to delalloc in the COW fork. All but the shared block at
offset 0 remains hole mapped in the data fork. The unshare command
redirties and flushes the folio at offset 0, removing the only
shared extent from the inode. Since the inode no longer maps shared
extents, unshare purges the COW fork before the remaining 28k may
have written back.
This leaves dirty pagecache backed by holes, which writeback quietly
skips, thus leaving clean, non-zeroed pagecache over holes in the
file. To verify, fiemap shows holes in the first 32k of the file and
reads return different data across a remount:
$ xfs_io -c "fiemap -v" <file>
<file>:
EXT: FILE-OFFSET BLOCK-RANGE TOTAL FLAGS
...
1: [8..511]: hole 504
...
$ xfs_io -c "pread -v 4k 8" <file>
00001000: cd cd cd cd cd cd cd cd ........
$ umount <mnt>; mount <dev> <mnt>
$ xfs_io -c "pread -v 4k 8" <file>
00001000: 00 00 00 00 00 00 00 00 ........
To avoid this problem, make unshare follow the same rules used for
background cowblock scanning and never purge the COW fork for inodes
with dirty pagecache or in-flight I/O.
Fixes: 46afb0628b86347 ("xfs: only flush the unshared range in xfs_reflink_unshare")
Signed-off-by: Brian Foster <bfoster@redhat.com>
Reviewed-by: Darrick J. Wong <djwong@kernel.org>
Signed-off-by: Carlos Maiolino <cem@kernel.org>
Signed-off-by: Catherine Hoang <catherine.hoang@oracle.com>
Acked-by: Darrick J. Wong <djwong@kernel.org>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
---
fs/xfs/xfs_icache.c | 8 +-------
fs/xfs/xfs_reflink.c | 3 +++
fs/xfs/xfs_reflink.h | 19 +++++++++++++++++++
3 files changed, 23 insertions(+), 7 deletions(-)
--- a/fs/xfs/xfs_icache.c
+++ b/fs/xfs/xfs_icache.c
@@ -1271,13 +1271,7 @@ xfs_prep_free_cowblocks(
*/
if (!sync && inode_is_open_for_write(VFS_I(ip)))
return false;
- if ((VFS_I(ip)->i_state & I_DIRTY_PAGES) ||
- mapping_tagged(VFS_I(ip)->i_mapping, PAGECACHE_TAG_DIRTY) ||
- mapping_tagged(VFS_I(ip)->i_mapping, PAGECACHE_TAG_WRITEBACK) ||
- atomic_read(&VFS_I(ip)->i_dio_count))
- return false;
-
- return true;
+ return xfs_can_free_cowblocks(ip);
}
/*
--- a/fs/xfs/xfs_reflink.c
+++ b/fs/xfs/xfs_reflink.c
@@ -1600,6 +1600,9 @@ xfs_reflink_clear_inode_flag(
ASSERT(xfs_is_reflink_inode(ip));
+ if (!xfs_can_free_cowblocks(ip))
+ return 0;
+
error = xfs_reflink_inode_has_shared_extents(*tpp, ip, &needs_flag);
if (error || needs_flag)
return error;
--- a/fs/xfs/xfs_reflink.h
+++ b/fs/xfs/xfs_reflink.h
@@ -16,6 +16,25 @@ static inline bool xfs_is_cow_inode(stru
return xfs_is_reflink_inode(ip) || xfs_is_always_cow_inode(ip);
}
+/*
+ * Check whether it is safe to free COW fork blocks from an inode. It is unsafe
+ * to do so when an inode has dirty cache or I/O in-flight, even if no shared
+ * extents exist in the data fork, because outstanding I/O may target blocks
+ * that were speculatively allocated to the COW fork.
+ */
+static inline bool
+xfs_can_free_cowblocks(struct xfs_inode *ip)
+{
+ struct inode *inode = VFS_I(ip);
+
+ if ((inode->i_state & I_DIRTY_PAGES) ||
+ mapping_tagged(inode->i_mapping, PAGECACHE_TAG_DIRTY) ||
+ mapping_tagged(inode->i_mapping, PAGECACHE_TAG_WRITEBACK) ||
+ atomic_read(&inode->i_dio_count))
+ return false;
+ return true;
+}
+
extern int xfs_reflink_trim_around_shared(struct xfs_inode *ip,
struct xfs_bmbt_irec *irec, bool *shared);
int xfs_bmap_trim_cow(struct xfs_inode *ip, struct xfs_bmbt_irec *imap,
next prev parent reply other threads:[~2025-02-24 14:37 UTC|newest]
Thread overview: 24+ messages / expand[flat|nested] mbox.gz Atom feed top
[not found] <20250224142602.998423469@linuxfoundation.org>
2025-02-24 14:33 ` [PATCH 6.6 002/140] xfs: assert a valid limit in xfs_rtfind_forw Greg Kroah-Hartman
2025-02-24 14:33 ` [PATCH 6.6 003/140] xfs: validate inumber in xfs_iget Greg Kroah-Hartman
2025-02-24 14:33 ` [PATCH 6.6 004/140] xfs: fix a sloppy memory handling bug in xfs_iroot_realloc Greg Kroah-Hartman
2025-02-24 14:33 ` [PATCH 6.6 005/140] xfs: fix a typo Greg Kroah-Hartman
2025-02-24 14:33 ` [PATCH 6.6 006/140] xfs: skip background cowblock trims on inodes open for write Greg Kroah-Hartman
2025-02-24 14:33 ` Greg Kroah-Hartman [this message]
2025-02-24 14:33 ` [PATCH 6.6 008/140] xfs: merge xfs_attr_leaf_try_add into xfs_attr_leaf_addname Greg Kroah-Hartman
2025-02-24 14:33 ` [PATCH 6.6 009/140] xfs: return bool from xfs_attr3_leaf_add Greg Kroah-Hartman
2025-02-24 14:33 ` [PATCH 6.6 010/140] xfs: distinguish extra split from real ENOSPC from xfs_attr3_leaf_split Greg Kroah-Hartman
2025-02-24 14:33 ` [PATCH 6.6 011/140] xfs: distinguish extra split from real ENOSPC from xfs_attr_node_try_addname Greg Kroah-Hartman
2025-02-24 14:33 ` [PATCH 6.6 012/140] xfs: fold xfs_bmap_alloc_userdata into xfs_bmapi_allocate Greg Kroah-Hartman
2025-02-24 14:33 ` [PATCH 6.6 013/140] xfs: dont ifdef around the exact minlen allocations Greg Kroah-Hartman
2025-02-24 14:33 ` [PATCH 6.6 014/140] xfs: call xfs_bmap_exact_minlen_extent_alloc from xfs_bmap_btalloc Greg Kroah-Hartman
2025-02-24 14:33 ` [PATCH 6.6 015/140] xfs: support lowmode allocations in xfs_bmap_exact_minlen_extent_alloc Greg Kroah-Hartman
2025-02-24 14:33 ` [PATCH 6.6 016/140] xfs: Use try_cmpxchg() in xlog_cil_insert_pcp_aggregate() Greg Kroah-Hartman
2025-02-24 14:33 ` [PATCH 6.6 017/140] xfs: Remove empty declartion in header file Greg Kroah-Hartman
2025-02-24 14:33 ` [PATCH 6.6 018/140] xfs: pass the exact range to initialize to xfs_initialize_perag Greg Kroah-Hartman
2025-02-24 14:33 ` [PATCH 6.6 019/140] xfs: update the file system geometry after recoverying superblock buffers Greg Kroah-Hartman
2025-02-24 14:33 ` [PATCH 6.6 020/140] xfs: error out when a superblock buffer update reduces the agcount Greg Kroah-Hartman
2025-02-24 14:33 ` [PATCH 6.6 021/140] xfs: dont use __GFP_RETRY_MAYFAIL in xfs_initialize_perag Greg Kroah-Hartman
2025-02-24 14:33 ` [PATCH 6.6 022/140] xfs: update the pag for the last AG at recovery time Greg Kroah-Hartman
2025-02-24 14:33 ` [PATCH 6.6 023/140] xfs: Reduce unnecessary searches when searching for the best extents Greg Kroah-Hartman
2025-02-24 14:33 ` [PATCH 6.6 024/140] xfs: streamline xfs_filestream_pick_ag Greg Kroah-Hartman
2025-02-24 14:33 ` [PATCH 6.6 025/140] xfs: Check for delayed allocations before setting extsize Greg Kroah-Hartman
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=20250224142603.295057663@linuxfoundation.org \
--to=gregkh@linuxfoundation.org \
--cc=bfoster@redhat.com \
--cc=catherine.hoang@oracle.com \
--cc=cem@kernel.org \
--cc=djwong@kernel.org \
--cc=patches@lists.linux.dev \
--cc=stable@vger.kernel.org \
--cc=xfs-stable@lists.linux.dev \
/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