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

On 2/16/18 9:17 , Prakash Surya wrote:
> @rmustacc OK. So, if I'm understanding you correctly, you'd like us to take 
> the taskq implementation from libfakekernel, move it into a new userspace 
> library (e.g. libutaskq), and then make libfakekernel and libzfs consume this 
> new library?
> 

I think that both matches what you would have originally done and it'd
probably make more sense from a design perspective. But I also don't
want to be the one forcing everyone to do more busy work if I'm the only
one who thinks that.


-- 
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-366300189
--
openzfs-developer
Archives: 
https://openzfs.topicbox.com/groups/developer/discussions/T22334a01fda83bfd-M591f1afa34ce04814ed6e930
Powered by Topicbox: https://topicbox.com

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

2018-02-16 Thread Prakash Surya

@rmustacc OK. So, if I'm understanding you correctly, you'd like us to take the 
taskq implementation from libfakekernel, move it into a new userspace library 
(e.g. libutaskq), and then make libfakekernel and libzfs consume this new 
library?

-- 
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-366299443
--
openzfs-developer
Archives: 
https://openzfs.topicbox.com/groups/developer/discussions/T22334a01fda83bfd-M2e4a6aa0ce06d2c496b1e4ca
Powered by Topicbox: https://topicbox.com

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

@prakashsurya I do think libzfs in general being a consumer of a fake kernel
library is the wrong thing. I mean, wouldn't you say the same thing if it was
linked against libzpool? It feels like we're going to all these contortions to
emulate a kernel environment which we don't do for other code and things that
are shared between user land and the kernel. Even things like ctf or avl trees
have code in common that builds in either environment and glue in the user land
and the kernel.

Again, I realize I'm probably to blame for how we got here since you all first
proposed a generic taskq API that looked like the kernels, but basically made
it impossible for the non-libzfs panic on out of memory case. It would probably
have been better to have that and have the fake kernel bits rely on that same
thing, then the other way around. But everyone's already done the work in the
other direction, which is unfortunate, but reality. I'm not sure I can sign off
as being a reviewer on this.

I can understand that folks don't want to change, but then I would request that
you make it very clear to the RTI advocates that there are actual concerns with
this from an architectural perspective. I would bet that changes to the fake
kernel code and header games are going to impact this in the future.

--
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-366295158
--
openzfs-developer
Archives:
https://openzfs.topicbox.com/groups/developer/discussions/T22334a01fda83bfd-Mf29bf8801d27f805c6924e09
Powered by Topicbox: https://topicbox.com

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

rmustacc 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 */

Well, it's pretty easy to go tune down the max lwps you have in a zone or 
similar. Though hopefully in those situations you won't have a lot of datasets. 
It's a bit unfortunate that the model doesn't fall back to the known single 
threaded bit on resource exhaustion. Hopefully we won't have people hitting 
this substantially.

-- 
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_r168811668
--
openzfs-developer
Archives: 
https://openzfs.topicbox.com/groups/developer/discussions/T22334a01fda83bfd-Mf3149967fea5ff9ec5334302
Powered by Topicbox: https://topicbox.com

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

rmustacc 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);

OK, thanks for clarifying that. That's unfortunate, but life.

-- 
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_r168811449
--
openzfs-developer
Archives: 
https://openzfs.topicbox.com/groups/developer/discussions/T22334a01fda83bfd-M1988e7aba66b86622e5eaa0f
Powered by Topicbox: https://topicbox.com

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

2018-02-16 Thread Prakash Surya

@andy-js do you have any more questions/comments on this? can I count you as a 
reviewer?

-- 
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-366291880
--
openzfs-developer
Archives: 
https://openzfs.topicbox.com/groups/developer/discussions/T22334a01fda83bfd-M5f7c0480f7b226815a716d93
Powered by Topicbox: https://topicbox.com

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

2018-02-15 Thread Andrew Stormont

andy-js 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 */

I suppose this could be made a tuneable through /etc/default/zfs?

-- 
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_r168622752
--
openzfs-developer
Archives: 
https://openzfs.topicbox.com/groups/developer/discussions/T22334a01fda83bfd-M74b52f1243a85b4d74c05d94
Powered by Topicbox: https://topicbox.com

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

2018-02-15 Thread Andrew Stormont

andy-js commented on this pull request.



> -#ifdef   __cplusplus
-extern "C" {
-#endif
-
-#include 
-#include 
-#include 
-#include 
-#include 
-#include 
-
-#ifdef __cplusplus
-}
-#endif
-
-#endif /* _SYS_ZFS_CONTEXT_H */

I'm glad to see this file go.  Too bad we can't also drop the one in libzpool.

-- 
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#pullrequestreview-97022266
--
openzfs-developer
Archives: 
https://openzfs.topicbox.com/groups/developer/discussions/T22334a01fda83bfd-Ma6c9f448bf27adab2b3ca91f
Powered by Topicbox: https://topicbox.com

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

2018-02-15 Thread Prakash Surya

@rmustacc any more questions/comments on 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#issuecomment-366075230
--
openzfs-developer
Archives: 
https://openzfs.topicbox.com/groups/developer/discussions/T22334a01fda83bfd-Mdf2bda32348d901ceb348e3c
Powered by Topicbox: https://topicbox.com

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

2018-02-12 Thread Matthew Ahrens

ahrens 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);

>From looking at the code, if libzfs_printerr is set then libzfs will call 
>exit() on memory allocation failure.  This is not set by default, but it is 
>set by the zfs, zpool, zinject, libshare, dumpadm, and swap commands.  So for 
>the common ways of hitting this code ("zfs mount", "zfs share", "zpool 
>import"), libzfs_printerr will be set, indicating that the desired behavior is 
>to exit.  The only caller of this code that might not have libzfs_printerr set 
>is syseventd (it does this when a pool goes from being FAULTED (or worse) to 
>DEGRADED or HEALTHY).

In practice, it looks like lots of (but not all) callers are assuming that we 
will die if allocation fails (see callers of no_memory(), like zfs_alloc() and 
the comments above them - the assume that libzfs_printerr is always set so we 
always exit on failure).

So I'd say this is in keeping with the current behavior of handling allocation 
failures poorly :-( 

-- 
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_r167727390
--
openzfs-developer
Archives: 
https://openzfs.topicbox.com/groups/developer/discussions/T22334a01fda83bfd-Ma2fd472957defdae8fc6d719
Powered by Topicbox: https://topicbox.com

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

2018-02-12 Thread 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 */

I talked to Seb about this today, and it's like I was assuming.

The number 512 here isn't necessarily special (we could change it), we just
want a number large enough such that we can create enough threads to saturate
the storage IOPs. Since each thread performing a mount will generally be IO
bound, we want to have enough threads mounting concurrently, so that we more
effectively utilize the storage IOPs.

As far as CPU caps and zone considerations, I _think_ 512 should be fine. The
performance consideration here is to make sure we issue enough IO, so we still
want to have multiple threads issues mounts concurrently, even if we can only
have a single thread running at a time. i.e. we want to be able to issue
another mount, when the previous thread/mount blocks on IO.

With that said, I'm not familiar with the CPU caps available, so perhaps I'm
overlooking something. Let me know if you'd like some more information, and/or
if you still have concerns 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/536#discussion_r167648019
--
openzfs-developer
Archives:
https://openzfs.topicbox.com/groups/developer/discussions/T22334a01fda83bfd-Ma5151a88b3323f09b4774cf3
Powered by Topicbox: https://topicbox.com

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

@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)

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)

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] 8115 parallel zfs mount (v2) (#536)

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)

@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)

2018-02-10 Thread 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)

2018-02-10 Thread 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] 8115 parallel zfs mount (v2) (#536)

2018-02-09 Thread Prakash Surya

@andy-js @rmustacc is this good to RTI? it's passing the build, zloop, and 
zfstest.

-- 
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-364567269
--
openzfs-developer
Archives: 
https://openzfs.topicbox.com/groups/developer/discussions/T22334a01fda83bfd-M3ede8a76504f85515a63f75e
Powered by Topicbox: https://topicbox.com

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

2018-02-09 Thread Prakash Surya

On Fri, Feb 9, 2018 at 7:33 AM, Schweiss, Chip  wrote:

> I've been following this review for a while.   Sorry, if this is not the
> correct place for this comment.
>
> Personally, I've been doing similar outside zpool import by doing a 'zpool
> import -N' then parallel executing 'zfs mount {folder}' followed by
> parallel executing 'zfs share {folder}'.  My zpool import times were over
> an hour and reduced to less than 2 minutes with parallel mounting and
> sharing.
>
> I'm sure this proposed parallel mount will outperform the multiple
> executions of 'zfs mount', but doesn't help with the subsequent sharing of
> folders. Currently, zpool import has '-N' to not mount folders.  However,
> with the addition of this parallel mount, it would be best if zpool import
> got a new feature to not share any folders upon importing.
>

Could you elaborate a little more on why you think that feature should be
a prerequisite of landing this change?


> Does this patch address unmount in a parallel fashion?
>

No. This only changes mount.


>
> Cheers!
> -Chip
>
> On Thu, Feb 8, 2018 at 2:02 PM, Prakash Surya 
> wrote:
>
>> @prakashsurya  pushed 1 commit.
>>
>>- c8a3e65  Fix
>>cstyle; use tab for indentation, not spaces
>>
>> —
>> You are receiving this because you are subscribed to this thread.
>> View it on GitHub
>> 
>> or mute the thread
>> 
>> .
>>
>>
> *openzfs-developer* | Archives
> 
> | Powered by Topicbox 

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

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

2018-02-09 Thread Schweiss, Chip

I've been following this review for a while.   Sorry, if this is not the
correct place for this comment.

Personally, I've been doing similar outside zpool import by doing a 'zpool
import -N' then parallel executing 'zfs mount {folder}' followed by
parallel executing 'zfs share {folder}'.  My zpool import times were over
an hour and reduced to less than 2 minutes with parallel mounting and
sharing.

I'm sure this proposed parallel mount will outperform the multiple
executions of 'zfs mount', but doesn't help with the subsequent sharing of
folders. Currently, zpool import has '-N' to not mount folders.  However,
with the addition of this parallel mount, it would be best if zpool import
got a new feature to not share any folders upon importing.

Does this patch address unmount in a parallel fashion?

Cheers!
-Chip

On Thu, Feb 8, 2018 at 2:02 PM, Prakash Surya 
wrote:

> @prakashsurya  pushed 1 commit.
>
>- c8a3e65  Fix
>cstyle; use tab for indentation, not spaces
>
> —
> You are receiving this because you are subscribed to this thread.
> View it on GitHub
> 
> or mute the thread
> 
> .
> *openzfs-developer* | Archives
> 
> | Powered by Topicbox 
>
>

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

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

2018-02-08 Thread Prakash Surya

@prakashsurya pushed 1 commit.

c8a3e65  Fix cstyle; use tab for indentation, not spaces


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

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

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

2018-02-08 Thread Prakash Surya

@prakashsurya pushed 2 commits.

20d3e56  Missed a mutex_{enter,exit} pair, causing a crash
3a83672  Fix lint; add guards for _TASKQUSER in lgrp_user.h


-- 
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/openzfs/openzfs/pull/536/files/7b561cc953f07bae0a3486d53d9a50e9efc8edde..3a83672ecf670c528f05209997189ad8c22ce836

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

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

@andy-js @rmustacc let me know if my latest change is what y'all were thinking. 
I haven't tested this yet, beyond verifying it builds.

-- 
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-363959194
--
openzfs-developer
Archives: 
https://openzfs.topicbox.com/groups/developer/discussions/T22334a01fda83bfd-M8ad87085cf0c233616017994
Powered by Topicbox: https://topicbox.com

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

@prakashsurya pushed 1 commit.

7b561cc  Iterate based Rob's and Andrew's feedback


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

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

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

Thanks. I'll look into that, and push a new revision when I have something 
working.

-- 
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-363920916
--
openzfs-developer
Archives: 
https://openzfs.topicbox.com/groups/developer/discussions/T22334a01fda83bfd-M3102d6e96f55c4b12887f4ef
Powered by Topicbox: https://topicbox.com

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

2018-02-07 Thread Andrew Stormont

I think @rmustacc is exactly right.  We should not be building the zfs command 
(or libzfs) in fakekernel context.  This is a highly specialised environment 
that really only makes sense for things like libzpool which include kernel code.

I suggest you have a look at this earlier webrev of mine which introduces 
_TASKQUSER which can be used to access the taskq API outside if fakekernel 
context.  It was modelled after the _KMEMUSER define which does something 
similar:

http://cr.illumos.org/~webrev/andy_js/8115-1/

-- 
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-363908847
--
openzfs-developer
Archives: 
https://openzfs.topicbox.com/groups/developer/discussions/T22334a01fda83bfd-Mf7f1a0daae724f19b46654e3
Powered by Topicbox: https://topicbox.com

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