Date: Sun, 07 Oct 2012 10:55:46 +0300 From: Andriy Gapon <avg@FreeBSD.org> To: Pawel Jakub Dawidek <pjd@FreeBSD.org>, "freebsd-fs@freebsd.org" <freebsd-fs@FreeBSD.org> Subject: zfs_remove: delete_now case Message-ID: <50713582.9040600@FreeBSD.org>
next in thread | raw e-mail | index | archive | help
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? -- Andriy Gapon
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?50713582.9040600>