Messages in this thread | ![/](/images/icornerl.gif) | | Subject | Re: [PATCH v3 3/9] xfs: make xfs_bmapi_convert_delalloc() to allocate the target offset | From | Zhang Yi <> | Date | Wed, 20 Mar 2024 09:51:26 +0800 |
| |
On 2024/3/20 4:45, Darrick J. Wong wrote: > On Tue, Mar 19, 2024 at 09:10:56AM +0800, Zhang Yi wrote: >> From: Zhang Yi <yi.zhang@huawei.com> >> >> Since xfs_bmapi_convert_delalloc() only attempts to allocate the entire >> delalloc extent and require multiple invocations to allocate the target >> offset. So xfs_convert_blocks() add a loop to do this job and we call it >> in the write back path, but xfs_convert_blocks() isn't a common helper. >> Let's do it in xfs_bmapi_convert_delalloc() and drop >> xfs_convert_blocks(), preparing for the post EOF delalloc blocks >> converting in the buffered write begin path. >> >> Signed-off-by: Zhang Yi <yi.zhang@huawei.com> >> Reviewed-by: Christoph Hellwig <hch@lst.de> >> --- >> fs/xfs/libxfs/xfs_bmap.c | 34 +++++++++++++++++++++++-- >> fs/xfs/xfs_aops.c | 54 +++++++++++----------------------------- >> 2 files changed, 46 insertions(+), 42 deletions(-) >> >> diff --git a/fs/xfs/libxfs/xfs_bmap.c b/fs/xfs/libxfs/xfs_bmap.c >> index 07dc35de8ce5..042e8d3ab0ba 100644 >> --- a/fs/xfs/libxfs/xfs_bmap.c >> +++ b/fs/xfs/libxfs/xfs_bmap.c >> @@ -4516,8 +4516,8 @@ xfs_bmapi_write( >> * invocations to allocate the target offset if a large enough physical extent >> * is not available. >> */ >> -int >> -xfs_bmapi_convert_delalloc( >> +static int > > static inline? >
I'd suggest to leave that to the compiler too.
>> +__xfs_bmapi_convert_delalloc( > > Double underscore prefixes read to me like "do this without grabbing > a lock or a resource", not just one step in a loop. > > Would you mind changing it to xfs_bmapi_convert_one_delalloc() ? > Then the callsite looks like: > > xfs_bmapi_convert_delalloc(...) > { > ... > do { > error = xfs_bmapi_convert_one_delalloc(ip, whichfork, offset, > iomap, seq); > if (error) > return error; > } while (iomap->offset + iomap->length <= offset); > } >
Thanks for your suggestions, all subsequent improvements in this series are also looks fine by me, I will revise them in my next iteration. Christoph, I will keep your review tag, please let me know if you have different opinion.
Thanks, Yi.
| ![\](/images/icornerr.gif) |