Skip to content

Commit d84d6c1

Browse files
nszeteicmaiolino
nszetei
authored and
cmaiolino
committed
xfs: stop exchanging reflink flags when exchanging mappings
When an exchange covers both files from offset 0 to i_disk_size, xmi_can_exchange_reflink_flags() assumes every mapping moved and hands the reflink flag over to the other file. That assumption is wrong. A file can still own shared blocks past i_disk_size, e.g. after EXCHANGE_RANGE_TO_EOF sets a smaller size without unmapping anything. The flag is cleared anyway, and the next write to such a block happens in place instead of CoW, corrupting the other file that shares it. Commit a23eca8 ("xfs: fix exchange-range reflink flag clearing issue with INO1_WRITTEN") fixed one instance of this. Rather than add another special case, stop moving the flags. xfs_exchmaps_ensure_reflink() already sets the flag on both inodes before the exchange, so both just keep it. Nothing sets CLEAR_INO{1,2}_REFLINK anymore. Intents logged by older kernels may still carry them; recovery already masks them off, so they stay in the log format mask and are ignored. XFS_IOC_SWAPEXT has the same problem: it moves the reflink flag from one inode to the other along with the extents. Just removing that is not enough here: the exchange path flags both inodes up front in xfs_exchmaps_ensure_reflink(), but swapext has no such step, so the inode receiving the shared extents would be left without the flag. Set it on both inodes instead, and make sure both have a CoW fork, since an inode with the reflink flag is expected to have one. The flag now stays until FALLOC_FL_UNSHARE_RANGE, xfs_scrub, or a truncate to zero removes it. Fixes: 966ceaf ("xfs: create deferred log items for file mapping exchanges") Fixes: f0bc4d1 ("xfs: swap inode reflink flags when swapping inode extents") Cc: stable@vger.kernel.org # v6.10 Assisted-by: LLM Signed-off-by: Norbert Szetei <norbert@doyensec.com> Reviewed-by: Dave Chinner <dchinner@redhat.com> Reviewed-by: Darrick J. Wong <djwong@kernel.org> Signed-off-by: Carlos Maiolino <cem@kernel.org>
1 parent a5ebb76 commit d84d6c1

3 files changed

Lines changed: 10 additions & 89 deletions

File tree

‎fs/xfs/libxfs/xfs_exchmaps.c‎

Lines changed: 3 additions & 77 deletions
Original file line numberDiff line numberDiff line change
@@ -49,7 +49,7 @@ struct xfs_exchmaps_adjacent {
4949
.right2 = { .br_startblock = HOLESTARTBLOCK }, \
5050
}
5151

52-
/* Information to reset reflink flag / CoW fork state after an exchange. */
52+
/* Information to reset CoW fork state after an exchange. */
5353

5454
/*
5555
* If the reflink flag is set on either inode, make sure it has an incore CoW
@@ -127,9 +127,7 @@ xmi_has_more_exchange_work(const struct xfs_exchmaps_intent *xmi)
127127
static inline bool
128128
xmi_has_postop_work(const struct xfs_exchmaps_intent *xmi)
129129
{
130-
return xmi->xmi_flags & (XFS_EXCHMAPS_CLEAR_INO1_REFLINK |
131-
XFS_EXCHMAPS_CLEAR_INO2_REFLINK |
132-
__XFS_EXCHMAPS_INO2_SHORTFORM);
130+
return xmi->xmi_flags & __XFS_EXCHMAPS_INO2_SHORTFORM;
133131
}
134132

135133
/* Check all mappings to make sure we can actually exchange them. */
@@ -525,18 +523,6 @@ xfs_exchmaps_link_to_sf(
525523
return error;
526524
}
527525

528-
/* Clear the reflink flag after an exchange. */
529-
static inline void
530-
xfs_exchmaps_clear_reflink(
531-
struct xfs_trans *tp,
532-
struct xfs_inode *ip)
533-
{
534-
trace_xfs_reflink_unset_inode_flag(ip);
535-
536-
ip->i_diflags2 &= ~XFS_DIFLAG2_REFLINK;
537-
xfs_trans_log_inode(tp, ip, XFS_ILOG_CORE);
538-
}
539-
540526
/* Finish whatever work might come after an exchange operation. */
541527
static int
542528
xfs_exchmaps_do_postop_work(
@@ -557,16 +543,6 @@ xfs_exchmaps_do_postop_work(
557543
return error;
558544
}
559545

560-
if (xmi->xmi_flags & XFS_EXCHMAPS_CLEAR_INO1_REFLINK) {
561-
xfs_exchmaps_clear_reflink(tp, xmi->xmi_ip1);
562-
xmi->xmi_flags &= ~XFS_EXCHMAPS_CLEAR_INO1_REFLINK;
563-
}
564-
565-
if (xmi->xmi_flags & XFS_EXCHMAPS_CLEAR_INO2_REFLINK) {
566-
xfs_exchmaps_clear_reflink(tp, xmi->xmi_ip2);
567-
xmi->xmi_flags &= ~XFS_EXCHMAPS_CLEAR_INO2_REFLINK;
568-
}
569-
570546
return 0;
571547
}
572548

@@ -948,46 +924,13 @@ xfs_exchmaps_intent_destroy_cache(void)
948924
}
949925

950926
/*
951-
* Decide if we will exchange the reflink flags between the two files after the
952-
* exchange. The only time we want to do this is if we're exchanging all
953-
* mappings under EOF and the inode reflink flags have different states.
927+
* Allocate and initialize a new incore intent item from a request.
954928
*/
955-
static inline bool
956-
xmi_can_exchange_reflink_flags(
957-
const struct xfs_exchmaps_req *req,
958-
unsigned int reflink_state)
959-
{
960-
struct xfs_mount *mp = req->ip1->i_mount;
961-
962-
/*
963-
* The INO1_WRITTEN optimization can skip exchanging hole and
964-
* unwritten mappings, which means we cannot guarantee that all
965-
* shared extents actually moved to the other file. Clearing the
966-
* reflink flag of an inode that still holds shared extents breaks
967-
* the CoW write path, so refuse to exchange the flags in that case.
968-
*/
969-
if (req->flags & XFS_EXCHMAPS_INO1_WRITTEN)
970-
return false;
971-
972-
if (hweight32(reflink_state) != 1)
973-
return false;
974-
if (req->startoff1 != 0 || req->startoff2 != 0)
975-
return false;
976-
if (req->blockcount != XFS_B_TO_FSB(mp, req->ip1->i_disk_size))
977-
return false;
978-
if (req->blockcount != XFS_B_TO_FSB(mp, req->ip2->i_disk_size))
979-
return false;
980-
return true;
981-
}
982-
983-
984-
/* Allocate and initialize a new incore intent item from a request. */
985929
struct xfs_exchmaps_intent *
986930
xfs_exchmaps_init_intent(
987931
const struct xfs_exchmaps_req *req)
988932
{
989933
struct xfs_exchmaps_intent *xmi;
990-
unsigned int rs = 0;
991934

992935
xmi = kmem_cache_zalloc(xfs_exchmaps_intent_cache,
993936
GFP_NOFS | __GFP_NOFAIL);
@@ -1011,23 +954,6 @@ xfs_exchmaps_init_intent(
1011954
xmi->xmi_isize2 = req->ip1->i_disk_size;
1012955
}
1013956

1014-
/* Record the state of each inode's reflink flag before the op. */
1015-
if (xfs_is_reflink_inode(req->ip1))
1016-
rs |= 1;
1017-
if (xfs_is_reflink_inode(req->ip2))
1018-
rs |= 2;
1019-
1020-
/*
1021-
* Figure out if we're clearing the reflink flags (which effectively
1022-
* exchanges them) after the operation.
1023-
*/
1024-
if (xmi_can_exchange_reflink_flags(req, rs)) {
1025-
if (rs & 1)
1026-
xmi->xmi_flags |= XFS_EXCHMAPS_CLEAR_INO1_REFLINK;
1027-
if (rs & 2)
1028-
xmi->xmi_flags |= XFS_EXCHMAPS_CLEAR_INO2_REFLINK;
1029-
}
1030-
1031957
if (S_ISDIR(VFS_I(xmi->xmi_ip2)->i_mode) ||
1032958
S_ISLNK(VFS_I(xmi->xmi_ip2)->i_mode))
1033959
xmi->xmi_flags |= __XFS_EXCHMAPS_INO2_SHORTFORM;

‎fs/xfs/libxfs/xfs_log_format.h‎

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -927,10 +927,8 @@ struct xfs_xmi_log_format {
927927
*/
928928
#define XFS_EXCHMAPS_INO1_WRITTEN (1ULL << 2)
929929

930-
/* Clear the reflink flag from inode1 after the operation. */
930+
/* No longer set, but older intents may carry them; keep them valid. */
931931
#define XFS_EXCHMAPS_CLEAR_INO1_REFLINK (1ULL << 3)
932-
933-
/* Clear the reflink flag from inode2 after the operation. */
934932
#define XFS_EXCHMAPS_CLEAR_INO2_REFLINK (1ULL << 4)
935933

936934
#define XFS_EXCHMAPS_LOGGED_FLAGS (XFS_EXCHMAPS_ATTR_FORK | \

‎fs/xfs/xfs_bmap_util.c‎

Lines changed: 6 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1544,7 +1544,6 @@ xfs_swap_extents(
15441544
struct xfs_bstat *sbp = &sxp->sx_stat;
15451545
int src_log_flags, target_log_flags;
15461546
int error = 0;
1547-
uint64_t f;
15481547
int resblks = 0;
15491548
unsigned int flags = 0;
15501549
struct timespec64 ctime, mtime;
@@ -1700,14 +1699,12 @@ xfs_swap_extents(
17001699
if (error)
17011700
goto out_trans_cancel;
17021701

1703-
/* Do we have to swap reflink flags? */
1704-
if ((ip->i_diflags2 & XFS_DIFLAG2_REFLINK) ^
1705-
(tip->i_diflags2 & XFS_DIFLAG2_REFLINK)) {
1706-
f = ip->i_diflags2 & XFS_DIFLAG2_REFLINK;
1707-
ip->i_diflags2 &= ~XFS_DIFLAG2_REFLINK;
1708-
ip->i_diflags2 |= tip->i_diflags2 & XFS_DIFLAG2_REFLINK;
1709-
tip->i_diflags2 &= ~XFS_DIFLAG2_REFLINK;
1710-
tip->i_diflags2 |= f & XFS_DIFLAG2_REFLINK;
1702+
/* Either inode may now own shared blocks; flag both, with CoW forks. */
1703+
if ((ip->i_diflags2 | tip->i_diflags2) & XFS_DIFLAG2_REFLINK) {
1704+
ip->i_diflags2 |= XFS_DIFLAG2_REFLINK;
1705+
tip->i_diflags2 |= XFS_DIFLAG2_REFLINK;
1706+
xfs_ifork_init_cow(ip);
1707+
xfs_ifork_init_cow(tip);
17111708
}
17121709

17131710
/* Swap the cow forks. */

0 commit comments

Comments
 (0)