diff mbox series

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

Message ID 161284383258.3057868.6167060787728229726.stgit@magnolia (mailing list archive)
State Superseded
Headers show
Series xfs: add the ability to flag a fs for repair | expand

Commit Message

Darrick J. Wong Feb. 9, 2021, 4:10 a.m. UTC 
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(-)

Comments

Christoph Hellwig Feb. 9, 2021, 9:10 a.m. UTC | #1 
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>

Looks good,

Reviewed-by: Christoph Hellwig <hch@lst.de>
Brian Foster Feb. 9, 2021, 5:20 p.m. UTC | #2 
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:

Reviewed-by: Brian Foster <bfoster@redhat.com>

>  	}
>  
>  	if (copied_sunit) {
>
Darrick J. Wong Feb. 9, 2021, 5:46 p.m. UTC | #3 
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) {
> > 
>
diff mbox series

Patch

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);
 	}
 
 	if (copied_sunit) {