Skip to content

Commit

Permalink
MFV r316926:
Browse files Browse the repository at this point in the history
7955 libshare needs to initialize only those datasets being modified by the consumer

illumos/illumos-gate@8a981c3
illumos/illumos-gate@8a981c3

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 <[email protected]>
Reviewed by: Sebastien Roy <[email protected]>
Reviewed by: Matthew Ahrens <[email protected]>
Reviewed by: George Wilson <[email protected]>
Reviewed by: Pavel Zakharov <[email protected]>
Reviewed by: Yuri Pankov <[email protected]>
Approved by: Gordon Ross <[email protected]>
Author: Daniel Hoffman <[email protected]>

For FreeBSD this is practically a NOP, just a diff reduction.
  • Loading branch information
amotin committed Aug 1, 2018
2 parents bba8f1f + 847b286 commit b59e9cd
Show file tree
Hide file tree
Showing 5 changed files with 103 additions and 15 deletions.
12 changes: 12 additions & 0 deletions cddl/contrib/opensolaris/cmd/zfs/zfs_main.c
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,7 @@
#include <aclutils.h>
#include <directory.h>
#include <idmap.h>
#include <libshare.h>
#endif

#include "zfs_iter.h"
Expand Down Expand Up @@ -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)
Expand Down
11 changes: 11 additions & 0 deletions cddl/contrib/opensolaris/lib/libzfs/common/libzfs.h
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
36 changes: 34 additions & 2 deletions cddl/contrib/opensolaris/lib/libzfs/common/libzfs_changelist.c
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@
* Portions Copyright 2007 Ramprakash Jelari
* Copyright (c) 2011 Pawel Jakub Dawidek <[email protected]>.
* 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 <[email protected]>
*/

Expand Down Expand Up @@ -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
Expand All @@ -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
Expand All @@ -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;
Expand Down
1 change: 0 additions & 1 deletion cddl/contrib/opensolaris/lib/libzfs/common/libzfs_impl.h
Original file line number Diff line number Diff line change
Expand Up @@ -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 *,
Expand Down
58 changes: 46 additions & 12 deletions cddl/contrib/opensolaris/lib/libzfs/common/libzfs_mount.c
Original file line number Diff line number Diff line change
Expand Up @@ -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 *);
Expand Down Expand Up @@ -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");
Expand All @@ -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;
Expand All @@ -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
/*
Expand All @@ -694,18 +698,30 @@ 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);
#endif

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)
Expand Down Expand Up @@ -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"),
Expand Down Expand Up @@ -930,15 +946,22 @@ 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
* value.
*/
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"),
Expand Down Expand Up @@ -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);

Expand Down Expand Up @@ -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);

/*
Expand Down

0 comments on commit b59e9cd

Please sign in to comment.