[PATCH] xfs: disable sparse inode chunk alignment check when there is no alignment

[PATCH] xfs: disable sparse inode chunk alignment check when there is no alignment

[Darrick J. Wong]

From: Darrick J. Wong <djwong@kernel.org>

While testing a 64k-blocksize filesystem, I noticed that xfs/709 fails
to rebuild the inode btree with a bunch of "Corruption remains"
messages.  It turns out that when the inode chunk size is smaller than a
single filesystem block, no block alignments constraints are necessary
for inode chunk allocations, and sb_spino_align is zero.  Hence we can
skip the check.

Fixes: dbfbf3bdf639 ("xfs: repair inode btrees")
Signed-off-by: Darrick J. Wong <djwong@kernel.org>
---
 fs/xfs/scrub/ialloc_repair.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/fs/xfs/scrub/ialloc_repair.c b/fs/xfs/scrub/ialloc_repair.c
index b3f7182dd2f5d..e94f108000825 100644
--- a/fs/xfs/scrub/ialloc_repair.c
+++ b/fs/xfs/scrub/ialloc_repair.c
@@ -369,7 +369,7 @@ xrep_ibt_check_inode_ext(
 	 * On a sparse inode fs, this cluster could be part of a sparse chunk.
 	 * Sparse clusters must be aligned to sparse chunk alignment.
 	 */
-	if (xfs_has_sparseinodes(mp) &&
+	if (xfs_has_sparseinodes(mp) && mp->m_sb.sb_spino_align &&
 	    (!IS_ALIGNED(agbno, mp->m_sb.sb_spino_align) ||
 	     !IS_ALIGNED(agbno + len, mp->m_sb.sb_spino_align)))
 		return -EFSCORRUPTED;

Re: [PATCH] xfs: disable sparse inode chunk alignment check when there is no alignment

[Dave Chinner]

On Wed, Jan 31, 2024 at 11:47:14AM -0800, Darrick J. Wong wrote:
> From: Darrick J. Wong <djwong@kernel.org>
> 
> While testing a 64k-blocksize filesystem, I noticed that xfs/709 fails
> to rebuild the inode btree with a bunch of "Corruption remains"
> messages.  It turns out that when the inode chunk size is smaller than a
> single filesystem block, no block alignments constraints are necessary
> for inode chunk allocations, and sb_spino_align is zero.  Hence we can
> skip the check.

Should sparse inodes even be enabled by mkfs in this case?

Regardless, if sb_spino_align = 0 then xfs_ialloc_setup_geometry()
does:

	igeo->ialloc_min_blks = igeo->ialloc_blks;

And this turns off sparse inode allocation for this situation....

> Fixes: dbfbf3bdf639 ("xfs: repair inode btrees")
> Signed-off-by: Darrick J. Wong <djwong@kernel.org>
> ---
>  fs/xfs/scrub/ialloc_repair.c |    2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/fs/xfs/scrub/ialloc_repair.c b/fs/xfs/scrub/ialloc_repair.c
> index b3f7182dd2f5d..e94f108000825 100644
> --- a/fs/xfs/scrub/ialloc_repair.c
> +++ b/fs/xfs/scrub/ialloc_repair.c
> @@ -369,7 +369,7 @@ xrep_ibt_check_inode_ext(
>  	 * On a sparse inode fs, this cluster could be part of a sparse chunk.
>  	 * Sparse clusters must be aligned to sparse chunk alignment.
>  	 */
> -	if (xfs_has_sparseinodes(mp) &&
> +	if (xfs_has_sparseinodes(mp) && mp->m_sb.sb_spino_align &&
>  	    (!IS_ALIGNED(agbno, mp->m_sb.sb_spino_align) ||
>  	     !IS_ALIGNED(agbno + len, mp->m_sb.sb_spino_align)))
>  		return -EFSCORRUPTED;

... which makes this additional check reasonable.

Reviewed-by: Dave Chinner <dchinner@redhat.com>

-- 
Dave Chinner
david@fromorbit.com

Re: [PATCH] xfs: disable sparse inode chunk alignment check when there is no alignment

[Darrick J. Wong]

On Thu, Feb 01, 2024 at 07:54:49AM +1100, Dave Chinner wrote:
> On Wed, Jan 31, 2024 at 11:47:14AM -0800, Darrick J. Wong wrote:
> > From: Darrick J. Wong <djwong@kernel.org>
> > 
> > While testing a 64k-blocksize filesystem, I noticed that xfs/709 fails
> > to rebuild the inode btree with a bunch of "Corruption remains"
> > messages.  It turns out that when the inode chunk size is smaller than a
> > single filesystem block, no block alignments constraints are necessary
> > for inode chunk allocations, and sb_spino_align is zero.  Hence we can
> > skip the check.
> 
> Should sparse inodes even be enabled by mkfs in this case?

Probably not, it's totally useless for inopblock <= 64.

> Regardless, if sb_spino_align = 0 then xfs_ialloc_setup_geometry()
> does:
> 
> 	igeo->ialloc_min_blks = igeo->ialloc_blks;
> 
> And this turns off sparse inode allocation for this situation....
> 
> > Fixes: dbfbf3bdf639 ("xfs: repair inode btrees")
> > Signed-off-by: Darrick J. Wong <djwong@kernel.org>
> > ---
> >  fs/xfs/scrub/ialloc_repair.c |    2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/fs/xfs/scrub/ialloc_repair.c b/fs/xfs/scrub/ialloc_repair.c
> > index b3f7182dd2f5d..e94f108000825 100644
> > --- a/fs/xfs/scrub/ialloc_repair.c
> > +++ b/fs/xfs/scrub/ialloc_repair.c
> > @@ -369,7 +369,7 @@ xrep_ibt_check_inode_ext(
> >  	 * On a sparse inode fs, this cluster could be part of a sparse chunk.
> >  	 * Sparse clusters must be aligned to sparse chunk alignment.
> >  	 */
> > -	if (xfs_has_sparseinodes(mp) &&
> > +	if (xfs_has_sparseinodes(mp) && mp->m_sb.sb_spino_align &&
> >  	    (!IS_ALIGNED(agbno, mp->m_sb.sb_spino_align) ||
> >  	     !IS_ALIGNED(agbno + len, mp->m_sb.sb_spino_align)))
> >  		return -EFSCORRUPTED;
> 
> ... which makes this additional check reasonable.
> 
> Reviewed-by: Dave Chinner <dchinner@redhat.com>

Thanks!

--D

> 
> -- 
> Dave Chinner
> david@fromorbit.com
> 

Re: [PATCH] xfs: disable sparse inode chunk alignment check when there is no alignment

[Christoph Hellwig]

Looks good:

Reviewed-by: Christoph Hellwig <hch@lst.de>

(and we really need large blocksize support to be able to test
64k block sizes on x86 :))