[PATCH 0/2] xfs: hopefully the last few rt fixes

[PATCH 0/2] xfs: hopefully the last few rt fixes

[Darrick J. Wong]

Hi all,

Here's the last couple of rt fixes for 5.10.  The first one fixes a
lockdep complaint, and the second one was me realizing I made a major
thinko in an earlier patch: not realizing that xfs_free_file_space
is really a "make contents zero by punching or manually zeroing as
needed" function; the solution is to push all the rt extent alignment
checking up to xfs_file_fallocate.

Updated test case attached.

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=rt-fixes-5.10
---
 fs/xfs/xfs_bmap_util.c |   17 ++++-------------
 fs/xfs/xfs_file.c      |   10 ++++------
 fs/xfs/xfs_inode.c     |   13 +++++++++++++
 fs/xfs/xfs_inode.h     |    1 +
 fs/xfs/xfs_rtalloc.c   |    4 ++--
 5 files changed, 24 insertions(+), 21 deletions(-)

[PATCH 1/2] xfs: annotate grabbing the realtime bitmap/summary locks in growfs

[Darrick J. Wong]

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

Use XFS_ILOCK_RT{BITMAP,SUM} to annotate grabbing the rt bitmap and
summary locks when we grow the realtime volume, just like we do most
everywhere else.  This shuts up lockdep warnings about grabbing the
ILOCK class of locks recursively:

============================================
WARNING: possible recursive locking detected
5.9.0-rc4-djw #rc4 Tainted: G           O
--------------------------------------------
xfs_growfs/4841 is trying to acquire lock:
ffff888035acc230 (&xfs_nondir_ilock_class){++++}-{3:3}, at: xfs_ilock+0xac/0x1a0 [xfs]

but task is already holding lock:
ffff888035acedb0 (&xfs_nondir_ilock_class){++++}-{3:3}, at: xfs_ilock+0xac/0x1a0 [xfs]

other info that might help us debug this:
 Possible unsafe locking scenario:

       CPU0
       ----
  lock(&xfs_nondir_ilock_class);
  lock(&xfs_nondir_ilock_class);

 *** DEADLOCK ***

 May be due to missing lock nesting notation

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


diff --git a/fs/xfs/xfs_rtalloc.c b/fs/xfs/xfs_rtalloc.c
index f9119ba3e9d0..ede1baf31413 100644
--- a/fs/xfs/xfs_rtalloc.c
+++ b/fs/xfs/xfs_rtalloc.c
@@ -1024,7 +1024,7 @@ xfs_growfs_rt(
 		/*
 		 * Lock out other callers by grabbing the bitmap inode lock.
 		 */
-		xfs_ilock(mp->m_rbmip, XFS_ILOCK_EXCL);
+		xfs_ilock(mp->m_rbmip, XFS_ILOCK_EXCL | XFS_ILOCK_RTBITMAP);
 		xfs_trans_ijoin(tp, mp->m_rbmip, XFS_ILOCK_EXCL);
 		/*
 		 * Update the bitmap inode's size ondisk and incore.  We need
@@ -1038,7 +1038,7 @@ xfs_growfs_rt(
 		/*
 		 * Get the summary inode into the transaction.
 		 */
-		xfs_ilock(mp->m_rsumip, XFS_ILOCK_EXCL);
+		xfs_ilock(mp->m_rsumip, XFS_ILOCK_EXCL | XFS_ILOCK_RTSUM);
 		xfs_trans_ijoin(tp, mp->m_rsumip, XFS_ILOCK_EXCL);
 		/*
 		 * Update the summary inode's size.  We need to update the

[PATCH 2/2] xfs: fix fallocate functions when rtextsize is larger than 1

[Darrick J. Wong]

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

In commit fe341eb151ec, I forgot that xfs_free_file_space isn't strictly
a "remove mapped blocks" function.  It is actually a function to zero
file space by punching out the middle and writing zeroes to the
unaligned ends of the specified range.  Therefore, putting a rtextsize
alignment check in that function is wrong because that breaks unaligned
ZERO_RANGE on the realtime volume.

Furthermore, xfs_file_fallocate already has alignment checks for the
functions require the file range to be aligned to the size of a
fundamental allocation unit (which is 1 FSB on the data volume and 1 rt
extent on the realtime volume).  Create a new helper to return the
desired allocation unit size, fix the fallocate frontend to use it,
fix free_file_space to delete the correct range, and remove a now
redundant check from insert_file_space.

Fixes: fe341eb151ec ("xfs: ensure that fpunch, fcollapse, and finsert operations are aligned to rt extent size")
Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
---
 fs/xfs/xfs_bmap_util.c |   17 ++++-------------
 fs/xfs/xfs_file.c      |   10 ++++------
 fs/xfs/xfs_inode.c     |   13 +++++++++++++
 fs/xfs/xfs_inode.h     |    1 +
 4 files changed, 22 insertions(+), 19 deletions(-)


diff --git a/fs/xfs/xfs_bmap_util.c b/fs/xfs/xfs_bmap_util.c
index f2a8a0e75e1f..52cddcfee8a1 100644
--- a/fs/xfs/xfs_bmap_util.c
+++ b/fs/xfs/xfs_bmap_util.c
@@ -947,11 +947,10 @@ xfs_free_file_space(
 	endoffset_fsb = XFS_B_TO_FSBT(mp, offset + len);
 
 	/* We can only free complete realtime extents. */
-	if (XFS_IS_REALTIME_INODE(ip)) {
-		xfs_extlen_t	extsz = xfs_get_extsz_hint(ip);
-
-		if ((startoffset_fsb | endoffset_fsb) & (extsz - 1))
-			return -EINVAL;
+	if (XFS_IS_REALTIME_INODE(ip) && mp->m_sb.sb_rextsize > 0) {
+		startoffset_fsb = round_up(startoffset_fsb,
+					   mp->m_sb.sb_rextsize);
+		endoffset_fsb = round_down(endoffset_fsb, mp->m_sb.sb_rextsize);
 	}
 
 	/*
@@ -1147,14 +1146,6 @@ xfs_insert_file_space(
 
 	trace_xfs_insert_file_space(ip);
 
-	/* We can only insert complete realtime extents. */
-	if (XFS_IS_REALTIME_INODE(ip)) {
-		xfs_extlen_t	extsz = xfs_get_extsz_hint(ip);
-
-		if ((stop_fsb | shift_fsb) & (extsz - 1))
-			return -EINVAL;
-	}
-
 	error = xfs_bmap_can_insert_extents(ip, stop_fsb, shift_fsb);
 	if (error)
 		return error;
diff --git a/fs/xfs/xfs_file.c b/fs/xfs/xfs_file.c
index 3d1b95124744..e9b4b1dada75 100644
--- a/fs/xfs/xfs_file.c
+++ b/fs/xfs/xfs_file.c
@@ -803,6 +803,8 @@ xfs_file_fallocate(
 	enum xfs_prealloc_flags	flags = 0;
 	uint			iolock = XFS_IOLOCK_EXCL | XFS_MMAPLOCK_EXCL;
 	loff_t			new_size = 0;
+	unsigned int		blksize = xfs_inode_alloc_blocksize(ip);
+	unsigned int		blkmask = blksize - 1;
 	bool			do_file_insert = false;
 
 	if (!S_ISREG(inode->i_mode))
@@ -850,9 +852,7 @@ xfs_file_fallocate(
 		if (error)
 			goto out_unlock;
 	} else if (mode & FALLOC_FL_COLLAPSE_RANGE) {
-		unsigned int blksize_mask = i_blocksize(inode) - 1;
-
-		if (offset & blksize_mask || len & blksize_mask) {
+		if ((offset | len) & blkmask) {
 			error = -EINVAL;
 			goto out_unlock;
 		}
@@ -872,10 +872,9 @@ xfs_file_fallocate(
 		if (error)
 			goto out_unlock;
 	} else if (mode & FALLOC_FL_INSERT_RANGE) {
-		unsigned int	blksize_mask = i_blocksize(inode) - 1;
 		loff_t		isize = i_size_read(inode);
 
-		if (offset & blksize_mask || len & blksize_mask) {
+		if ((offset | len) & blkmask) {
 			error = -EINVAL;
 			goto out_unlock;
 		}
@@ -917,7 +916,6 @@ xfs_file_fallocate(
 			 *   2.) If prealloc returns ENOSPC, the file range is
 			 *       still zero-valued by virtue of the hole punch.
 			 */
-			unsigned int blksize = i_blocksize(inode);
 
 			trace_xfs_zero_file_space(ip);
 
diff --git a/fs/xfs/xfs_inode.c b/fs/xfs/xfs_inode.c
index 2bfbcf28b1bd..20bb5fae0d00 100644
--- a/fs/xfs/xfs_inode.c
+++ b/fs/xfs/xfs_inode.c
@@ -3813,3 +3813,16 @@ xfs_iunlock2_io_mmap(
 	if (!same_inode)
 		inode_unlock(VFS_I(ip1));
 }
+
+/* Returns the size of fundamental allocation unit for a file, in bytes. */
+unsigned int
+xfs_inode_alloc_blocksize(
+	struct xfs_inode	*ip)
+{
+	unsigned int		blocks = 1;
+
+	if (XFS_IS_REALTIME_INODE(ip))
+		blocks = ip->i_mount->m_sb.sb_rextsize;
+
+	return XFS_FSB_TO_B(ip->i_mount, blocks);
+}
diff --git a/fs/xfs/xfs_inode.h b/fs/xfs/xfs_inode.h
index 751a3d1d7d84..270b35d9dcb0 100644
--- a/fs/xfs/xfs_inode.h
+++ b/fs/xfs/xfs_inode.h
@@ -475,5 +475,6 @@ void xfs_end_io(struct work_struct *work);
 
 int xfs_ilock2_io_mmap(struct xfs_inode *ip1, struct xfs_inode *ip2);
 void xfs_iunlock2_io_mmap(struct xfs_inode *ip1, struct xfs_inode *ip2);
+unsigned int xfs_inode_alloc_blocksize(struct xfs_inode *ip);
 
 #endif	/* __XFS_INODE_H__ */

[PATCH 3/2] xfs: test rtalloc alignment and math errors

[Darrick J. Wong]

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

Add a couple of regression tests for "xfs: make sure the rt allocator
doesn't run off the end" and "xfs: ensure that fpunch, fcollapse, and
finsert operations are aligned to rt extent size".

Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
---
 tests/xfs/759     |   99 +++++++++++++++++++++++++++++++++++++++++++++++++++++
 tests/xfs/759.out |    2 +
 tests/xfs/760     |   66 +++++++++++++++++++++++++++++++++++
 tests/xfs/760.out |    9 +++++
 tests/xfs/group   |    2 +
 5 files changed, 178 insertions(+)
 create mode 100755 tests/xfs/759
 create mode 100644 tests/xfs/759.out
 create mode 100755 tests/xfs/760
 create mode 100644 tests/xfs/760.out

diff --git a/tests/xfs/759 b/tests/xfs/759
new file mode 100755
index 00000000..00573786
--- /dev/null
+++ b/tests/xfs/759
@@ -0,0 +1,99 @@
+#! /bin/bash
+# SPDX-License-Identifier: GPL-2.0-or-later
+# Copyright (c) 2020, Oracle.  All Rights Reserved.
+#
+# FS QA Test No. 759
+#
+# This is a regression test for an overflow error in the _near realtime
+# allocator.  If the rt bitmap ends exactly at the end of a block and the
+# number of rt extents is large enough to allow an allocation request larger
+# than the maximum extent size, it's possible that during a large allocation
+# request, the allocator will fail to constrain maxlen on the second run
+# through the loop, and the rt bitmap range check will run right off the end of
+# the rtbitmap file.  When this happens, xfs triggers a verifier error and
+# returns EFSCORRUPTED.
+
+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_scratch
+_require_realtime
+_require_test_program "punch-alternating"
+
+rm -f $seqres.full
+
+# Format filesystem to get the block size
+_scratch_mkfs > $seqres.full
+_scratch_mount >> $seqres.full
+
+blksz=$(_get_block_size $SCRATCH_MNT)
+rextsize=$($XFS_INFO_PROG $SCRATCH_MNT | grep realtime.*extsz | sed -e 's/^.*extsz=\([0-9]*\).*$/\1/g')
+rextblks=$((rextsize / blksz))
+
+echo "blksz $blksz rextsize $rextsize rextblks $rextblks" >> $seqres.full
+
+_scratch_unmount
+
+# Format filesystem with a realtime volume whose size fits the following:
+# 1. Longer than (XFS MAXEXTLEN * blocksize) bytes.
+# 2. Exactly a multiple of (NBBY * blksz * rextsize) bytes.
+
+rtsize1=$((2097151 * blksz))
+rtsize2=$((8 * blksz * rextsize))
+rtsize=$(( $(blockdev --getsz $SCRATCH_RTDEV) * 512 ))
+
+echo "rtsize1 $rtsize1 rtsize2 $rtsize2 rtsize $rtsize" >> $seqres.full
+
+test $rtsize -gt $rtsize1 || \
+	_notrun "scratch rt device too small, need $rtsize1 bytes"
+test $rtsize -gt $rtsize2 || \
+	_notrun "scratch rt device too small, need $rtsize2 bytes"
+
+rtsize=$((rtsize - (rtsize % rtsize2)))
+
+echo "rt size will be $rtsize" >> $seqres.full
+
+_scratch_mkfs -r size=$rtsize >> $seqres.full
+_scratch_mount >> $seqres.full
+
+# Make sure the root directory has rtinherit set so our test file will too
+$XFS_IO_PROG -c 'chattr +t' $SCRATCH_MNT
+
+# Allocate some stuff at the start, to force the first falloc of the ouch file
+# to happen somewhere in the middle of the rt volume
+$XFS_IO_PROG -f -c 'falloc 0 64m' "$SCRATCH_MNT/b"
+$here/src/punch-alternating -i $((rextblks * 2)) -s $((rextblks)) "$SCRATCH_MNT/b"
+
+avail="$(df -P "$SCRATCH_MNT" | awk 'END {print $4}')"1
+toobig="$((avail * 2))"
+
+# falloc the ouch file in the middle of the rt extent to exercise the near
+# allocator in the last step.
+$XFS_IO_PROG -f -c 'falloc 0 1g' "$SCRATCH_MNT/ouch"
+
+# Try to get the near allocator to overflow on an allocation that matches
+# exactly one of the rtsummary size levels.  This should return ENOSPC and
+# not EFSCORRUPTED.
+$XFS_IO_PROG -f -c "falloc 0 ${toobig}k" "$SCRATCH_MNT/ouch"
+
+# success, all done
+status=0
+exit
diff --git a/tests/xfs/759.out b/tests/xfs/759.out
new file mode 100644
index 00000000..df693d50
--- /dev/null
+++ b/tests/xfs/759.out
@@ -0,0 +1,2 @@
+QA output created by 759
+fallocate: No space left on device
diff --git a/tests/xfs/760 b/tests/xfs/760
new file mode 100755
index 00000000..7baa346c
--- /dev/null
+++ b/tests/xfs/760
@@ -0,0 +1,66 @@
+#! /bin/bash
+# SPDX-License-Identifier: GPL-2.0-or-later
+# Copyright (c) 2020, Oracle.  All Rights Reserved.
+#
+# FS QA Test No. 760
+#
+# Make sure we validate realtime extent size alignment for fallocate modes.
+
+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_scratch
+_require_realtime
+_require_xfs_io_command "fcollapse"
+_require_xfs_io_command "finsert"
+_require_xfs_io_command "funshare"
+_require_xfs_io_command "fzero"
+_require_xfs_io_command "falloc"
+
+rm -f $seqres.full
+
+# Format filesystem with a 256k realtime extent size
+_scratch_mkfs -r extsize=256k > $seqres.full
+_scratch_mount >> $seqres.full
+
+blksz=$(_get_block_size $SCRATCH_MNT)
+rextsize=$($XFS_INFO_PROG $SCRATCH_MNT | grep realtime.*extsz | sed -e 's/^.*extsz=\([0-9]*\).*$/\1/g')
+rextblks=$((rextsize / blksz))
+
+echo "blksz $blksz rextsize $rextsize rextblks $rextblks" >> $seqres.full
+
+# Make sure the root directory has rtinherit set so our test file will too
+$XFS_IO_PROG -c 'chattr +t' $SCRATCH_MNT
+
+sz=$((rextsize * 100))
+range="$((blksz * 3)) $blksz"
+
+for verb in fpunch finsert fcollapse fzero funshare falloc; do
+	echo "test $verb"
+	$XFS_IO_PROG -f -c "falloc 0 $sz" "$SCRATCH_MNT/b"
+	$XFS_IO_PROG -f -c "$verb $range" "$SCRATCH_MNT/b"
+	rm -f "$SCRATCH_MNT/b"
+	_scratch_cycle_mount
+done
+
+# success, all done
+status=0
+exit
diff --git a/tests/xfs/760.out b/tests/xfs/760.out
new file mode 100644
index 00000000..3d73c6fa
--- /dev/null
+++ b/tests/xfs/760.out
@@ -0,0 +1,9 @@
+QA output created by 760
+test fpunch
+test finsert
+fallocate: Invalid argument
+test fcollapse
+fallocate: Invalid argument
+test fzero
+test funshare
+test falloc
diff --git a/tests/xfs/group b/tests/xfs/group
index c3c33b64..302f5157 100644
--- a/tests/xfs/group
+++ b/tests/xfs/group
@@ -520,6 +520,8 @@
 520 auto quick reflink
 747 auto quick scrub
 758 auto quick rw attr realtime
+759 auto quick rw realtime
+760 auto quick rw collapse punch insert zero prealloc
 908 auto quick bigtime
 909 auto quick bigtime quota
 910 auto quick inobtcount

[PATCH 4/2] xfs: test running growfs on the realtime volume

[Darrick J. Wong]

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

Make sure that we can run growfs to expand the realtime volume without
it blowing up.  This is a regression test for the following patches:

xfs: Set xfs_buf type flag when growing summary/bitmap files
xfs: Set xfs_buf's b_ops member when zeroing bitmap/summary files
xfs: fix realtime bitmap/summary file truncation when growing rt volume
xfs: make xfs_growfs_rt update secondary superblocks
xfs: annotate grabbing the realtime bitmap/summary locks in growfs

Because the xfs maintainer realized that no, we have no tests for this
particular piece of functionality.

Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
---
 tests/xfs/916     |   82 +++++++++++++++++++++++++++++++++++++++++++++++++++++
 tests/xfs/916.out |   10 ++++++
 tests/xfs/group   |    1 +
 3 files changed, 93 insertions(+)
 create mode 100755 tests/xfs/916
 create mode 100644 tests/xfs/916.out

diff --git a/tests/xfs/916 b/tests/xfs/916
new file mode 100755
index 00000000..cde00314
--- /dev/null
+++ b/tests/xfs/916
@@ -0,0 +1,82 @@
+#! /bin/bash
+# SPDX-License-Identifier: GPL-2.0-or-later
+# Copyright (c) 2020, Oracle and/or its affiliates.  All Rights Reserved.
+#
+# FS QA Test No. 916
+#
+# Tests xfs_growfs on the realtime volume to make sure none of it blows up.
+# This is a regression test for the following patches:
+#
+# xfs: Set xfs_buf type flag when growing summary/bitmap files
+# xfs: Set xfs_buf's b_ops member when zeroing bitmap/summary files
+# xfs: fix realtime bitmap/summary file truncation when growing rt volume
+# xfs: make xfs_growfs_rt update secondary superblocks
+# xfs: annotate grabbing the realtime bitmap/summary locks in growfs
+#
+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 /
+	_scratch_unmount >> $seqres.full 2>&1
+	test -e "$rtdev" && losetup -d $rtdev >> $seqres.full 2>&1
+	rm -f $tmp.* $TEST_DIR/$seq.rtvol
+}
+
+# get standard environment, filters and checks
+. ./common/rc
+. ./common/filter
+
+# real QA test starts here
+_supported_fs xfs
+# Note that we don't _require_realtime because we synthesize a rt volume
+# below.
+_require_scratch_nocheck
+_require_no_large_scratch_dev
+
+echo "Create fake rt volume"
+truncate -s 400m $TEST_DIR/$seq.rtvol
+rtdev=$(_create_loop_device $TEST_DIR/$seq.rtvol)
+
+echo "Format and mount 100m rt volume"
+export USE_EXTERNAL=yes
+export SCRATCH_RTDEV=$rtdev
+_scratch_mkfs -r size=100m > $seqres.full
+_scratch_mount || _notrun "Could not mount scratch with synthetic rt volume"
+
+testdir=$SCRATCH_MNT/test-$seq
+mkdir $testdir
+
+echo "Check rt volume stats"
+$XFS_IO_PROG -c 'chattr +t' $testdir
+$XFS_INFO_PROG $SCRATCH_MNT >> $seqres.full
+before=$(stat -f -c '%b' $testdir)
+
+echo "Create some files"
+_pwrite_byte 0x61 0 1m $testdir/original >> $seqres.full
+
+echo "Grow fs"
+$XFS_GROWFS_PROG $SCRATCH_MNT 2>&1 |  _filter_growfs >> $seqres.full
+_scratch_cycle_mount
+
+echo "Recheck 400m rt volume stats"
+$XFS_INFO_PROG $SCRATCH_MNT >> $seqres.full
+after=$(stat -f -c '%b' $testdir)
+_within_tolerance "rt volume size" $after $((before * 4)) 5% -v
+
+echo "Create more copies to make sure the bitmap really works"
+cp -p $testdir/original $testdir/copy3
+
+echo "Check filesystem"
+_check_xfs_filesystem $SCRATCH_DEV none $rtdev
+
+# success, all done
+status=0
+exit
diff --git a/tests/xfs/916.out b/tests/xfs/916.out
new file mode 100644
index 00000000..55f2356a
--- /dev/null
+++ b/tests/xfs/916.out
@@ -0,0 +1,10 @@
+QA output created by 916
+Create fake rt volume
+Format and mount 100m rt volume
+Check rt volume stats
+Create some files
+Grow fs
+Recheck 400m rt volume stats
+rt volume size is in range
+Create more copies to make sure the bitmap really works
+Check filesystem
diff --git a/tests/xfs/group b/tests/xfs/group
index c75d2b99..74a29bc0 100644
--- a/tests/xfs/group
+++ b/tests/xfs/group
@@ -528,3 +528,4 @@
 910 auto quick inobtcount
 911 auto quick bigtime
 915 auto quick quota
+916 auto quick realtime growfs

Re: [PATCH 2/2] xfs: fix fallocate functions when rtextsize is larger than 1

[Chandan Babu R]

On Saturday 10 October 2020 11:04:34 PM IST Darrick J. Wong wrote:
> From: Darrick J. Wong <darrick.wong@oracle.com>
> 
> In commit fe341eb151ec, I forgot that xfs_free_file_space isn't strictly
> a "remove mapped blocks" function.  It is actually a function to zero
> file space by punching out the middle and writing zeroes to the
> unaligned ends of the specified range.  Therefore, putting a rtextsize
> alignment check in that function is wrong because that breaks unaligned
> ZERO_RANGE on the realtime volume.
> 
> Furthermore, xfs_file_fallocate already has alignment checks for the
> functions require the file range to be aligned to the size of a
> fundamental allocation unit (which is 1 FSB on the data volume and 1 rt
> extent on the realtime volume).  Create a new helper to return the
> desired allocation unit size, fix the fallocate frontend to use it,
> fix free_file_space to delete the correct range, and remove a now
> redundant check from insert_file_space.
>

The changes look good to me.

Reviewed-by: Chandan Babu R <chandanrlinux@gmail.com>

> Fixes: fe341eb151ec ("xfs: ensure that fpunch, fcollapse, and finsert operations are aligned to rt extent size")
> Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
> ---
>  fs/xfs/xfs_bmap_util.c |   17 ++++-------------
>  fs/xfs/xfs_file.c      |   10 ++++------
>  fs/xfs/xfs_inode.c     |   13 +++++++++++++
>  fs/xfs/xfs_inode.h     |    1 +
>  4 files changed, 22 insertions(+), 19 deletions(-)
> 
> 
> diff --git a/fs/xfs/xfs_bmap_util.c b/fs/xfs/xfs_bmap_util.c
> index f2a8a0e75e1f..52cddcfee8a1 100644
> --- a/fs/xfs/xfs_bmap_util.c
> +++ b/fs/xfs/xfs_bmap_util.c
> @@ -947,11 +947,10 @@ xfs_free_file_space(
>  	endoffset_fsb = XFS_B_TO_FSBT(mp, offset + len);
>  
>  	/* We can only free complete realtime extents. */
> -	if (XFS_IS_REALTIME_INODE(ip)) {
> -		xfs_extlen_t	extsz = xfs_get_extsz_hint(ip);
> -
> -		if ((startoffset_fsb | endoffset_fsb) & (extsz - 1))
> -			return -EINVAL;
> +	if (XFS_IS_REALTIME_INODE(ip) && mp->m_sb.sb_rextsize > 0) {
> +		startoffset_fsb = round_up(startoffset_fsb,
> +					   mp->m_sb.sb_rextsize);
> +		endoffset_fsb = round_down(endoffset_fsb, mp->m_sb.sb_rextsize);
>  	}
>  
>  	/*
> @@ -1147,14 +1146,6 @@ xfs_insert_file_space(
>  
>  	trace_xfs_insert_file_space(ip);
>  
> -	/* We can only insert complete realtime extents. */
> -	if (XFS_IS_REALTIME_INODE(ip)) {
> -		xfs_extlen_t	extsz = xfs_get_extsz_hint(ip);
> -
> -		if ((stop_fsb | shift_fsb) & (extsz - 1))
> -			return -EINVAL;
> -	}
> -
>  	error = xfs_bmap_can_insert_extents(ip, stop_fsb, shift_fsb);
>  	if (error)
>  		return error;
> diff --git a/fs/xfs/xfs_file.c b/fs/xfs/xfs_file.c
> index 3d1b95124744..e9b4b1dada75 100644
> --- a/fs/xfs/xfs_file.c
> +++ b/fs/xfs/xfs_file.c
> @@ -803,6 +803,8 @@ xfs_file_fallocate(
>  	enum xfs_prealloc_flags	flags = 0;
>  	uint			iolock = XFS_IOLOCK_EXCL | XFS_MMAPLOCK_EXCL;
>  	loff_t			new_size = 0;
> +	unsigned int		blksize = xfs_inode_alloc_blocksize(ip);
> +	unsigned int		blkmask = blksize - 1;
>  	bool			do_file_insert = false;
>  
>  	if (!S_ISREG(inode->i_mode))
> @@ -850,9 +852,7 @@ xfs_file_fallocate(
>  		if (error)
>  			goto out_unlock;
>  	} else if (mode & FALLOC_FL_COLLAPSE_RANGE) {
> -		unsigned int blksize_mask = i_blocksize(inode) - 1;
> -
> -		if (offset & blksize_mask || len & blksize_mask) {
> +		if ((offset | len) & blkmask) {
>  			error = -EINVAL;
>  			goto out_unlock;
>  		}
> @@ -872,10 +872,9 @@ xfs_file_fallocate(
>  		if (error)
>  			goto out_unlock;
>  	} else if (mode & FALLOC_FL_INSERT_RANGE) {
> -		unsigned int	blksize_mask = i_blocksize(inode) - 1;
>  		loff_t		isize = i_size_read(inode);
>  
> -		if (offset & blksize_mask || len & blksize_mask) {
> +		if ((offset | len) & blkmask) {
>  			error = -EINVAL;
>  			goto out_unlock;
>  		}
> @@ -917,7 +916,6 @@ xfs_file_fallocate(
>  			 *   2.) If prealloc returns ENOSPC, the file range is
>  			 *       still zero-valued by virtue of the hole punch.
>  			 */
> -			unsigned int blksize = i_blocksize(inode);
>  
>  			trace_xfs_zero_file_space(ip);
>  
> diff --git a/fs/xfs/xfs_inode.c b/fs/xfs/xfs_inode.c
> index 2bfbcf28b1bd..20bb5fae0d00 100644
> --- a/fs/xfs/xfs_inode.c
> +++ b/fs/xfs/xfs_inode.c
> @@ -3813,3 +3813,16 @@ xfs_iunlock2_io_mmap(
>  	if (!same_inode)
>  		inode_unlock(VFS_I(ip1));
>  }
> +
> +/* Returns the size of fundamental allocation unit for a file, in bytes. */
> +unsigned int
> +xfs_inode_alloc_blocksize(
> +	struct xfs_inode	*ip)
> +{
> +	unsigned int		blocks = 1;
> +
> +	if (XFS_IS_REALTIME_INODE(ip))
> +		blocks = ip->i_mount->m_sb.sb_rextsize;
> +
> +	return XFS_FSB_TO_B(ip->i_mount, blocks);
> +}
> diff --git a/fs/xfs/xfs_inode.h b/fs/xfs/xfs_inode.h
> index 751a3d1d7d84..270b35d9dcb0 100644
> --- a/fs/xfs/xfs_inode.h
> +++ b/fs/xfs/xfs_inode.h
> @@ -475,5 +475,6 @@ void xfs_end_io(struct work_struct *work);
>  
>  int xfs_ilock2_io_mmap(struct xfs_inode *ip1, struct xfs_inode *ip2);
>  void xfs_iunlock2_io_mmap(struct xfs_inode *ip1, struct xfs_inode *ip2);
> +unsigned int xfs_inode_alloc_blocksize(struct xfs_inode *ip);
>  
>  #endif	/* __XFS_INODE_H__ */
> 
> 


-- 
chandan

Re: [PATCH 3/2] xfs: test rtalloc alignment and math errors

[Chandan Babu R]

On Saturday 10 October 2020 11:20:32 PM IST Darrick J. Wong wrote:
> From: Darrick J. Wong <darrick.wong@oracle.com>
> 
> Add a couple of regression tests for "xfs: make sure the rt allocator
> doesn't run off the end" and "xfs: ensure that fpunch, fcollapse, and
> finsert operations are aligned to rt extent size".
>

W.r.t "Make sure we validate realtime extent size alignment for fallocate
modes" test,

Reviewed-by: Chandan Babu R <chandanrlinux@gmail.com>

> Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
> ---
>  tests/xfs/759     |   99 +++++++++++++++++++++++++++++++++++++++++++++++++++++
>  tests/xfs/759.out |    2 +
>  tests/xfs/760     |   66 +++++++++++++++++++++++++++++++++++
>  tests/xfs/760.out |    9 +++++
>  tests/xfs/group   |    2 +
>  5 files changed, 178 insertions(+)
>  create mode 100755 tests/xfs/759
>  create mode 100644 tests/xfs/759.out
>  create mode 100755 tests/xfs/760
>  create mode 100644 tests/xfs/760.out
> 
> diff --git a/tests/xfs/759 b/tests/xfs/759
> new file mode 100755
> index 00000000..00573786
> --- /dev/null
> +++ b/tests/xfs/759
> @@ -0,0 +1,99 @@
> +#! /bin/bash
> +# SPDX-License-Identifier: GPL-2.0-or-later
> +# Copyright (c) 2020, Oracle.  All Rights Reserved.
> +#
> +# FS QA Test No. 759
> +#
> +# This is a regression test for an overflow error in the _near realtime
> +# allocator.  If the rt bitmap ends exactly at the end of a block and the
> +# number of rt extents is large enough to allow an allocation request larger
> +# than the maximum extent size, it's possible that during a large allocation
> +# request, the allocator will fail to constrain maxlen on the second run
> +# through the loop, and the rt bitmap range check will run right off the end of
> +# the rtbitmap file.  When this happens, xfs triggers a verifier error and
> +# returns EFSCORRUPTED.
> +
> +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_scratch
> +_require_realtime
> +_require_test_program "punch-alternating"
> +
> +rm -f $seqres.full
> +
> +# Format filesystem to get the block size
> +_scratch_mkfs > $seqres.full
> +_scratch_mount >> $seqres.full
> +
> +blksz=$(_get_block_size $SCRATCH_MNT)
> +rextsize=$($XFS_INFO_PROG $SCRATCH_MNT | grep realtime.*extsz | sed -e 's/^.*extsz=\([0-9]*\).*$/\1/g')
> +rextblks=$((rextsize / blksz))
> +
> +echo "blksz $blksz rextsize $rextsize rextblks $rextblks" >> $seqres.full
> +
> +_scratch_unmount
> +
> +# Format filesystem with a realtime volume whose size fits the following:
> +# 1. Longer than (XFS MAXEXTLEN * blocksize) bytes.
> +# 2. Exactly a multiple of (NBBY * blksz * rextsize) bytes.
> +
> +rtsize1=$((2097151 * blksz))
> +rtsize2=$((8 * blksz * rextsize))
> +rtsize=$(( $(blockdev --getsz $SCRATCH_RTDEV) * 512 ))
> +
> +echo "rtsize1 $rtsize1 rtsize2 $rtsize2 rtsize $rtsize" >> $seqres.full
> +
> +test $rtsize -gt $rtsize1 || \
> +	_notrun "scratch rt device too small, need $rtsize1 bytes"
> +test $rtsize -gt $rtsize2 || \
> +	_notrun "scratch rt device too small, need $rtsize2 bytes"
> +
> +rtsize=$((rtsize - (rtsize % rtsize2)))
> +
> +echo "rt size will be $rtsize" >> $seqres.full
> +
> +_scratch_mkfs -r size=$rtsize >> $seqres.full
> +_scratch_mount >> $seqres.full
> +
> +# Make sure the root directory has rtinherit set so our test file will too
> +$XFS_IO_PROG -c 'chattr +t' $SCRATCH_MNT
> +
> +# Allocate some stuff at the start, to force the first falloc of the ouch file
> +# to happen somewhere in the middle of the rt volume
> +$XFS_IO_PROG -f -c 'falloc 0 64m' "$SCRATCH_MNT/b"
> +$here/src/punch-alternating -i $((rextblks * 2)) -s $((rextblks)) "$SCRATCH_MNT/b"
> +
> +avail="$(df -P "$SCRATCH_MNT" | awk 'END {print $4}')"1
> +toobig="$((avail * 2))"
> +
> +# falloc the ouch file in the middle of the rt extent to exercise the near
> +# allocator in the last step.
> +$XFS_IO_PROG -f -c 'falloc 0 1g' "$SCRATCH_MNT/ouch"
> +
> +# Try to get the near allocator to overflow on an allocation that matches
> +# exactly one of the rtsummary size levels.  This should return ENOSPC and
> +# not EFSCORRUPTED.
> +$XFS_IO_PROG -f -c "falloc 0 ${toobig}k" "$SCRATCH_MNT/ouch"
> +
> +# success, all done
> +status=0
> +exit
> diff --git a/tests/xfs/759.out b/tests/xfs/759.out
> new file mode 100644
> index 00000000..df693d50
> --- /dev/null
> +++ b/tests/xfs/759.out
> @@ -0,0 +1,2 @@
> +QA output created by 759
> +fallocate: No space left on device
> diff --git a/tests/xfs/760 b/tests/xfs/760
> new file mode 100755
> index 00000000..7baa346c
> --- /dev/null
> +++ b/tests/xfs/760
> @@ -0,0 +1,66 @@
> +#! /bin/bash
> +# SPDX-License-Identifier: GPL-2.0-or-later
> +# Copyright (c) 2020, Oracle.  All Rights Reserved.
> +#
> +# FS QA Test No. 760
> +#
> +# Make sure we validate realtime extent size alignment for fallocate modes.
> +
> +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_scratch
> +_require_realtime
> +_require_xfs_io_command "fcollapse"
> +_require_xfs_io_command "finsert"
> +_require_xfs_io_command "funshare"
> +_require_xfs_io_command "fzero"
> +_require_xfs_io_command "falloc"
> +
> +rm -f $seqres.full
> +
> +# Format filesystem with a 256k realtime extent size
> +_scratch_mkfs -r extsize=256k > $seqres.full
> +_scratch_mount >> $seqres.full
> +
> +blksz=$(_get_block_size $SCRATCH_MNT)
> +rextsize=$($XFS_INFO_PROG $SCRATCH_MNT | grep realtime.*extsz | sed -e 's/^.*extsz=\([0-9]*\).*$/\1/g')
> +rextblks=$((rextsize / blksz))
> +
> +echo "blksz $blksz rextsize $rextsize rextblks $rextblks" >> $seqres.full
> +
> +# Make sure the root directory has rtinherit set so our test file will too
> +$XFS_IO_PROG -c 'chattr +t' $SCRATCH_MNT
> +
> +sz=$((rextsize * 100))
> +range="$((blksz * 3)) $blksz"
> +
> +for verb in fpunch finsert fcollapse fzero funshare falloc; do
> +	echo "test $verb"
> +	$XFS_IO_PROG -f -c "falloc 0 $sz" "$SCRATCH_MNT/b"
> +	$XFS_IO_PROG -f -c "$verb $range" "$SCRATCH_MNT/b"
> +	rm -f "$SCRATCH_MNT/b"
> +	_scratch_cycle_mount
> +done
> +
> +# success, all done
> +status=0
> +exit
> diff --git a/tests/xfs/760.out b/tests/xfs/760.out
> new file mode 100644
> index 00000000..3d73c6fa
> --- /dev/null
> +++ b/tests/xfs/760.out
> @@ -0,0 +1,9 @@
> +QA output created by 760
> +test fpunch
> +test finsert
> +fallocate: Invalid argument
> +test fcollapse
> +fallocate: Invalid argument
> +test fzero
> +test funshare
> +test falloc
> diff --git a/tests/xfs/group b/tests/xfs/group
> index c3c33b64..302f5157 100644
> --- a/tests/xfs/group
> +++ b/tests/xfs/group
> @@ -520,6 +520,8 @@
>  520 auto quick reflink
>  747 auto quick scrub
>  758 auto quick rw attr realtime
> +759 auto quick rw realtime
> +760 auto quick rw collapse punch insert zero prealloc
>  908 auto quick bigtime
>  909 auto quick bigtime quota
>  910 auto quick inobtcount
> 


-- 
chandan

Re: [PATCH 4/2] xfs: test running growfs on the realtime volume

[Chandan Babu R]

On Saturday 10 October 2020 11:20:59 PM IST Darrick J. Wong wrote:
> From: Darrick J. Wong <darrick.wong@oracle.com>
> 
> Make sure that we can run growfs to expand the realtime volume without
> it blowing up.  This is a regression test for the following patches:
> 
> xfs: Set xfs_buf type flag when growing summary/bitmap files
> xfs: Set xfs_buf's b_ops member when zeroing bitmap/summary files
> xfs: fix realtime bitmap/summary file truncation when growing rt volume
> xfs: make xfs_growfs_rt update secondary superblocks
> xfs: annotate grabbing the realtime bitmap/summary locks in growfs
> 
> Because the xfs maintainer realized that no, we have no tests for this
> particular piece of functionality.
>

The test looks good to me.

Reviewed-by: Chandan Babu R <chandanrlinux@gmail.com>

> Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
> ---
>  tests/xfs/916     |   82 +++++++++++++++++++++++++++++++++++++++++++++++++++++
>  tests/xfs/916.out |   10 ++++++
>  tests/xfs/group   |    1 +
>  3 files changed, 93 insertions(+)
>  create mode 100755 tests/xfs/916
>  create mode 100644 tests/xfs/916.out
> 
> diff --git a/tests/xfs/916 b/tests/xfs/916
> new file mode 100755
> index 00000000..cde00314
> --- /dev/null
> +++ b/tests/xfs/916
> @@ -0,0 +1,82 @@
> +#! /bin/bash
> +# SPDX-License-Identifier: GPL-2.0-or-later
> +# Copyright (c) 2020, Oracle and/or its affiliates.  All Rights Reserved.
> +#
> +# FS QA Test No. 916
> +#
> +# Tests xfs_growfs on the realtime volume to make sure none of it blows up.
> +# This is a regression test for the following patches:
> +#
> +# xfs: Set xfs_buf type flag when growing summary/bitmap files
> +# xfs: Set xfs_buf's b_ops member when zeroing bitmap/summary files
> +# xfs: fix realtime bitmap/summary file truncation when growing rt volume
> +# xfs: make xfs_growfs_rt update secondary superblocks
> +# xfs: annotate grabbing the realtime bitmap/summary locks in growfs
> +#
> +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 /
> +	_scratch_unmount >> $seqres.full 2>&1
> +	test -e "$rtdev" && losetup -d $rtdev >> $seqres.full 2>&1
> +	rm -f $tmp.* $TEST_DIR/$seq.rtvol
> +}
> +
> +# get standard environment, filters and checks
> +. ./common/rc
> +. ./common/filter
> +
> +# real QA test starts here
> +_supported_fs xfs
> +# Note that we don't _require_realtime because we synthesize a rt volume
> +# below.
> +_require_scratch_nocheck
> +_require_no_large_scratch_dev
> +
> +echo "Create fake rt volume"
> +truncate -s 400m $TEST_DIR/$seq.rtvol
> +rtdev=$(_create_loop_device $TEST_DIR/$seq.rtvol)
> +
> +echo "Format and mount 100m rt volume"
> +export USE_EXTERNAL=yes
> +export SCRATCH_RTDEV=$rtdev
> +_scratch_mkfs -r size=100m > $seqres.full
> +_scratch_mount || _notrun "Could not mount scratch with synthetic rt volume"
> +
> +testdir=$SCRATCH_MNT/test-$seq
> +mkdir $testdir
> +
> +echo "Check rt volume stats"
> +$XFS_IO_PROG -c 'chattr +t' $testdir
> +$XFS_INFO_PROG $SCRATCH_MNT >> $seqres.full
> +before=$(stat -f -c '%b' $testdir)
> +
> +echo "Create some files"
> +_pwrite_byte 0x61 0 1m $testdir/original >> $seqres.full
> +
> +echo "Grow fs"
> +$XFS_GROWFS_PROG $SCRATCH_MNT 2>&1 |  _filter_growfs >> $seqres.full
> +_scratch_cycle_mount
> +
> +echo "Recheck 400m rt volume stats"
> +$XFS_INFO_PROG $SCRATCH_MNT >> $seqres.full
> +after=$(stat -f -c '%b' $testdir)
> +_within_tolerance "rt volume size" $after $((before * 4)) 5% -v
> +
> +echo "Create more copies to make sure the bitmap really works"
> +cp -p $testdir/original $testdir/copy3
> +
> +echo "Check filesystem"
> +_check_xfs_filesystem $SCRATCH_DEV none $rtdev
> +
> +# success, all done
> +status=0
> +exit
> diff --git a/tests/xfs/916.out b/tests/xfs/916.out
> new file mode 100644
> index 00000000..55f2356a
> --- /dev/null
> +++ b/tests/xfs/916.out
> @@ -0,0 +1,10 @@
> +QA output created by 916
> +Create fake rt volume
> +Format and mount 100m rt volume
> +Check rt volume stats
> +Create some files
> +Grow fs
> +Recheck 400m rt volume stats
> +rt volume size is in range
> +Create more copies to make sure the bitmap really works
> +Check filesystem
> diff --git a/tests/xfs/group b/tests/xfs/group
> index c75d2b99..74a29bc0 100644
> --- a/tests/xfs/group
> +++ b/tests/xfs/group
> @@ -528,3 +528,4 @@
>  910 auto quick inobtcount
>  911 auto quick bigtime
>  915 auto quick quota
> +916 auto quick realtime growfs
> 


-- 
chandan

Re: [PATCH 1/2] xfs: annotate grabbing the realtime bitmap/summary locks in growfs

[Chandan Babu R]

On Saturday 10 October 2020 11:04:27 PM IST Darrick J. Wong wrote:
> From: Darrick J. Wong <darrick.wong@oracle.com>
> 
> Use XFS_ILOCK_RT{BITMAP,SUM} to annotate grabbing the rt bitmap and
> summary locks when we grow the realtime volume, just like we do most
> everywhere else.  This shuts up lockdep warnings about grabbing the
> ILOCK class of locks recursively:
> 
> ============================================
> WARNING: possible recursive locking detected
> 5.9.0-rc4-djw #rc4 Tainted: G           O
> --------------------------------------------
> xfs_growfs/4841 is trying to acquire lock:
> ffff888035acc230 (&xfs_nondir_ilock_class){++++}-{3:3}, at: xfs_ilock+0xac/0x1a0 [xfs]
> 
> but task is already holding lock:
> ffff888035acedb0 (&xfs_nondir_ilock_class){++++}-{3:3}, at: xfs_ilock+0xac/0x1a0 [xfs]
> 
> other info that might help us debug this:
>  Possible unsafe locking scenario:
> 
>        CPU0
>        ----
>   lock(&xfs_nondir_ilock_class);
>   lock(&xfs_nondir_ilock_class);
> 
>  *** DEADLOCK ***
> 
>  May be due to missing lock nesting notation
>

The changes in the patch are correct since it uses different lockdep
subclasses for rt bitmap and rt summary inodes.

Reviewed-by: Chandan Babu R <chandanrlinux@gmail.com>

> Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
> ---
>  fs/xfs/xfs_rtalloc.c |    4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> 
> diff --git a/fs/xfs/xfs_rtalloc.c b/fs/xfs/xfs_rtalloc.c
> index f9119ba3e9d0..ede1baf31413 100644
> --- a/fs/xfs/xfs_rtalloc.c
> +++ b/fs/xfs/xfs_rtalloc.c
> @@ -1024,7 +1024,7 @@ xfs_growfs_rt(
>  		/*
>  		 * Lock out other callers by grabbing the bitmap inode lock.
>  		 */
> -		xfs_ilock(mp->m_rbmip, XFS_ILOCK_EXCL);
> +		xfs_ilock(mp->m_rbmip, XFS_ILOCK_EXCL | XFS_ILOCK_RTBITMAP);
>  		xfs_trans_ijoin(tp, mp->m_rbmip, XFS_ILOCK_EXCL);
>  		/*
>  		 * Update the bitmap inode's size ondisk and incore.  We need
> @@ -1038,7 +1038,7 @@ xfs_growfs_rt(
>  		/*
>  		 * Get the summary inode into the transaction.
>  		 */
> -		xfs_ilock(mp->m_rsumip, XFS_ILOCK_EXCL);
> +		xfs_ilock(mp->m_rsumip, XFS_ILOCK_EXCL | XFS_ILOCK_RTSUM);
>  		xfs_trans_ijoin(tp, mp->m_rsumip, XFS_ILOCK_EXCL);
>  		/*
>  		 * Update the summary inode's size.  We need to update the
> 
> 


-- 
chandan

Re: [PATCH 1/2] xfs: annotate grabbing the realtime bitmap/summary locks in growfs

[Christoph Hellwig]

Looks good,

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

Re: [PATCH 2/2] xfs: fix fallocate functions when rtextsize is larger than 1

[Christoph Hellwig]

I don't really like the xfs_inode_alloc_blocksize helper, given that
it is very easy to be confused with the allocsize concept.

I'd just add a helper ala:

static bool
xfs_falloc_is_unaligned(
	struct inode		*inode,
	loff_t			offset,
	loff_t			len)
{
	struct xfs_mount	*mp = XFS_I(ip)->i_mount;

	unsigned int blksize_mask = i_blocksize(inode) - 1;

	if (XFS_IS_REALTIME_INODE(XFS_I(ip)))
		blksize_mask = XFS_FSB_TO_B(mp, mp->m_sb.sb_rextsize) - 1;

	return (offset & blksize_mask) || (len & blksize_mask);
}

Re: [PATCH 2/2] xfs: fix fallocate functions when rtextsize is larger than 1

[Darrick J. Wong]

On Thu, Oct 15, 2020 at 09:54:39AM +0200, Christoph Hellwig wrote:
> I don't really like the xfs_inode_alloc_blocksize helper, given that
> it is very easy to be confused with the allocsize concept.
> 
> I'd just add a helper ala:
> 
> static bool
> xfs_falloc_is_unaligned(
> 	struct inode		*inode,
> 	loff_t			offset,
> 	loff_t			len)
> {
> 	struct xfs_mount	*mp = XFS_I(ip)->i_mount;
> 
> 	unsigned int blksize_mask = i_blocksize(inode) - 1;
> 
> 	if (XFS_IS_REALTIME_INODE(XFS_I(ip)))
> 		blksize_mask = XFS_FSB_TO_B(mp, mp->m_sb.sb_rextsize) - 1;

UGH the thing that I just noticed (and fstests doesn't seem to cover
given the number of them that sort of blew up) is the fact that the rt
extent size only has to be some multiple of the fs blocksize, not the
power-of-2 multiple that I mistakenly assumed.

Soooo none of this masking stuff actually works properly and I'm going
to drop this patch until I figure out how to do this properly, with a
bunch of fugly division and whatnot... I guess the silver lining is that
rtextsize can't be larger than 1G so at least I can probably use simple
division and not the div64 mess.

Self NAK.

--D

> 
> 	return (offset & blksize_mask) || (len & blksize_mask);
> }