On Fri, Jun 05, 2020 at 08:56:04AM -0700, Darrick J. Wong wrote: > On Fri, Jun 05, 2020 at 04:52:00PM +0800, Gao Xiang wrote: ... > > --- > > changes since v2: > > kill unneeded ASSERTs, leaving which first brought > > into a context pointed out by Dave (including callback > > entrances). ... > > > > @@ -3006,7 +2999,8 @@ xfs_alloc_read_agf( > > ASSERT(!(*bpp)->b_error); > > > > agf = (*bpp)->b_addr; > > - pag = xfs_perag_get(mp, agno); > > + pag = (*bpp)->b_pag; > > + ASSERT(pag->pag_agno == agno); > > I thought these assertions were all dropped in v3? The ASSERT above is in xfs_alloc_read_agf. If I didn't misread what Dave said, I think this is necessary at least. > > Alternately-- if you want to sanity check that b_pag and the buffer > belong to the same ag, why not do that in xfs_buf_find for all the > buffers? Since that modification doesn't relate to this patch though (since the purpose of this patch is not add ASSERT to xfs_buf_find). If in that way, I think we can just kill all these ASSERTs. > > > ASSERT(ptr->s != 0); > > + ASSERT(pag->pag_agno == be32_to_cpu(agf->agf_seqno)); > > I still see a few of these after-the-fact agno checks throughout the patch. what I wrote above is: > > into a context pointed out by Dave (including callback > > entrances). I can leave these xfs_alloc_read_agf, xfs_ialloc_read_agi 2 assertions only if you want. Thanks, Gao Xiang > > --D >
