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

Brian Foster
2013-09-05T16:22:11Z

On 09/04/2013 09:40 PM, Dave Chinner wrote:
> 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.
>

Ok.

> 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....
>

I did have one of the enospc xfstests lead to this situation, though I
don't have the particular test in my notes. It initially manifested as
an assert failure due to the fs not being shutdown after an
xfs_trans_reserve() ENOSPC failure. Subsequent to avoiding that, I
believe there were inconsistent fs issues called out due to the unlinked
lists being populated after umount.

Taking a further look, I missed the XFS_TRANS_RESERVE flag and whole
m_resblks mechanism. I'll take a closer look at that and see if that
works to resolve the problem instead of the flush.

Brian

> Cheers,
>
> Dave.