From owner-svn-src-head@freebsd.org Mon Oct 2 11:15:33 2017 Return-Path: Delivered-To: svn-src-head@mailman.ysv.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:1900:2254:206a::19:1]) by mailman.ysv.freebsd.org (Postfix) with ESMTP id 6F042E3CBEB; Mon, 2 Oct 2017 11:15:33 +0000 (UTC) (envelope-from avg@FreeBSD.org) Received: from repo.freebsd.org (repo.freebsd.org [IPv6:2610:1c1:1:6068::e6a:0]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (Client did not present a certificate) by mx1.freebsd.org (Postfix) with ESMTPS id 45B9063B3D; Mon, 2 Oct 2017 11:15:33 +0000 (UTC) (envelope-from avg@FreeBSD.org) Received: from repo.freebsd.org ([127.0.1.37]) by repo.freebsd.org (8.15.2/8.15.2) with ESMTP id v92BFWtF096482; Mon, 2 Oct 2017 11:15:32 GMT (envelope-from avg@FreeBSD.org) Received: (from avg@localhost) by repo.freebsd.org (8.15.2/8.15.2/Submit) id v92BFW7l096479; Mon, 2 Oct 2017 11:15:32 GMT (envelope-from avg@FreeBSD.org) Message-Id: <201710021115.v92BFW7l096479@repo.freebsd.org> X-Authentication-Warning: repo.freebsd.org: avg set sender to avg@FreeBSD.org using -f From: Andriy Gapon Date: Mon, 2 Oct 2017 11:15:32 +0000 (UTC) To: src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org Subject: svn commit: r324195 - in head/sys/cddl/contrib/opensolaris/uts/common/fs/zfs: . sys X-SVN-Group: head X-SVN-Commit-Author: avg X-SVN-Commit-Paths: in head/sys/cddl/contrib/opensolaris/uts/common/fs/zfs: . sys X-SVN-Commit-Revision: 324195 X-SVN-Commit-Repository: base MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit X-BeenThere: svn-src-head@freebsd.org X-Mailman-Version: 2.1.23 Precedence: list List-Id: SVN commit messages for the src tree for head/-current List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 02 Oct 2017 11:15:33 -0000 Author: avg Date: Mon Oct 2 11:15:32 2017 New Revision: 324195 URL: https://svnweb.freebsd.org/changeset/base/324195 Log: MFV r323795: 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. Reviewed by: Matt Ahrens Reviewed by: George Wilson Reviewed by: Andy Stormont Approved by: Robert Mustacchi Author: Serapheim Dimitropoulos FreeBSD note: I added a FreeBSD specific function getzfsvfs_ref() which is like getzfsvfs() but returns a filesystem referenced, not busied. We want a busied filesystem in most cases, because we access its private data and, thus, we need to prevent the filesystem from being unmounted and its private data destroyed. But in some cases we can either get away with just a referenced filesystem or we must not busy the filesystem. Unmounting the filesystem is one of such cases. MFC after: 5 weeks X-MFC after: r324163 Modified: head/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/dsl_destroy.c head/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/sys/zfs_ioctl.h head/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/zfs_ioctl.c Directory Properties: head/sys/cddl/contrib/opensolaris/ (props changed) Modified: head/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/dsl_destroy.c ============================================================================== --- head/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/dsl_destroy.c Mon Oct 2 11:07:48 2017 (r324194) +++ head/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/dsl_destroy.c Mon Oct 2 11:15:32 2017 (r324195) @@ -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: head/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/sys/zfs_ioctl.h ============================================================================== --- head/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/sys/zfs_ioctl.h Mon Oct 2 11:07:48 2017 (r324194) +++ head/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/sys/zfs_ioctl.h Mon Oct 2 11:15:32 2017 (r324195) @@ -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] */ @@ -424,9 +424,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: head/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/zfs_ioctl.c ============================================================================== --- head/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/zfs_ioctl.c Mon Oct 2 11:07:48 2017 (r324194) +++ head/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/zfs_ioctl.c Mon Oct 2 11:15:32 2017 (r324195) @@ -1460,7 +1460,8 @@ getzfsvfs_impl(objset_t *os, zfsvfs_t **zfvp) return (error); } -static int +#ifdef illumos +int getzfsvfs(const char *dsname, zfsvfs_t **zfvp) { objset_t *os; @@ -1472,16 +1473,44 @@ getzfsvfs(const char *dsname, zfsvfs_t **zfvp) error = getzfsvfs_impl(os, zfvp); dmu_objset_rele(os, FTAG); - if (error == 0) { - error = vfs_busy((*zfvp)->z_vfs, 0); - vfs_rel((*zfvp)->z_vfs); - if (error != 0) { - *zfvp = NULL; - error = SET_ERROR(ESRCH); - } + return (error); +} + +#else + +static int +getzfsvfs_ref(const char *dsname, zfsvfs_t **zfvp) +{ + objset_t *os; + int error; + + error = dmu_objset_hold(dsname, FTAG, &os); + if (error != 0) + return (error); + + error = getzfsvfs_impl(os, zfvp); + dmu_objset_rele(os, FTAG); + return (error); +} + +int +getzfsvfs(const char *dsname, zfsvfs_t **zfvp) +{ + objset_t *os; + int error; + + error = getzfsvfs_ref(dsname, zfvp); + if (error != 0) + return (error); + error = vfs_busy((*zfvp)->z_vfs, 0); + vfs_rel((*zfvp)->z_vfs); + if (error != 0) { + *zfvp = NULL; + error = SET_ERROR(ESRCH); } return (error); } +#endif /* * Find a zfsvfs_t for a mounted filesystem, or create our own, in which @@ -3055,27 +3084,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) -{ - vfs_t *vfsp; - - mtx_lock(&mountlist_mtx); - TAILQ_FOREACH(vfsp, &mountlist, mnt_list) { - if (strcmp(refstr_value(vfsp->vfs_resource), resource) == 0) { - vfs_ref(vfsp); - break; - } - } - mtx_unlock(&mountlist_mtx); - return (vfsp); -} - /* ARGSUSED */ static void zfs_create_cb(objset_t *os, void *arg, cred_t *cr, dmu_tx_t *tx) @@ -3557,30 +3565,29 @@ zfs_ioc_nextboot(const char *unused, nvlist_t *innvl, * 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; -#ifdef illumos - int err; -#endif + 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_ref(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))); #ifdef illumos err = vn_vfswlock(vfsp->vfs_vnodecovered); VFS_RELE(vfsp); if (err != 0) - return (SET_ERROR(err)); + return; #endif /* @@ -3591,14 +3598,14 @@ zfs_unmount_snap(const char *snapname) #else (void) dounmount(vfsp, MS_FORCE, curthread); #endif - 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); } /* @@ -3621,7 +3628,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); } @@ -3662,9 +3669,7 @@ zfs_ioc_destroy_snaps(const char *poolname, nvlist_t * (name[poollen] != '/' && name[poollen] != '@')) return (SET_ERROR(EXDEV)); - error = zfs_unmount_snap(name); - if (error != 0) - return (error); + zfs_unmount_snap(nvpair_name(pair)); #if defined(__FreeBSD__) zvol_remove_minors(name); #endif @@ -3810,11 +3815,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); @@ -3886,7 +3888,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); } /*