From owner-freebsd-hackers  Thu Nov 30 17:23:31 2000
Delivered-To: freebsd-hackers@freebsd.org
Received: from salmon.maths.tcd.ie (salmon.maths.tcd.ie [134.226.81.11])
	by hub.freebsd.org (Postfix) with SMTP id CE45637B401
	for <hackers@freebsd.org>; Thu, 30 Nov 2000 17:23:27 -0800 (PST)
Received: from walton.maths.tcd.ie by salmon.maths.tcd.ie with SMTP
          id <aa58484@salmon>; 1 Dec 2000 01:23:27 +0000 (GMT)
To: hackers@freebsd.org
Cc: iedowse@maths.tcd.ie
Subject: rootvnode
Date: Fri, 01 Dec 2000 01:23:26 +0000
From: Ian Dowse <iedowse@maths.tcd.ie>
Message-ID:  <200012010123.aa58484@salmon.maths.tcd.ie>
Sender: owner-freebsd-hackers@FreeBSD.ORG
Precedence: bulk
X-Loop: FreeBSD.ORG


It appears that the pointer to the root vnode, 'rootvnode' does
not hold a corresponding vnode reference. Here's a fragment of code
from start_init():

	/* Get the vnode for '/'.  Set p->p_fd->fd_cdir to reference it. */
	if (VFS_ROOT(TAILQ_FIRST(&mountlist), &rootvnode))
	panic("cannot find root vnode");
	p->p_fd->fd_cdir = rootvnode;
	VREF(p->p_fd->fd_cdir);
	p->p_fd->fd_rdir = rootvnode;
	VOP_UNLOCK(rootvnode, 0, p);

Since rootvnode is a global variable, three pointers to the root
vnode are stored, but only two references are counted (one by
VFS_ROOT, one by VREF).

Normally this is not a problem, since proc0's fd_cdir and fd_rdir
keep their references until the system is rebooted. However the
code in vfs_syscalls.c's checkdirs() function assumes that rootvnode
does hold a reference on the vnode:

	if (rootvnode == olddp) {
		vrele(rootvnode);
		VREF(newdp);
		rootvnode = newdp;
	}

This bug reliably causes a panic on reboot if any filesystem has
been mounted directly over /. For example, try:

	mount_mfs -T fd1440 none /
	Ctrl-Alt-Delete

On -current the panic is 'vrele: missed vn_close'; on 4.1-STABLE it
is 'vrele: negative ref cnt'. It occurs in dounmount() at the lines

	if ((coveredvp = mp->mnt_vnodecovered) != NULLVP) {
		coveredvp->v_mountedhere = (struct mount *)0;
		vrele(coveredvp);
	}

when unmounting the second / filesystem. This occurs because
checkdirs() has stolen a reference to /, so the reference count
goes negative when we attempt to remove the last reference.

This brings up another question: should the code reverse the changes
made by checkdirs() when a filesystem is unmounted? It certainly
seems to make sense to make rootvnode point to underlying vnode
when the filesystem containing the current rootvnode is unmounted;
I'm not sure how useful fixing up other fd_cdir/fd_rdir pointers
would be.

I can produce a simple patch which does the following:
 -  vref(rootvnode) in start_init().
 -  vrele(rootvnode) if non-NULL, maybe in vfs_unmountall()
 -  point rootvnode at underlying vnode when the filesystem
    containing rootvnode is unmounted.

Does this sound reasonable?

Ian


To Unsubscribe: send mail to majordomo@FreeBSD.org
with "unsubscribe freebsd-hackers" in the body of the message