On Tue, Aug 27, 2024 at 08:50:48AM +0200, Christoph Hellwig wrote: > Call xfs_flush_unmap_range from xfs_free_file_space so that > xfs_file_fallocate doesn't have to predict which mode will call it. > > Signed-off-by: Christoph Hellwig <hch@xxxxxx> Hmm. I /think/ it's ok to shift the xfs_flush_unmap_range after the file_modified and some of the other EINVAL bailouts that can happen before xfs_free_file_space gets called. Effectively that means that we can fail faster now? :) Later on this means that the cow-around code that the rtreflink patchset introduces will also get flushed to disk before we start collapsing/zeroing/punching. AFAICT that should be fine. Reviewed-by: Darrick J. Wong <djwong@xxxxxxxxxx> --D > --- > fs/xfs/xfs_bmap_util.c | 8 ++++++++ > fs/xfs/xfs_file.c | 21 --------------------- > 2 files changed, 8 insertions(+), 21 deletions(-) > > diff --git a/fs/xfs/xfs_bmap_util.c b/fs/xfs/xfs_bmap_util.c > index fe2e2c93097550..187a0dbda24fc4 100644 > --- a/fs/xfs/xfs_bmap_util.c > +++ b/fs/xfs/xfs_bmap_util.c > @@ -848,6 +848,14 @@ xfs_free_file_space( > if (len <= 0) /* if nothing being freed */ > return 0; > > + /* > + * Now AIO and DIO has drained we flush and (if necessary) invalidate > + * the cached range over the first operation we are about to run. > + */ > + error = xfs_flush_unmap_range(ip, offset, len); > + if (error) > + return error; > + > startoffset_fsb = XFS_B_TO_FSB(mp, offset); > endoffset_fsb = XFS_B_TO_FSBT(mp, offset + len); > > diff --git a/fs/xfs/xfs_file.c b/fs/xfs/xfs_file.c > index 4cdc54dc96862e..5b9e49da06013c 100644 > --- a/fs/xfs/xfs_file.c > +++ b/fs/xfs/xfs_file.c > @@ -890,27 +890,6 @@ xfs_file_fallocate( > */ > inode_dio_wait(inode); > > - /* > - * Now AIO and DIO has drained we flush and (if necessary) invalidate > - * the cached range over the first operation we are about to run. > - * > - * We care about zero and collapse here because they both run a hole > - * punch over the range first. Because that can zero data, and the range > - * of invalidation for the shift operations is much larger, we still do > - * the required flush for collapse in xfs_prepare_shift(). > - * > - * Insert has the same range requirements as collapse, and we extend the > - * file first which can zero data. Hence insert has the same > - * flush/invalidate requirements as collapse and so they are both > - * handled at the right time by xfs_prepare_shift(). > - */ > - if (mode & (FALLOC_FL_PUNCH_HOLE | FALLOC_FL_ZERO_RANGE | > - FALLOC_FL_COLLAPSE_RANGE)) { > - error = xfs_flush_unmap_range(ip, offset, len); > - if (error) > - goto out_unlock; > - } > - > error = file_modified(file); > if (error) > goto out_unlock; > -- > 2.43.0 > >