Approval request of some additions to src/sys/kern/vfs_subr.c
kostikbel at gmail.com
Mon Apr 28 13:24:25 UTC 2008
On Mon, Apr 28, 2008 at 06:37:09PM +0900, Daichi GOTO wrote:
> Kostik Belousov wrote:
> >On Fri, Apr 25, 2008 at 07:21:20PM +0900, Daichi GOTO wrote:
> >>Hi Konstantin :)
> >>To fix a unionfs issue of
> >>we need to add new functions
> >> void vkernrele(struct vnode *vp);
> >> void vkernref(struct vnode *vp);
> >>and one value
> >> int v_kernusecount; /* i ref count of kernel */
> >>to src/sys/sys/vnode.h and rc/sys/kern/vfs_subr.c.
> >>Unionfs will be panic when lower fs layer is forced umounted by
> >>"umount -f". So to avoid this issue, we've added
> >>"v_kernusecount" value that means "a vnode count that kernel are
> >>using". vkernrele() and vkernref() are functions that manage
> >>"v_kernusecount" value.
> >>Please check those and give us an approve or some comments!
> >There is already the vnode reference count. From your description, I cannot
> >understand how the kernusecount would prevent the panic when forced unmount
> >is performed. Could you, please, show the actual code ? PR mentioned
> >does not contain any patch.
> Our patch realizes avoiding kernel panic by "umount -f" operation using with
> EBUSY process.
> On current implementation (not applied our patch), "umount -f" tries to
> release vnode at any vnode reference count value. Since that, unionfs
> and nullfs access invalid vnode and lead kernel panic. To prevent this
> issue, we need a some kind of not-umount-accept-mechanism in invalid case
> (e.x. fs in unionfsed stack, it must be umounted in correct order) and
> to realize that, current vnode reference count is not enough we are
> If you have any ideas to realize the same solution with current vnode
> reference, would you please tell us your idea :)
> >The problem you described is common for the kernel code, and right way
> >to handle it, for now, is to keep refcount _and_ check for the forced
Your patch in essence disables the forced unmount. I would object against
Even if taking this direction, I believe more cleaner solution would be
to introduce a counter that disables the (forced) unmount into the
struct mount, instead of the struct vnode. Having the counter in the
vnode, the unmount -f behaviour is non-deterministic and depended on
the presence of the cached vnodes of the upper layer. The mount counter
would be incremented by unionfs cover mount. But, as I said above, this
looks like a wrong solution.
The right way to handle the forced reclaim with the current VFS is to
add the explicit checks for the reclaimed vnodes where it is needed. The
vnode cannot be reclaimed while the vnode lock is held. When obtaining
the vnode lock, the reclamation can be detected. For instance, the
vget() without LK_RETRY shall be checked for ENOENT.
You said that that nullfs is vulnerable to the problem. Could you,
please, point me to the corresponding stack trace ? At least, the nullfs
vop_lock() seems to carefully check the possible problems.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Size: 195 bytes
Desc: not available
Url : http://lists.freebsd.org/pipermail/freebsd-fs/attachments/20080428/7cca421d/attachment.pgp
More information about the freebsd-fs