xfs
[Top] [All Lists]

Re: iomap infrastructure and multipage writes V5

To: Dave Chinner <david@xxxxxxxxxxxxx>, Christoph Hellwig <hch@xxxxxx>
Subject: Re: iomap infrastructure and multipage writes V5
From: Eric Sandeen <sandeen@xxxxxxxxxxx>
Date: Mon, 13 Feb 2017 16:31:55 -0600
Cc: rpeterso@xxxxxxxxxx, linux-fsdevel@xxxxxxxxxxxxxxx, xfs@xxxxxxxxxxx
Delivered-to: xfs@xxxxxxxxxxx
In-reply-to: <20160802234247.GO16044@dastard>
References: <1464792297-13185-1-git-send-email-hch@xxxxxx> <20160628002649.GI12670@dastard> <20160630172239.GA23082@xxxxxx> <20160718111400.GC16044@dastard> <20160718111851.GD16044@dastard> <20160731191900.GA29301@xxxxxx> <20160802234247.GO16044@dastard>
User-agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.11; rv:45.0) Gecko/20100101 Thunderbird/45.7.1
On 8/2/16 6:42 PM, Dave Chinner wrote:
> On Sun, Jul 31, 2016 at 09:19:00PM +0200, Christoph Hellwig wrote:
>> Now after spending this much time I've started wondering why we even
>> reserve blocks in xfs_iomap_write_allocate - after all we've reserved
>> space for the actual data blocks and the indlen worst case in
>> xfs_bmapi_reserve_delalloc.  And in fact a little hack to drop that
>> reservation seems to solve both the root cause (depleted reserved pool)
>> and the cleanup mess.  I just haven't spend enought time to convince
>> myself that it's actually safe, and in fact looking at the allocator
>> makes me thing it only works by accident currently despite generally
>> postive test results.
>>
>> Here is the quick patch if anyone wants to chime in:
>>
>> diff --git a/fs/xfs/xfs_iomap.c b/fs/xfs/xfs_iomap.c
>> index 620fc91..67c317f 100644
>> --- a/fs/xfs/xfs_iomap.c
>> +++ b/fs/xfs/xfs_iomap.c
>> @@ -717,7 +717,7 @@ xfs_iomap_write_allocate(
>>  
>>              nimaps = 0;
>>              while (nimaps == 0) {
>> -                    nres = XFS_EXTENTADD_SPACE_RES(mp, XFS_DATA_FORK);
>> +                    nres = 0; // XFS_EXTENTADD_SPACE_RES(mp, XFS_DATA_FORK);
>>  
>>                      error = xfs_trans_alloc(mp, &M_RES(mp)->tr_write, nres,
>>                                      0, XFS_TRANS_RESERVE, &tp);
>>
> 
> This solves the problem for me, and from history appears to be the
> right thing to do. Christoph, can you send a proper patch for this?

Did anything ever come of this?  I don't think I saw a patch, and it looks
like it is not upstream.

Thanks,
-Eric

> Cheers,
> 
> Dave.
> 

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