From owner-svn-src-head@freebsd.org Mon Aug 3 13:21:15 2015 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 3DE069B28B7; Mon, 3 Aug 2015 13:21:15 +0000 (UTC) (envelope-from avg@FreeBSD.org) Received: from citadel.icyb.net.ua (citadel.icyb.net.ua [212.40.38.140]) by mx1.freebsd.org (Postfix) with ESMTP id 9B2D7BF4; Mon, 3 Aug 2015 13:21:13 +0000 (UTC) (envelope-from avg@FreeBSD.org) Received: from porto.starpoint.kiev.ua (porto-e.starpoint.kiev.ua [212.40.38.100]) by citadel.icyb.net.ua (8.8.8p3/ICyb-2.3exp) with ESMTP id QAA22708; Mon, 03 Aug 2015 16:21:00 +0300 (EEST) (envelope-from avg@FreeBSD.org) Received: from localhost ([127.0.0.1]) by porto.starpoint.kiev.ua with esmtp (Exim 4.34 (FreeBSD)) id 1ZMFfn-000Bf6-P4; Mon, 03 Aug 2015 16:20:59 +0300 Message-ID: <55BF6A84.1040808@FreeBSD.org> Date: Mon, 03 Aug 2015 16:20:04 +0300 From: Andriy Gapon User-Agent: Mozilla/5.0 (X11; FreeBSD amd64; rv:31.0) Gecko/20100101 Thunderbird/31.7.0 MIME-Version: 1.0 To: "K. Macy" CC: "src-committers@freebsd.org" , "svn-src-all@freebsd.org" , "svn-src-head@freebsd.org" , Xin Li Subject: Re: svn commit: r285021 - head/sys/cddl/contrib/opensolaris/uts/common/fs/zfs References: <201507020832.t628W3WJ002944@repo.freebsd.org> In-Reply-To: Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 7bit X-BeenThere: svn-src-head@freebsd.org X-Mailman-Version: 2.1.20 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, 03 Aug 2015 13:21:15 -0000 On 30/07/2015 10:24, K. Macy wrote: > Just FYI this change introduces a deadlock with with the > spa_namespace_lock. Mount will be holding this lock while trying to > acquire the spa_namespace_lock. zfskern on the other hand holds the > spa_namespace_lock when calling zfs_freebsd_access which in turn > tries to acquire the teardown lock. I missed the fact that zpool.cache file is being written with spa_namespace_lock held. I'll try to either resolve the problem in the next day or I will revert the change. > I think this makes a pretty strong case for the need to educate > WITNESS about rrm locks. > > Cheers. > > -K > > > > > > > > > > > On Thu, Jul 2, 2015 at 1:32 AM, Andriy Gapon wrote: >> Author: avg >> Date: Thu Jul 2 08:32:02 2015 >> New Revision: 285021 >> URL: https://svnweb.freebsd.org/changeset/base/285021 >> >> Log: >> zfs_mount(MS_REMOUNT): protect zfs_(un)register_callbacks calls >> >> We now take z_teardown_lock as a writer to ensure that there is no I/O >> while the filesystem state is in a flux. Also, zfs_suspend_fs() -> >> zfsvfs_teardown() call zfs_unregister_callbacks() and zfs_resume_fs() -> >> zfsvfs_setup() call zfs_unregister_callbacks(). Previously there was no >> synchronization between those calls and the calls in the re-mounting >> case. That could lead to concurrent execution and a crash. >> >> PR: 180060 >> Differential Revision: https://reviews.freebsd.org/D2865 >> Suggested by: mahrens >> Reviewed by: delphij, pho, mahrens, will >> MFC after: 13 days >> Sponsored by: ClusterHQ >> >> Modified: >> head/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/zfs_vfsops.c >> >> Modified: head/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/zfs_vfsops.c >> ============================================================================== >> --- head/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/zfs_vfsops.c Thu Jul 2 08:25:45 2015 (r285020) >> +++ head/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/zfs_vfsops.c Thu Jul 2 08:32:02 2015 (r285021) >> @@ -1717,9 +1717,19 @@ zfs_mount(vfs_t *vfsp) >> * according to those options set in the current VFS options. >> */ >> if (vfsp->vfs_flag & MS_REMOUNT) { >> - /* refresh mount options */ >> - zfs_unregister_callbacks(vfsp->vfs_data); >> + zfsvfs_t *zfsvfs = vfsp->vfs_data; >> + >> + /* >> + * Refresh mount options with z_teardown_lock blocking I/O while >> + * the filesystem is in an inconsistent state. >> + * The lock also serializes this code with filesystem >> + * manipulations between entry to zfs_suspend_fs() and return >> + * from zfs_resume_fs(). >> + */ >> + rrm_enter(&zfsvfs->z_teardown_lock, RW_WRITER, FTAG); >> + zfs_unregister_callbacks(zfsvfs); >> error = zfs_register_callbacks(vfsp); >> + rrm_exit(&zfsvfs->z_teardown_lock, FTAG); >> goto out; >> } >> >> _______________________________________________ >> svn-src-head@freebsd.org mailing list >> http://lists.freebsd.org/mailman/listinfo/svn-src-head >> To unsubscribe, send any mail to "svn-src-head-unsubscribe@freebsd.org" -- Andriy Gapon