On Wed, Sep 18, 2019 at 10:55:38AM -0400, Brian Foster wrote: > The collapse range operation can merge extents if two newly adjacent > extents are physically contiguous. If the extent count is reduced on > a btree format inode, a change to extent format might be necessary. > This format change currently occurs as a side effect of the file > size update after extents have been shifted for the collapse. This > codepath ultimately calls xfs_bunmapi(), which happens to check for > and execute the format conversion even if there were no blocks > removed from the mapping. > > While this ultimately puts the inode into the correct state, the > fact the format conversion occurs in a separate transaction from the > change that called for it is a problem. If an extent shift > transaction commits and the filesystem happens to crash before the > format conversion, the inode fork is left in a corrupted state after > log recovery. The inode fork verifier fails and xfs_repair > ultimately nukes the inode. This problem was originally reproduced > by generic/388. > > Similar to how the insert range extent split code handles extent to > btree conversion, update the collapse range extent merge code to > handle btree to extent format conversion in the same transaction > that merges the extents. This ensures that the inode fork format > remains consistent if the filesystem happens to crash in the middle > of a collapse range operation that changes the inode fork format. > > Signed-off-by: Brian Foster <bfoster@xxxxxxxxxx> Seems sensible to me, Reviewed-by: Darrick J. Wong <darrick.wong@xxxxxxxxxx> --D > --- > fs/xfs/libxfs/xfs_bmap.c | 5 +++++ > 1 file changed, 5 insertions(+) > > diff --git a/fs/xfs/libxfs/xfs_bmap.c b/fs/xfs/libxfs/xfs_bmap.c > index 054b4ce30033..eaf2d4250a26 100644 > --- a/fs/xfs/libxfs/xfs_bmap.c > +++ b/fs/xfs/libxfs/xfs_bmap.c > @@ -5621,6 +5621,11 @@ xfs_bmse_merge( > if (error) > return error; > > + /* change to extent format if required after extent removal */ > + error = xfs_bmap_btree_to_extents(tp, ip, cur, logflags, whichfork); > + if (error) > + return error; > + > done: > xfs_iext_remove(ip, icur, 0); > xfs_iext_prev(XFS_IFORK_PTR(ip, whichfork), icur); > -- > 2.20.1 >
