On Tue, Sep 09, 2014 at 03:13:26PM +1000, Dave Chinner wrote:
> On Sun, Sep 07, 2014 at 08:25:59AM -0400, Brian Foster wrote:
> > The collapse range operation currently writes the entire file before
> > starting the collapse to avoid changes in the in-core extent list due to
> > writeback causing the extent count to change. Now that collapse range is
> > fsb based rather than extent index based it can sustain changes in the
> > extent list during the shift sequence without disruption.
> >
> > Modify xfs_collapse_file_space() to writeback and invalidate pages
> > associated with the range of the file to be shifted.
> > xfs_free_file_space() currently has similar behavior, but the space free
> > need only affect the region of the file that is freed and this could
> > change in the future.
> >
> > Also update the comments to reflect the current implementation. We
> > retain the eofblocks trim permanently as a best option for dealing with
> > delalloc extents. We don't shift delalloc extents because this scenario
> > only occurs with post-eof preallocation (since data must be flushed such
> > that the cache can be invalidated and data can be shifted). That means
> > said space must also be initialized before being shifted into the
> > accessible region of the file only to be immediately truncated off as
> > the last part of the collapse. In other words, the eofblocks trim will
> > happen anyways, we just run it first to ensure the file remains in a
> > consistent state throughout the collapse.
> >
> > Finally, BUG() in the event of a delalloc extent during the extent shift
> > such that a failure is obvious. The implementation explicitly does not
> > support delalloc extents and the caller is expected to prevent this
> > scenario in advance as is done by collapse.
> >
> > Signed-off-by: Brian Foster <bfoster@xxxxxxxxxx>
> > ---
> > fs/xfs/libxfs/xfs_bmap.c | 2 ++
> > fs/xfs/xfs_bmap_util.c | 32 +++++++++++++++++++-------------
> > 2 files changed, 21 insertions(+), 13 deletions(-)
> >
> > diff --git a/fs/xfs/libxfs/xfs_bmap.c b/fs/xfs/libxfs/xfs_bmap.c
> > index 449a016..1dd04c2 100644
> > --- a/fs/xfs/libxfs/xfs_bmap.c
> > +++ b/fs/xfs/libxfs/xfs_bmap.c
> > @@ -5617,6 +5617,8 @@ xfs_bmap_shift_extents(
> > */
> > total_extents = ifp->if_bytes / sizeof(xfs_bmbt_rec_t);
> > while (nexts++ < num_exts && current_ext < total_extents) {
> > + /* can't handle delalloc extents */
> > + BUG_ON(isnullstartblock(got.br_startblock));
>
> XFS_WANT_CORRUPTED_GOTO() would be better, I think.
>
Ok. I suppose we'll shutdown the fs if the transaction was dirtied by
that point anyways. I want to make sure the failure is explicit more
than anything, as opposed to an unclear and non-guaranteed lookup
failure in the event of a btree, so that works for me.
Brian
> Otherwise OK.
>
> Cheers,
>
> Dave.
> --
> Dave Chinner
> david@xxxxxxxxxxxxx
>
> _______________________________________________
> xfs mailing list
> xfs@xxxxxxxxxxx
> http://oss.sgi.com/mailman/listinfo/xfs
_______________________________________________
xfs mailing list
xfs@xxxxxxxxxxx
http://oss.sgi.com/mailman/listinfo/xfs
