From owner-freebsd-hackers  Tue Apr 17 11:28:18 2001
Delivered-To: freebsd-hackers@freebsd.org
Received: from earth.backplane.com (earth-nat-cw.backplane.com [208.161.114.67])
	by hub.freebsd.org (Postfix) with ESMTP id 3F4C637B423
	for <freebsd-hackers@FreeBSD.ORG>; Tue, 17 Apr 2001 11:28:12 -0700 (PDT)
	(envelope-from dillon@earth.backplane.com)
Received: (from dillon@localhost)
	by earth.backplane.com (8.11.2/8.11.2) id f3HIS3E96964;
	Tue, 17 Apr 2001 11:28:03 -0700 (PDT)
	(envelope-from dillon)
Date: Tue, 17 Apr 2001 11:28:03 -0700 (PDT)
From: Matt Dillon <dillon@earth.backplane.com>
Message-Id: <200104171828.f3HIS3E96964@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 
References:  <24891.987530101@critter>
Sender: owner-freebsd-hackers@FreeBSD.ORG
Precedence: bulk
X-Loop: FreeBSD.ORG

:
: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