Date: Fri, 14 Apr 2017 18:34:03 +0000 (UTC) From: Andriy Gapon <avg@FreeBSD.org> To: src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-vendor@freebsd.org Subject: svn commit: r316926 - in vendor/illumos/dist: cmd/zfs lib/libzfs/common Message-ID: <201704141834.v3EIY3Ya075442@repo.freebsd.org>
next in thread | raw e-mail | index | archive | help
Author: avg Date: Fri Apr 14 18:34:03 2017 New Revision: 316926 URL: https://svnweb.freebsd.org/changeset/base/316926 Log: 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 <steve.gonczi@delphix.com> Reviewed by: Sebastien Roy <sebastien.roy@delphix.com> Reviewed by: Matthew Ahrens <mahrens@delphix.com> Reviewed by: George Wilson <george.wilson@delphix.com> Reviewed by: Pavel Zakharov <pavel.zakharov@delphix.com> Reviewed by: Yuri Pankov <yuri.pankov@gmail.com> Approved by: Gordon Ross <gordon.w.ross@gmail.com> Author: Daniel Hoffman <dj.hoffman@delphix.com> Modified: vendor/illumos/dist/cmd/zfs/zfs_main.c vendor/illumos/dist/lib/libzfs/common/libzfs.h vendor/illumos/dist/lib/libzfs/common/libzfs_changelist.c vendor/illumos/dist/lib/libzfs/common/libzfs_impl.h vendor/illumos/dist/lib/libzfs/common/libzfs_mount.c Modified: vendor/illumos/dist/cmd/zfs/zfs_main.c ============================================================================== --- vendor/illumos/dist/cmd/zfs/zfs_main.c Fri Apr 14 18:33:20 2017 (r316925) +++ vendor/illumos/dist/cmd/zfs/zfs_main.c Fri Apr 14 18:34:03 2017 (r316926) @@ -68,6 +68,7 @@ #include <aclutils.h> #include <directory.h> #include <idmap.h> +#include <libshare.h> #include "zfs_iter.h" #include "zfs_util.h" @@ -6111,6 +6112,15 @@ share_mount(int op, int argc, char **arg return (0); qsort(dslist, count, sizeof (void *), libzfs_dataset_cmp); + 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); + } for (i = 0; i < count; i++) { if (verbose) Modified: vendor/illumos/dist/lib/libzfs/common/libzfs.h ============================================================================== --- vendor/illumos/dist/lib/libzfs/common/libzfs.h Fri Apr 14 18:33:20 2017 (r316925) +++ vendor/illumos/dist/lib/libzfs/common/libzfs.h Fri Apr 14 18:34:03 2017 (r316926) @@ -783,6 +783,17 @@ extern int zpool_fru_set(zpool_handle_t extern int zfs_get_hole_count(const char *, uint64_t *, uint64_t *); +/* 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: vendor/illumos/dist/lib/libzfs/common/libzfs_changelist.c ============================================================================== --- vendor/illumos/dist/lib/libzfs/common/libzfs_changelist.c Fri Apr 14 18:33:20 2017 (r316925) +++ vendor/illumos/dist/lib/libzfs/common/libzfs_changelist.c Fri Apr 14 18:34:03 2017 (r316926) @@ -24,7 +24,7 @@ * Use is subject to license terms. * * Portions Copyright 2007 Ramprakash Jelari - * Copyright (c) 2014, 2015 by Delphix. All rights reserved. + * Copyright (c) 2014, 2016 by Delphix. All rights reserved. * Copyright 2016 Igor Kozhukhov <ikozhukhov@gmail.com> */ @@ -162,6 +162,9 @@ changelist_postfix(prop_changelist_t *cl char shareopts[ZFS_MAXPROPLEN]; int errors = 0; libzfs_handle_t *hdl; + size_t num_datasets = 0, i; + zfs_handle_t **zhandle_arr; + sa_init_selective_arg_t sharearg; /* * If we're changing the mountpoint, attempt to destroy the underlying @@ -186,8 +189,31 @@ changelist_postfix(prop_changelist_t *cl hdl = cn->cn_handle->zfs_hdl; assert(hdl != NULL); zfs_uninit_libshare(hdl); - } + /* + * 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); + } /* * We walk the datasets in reverse, because we want to mount any parent * datasets before mounting the children. We walk all datasets even if @@ -212,8 +238,6 @@ changelist_postfix(prop_changelist_t *cl continue; cn->cn_needpost = B_FALSE; - zfs_refresh_properties(cn->cn_handle); - if (ZFS_IS_VOLUME(cn->cn_handle)) continue; Modified: vendor/illumos/dist/lib/libzfs/common/libzfs_impl.h ============================================================================== --- vendor/illumos/dist/lib/libzfs/common/libzfs_impl.h Fri Apr 14 18:33:20 2017 (r316925) +++ vendor/illumos/dist/lib/libzfs/common/libzfs_impl.h Fri Apr 14 18:34:03 2017 (r316926) @@ -203,7 +203,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: vendor/illumos/dist/lib/libzfs/common/libzfs_mount.c ============================================================================== --- vendor/illumos/dist/lib/libzfs/common/libzfs_mount.c Fri Apr 14 18:33:20 2017 (r316925) +++ vendor/illumos/dist/lib/libzfs/common/libzfs_mount.c Fri Apr 14 18:34:03 2017 (r316926) @@ -566,6 +566,7 @@ zfs_is_shared_smb(zfs_handle_t *zhp, cha */ 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 *); @@ -605,6 +606,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"); @@ -624,14 +627,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; @@ -654,8 +658,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) { if (_sa_init == NULL) return (SA_CONFIG_ERR); @@ -671,17 +675,29 @@ zfs_init_libshare(libzfs_handle_t *zhand 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); 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) @@ -786,8 +802,8 @@ zfs_share_proto(zfs_handle_t *zhp, zfs_s ZFS_MAXPROPLEN, B_FALSE) != 0 || strcmp(shareopts, "off") == 0) continue; - - 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"), @@ -881,6 +897,7 @@ unshare_one(libzfs_handle_t *hdl, const 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 @@ -888,8 +905,14 @@ unshare_one(libzfs_handle_t *hdl, const */ 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, EZFS_SHARENFSFAILED, dgettext(TEXT_DOMAIN, "cannot unshare '%s': %s"), @@ -1221,6 +1244,7 @@ zpool_disable_datasets(zpool_handle_t *z int i; int ret = -1; int flags = (force ? MS_FORCE : 0); + sa_init_selective_arg_t sharearg; namelen = strlen(zhp->zpool_name); @@ -1295,6 +1319,12 @@ zpool_disable_datasets(zpool_handle_t *z * At this point, we have the entire list of filesystems, so sort it by * mountpoint. */ + 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; qsort(mountpoints, used, sizeof (char *), mountpoint_compare); /*
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?201704141834.v3EIY3Ya075442>