svn commit: r285021 - head/sys/cddl/contrib/opensolaris/uts/common/fs/zfs

[Andriy Gapon]

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 at 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 at freebsd.org mailing list
>> http://lists.freebsd.org/mailman/listinfo/svn-src-head
>> To unsubscribe, send any mail to "svn-src-head-unsubscribe at freebsd.org"


-- 
Andriy Gapon