Skip to content

Commit 089558b

Browse files
author
Darrick J. Wong
committed
xfs: remove all COW fork extents when remounting readonly
As part of multiple customer escalations due to file data corruption after copy on write operations, I wrote some fstests that use fsstress to hammer on COW to shake things loose. Regrettably, I caught some filesystem shutdowns due to incorrect rmap operations with the following loop: mount <filesystem> # (0) fsstress <run only readonly ops> & # (1) while true; do fsstress <run all ops> mount -o remount,ro # (2) fsstress <run only readonly ops> mount -o remount,rw # (3) done When (2) happens, notice that (1) is still running. xfs_remount_ro will call xfs_blockgc_stop to walk the inode cache to free all the COW extents, but the blockgc mechanism races with (1)'s reader threads to take IOLOCKs and loses, which means that it doesn't clean them all out. Call such a file (A). When (3) happens, xfs_remount_rw calls xfs_reflink_recover_cow, which walks the ondisk refcount btree and frees any COW extent that it finds. This function does not check the inode cache, which means that incore COW forks of inode (A) is now inconsistent with the ondisk metadata. If one of those former COW extents are allocated and mapped into another file (B) and someone triggers a COW to the stale reservation in (A), A's dirty data will be written into (B) and once that's done, those blocks will be transferred to (A)'s data fork without bumping the refcount. The results are catastrophic -- file (B) and the refcount btree are now corrupt. Solve this race by forcing the xfs_blockgc_free_space to run synchronously, which causes xfs_icwalk to return to inodes that were skipped because the blockgc code couldn't take the IOLOCK. This is safe to do here because the VFS has already prohibited new writer threads. Fixes: 10ddf64 ("xfs: remove leftover CoW reservations when remounting ro") Signed-off-by: Darrick J. Wong <[email protected]> Reviewed-by: Dave Chinner <[email protected]> Reviewed-by: Chandan Babu R <[email protected]>
1 parent e445976 commit 089558b

File tree

1 file changed

+11
-3
lines changed

1 file changed

+11
-3
lines changed

fs/xfs/xfs_super.c

Lines changed: 11 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1765,16 +1765,24 @@ static int
17651765
xfs_remount_ro(
17661766
struct xfs_mount *mp)
17671767
{
1768-
int error;
1768+
struct xfs_icwalk icw = {
1769+
.icw_flags = XFS_ICWALK_FLAG_SYNC,
1770+
};
1771+
int error;
17691772

17701773
/*
17711774
* Cancel background eofb scanning so it cannot race with the final
17721775
* log force+buftarg wait and deadlock the remount.
17731776
*/
17741777
xfs_blockgc_stop(mp);
17751778

1776-
/* Get rid of any leftover CoW reservations... */
1777-
error = xfs_blockgc_free_space(mp, NULL);
1779+
/*
1780+
* Clear out all remaining COW staging extents and speculative post-EOF
1781+
* preallocations so that we don't leave inodes requiring inactivation
1782+
* cleanups during reclaim on a read-only mount. We must process every
1783+
* cached inode, so this requires a synchronous cache scan.
1784+
*/
1785+
error = xfs_blockgc_free_space(mp, &icw);
17781786
if (error) {
17791787
xfs_force_shutdown(mp, SHUTDOWN_CORRUPT_INCORE);
17801788
return error;

0 commit comments

Comments
 (0)