4.1-rc2 kvm-xfstests regressions for ext3 test config

I've got six new test failures in the ext3 test configuration when running a 4.1-rc2 kernel on x86_64 with the latest version of kvm-xfstests.

The new failures include generic/075, generic/091, generic/112, generic/127, generic/231, and generic/263. All also fail on ARM (Pandaboard) when running a 4.1-rc2 kernel on bare metal.

Reverting an -rc2 patch corrects all these failures:
ext4: move check under lock scope to close a race (280227a75b56)

All the failed tests use fsx, all fail when attempting to perform a COLLAPSE_RANGE, and fsx uses this statement to determine whether a particular fallocate mode is supported (and then doesn't use it if not):

	if (fallocate(fd, mode, 0, 1) && errno == EOPNOTSUPP)

In -rc1, ext4_fallocate()/extents.c returns EOPNOTSUPP, and in -rc2, EINVAL. What's happened is that we no longer immediately fail a COLLAPSE_RANGE request for a block-mapped file with EOPNOTSUPP. Instead, we now first check to see if the COLLAPSE_RANGE request is cluster-aligned and sized in ext4_collapse_range()/extents.c, and return EINVAL if not. (There's still a check for block-mapped files, but it's applied later in ext4_collapse_range().)

I think we'd want to return EOPNOTSUPP first in this case.

Eric

Thanks for the heads up. Yes, this is a problem and it has been ported to stable as well. I think that the best solution would be to have the check at both places to avoid unnecessary work in most cases.

Something like this untested patch

index d74e0802..fd2939f 100644
@@ -4927,6 +4927,11 @@ long ext4_fallocate(struct file *file, int mode, loff_t offset, loff_t len)
 	if (ret)
 		return ret;
 
+	if (!(ext4_test_inode_flag(inode, EXT4_INODE_EXTENTS))) {
+		ret = -EOPNOTSUPP;
+		goto out;
+	}
+
 	if (mode & FALLOC_FL_COLLAPSE_RANGE)
 		return ext4_collapse_range(inode, offset, len);
 

@@ -5431,6 +5436,12 @@ int ext4_collapse_range(struct inode *inode, loff_t offset, loff_t len)
 	/* Take mutex lock */
 	mutex_lock(&inode->i_mutex);
 
+	/* Currently just for extent based files */
+	if (!ext4_test_inode_flag(inode, EXT4_INODE_EXTENTS)) {
+		ret = -EOPNOTSUPP;
+		goto out_mutex;
+	}
+
 	/*
 	 * There is no need to overlap collapse range with EOF, in which case
 	 * it is effectively a truncate operation

@@ -5440,12 +5451,6 @@ int ext4_collapse_range(struct inode *inode, loff_t offset, loff_t len)
 		goto out_mutex;
 	}
 
-	/* Currently just for extent based files */
-	if (!ext4_test_inode_flag(inode, EXT4_INODE_EXTENTS)) {
-		ret = -EOPNOTSUPP;
-		goto out_mutex;
-	}