|
| 1 | +xfs: fix off-by-one-block in xfs_discard_folio() |
| 2 | + |
| 3 | +jira LE-1907 |
| 4 | +Rebuild_History Non-Buildable kernel-rt-5.14.0-284.30.1.rt14.315.el9_2 |
| 5 | +commit-author Dave Chinner < [email protected]> |
| 6 | +commit 8ac5b996bf5199f15b7687ceae989f8b2a410dda |
| 7 | +Empty-Commit: Cherry-Pick Conflicts during history rebuild. |
| 8 | +Will be included in final tarball splat. Ref for failed cherry-pick at: |
| 9 | +ciq/ciq_backports/kernel-rt-5.14.0-284.30.1.rt14.315.el9_2/8ac5b996.failed |
| 10 | + |
| 11 | +The recent writeback corruption fixes changed the code in |
| 12 | +xfs_discard_folio() to calculate a byte range to for punching |
| 13 | +delalloc extents. A mistake was made in using round_up(pos) for the |
| 14 | +end offset, because when pos points at the first byte of a block, it |
| 15 | +does not get rounded up to point to the end byte of the block. hence |
| 16 | +the punch range is short, and this leads to unexpected behaviour in |
| 17 | +certain cases in xfs_bmap_punch_delalloc_range. |
| 18 | + |
| 19 | +e.g. pos = 0 means we call xfs_bmap_punch_delalloc_range(0,0), so |
| 20 | +there is no previous extent and it rounds up the punch to the end of |
| 21 | +the delalloc extent it found at offset 0, not the end of the range |
| 22 | +given to xfs_bmap_punch_delalloc_range(). |
| 23 | + |
| 24 | +Fix this by handling the zero block offset case correctly. |
| 25 | + |
| 26 | +Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=217030 |
| 27 | +Link: https://lore.kernel.org/linux-xfs/Y+vOfaxIWX1c%2Fyy9@bfoster/ |
| 28 | +Fixes: 7348b322332d ("xfs: xfs_bmap_punch_delalloc_range() should take a byte range") |
| 29 | + Reported-by: Pengfei Xu < [email protected]> |
| 30 | +Found-by: Brian Foster < [email protected]> |
| 31 | + Signed-off-by: Dave Chinner < [email protected]> |
| 32 | + Reviewed-by: Darrick J. Wong < [email protected]> |
| 33 | + Signed-off-by: Darrick J. Wong < [email protected]> |
| 34 | +(cherry picked from commit 8ac5b996bf5199f15b7687ceae989f8b2a410dda) |
| 35 | + Signed-off-by: Jonathan Maple < [email protected]> |
| 36 | + |
| 37 | +# Conflicts: |
| 38 | +# fs/xfs/xfs_aops.c |
| 39 | +diff --cc fs/xfs/xfs_aops.c |
| 40 | +index c46a16ba4c33,2ef78aa1d3f6..000000000000 |
| 41 | +--- a/fs/xfs/xfs_aops.c |
| 42 | ++++ b/fs/xfs/xfs_aops.c |
| 43 | +@@@ -425,39 -449,44 +425,54 @@@ xfs_prepare_ioend |
| 44 | + } |
| 45 | + |
| 46 | + /* |
| 47 | +- * If the page has delalloc blocks on it, we need to punch them out before we |
| 48 | +- * invalidate the page. If we don't, we leave a stale delalloc mapping on the |
| 49 | +- * inode that can trip up a later direct I/O read operation on the same region. |
| 50 | ++ * If the folio has delalloc blocks on it, the caller is asking us to punch them |
| 51 | ++ * out. If we don't, we can leave a stale delalloc mapping covered by a clean |
| 52 | ++ * page that needs to be dirtied again before the delalloc mapping can be |
| 53 | ++ * converted. This stale delalloc mapping can trip up a later direct I/O read |
| 54 | ++ * operation on the same region. |
| 55 | + * |
| 56 | +- * We prevent this by truncating away the delalloc regions on the page. Because |
| 57 | ++ * We prevent this by truncating away the delalloc regions on the folio. Because |
| 58 | + * they are delalloc, we can do this without needing a transaction. Indeed - if |
| 59 | + * we get ENOSPC errors, we have to be able to do this truncation without a |
| 60 | +- * transaction as there is no space left for block reservation (typically why we |
| 61 | +- * see a ENOSPC in writeback). |
| 62 | ++ * transaction as there is no space left for block reservation (typically why |
| 63 | ++ * we see a ENOSPC in writeback). |
| 64 | + */ |
| 65 | + static void |
| 66 | + -xfs_discard_folio( |
| 67 | + - struct folio *folio, |
| 68 | + - loff_t pos) |
| 69 | + +xfs_discard_page( |
| 70 | + + struct page *page, |
| 71 | + + loff_t fileoff) |
| 72 | + { |
| 73 | + - struct xfs_inode *ip = XFS_I(folio->mapping->host); |
| 74 | + + struct xfs_inode *ip = XFS_I(page->mapping->host); |
| 75 | + struct xfs_mount *mp = ip->i_mount; |
| 76 | + + unsigned int pageoff = offset_in_page(fileoff); |
| 77 | + int error; |
| 78 | + |
| 79 | + if (xfs_is_shutdown(mp)) |
| 80 | + - return; |
| 81 | + + goto out_invalidate; |
| 82 | + |
| 83 | + xfs_alert_ratelimited(mp, |
| 84 | +++<<<<<<< HEAD |
| 85 | + + "page discard on page "PTR_FMT", inode 0x%llx, offset %llu.", |
| 86 | + + page, ip->i_ino, fileoff); |
| 87 | +++======= |
| 88 | ++ "page discard on page "PTR_FMT", inode 0x%llx, pos %llu.", |
| 89 | ++ folio, ip->i_ino, pos); |
| 90 | ++ |
| 91 | ++ /* |
| 92 | ++ * The end of the punch range is always the offset of the the first |
| 93 | ++ * byte of the next folio. Hence the end offset is only dependent on the |
| 94 | ++ * folio itself and not the start offset that is passed in. |
| 95 | ++ */ |
| 96 | ++ error = xfs_bmap_punch_delalloc_range(ip, pos, |
| 97 | ++ folio_pos(folio) + folio_size(folio)); |
| 98 | +++>>>>>>> 8ac5b996bf51 (xfs: fix off-by-one-block in xfs_discard_folio()) |
| 99 | + |
| 100 | + + error = xfs_bmap_punch_delalloc_range(ip, fileoff, |
| 101 | + + round_up(fileoff, PAGE_SIZE)); |
| 102 | + if (error && !xfs_is_shutdown(mp)) |
| 103 | + xfs_alert(mp, "page discard unable to remove delalloc mapping."); |
| 104 | + +out_invalidate: |
| 105 | + + iomap_invalidatepage(page, pageoff, PAGE_SIZE - pageoff); |
| 106 | + } |
| 107 | + |
| 108 | + static const struct iomap_writeback_ops xfs_writeback_ops = { |
| 109 | +* Unmerged path fs/xfs/xfs_aops.c |
0 commit comments