Re: [PATCH 05/10] xfs_repair: clear quota CHKD flags on the incore superblock too

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

On Tue, Feb 09, 2021 at 12:20:11PM -0500, Brian Foster wrote:
> On Mon, Feb 08, 2021 at 08:10:32PM -0800, Darrick J. Wong wrote:
> > From: Darrick J. Wong <djwong@kernel.org>
> > 
> > At the end of a repair run, xfs_repair clears the superblock's quota
> > checked flags if it found mistakes in the quota accounting to force a
> > quotacheck at the next mount.  This is currently the last time repair
> > modifies the primary superblock, so it is sufficient to update the
> > ondisk buffer and not the incore mount structure.
> > 
> > However, we're about to introduce code to clear the needsrepair feature
> > at the very end of repair, after all metadata blocks have been written
> > to disk and all disk caches flush.  Since the convention everywhere else
> > in xfs is to update the incore superblock, call libxfs_sb_to_disk to
> > translate that into the ondisk buffer, and then write the buffer to
> > disk, switch the quota CHKD code to use this mechanism too.
> > 
> > Signed-off-by: Darrick J. Wong <djwong@kernel.org>
> > ---
> >  repair/xfs_repair.c |    7 +++----
> >  1 file changed, 3 insertions(+), 4 deletions(-)
> > 
> > 
> > diff --git a/repair/xfs_repair.c b/repair/xfs_repair.c
> > index 9409f0d8..32755821 100644
> > --- a/repair/xfs_repair.c
> > +++ b/repair/xfs_repair.c
> > @@ -1108,10 +1108,9 @@ _("Warning:  project quota information would be cleared.\n"
> >  	if ((mp->m_sb.sb_qflags & XFS_ALL_QUOTA_CHKD) != quotacheck_results()) {
> >  		do_warn(_("Note - quota info will be regenerated on next "
> >  			"quota mount.\n"));
> > -		dsb->sb_qflags &= cpu_to_be16(~(XFS_UQUOTA_CHKD |
> > -						XFS_GQUOTA_CHKD |
> > -						XFS_PQUOTA_CHKD |
> > -						XFS_OQUOTA_CHKD));
> > +		mp->m_sb.sb_qflags &= ~(XFS_UQUOTA_CHKD | XFS_GQUOTA_CHKD |
> > +					XFS_PQUOTA_CHKD | XFS_OQUOTA_CHKD);
> > +		libxfs_sb_to_disk(sbp->b_addr, &mp->m_sb);
> 
> Nit: we can still use dsb here instead of open-coding the buffer
> address. Otherwise:

Or I could remove dsb entirely, since the xfs_mount's sb_unit/sb_width
should reflect the values copied from the backup super during phase 1
(aka before we actually libxfs_mount) when we set copied_sunit=1.

--D

> 
> Reviewed-by: Brian Foster <bfoster@redhat.com>
> 
> >  	}
> >  
> >  	if (copied_sunit) {
> > 
>