[PATCH v2] xfs: trim the mapp array accordingly in xfs_da_grow_inode_int

[PATCH v2] xfs: trim the mapp array accordingly in xfs_da_grow_inode_int

[Stephen Zhang]

Take a look at the for-loop in xfs_da_grow_inode_int:
======
for(){
        nmap = min(XFS_BMAP_MAX_NMAP, count);
        ...
        error = xfs_bmapi_write(...,&mapp[mapi], &nmap);//(..., $1, $2)
        ...
        mapi += nmap;
}
=====
where $1 stands for the start address of the array,
while $2 is used to indicate the size of the array.

The array $1 will advance by $nmap in each iteration after
the allocation of extents.
But the size $2 still remains unchanged, which is determined by
min(XFS_BMAP_MAX_NMAP, count).

It seems that it has forgotten to trim the mapp array after each
iteration, so change it.

Signed-off-by: Shida Zhang <zhangshida@kylinos.cn>
---
Changes from v1:
- Using the current calculation to calculate the remaining number of
  blocks is enough, as suggested by Dave.
---
 fs/xfs/libxfs/xfs_da_btree.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/fs/xfs/libxfs/xfs_da_btree.c b/fs/xfs/libxfs/xfs_da_btree.c
index e7201dc68f43..e576560b46e9 100644
--- a/fs/xfs/libxfs/xfs_da_btree.c
+++ b/fs/xfs/libxfs/xfs_da_btree.c
@@ -2192,8 +2192,8 @@ xfs_da_grow_inode_int(
 		 */
 		mapp = kmem_alloc(sizeof(*mapp) * count, 0);
 		for (b = *bno, mapi = 0; b < *bno + count; ) {
-			nmap = min(XFS_BMAP_MAX_NMAP, count);
 			c = (int)(*bno + count - b);
+			nmap = min(XFS_BMAP_MAX_NMAP, c);
 			error = xfs_bmapi_write(tp, dp, b, c,
 					xfs_bmapi_aflag(w)|XFS_BMAPI_METADATA,
 					args->total, &mapp[mapi], &nmap);
-- 
2.27.0

Re: [PATCH v2] xfs: trim the mapp array accordingly in xfs_da_grow_inode_int

[Darrick J. Wong]

On Sun, Sep 18, 2022 at 02:48:08PM +0800, Stephen Zhang wrote:
> Take a look at the for-loop in xfs_da_grow_inode_int:
> ======
> for(){
>         nmap = min(XFS_BMAP_MAX_NMAP, count);
>         ...
>         error = xfs_bmapi_write(...,&mapp[mapi], &nmap);//(..., $1, $2)
>         ...
>         mapi += nmap;
> }
> =====
> where $1 stands for the start address of the array,
> while $2 is used to indicate the size of the array.
> 
> The array $1 will advance by $nmap in each iteration after
> the allocation of extents.
> But the size $2 still remains unchanged, which is determined by
> min(XFS_BMAP_MAX_NMAP, count).
> 
> It seems that it has forgotten to trim the mapp array after each
> iteration, so change it.
> 
> Signed-off-by: Shida Zhang <zhangshida@kylinos.cn>

I think this look correct...
Reviewed-by: Darrick J. Wong <djwong@kernel.org>

--D

> ---
> Changes from v1:
> - Using the current calculation to calculate the remaining number of
>   blocks is enough, as suggested by Dave.
> ---
>  fs/xfs/libxfs/xfs_da_btree.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/fs/xfs/libxfs/xfs_da_btree.c b/fs/xfs/libxfs/xfs_da_btree.c
> index e7201dc68f43..e576560b46e9 100644
> --- a/fs/xfs/libxfs/xfs_da_btree.c
> +++ b/fs/xfs/libxfs/xfs_da_btree.c
> @@ -2192,8 +2192,8 @@ xfs_da_grow_inode_int(
>  		 */
>  		mapp = kmem_alloc(sizeof(*mapp) * count, 0);
>  		for (b = *bno, mapi = 0; b < *bno + count; ) {
> -			nmap = min(XFS_BMAP_MAX_NMAP, count);
>  			c = (int)(*bno + count - b);
> +			nmap = min(XFS_BMAP_MAX_NMAP, c);
>  			error = xfs_bmapi_write(tp, dp, b, c,
>  					xfs_bmapi_aflag(w)|XFS_BMAPI_METADATA,
>  					args->total, &mapp[mapi], &nmap);
> -- 
> 2.27.0
>