diff mbox series

[22/21] xfs: fix sb_spino_align checks for large fsblock sizes

Message ID 20241126202619.GO9438@frogsfrogsfrogs (mailing list archive)
State Deferred, archived
Headers show
Series [01/21] xfs: fix off-by-one error in fsmap's end_daddr usage | expand

Commit Message

Darrick J. Wong Nov. 26, 2024, 8:26 p.m. UTC 
From: Darrick J. Wong <djwong@kernel.org>

For a sparse inodes filesystem, mkfs.xfs computes the values of
sb_spino_align and sb_inoalignmt with the following code:

	int     cluster_size = XFS_INODE_BIG_CLUSTER_SIZE;

	if (cfg->sb_feat.crcs_enabled)
		cluster_size *= cfg->inodesize / XFS_DINODE_MIN_SIZE;

	sbp->sb_spino_align = cluster_size >> cfg->blocklog;
	sbp->sb_inoalignmt = XFS_INODES_PER_CHUNK *
			cfg->inodesize >> cfg->blocklog;

On a V5 filesystem with 64k fsblocks and 512 byte inodes, this results
in cluster_size = 8192 * (512 / 256) = 16384.  As a result,
sb_spino_align and sb_inoalignmt are both set to zero.  Unfortunately,
this trips the new sb_spino_align check that was just added to
xfs_validate_sb_common, and the mkfs fails:

# mkfs.xfs -f -b size=64k, /dev/sda
meta-data=/dev/sda               isize=512    agcount=4, agsize=81136 blks
         =                       sectsz=512   attr=2, projid32bit=1
         =                       crc=1        finobt=1, sparse=1, rmapbt=1
         =                       reflink=1    bigtime=1 inobtcount=1 nrext64=1
         =                       exchange=0   metadir=0
data     =                       bsize=65536  blocks=324544, imaxpct=25
         =                       sunit=0      swidth=0 blks
naming   =version 2              bsize=65536  ascii-ci=0, ftype=1, parent=0
log      =internal log           bsize=65536  blocks=5006, version=2
         =                       sectsz=512   sunit=0 blks, lazy-count=1
realtime =none                   extsz=65536  blocks=0, rtextents=0
         =                       rgcount=0    rgsize=0 extents
Discarding blocks...Sparse inode alignment (0) is invalid.
Metadata corruption detected at 0x560ac5a80bbe, xfs_sb block 0x0/0x200
libxfs_bwrite: write verifier failed on xfs_sb bno 0x0/0x1
mkfs.xfs: Releasing dirty buffer to free list!
found dirty buffer (bulk) on free list!
Sparse inode alignment (0) is invalid.
Metadata corruption detected at 0x560ac5a80bbe, xfs_sb block 0x0/0x200
libxfs_bwrite: write verifier failed on xfs_sb bno 0x0/0x1
mkfs.xfs: writing AG headers failed, err=22

Prior to commit 59e43f5479cce1 this all worked fine, even if "sparse"
inodes are somewhat meaningless when everything fits in a single
fsblock.  Adjust the checks to handle existing filesystems.

Fixes: 59e43f5479cce1 ("xfs: sb_spino_align is not verified")
Signed-off-by: "Darrick J. Wong" <djwong@kernel.org>
---
 fs/xfs/libxfs/xfs_sb.c |   11 ++++++-----
 1 file changed, 6 insertions(+), 5 deletions(-)

Comments

Christoph Hellwig Nov. 29, 2024, 8:20 a.m. UTC | #1 
Looks good:

Reviewed-by: Christoph Hellwig <hch@lst.de>
Luis Chamberlain Dec. 7, 2024, 12:41 a.m. UTC | #2 
On Tue, Nov 26, 2024 at 12:26:19PM -0800, Darrick J. Wong wrote:
> From: Darrick J. Wong <djwong@kernel.org>
> 
> For a sparse inodes filesystem, mkfs.xfs computes the values of
> sb_spino_align and sb_inoalignmt with the following code:
> 
> 	int     cluster_size = XFS_INODE_BIG_CLUSTER_SIZE;
> 
> 	if (cfg->sb_feat.crcs_enabled)
> 		cluster_size *= cfg->inodesize / XFS_DINODE_MIN_SIZE;
> 
> 	sbp->sb_spino_align = cluster_size >> cfg->blocklog;
> 	sbp->sb_inoalignmt = XFS_INODES_PER_CHUNK *
> 			cfg->inodesize >> cfg->blocklog;
> 
> On a V5 filesystem with 64k fsblocks and 512 byte inodes, this results
> in cluster_size = 8192 * (512 / 256) = 16384.  As a result,
> sb_spino_align and sb_inoalignmt are both set to zero.  Unfortunately,
> this trips the new sb_spino_align check that was just added to
> xfs_validate_sb_common, and the mkfs fails:
> 
> # mkfs.xfs -f -b size=64k, /dev/sda
> meta-data=/dev/sda               isize=512    agcount=4, agsize=81136 blks
>          =                       sectsz=512   attr=2, projid32bit=1
>          =                       crc=1        finobt=1, sparse=1, rmapbt=1
>          =                       reflink=1    bigtime=1 inobtcount=1 nrext64=1
>          =                       exchange=0   metadir=0
> data     =                       bsize=65536  blocks=324544, imaxpct=25
>          =                       sunit=0      swidth=0 blks
> naming   =version 2              bsize=65536  ascii-ci=0, ftype=1, parent=0
> log      =internal log           bsize=65536  blocks=5006, version=2
>          =                       sectsz=512   sunit=0 blks, lazy-count=1
> realtime =none                   extsz=65536  blocks=0, rtextents=0
>          =                       rgcount=0    rgsize=0 extents
> Discarding blocks...Sparse inode alignment (0) is invalid.
> Metadata corruption detected at 0x560ac5a80bbe, xfs_sb block 0x0/0x200
> libxfs_bwrite: write verifier failed on xfs_sb bno 0x0/0x1
> mkfs.xfs: Releasing dirty buffer to free list!
> found dirty buffer (bulk) on free list!
> Sparse inode alignment (0) is invalid.
> Metadata corruption detected at 0x560ac5a80bbe, xfs_sb block 0x0/0x200
> libxfs_bwrite: write verifier failed on xfs_sb bno 0x0/0x1
> mkfs.xfs: writing AG headers failed, err=22
> 
> Prior to commit 59e43f5479cce1 this all worked fine, even if "sparse"
> inodes are somewhat meaningless when everything fits in a single
> fsblock.  Adjust the checks to handle existing filesystems.
> 
> Fixes: 59e43f5479cce1 ("xfs: sb_spino_align is not verified")
> Signed-off-by: "Darrick J. Wong" <djwong@kernel.org>

Tested-by: Luis Chamberlain <mcgrof@kernel.org>

  Luis
diff mbox series

Patch

diff --git a/fs/xfs/libxfs/xfs_sb.c b/fs/xfs/libxfs/xfs_sb.c
index a809513a290cf4..3b5623611eba02 100644
--- a/fs/xfs/libxfs/xfs_sb.c
+++ b/fs/xfs/libxfs/xfs_sb.c
@@ -494,12 +494,13 @@  xfs_validate_sb_common(
 				return -EINVAL;
 			}
 
-			if (!sbp->sb_spino_align ||
-			    sbp->sb_spino_align > sbp->sb_inoalignmt ||
-			    (sbp->sb_inoalignmt % sbp->sb_spino_align) != 0) {
+			if (sbp->sb_spino_align &&
+			    (sbp->sb_spino_align > sbp->sb_inoalignmt ||
+			     (sbp->sb_inoalignmt % sbp->sb_spino_align) != 0)) {
 				xfs_warn(mp,
-				"Sparse inode alignment (%u) is invalid.",
-					sbp->sb_spino_align);
+"Sparse inode alignment (%u) is invalid, must be integer factor of (%u).",
+					sbp->sb_spino_align,
+					sbp->sb_inoalignmt);
 				return -EINVAL;
 			}
 		} else if (sbp->sb_spino_align) {