Re: [PATCH 7/8] xfs: create helpers for rtsummary block/wordcount computations

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

On Fri, Oct 13, 2023 at 06:29:47AM +0200, Christoph Hellwig wrote:
> On Thu, Oct 12, 2023 at 03:18:36PM -0700, Darrick J. Wong wrote:
> > > > @@ -54,8 +55,10 @@ xchk_setup_rtsummary(
> > > >  	 * Create an xfile to construct a new rtsummary file.  The xfile allows
> > > >  	 * us to avoid pinning kernel memory for this purpose.
> > > >  	 */
> > > > +	wordcnt = xfs_rtsummary_wordcount(mp, mp->m_rsumlevels,
> > > > +			mp->m_sb.sb_rbmblocks);
> > > >  	descr = xchk_xfile_descr(sc, "realtime summary file");
> > > > -	error = xfile_create(descr, mp->m_rsumsize, &sc->xfile);
> > > > +	error = xfile_create(descr, wordcnt << XFS_WORDLOG, &sc->xfile);
> > > >  	kfree(descr);
> > > 
> > > But this confuses me.  What problem does it solve over just using
> > > m_rsumsize?
> > 
> > The rtbitmap and rtsummary repair code should be computing rbmblocks and
> > rsumsize from sb_rextents.
> > 
> > rbmblocks = xfs_rtbitmap_wordcount(mp, mp->m_sb.sb_rextents);
> > rsumsize = xfs_rtsummary_wordcount(mp, mp->m_rsumlevels, rbmblocks);
> > 
> > >From that, it should be checking isize and the data fork mappings of
> > the file and the superblock values.  Repair ought to map (or unmap)
> > blocks as necessary, update isize if needed, and update the superblock
> > if the values there are incorrect.
> 
> So this is really a feature path that should be documented as such
> and not just be about adding a helper?

Yeah.  If these rt cleanups land before online repair, then that whole
hunk won't be in the patch that we merge anyway.  In that case, I'll
want the helpers simply to reduce the number of things I have to keep
track of.

(meanwhile I think the pre-rtgroups rtbitmap and rtsummary code need
some fixing...)

--D