On 2/12/24 22:11, Josef Bacik wrote:
Btrfs had a deadlock that you could trigger by mmap'ing a large file and
using that as the buffer for fiemap. This test adds a c program to do
this, and the fstest creates a large enough file and then runs the
reproducer on the file. Without the fix btrfs deadlocks, with the fix
we pass fine.
Signed-off-by: Josef Bacik <josef@xxxxxxxxxxxxxx>
---
v1->v2:
- Add the fiemap group to the test.
- Actually include the reproducer helper program.
src/Makefile | 2 +-
src/fiemap-fault.c | 73 +++++++++++++++++++++++++++++++++++++++++++
tests/generic/740 | 41 ++++++++++++++++++++++++
tests/generic/740.out | 2 ++
4 files changed, 117 insertions(+), 1 deletion(-)
create mode 100644 src/fiemap-fault.c
create mode 100644 tests/generic/740
create mode 100644 tests/generic/740.out
diff --git a/src/Makefile b/src/Makefile
index d79015ce..916f6755 100644
--- a/src/Makefile
+++ b/src/Makefile
@@ -34,7 +34,7 @@ LINUX_TARGETS = xfsctl bstat t_mtab getdevicesize preallo_rw_pattern_reader \
attr_replace_test swapon mkswap t_attr_corruption t_open_tmpfiles \
fscrypt-crypt-util bulkstat_null_ocount splice-test chprojid_fail \
detached_mounts_propagation ext4_resize t_readdir_3 splice2pipe \
- uuid_ioctl
+ uuid_ioctl fiemap-fault
EXTRA_EXECS = dmerror fill2attr fill2fs fill2fs_check scaleread.sh \
btrfs_crc32c_forged_name.py popdir.pl popattr.py \
diff --git a/src/fiemap-fault.c b/src/fiemap-fault.c
new file mode 100644
index 00000000..27081188
--- /dev/null
+++ b/src/fiemap-fault.c
Pls add fiemap-fault file to .gitignore
more below.
@@ -0,0 +1,73 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (c) 2024 Meta Platforms, Inc. All Rights Reserved.
+ */
+
+#include <sys/ioctl.h>
+#include <sys/mman.h>
+#include <sys/types.h>
+#include <sys/stat.h>
+#include <linux/fs.h>
+#include <linux/types.h>
+#include <linux/fiemap.h>
+#include <err.h>
+#include <errno.h>
+#include <fcntl.h>
+#include <stdio.h>
+#include <string.h>
+#include <unistd.h>
+
+int prep_mmap_buffer(int fd, void **addr)
+{
+ struct stat st;
+ int ret;
+
+ ret = fstat(fd, &st);
+ if (ret)
+ err(1, "failed to stat %d", fd);
+
+ *addr = mmap(NULL, st.st_size, PROT_READ|PROT_WRITE, MAP_SHARED, fd, 0);
+ if (*addr == MAP_FAILED)
+ err(1, "failed to mmap %d", fd);
+
+ return st.st_size;
+}
+
+int main(int argc, char *argv[])
+{
+ struct fiemap *fiemap;
+ size_t sz, last = 0;
+ void *buf = NULL;
+ int ret, fd;
+
+ if (argc != 2)
+ errx(1, "no in and out file name arguments given");
+
+ fd = open(argv[1], O_RDWR, 0666);
+ if (fd == -1)
+ err(1, "failed to open %s", argv[1]);
+
+ sz = prep_mmap_buffer(fd, &buf);
+
+ fiemap = (struct fiemap *)buf;
+ fiemap->fm_flags = 0;
+ fiemap->fm_extent_count = (sz - sizeof(struct fiemap)) /
+ sizeof(struct fiemap_extent);
+
+ while (last < sz) {
+ int i;
+
+ fiemap->fm_start = last;
+ fiemap->fm_length = sz - last;
+
+ ret = ioctl(fd, FS_IOC_FIEMAP, (unsigned long)fiemap);
+ if (ret < 0)
+ err(1, "fiemap failed %d (%s)", errno, strerror(errno));
+ for (i = 0; i < fiemap->fm_mapped_extents; i++)
+ last = fiemap->fm_extents[i].fe_logical +
+ fiemap->fm_extents[i].fe_length;
+ }
+
munmap() is missing.
a few more below.
+ close(fd);
+ return 0;
+}
diff --git a/tests/generic/740 b/tests/generic/740
new file mode 100644
index 00000000..30ace1dd
--- /dev/null
+++ b/tests/generic/740
@@ -0,0 +1,41 @@
+#! /bin/bash
+# SPDX-License-Identifier: GPL-2.0
+# Copyright (c) 2023 Meta Platforms, Inc. All Rights Reserved.
+#
+# FS QA Test 708
+#
+# Test fiemap into an mmaped buffer of the same file
+#
+# Create a reasonably large file, then run a program which mmaps it and uses
+# that as a buffer for an fiemap call. This is a regression test for btrfs
+# where we used to hold a lock for the duration of the fiemap call which would
+# result in a deadlock if we page faulted.
+#
+. ./common/preamble
+_begin_fstest quick auto fiemap
+[ $FSTYP == "btrfs" ] && \
+ _fixed_by_kernel_commit xxxxxxxxxxxx \
+ "btrfs: fix deadlock with fiemap and extent locking"
+
+# real QA test starts here
+_supported_fs generic
+_require_test
+_require_odirect
+_require_test_program fiemap-fault
+dst=$TEST_DIR/fiemap-fault-$seq
Could we pls use:
dst=$TEST_DIR/$seq/fiemap-fault
mkdir -p $dst
Keeps $TEST_DIR organized.
+
+echo "Silence is golden"
+
+for i in $(seq 0 2 1000)
+do
+ $XFS_IO_PROG -d -f -c "pwrite -q $((i * 4096)) 4096" $dst
+done
+
+$here/src/fiemap-fault $dst > /dev/null || _fail "failed doing fiemap"
+
+rm -f $dst
Pls move this to custom clean up at _cleanup() within the testcase.
Thanks, Anand
+
+# success, all done
+status=$?
+exit
+
diff --git a/tests/generic/740.out b/tests/generic/740.out
new file mode 100644
index 00000000..3f841e60
--- /dev/null
+++ b/tests/generic/740.out
@@ -0,0 +1,2 @@
+QA output created by 740
+Silence is golden
