MFV r316926:

7955 libshare needs to initialize only those datasets being modified by the consumer

illumos/illumos-gate@8a981c3356
8a981c3356

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>

For FreeBSD this is practically a NOP, just a diff reduction.
This commit is contained in:
mav 2018-08-01 21:51:49 +00:00
parent 74ed0d8f50
commit ef0cebe7e1
5 changed files with 103 additions and 15 deletions

View File

@ -72,6 +72,7 @@
#include <aclutils.h>
#include <directory.h>
#include <idmap.h>
#include <libshare.h>
#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)

View File

@ -843,6 +843,17 @@ extern int zmount(const char *, const char *, int, char *, char *, int, char *,
#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

View File

@ -26,7 +26,7 @@
* Portions Copyright 2007 Ramprakash Jelari
* Copyright (c) 2011 Pawel Jakub Dawidek <pawel@dawidek.net>.
* 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 <ikozhukhov@gmail.com>
*/
@ -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;

View File

@ -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 *,

View File

@ -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 service)
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,6 +710,18 @@ zfs_init_libshare(libzfs_handle_t *zhandle, int service)
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 *proto)
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, const char *mountpoint,
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, const char *mountpoint,
*/
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 force)
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 force)
* 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);
/*