[PATCHSET RFC 0/3] fstests: add inode btree blocks counters to the AGI header

[PATCHSET RFC 0/3] fstests: add inode btree blocks counters to the AGI header

[Darrick J. Wong]

Hi all,

Years ago, Christoph diagnosed a problem where freeing an inode on a
totally full filesystem could fail due to finobt expansion not being
able to allocate enough blocks.  He solved the problem by using the
per-AG block reservation system to ensure that there are always enough
blocks for finobt expansion, but that came at the cost of having to walk
the entire finobt at mount time.  This new feature solves that
performance regression by adding inode btree block counts to the AGI
header.  The patches in this series amend fstests to handle the new
metadata fields and to test that upgrades work properly.

If you're going to start using this mess, you probably ought to just
pull from my git trees, which are linked below.

This is an extraordinary way to destroy everything.  Enjoy!
Comments and questions are, as always, welcome.

--D

kernel git tree:
https://git.kernel.org/cgit/linux/kernel/git/djwong/xfs-linux.git/log/?h=inobt-counters

xfsprogs git tree:
https://git.kernel.org/cgit/linux/kernel/git/djwong/xfsprogs-dev.git/log/?h=inobt-counters

fstests git tree:
https://git.kernel.org/cgit/linux/kernel/git/djwong/xfstests-dev.git/log/?h=inobt-counters
---
 common/xfs        |   20 ++++++
 tests/xfs/010     |    3 +
 tests/xfs/030     |    2 +
 tests/xfs/122.out |    2 -
 tests/xfs/764     |  190 +++++++++++++++++++++++++++++++++++++++++++++++++++++
 tests/xfs/764.out |   21 ++++++
 tests/xfs/910     |   84 +++++++++++++++++++++++
 tests/xfs/910.out |    8 ++
 tests/xfs/group   |    2 +
 9 files changed, 330 insertions(+), 2 deletions(-)
 create mode 100755 tests/xfs/764
 create mode 100644 tests/xfs/764.out
 create mode 100755 tests/xfs/910
 create mode 100644 tests/xfs/910.out

[PATCH 1/3] xfs/{010,030}: update repair output to deal with inobtcount feature

[Darrick J. Wong]

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

Update both of these tests to filter out the new error messages from
repair when the inode btree counter feature is enabled.

Signed-off-by: Darrick J. Wong <djwong@kernel.org>
Reviewed-by: Brian Foster <bfoster@redhat.com>
---
 tests/xfs/010 |    3 ++-
 tests/xfs/030 |    2 ++
 2 files changed, 4 insertions(+), 1 deletion(-)


diff --git a/tests/xfs/010 b/tests/xfs/010
index 1f9bcdff..95cc2555 100755
--- a/tests/xfs/010
+++ b/tests/xfs/010
@@ -113,7 +113,8 @@ _check_scratch_fs
 _corrupt_finobt_root $SCRATCH_DEV
 
 filter_finobt_repair() {
-	sed -e '/^agi has bad CRC/d' | \
+	sed -e '/^agi has bad CRC/d' \
+	    -e '/^bad finobt block/d' | \
 		_filter_repair_lostblocks
 }
 
diff --git a/tests/xfs/030 b/tests/xfs/030
index 04440f9c..906d9019 100755
--- a/tests/xfs/030
+++ b/tests/xfs/030
@@ -44,6 +44,8 @@ _check_ag()
 			    -e '/^bad agbno AGBNO for refcntbt/d' \
 			    -e '/^agf has bad CRC/d' \
 			    -e '/^agi has bad CRC/d' \
+			    -e '/^bad inobt block count/d' \
+			    -e '/^bad finobt block count/d' \
 			    -e '/^Missing reverse-mapping record.*/d' \
 			    -e '/^bad levels LEVELS for [a-z]* root.*/d' \
 			    -e '/^unknown block state, ag AGNO, block.*/d'

[PATCH 2/3] xfs/122: embiggen struct xfs_agi size for inobtcount feature

[Darrick J. Wong]

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

Make the expected AGI size larger for the inobtcount feature.

Signed-off-by: Darrick J. Wong <djwong@kernel.org>
Reviewed-by: Brian Foster <bfoster@redhat.com>
---
 tests/xfs/122.out |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)


diff --git a/tests/xfs/122.out b/tests/xfs/122.out
index cfe09c6d..b0773756 100644
--- a/tests/xfs/122.out
+++ b/tests/xfs/122.out
@@ -113,7 +113,7 @@ sizeof(struct xfs_scrub_metadata) = 64
 sizeof(struct xfs_unmount_log_format) = 8
 sizeof(xfs_agf_t) = 224
 sizeof(xfs_agfl_t) = 36
-sizeof(xfs_agi_t) = 336
+sizeof(xfs_agi_t) = 344
 sizeof(xfs_alloc_key_t) = 8
 sizeof(xfs_alloc_rec_incore_t) = 8
 sizeof(xfs_alloc_rec_t) = 8

[PATCH 3/3] xfs: test inobtcount upgrade

[Darrick J. Wong]

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

Make sure we can actually upgrade filesystems to support inode btree
counters.

Signed-off-by: Darrick J. Wong <djwong@kernel.org>
---
 common/xfs        |   20 ++++++
 tests/xfs/764     |  190 +++++++++++++++++++++++++++++++++++++++++++++++++++++
 tests/xfs/764.out |   21 ++++++
 tests/xfs/910     |   84 +++++++++++++++++++++++
 tests/xfs/910.out |    8 ++
 tests/xfs/group   |    2 +
 6 files changed, 325 insertions(+)
 create mode 100755 tests/xfs/764
 create mode 100644 tests/xfs/764.out
 create mode 100755 tests/xfs/910
 create mode 100644 tests/xfs/910.out


diff --git a/common/xfs b/common/xfs
index 7795856a..30544f88 100644
--- a/common/xfs
+++ b/common/xfs
@@ -1045,3 +1045,23 @@ _require_xfs_copy()
 	[ "$USE_EXTERNAL" = yes ] && \
 		_notrun "Cannot xfs_copy with external devices"
 }
+
+_require_xfs_repair_upgrade()
+{
+	local type="$1"
+
+	$XFS_REPAIR_PROG -c "$type=narf" 2>&1 | \
+		grep -q 'unknown option' && \
+		_notrun "xfs_repair does not support upgrading fs with $type"
+}
+
+_require_xfs_scratch_inobtcount()
+{
+	_require_scratch
+
+	_scratch_mkfs -m inobtcount=1 &> /dev/null || \
+		_notrun "mkfs.xfs doesn't have inobtcount feature"
+	_try_scratch_mount || \
+		_notrun "inobtcount not supported by scratch filesystem type: $FSTYP"
+	_scratch_unmount
+}
diff --git a/tests/xfs/764 b/tests/xfs/764
new file mode 100755
index 00000000..ce815a02
--- /dev/null
+++ b/tests/xfs/764
@@ -0,0 +1,190 @@
+#! /bin/bash
+# SPDX-License-Identifier: GPL-2.0-or-later
+# Copyright (c) 2021 Oracle.  All Rights Reserved.
+#
+# FS QA Test No. 764
+#
+# Functional testing for xfs_admin -O, which is a new switch that enables us to
+# add features to an existing filesystem.  In these test scenarios, we try to
+# add the inode btree counter 'feature' to the filesystem, and make sure that
+# NEEDSREPAIR (aka the thing that prevents us from mounting after an upgrade
+# fails) is clear if the upgraded succeeds and set if it fails.
+#
+# The first scenario tests that we can't add inobtcount to the V4 format,
+# which is now deprecated.
+#
+# The middle five scenarios ensure that xfs_admin -O works even when external
+# log devices and realtime volumes are specified.  This part is purely an
+# exerciser for the userspace tools; kernel support for those features is not
+# required.
+#
+# The last scenario uses a xfs_repair debug knob to simulate failure during an
+# inobtcount upgrade, then checks that mounts fail when the flag is enabled,
+# that repair clears the flag, and mount works after repair.
+
+seq=`basename $0`
+seqres=$RESULT_DIR/$seq
+echo "QA output created by $seq"
+
+here=`pwd`
+tmp=/tmp/$$
+status=1    # failure is the default!
+trap "_cleanup; exit \$status" 0 1 2 3 15
+
+_cleanup()
+{
+	cd /
+	rm -f $tmp.* $fake_logfile $fake_rtfile
+}
+
+# get standard environment, filters and checks
+. ./common/rc
+. ./common/filter
+
+# real QA test starts here
+_supported_fs xfs
+_require_test
+_require_xfs_scratch_inobtcount
+_require_command "$XFS_ADMIN_PROG" "xfs_admin"
+_require_xfs_repair_upgrade inobtcount
+
+rm -f $seqres.full
+
+note() {
+	echo "$@" | tee -a $seqres.full
+}
+
+note "S1: Cannot add inobtcount to a V4 fs"
+_scratch_mkfs -m crc=0 >> $seqres.full
+_scratch_xfs_admin -O inobtcount=1
+
+# Middle five scenarios: Make sure upgrades work with various external device
+# configurations.
+note "S2: Check that setting with xfs_admin + logdev works"
+fake_logfile=$TEST_DIR/scratch.log
+rm -f $fake_logfile
+truncate -s 500m $fake_logfile
+
+old_external=$USE_EXTERNAL
+old_logdev=$SCRATCH_LOGDEV
+USE_EXTERNAL=yes
+SCRATCH_LOGDEV=$fake_logfile
+
+_scratch_mkfs -m crc=1,inobtcount=0 >> $seqres.full
+_scratch_xfs_admin -O inobtcount=1
+_scratch_xfs_db -c 'version' | grep -q NEEDSREPAIR && \
+	echo "xfs_admin should have cleared needsrepair?"
+_scratch_xfs_db -c 'version' | grep -q INOBTCNT || \
+	echo "xfs_admin should have set inobtcount?"
+
+note "Check clean"
+_scratch_xfs_repair -n &>> $seqres.full || echo "Check failed?"
+
+USE_EXTERNAL=$old_external
+SCRATCH_LOGDEV=$old_logdev
+
+note "S3: Check that setting with xfs_admin + realtime works"
+fake_rtfile=$TEST_DIR/scratch.rt
+rm -f $fake_rtfile
+truncate -s 500m $fake_rtfile
+
+old_external=$USE_EXTERNAL
+old_rtdev=$SCRATCH_RTDEV
+USE_EXTERNAL=yes
+SCRATCH_RTDEV=$fake_rtfile
+
+_scratch_mkfs -m crc=1,inobtcount=0 >> $seqres.full
+_scratch_xfs_admin -O inobtcount=1
+_scratch_xfs_db -c 'version' | grep -q NEEDSREPAIR && \
+	echo "xfs_admin should have cleared needsrepair?"
+_scratch_xfs_db -c 'version' | grep -q INOBTCNT || \
+	echo "xfs_admin should have set inobtcount?"
+
+note "Check clean"
+_scratch_xfs_repair -n &>> $seqres.full || echo "Check failed?"
+
+USE_EXTERNAL=$old_external
+SCRATCH_RTDEV=$old_rtdev
+
+note "S4: Check that setting with xfs_admin + realtime + logdev works"
+old_external=$USE_EXTERNAL
+old_logdev=$SCRATCH_LOGDEV
+old_rtdev=$SCRATCH_RTDEV
+USE_EXTERNAL=yes
+SCRATCH_LOGDEV=$fake_logfile
+SCRATCH_RTDEV=$fake_rtfile
+
+_scratch_mkfs -m crc=1,inobtcount=0 >> $seqres.full
+_scratch_xfs_admin -O inobtcount=1
+_scratch_xfs_db -c 'version' | grep -q NEEDSREPAIR && \
+	echo "xfs_admin should have cleared needsrepair?"
+_scratch_xfs_db -c 'version' | grep -q INOBTCNT || \
+	echo "xfs_admin should have set inobtcount?"
+
+note "Check clean"
+_scratch_xfs_repair -n &>> $seqres.full || echo "Check failed?"
+
+USE_EXTERNAL=$old_external
+SCRATCH_LOGDEV=$old_logdev
+SCRATCH_RTDEV=$old_rtdev
+
+note "S5: Check that setting with xfs_admin + nortdev + nologdev works"
+old_external=$USE_EXTERNAL
+old_logdev=$SCRATCH_LOGDEV
+old_rtdev=$SCRATCH_RTDEV
+USE_EXTERNAL=
+SCRATCH_LOGDEV=
+SCRATCH_RTDEV=
+
+_scratch_mkfs -m crc=1,inobtcount=0 >> $seqres.full
+_scratch_xfs_admin -O inobtcount=1
+_scratch_xfs_db -c 'version' | grep -q NEEDSREPAIR && \
+	echo "xfs_admin should have cleared needsrepair?"
+_scratch_xfs_db -c 'version' | grep -q INOBTCNT || \
+	echo "xfs_admin should have set inobtcount?"
+
+note "Check clean"
+_scratch_xfs_repair -n &>> $seqres.full || echo "Check failed?"
+
+USE_EXTERNAL=$old_external
+SCRATCH_LOGDEV=$old_logdev
+SCRATCH_RTDEV=$old_rtdev
+
+# Run our test with the test runner's config last so that the post-test check
+# won't trip over our artificial log/rt devices
+note "S6: Check that setting with xfs_admin testrunner config works"
+_scratch_mkfs -m crc=1,inobtcount=0 >> $seqres.full
+_scratch_xfs_admin -O inobtcount=1
+_scratch_xfs_db -c 'version' | grep -q NEEDSREPAIR && \
+	echo "xfs_admin should have cleared needsrepair?"
+_scratch_xfs_db -c 'version' | grep -q INOBTCNT || \
+	echo "xfs_admin should have set inobtcount?"
+
+note "Check clean"
+_scratch_xfs_repair -n &>> $seqres.full || echo "Check failed?"
+
+note "S7: Simulate failure during upgrade process"
+_scratch_mkfs -m crc=1,inobtcount=0 >> $seqres.full
+XFS_REPAIR_FAIL_AFTER_PHASE=2 _scratch_xfs_repair -c inobtcount=1 2>> $seqres.full
+test $? -eq 137 || echo "repair should have been killed??"
+_scratch_xfs_db -c 'version' | grep -q NEEDSREPAIR || \
+	echo "needsrepair should have been set on fs"
+_scratch_xfs_db -c 'version' | grep -q INOBTCNT || \
+	echo "inobtcount should have been set on fs"
+_try_scratch_mount &> $tmp.mount
+res=$?
+_filter_scratch < $tmp.mount
+if [ $res -eq 0 ]; then
+	echo "needsrepair should have prevented mount"
+	_scratch_unmount
+fi
+_scratch_xfs_repair 2>> $seqres.full
+_scratch_xfs_db -c 'version' | grep -q NEEDSREPAIR && \
+	echo "xfs_repair should have cleared needsrepair?"
+_scratch_xfs_db -c 'version' | grep -q INOBTCNT || \
+	echo "xfs_admin should have set inobtcount?"
+_scratch_mount
+
+# success, all done
+status=0
+exit
diff --git a/tests/xfs/764.out b/tests/xfs/764.out
new file mode 100644
index 00000000..e64bce21
--- /dev/null
+++ b/tests/xfs/764.out
@@ -0,0 +1,21 @@
+QA output created by 764
+S1: Cannot add inobtcount to a V4 fs
+Inode btree count feature only supported on V5 filesystems.
+S2: Check that setting with xfs_admin + logdev works
+Adding inode btree counts to filesystem.
+Check clean
+S3: Check that setting with xfs_admin + realtime works
+Adding inode btree counts to filesystem.
+Check clean
+S4: Check that setting with xfs_admin + realtime + logdev works
+Adding inode btree counts to filesystem.
+Check clean
+S5: Check that setting with xfs_admin + nortdev + nologdev works
+Adding inode btree counts to filesystem.
+Check clean
+S6: Check that setting with xfs_admin testrunner config works
+Adding inode btree counts to filesystem.
+Check clean
+S7: Simulate failure during upgrade process
+Adding inode btree counts to filesystem.
+mount: SCRATCH_MNT: mount(2) system call failed: Structure needs cleaning.
diff --git a/tests/xfs/910 b/tests/xfs/910
new file mode 100755
index 00000000..eb4fd23d
--- /dev/null
+++ b/tests/xfs/910
@@ -0,0 +1,84 @@
+#! /bin/bash
+# SPDX-License-Identifier: GPL-2.0-or-later
+# Copyright (c) 2021 Oracle.  All Rights Reserved.
+#
+# FS QA Test No. 910
+#
+# Check that we can upgrade a filesystem to support inobtcount and that
+# everything works properly after the upgrade.
+
+seq=`basename $0`
+seqres=$RESULT_DIR/$seq
+echo "QA output created by $seq"
+
+here=`pwd`
+tmp=/tmp/$$
+status=1    # failure is the default!
+trap "_cleanup; exit \$status" 0 1 2 3 15
+
+_cleanup()
+{
+	cd /
+	rm -f $tmp.*
+}
+
+# get standard environment, filters and checks
+. ./common/rc
+. ./common/filter
+
+# real QA test starts here
+_supported_fs xfs
+_require_xfs_scratch_inobtcount
+_require_command "$XFS_ADMIN_PROG" "xfs_admin"
+_require_xfs_repair_upgrade inobtcount
+
+rm -f $seqres.full
+
+# Make sure we can't format a filesystem with inobtcount and not finobt.
+_scratch_mkfs -m crc=1,inobtcount=1,finobt=0 &> $seqres.full && \
+	echo "Should not be able to format with inobtcount but not finobt."
+
+# Make sure we can't upgrade a V4 filesystem
+_scratch_mkfs -m crc=0,inobtcount=0,finobt=0 >> $seqres.full
+_scratch_xfs_admin -O inobtcount=1
+_scratch_xfs_db -c 'version' | grep -q INOBTCNT && \
+	echo "Should not be able to upgrade to inobtcount without V5."
+
+# Make sure we can't upgrade a filesystem to inobtcount without finobt.
+_scratch_mkfs -m crc=1,inobtcount=0,finobt=0 >> $seqres.full
+_scratch_xfs_admin -O inobtcount=1
+_scratch_xfs_db -c 'version' | grep -q INOBTCNT && \
+	echo "Should not be able to upgrade to inobtcount without finobt."
+
+# Format V5 filesystem without inode btree counter support and populate it
+_scratch_mkfs -m crc=1,inobtcount=0 >> $seqres.full
+_scratch_xfs_db -c 'version' -c 'sb 0' -c 'p' >> $seqres.full
+_scratch_mount >> $seqres.full
+
+echo moo > $SCRATCH_MNT/urk
+
+_scratch_unmount
+_check_scratch_fs
+
+# Now upgrade to inobtcount support
+_scratch_xfs_admin -O inobtcount=1
+_scratch_xfs_db -c 'version' | grep -q INOBTCNT || \
+	echo "Cannot detect new feature?"
+_check_scratch_fs
+_scratch_xfs_db -c 'version' -c 'sb 0' -c 'p' -c 'agi 0' -c 'p' >> $seqres.full
+
+# Make sure we have nonzero counters
+_scratch_xfs_db -c 'agi 0' -c 'print ino_blocks fino_blocks' | \
+	sed -e 's/= [1-9]*/= NONZERO/g'
+
+# Mount again, look at our files
+_scratch_mount >> $seqres.full
+cat $SCRATCH_MNT/urk
+
+# Make sure we can't re-add inobtcount
+_scratch_unmount
+_scratch_xfs_admin -O inobtcount=1
+_scratch_mount >> $seqres.full
+
+status=0
+exit
diff --git a/tests/xfs/910.out b/tests/xfs/910.out
new file mode 100644
index 00000000..56e6d931
--- /dev/null
+++ b/tests/xfs/910.out
@@ -0,0 +1,8 @@
+QA output created by 910
+Inode btree count feature only supported on V5 filesystems.
+Inode btree count feature requires free inode btree.
+Adding inode btree counts to filesystem.
+ino_blocks = NONZERO
+fino_blocks = NONZERO
+moo
+Filesystem already has inode btree counts.
diff --git a/tests/xfs/group b/tests/xfs/group
index b4bb006d..90d2098f 100644
--- a/tests/xfs/group
+++ b/tests/xfs/group
@@ -505,9 +505,11 @@
 760 auto quick rw collapse punch insert zero prealloc
 761 auto quick realtime
 763 auto quick rw realtime
+764 auto quick repair
 765 auto quick quota
 768 auto quick repair
 770 auto repair
+910 auto quick inobtcount
 915 auto quick quota
 917 auto quick db
 918 auto quick db

Re: [PATCH 1/3] xfs/{010,030}: update repair output to deal with inobtcount feature

[Christoph Hellwig]

On Fri, Feb 12, 2021 at 09:33:37PM -0800, Darrick J. Wong wrote:
> From: Darrick J. Wong <djwong@kernel.org>
> 
> Update both of these tests to filter out the new error messages from
> repair when the inode btree counter feature is enabled.
> 
> Signed-off-by: Darrick J. Wong <djwong@kernel.org>
> Reviewed-by: Brian Foster <bfoster@redhat.com>

Looks good,

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

Re: [PATCH 2/3] xfs/122: embiggen struct xfs_agi size for inobtcount feature

[Christoph Hellwig]

On Fri, Feb 12, 2021 at 09:33:42PM -0800, Darrick J. Wong wrote:
> From: Darrick J. Wong <djwong@kernel.org>
> 
> Make the expected AGI size larger for the inobtcount feature.

Is embiggen really a word?  Otherwise looks good:

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

Re: [PATCH 2/3] xfs/122: embiggen struct xfs_agi size for inobtcount feature

[Darrick J. Wong]

On Thu, Feb 25, 2021 at 07:38:48AM +0000, Christoph Hellwig wrote:
> On Fri, Feb 12, 2021 at 09:33:42PM -0800, Darrick J. Wong wrote:
> > From: Darrick J. Wong <djwong@kernel.org>
> > 
> > Make the expected AGI size larger for the inobtcount feature.
> 
> Is embiggen really a word?  Otherwise looks good:

Yes.

Original source: https://youtu.be/FcxsgZxqnEg?t=65

Legitimate source: https://www.merriam-webster.com/dictionary/embiggen

:D

--D

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

[PATCH 2/3] xfs/122: embiggen struct xfs_agi size for inobtcount feature

[Darrick J. Wong]

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

Make the expected AGI size larger for the inobtcount feature.

Signed-off-by: Darrick J. Wong <djwong@kernel.org>
Reviewed-by: Brian Foster <bfoster@redhat.com>
---
 tests/xfs/122.out |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)


diff --git a/tests/xfs/122.out b/tests/xfs/122.out
index cfe09c6d..b0773756 100644
--- a/tests/xfs/122.out
+++ b/tests/xfs/122.out
@@ -113,7 +113,7 @@ sizeof(struct xfs_scrub_metadata) = 64
 sizeof(struct xfs_unmount_log_format) = 8
 sizeof(xfs_agf_t) = 224
 sizeof(xfs_agfl_t) = 36
-sizeof(xfs_agi_t) = 336
+sizeof(xfs_agi_t) = 344
 sizeof(xfs_alloc_key_t) = 8
 sizeof(xfs_alloc_rec_incore_t) = 8
 sizeof(xfs_alloc_rec_t) = 8