aboutsummaryrefslogtreecommitdiff
path: root/cddl/contrib/opensolaris/lib
diff options
context:
space:
mode:
authorBaptiste Daroussin <bapt@FreeBSD.org>2019-07-26 13:12:33 +0000
committerBaptiste Daroussin <bapt@FreeBSD.org>2019-07-26 13:12:33 +0000
commitf34d9e5d6b1ef2b93389b86d36e89a85913529f6 (patch)
tree89b445fec350d6b343240bd8bd8647c21908d2ce /cddl/contrib/opensolaris/lib
parent45a5aec3f156d8a01fe1ea4ec87c1b1d489f13ac (diff)
downloadsrc-f34d9e5d6b1ef2b93389b86d36e89a85913529f6.tar.gz
src-f34d9e5d6b1ef2b93389b86d36e89a85913529f6.zip
Fix a bug introduced with parallel mounting of zfs
Incorporate a fix from zol: https://github.com/zfsonlinux/zfs/commit/ab5036df1ccbe1b18c1ce6160b5829e8039d94ce commit log from upstream: Fix race in parallel mount's thread dispatching algorithm Strategy of parallel mount is as follows. 1) Initial thread dispatching is to select sets of mount points that don't have dependencies on other sets, hence threads can/should run lock-less and shouldn't race with other threads for other sets. Each thread dispatched corresponds to top level directory which may or may not have datasets to be mounted on sub directories. 2) Subsequent recursive thread dispatching for each thread from 1) is to mount datasets for each set of mount points. The mount points within each set have dependencies (i.e. child directories), so child directories are processed only after parent directory completes. The problem is that the initial thread dispatching in zfs_foreach_mountpoint() can be multi-threaded when it needs to be single-threaded, and this puts threads under race condition. This race appeared as mount/unmount issues on ZoL for ZoL having different timing regarding mount(2) execution due to fork(2)/exec(2) of mount(8). `zfs unmount -a` which expects proper mount order can't unmount if the mounts were reordered by the race condition. There are currently two known patterns of input list `handles` in `zfs_foreach_mountpoint(..,handles,..)` which cause the race condition. 1) #8833 case where input is `/a /a /a/b` after sorting. The problem is that libzfs_path_contains() can't correctly handle an input list with two same top level directories. There is a race between two POSIX threads A and B, * ThreadA for "/a" for test1 and "/a/b" * ThreadB for "/a" for test0/a and in case of #8833, ThreadA won the race. Two threads were created because "/a" wasn't considered as `"/a" contains "/a"`. 2) #8450 case where input is `/ /var/data /var/data/test` after sorting. The problem is that libzfs_path_contains() can't correctly handle an input list containing "/". There is a race between two POSIX threads A and B, * ThreadA for "/" and "/var/data/test" * ThreadB for "/var/data" and in case of #8450, ThreadA won the race. Two threads were created because "/var/data" wasn't considered as `"/" contains "/var/data"`. In other words, if there is (at least one) "/" in the input list, the initial thread dispatching must be single-threaded since every directory is a child of "/", meaning they all directly or indirectly depend on "/". In both cases, the first non_descendant_idx() call fails to correctly determine "path1-contains-path2", and as a result the initial thread dispatching creates another thread when it needs to be single-threaded. Fix a conditional in libzfs_path_contains() to consider above two. Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov> Reviewed by: Sebastien Roy <sebastien.roy@delphix.com> Signed-off-by: Tomohiro Kusumi <kusumi.tomohiro@gmail.com> PR: 237517, 237397, 239243 Submitted by: Matthew D. Fuller <fullermd@over-yonder.net> (by email) MFC after: 3 days
Notes
Notes: svn path=/head/; revision=350358
Diffstat (limited to 'cddl/contrib/opensolaris/lib')
-rw-r--r--cddl/contrib/opensolaris/lib/libzfs/common/libzfs_mount.c6
1 files changed, 4 insertions, 2 deletions
diff --git a/cddl/contrib/opensolaris/lib/libzfs/common/libzfs_mount.c b/cddl/contrib/opensolaris/lib/libzfs/common/libzfs_mount.c
index a53020f86667..0efeba631aff 100644
--- a/cddl/contrib/opensolaris/lib/libzfs/common/libzfs_mount.c
+++ b/cddl/contrib/opensolaris/lib/libzfs/common/libzfs_mount.c
@@ -1281,12 +1281,14 @@ mountpoint_cmp(const void *arga, const void *argb)
}
/*
- * Reutrn true if path2 is a child of path1
+ * Return true if path2 is a child of path1 or path2 equals path1 or
+ * path1 is "/" (path2 is always a child of "/").
*/
static boolean_t
libzfs_path_contains(const char *path1, const char *path2)
{
- return (strstr(path2, path1) == path2 && path2[strlen(path1)] == '/');
+ return (strcmp(path1, path2) == 0 || strcmp(path1, "/") == 0 ||
+ (strstr(path2, path1) == path2 && path2[strlen(path1)] == '/'));
}