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

[K. Macy]

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.

static int
zfs_access(vnode_t *vp, int mode, int flag, cred_t *cr,
    caller_context_t *ct)
{
znode_t *zp = VTOZ(vp);
zfsvfs_t *zfsvfs = zp->z_zfsvfs;
int error;

ZFS_ENTER(zfsvfs);

where:
/* Called on entry to each ZFS vnode and vfs operation  */
#define ZFS_ENTER(zfsvfs) \
{ \
rrm_enter_read(&(zfsvfs)->z_teardown_lock, FTAG); \
if ((zfsvfs)->z_unmounted) { \
ZFS_EXIT(zfsvfs); \
return (EIO); \
} \


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"