deadlock between mm_sem and tx assign in zfs_write() and page fault

The bug time sequence:
1. thread #1, `zfs_write` assign a txg "n".
2. In a same process, thread #2, mmap page fault (which means the
   `mm_sem` is hold) occurred, `zfs_dirty_inode` open a txg failed,
   and wait previous txg "n" completed.
3. thread #1 call `uiomove` to write, however page fault is occurred
   in `uiomove`, which means it need `mm_sem`, but `mm_sem` is hold by
   thread #2, so it stuck and can't complete,  then txg "n" will
   not complete.

So thread #1 and thread #2 are deadlocked.

Reviewed-by: Chunwei Chen <[email protected]>
Reviewed-by: Brian Behlendorf <[email protected]>
Reviewed-by: Matthew Ahrens <[email protected]>
Signed-off-by: Grady Wong <[email protected]>
Closes #7939 

2 files changed, 104 insertions, 44 deletions

diff --git a/tests/zfs-tests/cmd/mmapwrite/mmapwrite.c b/tests/zfs-tests/cmd/mmapwrite/mmapwrite.c
index 190d31af3..b9915d5d3 100644
--- a/tests/zfs-tests/cmd/mmapwrite/mmapwrite.c
+++ b/tests/zfs-tests/cmd/mmapwrite/mmapwrite.c
@@ -31,74 +31,132 @@
 #include <string.h>
 #include <sys/mman.h>
 #include <pthread.h>
+#include <errno.h>
+#include <err.h>
 
 /*
  * --------------------------------------------------------------------
- * Bug Id: 5032643
+ * Bug Issue Id: #7512
+ * The bug time sequence:
+ * 1. context #1, zfs_write assign a txg "n".
+ * 2. In the same process, context #2, mmap page fault (which means the mm_sem
+ *    is hold) occurred, zfs_dirty_inode open a txg failed, and wait previous
+ *    txg "n" completed.
+ * 3. context #1 call uiomove to write, however page fault is occurred in
+ *    uiomove, which means it need mm_sem, but mm_sem is hold by
+ *    context #2, so it stuck and can't complete, then txg "n" will not
+ *    complete.
  *
- * Simply writing to a file and mmaping that file at the same time can
- * result in deadlock.  Nothing perverse like writing from the file's
- * own mapping is required.
+ * So context #1 and context #2 trap into the "dead lock".
  * --------------------------------------------------------------------
  */
 
+#define	NORMAL_WRITE_TH_NUM	2
+
 static void *
-mapper(void *fdp)
+normal_writer(void *filename)
 {
-	void *addr;
-	int fd = *(int *)fdp;
+	char *file_path = filename;
+	int fd = -1;
+	ssize_t write_num = 0;
+	int page_size = getpagesize();
 
-	if ((addr =
-	    mmap(0, 8192, PROT_READ, MAP_SHARED, fd, 0)) == MAP_FAILED) {
-		perror("mmap");
-		exit(1);
+	fd = open(file_path, O_RDWR | O_CREAT, 0777);
+	if (fd == -1) {
+		err(1, "failed to open %s", file_path);
 	}
-	for (;;) {
-		if (mmap(addr, 8192, PROT_READ,
-		    MAP_SHARED|MAP_FIXED, fd, 0) == MAP_FAILED) {
-			perror("mmap");
-			exit(1);
+
+	char *buf = malloc(1);
+	while (1) {
+		write_num = write(fd, buf, 1);
+		if (write_num == 0) {
+			err(1, "write failed!");
+			break;
 		}
+		lseek(fd, page_size, SEEK_CUR);
+	}
+
+	if (buf) {
+		free(buf);
 	}
-	/* NOTREACHED */
-	return ((void *)1);
 }
 
-int
-main(int argc, char **argv)
+static void *
+map_writer(void *filename)
 {
-	int fd;
-	char buf[1024];
-	pthread_t tid;
+	int fd = -1;
+	int ret = 0;
+	char *buf = NULL;
+	int page_size = getpagesize();
+	int op_errno = 0;
+	char *file_path = filename;
 
-	memset(buf, 'a', sizeof (buf));
+	while (1) {
+		ret = access(file_path, F_OK);
+		if (ret) {
+			op_errno = errno;
+			if (op_errno == ENOENT) {
+				fd = open(file_path, O_RDWR | O_CREAT, 0777);
+				if (fd == -1) {
+					err(1, "open file failed");
+				}
 
-	if (argc != 2) {
-		(void) printf("usage: %s <file name>\n", argv[0]);
-		exit(1);
-	}
+				ret = ftruncate(fd, page_size);
+				if (ret == -1) {
+					err(1, "truncate file failed");
+				}
+			} else {
+				err(1, "access file failed!");
+			}
+		} else {
+			fd = open(file_path, O_RDWR, 0777);
+			if (fd == -1) {
+				err(1, "open file failed");
+			}
+		}
 
-	if ((fd = open(argv[1], O_RDWR|O_CREAT|O_TRUNC, 0666)) == -1) {
-		perror("open");
-		exit(1);
+		if ((buf = mmap(NULL, page_size, PROT_READ | PROT_WRITE,
+		    MAP_SHARED, fd, 0)) == MAP_FAILED) {
+			err(1, "map file failed");
+		}
+
+		if (fd != -1)
+			close(fd);
+
+		char s[10] = {0, };
+		memcpy(buf, s, 10);
+		ret = munmap(buf, page_size);
+		if (ret != 0) {
+			err(1, "unmap file failed");
+		}
 	}
+}
 
-	(void) pthread_setconcurrency(2);
-	if (pthread_create(&tid, NULL, mapper, &fd) != 0) {
-		perror("pthread_create");
-		close(fd);
+int
+main(int argc, char **argv)
+{
+	pthread_t map_write_tid;
+	pthread_t normal_write_tid[NORMAL_WRITE_TH_NUM];
+	int i = 0;
+
+	if (argc != 3) {
+		(void) printf("usage: %s <normal write file name>"
+		    "<map write file name>\n", argv[0]);
 		exit(1);
 	}
-	for (;;) {
-		if (write(fd, buf, sizeof (buf)) == -1) {
-			perror("write");
-			close(fd);
-			exit(1);
+
+	for (i = 0; i < NORMAL_WRITE_TH_NUM; i++) {
+		if (pthread_create(&normal_write_tid[i], NULL, normal_writer,
+		    argv[1])) {
+			err(1, "pthread_create normal_writer failed.");
 		}
 	}
 
-	close(fd);
+	if (pthread_create(&map_write_tid, NULL, map_writer, argv[2])) {
+		err(1, "pthread_create map_writer failed.");
+	}
 
 	/* NOTREACHED */
+	pthread_join(map_write_tid, NULL);
 	return (0);
 }
diff --git a/tests/zfs-tests/tests/functional/mmap/mmap_write_001_pos.ksh b/tests/zfs-tests/tests/functional/mmap/mmap_write_001_pos.ksh
index 1eda97104..24150b827 100755
--- a/tests/zfs-tests/tests/functional/mmap/mmap_write_001_pos.ksh
+++ b/tests/zfs-tests/tests/functional/mmap/mmap_write_001_pos.ksh
@@ -53,12 +53,14 @@ if ! is_mp; then
 fi
 
 log_must chmod 777 $TESTDIR
-mmapwrite $TESTDIR/test-write-file &
+mmapwrite $TESTDIR/normal_write_file $TESTDIR/map_write_file &
 PID_MMAPWRITE=$!
-log_note "mmapwrite $TESTDIR/test-write-file pid: $PID_MMAPWRITE"
+log_note "mmapwrite $TESTDIR/normal_write_file $TESTDIR/map_write_file"\
+	 "pid: $PID_MMAPWRITE"
 log_must sleep 30
 
 log_must kill -9 $PID_MMAPWRITE
-log_must ls -l $TESTDIR/test-write-file
+log_must ls -l $TESTDIR/normal_write_file
+log_must ls -l $TESTDIR/map_write_file
 
 log_pass "write(2) a mmap(2)'ing file succeeded."