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 <a...@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-unsubscr...@freebsd.org" -- Andriy Gapon _______________________________________________ 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-unsubscr...@freebsd.org"
