Re: [RFC PATCH 07/11] xfs: retry trans reservation on ENOSPC in xfs_inactive()

Dave Chinner
2013-09-05T01:40:46Z

On Tue, Sep 03, 2013 at 02:25:04PM -0400, Brian Foster wrote:
> An ifree data block reservation can fail with ENOSPC. Flush inodes
> to try and free up space or attempt without a data block
> reservation to avoid failing out of xfs_inactive().
>
> Signed-off-by: Brian Foster <bfoster@redhat.com>
> ---
> fs/xfs/xfs_inode.c | 11 +++++++++++
> 1 file changed, 11 insertions(+)
>
> diff --git a/fs/xfs/xfs_inode.c b/fs/xfs/xfs_inode.c
> index 56cbf63..92de4b7 100644
> --- a/fs/xfs/xfs_inode.c
> +++ b/fs/xfs/xfs_inode.c
> @@ -1779,7 +1779,18 @@ xfs_inactive(
> tp = xfs_trans_alloc(mp, XFS_TRANS_INACTIVE);
> error = xfs_trans_reserve(tp, &M_RES(mp)->tr_ifree,
> XFS_IFREE_SPACE_RES(mp), 0);
> + if (error == ENOSPC) {
> + /* flush outstanding delalloc blocks and retry */
> + xfs_flush_inodes(mp);
> + error = xfs_trans_reserve(tp, &M_RES(mp)->tr_ifree,
> + XFS_IFREE_SPACE_RES(mp), 0);
> + }

We don't want to be blocking for inode flushes here. We might be in
a shrinker context, for example, and blocking those for a filesystem
sync is going to be unfriendly.

If this really is a problem, then the right thing to do is to allow
this transaction to dip into the reserve block pool so the
transaction can complete and make progress - other write operations
will trigger the flushing of the filesystem, and freeing of whole
inode chunks should return more free space than we need for the
finobt modifications in the removing lots of zero length inodes
at ENOSPC case....

Cheers,

Dave.
--
Dave Chinner
david@fromorbit.com