From owner-freebsd-fs@FreeBSD.ORG Sat Jul 19 22:42:26 2014 Return-Path: Delivered-To: freebsd-fs@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:1900:2254:206a::19:1]) (using TLSv1 with cipher ADH-AES256-SHA (256/256 bits)) (No client certificate requested) by hub.freebsd.org (Postfix) with ESMTPS id EB515B2C for ; Sat, 19 Jul 2014 22:42:25 +0000 (UTC) Received: from venus.codepro.be (venus.codepro.be [IPv6:2a01:4f8:162:1127::2]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (Client CN "*.codepro.be", Issuer "Gandi Standard SSL CA" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id 7C0692612 for ; Sat, 19 Jul 2014 22:42:24 +0000 (UTC) Received: from vega.codepro.be (unknown [172.16.1.3]) by venus.codepro.be (Postfix) with ESMTP id 715AF1211E for ; Sun, 20 Jul 2014 00:42:22 +0200 (CEST) Received: by vega.codepro.be (Postfix, from userid 1001) id 684E6198C1; Sun, 20 Jul 2014 00:42:22 +0200 (CEST) Date: Sun, 20 Jul 2014 00:42:22 +0200 From: Kristof Provost To: freebsd-fs@freebsd.org Subject: Re: ZFS panic on zvol resize Message-ID: <20140719224222.GB2406@vega.codepro.be> References: <20140704194750.GU75721@vega.codepro.be> <20140719164113.GA2406@vega.codepro.be> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline In-Reply-To: <20140719164113.GA2406@vega.codepro.be> X-PGP-Fingerprint: E114 D9EA 909E D469 8F57 17A5 7D15 91C6 9EFA F286 X-Checked-By-NSA: Probably User-Agent: Mutt/1.5.23 (2014-03-12) X-BeenThere: freebsd-fs@freebsd.org X-Mailman-Version: 2.1.18 Precedence: list List-Id: Filesystems List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Sat, 19 Jul 2014 22:42:26 -0000 On 2014-07-19 18:41:13 (+0200), Kristof Provost wrote: > With this patch I no longer see the original panic, but I get a shiny > new one in its place: > > panic: solaris assert: txg_how != TXG_WAIT || !dsl_pool_config_held(tx->tx_pool), file: /usr/src/sys/modules/zfs/../../cddl/contrib/opensolaris/uts/common/fs/zfs/dmu_tx.c, line: 1279 Both problems appear to be fixed in Illumos commit 3b2aab18808792cbd248a12f1edf139b89833c13 Essentially they've changed from using dmu_objset_hold() to dmu_objset_own(). See: - https://www.illumos.org/issues/3464 - https://github.com/illumos/illumos-gate/commit/3b2aab18808792cbd248a12f1edf139b89833c13 - ZoL: https://github.com/zfsonlinux/zfs/pull/2048 I included the zvol resize bits of the patch in my tree, and can now resize zvols without panicing the machine. Index: cddl/contrib/opensolaris/uts/common/fs/zfs/sys/zvol.h =================================================================== --- cddl/contrib/opensolaris/uts/common/fs/zfs/sys/zvol.h (revision 268881) +++ cddl/contrib/opensolaris/uts/common/fs/zfs/sys/zvol.h (working copy) @@ -43,7 +43,7 @@ extern int zvol_create_minor(const char *); extern int zvol_remove_minor(const char *); extern void zvol_remove_minors(const char *); -extern int zvol_set_volsize(const char *, major_t, uint64_t); +extern int zvol_set_volsize(const char *, uint64_t); #ifdef sun extern int zvol_open(dev_t *devp, int flag, int otyp, cred_t *cr); Index: cddl/contrib/opensolaris/uts/common/fs/zfs/zfs_ioctl.c =================================================================== --- cddl/contrib/opensolaris/uts/common/fs/zfs/zfs_ioctl.c (revision 268881) +++ cddl/contrib/opensolaris/uts/common/fs/zfs/zfs_ioctl.c (working copy) @@ -2482,8 +2482,7 @@ err = dsl_dataset_set_refreservation(dsname, source, intval); break; case ZFS_PROP_VOLSIZE: - err = zvol_set_volsize(dsname, ddi_driver_major(zfs_dip), - intval); + err = zvol_set_volsize(dsname, intval); break; case ZFS_PROP_VERSION: { Index: cddl/contrib/opensolaris/uts/common/fs/zfs/zvol.c =================================================================== --- cddl/contrib/opensolaris/uts/common/fs/zfs/zvol.c (revision 268881) +++ cddl/contrib/opensolaris/uts/common/fs/zfs/zvol.c (working copy) @@ -203,11 +203,12 @@ static void zvol_geom_worker(void *arg); static void -zvol_size_changed(zvol_state_t *zv) +zvol_size_changed(zvol_state_t *zv, uint64_t volsize) { #ifdef sun dev_t dev = makedevice(maj, min); + zv->zv_volsize = volsize; VERIFY(ddi_prop_update_int64(dev, zfs_dip, "Size", volsize) == DDI_SUCCESS); VERIFY(ddi_prop_update_int64(dev, zfs_dip, @@ -765,9 +766,9 @@ dmu_objset_disown(os, zvol_tag); return (error); } - zv->zv_volsize = volsize; + + zvol_size_changed(zv, volsize); zv->zv_zilog = zil_open(os, zvol_get_data); - zvol_size_changed(zv); VERIFY(dsl_prop_get_integer(zv->zv_name, "readonly", &readonly, NULL) == 0); @@ -891,65 +892,42 @@ PICKUP_GIANT(); } -int -zvol_set_volsize(const char *name, major_t maj, uint64_t volsize) +static int +zvol_update_live_volsize(zvol_state_t *zv, uint64_t volsize) { - zvol_state_t *zv = NULL; - objset_t *os; - int error; - dmu_object_info_t doi; uint64_t old_volsize = 0ULL; - uint64_t readonly; + int error = 0; - mutex_enter(&spa_namespace_lock); - zv = zvol_minor_lookup(name); - if ((error = dmu_objset_hold(name, FTAG, &os)) != 0) { - mutex_exit(&spa_namespace_lock); - return (error); - } + ASSERT(MUTEX_HELD(&spa_namespace_lock)); - if ((error = dmu_object_info(os, ZVOL_OBJ, &doi)) != 0 || - (error = zvol_check_volsize(volsize, - doi.doi_data_block_size)) != 0) - goto out; - - VERIFY(dsl_prop_get_integer(name, "readonly", &readonly, - NULL) == 0); - if (readonly) { - error = EROFS; - goto out; - } - - error = zvol_update_volsize(os, volsize); /* * Reinitialize the dump area to the new size. If we * failed to resize the dump area then restore it back to - * its original size. + * its original size. We must set the new volsize prior + * to calling dumpvp_resize() to ensure that the devices' + * size(9P) is not visible by the dump subsystem. */ - if (zv && error == 0) { + old_volsize = zv->zv_volsize; + zvol_size_changed(zv, volsize); #ifdef ZVOL_DUMP - if (zv->zv_flags & ZVOL_DUMPIFIED) { - old_volsize = zv->zv_volsize; - zv->zv_volsize = volsize; - if ((error = zvol_dumpify(zv)) != 0 || - (error = dumpvp_resize()) != 0) { - (void) zvol_update_volsize(os, old_volsize); - zv->zv_volsize = old_volsize; - error = zvol_dumpify(zv); - } + if (zv->zv_flags & ZVOL_DUMPIFIED) { + if ((error = zvol_dumpify(zv)) != 0 || + (error = dumpvp_resize()) != 0) { + int dumpify_error; + + (void) zvol_update_volsize(zv->zv_objset, old_volsize); + zvol_size_changed(zv, old_volsize); + dumpify_error = zvol_dumpify(zv); + error = dumpify_error ? dumpify_error : error; } -#endif /* ZVOL_DUMP */ - if (error == 0) { - zv->zv_volsize = volsize; - zvol_size_changed(zv); - } } +#endif /* ZVOL_DUMP */ #ifdef sun /* * Generate a LUN expansion event. */ - if (zv && error == 0) { + if (error == 0) { sysevent_id_t eid; nvlist_t *attr; char *physpath = kmem_zalloc(MAXPATHLEN, KM_SLEEP); @@ -967,12 +945,57 @@ kmem_free(physpath, MAXPATHLEN); } #endif /* sun */ + return (error); +} +int +zvol_set_volsize(const char *name, uint64_t volsize) +{ + zvol_state_t *zv = NULL; + objset_t *os; + int error; + dmu_object_info_t doi; + uint64_t readonly; + boolean_t owned = B_FALSE; + + error = dsl_prop_get_integer(name, + zfs_prop_to_name(ZFS_PROP_READONLY), &readonly, NULL); + if (error != 0) + return (error); + if (readonly) + return (SET_ERROR(EROFS)); + + mutex_enter(&spa_namespace_lock); + zv = zvol_minor_lookup(name); + + if (zv == NULL || zv->zv_objset == NULL) { + if ((error = dmu_objset_own(name, DMU_OST_ZVOL, B_FALSE, + FTAG, &os)) != 0) { + mutex_exit(&spa_namespace_lock); + return (error); + } + owned = B_TRUE; + if (zv != NULL) + zv->zv_objset = os; + } else { + os = zv->zv_objset; + } + + if ((error = dmu_object_info(os, ZVOL_OBJ, &doi)) != 0 || + (error = zvol_check_volsize(volsize, doi.doi_data_block_size)) != 0) + goto out; + + error = zvol_update_volsize(os, volsize); + + if (error == 0 && zv != NULL) + error = zvol_update_live_volsize(zv, volsize); out: - dmu_objset_rele(os, FTAG); - + if (owned) { + dmu_objset_disown(os, FTAG); + if (zv != NULL) + zv->zv_objset = NULL; + } mutex_exit(&spa_namespace_lock); - return (error); } Regards, Kristof