[PATCH v2] generic: add test for direct io partial writes

[Date Prev] [Date Next] [Thread Prev] [Thread Next] [Date Index] [Thread Index]



btrfs recently had a bug where a direct io partial write resulted in a
hole in the file. Add a new generic test which creates a 2MiB file,
mmaps it, touches the first byte, then does an O_DIRECT write of the
mmapped buffer into a new file. This should result in the mapped pages
being a mix of in and out of page cache and thus a partial write, for
filesystems using iomap and IOMAP_DIO_PARTIAL.

Signed-off-by: Boris Burkov <boris@xxxxxx>
---
Changelog:
v2:
- hide fd in prep_mmap_buffer, we weren't closing it in main
- get rid of unneeded filters/cleanup in test script
- make pwrite pattern explicit
- send random mmapped char to /dev/null
- gate _fixed_by_kernel_commit by FSTYP
- remove extra sync after writing file
- use $seq in test filenames

 .gitignore            |  1 +
 src/Makefile          |  2 +-
 src/dio-buf-fault.c   | 83 +++++++++++++++++++++++++++++++++++++++++++
 tests/generic/708     | 37 +++++++++++++++++++
 tests/generic/708.out |  2 ++
 5 files changed, 124 insertions(+), 1 deletion(-)
 create mode 100644 src/dio-buf-fault.c
 create mode 100755 tests/generic/708
 create mode 100644 tests/generic/708.out

diff --git a/.gitignore b/.gitignore
index cfff8f85..644290f0 100644
--- a/.gitignore
+++ b/.gitignore
@@ -72,6 +72,7 @@ tags
 /src/deduperace
 /src/detached_mounts_propagation
 /src/devzero
+/src/dio-buf-fault
 /src/dio-interleaved
 /src/dio-invalidate-cache
 /src/dirhash_collide
diff --git a/src/Makefile b/src/Makefile
index a574f7bd..24cd4747 100644
--- a/src/Makefile
+++ b/src/Makefile
@@ -19,7 +19,7 @@ TARGETS = dirstress fill fill2 getpagesize holes lstat64 \
 	t_ofd_locks t_mmap_collision mmap-write-concurrent \
 	t_get_file_time t_create_short_dirs t_create_long_dirs t_enospc \
 	t_mmap_writev_overlap checkpoint_journal mmap-rw-fault allocstale \
-	t_mmap_cow_memory_failure fake-dump-rootino
+	t_mmap_cow_memory_failure fake-dump-rootino dio-buf-fault
 
 LINUX_TARGETS = xfsctl bstat t_mtab getdevicesize preallo_rw_pattern_reader \
 	preallo_rw_pattern_writer ftrunc trunc fs_perms testx looptest \
diff --git a/src/dio-buf-fault.c b/src/dio-buf-fault.c
new file mode 100644
index 00000000..911c3e1f
--- /dev/null
+++ b/src/dio-buf-fault.c
@@ -0,0 +1,83 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (c) 2023 Meta Platforms, Inc.  All Rights Reserved.
+ */
+
+#ifndef _GNU_SOURCE
+#define _GNU_SOURCE /* to get definition of O_DIRECT flag. */
+#endif
+
+#include <sys/mman.h>
+#include <sys/types.h>
+#include <sys/stat.h>
+#include <err.h>
+#include <errno.h>
+#include <fcntl.h>
+#include <stdio.h>
+#include <unistd.h>
+
+/*
+ * mmap a source file, then do a direct write of that mmapped region to a
+ * destination file.
+ */
+
+int prep_mmap_buffer(char *src_filename, void **addr)
+{
+	struct stat st;
+	int fd;
+	int ret;
+
+	fd = open(src_filename, O_RDWR, 0666);
+	if (fd == -1)
+		err(1, "failed to open %s", src_filename);
+
+	ret = fstat(fd, &st);
+	if (ret)
+		err(1, "failed to stat %d", fd);
+
+	*addr = mmap(NULL, st.st_size, PROT_READ, MAP_PRIVATE, fd, 0);
+	if (*addr == MAP_FAILED)
+		err(1, "failed to mmap %d", fd);
+
+	return st.st_size;
+}
+
+int do_dio(char *dst_filename, void *buf, size_t sz)
+{
+	int fd;
+	ssize_t ret;
+
+	fd = open(dst_filename, O_CREAT | O_TRUNC | O_WRONLY | O_DIRECT, 0666);
+	if (fd == -1)
+		err(1, "failed to open %s", dst_filename);
+	while (sz) {
+		ret = write(fd, buf, sz);
+		if (ret < 0) {
+			if (errno == -EINTR)
+				continue;
+			else
+				err(1, "failed to write %lu bytes to %d", sz, fd);
+		} else if (ret == 0) {
+			break;
+		}
+		buf += ret;
+		sz -= ret;
+	}
+	return sz;
+}
+
+int main(int argc, char *argv[]) {
+	size_t sz;
+	void *buf = NULL;
+	char c;
+
+	if (argc != 3)
+		errx(1, "no in and out file name arguments given");
+	sz = prep_mmap_buffer(argv[1], &buf);
+
+	/* touch the first page of the mapping to bring it into cache */
+	c = ((char *)buf)[0];
+	printf("%u\n", c);
+
+	do_dio(argv[2], buf, sz);
+}
diff --git a/tests/generic/708 b/tests/generic/708
new file mode 100755
index 00000000..1f0843c7
--- /dev/null
+++ b/tests/generic/708
@@ -0,0 +1,37 @@
+#! /bin/bash
+# SPDX-License-Identifier: GPL-2.0
+# Copyright (c) 2023 Meta Platforms, Inc.  All Rights Reserved.
+#
+# FS QA Test 708
+#
+# Test iomap direct_io partial writes.
+#
+# Create a reasonably large file, then run a program which mmaps it,
+# touches the first page, then dio writes it to a second file. This
+# can result in a page fault reading from the mmapped dio write buffer and
+# thus the iomap direct_io partial write codepath.
+#
+. ./common/preamble
+_begin_fstest quick auto
+[ $FSTYP == "btrfs" ] && \
+	_fixed_by_kernel_commit XXXX 'btrfs: fix dio continue after short write due to buffer page fault'
+
+# real QA test starts here
+_supported_fs generic
+_require_test
+_require_odirect
+_require_test_program dio-buf-fault
+src=$TEST_DIR/dio-buf-fault-$seq.src
+dst=$TEST_DIR/dio-buf-fault-$seq.dst
+
+rm -rf "$src" "$dst"
+
+echo "Silence is golden"
+
+$XFS_IO_PROG -fc "pwrite -q -S 0xcd 0 $((2 * 1024 * 1024))" $src
+$here/src/dio-buf-fault $src $dst > /dev/null || _fail "failed doing the dio copy"
+diff $src $dst
+
+# success, all done
+status=$?
+exit
diff --git a/tests/generic/708.out b/tests/generic/708.out
new file mode 100644
index 00000000..33c478ad
--- /dev/null
+++ b/tests/generic/708.out
@@ -0,0 +1,2 @@
+QA output created by 708
+Silence is golden
-- 
2.39.1




[Index of Archives]     [Linux Filesystems Development]     [Linux NFS]     [Linux NILFS]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux