zfs_remove: delete_now case

[Pawel Jakub Dawidek]

On Sun, Oct 07, 2012 at 10:55:46AM +0300, Andriy Gapon wrote:
> 
> It seems that the delete_now path is never taken in zfs_remove().
> This is probably good in the bug-cancels-bug way...
> 
> On FreeBSD VOP_REMOVE is always called with vp being referenced.  zfs_remove
> doesn't take advantage of VOP_REMOVE interface.  It ignores the vp argument and
> instead re-looks up a directory entry id by name (a small performance hit here)
> and then uses zfs_zget, which adds another reference to the entry's vnode.
> Thus, a reference count of the vnode is always not less than two.  So
> may_delete_now and delete_now are always false.
> 
> Why this is good?  Because FreeBSD VFS doesn't support direct destruction (or
> corruption) of the vnode in VOP_REMOVE.  It expects to still have a valid vnode
> with a valid reference count after VOP_REMOVE and then calls vput/vrele on it.
> But the code in the delete_now branch does some nasty things.  It directly
> decrements the use count and it directly destroys the underlying znode (which is
> fine in Solaris but not in FreeBSD).
> But FreeBSD VFS wouldn't even have a chance to panic on the damaged vnode
> because ZFS code would sooner panic in zfs_znode_delete -> zfs_znode_free ->
> ASSERT(ZTOV(zp) == NULL) [a FreeBSD-specific assertion).
> 
> I think that we should make zfs_remove code less confusing and more FreeBSD
> friendly.  We should explicitly rely on zfs_inactive doing the right thing after
> VOP_REMOVE and drop all the "direct action" code.
> 
> What do you think?

I'm fully aware of this code path being dead on FreeBSD. It is left
there only to minimize diff against vendor, so I'd prefer not to remove
it. Surrounding the code with '#ifdef sun' or similar is ok, I think.

-- 
Pawel Jakub Dawidek                       http://www.wheelsystems.com
FreeBSD committer                         http://www.FreeBSD.org
Am I Evil? Yes, I Am!                     http://tupytaj.pl
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 196 bytes
Desc: not available
Url : http://lists.freebsd.org/pipermail/freebsd-fs/attachments/20121007/7de32d18/attachment.pgp