d/freebsd-skq
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

8604 Avoid unnecessary work search in VFS when unmounting snapshots

Browse Source 
illumos/illumos-gate@ed992b0aac
ed992b0aac

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>
This commit is contained in:
Andriy Gapon 2017-09-20 07:28:18 +00:00
parent de90fd2168
commit 67effa3d26
3 changed files with 36 additions and 54 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

18
uts/common/fs/zfs/dsl_destroy.c
View File

@ -488,23 +488,29 @@ dsl_destroy_snapshots_nvl(nvlist_t *snaps, boolean_t defer,
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 defer,
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);

5
uts/common/fs/zfs/sys/zfs_ioctl.h
View File

@ -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 *, cred_t *);
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

67
uts/common/fs/zfs/zfs_ioctl.c
View File

@ -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 *innvl, nvlist_t *outnvl)
* 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 *innvl, nvlist_t *outnvl)
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);
}
/*