From owner-svn-src-stable@freebsd.org Wed Oct 3 02:51:15 2018 Return-Path: Delivered-To: svn-src-stable@mailman.ysv.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2610:1c1:1:606c::19:1]) by mailman.ysv.freebsd.org (Postfix) with ESMTP id 22ADD10B5A78; Wed, 3 Oct 2018 02:51:15 +0000 (UTC) (envelope-from mav@FreeBSD.org) Received: from mxrelay.nyi.freebsd.org (mxrelay.nyi.freebsd.org [IPv6:2610:1c1:1:606c::19:3]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (Client CN "mxrelay.nyi.freebsd.org", Issuer "Let's Encrypt Authority X3" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id CCFD5888E3; Wed, 3 Oct 2018 02:51:14 +0000 (UTC) (envelope-from mav@FreeBSD.org) Received: from repo.freebsd.org (repo.freebsd.org [IPv6:2610:1c1:1:6068::e6a:0]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (Client did not present a certificate) by mxrelay.nyi.freebsd.org (Postfix) with ESMTPS id C7E4810EA8; Wed, 3 Oct 2018 02:51:14 +0000 (UTC) (envelope-from mav@FreeBSD.org) Received: from repo.freebsd.org ([127.0.1.37]) by repo.freebsd.org (8.15.2/8.15.2) with ESMTP id w932pEOp062593; Wed, 3 Oct 2018 02:51:14 GMT (envelope-from mav@FreeBSD.org) Received: (from mav@localhost) by repo.freebsd.org (8.15.2/8.15.2/Submit) id w932pDdo062588; Wed, 3 Oct 2018 02:51:13 GMT (envelope-from mav@FreeBSD.org) Message-Id: <201810030251.w932pDdo062588@repo.freebsd.org> X-Authentication-Warning: repo.freebsd.org: mav set sender to mav@FreeBSD.org using -f From: Alexander Motin Date: Wed, 3 Oct 2018 02:51:13 +0000 (UTC) To: src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-stable@freebsd.org, svn-src-stable-11@freebsd.org Subject: svn commit: r339117 - in stable/11/cddl/contrib/opensolaris: cmd/zfs lib/libzfs/common X-SVN-Group: stable-11 X-SVN-Commit-Author: mav X-SVN-Commit-Paths: in stable/11/cddl/contrib/opensolaris: cmd/zfs lib/libzfs/common X-SVN-Commit-Revision: 339117 X-SVN-Commit-Repository: base MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit X-BeenThere: svn-src-stable@freebsd.org X-Mailman-Version: 2.1.27 Precedence: list List-Id: SVN commit messages for all the -stable branches of the src tree List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 03 Oct 2018 02:51:15 -0000 Author: mav Date: Wed Oct 3 02:51:13 2018 New Revision: 339117 URL: https://svnweb.freebsd.org/changeset/base/339117 Log: MFC r337063: MFV r316926: 7955 libshare needs to initialize only those datasets being modified by the consumer illumos/illumos-gate@8a981c3356b194b3b5c0ae9276a9cc31cd2f93a3 https://github.com/illumos/illumos-gate/commit/8a981c3356b194b3b5c0ae9276a9cc31cd2f93a3 https://www.illumos.org/issues/7955 Libshare currently initializes all available filesystems when doing any libshare operation. This requires iterating through all the filesystem multiple times, which is a huge performance problem for sharing and unsharing operations. Reviewed by: Steve Gonczi Reviewed by: Sebastien Roy Reviewed by: Matthew Ahrens Reviewed by: George Wilson Reviewed by: Pavel Zakharov Reviewed by: Yuri Pankov Approved by: Gordon Ross Author: Daniel Hoffman For FreeBSD this is practically a NOP, just a diff reduction. Modified: stable/11/cddl/contrib/opensolaris/cmd/zfs/zfs_main.c stable/11/cddl/contrib/opensolaris/lib/libzfs/common/libzfs.h stable/11/cddl/contrib/opensolaris/lib/libzfs/common/libzfs_changelist.c stable/11/cddl/contrib/opensolaris/lib/libzfs/common/libzfs_impl.h stable/11/cddl/contrib/opensolaris/lib/libzfs/common/libzfs_mount.c Directory Properties: stable/11/ (props changed) Modified: stable/11/cddl/contrib/opensolaris/cmd/zfs/zfs_main.c ============================================================================== --- stable/11/cddl/contrib/opensolaris/cmd/zfs/zfs_main.c Wed Oct 3 02:50:07 2018 (r339116) +++ stable/11/cddl/contrib/opensolaris/cmd/zfs/zfs_main.c Wed Oct 3 02:51:13 2018 (r339117) @@ -72,6 +72,7 @@ #include #include #include +#include #endif #include "zfs_iter.h" @@ -6221,6 +6222,17 @@ share_mount(int op, int argc, char **argv) return (0); qsort(dslist, count, sizeof (void *), libzfs_dataset_cmp); +#ifdef illumos + sa_init_selective_arg_t sharearg; + sharearg.zhandle_arr = dslist; + sharearg.zhandle_len = count; + if ((ret = zfs_init_libshare_arg(zfs_get_handle(dslist[0]), + SA_INIT_SHARE_API_SELECTIVE, &sharearg)) != SA_OK) { + (void) fprintf(stderr, + gettext("Could not initialize libshare, %d"), ret); + return (ret); + } +#endif for (i = 0; i < count; i++) { if (verbose) Modified: stable/11/cddl/contrib/opensolaris/lib/libzfs/common/libzfs.h ============================================================================== --- stable/11/cddl/contrib/opensolaris/lib/libzfs/common/libzfs.h Wed Oct 3 02:50:07 2018 (r339116) +++ stable/11/cddl/contrib/opensolaris/lib/libzfs/common/libzfs.h Wed Oct 3 02:51:13 2018 (r339117) @@ -843,6 +843,17 @@ extern int zmount(const char *, const char *, int, cha #endif extern int zfs_remap_indirects(libzfs_handle_t *hdl, const char *); +/* Allow consumers to initialize libshare externally for optimal performance */ +extern int zfs_init_libshare_arg(libzfs_handle_t *, int, void *); +/* + * For most consumers, zfs_init_libshare_arg is sufficient on its own, and + * zfs_uninit_libshare is unnecessary. zfs_uninit_libshare should only be called + * if the caller has already initialized libshare for one set of zfs handles, + * and wishes to share or unshare filesystems outside of that set. In that case, + * the caller should uninitialize libshare, and then re-initialize it with the + * new handles being shared or unshared. + */ +extern void zfs_uninit_libshare(libzfs_handle_t *); #ifdef __cplusplus } #endif Modified: stable/11/cddl/contrib/opensolaris/lib/libzfs/common/libzfs_changelist.c ============================================================================== --- stable/11/cddl/contrib/opensolaris/lib/libzfs/common/libzfs_changelist.c Wed Oct 3 02:50:07 2018 (r339116) +++ stable/11/cddl/contrib/opensolaris/lib/libzfs/common/libzfs_changelist.c Wed Oct 3 02:51:13 2018 (r339117) @@ -26,7 +26,7 @@ * Portions Copyright 2007 Ramprakash Jelari * Copyright (c) 2011 Pawel Jakub Dawidek . * All rights reserved. - * Copyright (c) 2014, 2015 by Delphix. All rights reserved. + * Copyright (c) 2014, 2016 by Delphix. All rights reserved. * Copyright 2016 Igor Kozhukhov */ @@ -166,6 +166,11 @@ changelist_postfix(prop_changelist_t *clp) char shareopts[ZFS_MAXPROPLEN]; int errors = 0; libzfs_handle_t *hdl; +#ifdef illumos + size_t num_datasets = 0, i; + zfs_handle_t **zhandle_arr; + sa_init_selective_arg_t sharearg; +#endif /* * If we're changing the mountpoint, attempt to destroy the underlying @@ -192,8 +197,33 @@ changelist_postfix(prop_changelist_t *clp) hdl = cn->cn_handle->zfs_hdl; assert(hdl != NULL); zfs_uninit_libshare(hdl); - } +#ifdef illumos + /* + * For efficiencies sake, we initialize libshare for only a few + * shares (the ones affected here). Future initializations in + * this process should just use the cached initialization. + */ + for (cn = uu_list_last(clp->cl_list); cn != NULL; + cn = uu_list_prev(clp->cl_list, cn)) { + num_datasets++; + } + + zhandle_arr = zfs_alloc(hdl, + num_datasets * sizeof (zfs_handle_t *)); + for (i = 0, cn = uu_list_last(clp->cl_list); cn != NULL; + cn = uu_list_prev(clp->cl_list, cn)) { + zhandle_arr[i++] = cn->cn_handle; + zfs_refresh_properties(cn->cn_handle); + } + assert(i == num_datasets); + sharearg.zhandle_arr = zhandle_arr; + sharearg.zhandle_len = num_datasets; + errors = zfs_init_libshare_arg(hdl, SA_INIT_SHARE_API_SELECTIVE, + &sharearg); + free(zhandle_arr); +#endif + } /* * We walk the datasets in reverse, because we want to mount any parent * datasets before mounting the children. We walk all datasets even if @@ -218,7 +248,9 @@ changelist_postfix(prop_changelist_t *clp) continue; cn->cn_needpost = B_FALSE; +#ifndef illumos zfs_refresh_properties(cn->cn_handle); +#endif if (ZFS_IS_VOLUME(cn->cn_handle)) continue; Modified: stable/11/cddl/contrib/opensolaris/lib/libzfs/common/libzfs_impl.h ============================================================================== --- stable/11/cddl/contrib/opensolaris/lib/libzfs/common/libzfs_impl.h Wed Oct 3 02:50:07 2018 (r339116) +++ stable/11/cddl/contrib/opensolaris/lib/libzfs/common/libzfs_impl.h Wed Oct 3 02:51:13 2018 (r339117) @@ -207,7 +207,6 @@ void namespace_clear(libzfs_handle_t *); */ extern int zfs_init_libshare(libzfs_handle_t *, int); -extern void zfs_uninit_libshare(libzfs_handle_t *); extern int zfs_parse_options(char *, zfs_share_proto_t); extern int zfs_unshare_proto(zfs_handle_t *, Modified: stable/11/cddl/contrib/opensolaris/lib/libzfs/common/libzfs_mount.c ============================================================================== --- stable/11/cddl/contrib/opensolaris/lib/libzfs/common/libzfs_mount.c Wed Oct 3 02:50:07 2018 (r339116) +++ stable/11/cddl/contrib/opensolaris/lib/libzfs/common/libzfs_mount.c Wed Oct 3 02:51:13 2018 (r339117) @@ -579,6 +579,7 @@ zfs_is_shared_smb(zfs_handle_t *zhp, char **where) #ifdef illumos static sa_handle_t (*_sa_init)(int); +static sa_handle_t (*_sa_init_arg)(int, void *); 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 *); @@ -620,6 +621,8 @@ _zfs_init_libshare(void) if ((libshare = dlopen(path, RTLD_LAZY | RTLD_GLOBAL)) != NULL) { _sa_init = (sa_handle_t (*)(int))dlsym(libshare, "sa_init"); + _sa_init_arg = (sa_handle_t (*)(int, void *))dlsym(libshare, + "sa_init_arg"); _sa_fini = (void (*)(sa_handle_t))dlsym(libshare, "sa_fini"); _sa_find_share = (sa_share_t (*)(sa_handle_t, char *)) dlsym(libshare, "sa_find_share"); @@ -639,14 +642,15 @@ _zfs_init_libshare(void) char *, char *))dlsym(libshare, "sa_zfs_process_share"); _sa_update_sharetab_ts = (void (*)(sa_handle_t)) dlsym(libshare, "sa_update_sharetab_ts"); - if (_sa_init == NULL || _sa_fini == NULL || - _sa_find_share == NULL || _sa_enable_share == NULL || - _sa_disable_share == NULL || _sa_errorstr == NULL || - _sa_parse_legacy_options == NULL || + if (_sa_init == NULL || _sa_init_arg == NULL || + _sa_fini == NULL || _sa_find_share == NULL || + _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_update_sharetab_ts == NULL) { _sa_init = NULL; + _sa_init_arg = NULL; _sa_fini = NULL; _sa_disable_share = NULL; _sa_enable_share = NULL; @@ -670,8 +674,8 @@ _zfs_init_libshare(void) * service value is which part(s) of the API to initialize and is a * direct map to the libshare sa_init(service) interface. */ -int -zfs_init_libshare(libzfs_handle_t *zhandle, int service) +static int +zfs_init_libshare_impl(libzfs_handle_t *zhandle, int service, void *arg) { #ifdef illumos /* @@ -694,11 +698,11 @@ zfs_init_libshare(libzfs_handle_t *zhandle, int servic if (_sa_needs_refresh != NULL && _sa_needs_refresh(zhandle->libzfs_sharehdl)) { zfs_uninit_libshare(zhandle); - zhandle->libzfs_sharehdl = _sa_init(service); + zhandle->libzfs_sharehdl = _sa_init_arg(service, arg); } if (zhandle && zhandle->libzfs_sharehdl == NULL) - zhandle->libzfs_sharehdl = _sa_init(service); + zhandle->libzfs_sharehdl = _sa_init_arg(service, arg); if (zhandle->libzfs_sharehdl == NULL) return (SA_NO_MEMORY); @@ -706,7 +710,19 @@ zfs_init_libshare(libzfs_handle_t *zhandle, int servic return (SA_OK); } +int +zfs_init_libshare(libzfs_handle_t *zhandle, int service) +{ + return (zfs_init_libshare_impl(zhandle, service, NULL)); +} +int +zfs_init_libshare_arg(libzfs_handle_t *zhandle, int service, void *arg) +{ + return (zfs_init_libshare_impl(zhandle, service, arg)); +} + + /* * zfs_uninit_libshare(zhandle) * @@ -817,9 +833,9 @@ zfs_share_proto(zfs_handle_t *zhp, zfs_share_proto_t * ZFS_MAXPROPLEN, B_FALSE) != 0 || strcmp(shareopts, "off") == 0) continue; - #ifdef illumos - ret = zfs_init_libshare(hdl, SA_INIT_SHARE_API); + ret = zfs_init_libshare_arg(hdl, SA_INIT_ONE_SHARE_FROM_HANDLE, + zhp); if (ret != SA_OK) { (void) zfs_error_fmt(hdl, EZFS_SHARENFSFAILED, dgettext(TEXT_DOMAIN, "cannot share '%s': %s"), @@ -930,6 +946,7 @@ unshare_one(libzfs_handle_t *hdl, const char *name, co sa_share_t share; int err; char *mntpt; + /* * Mountpoint could get trashed if libshare calls getmntany * which it does during API initialization, so strdup the @@ -937,8 +954,14 @@ unshare_one(libzfs_handle_t *hdl, const char *name, co */ mntpt = zfs_strdup(hdl, mountpoint); - /* make sure libshare initialized */ - if ((err = zfs_init_libshare(hdl, SA_INIT_SHARE_API)) != SA_OK) { + /* + * 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. + */ + if ((err = zfs_init_libshare_arg(hdl, SA_INIT_ONE_SHARE_FROM_NAME, + (void *)name)) != 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"), @@ -1289,6 +1312,9 @@ zpool_disable_datasets(zpool_handle_t *zhp, boolean_t int i; int ret = -1; int flags = (force ? MS_FORCE : 0); +#ifdef illumos + sa_init_selective_arg_t sharearg; +#endif namelen = strlen(zhp->zpool_name); @@ -1363,6 +1389,14 @@ zpool_disable_datasets(zpool_handle_t *zhp, boolean_t * At this point, we have the entire list of filesystems, so sort it by * mountpoint. */ +#ifdef illumos + sharearg.zhandle_arr = datasets; + sharearg.zhandle_len = used; + ret = zfs_init_libshare_arg(hdl, SA_INIT_SHARE_API_SELECTIVE, + &sharearg); + if (ret != 0) + goto out; +#endif qsort(mountpoints, used, sizeof (char *), mountpoint_compare); /*