Re: [PATCH] xfs: Remove unnecessary checks in functions related to xfs_fsmap

[Nirjhar Roy (IBM)]

On Sat, 2025-05-17 at 15:43 +0800, Zizhi Wo wrote:
> From: Zizhi Wo <wozizhi@xxxxxxxxxxxxxxx>
> 
> In __xfs_getfsmap_datadev(), if "pag_agno(pag) == end_ag", we don't need
> to check the result of query_fn(), because there won't be another iteration
Yes, this makes sense to me. Once pag_agno(pag) == end_ag, that will be the last iteration. "error"
is used later after coming out of the while loop. This looks good to me. 
Feel free to add

Reviewed-by: Nirjhar Roy (IBM) <nirjhar.roy.lists@xxxxxxxxx>

> of the loop anyway. Also, both before and after the change, info->group
> will eventually be set to NULL and the reference count of xfs_group will
> also be decremented before exiting the iteration.
> 
> The same logic applies to other similar functions as well, so related
> cleanup operations are performed together.
> 
> Signed-off-by: Zizhi Wo <wozizhi@xxxxxxxxxxxxxxx>
> Signed-off-by: Zizhi Wo <wozizhi@xxxxxxxxxx>
> ---
>  fs/xfs/xfs_fsmap.c | 6 ------
>  1 file changed, 6 deletions(-)
> 
> diff --git a/fs/xfs/xfs_fsmap.c b/fs/xfs/xfs_fsmap.c
> index 414b27a86458..792282aa8a29 100644
> --- a/fs/xfs/xfs_fsmap.c
> +++ b/fs/xfs/xfs_fsmap.c
> @@ -579,8 +579,6 @@ __xfs_getfsmap_datadev(
>  		if (pag_agno(pag) == end_ag) {
>  			info->last = true;
>  			error = query_fn(tp, info, &bt_cur, priv);
> -			if (error)
> -				break;
>  		}
>  		info->group = NULL;
>  	}
> @@ -813,8 +811,6 @@ xfs_getfsmap_rtdev_rtbitmap(
>  			info->last = true;
>  			error = xfs_getfsmap_rtdev_rtbitmap_helper(rtg, tp,
>  					&ahigh, info);
> -			if (error)
> -				break;
>  		}
>  
>  		xfs_rtgroup_unlock(rtg, XFS_RTGLOCK_BITMAP_SHARED);
> @@ -1018,8 +1014,6 @@ xfs_getfsmap_rtdev_rmapbt(
>  			info->last = true;
>  			error = xfs_getfsmap_rtdev_rmapbt_helper(bt_cur,
>  					&info->high, info);
> -			if (error)
> -				break;
>  		}
>  		info->group = NULL;
>  	}