xfs
[Top] [All Lists]

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

To: Dave Chinner <david@xxxxxxxxxxxxx>
Subject: Re: [RFC PATCH 07/11] xfs: retry trans reservation on ENOSPC in xfs_inactive()
From: Brian Foster <bfoster@xxxxxxxxxx>
Date: Thu, 05 Sep 2013 12:18:31 -0400
Cc: xfs@xxxxxxxxxxx
Delivered-to: xfs@xxxxxxxxxxx
In-reply-to: <20130905014016.GU23571@dastard>
References: <1378232708-57156-1-git-send-email-bfoster@xxxxxxxxxx> <1378232708-57156-8-git-send-email-bfoster@xxxxxxxxxx> <20130905014016.GU23571@dastard>
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:17.0) Gecko/20130805 Thunderbird/17.0.8
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@xxxxxxxxxx>
>> ---
>>  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.
> 

<Prev in Thread] Current Thread [Next in Thread>