On Tue, Nov 20, 2018 at 08:45:38AM -0500, Brian Foster wrote: > > * Filling in all of a previously delayed allocation extent. > > - * The right neighbor is contiguous, the left is not. > > + * The right neighbor is contiguous, the left is not. Take care > > + * with delay -> unwritten extent allocation here because the > > + * delalloc record we are overwriting is always written. > > */ > > PREV.br_startblock = new->br_startblock; > > PREV.br_blockcount += RIGHT.br_blockcount; > > + PREV.br_state = new->br_state; > > > > xfs_iext_next(ifp, &bma->icur); > > xfs_iext_remove(bma->ip, &bma->icur, state); > > Fix looks sane to me, though I'm a little curious why this doesn't > follow the "contiguous extent extension" pattern that most of the other > contig cases seem to follow. For example, something like the following > for the right contig case: > > old = RIGHT; > RIGHT.br_startoff = new->br_startoff; > RIGHT.br_startblock = new->br_startblock; > RIGHT.br_blockcount += new.br_blockcount; > > xfs_iext_remove(bma->ip, &bma->icur, state); /* PREV */ > xfs_iext_next(ifp, &bma->icur); > xfs_iext_update_extent(bma->ip, state, &bma->icur, &RIGHT); > > ... and change the subsequent btree update to use old/RIGHT. Maybe > Christoph has thoughts on that. Not sure the above looks much better. If I had to do something different I'd apply something like this (untested) on top of Daves patch: diff --git a/fs/xfs/libxfs/xfs_bmap.c b/fs/xfs/libxfs/xfs_bmap.c index 91b94c5c0cae..efe9e554fd0d 100644 --- a/fs/xfs/libxfs/xfs_bmap.c +++ b/fs/xfs/libxfs/xfs_bmap.c @@ -1698,14 +1698,12 @@ xfs_bmap_add_extent_delay_real( * with delay -> unwritten extent allocation here because the * delalloc record we are overwriting is always written. */ - PREV.br_startblock = new->br_startblock; - PREV.br_blockcount += RIGHT.br_blockcount; - PREV.br_state = new->br_state; + new->br_blockcount += RIGHT.br_blockcount; xfs_iext_next(ifp, &bma->icur); xfs_iext_remove(bma->ip, &bma->icur, state); xfs_iext_prev(ifp, &bma->icur); - xfs_iext_update_extent(bma->ip, state, &bma->icur, &PREV); + xfs_iext_update_extent(bma->ip, state, &bma->icur, new); if (bma->cur == NULL) rval = XFS_ILOG_DEXT;
