Mercurial > illumos > git > illumos-gate

changeset 19270:1fc67d6c12d1

Find changesets by keywords (author, files, the commit message), revision number or hash, or revset expression.
11945 pool import performance regression due to repeated libshare initialization Reviewed by: Sanjay Nadkarni <sanjay.nadkarni@nexenta.com> Reviewed by: Evan Layton <evan.layton@nexenta.com> Reviewed by: Dan McDonald <danmcd@joyent.com> Reviewed by: Jason King <jason.brian.king@gmail.com> Approved by: Garrett D'Amore <garrett@damore.org>
author Joyce McIntosh <joyce.mcintosh@nexenta.com>
date Thu, 01 Mar 2018 10:18:07 -0800
parents e1e56202a7e4
children 70ce0aa98662
files usr/src/cmd/zfs/zfs_main.c usr/src/lib/libshare/common/libshare.c usr/src/lib/libshare/common/libshare.h usr/src/lib/libshare/common/mapfile-vers usr/src/lib/libzfs/common/libzfs_mount.c
diffstat 5 files changed, 81 insertions(+), 14 deletions(-) [+]
line wrap: on
line diff
--- a/usr/src/cmd/zfs/zfs_main.c	Thu Nov 22 12:22:30 2018 +0200
+++ b/usr/src/cmd/zfs/zfs_main.c	Thu Mar 01 10:18:07 2018 -0800
@@ -6849,6 +6849,30 @@
 		}
 
 		/*
+		 * Initialize libshare SA_INIT_SHARE_API_SELECTIVE here
+		 * to avoid unnecessary load/unload of the libshare API
+		 * per shared dataset downstream.
+		 */
+		if (op == OP_SHARE) {
+			get_all_cb_t dslist = { 0 };
+			get_all_datasets(&dslist, B_FALSE);
+
+			if (dslist.cb_used != 0) {
+				sa_init_selective_arg_t sharearg;
+				sharearg.zhandle_arr = dslist.cb_handles;
+				sharearg.zhandle_len = dslist.cb_used;
+				if ((ret = zfs_init_libshare_arg(g_zfs,
+				    SA_INIT_SHARE_API_SELECTIVE, &sharearg)) !=
+				    SA_OK) {
+					(void) fprintf(stderr, gettext(
+					    "Could not initialize libshare, "
+					    "%d"), ret);
+					return (1);
+				}
+			}
+		}
+
+		/*
 		 * Walk the AVL tree in reverse, unmounting each filesystem and
 		 * removing it from the AVL tree in the process.
 		 */
--- a/usr/src/lib/libshare/common/libshare.c	Thu Nov 22 12:22:30 2018 +0200
+++ b/usr/src/lib/libshare/common/libshare.c	Thu Mar 01 10:18:07 2018 -0800
@@ -21,7 +21,7 @@
 
 /*
  * Copyright (c) 2006, 2010, Oracle and/or its affiliates. All rights reserved.
- * Copyright 2014 Nexenta Systems, Inc.  All rights reserved.
+ * Copyright 2018 Nexenta Systems, Inc.  All rights reserved.
  * Copyright (c) 2016 by Delphix. All rights reserved.
  */
 
@@ -1153,6 +1153,20 @@
 }
 
 /*
+ * sa_service(sa_handle_t handle)
+ *
+ * Returns the service for which the handle is currently initialized.
+ */
+int
+sa_service(sa_handle_t handle)
+{
+	if (handle == NULL)
+		return (0);
+
+	return (((sa_handle_impl_t)handle)->sa_service);
+}
+
+/*
  * sa_get_protocols(char **protocol)
  *	Get array of protocols that are supported
  *	Returns pointer to an allocated and NULL terminated
--- a/usr/src/lib/libshare/common/libshare.h	Thu Nov 22 12:22:30 2018 +0200
+++ b/usr/src/lib/libshare/common/libshare.h	Thu Mar 01 10:18:07 2018 -0800
@@ -21,7 +21,7 @@
 
 /*
  * Copyright (c) 2006, 2010, Oracle and/or its affiliates. All rights reserved.
- * Copyright 2015 Nexenta Systems, Inc.  All rights reserved.
+ * Copyright 2018 Nexenta Systems, Inc.  All rights reserved.
  * Copyright (c) 2016 by Delphix. All rights reserved.
  */
 
@@ -171,6 +171,8 @@
 extern sa_handle_t sa_init(int);
 extern sa_handle_t sa_init_arg(int, void *);
 extern void sa_fini(sa_handle_t);
+extern int sa_service(sa_handle_t);
+
 extern int sa_update_config(sa_handle_t);
 extern boolean_t sa_needs_refresh(sa_handle_t);
 extern char *sa_errorstr(int);
--- a/usr/src/lib/libshare/common/mapfile-vers	Thu Nov 22 12:22:30 2018 +0200
+++ b/usr/src/lib/libshare/common/mapfile-vers	Thu Mar 01 10:18:07 2018 -0800
@@ -21,6 +21,7 @@
 #
 # Copyright (c) 2006, 2010, Oracle and/or its affiliates. All rights reserved.
 # Copyright (c) 2016 by Delphix. All rights reserved.
+# Copyright 2018 Nexenta Systems, Inc.  All rights reserved.
 #
 
 #
@@ -114,6 +115,7 @@
 	sa_get_protocol_status;
 	sa_init;
 	sa_init_arg;
+	sa_service;
 	sa_find_share;
 	sa_set_protocol_property;
 	sa_fini;
--- a/usr/src/lib/libzfs/common/libzfs_mount.c	Thu Nov 22 12:22:30 2018 +0200
+++ b/usr/src/lib/libzfs/common/libzfs_mount.c	Thu Mar 01 10:18:07 2018 -0800
@@ -612,6 +612,7 @@
 
 static sa_handle_t (*_sa_init)(int);
 static sa_handle_t (*_sa_init_arg)(int, void *);
+static int (*_sa_service)(sa_handle_t);
 static void (*_sa_fini)(sa_handle_t);
 static sa_share_t (*_sa_find_share)(sa_handle_t, char *);
 static int (*_sa_enable_share)(sa_share_t, char *);
@@ -654,6 +655,8 @@
 		_sa_init_arg = (sa_handle_t (*)(int, void *))dlsym(libshare,
 		    "sa_init_arg");
 		_sa_fini = (void (*)(sa_handle_t))dlsym(libshare, "sa_fini");
+		_sa_service = (int (*)(sa_handle_t))dlsym(libshare,
+		    "sa_service");
 		_sa_find_share = (sa_share_t (*)(sa_handle_t, char *))
 		    dlsym(libshare, "sa_find_share");
 		_sa_enable_share = (int (*)(sa_share_t, char *))dlsym(libshare,
@@ -677,10 +680,11 @@
 		    _sa_enable_share == NULL || _sa_disable_share == NULL ||
 		    _sa_errorstr == NULL || _sa_parse_legacy_options == NULL ||
 		    _sa_needs_refresh == NULL || _sa_get_zfs_handle == NULL ||
-		    _sa_zfs_process_share == NULL ||
+		    _sa_zfs_process_share == NULL || _sa_service == NULL ||
 		    _sa_update_sharetab_ts == NULL) {
 			_sa_init = NULL;
 			_sa_init_arg = NULL;
+			_sa_service = NULL;
 			_sa_fini = NULL;
 			_sa_disable_share = NULL;
 			_sa_enable_share = NULL;
@@ -839,11 +843,25 @@
 	sa_share_t share;
 	zfs_share_proto_t *curr_proto;
 	zprop_source_t sourcetype;
+	int service = SA_INIT_ONE_SHARE_FROM_HANDLE;
 	int ret;
 
 	if (!zfs_is_mountable(zhp, mountpoint, sizeof (mountpoint), NULL))
 		return (0);
 
+	/*
+	 * Function may be called in a loop from higher up stack, with libshare
+	 * initialized for multiple shares (SA_INIT_SHARE_API_SELECTIVE).
+	 * zfs_init_libshare_arg will refresh the handle's cache if necessary.
+	 * In this case we do not want to switch to per share initialization.
+	 * Specify SA_INIT_SHARE_API to do full refresh, if refresh required.
+	 */
+	if ((hdl->libzfs_sharehdl != NULL) && (_sa_service != NULL) &&
+	    (_sa_service(hdl->libzfs_sharehdl) ==
+	    SA_INIT_SHARE_API_SELECTIVE)) {
+		service = SA_INIT_SHARE_API;
+	}
+
 	for (curr_proto = proto; *curr_proto != PROTO_END; curr_proto++) {
 		/*
 		 * Return success if there are no share options.
@@ -853,8 +871,7 @@
 		    ZFS_MAXPROPLEN, B_FALSE) != 0 ||
 		    strcmp(shareopts, "off") == 0)
 			continue;
-		ret = zfs_init_libshare_arg(hdl, SA_INIT_ONE_SHARE_FROM_HANDLE,
-		    zhp);
+		ret = zfs_init_libshare_arg(hdl, service, zhp);
 		if (ret != SA_OK) {
 			(void) zfs_error_fmt(hdl, EZFS_SHARENFSFAILED,
 			    dgettext(TEXT_DOMAIN, "cannot share '%s': %s"),
@@ -948,6 +965,7 @@
 	sa_share_t share;
 	int err;
 	char *mntpt;
+	int service = SA_INIT_ONE_SHARE_FROM_NAME;
 
 	/*
 	 * Mountpoint could get trashed if libshare calls getmntany
@@ -957,13 +975,20 @@
 	mntpt = zfs_strdup(hdl, mountpoint);
 
 	/*
-	 * make sure libshare initialized, initialize everything because we
-	 * don't know what other unsharing may happen later. Functions up the
-	 * stack are allowed to initialize instead a subset of shares at the
-	 * time the set is known.
+	 * Function may be called in a loop from higher up stack, with libshare
+	 * initialized for multiple shares (SA_INIT_SHARE_API_SELECTIVE).
+	 * zfs_init_libshare_arg will refresh the handle's cache if necessary.
+	 * In this case we do not want to switch to per share initialization.
+	 * Specify SA_INIT_SHARE_API to do full refresh, if refresh required.
 	 */
-	if ((err = zfs_init_libshare_arg(hdl, SA_INIT_ONE_SHARE_FROM_NAME,
-	    (void *)name)) != SA_OK) {
+	if ((hdl->libzfs_sharehdl != NULL) && (_sa_service != NULL) &&
+	    (_sa_service(hdl->libzfs_sharehdl) ==
+	    SA_INIT_SHARE_API_SELECTIVE)) {
+		service = SA_INIT_SHARE_API;
+	}
+
+	err = zfs_init_libshare_arg(hdl, service, (void *)name);
+	if (err != SA_OK) {
 		free(mntpt);	/* don't need the copy anymore */
 		return (zfs_error_fmt(hdl, proto_table[proto].p_unshare_err,
 		    dgettext(TEXT_DOMAIN, "cannot unshare '%s': %s"),
@@ -1532,9 +1557,9 @@
 		ret = ms.ms_mntstatus;
 
 	/*
-	 * Share all filesystems that need to be shared. This needs to be
-	 * a separate pass because libshare is not mt-safe, and so we need
-	 * to share serially.
+	 * Initialize libshare SA_INIT_SHARE_API_SELECTIVE here
+	 * to avoid unnecessary load/unload of the libshare API
+	 * per shared dataset downstream.
 	 */
 	sharearg.zhandle_arr = cb.cb_handles;
 	sharearg.zhandle_len = cb.cb_used;