Print this page
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>
        
*** 610,619 ****
--- 610,620 ----
   * initialized in _zfs_init_libshare() are actually present.
   */
  
  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 *);
  static int (*_sa_disable_share)(sa_share_t, char *);
  static char *(*_sa_errorstr)(int);
*** 652,661 ****
--- 653,664 ----
          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_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,
                      "sa_enable_share");
                  _sa_disable_share = (int (*)(sa_share_t, char *))dlsym(libshare,
*** 675,688 ****
                  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;
                          _sa_errorstr = NULL;
                          _sa_parse_legacy_options = NULL;
--- 678,692 ----
                  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_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;
                          _sa_errorstr = NULL;
                          _sa_parse_legacy_options = NULL;
*** 837,862 ****
          char sourcestr[ZFS_MAXPROPLEN];
          libzfs_handle_t *hdl = zhp->zfs_hdl;
          sa_share_t share;
          zfs_share_proto_t *curr_proto;
          zprop_source_t sourcetype;
          int ret;
  
          if (!zfs_is_mountable(zhp, mountpoint, sizeof (mountpoint), NULL))
                  return (0);
  
          for (curr_proto = proto; *curr_proto != PROTO_END; curr_proto++) {
                  /*
                   * Return success if there are no share options.
                   */
                  if (zfs_prop_get(zhp, proto_table[*curr_proto].p_prop,
                      shareopts, sizeof (shareopts), &sourcetype, sourcestr,
                      ZFS_MAXPROPLEN, B_FALSE) != 0 ||
                      strcmp(shareopts, "off") == 0)
                          continue;
!                 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"),
                              zfs_get_name(zhp), _sa_errorstr != NULL ?
                              _sa_errorstr(ret) : "");
--- 841,879 ----
          char sourcestr[ZFS_MAXPROPLEN];
          libzfs_handle_t *hdl = zhp->zfs_hdl;
          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.
                   */
                  if (zfs_prop_get(zhp, proto_table[*curr_proto].p_prop,
                      shareopts, sizeof (shareopts), &sourcetype, sourcestr,
                      ZFS_MAXPROPLEN, B_FALSE) != 0 ||
                      strcmp(shareopts, "off") == 0)
                          continue;
!                 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"),
                              zfs_get_name(zhp), _sa_errorstr != NULL ?
                              _sa_errorstr(ret) : "");
*** 946,971 ****
      zfs_share_proto_t proto)
  {
          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, 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"),
                      name, _sa_errorstr(err)));
          }
--- 963,996 ----
      zfs_share_proto_t proto)
  {
          sa_share_t share;
          int err;
          char *mntpt;
+         int service = SA_INIT_ONE_SHARE_FROM_NAME;
  
          /*
           * Mountpoint could get trashed if libshare calls getmntany
           * which it does during API initialization, so strdup the
           * value.
           */
          mntpt = zfs_strdup(hdl, mountpoint);
  
          /*
!          * 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;
!         }
! 
!         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"),
                      name, _sa_errorstr(err)));
          }
*** 1530,1542 ****
              zfs_mount_one, &ms, B_TRUE);
          if (ms.ms_mntstatus != 0)
                  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.
           */
          sharearg.zhandle_arr = cb.cb_handles;
          sharearg.zhandle_len = cb.cb_used;
          if ((ret = zfs_init_libshare_arg(zhp->zpool_hdl,
              SA_INIT_SHARE_API_SELECTIVE, &sharearg)) != 0)
--- 1555,1567 ----
              zfs_mount_one, &ms, B_TRUE);
          if (ms.ms_mntstatus != 0)
                  ret = ms.ms_mntstatus;
  
          /*
!          * 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;
          if ((ret = zfs_init_libshare_arg(zhp->zpool_hdl,
              SA_INIT_SHARE_API_SELECTIVE, &sharearg)) != 0)