Date: Fri, 01 Dec 2000 01:23:26 +0000 From: Ian Dowse <iedowse@maths.tcd.ie> To: hackers@freebsd.org Cc: iedowse@maths.tcd.ie Subject: rootvnode Message-ID: <200012010123.aa58484@salmon.maths.tcd.ie>
next in thread | raw e-mail | index | archive | help
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
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi? <200012010123.aa58484>