On Sat, Apr 07, 2018 at 11:04:48AM -0700, Darrick J. Wong wrote:
> On Fri, Apr 06, 2018 at 11:05:20AM +1000, Dave Chinner wrote:
> > On Mon, Apr 02, 2018 at 12:57:24PM -0700, Darrick J. Wong wrote:
> > > From: Darrick J. Wong <darrick.wong@xxxxxxxxxx>
> > >
> > > If one of the backup superblocks is found to differ seriously from
> > > superblock 0, write out a fresh copy from the in-core sb.
> > >
> > > Signed-off-by: Darrick J. Wong <darrick.wong@xxxxxxxxxx>
> > .....
> > > +/* Repair the superblock. */
> > > +int
> > > +xfs_repair_superblock(
> > > + struct xfs_scrub_context *sc)
> > > +{
> > > + struct xfs_mount *mp = sc->mp;
> > > + struct xfs_buf *bp;
> > > + struct xfs_dsb *sbp;
> > > + xfs_agnumber_t agno;
> > > + int error;
> > > +
> > > + /* Don't try to repair AG 0's sb; let xfs_repair deal with it. */
> > > + agno = sc->sm->sm_agno;
> > > + if (agno == 0)
> > > + return -EOPNOTSUPP;
> > > +
> > > + error = xfs_trans_read_buf(mp, sc->tp, mp->m_ddev_targp,
> > > + XFS_AG_DADDR(mp, agno, XFS_SB_BLOCK(mp)),
> > > + XFS_FSS_TO_BB(mp, 1), 0, &bp, NULL);
> >
> > If we are just overwriting the superblock, then use
> > xfs_trans_get_buf() so we don't need to do IO here.
>
> Ok.
>
> > > + if (error)
> > > + return error;
> > > + bp->b_ops = &xfs_sb_buf_ops;
> > > +
> > > + /* Copy AG 0's superblock to this one. */
> > > + sbp = XFS_BUF_TO_SBP(bp);
> > > + memset(sbp, 0, mp->m_sb.sb_sectsize);
> >
> > That's a bit nasty. struct xfs_dsb is not a sector size in length.
> >
> > xfs_buf_zero(bp, 0, BBTOB(bp->b_length));
>
> Fixed.
>
> > > + xfs_sb_to_disk(sbp, &mp->m_sb);
> > > + sbp->sb_bad_features2 = sbp->sb_features2;
> >
> > Why is sb_bad_features2 not correct when taken from the incore
> > superblock?
>
> It is correct, this is unnecessary.
>
> > > + /* Write this to disk. */
> > > + xfs_trans_buf_set_type(sc->tp, bp, XFS_BLFT_SB_BUF);
> > > + xfs_trans_log_buf(sc->tp, bp, 0, mp->m_sb.sb_sectsize - 1);
> >
> > And why log this rather than just write it straight to disk? We've
> > never logged secondary superblocks before, so why now?
>
> Ok, will do. I might as well change the sb scrubber to use uncached
> buffers while I'm at it (separate patch, obviously).
We discussed this with my growfs rework that used uncached buffers -
if there's a concurrent growfs then the secondary superblock buffers
need to be coherent and access correctly serialised. Hence they need to
remain cached buffers. What we need to do is make them single use
cached buffers, so they are discarded immediately the last reference
goes away and are not put on the lru....
Cheers,
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
