Re: [PATCH 5/6] xfs: flush speculative space allocations when we run out of quota
From: Dave Chinner <david@fromorbit.com>
Date: 2021-01-12 01:23:54
On Mon, Jan 11, 2021 at 03:23:05PM -0800, Darrick J. Wong wrote:
quoted hunk ↗ jump to hunk
From: Darrick J. Wong <djwong@kernel.org> If a fs modification (creation, file write, reflink, etc.) is unable to reserve enough quota to handle the modification, try clearing whatever space the filesystem might have been hanging onto in the hopes of speeding up the filesystem. The flushing behavior will become particularly important when we add deferred inode inactivation because that will increase the amount of space that isn't actively tied to user data. Signed-off-by: Darrick J. Wong <djwong@kernel.org> --- fs/xfs/xfs_bmap_util.c | 16 ++++++++++++++++ fs/xfs/xfs_file.c | 2 +- fs/xfs/xfs_icache.c | 9 +++++++-- fs/xfs/xfs_icache.h | 2 +- fs/xfs/xfs_inode.c | 17 +++++++++++++++++ fs/xfs/xfs_ioctl.c | 2 ++ fs/xfs/xfs_iomap.c | 20 +++++++++++++++++++- fs/xfs/xfs_reflink.c | 40 +++++++++++++++++++++++++++++++++++++--- fs/xfs/xfs_trace.c | 1 + fs/xfs/xfs_trace.h | 40 ++++++++++++++++++++++++++++++++++++++++ 10 files changed, 141 insertions(+), 8 deletions(-)diff --git a/fs/xfs/xfs_bmap_util.c b/fs/xfs/xfs_bmap_util.c index 7371a7f7c652..437fdc8a8fbd 100644 --- a/fs/xfs/xfs_bmap_util.c +++ b/fs/xfs/xfs_bmap_util.c@@ -761,6 +761,7 @@ xfs_alloc_file_space( */ while (allocatesize_fsb && !error) { xfs_fileoff_t s, e; + bool cleared_space = false; /* * Determine space reservations for data/realtime.@@ -803,6 +804,7 @@ xfs_alloc_file_space( /* * Allocate and setup the transaction. */ +retry: error = xfs_trans_alloc(mp, &M_RES(mp)->tr_write, resblks, resrtextents, 0, &tp);@@ -819,6 +821,20 @@ xfs_alloc_file_space( xfs_ilock(ip, XFS_ILOCK_EXCL); error = xfs_trans_reserve_quota_nblks(tp, ip, qblocks, 0, quota_flag); + /* + * We weren't able to reserve enough quota to handle fallocate. + * Flush any disk space that was being held in the hopes of + * speeding up the filesystem. We hold the IOLOCK so we cannot + * do a synchronous scan. + */ + if ((error == -ENOSPC || error == -EDQUOT) && !cleared_space) { + xfs_trans_cancel(tp); + xfs_iunlock(ip, XFS_ILOCK_EXCL); + cleared_space = xfs_inode_free_quota_blocks(ip, false); + if (cleared_space) + goto retry; + return error;
Can't say I'm a fan of repeating this everywhere. Can we move this
into xfs_trans_reserve_quota_nblks() with a "retry" flag such that
we do:
error = xfs_trans_reserve_quota_nblks(tp, ip, qblocks, 0,
quota_flag, &retry);
if (error) {
/* tp already cancelled, inode unlocked */
return error;
}
if (retry) {
/* tp already cancelled, inode unlocked */
goto retry;
}
Cheers,
Dave.
--
Dave Chinner
david@fromorbit.com