[PATCH] xfs: check if reserved free disk blocks is needed

[PATCH] xfs: check if reserved free disk blocks is needed

[xiakaixu1987]

From: Kaixu Xia <kaixuxia@tencent.com>

We don't need to create the new quota inodes from disk when
they exist already, so add check if reserved free disk blocks
is needed in xfs_trans_alloc().

Signed-off-by: Kaixu Xia <kaixuxia@tencent.com>
---
 fs/xfs/xfs_qm.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/fs/xfs/xfs_qm.c b/fs/xfs/xfs_qm.c
index 6678baa..b684b04 100644
--- a/fs/xfs/xfs_qm.c
+++ b/fs/xfs/xfs_qm.c
@@ -780,7 +780,8 @@ struct xfs_qm_isolate {
 	}
 
 	error = xfs_trans_alloc(mp, &M_RES(mp)->tr_create,
-			XFS_QM_QINOCREATE_SPACE_RES(mp), 0, 0, &tp);
+			need_alloc ? XFS_QM_QINOCREATE_SPACE_RES(mp) : 0,
+			0, 0, &tp);
 	if (error)
 		return error;
 
-- 
1.8.3.1

Re: [PATCH] xfs: check if reserved free disk blocks is needed

[Brian Foster]

On Sun, Apr 05, 2020 at 05:17:19PM +0800, xiakaixu1987@gmail.com wrote:
> From: Kaixu Xia <kaixuxia@tencent.com>
> 
> We don't need to create the new quota inodes from disk when
> they exist already, so add check if reserved free disk blocks
> is needed in xfs_trans_alloc().
> 

I find the commit log to be a bit misleading. We don't actually get into
this code if the inodes exist already. The need_alloc == false case
looks like it has more to do with the scenario with the project/group
inode is shared on older superblocks (explained in the comment near the
top of the alloc function).

That aside, the code looks fine to me, so with an improved commit log:

Reviewed-by: Brian Foster <bfoster@redhat.com>

> Signed-off-by: Kaixu Xia <kaixuxia@tencent.com>
> ---
>  fs/xfs/xfs_qm.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/fs/xfs/xfs_qm.c b/fs/xfs/xfs_qm.c
> index 6678baa..b684b04 100644
> --- a/fs/xfs/xfs_qm.c
> +++ b/fs/xfs/xfs_qm.c
> @@ -780,7 +780,8 @@ struct xfs_qm_isolate {
>  	}
>  
>  	error = xfs_trans_alloc(mp, &M_RES(mp)->tr_create,
> -			XFS_QM_QINOCREATE_SPACE_RES(mp), 0, 0, &tp);
> +			need_alloc ? XFS_QM_QINOCREATE_SPACE_RES(mp) : 0,
> +			0, 0, &tp);
>  	if (error)
>  		return error;
>  
> -- 
> 1.8.3.1
> 

Re: [PATCH] xfs: check if reserved free disk blocks is needed

[kaixuxia]

On 2020/4/6 21:38, Brian Foster wrote:
> On Sun, Apr 05, 2020 at 05:17:19PM +0800, xiakaixu1987@gmail.com wrote:
>> From: Kaixu Xia <kaixuxia@tencent.com>
>>
>> We don't need to create the new quota inodes from disk when
>> they exist already, so add check if reserved free disk blocks
>> is needed in xfs_trans_alloc().
>>
> 
> I find the commit log to be a bit misleading. We don't actually get into
> this code if the inodes exist already. The need_alloc == false case
> looks like it has more to do with the scenario with the project/group
> inode is shared on older superblocks (explained in the comment near the
> top of the alloc function).
> 
> That aside, the code looks fine to me, so with an improved commit log:
> 
> Reviewed-by: Brian Foster <bfoster@redhat.com>
> 
Thanks for your comments, will send the new version with the improved
commit log and add your Reviewed-by.

>> Signed-off-by: Kaixu Xia <kaixuxia@tencent.com>
>> ---
>>  fs/xfs/xfs_qm.c | 3 ++-
>>  1 file changed, 2 insertions(+), 1 deletion(-)
>>
>> diff --git a/fs/xfs/xfs_qm.c b/fs/xfs/xfs_qm.c
>> index 6678baa..b684b04 100644
>> --- a/fs/xfs/xfs_qm.c
>> +++ b/fs/xfs/xfs_qm.c
>> @@ -780,7 +780,8 @@ struct xfs_qm_isolate {
>>  	}
>>  
>>  	error = xfs_trans_alloc(mp, &M_RES(mp)->tr_create,
>> -			XFS_QM_QINOCREATE_SPACE_RES(mp), 0, 0, &tp);
>> +			need_alloc ? XFS_QM_QINOCREATE_SPACE_RES(mp) : 0,
>> +			0, 0, &tp);
>>  	if (error)
>>  		return error;
>>  
>> -- 
>> 1.8.3.1
>>
> 

-- 
kaixuxia