freebsd-dev/cddl
Baptiste Daroussin f34d9e5d6b Fix a bug introduced with parallel mounting of zfs
Incorporate a fix from zol:
ab5036df1c

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
2019-07-26 13:12:33 +00:00
..
compat/opensolaris Honor all options of "zfs mount -o" 2017-09-05 19:28:35 +00:00
contrib/opensolaris Fix a bug introduced with parallel mounting of zfs 2019-07-26 13:12:33 +00:00
lib Create new EINTEGRITY error with message "Integrity check failed". 2019-01-17 06:35:45 +00:00
sbin Do not blindly include illumos kernel headers instead of user-space. 2018-08-02 18:55:55 +00:00
tests Merge ^/user/ngie/release-pkg-fix-tests to unbreak how test files are installed 2016-05-04 23:20:53 +00:00
usr.bin Make dnode definition uniform on !x86 2018-08-21 03:45:09 +00:00
usr.sbin DTrace: add a top level makefile to the new test suit 2019-06-09 22:45:07 +00:00
Makefile Convert traditional ${MK_TESTS} conditional idiom for including test 2017-08-02 08:35:51 +00:00
Makefile.inc Make DTrace stuff compile with C99 standard. 2014-08-22 20:04:51 +00:00