[developer] [openzfs/openzfs] Merge remote-tracking branch 'illumos/master' into illumos-sync (#545)

[zettabot]

You can view, comment on, or merge this pull request online at:

  https://github.com/openzfs/openzfs/pull/545

-- Commit Summary --

  * 8792 mac_tx_srs_quiesce() can kill a maxbw link
  * 8652 Tautological comparisons with ZPROP_INVAL (followup)
  * 8760 troff: variable 'type' set but not used
  * 8967 libipmi: add support for GET_CHASSIS_STATUS command
  * 8493 kmem_move taskq appears to be inducing significant system latency
  * 8805 xattr_dir_lookup() can leak a vnode hold
  * 8731 ASSERT3U(nui64s, <=, UINT16_MAX) fails for large blocks
  * 8680 Time of Day clock error
  * 8985 update tzdata to 2018c
  * 8389 Convert rm(1) manpage to mdoc(5)
  * 8986 loader: try_multiple_kernels does not try multiple kernels
  * 8987 bootadm: add bootfile fallback to unix
  * 8685 uts and mdb: do not build 32bit kernel
  * 8997 ztest assertion failure in zil_lwb_write_issue
  * 8571 Makefile.master should not trust $PATH
  * 8990 /opt/onbld/gk is useless
  * 8991 pmodes is useless and can be deleted
  * 8992 checkproto is useless and can be deleted
  * 8998 depcheck is useless and should be removed
  * backout: 8571 Makefile.master should not trust $PATH (insufficient testing, 
broke bootstrap)
  * 8994 libc-tests: nl_langinfo_test fails after 1198
  * 9003 libc-tests: newlocale_test silently fails after CLDR updates
  * 8668 mac_srs_change_upcall() is unused
  * 8969 Cannot boot from RAIDZ with parity > 1
  * 8809 libzpool should leverage work done in libfakekernel
  * 9015 krb5-config emits unnecessary -R flags
  * 6690 set_nfsv4_ephemeral_mount_to() tries to read AUTOMOUNT_TIMEOUT from 
defunct /etc/default/autofs
  * 9007 import AT regex test suite
  * 9012 cmd/grep: don't use obsolete REG_ANCHOR
  * 9013 cmd/vi: don't use obsolete REG_WORDS
  * 9001 cdm is useless, remove it
  * 9005 remove .hgignore
  * 9006 parallel loader builds fail sporadically
  * 9023 port Delphix Xen-related fixes
  * 9024 rework PV-HVM disk device handling
  * 8914 loader: gcc 4.4.4 fails to allocate register for do_cpuid()
  * 8498 ficl: variable 'count' might be clobbered by 'longjmp' or 'vfork'
  * 9029 libc: duplicate 'const' declaration specifier
  * 8999 SMBIOS: cleanup 32-bit specific code
  * 9000 unix: cleanup 32-bit specific code in fakebop
  * 8210 uts: remove kb streams module
  * 9017 Introduce taskq_empty()
  * 9018 Replace kmem_cache_reap_now() with kmem_cache_reap_soon()
  * 9028 libc: comparison between pointer and zero character constant
  * 9027 Makefiles need to specify C99 mode consistently
  * 9022 loader.efi: module placement must check memory map
  * 9039 tcp(7p): Duplicate paragraph
  * 8966 Source file zfs_acl.c, function zfs_aclset_common contains a use after 
end of the lifetime of a local variable
  * 7638 Refactor spa_load_impl into several functions
  * 8961 SPA load/import should tell us why it failed
  * 8962 zdb should work on non-idle pools
  * 8706 libc lint library missing endian.h functions
  * 8472 Want docs for iports, tgtmaps, and friends
  * 8733 cxgbe: variable 'execute' set but not used
  * 9031 sgs/libld: comparison between pointer and zero character constant
  * 9032 sgs: this statement may fall through
  * 9034 pfexecd: duplicate lint target in Makefile
  * 9035 zfs: this statement may fall through
  * 9036 zfs: duplicate 'const' declaration specifier
  * 9037 pfexecd: this statement may fall through
  * 9038 fs.d/nfs: this statement may fall through
  * 9040 libnsl: unused local typedef in rpc code
  * 9025 libzfs missing dependency on libcmdutils
  * 9026 lib/fm missing dependency on libsff
  * 9033 nightly MUST use cw in tools proto
  * 8408 dsl_props_set_sync_impl() does not handle nested nvlists correctly
  * 8477 Assertion failed in vdev_state_dirty(): spa_writeable(spa)
  * 8520 lzc_rollback_to should support rolling back to origin
  * 8941 zpool add: assertion failed in get_replication() with nested interior 
VDEVs
  * 8942 zfs promote .../%recv should be an error
  * 9004 Some ZFS tests used files removed with 32 bit kernel
  * 8965 zfs_acl_ls_001_pos fails due to no longer supported grep regex
  * 9064 ZFS test remove_mirror should wait for device removal to complete
  * 8989 Allow IKEV2 pf_key(7P) key management cookies to be updated after set
  * 8933 libefi: Add definitions and utilities for EFI drivers
  * 9043 ipadm(1M) is misrendering the create-addr subcommand description
  * 9069 libfru: comparison between pointer and zero character constant
  * 9066 xdf devices attach hybrid VTOC/EFI label
  * 9070 Remove wanboot from gate
  * 8871 Want means of toggling data link LEDs
  * 8867 add MAC_CAPAB_TRANSCEIVER support for cxgbe
  * 8866 bnxe MAC_CAPAB_TRANSCEIVER support
  * 7094 mdb can't print types from an object file with ctf
  * Merge remote-tracking branch 'illumos/master' into illumos-sync

-- File Changes --

D .hgignore (28)
M exception_lists/closed-bins (32)
M exception_lists/cstyle (1)
M exception_lists/packaging (13)
M usr/src/Makefile.ast (2)
   

[developer] Re: [openzfs/openzfs] 8115 parallel zfs mount (v2) (#536)

[Prakash Surya]

@prakashsurya pushed 1 commit.

181dfd2  Use LOCK_ERRORCHECK plus mutex_enter/mutex_exit


-- 
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/openzfs/openzfs/pull/536/files/b3f4bdf804d187b4d17d6721d3b47c2e53685e07..181dfd22949ac3867ec32f0a039fa804c84492c1

--
openzfs-developer
Archives: 
https://openzfs.topicbox.com/groups/developer/discussions/T22334a01fda83bfd-Me846074996a121f6a998f1a4
Powered by Topicbox: https://topicbox.com

[developer] Re: [openzfs/openzfs] 8115 parallel zfs mount (v2) (#536)

[Prakash Surya]

prakashsurya commented on this pull request.



> + */
+static void
+zfs_dispatch_mount(libzfs_handle_t *hdl, zfs_handle_t **handles,
+size_t num_handles, int idx, zfs_iter_f func, void *data, taskq_t *tq)
+{
+   mnt_param_t *mnt_param = zfs_alloc(hdl, sizeof (mnt_param_t));
+
+   mnt_param->mnt_hdl = hdl;
+   mnt_param->mnt_tq = tq;
+   mnt_param->mnt_zhps = handles;
+   mnt_param->mnt_num_handles = num_handles;
+   mnt_param->mnt_idx = idx;
+   mnt_param->mnt_func = func;
+   mnt_param->mnt_data = data;
+
+   (void) taskq_dispatch(tq, zfs_mount_task, (void*)mnt_param, TQ_SLEEP);

@ahrens how do we handle allocation failures elsewhere in the zfs userland code?

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/openzfs/openzfs/pull/536#discussion_r167410762
--
openzfs-developer
Archives: 
https://openzfs.topicbox.com/groups/developer/discussions/T22334a01fda83bfd-Mbdca5797a15afaf77da84a8f
Powered by Topicbox: https://topicbox.com

[developer] Re: [openzfs/openzfs] 8115 parallel zfs mount (v2) (#536)

[Prakash Surya]

prakashsurya commented on this pull request.



> @@ -88,6 +89,9 @@
 #include 
 #defineMAXISALEN   257 /* based on sysinfo(2) man page */
 
+static int mount_tq_nthr = 512;/* taskq threads for multi-threaded 
mounting */

@sebroy can you comment here? I don't see anything in the original review about 
this.

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/openzfs/openzfs/pull/536#discussion_r167410760
--
openzfs-developer
Archives: 
https://openzfs.topicbox.com/groups/developer/discussions/T22334a01fda83bfd-M54fbae5bdc60397f9ebc68ac
Powered by Topicbox: https://topicbox.com

[developer] Re: [openzfs/openzfs] 9074 domount() interprets ZFS filesystem names as relative paths (#538)

[Robert Mustacchi]

Thanks for the detailed write up and separating this out. In general, this 
looks good and I think we have the right file systems marked as required.

@jjelinek does the mount in progress table get used for some of the zone/NFS 
related checks? I remember there being something that we talked about here in 
the ancient past, but I can't find any notes from that discussion.

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/openzfs/openzfs/pull/538#issuecomment-364684165
--
openzfs-developer
Archives: 
https://openzfs.topicbox.com/groups/developer/discussions/T7060187599fd888c-M0948d9ebc21709d46c96f7c3
Powered by Topicbox: https://topicbox.com

[developer] Re: [openzfs/openzfs] 9091 Add mdb smart write (#544)

[Robert Mustacchi]

rmustacc commented on this pull request.



> @@ -198,6 +198,100 @@ write_uint64(mdb_tgt_as_t as, mdb_tgt_addr_t addr, 
> uint64_t n, uint_t rdback)
return (addr + sizeof (n));
 }
 
+/*
+ * Writes to objects of size 1, 2, 4, or 8 bytes. The function
+ * doesn't care if the object is a number or not (e.g. it could
+ * be a byte array, or a struct) as long as the size of the write
+ * is one of the aforementioned ones.
+ */
+static mdb_tgt_addr_t
+write_var_uint(mdb_tgt_as_t as, mdb_tgt_addr_t addr, uint64_t val, size_t size,
+uint_t rdback)
+{
+   if (size > sizeof (uintptr_t)) {

It might be, but even that is a little questionable. The max we have in the 
code is 8 and we're not relating that to a type. So I'd probably actually use 
that and just make a `#define` with a comment that explains its the max size 
that the targets can write today. As it's really less specific to a type and 
more specific to what the target allows.

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/openzfs/openzfs/pull/544#discussion_r167407506
--
openzfs-developer
Archives: 
https://openzfs.topicbox.com/groups/developer/discussions/Ta82fe96bb714cae3-M47dbcd7b02256b2841b9b854
Powered by Topicbox: https://topicbox.com

[developer] Re: [openzfs/openzfs] 9091 Add mdb smart write (#544)

[Serapheim Dimitropoulos]

sdimitro commented on this pull request.



> @@ -198,6 +198,100 @@ write_uint64(mdb_tgt_as_t as, mdb_tgt_addr_t addr, 
> uint64_t n, uint_t rdback)
return (addr + sizeof (n));
 }
 
+/*
+ * Writes to objects of size 1, 2, 4, or 8 bytes. The function
+ * doesn't care if the object is a number or not (e.g. it could
+ * be a byte array, or a struct) as long as the size of the write
+ * is one of the aforementioned ones.
+ */
+static mdb_tgt_addr_t
+write_var_uint(mdb_tgt_as_t as, mdb_tgt_addr_t addr, uint64_t val, size_t size,
+uint_t rdback)
+{
+   if (size > sizeof (uintptr_t)) {

You are right. The check here is to ensure that we don't write more bytes than 
the longest integer supported.

Looking into the different integer types it seems like `uintmax_t` would be the 
right type for this. Does this sound reasonable to you?

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/openzfs/openzfs/pull/544#discussion_r167407281
--
openzfs-developer
Archives: 
https://openzfs.topicbox.com/groups/developer/discussions/Ta82fe96bb714cae3-Mf451df75631a9a9e19aafcad
Powered by Topicbox: https://topicbox.com

[developer] Re: [openzfs/openzfs] 9075 Improve ZFS pool import/load process and corrupted pool recovery (#539)

[Andrew Stormont]

andy-js commented on this pull request.

Overall looks good.

> +#
+# We want to test the case where a whole created by a log device is filled
+# by a regular device
+#
+function test_remove_log_then_add_vdev
+{
+   log_note "$0."
+   log_must zpool create -o cachefile=$CPATH $TESTPOOL1 \
+   $VDEV0 $VDEV1 $VDEV2 log $VDEV3
+
+   log_must cp $CPATH $CPATHBKP
+
+   log_must zpool remove $TESTPOOL1 $VDEV1
+   log_must wait_for_pool_config $TESTPOOL1 "$VDEV0 $VDEV2 log $VDEV3"
+   log_must zpool remove $TESTPOOL1 $VDEV3
+log_must check_pool_config $TESTPOOL1 "$VDEV0 $VDEV2"

The indentation looks wonky here.

> @@ -461,6 +461,16 @@ kernel_fini(void)
system_taskq_fini();
 }
 
+/* ARGSUSED */
+uint32_t
+zone_get_hostid(void *zonep)

I think this probably belongs in libfakekernel.

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/openzfs/openzfs/pull/539#pullrequestreview-95153387
--
openzfs-developer
Archives: 
https://openzfs.topicbox.com/groups/developer/discussions/T603f896faeeb4d25-Mf1cc508ea5824637283cc241
Powered by Topicbox: https://topicbox.com

[developer] Re: [openzfs/openzfs] 9091 Add mdb smart write (#544)

[Robert Mustacchi]

rmustacc commented on this pull request.

In general, this looks good. Thanks.

> @@ -198,6 +198,100 @@ write_uint64(mdb_tgt_as_t as, mdb_tgt_addr_t addr, 
> uint64_t n, uint_t rdback)
return (addr + sizeof (n));
 }
 
+/*
+ * Writes to objects of size 1, 2, 4, or 8 bytes. The function
+ * doesn't care if the object is a number or not (e.g. it could
+ * be a byte array, or a struct) as long as the size of the write
+ * is one of the aforementioned ones.
+ */
+static mdb_tgt_addr_t
+write_var_uint(mdb_tgt_as_t as, mdb_tgt_addr_t addr, uint64_t val, size_t size,
+uint_t rdback)
+{
+   if (size > sizeof (uintptr_t)) {

Why are we using the uintptr_t as the sentinel for the size? For example, if we 
were building a 32-bit version of this, then we'd have an ILP32 system so the 
uintptr_t would be 4 bytes; however, you can write 8 bytes.

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/openzfs/openzfs/pull/544#pullrequestreview-95628953
--
openzfs-developer
Archives: 
https://openzfs.topicbox.com/groups/developer/discussions/Ta82fe96bb714cae3-M39dc308abc7dc1f1a20217a8
Powered by Topicbox: https://topicbox.com

[developer] [openzfs/openzfs] Add mdb smart write (#544)

[Serapheim Dimitropoulos]

Reviewed by: Matthew Ahrens 
Reviewed by: Paul Dagnelie 
Reviewed by: Prashanth Sreenivasa 

Currently, if you want to write to a kernel tunable using
mdb, you need to use the /Z or /W flags to write 8 or 4
bytes respectively. If you use the wrong one, you will either
overwrite only part of the tunable, or overrun the end of the
tunable and zero out the next 4 bytes of memory. That second
option in particular can be disastrous. Given that MDB knows
the size of the tunables because it has CTF data, it would be
nice if MDB supported a "smart write" format string that
selected the size of the write based on the size of the thing
being written to. It should fail or require a size be specified
if we're writing to an area without a specific size.

A new dcmd and a new format for writing are added to mdb.

New format:
- Looks like this -> [address]/z [value]
- Writes the value in the address starting from the specified
  address
- The size of the write is inferred by the CTF data of the
  symbol at the address provided
- If no CTF data exist at the given address, the write fails

New dcmd:
- Looks like this -> [address]::write [value]
- Works exactly like the format specifier above
- For physical writes specify the -p argument
  (i.e. [address]\z [value])
- For object file writes specify the -o argument
  (i.e. [address]?z [value])
- If no CTF data was found and the write fails the
  user can force a write with -l [size]
- The allowed lengths of a forced write are 1, 2, 4,
  and 8 bytes.

Both of the above work with integer, pointer, and enum types
(except the -l option in the dcmd that doesn't care about the type).

Upstream Bugs: DLPX-48141
You can view, comment on, or merge this pull request online at:

  https://github.com/openzfs/openzfs/pull/544

-- Commit Summary --

  * Add mdb smart write

-- File Changes --

M usr/src/cmd/mdb/common/mdb/mdb_cmds.c (192)
M usr/src/cmd/mdb/common/mdb/mdb_fmt.c (4)
M usr/src/cmd/mdb/common/mdb/mdb_lex.l (3)
M usr/src/man/man1/mdb.1 (2)

-- Patch Links --

https://github.com/openzfs/openzfs/pull/544.patch
https://github.com/openzfs/openzfs/pull/544.diff

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/openzfs/openzfs/pull/544

--
openzfs-developer
Archives: 
https://openzfs.topicbox.com/groups/developer/discussions/Tab9df74196897556-M3a6e2ea09de07e449b354460
Powered by Topicbox: https://topicbox.com

[developer] Re: [openzfs/openzfs] 8115 parallel zfs mount (v2) (#536)

[Prakash Surya]

prakashsurya commented on this pull request.



> @@ -416,6 +416,7 @@ enum {
 #defineVSW_XID 0x40/* file system supports extended ids */
 #defineVSW_CANLOFI 0x80/* file system supports lofi mounts */
 #defineVSW_ZMOUNT  0x100   /* file system always allowed in a zone 
*/
+#defineVSW_MOUNTDEV0x200   /* file system is mounted via device 
path */

Yes, this is in the PR here: https://github.com/openzfs/openzfs/pull/538

Since that PR is a pre-requisite of this ZFS change, I have both changes in 
this diff. If you only look at the second commit of the PR, 
[here](https://github.com/openzfs/openzfs/pull/536/commits/b3f4bdf804d187b4d17d6721d3b47c2e53685e07),
 that'll contain only the ZFS changes.

Sorry, Github's PR system doesn't have a way to chain dependent PRs, AFAIK.

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/openzfs/openzfs/pull/536#discussion_r167405524
--
openzfs-developer
Archives: 
https://openzfs.topicbox.com/groups/developer/discussions/T22334a01fda83bfd-M8ebebf19877c41d5d0362473
Powered by Topicbox: https://topicbox.com

[developer] Re: [openzfs/openzfs] 8115 parallel zfs mount (v2) (#536)

[Prakash Surya]

@rmustacc Thanks for the reply. Let me know after you give this version a look. 
If you think `libzfs` being a consumer of `libfakekernel`'s taskq 
implementation, then let's discuss the best way to break that out so we can do 
this properly.

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/openzfs/openzfs/pull/536#issuecomment-364674812
--
openzfs-developer
Archives: 
https://openzfs.topicbox.com/groups/developer/discussions/T22334a01fda83bfd-M8d4fa57dea868d8d70fb
Powered by Topicbox: https://topicbox.com

[developer] Re: [openzfs/openzfs] 8115 parallel zfs mount (v2) (#536)

[Robert Mustacchi]

rmustacc commented on this pull request.



> @@ -416,6 +416,7 @@ enum {
 #defineVSW_XID 0x40/* file system supports extended ids */
 #defineVSW_CANLOFI 0x80/* file system supports lofi mounts */
 #defineVSW_ZMOUNT  0x100   /* file system always allowed in a zone 
*/
+#defineVSW_MOUNTDEV0x200   /* file system is mounted via device 
path */

I thought we had said this was being pulled out into a separate change?

> - if (verbose)
-   report_mount_progress(i, count);
-
-   if (share_mount_one(dslist[i], op, flags, protocol,
-   B_FALSE, options) != 0)
-   ret = 1;
-   zfs_close(dslist[i]);
-   }
+   share_mount_state_t share_mount_state = { 0 };
+   share_mount_state.sm_op = op;
+   share_mount_state.sm_verbose = verbose;
+   share_mount_state.sm_flags = flags;
+   share_mount_state.sm_options = options;
+   share_mount_state.sm_proto = protocol;
+   share_mount_state.sm_total = cb.cb_used;
+   (void) mutex_init(_mount_state.sm_lock, USYNC_THREAD,

Missing LOCK_ERRORCHECK

> @@ -785,6 +786,7 @@ libzfs_mnttab_cache_compare(const void *arg1, const void 
> *arg2)
 void
 libzfs_mnttab_init(libzfs_handle_t *hdl)
 {
+   (void) mutex_init(>libzfs_mnttab_cache_lock, USYNC_THREAD, NULL);

Missing LOCK_ERRORCHECK

> @@ -854,16 +858,18 @@ libzfs_mnttab_find(libzfs_handle_t *hdl, const char 
> *fsname,
return (ENOENT);
}
 
+   (void) mutex_lock(>libzfs_mnttab_cache_lock);

Use mutex_enter(), the version in user land. It will do all the error checking 
and abort appropriately. Otherwise, you need to check the return value and blow 
up if it's non-zero so we abort on deadlock.

>   }
-   return (ENOENT);
+   (void) mutex_unlock(>libzfs_mnttab_cache_lock);

Use mutex_exit(), the version in user land. It will do all the error checking 
and abort appropriately. Otherwise, you need to check the return value and blow 
up if it's non-zero so we abort on deadlock or unlocking a lock that we didn't 
own.

> @@ -872,14 +878,16 @@ libzfs_mnttab_add(libzfs_handle_t *hdl, const char 
> *special,
 {
mnttab_node_t *mtn;
 
-   if (avl_numnodes(>libzfs_mnttab_cache) == 0)
-   return;
-   mtn = zfs_alloc(hdl, sizeof (mnttab_node_t));
-   mtn->mtn_mt.mnt_special = zfs_strdup(hdl, special);
-   mtn->mtn_mt.mnt_mountp = zfs_strdup(hdl, mountp);
-   mtn->mtn_mt.mnt_fstype = zfs_strdup(hdl, MNTTYPE_ZFS);
-   mtn->mtn_mt.mnt_mntopts = zfs_strdup(hdl, mntopts);
-   avl_add(>libzfs_mnttab_cache, mtn);
+   (void) mutex_lock(>libzfs_mnttab_cache_lock);



Use mutex_enter(), the version in user land. It will do all the error checking 
and abort appropriately. Otherwise, you need to check the return value and blow 
up if it's non-zero so we abort on deadlock.


> - mtn = zfs_alloc(hdl, sizeof (mnttab_node_t));
-   mtn->mtn_mt.mnt_special = zfs_strdup(hdl, special);
-   mtn->mtn_mt.mnt_mountp = zfs_strdup(hdl, mountp);
-   mtn->mtn_mt.mnt_fstype = zfs_strdup(hdl, MNTTYPE_ZFS);
-   mtn->mtn_mt.mnt_mntopts = zfs_strdup(hdl, mntopts);
-   avl_add(>libzfs_mnttab_cache, mtn);
+   (void) mutex_lock(>libzfs_mnttab_cache_lock);
+   if (avl_numnodes(>libzfs_mnttab_cache) != 0) {
+   mtn = zfs_alloc(hdl, sizeof (mnttab_node_t));
+   mtn->mtn_mt.mnt_special = zfs_strdup(hdl, special);
+   mtn->mtn_mt.mnt_mountp = zfs_strdup(hdl, mountp);
+   mtn->mtn_mt.mnt_fstype = zfs_strdup(hdl, MNTTYPE_ZFS);
+   mtn->mtn_mt.mnt_mntopts = zfs_strdup(hdl, mntopts);
+   avl_add(>libzfs_mnttab_cache, mtn);
+   }
+   (void) mutex_unlock(>libzfs_mnttab_cache_lock);



Use mutex_exit(), the version in user land. It will do all the error checking 
and abort appropriately. Otherwise, you need to check the return value and blow 
up if it's non-zero so we abort on deadlock or unlocking a lock that we didn't 
own.


> @@ -888,6 +896,7 @@ libzfs_mnttab_remove(libzfs_handle_t *hdl, const char 
> *fsname)
mnttab_node_t find;
mnttab_node_t *ret;
 
+   (void) mutex_lock(>libzfs_mnttab_cache_lock);

Same mutex comments.

> @@ -898,6 +907,7 @@ libzfs_mnttab_remove(libzfs_handle_t *hdl, const char 
> *fsname)
free(ret->mtn_mt.mnt_mntopts);
free(ret);
}
+   (void) mutex_unlock(>libzfs_mnttab_cache_lock);

Same mutex comments.

> @@ -88,6 +89,9 @@
 #include 
 #defineMAXISALEN   257 /* based on sysinfo(2) man page */
 
+static int mount_tq_nthr = 512;/* taskq threads for multi-threaded 
mounting */

How did we determine this number? How does this change if you're not in the 
global zone trying to 

[developer] Re: [openzfs/openzfs] 8115 parallel zfs mount (v2) (#536)

[Robert Mustacchi]

@prakashsurya Thanks for splitting out the change regarding the file system 
part, that makes things clearer.

Honestly though, no, I still feel like this is the wrong approach in terms of 
leveraging the fake kernel library. But the fake kernel and libzpool are 
similar to me in so far as their goal is to emulate the kernel. This is a user 
land program which really shouldn't need any of these and while portability 
isn't my first concern, starts making things pretty odd for those porting this.

Honestly, if we take a step back at the original version, the only thing I was 
unhappy with was saying that we had a general purpose taskq user library 
because you weren't giving control to applications about how to handle memory 
allocation by using the umem fatal stuff. If we had scoped that to be private 
just for zfs or private in so far as just the fake kernel and the zfs user land 
commands using it, it probably would be a better approach. I'm sorry that I 
haven't been as active in the other parts. When I saw the fake kernel bits 
going on, I hadn't realized it was going on for use here.

I'm not sure if it makes sense. I see that there's now been Andy's changes and 
breaking it up further. But honestly, this still feels like a lot of complexity 
that we're introducing into the headers and builds and complications. I'll take 
a look at this current version and try to evaluate it. I'll try and hold my 
breath about the fake kernel bits.

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/openzfs/openzfs/pull/536#issuecomment-364673929
--
openzfs-developer
Archives: 
https://openzfs.topicbox.com/groups/developer/discussions/T22334a01fda83bfd-M1d32a62dcfc3381456eefbc4
Powered by Topicbox: https://topicbox.com

[developer] Re: [openzfs/openzfs] 8857 zio_remove_child() panic due to already destroyed parent zio (#505)

[Andriy Gapon]

avg-I commented on this pull request.



> @@ -3534,11 +3545,11 @@ zio_done(zio_t *zio)
 * If our children haven't all completed,
 * wait for them and then repeat this pipeline stage.
 */
-   if (zio_wait_for_children(zio, ZIO_CHILD_VDEV, ZIO_WAIT_DONE) ||
-   zio_wait_for_children(zio, ZIO_CHILD_GANG, ZIO_WAIT_DONE) ||
-   zio_wait_for_children(zio, ZIO_CHILD_DDT, ZIO_WAIT_DONE) ||
-   zio_wait_for_children(zio, ZIO_CHILD_LOGICAL, ZIO_WAIT_DONE))
+   if (zio_wait_for_children(zio, ZIO_CHILD_VDEV_BIT |
+   ZIO_CHILD_GANG_BIT | ZIO_CHILD_DDT_BIT | ZIO_CHILD_LOGICAL_BIT,
+   ZIO_WAIT_DONE)) {

One small and probably last nit-pick, would we benefit from defining 
`ZIO_CHILD_ALL_BITS` and using it here?

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/openzfs/openzfs/pull/505#pullrequestreview-95624819
--
openzfs-developer
Archives: 
https://openzfs.topicbox.com/groups/developer/discussions/Td0adf1bfacf39ae0-M769c82a642435d083fd6bfda
Powered by Topicbox: https://topicbox.com

[developer] Re: [openzfs/openzfs] 8857 zio_remove_child() panic due to already destroyed parent zio (#505)

[Andriy Gapon]

avg-I approved this pull request.

LGTM



-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/openzfs/openzfs/pull/505#pullrequestreview-95624832
--
openzfs-developer
Archives: 
https://openzfs.topicbox.com/groups/developer/discussions/Td0adf1bfacf39ae0-Md31dbd08b553202ae00674f9
Powered by Topicbox: https://topicbox.com

[developer] Re: [openzfs/openzfs] 8857 zio_remove_child() panic due to already destroyed parent zio (#505)

[George Wilson]

@grwilson pushed 1 commit.

445e551  predefine child bits


-- 
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/openzfs/openzfs/pull/505/files/b1342b226af1aa8a39cbcbe3b59a9270765c8ef2..445e55128ca0a1cf9b0a7013d996edd50da3a0f4

--
openzfs-developer
Archives: 
https://openzfs.topicbox.com/groups/developer/discussions/Td0adf1bfacf39ae0-M70d1f4a1e1285c1d085c2cc6
Powered by Topicbox: https://topicbox.com