On Wed, May 23, 2018 at 12:17:11PM -0400, Brian Foster wrote: > Mostly looks Ok, but I'm not following what this get_extent() call is > for..? It also doesn't look like it would always do the right thing with > sub-page blocks. Consider a page with a couple discontig delalloc blocks > that happen to be the first extents in the file. The first > xfs_bmap_del_extent_delay() would do: > > xfs_iext_remove(ip, icur, state); > xfs_iext_prev(ifp, icur); > > ... which I think sets cur->pos to -1, causes the get_extent() to fail > and thus fails to remove the subsequent delalloc blocks. Hm? True. This function should probably walk the extent list backwards like xfs_bunmapi as that is the model that xfs_bmap_del_extent_* is built around.