Fix mmap / libaio deadlock

Calling uiomove() in mappedread() under the page lock can result
in a deadlock if the user space page needs to be faulted in.

Resolve the issue by dropping the page lock before the uiomove().
The inode range lock protects against concurrent updates via
zfs_read() and zfs_write().

Reviewed-by: Albert Lee <[email protected]>
Reviewed-by: Chunwei Chen <[email protected]>
Signed-off-by: Brian Behlendorf <[email protected]>
Closes #7335 
Closes #7339 

14 files changed, 184 insertions, 5 deletions

diff --git a/config/user-libaio.m4 b/config/user-libaio.m4
new file mode 100644
index 000000000..d7a7cb508
--- /dev/null
+++ b/config/user-libaio.m4
@@ -0,0 +1,14 @@
+dnl #
+dnl # Check for libaio - only used for libaiot test cases.
+dnl #
+AC_DEFUN([ZFS_AC_CONFIG_USER_LIBAIO], [
+	LIBAIO=
+
+	AC_CHECK_HEADER([libaio.h], [
+	    user_libaio=yes
+	    AC_SUBST([LIBAIO], ["-laio"])
+	    AC_DEFINE([HAVE_LIBAIO], 1, [Define if you have libaio])
+	], [
+	    user_libaio=no
+	])
+])
diff --git a/config/user.m4 b/config/user.m4
index f5e5e9acd..f2f366e3c 100644
--- a/config/user.m4
+++ b/config/user.m4
@@ -13,6 +13,7 @@ AC_DEFUN([ZFS_AC_CONFIG_USER], [
 	ZFS_AC_CONFIG_USER_LIBBLKID
 	ZFS_AC_CONFIG_USER_LIBUDEV
 	ZFS_AC_CONFIG_USER_LIBSSL
+	ZFS_AC_CONFIG_USER_LIBAIO
 	ZFS_AC_CONFIG_USER_RUNSTATEDIR
 	ZFS_AC_CONFIG_USER_MAKEDEV_IN_SYSMACROS
 	ZFS_AC_CONFIG_USER_MAKEDEV_IN_MKDEV
diff --git a/config/zfs-build.m4 b/config/zfs-build.m4
index 818e99982..d9b052e27 100644
--- a/config/zfs-build.m4
+++ b/config/zfs-build.m4
@@ -140,11 +140,11 @@ AC_DEFUN([ZFS_AC_CONFIG], [
 	AM_CONDITIONAL([CONFIG_KERNEL],
 	    [test "$ZFS_CONFIG" = kernel -o "$ZFS_CONFIG" = all] &&
 	    [test "x$enable_linux_builtin" != xyes ])
-	AM_CONDITIONAL([WANT_DEVNAME2DEVID],
-	    [test "x$user_libudev" = xyes ])
 	AM_CONDITIONAL([CONFIG_QAT],
 	    [test "$ZFS_CONFIG" = kernel -o "$ZFS_CONFIG" = all] &&
 	    [test "x$qatsrc" != x ])
+	AM_CONDITIONAL([WANT_DEVNAME2DEVID], [test "x$user_libudev" = xyes ])
+	AM_CONDITIONAL([WANT_MMAP_LIBAIO], [test "x$user_libaio" = xyes ])
 ])
 
 dnl #
diff --git a/configure.ac b/configure.ac
index c5585ea40..5a84ffcbf 100644
--- a/configure.ac
+++ b/configure.ac
@@ -165,6 +165,7 @@ AC_CONFIG_FILES([
 	tests/zfs-tests/cmd/mkfiles/Makefile
 	tests/zfs-tests/cmd/mktree/Makefile
 	tests/zfs-tests/cmd/mmap_exec/Makefile
+	tests/zfs-tests/cmd/mmap_libaio/Makefile
 	tests/zfs-tests/cmd/mmapwrite/Makefile
 	tests/zfs-tests/cmd/nvlist_to_lua/Makefile
 	tests/zfs-tests/cmd/randfree_file/Makefile
diff --git a/module/zfs/zfs_vnops.c b/module/zfs/zfs_vnops.c
index a2d7d7b24..14caa80e5 100644
--- a/module/zfs/zfs_vnops.c
+++ b/module/zfs/zfs_vnops.c
@@ -400,6 +400,7 @@ mappedread(struct inode *ip, int nbytes, uio_t *uio)
 		pp = find_lock_page(mp, start >> PAGE_SHIFT);
 		if (pp) {
 			ASSERT(PageUptodate(pp));
+			unlock_page(pp);
 
 			pb = kmap(pp);
 			error = uiomove(pb + off, bytes, UIO_READ, uio);
@@ -409,7 +410,6 @@ mappedread(struct inode *ip, int nbytes, uio_t *uio)
 				flush_dcache_page(pp);
 
 			mark_page_accessed(pp);
-			unlock_page(pp);
 			put_page(pp);
 		} else {
 			error = dmu_read_uio_dbuf(sa_get_db(zp->z_sa_hdl),
diff --git a/rpm/generic/zfs.spec.in b/rpm/generic/zfs.spec.in
index 52dc80a4e..beb7671a1 100644
--- a/rpm/generic/zfs.spec.in
+++ b/rpm/generic/zfs.spec.in
@@ -189,6 +189,7 @@ Requires:       fio
 Requires:       acl
 Requires:       sudo
 Requires:       sysstat
+Requires:	libaio
 AutoReqProv:    no
 
 %description test
diff --git a/tests/runfiles/linux.run b/tests/runfiles/linux.run
index 7c2ca84bf..185293716 100644
--- a/tests/runfiles/linux.run
+++ b/tests/runfiles/linux.run
@@ -572,7 +572,7 @@ tests = ['migration_001_pos', 'migration_002_pos', 'migration_003_pos',
 tags = ['functional', 'migration']
 
 [tests/functional/mmap]
-tests = ['mmap_write_001_pos', 'mmap_read_001_pos']
+tests = ['mmap_write_001_pos', 'mmap_read_001_pos', 'mmap_libaio_001_pos']
 tags = ['functional', 'mmap']
 
 [tests/functional/mmp]
diff --git a/tests/zfs-tests/cmd/Makefile.am b/tests/zfs-tests/cmd/Makefile.am
index bc19f184c..0673fcbf6 100644
--- a/tests/zfs-tests/cmd/Makefile.am
+++ b/tests/zfs-tests/cmd/Makefile.am
@@ -14,6 +14,7 @@ SUBDIRS = \
 	mkfiles \
 	mktree \
 	mmap_exec \
+	mmap_libaio \
 	mmapwrite \
 	nvlist_to_lua \
 	randfree_file \
diff --git a/tests/zfs-tests/cmd/mmap_libaio/.gitignore b/tests/zfs-tests/cmd/mmap_libaio/.gitignore
new file mode 100644
index 000000000..792c8d340
--- /dev/null
+++ b/tests/zfs-tests/cmd/mmap_libaio/.gitignore
@@ -0,0 +1 @@
+/mmap_libaio
diff --git a/tests/zfs-tests/cmd/mmap_libaio/Makefile.am b/tests/zfs-tests/cmd/mmap_libaio/Makefile.am
new file mode 100644
index 000000000..67d0f0ece
--- /dev/null
+++ b/tests/zfs-tests/cmd/mmap_libaio/Makefile.am
@@ -0,0 +1,9 @@
+include $(top_srcdir)/config/Rules.am
+
+pkgexecdir = $(datadir)/@PACKAGE@/zfs-tests/bin
+
+if WANT_MMAP_LIBAIO
+pkgexec_PROGRAMS = mmap_libaio
+mmap_libaio_SOURCES = mmap_libaio.c
+mmap_libaio_LDADD = $(LIBAIO)
+endif
diff --git a/tests/zfs-tests/cmd/mmap_libaio/mmap_libaio.c b/tests/zfs-tests/cmd/mmap_libaio/mmap_libaio.c
new file mode 100644
index 000000000..21119ebca
--- /dev/null
+++ b/tests/zfs-tests/cmd/mmap_libaio/mmap_libaio.c
@@ -0,0 +1,88 @@
+/*
+ * CDDL HEADER START
+ *
+ * The contents of this file are subject to the terms of the
+ * Common Development and Distribution License (the "License").
+ * You may not use this file except in compliance with the License.
+ *
+ * You can obtain a copy of the license at usr/src/OPENSOLARIS.LICENSE
+ * or http://www.opensolaris.org/os/licensing.
+ * See the License for the specific language governing permissions
+ * and limitations under the License.
+ *
+ * When distributing Covered Code, include this CDDL HEADER in each
+ * file and include the License file at usr/src/OPENSOLARIS.LICENSE.
+ * If applicable, add the following below this CDDL HEADER, with the
+ * fields enclosed by brackets "[]" replaced with your own identifying
+ * information: Portions Copyright [yyyy] [name of copyright owner]
+ *
+ * CDDL HEADER END
+ */
+
+/*
+ * Copyright 2018 Canonical.  All rights reserved.
+ */
+
+#include <stdio.h>
+#include <stdlib.h>
+#include <unistd.h>
+#include <fcntl.h>
+#include <sys/mman.h>
+#include <sys/types.h>
+#include <sys/stat.h>
+#include <libaio.h>
+#include <err.h>
+
+io_context_t io_ctx;
+
+static void
+do_sync_io(struct iocb *iocb)
+{
+	struct io_event event;
+	struct iocb *iocbs[] = { iocb };
+	struct timespec ts = { 30, 0 };
+
+	if (io_submit(io_ctx, 1, iocbs) != 1)
+		err(1, "io_submit failed");
+
+	if (io_getevents(io_ctx, 0, 1, &event, &ts) != 1)
+		err(1, "io_getevents failed");
+}
+
+int
+main(int argc, char **argv)
+{
+	char *buf;
+	int page_size = getpagesize();
+	int buf_size = strtol(argv[2], NULL, 0);
+	int rwfd;
+	struct iocb iocb;
+
+	if (io_queue_init(1024, &io_ctx))
+		err(1, "io_queue_init failed");
+
+	rwfd = open(argv[1], O_RDWR | O_CREAT, S_IRUSR | S_IWUSR);
+	if (rwfd < 0)
+		err(1, "open failed");
+
+	if (ftruncate(rwfd, buf_size) < 0)
+		err(1, "ftruncate failed");
+
+	buf = mmap(0, page_size, PROT_READ | PROT_WRITE, MAP_SHARED, rwfd, 0);
+	if (buf == MAP_FAILED)
+		err(1, "mmap failed");
+
+	(void) io_prep_pwrite(&iocb, rwfd, buf, buf_size, 0);
+	do_sync_io(&iocb);
+
+	(void) io_prep_pread(&iocb, rwfd, buf, buf_size, 0);
+	do_sync_io(&iocb);
+
+	if (close(rwfd))
+		err(1, "close failed");
+
+	if (io_queue_release(io_ctx) != 0)
+		err(1, "io_queue_release failed");
+
+	return (0);
+}
diff --git a/tests/zfs-tests/include/commands.cfg b/tests/zfs-tests/include/commands.cfg
index 64d6ad5d7..2a6c3b13d 100644
--- a/tests/zfs-tests/include/commands.cfg
+++ b/tests/zfs-tests/include/commands.cfg
@@ -164,6 +164,7 @@ export ZFSTEST_FILES='chg_usr_exec
     mkfiles
     mktree
     mmap_exec
+    mmap_libaio
     mmapwrite
     nvlist_to_lua
     randfree_file
diff --git a/tests/zfs-tests/tests/functional/mmap/Makefile.am b/tests/zfs-tests/tests/functional/mmap/Makefile.am
index cb444ec37..4e1db6041 100644
--- a/tests/zfs-tests/tests/functional/mmap/Makefile.am
+++ b/tests/zfs-tests/tests/functional/mmap/Makefile.am
@@ -4,4 +4,5 @@ dist_pkgdata_SCRIPTS = \
 	cleanup.ksh \
 	mmap.cfg \
 	mmap_read_001_pos.ksh \
-	mmap_write_001_pos.ksh
+	mmap_write_001_pos.ksh \
+	mmap_libaio_001_pos.ksh
diff --git a/tests/zfs-tests/tests/functional/mmap/mmap_libaio_001_pos.ksh b/tests/zfs-tests/tests/functional/mmap/mmap_libaio_001_pos.ksh
new file mode 100755
index 000000000..36a7e76f9
--- /dev/null
+++ b/tests/zfs-tests/tests/functional/mmap/mmap_libaio_001_pos.ksh
@@ -0,0 +1,61 @@
+#!/bin/ksh -p
+#
+# CDDL HEADER START
+#
+# The contents of this file are subject to the terms of the
+# Common Development and Distribution License (the "License").
+# You may not use this file except in compliance with the License.
+#
+# You can obtain a copy of the license at usr/src/OPENSOLARIS.LICENSE
+# or http://www.opensolaris.org/os/licensing.
+# See the License for the specific language governing permissions
+# and limitations under the License.
+#
+# When distributing Covered Code, include this CDDL HEADER in each
+# file and include the License file at usr/src/OPENSOLARIS.LICENSE.
+# If applicable, add the following below this CDDL HEADER, with the
+# fields enclosed by brackets "[]" replaced with your own identifying
+# information: Portions Copyright [yyyy] [name of copyright owner]
+#
+# CDDL HEADER END
+#
+
+#
+# Copyright 2018 Canonical.  All rights reserved.
+#
+
+. $STF_SUITE/include/libtest.shlib
+. $STF_SUITE/tests/functional/mmap/mmap.cfg
+
+#
+# DESCRIPTION:
+# Verify libaio functions correctly with mmap()'d files.
+#
+# STRATEGY:
+# 1. Call mmap_libaio binary
+# 2. Verify the file exists and is the expected size
+# 3. Verify the filesystem is intact and not hung in any way
+#
+
+verify_runnable "global"
+
+log_assert "verify mmap'd pages work with libaio"
+
+# mmap_libaio is built when the libaio-devel package is installed.
+if ! which mmap_libaio; then
+	log_unsupported "This test requires mmap_libaio."
+fi
+
+log_must chmod 777 $TESTDIR
+
+for size in 512 4096 8192; do
+	log_mustnot stat $TESTDIR/test-libaio-file
+	log_must mmap_libaio $TESTDIR/test-libaio-file $size
+	log_must verify_eq $(stat --format=%s $TESTDIR/test-libaio-file) $size
+	log_must rm $TESTDIR/test-libaio-file
+done
+
+typeset dir=$(get_device_dir $DISKS)
+verify_filesys "$TESTPOOL" "$TESTPOOL/$TESTFS" "$dir"
+
+log_pass "mmap'd pages work with libaio"