]> granicus.if.org Git - zfs/commitdiff
Fix mmap / libaio deadlock
authorBrian Behlendorf <behlendorf1@llnl.gov>
Wed, 28 Mar 2018 17:19:22 +0000 (10:19 -0700)
committerTony Hutter <hutter2@llnl.gov>
Tue, 8 May 2018 00:19:57 +0000 (17:19 -0700)
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 <trisk@forkgnu.org>
Reviewed-by: Chunwei Chen <david.chen@nutanix.com>
Signed-off-by: Brian Behlendorf <behlendorf1@llnl.gov>
Closes #7335
Closes #7339

14 files changed:
config/user-libaio.m4 [new file with mode: 0644]
config/user.m4
config/zfs-build.m4
configure.ac
module/zfs/zfs_vnops.c
rpm/generic/zfs.spec.in
tests/runfiles/linux.run
tests/zfs-tests/cmd/Makefile.am
tests/zfs-tests/cmd/mmap_libaio/.gitignore [new file with mode: 0644]
tests/zfs-tests/cmd/mmap_libaio/Makefile.am [new file with mode: 0644]
tests/zfs-tests/cmd/mmap_libaio/mmap_libaio.c [new file with mode: 0644]
tests/zfs-tests/include/commands.cfg
tests/zfs-tests/tests/functional/mmap/Makefile.am
tests/zfs-tests/tests/functional/mmap/mmap_libaio_001_pos.ksh [new file with mode: 0755]

diff --git a/config/user-libaio.m4 b/config/user-libaio.m4
new file mode 100644 (file)
index 0000000..d7a7cb5
--- /dev/null
@@ -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
+       ])
+])
index 3d725195d82dedbe7c0ea9ca7d01678dc832b814..3f52c3e5ca39c133e41ef90fa114e9deaf6aac9f 100644 (file)
@@ -13,6 +13,7 @@ AC_DEFUN([ZFS_AC_CONFIG_USER], [
        ZFS_AC_CONFIG_USER_LIBBLKID
        ZFS_AC_CONFIG_USER_LIBUDEV
        ZFS_AC_CONFIG_USER_FRAME_LARGER_THAN
+       ZFS_AC_CONFIG_USER_LIBAIO
        ZFS_AC_CONFIG_USER_RUNSTATEDIR
        ZFS_AC_CONFIG_USER_MAKEDEV_IN_SYSMACROS
        ZFS_AC_CONFIG_USER_MAKEDEV_IN_MKDEV
index e6c9c21e0730b9822d4580c8d6550870d83bf639..85571a8addde6bc1aee08fd36a683c39590fa0eb 100644 (file)
@@ -118,11 +118,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 #
index 77e5764fc7652513fe295128c13b5207da3a4078..d9441a0f506c2401b3691764b066147cae557be4 100644 (file)
@@ -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/randfree_file/Makefile
        tests/zfs-tests/cmd/readmmap/Makefile
index 6f6ce79db20e92c66412387bd336e230c0143531..9646a1fa9461164382b9d48b5734157984682d5a 100644 (file)
@@ -398,6 +398,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);
@@ -407,7 +408,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),
index 769bdab104c7863c494f04172fa771ba836e8177..0a5d1c543a90c8d329471786f5421a31588b1484 100644 (file)
@@ -183,6 +183,7 @@ Requires:       acl
 Requires:       sudo
 Requires:       sysstat
 Requires:       rng-tools
+Requires:      libaio
 AutoReqProv:    no
 
 %description test
index 516c1c3b852d2c2364d02e548aeb851c0d5bd364..a26f3b89b1f1cc2e39eaeeb416df239a32a0d6ed 100644 (file)
@@ -485,7 +485,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]
index 1cce6947b9714601a21adccc0e8fa678c47b66a4..7c3a9653945b8acbb053b33190483ca99b88e452 100644 (file)
@@ -14,6 +14,7 @@ SUBDIRS = \
        mkfiles \
        mktree \
        mmap_exec \
+       mmap_libaio \
        mmapwrite \
        randfree_file \
        readmmap \
diff --git a/tests/zfs-tests/cmd/mmap_libaio/.gitignore b/tests/zfs-tests/cmd/mmap_libaio/.gitignore
new file mode 100644 (file)
index 0000000..792c8d3
--- /dev/null
@@ -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 (file)
index 0000000..67d0f0e
--- /dev/null
@@ -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 (file)
index 0000000..21119eb
--- /dev/null
@@ -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);
+}
index 0d768a3cfa221e6077e046e06f82fa3f67a08372..1399ad55f1c9f26bc7437d9a332d1e21bcbd2422 100644 (file)
@@ -158,6 +158,7 @@ export ZFSTEST_FILES='chg_usr_exec
     mkfiles
     mktree
     mmap_exec
+    mmap_libaio
     mmapwrite
     randfree_file
     readmmap
index cb444ec379d0ab11ab1a02021a7883fd69f83152..4e1db6041f41d7cdc2897073b71a878a9d9684a9 100644 (file)
@@ -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 (executable)
index 0000000..36a7e76
--- /dev/null
@@ -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"