dounmount: do not release the mount point's reference on the covered vnode
ClosedPublic
Actions

Authored by avg on Sep 12 2017, 6:08 AM.

Details

Reviewers

kib
mckusick

Commits

rS323578: dounmount: do not release the mount point's reference on the covered vnode

Summary

As long as mnt_ref is not zero there can be a consumer that might try
to access mnt_vnodecovered. For this reason the covered vnode must not
be freed until mnt_ref goes to zero.
So, move the release of the covered vnode to vfs_mount_destroy.

Diff Detail

Repository

rS FreeBSD src repository - subversion

Lint

Lint Passed

Unit

No Test Coverage

Build Status

Buildable 11505
Build 11860: arc lint + arc unit

Event Timeline

avg created this revision.Sep 12 2017, 6:08 AM

Herald added a subscriber: imp. · View Herald TranscriptSep 12 2017, 6:08 AM

avg added a reviewer: mckusick.Sep 12 2017, 6:09 AM

There is also one use of vfs_mount_destroy() in zfs, but I did not looked at it. You should know it much better.

sys/kern/vfs_mount.c
826	Look at this fragment. I believe that you need vref(vp) before the call to vfs_mount_destroy() at line 822.

avg added inline comments.Sep 12 2017, 9:29 AM

sys/kern/vfs_mount.c
826	Maybe setting mnt_vnodecovered to NULL would be a better solution? The `mp` should not be visible to any external consumers yet. The same would apply to `mount_snapshot` in ZFS code.

kib added inline comments.Sep 12 2017, 9:57 AM

sys/kern/vfs_mount.c
826	Fine with me.

clear mnt_vnodecovered before calling vfs_mount_destroy in case of the mount failure,
that's safe to do because the new struct mount is not visible to the outside world

Harbormaster completed remote builds in B11513: Diff 32956.Sep 12 2017, 10:53 AM

kib accepted this revision.Sep 12 2017, 11:09 AM

This revision is now accepted and ready to land.Sep 12 2017, 11:09 AM

Closed by commit rS323578: dounmount: do not release the mount point's reference on the covered vnode (authored by avg). · Explain WhySep 14 2017, 8:47 AM

This revision was automatically updated to reflect the committed changes.