On Fri, Nov 24, 2023 at 10:11:18PM -0800, Christoph Hellwig wrote: > On Fri, Nov 24, 2023 at 03:50:33PM -0800, Darrick J. Wong wrote: > > From: Darrick J. Wong <djwong@xxxxxxxxxx> > > > > Rebuild the free space btrees from the gaps in the rmap btree. > > This commit message feels a bit sparse for the amount of code added, > although I can't really offer a good idea of what to add. "Refer to the design documentation for more details: Link: https://docs.kernel.org/filesystems/xfs-online-fsck-design.html?highlight=xfs#case-study-rebuilding-the-free-space-indices" ? > Otherwise just two comments on the interaction with the rest of the > xfs code, I'll try to digest the new repair code a bit more in the > meantime. > > > +#ifdef CONFIG_XFS_ONLINE_REPAIR > > + /* > > + * Alternate btree heights so that online repair won't trip the write > > + * verifiers while rebuilding the AG btrees. > > + */ > > + uint8_t pagf_alt_levels[XFS_BTNUM_AGF]; > > +#endif > > Alternate and the alt_ prefix doesn't feel very descriptive. As far as > I can tell these are about an ongoign repair, so as a at lest somewhat > better choice call it "pagf_repair_levels"? Done. > > +xfs_failaddr_t > > +xfs_alloc_check_irec( > > + struct xfs_btree_cur *cur, > > + const struct xfs_alloc_rec_incore *irec) > > +{ > > + return xfs_alloc_check_perag_irec(cur->bc_ag.pag, irec); > > +} > > Is there much of a point in even keeping this wrapper vs just > switching xfs_alloc_check_irec to pass the pag instead of the > cursor? Not really. I'll remove this from the next spin. --D