[PATCH 0/2] xfs: fix a couple post-merge sparse inode chunks issues

[PATCH 0/2] xfs: fix a couple post-merge sparse inode chunks issues

[Brian Foster]

Hi all,

Here are a couple quick hitter fixes for sparse inodes. Patch 1 fixes a
compile failure detected by the kbuild robot with 32-bit configs. Patch
2 fixes a problem with the DEBUG mode random sparse inode chunk
allocation on large block size filesystems.

Brian

Brian Foster (2):
  xfs: fix sparse inodes 32-bit compile failure
  xfs: check min blks for random debug mode sparse allocations

 fs/xfs/libxfs/xfs_ialloc.c | 15 ++++++++-------
 fs/xfs/xfs_inode.c         |  2 +-
 2 files changed, 9 insertions(+), 8 deletions(-)

-- 
1.9.3

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

[PATCH 1/2] xfs: fix sparse inodes 32-bit compile failure

[Brian Foster]

The kbuild test robot reports the following compilation failure with a
32-bit kernel configuration:

	fs/built-in.o: In function `xfs_ifree_cluster':
	>> xfs_inode.c:(.text+0x17ac84): undefined reference to `__umoddi3'

This is due to the use of the modulus operator on a 64-bit variable in
the ASSERT() added as part of the following commit:

	xfs: skip unallocated regions of inode chunks in xfs_ifree_cluster()

This ASSERT() simply checks that the offset of the inode in a sparse
cluster is appropriately aligned. Since the maximum inode record offset
is 63 (for a 64 inode record) and the calculated offset here should be
something less than that, cast the offset value to a 32-bit type before
the mod to prevent the error.

Reported-by: kbuild test robot <fengguang.wu@intel.com>
Signed-off-by: Brian Foster <bfoster@redhat.com>
---
 fs/xfs/xfs_inode.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/fs/xfs/xfs_inode.c b/fs/xfs/xfs_inode.c
index 4c054f6..16bd01b 100644
--- a/fs/xfs/xfs_inode.c
+++ b/fs/xfs/xfs_inode.c
@@ -2265,7 +2265,7 @@ xfs_ifree_cluster(
 		 * a sparse region.
 		 */
 		if ((xic->alloc & XFS_INOBT_MASK(inum - xic->first_ino)) == 0) {
-			ASSERT(((inum - xic->first_ino) %
+			ASSERT(((int)(inum - xic->first_ino) %
 				inodes_per_cluster) == 0);
 			continue;
 		}
-- 
1.9.3

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

Re: [PATCH 1/2] xfs: fix sparse inodes 32-bit compile failure

[Dave Chinner]

On Mon, Jun 01, 2015 at 10:33:13AM -0400, Brian Foster wrote:
> The kbuild test robot reports the following compilation failure with a
> 32-bit kernel configuration:
> 
> 	fs/built-in.o: In function `xfs_ifree_cluster':
> 	>> xfs_inode.c:(.text+0x17ac84): undefined reference to `__umoddi3'
> 
> This is due to the use of the modulus operator on a 64-bit variable in
> the ASSERT() added as part of the following commit:
> 
> 	xfs: skip unallocated regions of inode chunks in xfs_ifree_cluster()
> 
> This ASSERT() simply checks that the offset of the inode in a sparse
> cluster is appropriately aligned. Since the maximum inode record offset
> is 63 (for a 64 inode record) and the calculated offset here should be
> something less than that, cast the offset value to a 32-bit type before
> the mod to prevent the error.
> 
> Reported-by: kbuild test robot <fengguang.wu@intel.com>
> Signed-off-by: Brian Foster <bfoster@redhat.com>
> ---
>  fs/xfs/xfs_inode.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/fs/xfs/xfs_inode.c b/fs/xfs/xfs_inode.c
> index 4c054f6..16bd01b 100644
> --- a/fs/xfs/xfs_inode.c
> +++ b/fs/xfs/xfs_inode.c
> @@ -2265,7 +2265,7 @@ xfs_ifree_cluster(
>  		 * a sparse region.
>  		 */
>  		if ((xic->alloc & XFS_INOBT_MASK(inum - xic->first_ino)) == 0) {
> -			ASSERT(((inum - xic->first_ino) %
> +			ASSERT(((int)(inum - xic->first_ino) %
>  				inodes_per_cluster) == 0);

do_mod()

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

[PATCH v2] xfs: fix sparse inodes 32-bit compile failure

[Brian Foster]

The kbuild test robot reports the following compilation failure with a
32-bit kernel configuration:

	fs/built-in.o: In function `xfs_ifree_cluster':
	>> xfs_inode.c:(.text+0x17ac84): undefined reference to `__umoddi3'

This is due to the use of the modulus operator on a 64-bit variable in
the ASSERT() added as part of the following commit:

	xfs: skip unallocated regions of inode chunks in xfs_ifree_cluster()

This ASSERT() simply checks that the offset of the inode in a sparse
cluster is appropriately aligned. Since the maximum inode record offset
is 63 (for a 64 inode record) and the calculated offset here should be
something less than that, just use a 32-bit variable to store the offset
and call the do_mod() helper.

Reported-by: kbuild test robot <fengguang.wu@intel.com>
Signed-off-by: Brian Foster <bfoster@redhat.com>
---

The do_mod() helper requires use of a variable for the first parameter,
so that alone probably resolves the issue. This uses do_mod()
nonetheless...

Brian

 fs/xfs/xfs_inode.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/fs/xfs/xfs_inode.c b/fs/xfs/xfs_inode.c
index 4c054f6..17cd90c 100644
--- a/fs/xfs/xfs_inode.c
+++ b/fs/xfs/xfs_inode.c
@@ -2244,6 +2244,7 @@ xfs_ifree_cluster(
 	int			inodes_per_cluster;
 	int			nbufs;
 	int			i, j;
+	int			ioffset;
 	xfs_daddr_t		blkno;
 	xfs_buf_t		*bp;
 	xfs_inode_t		*ip;
@@ -2264,9 +2265,9 @@ xfs_ifree_cluster(
 		 * physically allocated. Skip the cluster if an inode falls into
 		 * a sparse region.
 		 */
-		if ((xic->alloc & XFS_INOBT_MASK(inum - xic->first_ino)) == 0) {
-			ASSERT(((inum - xic->first_ino) %
-				inodes_per_cluster) == 0);
+		ioffset = inum - xic->first_ino;
+		if ((xic->alloc & XFS_INOBT_MASK(ioffset)) == 0) {
+			ASSERT(do_mod(ioffset, inodes_per_cluster) == 0);
 			continue;
 		}
 
-- 
1.9.3

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

[PATCH 2/2] xfs: check min blks for random debug mode sparse allocations

[Brian Foster]

The inode allocator enables random sparse inode chunk allocations in
DEBUG mode to facilitate testing. Sparse inode allocations are not
always possible, however, depending on the fs geometry. For example,
there is no possibility for a sparse inode allocation on filesystems
where the block size is large enough to fit one or more inode chunks
within a single block.

Fix up the DEBUG mode sparse inode allocation logic to trigger random
sparse allocations only when the geometry of the fs allows it.

Signed-off-by: Brian Foster <bfoster@redhat.com>
---
 fs/xfs/libxfs/xfs_ialloc.c | 15 ++++++++-------
 1 file changed, 8 insertions(+), 7 deletions(-)

diff --git a/fs/xfs/libxfs/xfs_ialloc.c b/fs/xfs/libxfs/xfs_ialloc.c
index a18bc75..66efc70 100644
--- a/fs/xfs/libxfs/xfs_ialloc.c
+++ b/fs/xfs/libxfs/xfs_ialloc.c
@@ -606,20 +606,20 @@ xfs_ialloc_ag_alloc(
 	uint16_t	allocmask = (uint16_t) -1; /* init. to full chunk */
 	struct xfs_inobt_rec_incore rec;
 	struct xfs_perag *pag;
-
 	int		do_sparse = 0;
 
-#ifdef DEBUG
-	/* randomly do sparse inode allocations */
-	if (xfs_sb_version_hassparseinodes(&tp->t_mountp->m_sb))
-		do_sparse = prandom_u32() & 1;
-#endif
-
 	memset(&args, 0, sizeof(args));
 	args.tp = tp;
 	args.mp = tp->t_mountp;
 	args.fsbno = NULLFSBLOCK;
 
+#ifdef DEBUG
+	/* randomly do sparse inode allocations */
+	if (xfs_sb_version_hassparseinodes(&tp->t_mountp->m_sb) &&
+	    args.mp->m_ialloc_min_blks < args.mp->m_ialloc_blks)
+		do_sparse = prandom_u32() & 1;
+#endif
+
 	/*
 	 * Locking will ensure that we don't have two callers in here
 	 * at one time.
@@ -768,6 +768,7 @@ sparse_alloc:
 			return error;
 
 		newlen = args.len << args.mp->m_sb.sb_inopblog;
+		ASSERT(newlen <= XFS_INODES_PER_CHUNK);
 		allocmask = (1 << (newlen / XFS_INODES_PER_HOLEMASK_BIT)) - 1;
 	}
 
-- 
1.9.3

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs