Date: Mon, 03 Aug 2015 16:20:04 +0300 From: Andriy Gapon <avg@FreeBSD.org> To: "K. Macy" <kmacy@FreeBSD.org> Cc: "src-committers@freebsd.org" <src-committers@FreeBSD.org>, "svn-src-all@freebsd.org" <svn-src-all@FreeBSD.org>, "svn-src-head@freebsd.org" <svn-src-head@FreeBSD.org>, Xin Li <delphij@delphij.net> Subject: Re: svn commit: r285021 - head/sys/cddl/contrib/opensolaris/uts/common/fs/zfs Message-ID: <55BF6A84.1040808@FreeBSD.org> In-Reply-To: <CAHM0Q_OcLqT_VuxmL4tFrDyEhAEprm_ivP8j_mrow6ZG1FLcsg@mail.gmail.com> References: <201507020832.t628W3WJ002944@repo.freebsd.org> <CAHM0Q_OcLqT_VuxmL4tFrDyEhAEprm_ivP8j_mrow6ZG1FLcsg@mail.gmail.com>
next in thread | previous in thread | raw e-mail | index | archive | help
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 <avg@freebsd.org> 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
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?55BF6A84.1040808>