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@xxxxxxxxxx> > --- > 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 >
