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;
> }
