CFR: Fix panic in zfs_umount() due to dropped root vnode refcount

Andriy Gapon avg at FreeBSD.org
Wed Mar 13 21:57:14 UTC 2013


on 13/03/2013 23:21 Will Andrews said the following:
> Hi,
> 
> Diff:
> http://people.freebsd.org/~will/zfs/zfs_vfsops.c-fix-unmount-panic.diff
> 
> Change 660985 by willa at willa_SpectraBSD on 2013/03/08 17:43:49
> 
>         Fix the zfs unmount panic that is due to a missed refcount drop.
> 
>         When checking for activity in a non-forced unmount, remember that
>         we called vflush() releasing the refcount that zfs_mount() placed
>         on the filesystem's root vnode using VFS_ROOT, and restore it.
> 
>         Without this change, the next time around, the missing refcount
>         led to a second release of the root vnode when its refcount was
>         already zero, and thus the panic occurred while trying to grab
>         the vnode's interlock, which had been destroyed when the vnode's
>         refcount dropped to zero.
> 
>         While I'm here, refactor the unmount failure code to ensure that
>         the zfs ctldir (i.e. .zfs/.snapshots) is always restored, along
> with,
>         if needed, zfsvfs->z_unmounted = B_FALSE.  In short, if the unmount
>         fails, restore all original functionality prior to unmounting.
> 
> This particular panic manifests easily if you have active NFS I/O on a ZFS
> while trying to do a non-forced unmount.  We've also seen it happen on
> shutdown occasionally.
> 
> I would like to commit this or a version of it, to FreeBSD/head next week.

I propose to put the whole troublesome if (!(fflag & MS_FORCE)) {... } block
under #ifdef sun -- that's what I have in my tree.
Solaris uses its own mechanism to decide whether unmounting is allowed, but
FreeBSD has its own -- vflush success is sufficient.

References:
http://code.metager.de/source/xref/freebsd/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/zfs_vfsops.c#1942
http://code.metager.de/source/search?q=&defs=&refs=mnt_ref&path=%2Ffreebsd%2F&hist=&project=freebsd
http://src.illumos.org/source/xref/illumos-gate/usr/src/uts/common/fs/zfs/zfs_vfsops.c#1866

Thank you for catching this edge case.
-- 
Andriy Gapon


More information about the zfs-devel mailing list