Re: [PATCH v3] xfs: get rid of unnecessary xfs_perag_{get,put} pairs

[Gao Xiang <hsiangkao@redhat.com>]

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
>