On Tue, May 15, 2018 at 03:33:45PM -0700, Darrick J. Wong wrote:
> From: Darrick J. Wong <darrick.wong@xxxxxxxxxx>
>
> For repairs, we need to reserve at least as many blocks as we think
> we're going to need to rebuild the data structure, and we're going to
> need some helpers to roll transactions while maintaining locks on the AG
> headers so that other threads cannot wander into the middle of a repair.
>
> Signed-off-by: Darrick J. Wong <darrick.wong@xxxxxxxxxx>
> ---
> fs/xfs/scrub/bmap.c | 2 -
> fs/xfs/scrub/common.c | 21 ++++++-
> fs/xfs/scrub/common.h | 2 -
> fs/xfs/scrub/inode.c | 4 +
> fs/xfs/scrub/repair.c | 152 +++++++++++++++++++++++++++++++++++++++++++++++++
> fs/xfs/scrub/repair.h | 12 ++++
> 6 files changed, 186 insertions(+), 7 deletions(-)
mostly looks good.
[...]
> +/*
> + * Figure out how many blocks to reserve for an AG repair. We calculate the
> + * worst case estimate for the number of blocks we'd need to rebuild one of
> + * any type of per-AG btree.
> + */
> +xfs_extlen_t
> +xfs_repair_calc_ag_resblks(
> + struct xfs_scrub_context *sc)
> +{
> + struct xfs_mount *mp = sc->mp;
> + struct xfs_scrub_metadata *sm = sc->sm;
> + struct xfs_perag *pag;
> + struct xfs_buf *bp;
> + xfs_agino_t icount = 0;
> + xfs_extlen_t aglen = 0;
> + xfs_extlen_t usedlen;
> + xfs_extlen_t freelen;
> + xfs_extlen_t bnobt_sz;
> + xfs_extlen_t inobt_sz;
> + xfs_extlen_t rmapbt_sz;
> + xfs_extlen_t refcbt_sz;
> + int error;
> +
> + if (!(sm->sm_flags & XFS_SCRUB_IFLAG_REPAIR))
> + return 0;
> +
> + /* Use in-core counters if possible. */
> + pag = xfs_perag_get(mp, sm->sm_agno);
> + if (pag->pagi_init)
> + icount = pag->pagi_count;
> + xfs_perag_put(pag);
Don't drop the pag here, do it ....
> +
> + /*
> + * Otherwise try to get the actual counters from disk; if not, make
> + * some worst case assumptions.
> + */
> + if (icount == 0) {
> + error = xfs_ialloc_read_agi(mp, NULL, sm->sm_agno, &bp);
> + if (error) {
> + icount = mp->m_sb.sb_agblocks / mp->m_sb.sb_inopblock;
> + } else {
> + pag = xfs_perag_get(mp, sm->sm_agno);
> + icount = pag->pagi_count;
> + xfs_perag_put(pag);
> + xfs_buf_relse(bp);
> + }
> + }
> +
> + /* Now grab the block counters from the AGF. */
> + error = xfs_alloc_read_agf(mp, NULL, sm->sm_agno, 0, &bp);
> + if (error) {
> + aglen = mp->m_sb.sb_agblocks;
> + freelen = aglen;
> + usedlen = aglen;
> + } else {
> + pag = xfs_perag_get(mp, sm->sm_agno);
> + aglen = be32_to_cpu(XFS_BUF_TO_AGF(bp)->agf_length);
> + freelen = pag->pagf_freeblks;
> + usedlen = aglen - freelen;
> + xfs_perag_put(pag);
> + xfs_buf_relse(bp);
> + }
.... here, so we don't have to do repeated lookups.
> + trace_xfs_repair_calc_ag_resblks(mp, sm->sm_agno, icount, aglen,
> + freelen, usedlen);
> +
> + /*
> + * Figure out how many blocks we'd need worst case to rebuild
> + * each type of btree. Note that we can only rebuild the
> + * bnobt/cntbt or inobt/finobt as pairs.
> + */
> + bnobt_sz = 2 * xfs_allocbt_calc_size(mp, freelen);
> + if (xfs_sb_version_hassparseinodes(&mp->m_sb))
> + inobt_sz = xfs_iallocbt_calc_size(mp, icount /
> + XFS_INODES_PER_HOLEMASK_BIT);
> + else
> + inobt_sz = xfs_iallocbt_calc_size(mp, icount /
> + XFS_INODES_PER_CHUNK);
> + if (xfs_sb_version_hasfinobt(&mp->m_sb))
> + inobt_sz *= 2;
> + if (xfs_sb_version_hasreflink(&mp->m_sb)) {
> + rmapbt_sz = xfs_rmapbt_calc_size(mp, aglen);
> + refcbt_sz = xfs_refcountbt_calc_size(mp, usedlen);
> + } else {
> + rmapbt_sz = xfs_rmapbt_calc_size(mp, usedlen);
> + refcbt_sz = 0;
> + }
> + if (!xfs_sb_version_hasrmapbt(&mp->m_sb))
> + rmapbt_sz = 0;
This looks kinda whacky. reflink and rmapbt are different features,
maybe a comment to explain why the rmapbt size calc is done all
back to front?
Otherwise looks ok.
Reviewed-by: Dave Chinner <dchinner@xxxxxxxxxx>
--
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
