Skip to content

Commit 8ac5b99

Browse files
Dave ChinnerDarrick J. Wong
authored andcommitted
xfs: fix off-by-one-block in xfs_discard_folio()
The recent writeback corruption fixes changed the code in xfs_discard_folio() to calculate a byte range to for punching delalloc extents. A mistake was made in using round_up(pos) for the end offset, because when pos points at the first byte of a block, it does not get rounded up to point to the end byte of the block. hence the punch range is short, and this leads to unexpected behaviour in certain cases in xfs_bmap_punch_delalloc_range. e.g. pos = 0 means we call xfs_bmap_punch_delalloc_range(0,0), so there is no previous extent and it rounds up the punch to the end of the delalloc extent it found at offset 0, not the end of the range given to xfs_bmap_punch_delalloc_range(). Fix this by handling the zero block offset case correctly. Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=217030 Link: https://lore.kernel.org/linux-xfs/Y+vOfaxIWX1c%2Fyy9@bfoster/ Fixes: 7348b32 ("xfs: xfs_bmap_punch_delalloc_range() should take a byte range") Reported-by: Pengfei Xu <pengfei.xu@intel.com> Found-by: Brian Foster <bfoster@redhat.com> Signed-off-by: Dave Chinner <dchinner@redhat.com> Reviewed-by: Darrick J. Wong <djwong@kernel.org> Signed-off-by: Darrick J. Wong <djwong@kernel.org>
1 parent 0c7273e commit 8ac5b99

File tree

1 file changed

+14
-7
lines changed

1 file changed

+14
-7
lines changed

fs/xfs/xfs_aops.c

Lines changed: 14 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -449,15 +449,17 @@ xfs_prepare_ioend(
449449
}
450450

451451
/*
452-
* If the page has delalloc blocks on it, we need to punch them out before we
453-
* invalidate the page. If we don't, we leave a stale delalloc mapping on the
454-
* inode that can trip up a later direct I/O read operation on the same region.
452+
* If the folio has delalloc blocks on it, the caller is asking us to punch them
453+
* out. If we don't, we can leave a stale delalloc mapping covered by a clean
454+
* page that needs to be dirtied again before the delalloc mapping can be
455+
* converted. This stale delalloc mapping can trip up a later direct I/O read
456+
* operation on the same region.
455457
*
456-
* We prevent this by truncating away the delalloc regions on the page. Because
458+
* We prevent this by truncating away the delalloc regions on the folio. Because
457459
* they are delalloc, we can do this without needing a transaction. Indeed - if
458460
* we get ENOSPC errors, we have to be able to do this truncation without a
459-
* transaction as there is no space left for block reservation (typically why we
460-
* see a ENOSPC in writeback).
461+
* transaction as there is no space left for block reservation (typically why
462+
* we see a ENOSPC in writeback).
461463
*/
462464
static void
463465
xfs_discard_folio(
@@ -475,8 +477,13 @@ xfs_discard_folio(
475477
"page discard on page "PTR_FMT", inode 0x%llx, pos %llu.",
476478
folio, ip->i_ino, pos);
477479

480+
/*
481+
* The end of the punch range is always the offset of the the first
482+
* byte of the next folio. Hence the end offset is only dependent on the
483+
* folio itself and not the start offset that is passed in.
484+
*/
478485
error = xfs_bmap_punch_delalloc_range(ip, pos,
479-
round_up(pos, folio_size(folio)));
486+
folio_pos(folio) + folio_size(folio));
480487

481488
if (error && !xfs_is_shutdown(mp))
482489
xfs_alert(mp, "page discard unable to remove delalloc mapping.");

0 commit comments

Comments
 (0)