Date: Wed, 20 Sep 2017 07:28:19 +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: r323795 - in vendor-sys/illumos/dist/uts/common/fs/zfs: . sys Message-ID: <201709200728.v8K7SJQS007836@repo.freebsd.org>
next in thread | raw e-mail | index | archive | help
Author: avg Date: Wed Sep 20 07:28:18 2017 New Revision: 323795 URL: https://svnweb.freebsd.org/changeset/base/323795 Log: 8604 Avoid unnecessary work search in VFS when unmounting snapshots illumos/illumos-gate@ed992b0aac4e5b70dc1273b1d055c0d471fbb4b1 https://github.com/illumos/illumos-gate/commit/ed992b0aac4e5b70dc1273b1d055c0d471fbb4b1 https://www.illumos.org/issues/8604 Every time we want to unmount a snapshot (happens during snapshot deletion or renaming) we unnecessarily iterate through all the mountpoints in the VFS layer (see zfs_get_vfs). Ideally we would just put a hold on the snapshot and access its respective VFS resource directly. gwilson_snap_unmount.svg - Flamegraph indicating the issue discussed (138 KB) Serapheim Dimitropoulos, 2017-09-14 06:36 PM Reviewed by: Matt Ahrens <mahrens@delphix.com> Reviewed by: George Wilson <george.wilson@delphix.com> Reviewed by: Andy Stormont <astormont@racktopsystems.com> Approved by: Robert Mustacchi <rm@joyent.com> Author: Serapheim Dimitropoulos <serapheim@delphix.com> Modified: vendor-sys/illumos/dist/uts/common/fs/zfs/dsl_destroy.c vendor-sys/illumos/dist/uts/common/fs/zfs/sys/zfs_ioctl.h vendor-sys/illumos/dist/uts/common/fs/zfs/zfs_ioctl.c Modified: vendor-sys/illumos/dist/uts/common/fs/zfs/dsl_destroy.c ============================================================================== --- vendor-sys/illumos/dist/uts/common/fs/zfs/dsl_destroy.c Wed Sep 20 07:27:45 2017 (r323794) +++ vendor-sys/illumos/dist/uts/common/fs/zfs/dsl_destroy.c Wed Sep 20 07:28:18 2017 (r323795) @@ -488,23 +488,29 @@ dsl_destroy_snapshots_nvl(nvlist_t *snaps, boolean_t d if (nvlist_next_nvpair(snaps, NULL) == NULL) return (0); - nvlist_t *arg = fnvlist_alloc(); - nvlist_t *snaps_normalized = fnvlist_alloc(); /* * lzc_destroy_snaps() is documented to take an nvlist whose - * values "don't matter". We need to convert that nvlist to one - * that we know can be converted to LUA. + * values "don't matter". We need to convert that nvlist to + * one that we know can be converted to LUA. We also don't + * care about any duplicate entries because the nvlist will + * be converted to a LUA table which should take care of this. */ + nvlist_t *snaps_normalized; + VERIFY0(nvlist_alloc(&snaps_normalized, 0, KM_SLEEP)); for (nvpair_t *pair = nvlist_next_nvpair(snaps, NULL); pair != NULL; pair = nvlist_next_nvpair(snaps, pair)) { fnvlist_add_boolean_value(snaps_normalized, nvpair_name(pair), B_TRUE); } + + nvlist_t *arg; + VERIFY0(nvlist_alloc(&arg, 0, KM_SLEEP)); fnvlist_add_nvlist(arg, "snaps", snaps_normalized); fnvlist_free(snaps_normalized); fnvlist_add_boolean_value(arg, "defer", defer); - nvlist_t *wrapper = fnvlist_alloc(); + nvlist_t *wrapper; + VERIFY0(nvlist_alloc(&wrapper, 0, KM_SLEEP)); fnvlist_add_nvlist(wrapper, ZCP_ARG_ARGLIST, arg); fnvlist_free(arg); @@ -538,7 +544,7 @@ dsl_destroy_snapshots_nvl(nvlist_t *snaps, boolean_t d program, 0, zfs_lua_max_memlimit, - fnvlist_lookup_nvpair(wrapper, ZCP_ARG_ARGLIST), result); + nvlist_next_nvpair(wrapper, NULL), result); if (error != 0) { char *errorstr = NULL; (void) nvlist_lookup_string(result, ZCP_RET_ERROR, &errorstr); Modified: vendor-sys/illumos/dist/uts/common/fs/zfs/sys/zfs_ioctl.h ============================================================================== --- vendor-sys/illumos/dist/uts/common/fs/zfs/sys/zfs_ioctl.h Wed Sep 20 07:27:45 2017 (r323794) +++ vendor-sys/illumos/dist/uts/common/fs/zfs/sys/zfs_ioctl.h Wed Sep 20 07:28:18 2017 (r323795) @@ -21,7 +21,7 @@ /* * Copyright (c) 2005, 2010, Oracle and/or its affiliates. All rights reserved. * Copyright (c) 2011-2012 Pawel Jakub Dawidek. All rights reserved. - * Copyright (c) 2012, 2016 by Delphix. All rights reserved. + * Copyright (c) 2012, 2017 by Delphix. All rights reserved. * Copyright 2016 RackTop Systems. * Copyright (c) 2014 Integros [integros.com] */ @@ -428,9 +428,10 @@ extern int zfs_secpolicy_snapshot_perms(const char *, extern int zfs_secpolicy_rename_perms(const char *, const char *, cred_t *); extern int zfs_secpolicy_destroy_perms(const char *, cred_t *); extern int zfs_busy(void); -extern int zfs_unmount_snap(const char *); +extern void zfs_unmount_snap(const char *); extern void zfs_destroy_unmount_origin(const char *); extern int getzfsvfs_impl(struct objset *, struct zfsvfs **); +extern int getzfsvfs(const char *, struct zfsvfs **); /* * ZFS minor numbers can refer to either a control device instance or Modified: vendor-sys/illumos/dist/uts/common/fs/zfs/zfs_ioctl.c ============================================================================== --- vendor-sys/illumos/dist/uts/common/fs/zfs/zfs_ioctl.c Wed Sep 20 07:27:45 2017 (r323794) +++ vendor-sys/illumos/dist/uts/common/fs/zfs/zfs_ioctl.c Wed Sep 20 07:28:18 2017 (r323795) @@ -1417,7 +1417,7 @@ getzfsvfs_impl(objset_t *os, zfsvfs_t **zfvp) return (error); } -static int +int getzfsvfs(const char *dsname, zfsvfs_t **zfvp) { objset_t *os; @@ -2988,31 +2988,6 @@ zfs_ioc_get_fsacl(zfs_cmd_t *zc) return (error); } -/* - * Search the vfs list for a specified resource. Returns a pointer to it - * or NULL if no suitable entry is found. The caller of this routine - * is responsible for releasing the returned vfs pointer. - */ -static vfs_t * -zfs_get_vfs(const char *resource) -{ - struct vfs *vfsp; - struct vfs *vfs_found = NULL; - - vfs_list_read_lock(); - vfsp = rootvfs; - do { - if (strcmp(refstr_value(vfsp->vfs_resource), resource) == 0) { - VFS_HOLD(vfsp); - vfs_found = vfsp; - break; - } - vfsp = vfsp->vfs_next; - } while (vfsp != rootvfs); - vfs_list_unlock(); - return (vfs_found); -} - /* ARGSUSED */ static void zfs_create_cb(objset_t *os, void *arg, cred_t *cr, dmu_tx_t *tx) @@ -3439,40 +3414,41 @@ zfs_ioc_log_history(const char *unused, nvlist_t *innv * Returns 0 if the argument is not a snapshot, or it is not currently a * filesystem, or we were able to unmount it. Returns error code otherwise. */ -int +void zfs_unmount_snap(const char *snapname) { - vfs_t *vfsp; - zfsvfs_t *zfsvfs; - int err; + vfs_t *vfsp = NULL; + zfsvfs_t *zfsvfs = NULL; if (strchr(snapname, '@') == NULL) - return (0); + return; - vfsp = zfs_get_vfs(snapname); - if (vfsp == NULL) - return (0); + int err = getzfsvfs(snapname, &zfsvfs); + if (err != 0) { + ASSERT3P(zfsvfs, ==, NULL); + return; + } + vfsp = zfsvfs->z_vfs; - zfsvfs = vfsp->vfs_data; ASSERT(!dsl_pool_config_held(dmu_objset_pool(zfsvfs->z_os))); err = vn_vfswlock(vfsp->vfs_vnodecovered); VFS_RELE(vfsp); if (err != 0) - return (SET_ERROR(err)); + return; /* * Always force the unmount for snapshots. */ (void) dounmount(vfsp, MS_FORCE, kcred); - return (0); } /* ARGSUSED */ static int zfs_unmount_snap_cb(const char *snapname, void *arg) { - return (zfs_unmount_snap(snapname)); + zfs_unmount_snap(snapname); + return (0); } /* @@ -3495,7 +3471,7 @@ zfs_destroy_unmount_origin(const char *fsname) char originname[ZFS_MAX_DATASET_NAME_LEN]; dsl_dataset_name(ds->ds_prev, originname); dmu_objset_rele(os, FTAG); - (void) zfs_unmount_snap(originname); + zfs_unmount_snap(originname); } else { dmu_objset_rele(os, FTAG); } @@ -3524,7 +3500,7 @@ zfs_ioc_destroy_snaps(const char *poolname, nvlist_t * for (pair = nvlist_next_nvpair(snaps, NULL); pair != NULL; pair = nvlist_next_nvpair(snaps, pair)) { - (void) zfs_unmount_snap(nvpair_name(pair)); + zfs_unmount_snap(nvpair_name(pair)); } return (dsl_destroy_snapshots_nvl(snaps, defer, outnvl)); @@ -3667,11 +3643,8 @@ zfs_ioc_destroy(zfs_cmd_t *zc) { int err; - if (zc->zc_objset_type == DMU_OST_ZFS) { - err = zfs_unmount_snap(zc->zc_name); - if (err != 0) - return (err); - } + if (zc->zc_objset_type == DMU_OST_ZFS) + zfs_unmount_snap(zc->zc_name); if (strchr(zc->zc_name, '@')) err = dsl_destroy_snapshot(zc->zc_name, zc->zc_defer_destroy); @@ -3735,7 +3708,9 @@ recursive_unmount(const char *fsname, void *arg) char fullname[ZFS_MAX_DATASET_NAME_LEN]; (void) snprintf(fullname, sizeof (fullname), "%s@%s", fsname, snapname); - return (zfs_unmount_snap(fullname)); + zfs_unmount_snap(fullname); + + return (0); } /*
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?201709200728.v8K7SJQS007836>