| Message ID | 20240906211136.70391-3-catherine.hoang@oracle.com (mailing list archive) |
|---|---|
| State | Not Applicable, archived |
| Headers | show |
| Series | xfs backports for 6.6.y (from 6.10) | expand |
On Fri, Sep 06, 2024 at 02:11:16 PM -0700, Catherine Hoang wrote: > From: Christoph Hellwig <hch@lst.de> > > commit d69bee6a35d3c5e4873b9e164dd1a9711351a97c upstream. > > [backport: resolve conflict due to xfs_mod_freecounter refactor] > > xfs_bmap_add_extent_delay_real takes parts or all of a delalloc extent > and converts them to a real extent. It is written to deal with any > potential overlap of the to be converted range with the delalloc extent, > but it turns out that currently only converting the entire extents, or a > part starting at the beginning is actually exercised, as the only caller > always tries to convert the entire delalloc extent, and either succeeds > or at least progresses partially from the start. > > If it only converts a tiny part of a delalloc extent, the indirect block > calculation for the new delalloc extent (da_new) might be equivalent to that > of the existing delalloc extent (da_old). If this extent conversion now > requires allocating an indirect block that gets accounted into da_new, > leading to the assert that da_new must be smaller or equal to da_new > unless we split the extent to trigger. > > Except for the assert that case is actually handled by just trying to > allocate more space, as that already handled for the split case (which > currently can't be reached at all), so just reusing it should be fine. > Except that without dipping into the reserved block pool that would make > it a bit too easy to trigger a fs shutdown due to ENOSPC. So in addition > to adjusting the assert, also dip into the reserved block pool. > > Note that I could only reproduce the assert with a change to only convert > the actually asked range instead of the full delalloc extent from > xfs_bmapi_write. > > Signed-off-by: Christoph Hellwig <hch@lst.de> > Reviewed-by: "Darrick J. Wong" <djwong@kernel.org> > Signed-off-by: Chandan Babu R <chandanbabu@kernel.org> > Signed-off-by: Catherine Hoang <catherine.hoang@oracle.com> > --- > fs/xfs/libxfs/xfs_bmap.c | 11 ++++++++--- > 1 file changed, 8 insertions(+), 3 deletions(-) > > diff --git a/fs/xfs/libxfs/xfs_bmap.c b/fs/xfs/libxfs/xfs_bmap.c > index 97f575e21f86..0ac533a18065 100644 > --- a/fs/xfs/libxfs/xfs_bmap.c > +++ b/fs/xfs/libxfs/xfs_bmap.c > @@ -1549,6 +1549,7 @@ xfs_bmap_add_extent_delay_real( > if (error) > goto done; > } > + ASSERT(da_new <= da_old); > break; > > case BMAP_LEFT_FILLING | BMAP_RIGHT_FILLING | BMAP_LEFT_CONTIG: > @@ -1578,6 +1579,7 @@ xfs_bmap_add_extent_delay_real( > if (error) > goto done; > } > + ASSERT(da_new <= da_old); > break; > > case BMAP_LEFT_FILLING | BMAP_RIGHT_FILLING | BMAP_RIGHT_CONTIG: > @@ -1611,6 +1613,7 @@ xfs_bmap_add_extent_delay_real( > if (error) > goto done; > } > + ASSERT(da_new <= da_old); > break; > > case BMAP_LEFT_FILLING | BMAP_RIGHT_FILLING: > @@ -1643,6 +1646,7 @@ xfs_bmap_add_extent_delay_real( > goto done; > } > } > + ASSERT(da_new <= da_old); > break; > > case BMAP_LEFT_FILLING | BMAP_LEFT_CONTIG: > @@ -1680,6 +1684,7 @@ xfs_bmap_add_extent_delay_real( > if (error) > goto done; > } > + ASSERT(da_new <= da_old); > break; > > case BMAP_LEFT_FILLING: > @@ -1767,6 +1772,7 @@ xfs_bmap_add_extent_delay_real( > xfs_iext_update_extent(bma->ip, state, &bma->icur, &PREV); > xfs_iext_next(ifp, &bma->icur); > xfs_iext_update_extent(bma->ip, state, &bma->icur, &RIGHT); > + ASSERT(da_new <= da_old); > break; > > case BMAP_RIGHT_FILLING: > @@ -1814,6 +1820,7 @@ xfs_bmap_add_extent_delay_real( > PREV.br_blockcount = temp; > xfs_iext_insert(bma->ip, &bma->icur, &PREV, state); > xfs_iext_next(ifp, &bma->icur); > + ASSERT(da_new <= da_old); > break; > > case 0: > @@ -1934,11 +1941,9 @@ xfs_bmap_add_extent_delay_real( > } > > /* adjust for changes in reserved delayed indirect blocks */ > - if (da_new != da_old) { > - ASSERT(state == 0 || da_new < da_old); > + if (da_new != da_old) > error = xfs_mod_fdblocks(mp, (int64_t)(da_old - da_new), > false); I think we need to allow space to be allocated from the reservation pool when da_new > da_old. Hence, the last argument to xfs_mod_fdblocks() should probably be true. > - } > > xfs_bmap_check_leaf_extents(bma->cur, bma->ip, whichfork); > done:
diff --git a/fs/xfs/libxfs/xfs_bmap.c b/fs/xfs/libxfs/xfs_bmap.c index 97f575e21f86..0ac533a18065 100644 --- a/fs/xfs/libxfs/xfs_bmap.c +++ b/fs/xfs/libxfs/xfs_bmap.c @@ -1549,6 +1549,7 @@ xfs_bmap_add_extent_delay_real( if (error) goto done; } + ASSERT(da_new <= da_old); break; case BMAP_LEFT_FILLING | BMAP_RIGHT_FILLING | BMAP_LEFT_CONTIG: @@ -1578,6 +1579,7 @@ xfs_bmap_add_extent_delay_real( if (error) goto done; } + ASSERT(da_new <= da_old); break; case BMAP_LEFT_FILLING | BMAP_RIGHT_FILLING | BMAP_RIGHT_CONTIG: @@ -1611,6 +1613,7 @@ xfs_bmap_add_extent_delay_real( if (error) goto done; } + ASSERT(da_new <= da_old); break; case BMAP_LEFT_FILLING | BMAP_RIGHT_FILLING: @@ -1643,6 +1646,7 @@ xfs_bmap_add_extent_delay_real( goto done; } } + ASSERT(da_new <= da_old); break; case BMAP_LEFT_FILLING | BMAP_LEFT_CONTIG: @@ -1680,6 +1684,7 @@ xfs_bmap_add_extent_delay_real( if (error) goto done; } + ASSERT(da_new <= da_old); break; case BMAP_LEFT_FILLING: @@ -1767,6 +1772,7 @@ xfs_bmap_add_extent_delay_real( xfs_iext_update_extent(bma->ip, state, &bma->icur, &PREV); xfs_iext_next(ifp, &bma->icur); xfs_iext_update_extent(bma->ip, state, &bma->icur, &RIGHT); + ASSERT(da_new <= da_old); break; case BMAP_RIGHT_FILLING: @@ -1814,6 +1820,7 @@ xfs_bmap_add_extent_delay_real( PREV.br_blockcount = temp; xfs_iext_insert(bma->ip, &bma->icur, &PREV, state); xfs_iext_next(ifp, &bma->icur); + ASSERT(da_new <= da_old); break; case 0: @@ -1934,11 +1941,9 @@ xfs_bmap_add_extent_delay_real( } /* adjust for changes in reserved delayed indirect blocks */ - if (da_new != da_old) { - ASSERT(state == 0 || da_new < da_old); + if (da_new != da_old) error = xfs_mod_fdblocks(mp, (int64_t)(da_old - da_new), false); - } xfs_bmap_check_leaf_extents(bma->cur, bma->ip, whichfork); done: