Re: [PATCH 1/7] xfs: pass the exact range to initialize to xfs_initialize_perag

[Christoph Hellwig <hch@lst.de>]

On Thu, Oct 10, 2024 at 10:02:49AM -0400, Brian Foster wrote:
> > -	error = xfs_initialize_perag(mp, sbp->sb_agcount, sbp->sb_dblocks,
> > -			&mp->m_maxagi);
> > +	error = xfs_initialize_perag(mp, old_agcount, sbp->sb_agcount,
> > +			sbp->sb_dblocks, &mp->m_maxagi);
> 
> I assume this is because the superblock can change across recovery, but
> code wise this seems kind of easy to misread into thinking the variable
> is the same.

Which variable?

> I think the whole old/new terminology is kind of clunky for
> an interface that is not just for growfs. Maybe it would be more clear
> to use start/end terminology for xfs_initialize_perag(), then it's more
> straightforward that mount would init the full range whereas growfs
> inits a subrange.

fine with me.

> A oneliner comment or s/old_agcount/orig_agcount/ wouldn't hurt here
> either. Actually if that's the only purpose for this call and if you
> already have to sample sb_agcount, maybe just lifting/copying the if
> (old_agcount >= new_agcount) check into the caller would make the logic
> more self-explanatory. Hm?

Sure.