rmustacc commented on this pull request.
> @@ -416,6 +416,7 @@ enum {
#define VSW_XID 0x40 /* file system supports extended ids */
#define VSW_CANLOFI 0x80 /* file system supports lofi mounts */
#define VSW_ZMOUNT 0x100 /* file system always allowed in a zone
*/
+#define VSW_MOUNTDEV 0x200 /* 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(&share_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(&hdl->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(&hdl->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(&hdl->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(&hdl->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(&hdl->libzfs_mnttab_cache, mtn);
+ (void) mutex_lock(&hdl->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(&hdl->libzfs_mnttab_cache, mtn);
+ (void) mutex_lock(&hdl->libzfs_mnttab_cache_lock);
+ if (avl_numnodes(&hdl->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(&hdl->libzfs_mnttab_cache, mtn);
+ }
+ (void) mutex_unlock(&hdl->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(&hdl->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(&hdl->libzfs_mnttab_cache_lock);
Same mutex comments.
> @@ -88,6 +89,9 @@
#include <sys/systeminfo.h>
#define MAXISALEN 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 mount a lot of things and you have CPU caps that limit
the amount of parallelism?
> + */
+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);
In this case if we run low on memory this means we'll abort, right? Is that
what we want?
> @@ -150,7 +151,7 @@ static vfsdef_t vfw = {
"hsfs",
hsfsinit,
/* We don't suppport remounting */
- VSW_HASPROTO|VSW_STATS|VSW_CANLOFI,
+ VSW_HASPROTO|VSW_STATS|VSW_CANLOFI|VSW_MOUNTDEV,
Here and all the others, aren't these supposed to be pulled out?
--
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-95628091
------------------------------------------
openzfs-developer
Archives:
https://openzfs.topicbox.com/groups/developer/discussions/T22334a01fda83bfd-M8e229549658a3dee38c18959
Powered by Topicbox: https://topicbox.com
