On Thu, Aug 21, 2014 at 03:09:14PM +1000, Dave Chinner wrote: > From: Dave Chinner <dchinner@xxxxxxxxxx> > > If we have delalloc extents on a file before we run a collapse range > opertaion, we sync the range that we are going to collapse to > convert delalloc extents in that region to real extents to simplify > the shift operation. > > However, the shift operation then assumes that the extent list is > not going to change as it iterates over the extent list moving > things about. Unfortunately, this isn't true because we can't hold > the ILOCK over all the operations. We can prevent new IO from > modifying the extent list by holding the IOLOCK, but that doesn't > prevent writeback from running.... > > And when writeback runs, it can convert delalloc extents is the > range of the file prior to the region being collapsed, and this > changes the indexes of all the extents in the file. That causes the > collapse range operation to Go Bad. > > The right fix is to rewrite the extent shift operation not to be > dependent on the extent list not changing across the entire > operation, but this is a fairly significant piece of work to do. > Hence, as a short-term workaround for the problem, sync the entire > file before starting a collapse operation to remove all delalloc > ranges from the file and so avoid the problem of concurrent > writeback changing the extent list. > > Diagnosed-and-Reported-by: Brian Foster <bfoster@xxxxxxxxxx> > Signed-off-by: Dave Chinner <dchinner@xxxxxxxxxx> > --- Reviewed-by: Brian Foster <bfoster@xxxxxxxxxx> > fs/xfs/xfs_bmap_util.c | 13 +++++++++++++ > 1 file changed, 13 insertions(+) > > diff --git a/fs/xfs/xfs_bmap_util.c b/fs/xfs/xfs_bmap_util.c > index c53cc03..035041d 100644 > --- a/fs/xfs/xfs_bmap_util.c > +++ b/fs/xfs/xfs_bmap_util.c > @@ -1460,6 +1460,19 @@ xfs_collapse_file_space( > start_fsb = XFS_B_TO_FSB(mp, offset + len); > shift_fsb = XFS_B_TO_FSB(mp, len); > > + /* > + * writeback the entire file to prevent concurrent writeback of ranges > + * outside the collapsing region from changing the extent list. > + * > + * XXX: This is a temporary fix until the extent shift loop below is > + * converted to use offsets and lookups within the ILOCK rather than > + * carrying around the index into the extent list for the next > + * iteration. > + */ > + error = filemap_write_and_wait(VFS_I(ip)->i_mapping); > + if (error) > + return error; > + > error = xfs_free_file_space(ip, offset, len); > if (error) > return error; > -- > 2.0.0 > > _______________________________________________ > xfs mailing list > xfs@xxxxxxxxxxx > http://oss.sgi.com/mailman/listinfo/xfs _______________________________________________ xfs mailing list xfs@xxxxxxxxxxx http://oss.sgi.com/mailman/listinfo/xfs
