Re: [PATCH v2 3/4] xfs: terminate perag iteration reliably on agcount

["Darrick J. Wong" <djwong@kernel.org>]

On Tue, Oct 12, 2021 at 12:52:02PM -0400, Brian Foster wrote:
> The for_each_perag_from() iteration macro relies on sb_agcount to
> process every perag currently within EOFS from a given starting
> point. It's perfectly valid to have perag structures beyond
> sb_agcount, however, such as if a growfs is in progress. If a perag
> loop happens to race with growfs in this manner, it will actually
> attempt to process the post-EOFS perag where ->pag_agno ==
> sb_agcount. This is reproduced by xfs/104 and manifests as the
> following assert failure in superblock write verifier context:
> 
>  XFS: Assertion failed: agno < mp->m_sb.sb_agcount, file: fs/xfs/libxfs/xfs_types.c, line: 22
> 
> Update the corresponding macro to only process perags that are
> within the current sb_agcount.

Does this need a Fixes: tag?

Also ... should we be checking for agno <= agcount-1 for the initial
xfs_perag_get in the first for loop clause of for_each_perag_range?
I /think/ the answer is that the current users are careful enough to
check that race, but I haven't looked exhaustively.

Welcome back, by the way. :)

--D

> Signed-off-by: Brian Foster <bfoster@redhat.com>
> ---
>  fs/xfs/libxfs/xfs_ag.h | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/fs/xfs/libxfs/xfs_ag.h b/fs/xfs/libxfs/xfs_ag.h
> index cf8baae2ba18..b8cc5017efba 100644
> --- a/fs/xfs/libxfs/xfs_ag.h
> +++ b/fs/xfs/libxfs/xfs_ag.h
> @@ -142,7 +142,7 @@ struct xfs_perag *xfs_perag_next(
>  		(pag) = xfs_perag_next((pag), &(agno)))
>  
>  #define for_each_perag_from(mp, agno, pag) \
> -	for_each_perag_range((mp), (agno), (mp)->m_sb.sb_agcount, (pag))
> +	for_each_perag_range((mp), (agno), (mp)->m_sb.sb_agcount - 1, (pag))
>  
>  
>  #define for_each_perag(mp, agno, pag) \
> -- 
> 2.31.1
>