Date: Tue, 17 Apr 2001 11:28:03 -0700 (PDT) From: Matt Dillon <dillon@earth.backplane.com> To: Poul-Henning Kamp <phk@critter.freebsd.dk> Cc: Kirk McKusick <mckusick@mckusick.com>, Julian Elischer <julian@elischer.org>, Rik van Riel <riel@conectiva.com.br>, freebsd-hackers@FreeBSD.ORG, David Xu <bsddiy@21cn.com> Subject: Re: vm balance Message-ID: <200104171828.f3HIS3E96964@earth.backplane.com> References: <24891.987530101@critter>
next in thread | previous in thread | raw e-mail | index | archive | help
: :In message <200104171745.f3HHj2K95255@earth.backplane.com>, Matt Dillon writes: :>:> I don't think NFS relies on vnodes never being freed. :>: :>:It does, in some case nfs stashes a vnode pointer and the v_id :>:value away, and some time later tries to use that pair to try to :>:refind the vnode again. If you free vnodes, it will still think :>:the pointer is a vnode and if junk happens to be right it will :>:think it is still a vnode. QED: Bad things (TM) will happen. :>: :>:# cd /sys/nfs :>:# grep v_id * :>:nfs_nqlease.c: vpid = vp->v_id; :>:nfs_nqlease.c: if (vpid == vp->v_id) { :>:nfs_nqlease.c: if (vpid == vp->v_id && :>:nfs_vnops.c: vpid = newvp->v_id; :>:nfs_vnops.c: if (vpid == newvp->v_id) { :>: :>:-- :>:Poul-Henning Kamp | UNIX since Zilog Zeus 3.20 :> :> hahahahahahahaha.. Look at the code more closely. v_id is not :> managed by NFS, it's managed by vfs_cache.c. There's a big XXX :> comment just before cache_purge() that explains it. Believe me, :> NFS is the least of your worries here. : :Matt, you try to free vnodes back to the malloc pool and you will :see what happens OK ? : :-- :Poul-Henning Kamp | UNIX since Zilog Zeus 3.20 ok ok... lets see. Oh, ok I see what it's doing. Actually I think you just found a bug. if ((error = cache_lookup(dvp, vpp, cnp)) && error != ENOENT) { struct vattr vattr; int vpid; if ((error = VOP_ACCESS(dvp, VEXEC, cnp->cn_cred, p)) != 0) { *vpp = NULLVP; return (error); } newvp = *vpp; vpid = newvp->v_id; This is totally bogus. VOP_ACCESS can block, so even using vpid above to check that the vnode hasn't been ripped out from under the code won't work. Also, take a look at the vput() later on, and also the vput() in kern/vfs_cache.c/vfs_cache_lookup() - that looks bogus to me too and would probably crash the machine. The easiest solution here is to make cache_lookup bump the ref count on the returned vnode and require that all users of cache_lookup vrele() it. -Matt To Unsubscribe: send mail to majordomo@FreeBSD.org with "unsubscribe freebsd-hackers" in the body of the message
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?200104171828.f3HIS3E96964>