[ciqlts9_2] xfs: resample the data fork mapping after cycling ILOCK#1450
Merged
Conversation
commit-author Darrick J. Wong <djwong@kernel.org> commit 2f4acd0 upstream-diff | Between this kernel and the recent change xfs_reflink_allocate_cow() was refactored into two separate functions in d621133 xfs: Fix false ENOSPC when performing direct write on a delalloc extent in cow fork In addition after fixing the bug the offset_fsb and count_fsb need to be refreshed as a part of the above commit it creates xfs_reflink_convert_unwritten() which refreshes prior to returns. xfs_reflink_fill_{cow_hole,delalloc} are both presented with an inode, a data fork mapping, and a cow fork mapping. Unfortunately, these two helpers cycle the ILOCK to grab a transaction, which means that the mappings are stale as soon as we reacquire the ILOCK. Currently we refresh the cow fork mapping by re-calling xfs_find_trim_cow_extent, but we don't refresh the data fork mapping beforehand, which means that the xfs_bmap_trim_cow in that function queries the refcount btree about the wrong physical blocks and returns an inaccurate value in *shared. If *shared is now false, the directio write proceeds with a stale data fork mapping. Fix this by querying the data fork mapping if the sequence counter changes across the ILOCK cycle. Cc: hch@lst.de Cc: stable@vger.kernel.org # v4.11 Fixes: 3c68d44 ("xfs: allocate direct I/O COW blocks in iomap_begin") Signed-off-by: "Darrick J. Wong" <djwong@kernel.org> Reviewed-by: Christoph Hellwig <hch@lst.de> Reviewed-by: Carlos Maiolino <cmaiolino@redhat.com> Signed-off-by: Carlos Maiolino <cem@kernel.org> Signed-off-by: Jonathan Maple <jmaple@ciq.com>
|
🤖 Validation Checks In Progress Workflow run: https://github.com/ctrliq/kernel-src-tree/actions/runs/29626705315 |
🔍 Interdiff Analysis
================================================================================
* DELTA DIFFERENCES - code changes that differ between the patches *
================================================================================
--- b/fs/xfs/xfs_reflink.c
+++ b/fs/xfs/xfs_reflink.c
@@ -388,22 +387,4 @@
/*
- * The data fork mapping may have changed while we dropped the ILOCK
- * (a racing O_DIRECT writer under IOLOCK_SHARED can complete a full
- * CoW cycle including xfs_reflink_end_cow(), which remaps this offset
- * and drops the refcount of the old shared block). Re-read it so the
- * shared-status recheck below and the caller's in-place iomap both
- * operate on the current mapping rather than a stale physical block.
- */
- if (seq_before != READ_ONCE(ip->i_df.if_seq)) {
- nimaps = 1;
- error = xfs_bmapi_read(ip, imap->br_startoff,
- imap->br_blockcount, imap, &nimaps, 0);
- if (error)
- goto out_trans_cancel;
- offset_fsb = imap->br_startoff;
- count_fsb = imap->br_blockcount;
- }
-
- /*
* Check for an overlapping extent again now that we dropped the ilock.
*/
################################################################################
! REJECTED PATCH2 HUNKS - could not be compared; manual review needed !
################################################################################
--- b/fs/xfs/xfs_reflink.c
+++ b/fs/xfs/xfs_reflink.c
@@ -440,6 +440,7 @@
struct xfs_mount *mp = ip->i_mount;
struct xfs_trans *tp;
xfs_filblks_t resaligned;
+ unsigned int seq_before = READ_ONCE(ip->i_df.if_seq);
unsigned int dblocks = 0, rblocks = 0;
int nimaps;
int error;
@@ -465,6 +466,22 @@
*lockmode = XFS_ILOCK_EXCL;
+ /*
+ * The data fork mapping may have changed while we dropped the ILOCK
+ * (a racing O_DIRECT writer under IOLOCK_SHARED can complete a full
+ * CoW cycle including xfs_reflink_end_cow(), which remaps this offset
+ * and drops the refcount of the old shared block). Re-read it so the
+ * shared-status recheck below and the caller's in-place iomap both
+ * operate on the current mapping rather than a stale physical block.
+ */
+ if (seq_before != READ_ONCE(ip->i_df.if_seq)) {
+ nimaps = 1;
+ error = xfs_bmapi_read(ip, imap->br_startoff,
+ imap->br_blockcount, imap, &nimaps, 0);
+ if (error)
+ goto out_trans_cancel;
+ }
+
error = xfs_find_trim_cow_extent(ip, imap, cmap, shared, &found);
if (error || !*shared)
goto out_trans_cancel;
@@ -511,6 +528,8 @@
bool found;
do {
+ unsigned int seq_before = READ_ONCE(ip->i_df.if_seq);
+
xfs_iunlock(ip, *lockmode);
*lockmode = 0;
@@ -521,6 +540,23 @@
*lockmode = XFS_ILOCK_EXCL;
+ /*
+ * The data fork mapping may have changed while we dropped the
+ * ILOCK (a racing O_DIRECT writer under IOLOCK_SHARED can
+ * complete a full CoW cycle including xfs_reflink_end_cow(),
+ * which remaps this offset and drops the refcount of the old
+ * shared block). Re-read it so the shared-status recheck
+ * below and the caller's in-place iomap both operate on the
+ * current mapping rather than a stale physical block.
+ */
+ if (seq_before != READ_ONCE(ip->i_df.if_seq)) {
+ nimaps = 1;
+ error = xfs_bmapi_read(ip, imap->br_startoff,
+ imap->br_blockcount, imap, &nimaps, 0);
+ if (error)
+ goto out_trans_cancel;
+ }
+
error = xfs_find_trim_cow_extent(ip, imap, cmap, shared,
&found);
if (error || !*shared)
================================================================================
* CONTEXT DIFFERENCES - surrounding code differences between the patches *
================================================================================
--- b/fs/xfs/xfs_reflink.c
+++ b/fs/xfs/xfs_reflink.c
@@ -354,2 +356,4 @@
xfs_filblks_t resaligned;
- xfs_extlen_t resblks = 0;
+ unsigned int dblocks = 0, rblocks = 0;
+ int nimaps;
+ int error;
@@ -356,2 +362,4 @@
- ASSERT(xfs_isilocked(ip, XFS_ILOCK_EXCL));
+ error = xfs_find_trim_cow_extent(ip, imap, cmap, shared, &found);
+ if (error || !*shared)
+ goto out_trans_cancel;
@@ -382,6 +498,11 @@
- *lockmode = XFS_ILOCK_EXCL;
+ do {
+ xfs_iunlock(ip, *lockmode);
+ *lockmode = 0;
+
+
+ *lockmode = XFS_ILOCK_EXCL;
- /*
- * Check for an overlapping extent again now that we dropped the ilock.
- */
+ error = xfs_find_trim_cow_extent(ip, imap, cmap, shared,
+ &found);
+ if (error || !*shared)This is an automated interdiff check for backported commits. |
|
✅ Validation checks completed successfully View full results: https://github.com/ctrliq/kernel-src-tree/actions/runs/29626705315 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
This PR has been automatically created after successful completion of all CI stages.
Commit Message(s)
Test Results
✅ Build Stage
✅ Boot Verification
✅ Kernel Selftests
✅ LTP Results
aarch64 regressions:
🤖 This PR was automatically generated by GitHub Actions
Run ID: 29616164768