svn commit: r323795 - in vendor-sys/illumos/dist/uts/common/fs/zfs: . sys
Andriy Gapon
avg at FreeBSD.org
Wed Sep 20 07:28:20 UTC 2017
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 at 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 at delphix.com>
Reviewed by: George Wilson <george.wilson at delphix.com>
Reviewed by: Andy Stormont <astormont at racktopsystems.com>
Approved by: Robert Mustacchi <rm at joyent.com>
Author: Serapheim Dimitropoulos <serapheim at 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);
}
/*
More information about the svn-src-all
mailing list