[PATCH] xfs: change xfs_update_secondary_supers to use xfS_sb_read_secondary

[PATCH] xfs: change xfs_update_secondary_supers to use xfS_sb_read_secondary

[Darrick J. Wong]

From: Darrick J. Wong <darrick.wong@oracle.com>

Use the new helper to read secondary superblocks instead of opencoding
it ourselves.

Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
---
 fs/xfs/xfs_fsops.c |   10 ++++------
 1 file changed, 4 insertions(+), 6 deletions(-)

diff --git a/fs/xfs/xfs_fsops.c b/fs/xfs/xfs_fsops.c
index 056a76689197..40c131ebf772 100644
--- a/fs/xfs/xfs_fsops.c
+++ b/fs/xfs/xfs_fsops.c
@@ -191,18 +191,15 @@ xfs_growfs_data_private(
  */
 int
 xfs_update_secondary_supers(
-	xfs_mount_t		*mp)
+	struct xfs_mount	*mp)
 {
-	int			error, saved_error;
+	struct xfs_buf		*bp;
 	xfs_agnumber_t		agno;
-	xfs_buf_t		*bp;
+	int			error, saved_error;
 
 	error = saved_error = 0;
 
 	for (agno = 1; agno < mp->m_sb.sb_agcount; agno++) {
-		error = xfs_trans_read_buf(mp, NULL, mp->m_ddev_targp,
-			  XFS_AGB_TO_DADDR(mp, agno, XFS_SB_BLOCK(mp)),
-			  XFS_FSS_TO_BB(mp, 1), 0, &bp, &xfs_sb_buf_ops);
 		/*
 		 * If we get an error reading or writing alternate superblocks,
 		 * continue.  xfs_repair chooses the "best" superblock based
@@ -210,6 +207,7 @@ xfs_update_secondary_supers(
 		 * superblocks un-updated than updated, and xfs_repair may
 		 * pick them over the properly-updated primary.
 		 */
+		error = xfs_sb_read_secondary(mp, NULL, agno, &bp);
 		if (error) {
 			xfs_warn(mp,
 		"error %d reading secondary superblock for ag %d",

Re: [PATCH] xfs: change xfs_update_secondary_supers to use xfS_sb_read_secondary

[Eric Sandeen]

On 5/15/18 3:47 PM, Darrick J. Wong wrote:
> From: Darrick J. Wong <darrick.wong@oracle.com>
> 
> Use the new helper to read secondary superblocks instead of opencoding
> it ourselves.
> 
> Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>

Thanks, sorry I forgot^Wdidn't know that existed.

Reviewed-by: Eric Sandeen <sandeen@redhat.com>

> ---
>   fs/xfs/xfs_fsops.c |   10 ++++------
>   1 file changed, 4 insertions(+), 6 deletions(-)
> 
> diff --git a/fs/xfs/xfs_fsops.c b/fs/xfs/xfs_fsops.c
> index 056a76689197..40c131ebf772 100644
> --- a/fs/xfs/xfs_fsops.c
> +++ b/fs/xfs/xfs_fsops.c
> @@ -191,18 +191,15 @@ xfs_growfs_data_private(
>    */
>   int
>   xfs_update_secondary_supers(
> -	xfs_mount_t		*mp)
> +	struct xfs_mount	*mp)
>   {
> -	int			error, saved_error;
> +	struct xfs_buf		*bp;
>   	xfs_agnumber_t		agno;
> -	xfs_buf_t		*bp;
> +	int			error, saved_error;
>   
>   	error = saved_error = 0;
>   
>   	for (agno = 1; agno < mp->m_sb.sb_agcount; agno++) {
> -		error = xfs_trans_read_buf(mp, NULL, mp->m_ddev_targp,
> -			  XFS_AGB_TO_DADDR(mp, agno, XFS_SB_BLOCK(mp)),
> -			  XFS_FSS_TO_BB(mp, 1), 0, &bp, &xfs_sb_buf_ops);
>   		/*
>   		 * If we get an error reading or writing alternate superblocks,
>   		 * continue.  xfs_repair chooses the "best" superblock based
> @@ -210,6 +207,7 @@ xfs_update_secondary_supers(
>   		 * superblocks un-updated than updated, and xfs_repair may
>   		 * pick them over the properly-updated primary.
>   		 */
> +		error = xfs_sb_read_secondary(mp, NULL, agno, &bp);
>   		if (error) {
>   			xfs_warn(mp,
>   		"error %d reading secondary superblock for ag %d",
> --
> To unsubscribe from this list: send the line "unsubscribe linux-xfs" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

Re: [PATCH] xfs: change xfs_update_secondary_supers to use xfS_sb_read_secondary

[Dave Chinner]

On Tue, May 15, 2018 at 01:47:21PM -0700, Darrick J. Wong wrote:
> From: Darrick J. Wong <darrick.wong@oracle.com>
> 
> Use the new helper to read secondary superblocks instead of opencoding
> it ourselves.
> 
> Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
> ---
>  fs/xfs/xfs_fsops.c |   10 ++++------
>  1 file changed, 4 insertions(+), 6 deletions(-)
> 
> diff --git a/fs/xfs/xfs_fsops.c b/fs/xfs/xfs_fsops.c
> index 056a76689197..40c131ebf772 100644
> --- a/fs/xfs/xfs_fsops.c
> +++ b/fs/xfs/xfs_fsops.c
> @@ -191,18 +191,15 @@ xfs_growfs_data_private(
>   */
>  int
>  xfs_update_secondary_supers(
> -	xfs_mount_t		*mp)
> +	struct xfs_mount	*mp)
>  {
> -	int			error, saved_error;
> +	struct xfs_buf		*bp;
>  	xfs_agnumber_t		agno;
> -	xfs_buf_t		*bp;
> +	int			error, saved_error;
>  
>  	error = saved_error = 0;
>  
>  	for (agno = 1; agno < mp->m_sb.sb_agcount; agno++) {
> -		error = xfs_trans_read_buf(mp, NULL, mp->m_ddev_targp,
> -			  XFS_AGB_TO_DADDR(mp, agno, XFS_SB_BLOCK(mp)),
> -			  XFS_FSS_TO_BB(mp, 1), 0, &bp, &xfs_sb_buf_ops);
>  		/*
>  		 * If we get an error reading or writing alternate superblocks,
>  		 * continue.  xfs_repair chooses the "best" superblock based
> @@ -210,6 +207,7 @@ xfs_update_secondary_supers(
>  		 * superblocks un-updated than updated, and xfs_repair may
>  		 * pick them over the properly-updated primary.
>  		 */
> +		error = xfs_sb_read_secondary(mp, NULL, agno, &bp);
>  		if (error) {
>  			xfs_warn(mp,
>  		"error %d reading secondary superblock for ag %d",

Why change this now when my growfs patchset ireworks it and then
moves the function entirely? Doing this now just breaks that
patchset unnecessarily and forces another rebase - can we move this
to the end of the growfs patchset?

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com

Re: [PATCH] xfs: change xfs_update_secondary_supers to use xfS_sb_read_secondary

[Darrick J. Wong]

On Wed, May 16, 2018 at 09:08:11AM +1000, Dave Chinner wrote:
> On Tue, May 15, 2018 at 01:47:21PM -0700, Darrick J. Wong wrote:
> > From: Darrick J. Wong <darrick.wong@oracle.com>
> > 
> > Use the new helper to read secondary superblocks instead of opencoding
> > it ourselves.
> > 
> > Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
> > ---
> >  fs/xfs/xfs_fsops.c |   10 ++++------
> >  1 file changed, 4 insertions(+), 6 deletions(-)
> > 
> > diff --git a/fs/xfs/xfs_fsops.c b/fs/xfs/xfs_fsops.c
> > index 056a76689197..40c131ebf772 100644
> > --- a/fs/xfs/xfs_fsops.c
> > +++ b/fs/xfs/xfs_fsops.c
> > @@ -191,18 +191,15 @@ xfs_growfs_data_private(
> >   */
> >  int
> >  xfs_update_secondary_supers(
> > -	xfs_mount_t		*mp)
> > +	struct xfs_mount	*mp)
> >  {
> > -	int			error, saved_error;
> > +	struct xfs_buf		*bp;
> >  	xfs_agnumber_t		agno;
> > -	xfs_buf_t		*bp;
> > +	int			error, saved_error;
> >  
> >  	error = saved_error = 0;
> >  
> >  	for (agno = 1; agno < mp->m_sb.sb_agcount; agno++) {
> > -		error = xfs_trans_read_buf(mp, NULL, mp->m_ddev_targp,
> > -			  XFS_AGB_TO_DADDR(mp, agno, XFS_SB_BLOCK(mp)),
> > -			  XFS_FSS_TO_BB(mp, 1), 0, &bp, &xfs_sb_buf_ops);
> >  		/*
> >  		 * If we get an error reading or writing alternate superblocks,
> >  		 * continue.  xfs_repair chooses the "best" superblock based
> > @@ -210,6 +207,7 @@ xfs_update_secondary_supers(
> >  		 * superblocks un-updated than updated, and xfs_repair may
> >  		 * pick them over the properly-updated primary.
> >  		 */
> > +		error = xfs_sb_read_secondary(mp, NULL, agno, &bp);
> >  		if (error) {
> >  			xfs_warn(mp,
> >  		"error %d reading secondary superblock for ag %d",
> 
> Why change this now when my growfs patchset ireworks it and then
> moves the function entirely? Doing this now just breaks that
> patchset unnecessarily and forces another rebase - can we move this
> to the end of the growfs patchset?

The label stuff will come after your growfs stuff... maybe I'll try this
again tomorrow when I'm more awake.

Scrub wants a function to read a secondary sb, and a second one to clone
the primary to fix damage by getting the sb, formatting the incore sb
into the buffer, and writing it to disk.

Label wants a function to read each secondary sb, format the incore sb
into the buffer, and write it to disk.

Growfs wants to (read an existing ag's secondary sb | get a new ag's
sb), format the incore sb into the buffer, and write it to disk.

Does that sound right?

--D

> Cheers,
> 
> Dave.
> -- 
> Dave Chinner
> david@fromorbit.com
> --
> To unsubscribe from this list: send the line "unsubscribe linux-xfs" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] xfs: change xfs_update_secondary_supers to use xfS_sb_read_secondary

[Dave Chinner]

On Tue, May 15, 2018 at 04:36:56PM -0700, Darrick J. Wong wrote:
> On Wed, May 16, 2018 at 09:08:11AM +1000, Dave Chinner wrote:
> > On Tue, May 15, 2018 at 01:47:21PM -0700, Darrick J. Wong wrote:
> > > From: Darrick J. Wong <darrick.wong@oracle.com>
> > > 
> > > Use the new helper to read secondary superblocks instead of opencoding
> > > it ourselves.
> > > 
> > > Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
> > > ---
> > >  fs/xfs/xfs_fsops.c |   10 ++++------
> > >  1 file changed, 4 insertions(+), 6 deletions(-)
> > > 
> > > diff --git a/fs/xfs/xfs_fsops.c b/fs/xfs/xfs_fsops.c
> > > index 056a76689197..40c131ebf772 100644
> > > --- a/fs/xfs/xfs_fsops.c
> > > +++ b/fs/xfs/xfs_fsops.c
> > > @@ -191,18 +191,15 @@ xfs_growfs_data_private(
> > >   */
> > >  int
> > >  xfs_update_secondary_supers(
> > > -	xfs_mount_t		*mp)
> > > +	struct xfs_mount	*mp)
> > >  {
> > > -	int			error, saved_error;
> > > +	struct xfs_buf		*bp;
> > >  	xfs_agnumber_t		agno;
> > > -	xfs_buf_t		*bp;
> > > +	int			error, saved_error;
> > >  
> > >  	error = saved_error = 0;
> > >  
> > >  	for (agno = 1; agno < mp->m_sb.sb_agcount; agno++) {
> > > -		error = xfs_trans_read_buf(mp, NULL, mp->m_ddev_targp,
> > > -			  XFS_AGB_TO_DADDR(mp, agno, XFS_SB_BLOCK(mp)),
> > > -			  XFS_FSS_TO_BB(mp, 1), 0, &bp, &xfs_sb_buf_ops);
> > >  		/*
> > >  		 * If we get an error reading or writing alternate superblocks,
> > >  		 * continue.  xfs_repair chooses the "best" superblock based
> > > @@ -210,6 +207,7 @@ xfs_update_secondary_supers(
> > >  		 * superblocks un-updated than updated, and xfs_repair may
> > >  		 * pick them over the properly-updated primary.
> > >  		 */
> > > +		error = xfs_sb_read_secondary(mp, NULL, agno, &bp);
> > >  		if (error) {
> > >  			xfs_warn(mp,
> > >  		"error %d reading secondary superblock for ag %d",
> > 
> > Why change this now when my growfs patchset ireworks it and then
> > moves the function entirely? Doing this now just breaks that
> > patchset unnecessarily and forces another rebase - can we move this
> > to the end of the growfs patchset?
> 
> The label stuff will come after your growfs stuff... maybe I'll try this
> again tomorrow when I'm more awake.

*nod*

> Scrub wants a function to read a secondary sb, and a second one to clone
> the primary to fix damage by getting the sb, formatting the incore sb
> into the buffer, and writing it to disk.

This is all done in a transaction context during scrub, isn't it?
Which means it's mostly separate to the non-transactional "update
all secondary SB's?

> Label wants a function to read each secondary sb, format the incore sb
> into the buffer, and write it to disk.

Yup - it can just use the function I added to rewrite all the
secondary superblocks.

Hmmmm - just a thought here: does repair check and repair
inconsistent labels across secondary superblocks? What about UUID
updates? Do we have a generic need for some superblock updates to be
atomic across all superblocks?

> Growfs wants to (read an existing ag's secondary sb | get a new ag's
> sb), format the incore sb into the buffer, and write it to disk.

After my changes, growfs no longer reads the secondary superblocks.
It just rewrites them all completely by rewriting the primary SB
into them. The existing code rewrites them completely, too, so the
read is actually redundant and, potentially, can cause the grow to
fail.

Cheers,

Dave.

-- 
Dave Chinner
david@fromorbit.com

Re: [PATCH] xfs: change xfs_update_secondary_supers to use xfS_sb_read_secondary

[Darrick J. Wong]

On Wed, May 16, 2018 at 10:23:46AM +1000, Dave Chinner wrote:
> On Tue, May 15, 2018 at 04:36:56PM -0700, Darrick J. Wong wrote:
> > On Wed, May 16, 2018 at 09:08:11AM +1000, Dave Chinner wrote:
> > > On Tue, May 15, 2018 at 01:47:21PM -0700, Darrick J. Wong wrote:
> > > > From: Darrick J. Wong <darrick.wong@oracle.com>
> > > > 
> > > > Use the new helper to read secondary superblocks instead of opencoding
> > > > it ourselves.
> > > > 
> > > > Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
> > > > ---
> > > >  fs/xfs/xfs_fsops.c |   10 ++++------
> > > >  1 file changed, 4 insertions(+), 6 deletions(-)
> > > > 
> > > > diff --git a/fs/xfs/xfs_fsops.c b/fs/xfs/xfs_fsops.c
> > > > index 056a76689197..40c131ebf772 100644
> > > > --- a/fs/xfs/xfs_fsops.c
> > > > +++ b/fs/xfs/xfs_fsops.c
> > > > @@ -191,18 +191,15 @@ xfs_growfs_data_private(
> > > >   */
> > > >  int
> > > >  xfs_update_secondary_supers(
> > > > -	xfs_mount_t		*mp)
> > > > +	struct xfs_mount	*mp)
> > > >  {
> > > > -	int			error, saved_error;
> > > > +	struct xfs_buf		*bp;
> > > >  	xfs_agnumber_t		agno;
> > > > -	xfs_buf_t		*bp;
> > > > +	int			error, saved_error;
> > > >  
> > > >  	error = saved_error = 0;
> > > >  
> > > >  	for (agno = 1; agno < mp->m_sb.sb_agcount; agno++) {
> > > > -		error = xfs_trans_read_buf(mp, NULL, mp->m_ddev_targp,
> > > > -			  XFS_AGB_TO_DADDR(mp, agno, XFS_SB_BLOCK(mp)),
> > > > -			  XFS_FSS_TO_BB(mp, 1), 0, &bp, &xfs_sb_buf_ops);
> > > >  		/*
> > > >  		 * If we get an error reading or writing alternate superblocks,
> > > >  		 * continue.  xfs_repair chooses the "best" superblock based
> > > > @@ -210,6 +207,7 @@ xfs_update_secondary_supers(
> > > >  		 * superblocks un-updated than updated, and xfs_repair may
> > > >  		 * pick them over the properly-updated primary.
> > > >  		 */
> > > > +		error = xfs_sb_read_secondary(mp, NULL, agno, &bp);
> > > >  		if (error) {
> > > >  			xfs_warn(mp,
> > > >  		"error %d reading secondary superblock for ag %d",
> > > 
> > > Why change this now when my growfs patchset ireworks it and then
> > > moves the function entirely? Doing this now just breaks that
> > > patchset unnecessarily and forces another rebase - can we move this
> > > to the end of the growfs patchset?
> > 
> > The label stuff will come after your growfs stuff... maybe I'll try this
> > again tomorrow when I'm more awake.
> 
> *nod*

Aha, just s/xfs_update_secondary_supers/xfs_update_secondary_sbs/ in
Eric's online label patch, and drop the one where he adds
xfs_update_secondary_supers.

> > Scrub wants a function to read a secondary sb, and a second one to clone
> > the primary to fix damage by getting the sb, formatting the incore sb
> > into the buffer, and writing it to disk.
> 
> This is all done in a transaction context during scrub, isn't it?
> Which means it's mostly separate to the non-transactional "update
> all secondary SB's?

Yes.  It's likely we only want to update a single secondary sb, and
updating all of them is overkill, so maybe it's fine just to open code
this one use?  Seeing as I don't think we'd ordinarily be updating only
a single secondary sb.

> > Label wants a function to read each secondary sb, format the incore sb
> > into the buffer, and write it to disk.
> 
> Yup - it can just use the function I added to rewrite all the
> secondary superblocks.
> 
> Hmmmm - just a thought here: does repair check and repair
> inconsistent labels across secondary superblocks? What about UUID
> updates? Do we have a generic need for some superblock updates to be
> atomic across all superblocks?

Online fsck checks both sb_fname and sb_uuid and complains if the
secondaries are out of date.  xfs_repair doesn't check at all.

> > Growfs wants to (read an existing ag's secondary sb | get a new ag's
> > sb), format the incore sb into the buffer, and write it to disk.
> 
> After my changes, growfs no longer reads the secondary superblocks.
> It just rewrites them all completely by rewriting the primary SB
> into them. The existing code rewrites them completely, too, so the
> read is actually redundant and, potentially, can cause the grow to
> fail.

Ok, I had wondered myself if we should unconditionally rewrite the
secondary sbs, because why would we care what the previous contents
were?

--D

> Cheers,
> 
> Dave.
> 
> -- 
> Dave Chinner
> david@fromorbit.com
> --
> To unsubscribe from this list: send the line "unsubscribe linux-xfs" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] xfs: change xfs_update_secondary_supers to use xfS_sb_read_secondary

[Dave Chinner]

On Tue, May 15, 2018 at 05:41:29PM -0700, Darrick J. Wong wrote:
> On Wed, May 16, 2018 at 10:23:46AM +1000, Dave Chinner wrote:
> > On Tue, May 15, 2018 at 04:36:56PM -0700, Darrick J. Wong wrote:
> > > On Wed, May 16, 2018 at 09:08:11AM +1000, Dave Chinner wrote:
> > > > On Tue, May 15, 2018 at 01:47:21PM -0700, Darrick J. Wong wrote:
> > > > > From: Darrick J. Wong <darrick.wong@oracle.com>
> > > > > 
> > > > > Use the new helper to read secondary superblocks instead of opencoding
> > > > > it ourselves.
> > > > > 
> > > > > Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
> > > > > ---
> > > > >  fs/xfs/xfs_fsops.c |   10 ++++------
> > > > >  1 file changed, 4 insertions(+), 6 deletions(-)
> > > > > 
> > > > > diff --git a/fs/xfs/xfs_fsops.c b/fs/xfs/xfs_fsops.c
> > > > > index 056a76689197..40c131ebf772 100644
> > > > > --- a/fs/xfs/xfs_fsops.c
> > > > > +++ b/fs/xfs/xfs_fsops.c
> > > > > @@ -191,18 +191,15 @@ xfs_growfs_data_private(
> > > > >   */
> > > > >  int
> > > > >  xfs_update_secondary_supers(
> > > > > -	xfs_mount_t		*mp)
> > > > > +	struct xfs_mount	*mp)
> > > > >  {
> > > > > -	int			error, saved_error;
> > > > > +	struct xfs_buf		*bp;
> > > > >  	xfs_agnumber_t		agno;
> > > > > -	xfs_buf_t		*bp;
> > > > > +	int			error, saved_error;
> > > > >  
> > > > >  	error = saved_error = 0;
> > > > >  
> > > > >  	for (agno = 1; agno < mp->m_sb.sb_agcount; agno++) {
> > > > > -		error = xfs_trans_read_buf(mp, NULL, mp->m_ddev_targp,
> > > > > -			  XFS_AGB_TO_DADDR(mp, agno, XFS_SB_BLOCK(mp)),
> > > > > -			  XFS_FSS_TO_BB(mp, 1), 0, &bp, &xfs_sb_buf_ops);
> > > > >  		/*
> > > > >  		 * If we get an error reading or writing alternate superblocks,
> > > > >  		 * continue.  xfs_repair chooses the "best" superblock based
> > > > > @@ -210,6 +207,7 @@ xfs_update_secondary_supers(
> > > > >  		 * superblocks un-updated than updated, and xfs_repair may
> > > > >  		 * pick them over the properly-updated primary.
> > > > >  		 */
> > > > > +		error = xfs_sb_read_secondary(mp, NULL, agno, &bp);
> > > > >  		if (error) {
> > > > >  			xfs_warn(mp,
> > > > >  		"error %d reading secondary superblock for ag %d",
> > > > 
> > > > Why change this now when my growfs patchset ireworks it and then
> > > > moves the function entirely? Doing this now just breaks that
> > > > patchset unnecessarily and forces another rebase - can we move this
> > > > to the end of the growfs patchset?
> > > 
> > > The label stuff will come after your growfs stuff... maybe I'll try this
> > > again tomorrow when I'm more awake.
> > 
> > *nod*
> 
> Aha, just s/xfs_update_secondary_supers/xfs_update_secondary_sbs/ in
> Eric's online label patch, and drop the one where he adds
> xfs_update_secondary_supers.
> 
> > > Scrub wants a function to read a secondary sb, and a second one to clone
> > > the primary to fix damage by getting the sb, formatting the incore sb
> > > into the buffer, and writing it to disk.
> > 
> > This is all done in a transaction context during scrub, isn't it?
> > Which means it's mostly separate to the non-transactional "update
> > all secondary SB's?
> 
> Yes.  It's likely we only want to update a single secondary sb, and
> updating all of them is overkill, so maybe it's fine just to open code
> this one use?  Seeing as I don't think we'd ordinarily be updating only
> a single secondary sb.

Ok, so that means - initially, at least - they will be separate
functions.

In time, I'd like to explore making the secondary superblock
updates ordered buffers attached to a special superblock update
transaction (i.e. like the inode extent swap intent in the inode log
item flags), and that would probably allow us to factor it all down
to a single set of transaction aware functions...

> > > Label wants a function to read each secondary sb, format the incore sb
> > > into the buffer, and write it to disk.
> > 
> > Yup - it can just use the function I added to rewrite all the
> > secondary superblocks.
> > 
> > Hmmmm - just a thought here: does repair check and repair
> > inconsistent labels across secondary superblocks? What about UUID
> > updates? Do we have a generic need for some superblock updates to be
> > atomic across all superblocks?
> 
> Online fsck checks both sb_fname and sb_uuid and complains if the
> secondaries are out of date.  xfs_repair doesn't check at all.

OK, so I'm guessing that will get added to the growing list of
things that repair needs to check?

> > > Growfs wants to (read an existing ag's secondary sb | get a new ag's
> > > sb), format the incore sb into the buffer, and write it to disk.
> > 
> > After my changes, growfs no longer reads the secondary superblocks.
> > It just rewrites them all completely by rewriting the primary SB
> > into them. The existing code rewrites them completely, too, so the
> > read is actually redundant and, potentially, can cause the grow to
> > fail.
> 
> Ok, I had wondered myself if we should unconditionally rewrite the
> secondary sbs, because why would we care what the previous contents
> were?

Exactly :)

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com