Merge pull request #74 from versity/zab/fallocate_read_inversion

Zab/fallocate read inversion

kmod/src/data.c

@@ -983,9 +983,6 @@ long scoutfs_fallocate(struct file *file, int mode, loff_t offset, loff_t len)
 	u64 last;
 	s64 ret;
 
-	mutex_lock(&inode->i_mutex);
-	down_write(&si->extent_sem);
-
 	/* XXX support more flags */
         if (mode & ~(FALLOC_FL_KEEP_SIZE)) {
 		ret = -EOPNOTSUPP;
@@ -1003,18 +1000,22 @@ long scoutfs_fallocate(struct file *file, int mode, loff_t offset, loff_t len)
 		goto out;
 	}
 
+	mutex_lock(&inode->i_mutex);
+
 	ret = scoutfs_lock_inode(sb, SCOUTFS_LOCK_WRITE,
 				 SCOUTFS_LKF_REFRESH_INODE, inode, &lock);
 	if (ret)
-		goto out;
+		goto out_mutex;
 
 	inode_dio_wait(inode);
 
+	down_write(&si->extent_sem);
+
 	if (!(mode & FALLOC_FL_KEEP_SIZE) &&
 	    (offset + len > i_size_read(inode))) {
                 ret = inode_newsize_ok(inode, offset + len);
                 if (ret)
-                        goto out;
+                        goto out_extent;
         }
 
 	iblock = offset >> SCOUTFS_BLOCK_SM_SHIFT;
@@ -1024,7 +1025,7 @@ long scoutfs_fallocate(struct file *file, int mode, loff_t offset, loff_t len)
 
 		ret = scoutfs_inode_index_lock_hold(inode, &ind_locks, false, true);
 		if (ret)
-			goto out;
+			goto out_extent;
 
 		ret = fallocate_extents(sb, inode, iblock, last, lock);
 
@@ -1050,17 +1051,19 @@ long scoutfs_fallocate(struct file *file, int mode, loff_t offset, loff_t len)
 		}
 
 		if (ret <= 0)
-			goto out;
+			goto out_extent;
 
 		iblock += ret;
 		ret = 0;
 	}
 
-out:
-	scoutfs_unlock(sb, lock, SCOUTFS_LOCK_WRITE);
+out_extent:
 	up_write(&si->extent_sem);
+out_mutex:
+	scoutfs_unlock(sb, lock, SCOUTFS_LOCK_WRITE);
 	mutex_unlock(&inode->i_mutex);
 
+out:
 	trace_scoutfs_data_fallocate(sb, ino, mode, offset, len, ret);
 	return ret;
 }

tests/golden/fallocate

@@ -0,0 +1,3 @@
+== creating reasonably large per-mount files
+== 10s of racing cold reads and fallocate nop
+== cleaning up files

tests/sequence

@@ -4,6 +4,7 @@ inode-items-updated.sh
 simple-inode-index.sh
 simple-staging.sh
 simple-release-extents.sh
+fallocate.sh
 setattr_more.sh
 offline-extent-waiting.sh
 move-blocks.sh

tests/tests/fallocate.sh

@@ -0,0 +1,38 @@
+
+t_require_commands fallocate cat
+
+echo "== creating reasonably large per-mount files"
+for n in $(t_fs_nrs); do
+	eval path="\$T_D${n}/file-$n"
+
+	LC_ALL=C fallocate -l 128MiB  "$path" || \
+		t_fail "initial creating fallocate failed"
+done
+
+#
+# we had lock inversions between read and fallocate, dropping
+# the cache each time forces waiting for IO during the calls
+# with the inverted locks held so we have a better chance
+# of the deadlock happening.
+#
+DURATION=10
+echo "== ${DURATION}s of racing cold reads and fallocate nop"
+END=$((SECONDS + DURATION))
+while [ $SECONDS -le $END ]; do
+
+	echo 3 > /proc/sys/vm/drop_caches
+
+	for n in $(t_fs_nrs); do
+		eval path="\$T_D${n}/file-$n"
+		
+		LC_ALL=C fallocate -o 0 -l 4KiB  "$path" &
+		cat "$path" > /dev/null &
+	done
+
+	wait || t_fail "fallocate or cat failed"
+done
+
+echo "== cleaning up files"
+rm -f "$T_D0"/file-*
+
+t_pass