On Wed, May 16, 2018 at 05:07:46PM +1000, Dave Chinner wrote:
> On Tue, May 15, 2018 at 03:33:51PM -0700, Darrick J. Wong wrote:
> > From: Darrick J. Wong <darrick.wong@xxxxxxxxxx>
> >
> > Add a pair of helper functions to allocate and initialize fresh btree
> > roots. The repair functions will use these as part of recreating
> > corrupted metadata.
> >
> > Signed-off-by: Darrick J. Wong <darrick.wong@xxxxxxxxxx>
> > ---
> > fs/xfs/scrub/repair.c | 81 +++++++++++++++++++++++++++++++++++++++++++++++++
> > fs/xfs/scrub/repair.h | 6 ++++
> > 2 files changed, 87 insertions(+)
>
> Looks good, but....
>
> > +/* Initialize a new AG btree root block with zero entries. */
> > +int
> > +xfs_repair_init_btblock(
> > + struct xfs_scrub_context *sc,
> > + xfs_fsblock_t fsb,
> > + struct xfs_buf **bpp,
> > + xfs_btnum_t btnum,
> > + const struct xfs_buf_ops *ops)
> > +{
> > + struct xfs_trans *tp = sc->tp;
> > + struct xfs_mount *mp = sc->mp;
> > + struct xfs_buf *bp;
> > +
> > + trace_xfs_repair_init_btblock(mp, XFS_FSB_TO_AGNO(mp, fsb),
> > + XFS_FSB_TO_AGBNO(mp, fsb), btnum);
> > +
> > + ASSERT(XFS_FSB_TO_AGNO(mp, fsb) == sc->sa.agno);
> > + bp = xfs_trans_get_buf(tp, mp->m_ddev_targp, XFS_FSB_TO_DADDR(mp, fsb),
> > + XFS_FSB_TO_BB(mp, 1), 0);
> > + xfs_buf_zero(bp, 0, BBTOB(bp->b_length));
> > + xfs_btree_init_block(mp, bp, btnum, 0, 0, sc->sa.agno,
> > + XFS_BTREE_CRC_BLOCKS);
>
> This flag does nothing in xfs_btree_init_block(). Any reason for
> setting it?
Nope. Will fix.
> With that fixed, though,
>
> Reviewed-by: Dave Chinner <dchinner@xxxxxxxxxx>
>
> > + xfs_trans_buf_set_type(tp, bp, XFS_BLFT_BTREE_BUF);
> > + xfs_trans_log_buf(tp, bp, 0, bp->b_length);
> > + bp->b_ops = ops;
> > + *bpp = bp;
> > +
> > + return 0;
> > +}
>
> For followup patches, I think there's some overlap with the new
> libxfs/xfs_ag.c functions for initialising new btree root blocks for
> growfs? e.g. Make the struct xfs_aghdr_grow_data aghdr_data[] array
> a static global, and then we can do something like:
>
> bp = xfs_ag_init_btroot(mp, tp, agno, agbno, btnum);
> xfs_trans_log_buf(tp, bp, 0, bp->b_length);
>
> and all the details of reading the btree block and setting it up go
> away from this code....
I'm not sure aghdr_data can be made static global since parts of it
depend on the struct xfs_mount, but at the very least we should try to
refactor this (in the followup series) to simplify the repair code.
There may be complications relating to the xfs_*root_init functions
initializing default records that are appropriate for a freshly created
AG that are totally wrong for regenerating a broken AG, and in any case
repair really requires totally empty root blocks since it has already
generated the list of records it's going to put into the new btree.
--D
> -Dave.
> --
> Dave Chinner
> david@xxxxxxxxxxxxx
> --
> To unsubscribe from this list: send the line "unsubscribe linux-xfs" in
> the body of a message to majordomo@xxxxxxxxxxxxxxx
> More majordomo info at http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe linux-xfs" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
