Tue Dec 20 16:49:37 2011 UTC ()
Move the diagnostic check for a missing VOP_CLOSE() to the top of vrelel().
As long as we hold the vnode interlock there is no chance for this vnode
to gain new references.

Fixes false alarms observed by Thor Lancelot Simon and reported on tech-kern.

Ok: David Holland <dholland@netbsd.org>


(hannken)
diff -r1.14 -r1.15 src/sys/kern/vfs_vnode.c
cvs diff -r1.14 -r1.15 src/sys/kern/vfs_vnode.c (expand / switch to unified diff)

--- src/sys/kern/vfs_vnode.c 2011/10/07 09:35:06 1.14
+++ src/sys/kern/vfs_vnode.c 2011/12/20 16:49:37 1.15
@@ -1,14 +1,14 @@ @@ -1,14 +1,14 @@
1/* $NetBSD: vfs_vnode.c,v 1.14 2011/10/07 09:35:06 hannken Exp $ */ 1/* $NetBSD: vfs_vnode.c,v 1.15 2011/12/20 16:49:37 hannken Exp $ */
2 2
3/*- 3/*-
4 * Copyright (c) 1997-2011 The NetBSD Foundation, Inc. 4 * Copyright (c) 1997-2011 The NetBSD Foundation, Inc.
5 * All rights reserved. 5 * All rights reserved.
6 * 6 *
7 * This code is derived from software contributed to The NetBSD Foundation 7 * This code is derived from software contributed to The NetBSD Foundation
8 * by Jason R. Thorpe of the Numerical Aerospace Simulation Facility, 8 * by Jason R. Thorpe of the Numerical Aerospace Simulation Facility,
9 * NASA Ames Research Center, by Charles M. Hannum, and by Andrew Doran. 9 * NASA Ames Research Center, by Charles M. Hannum, and by Andrew Doran.
10 * 10 *
11 * Redistribution and use in source and binary forms, with or without 11 * Redistribution and use in source and binary forms, with or without
12 * modification, are permitted provided that the following conditions 12 * modification, are permitted provided that the following conditions
13 * are met: 13 * are met:
14 * 1. Redistributions of source code must retain the above copyright 14 * 1. Redistributions of source code must retain the above copyright
@@ -110,27 +110,27 @@ @@ -110,27 +110,27 @@
110 * 110 *
111 * Unless the VC_XLOCK bit is set, changing the usecount from a non-zero 111 * Unless the VC_XLOCK bit is set, changing the usecount from a non-zero
112 * value to a non-zero value can safely be done using atomic operations, 112 * value to a non-zero value can safely be done using atomic operations,
113 * without the interlock held. 113 * without the interlock held.
114 * 114 *
115 * Even if the VC_XLOCK bit is set, decreasing the usecount to a non-zero 115 * Even if the VC_XLOCK bit is set, decreasing the usecount to a non-zero
116 * value can be done using atomic operations, without the interlock held. 116 * value can be done using atomic operations, without the interlock held.
117 * 117 *
118 * Note: if VI_CLEAN is set, vnode_t::v_interlock will be released while 118 * Note: if VI_CLEAN is set, vnode_t::v_interlock will be released while
119 * mntvnode_lock is still held. 119 * mntvnode_lock is still held.
120 */ 120 */
121 121
122#include <sys/cdefs.h> 122#include <sys/cdefs.h>
123__KERNEL_RCSID(0, "$NetBSD: vfs_vnode.c,v 1.14 2011/10/07 09:35:06 hannken Exp $"); 123__KERNEL_RCSID(0, "$NetBSD: vfs_vnode.c,v 1.15 2011/12/20 16:49:37 hannken Exp $");
124 124
125#include <sys/param.h> 125#include <sys/param.h>
126#include <sys/kernel.h> 126#include <sys/kernel.h>
127 127
128#include <sys/atomic.h> 128#include <sys/atomic.h>
129#include <sys/buf.h> 129#include <sys/buf.h>
130#include <sys/conf.h> 130#include <sys/conf.h>
131#include <sys/device.h> 131#include <sys/device.h>
132#include <sys/kauth.h> 132#include <sys/kauth.h>
133#include <sys/kmem.h> 133#include <sys/kmem.h>
134#include <sys/kthread.h> 134#include <sys/kthread.h>
135#include <sys/module.h> 135#include <sys/module.h>
136#include <sys/mount.h> 136#include <sys/mount.h>
@@ -627,26 +627,33 @@ vrelel(vnode_t *vp, int flags) @@ -627,26 +627,33 @@ vrelel(vnode_t *vp, int flags)
627 * and unlock. 627 * and unlock.
628 */ 628 */
629 if (vtryrele(vp)) { 629 if (vtryrele(vp)) {
630 vp->v_iflag |= VI_INACTREDO; 630 vp->v_iflag |= VI_INACTREDO;
631 mutex_exit(vp->v_interlock); 631 mutex_exit(vp->v_interlock);
632 return; 632 return;
633 } 633 }
634 if (vp->v_usecount <= 0 || vp->v_writecount != 0) { 634 if (vp->v_usecount <= 0 || vp->v_writecount != 0) {
635 vnpanic(vp, "%s: bad ref count", __func__); 635 vnpanic(vp, "%s: bad ref count", __func__);
636 } 636 }
637 637
638 KASSERT((vp->v_iflag & VI_XLOCK) == 0); 638 KASSERT((vp->v_iflag & VI_XLOCK) == 0);
639 639
 640#ifdef DIAGNOSTIC
 641 if ((vp->v_type == VBLK || vp->v_type == VCHR) &&
 642 vp->v_specnode != NULL && vp->v_specnode->sn_opencnt != 0) {
 643 vprint("vrelel: missing VOP_CLOSE()", vp);
 644 }
 645#endif
 646
640 /* 647 /*
641 * If not clean, deactivate the vnode, but preserve 648 * If not clean, deactivate the vnode, but preserve
642 * our reference across the call to VOP_INACTIVE(). 649 * our reference across the call to VOP_INACTIVE().
643 */ 650 */
644retry: 651retry:
645 if ((vp->v_iflag & VI_CLEAN) == 0) { 652 if ((vp->v_iflag & VI_CLEAN) == 0) {
646 recycle = false; 653 recycle = false;
647 vp->v_iflag |= VI_INACTNOW; 654 vp->v_iflag |= VI_INACTNOW;
648 655
649 /* 656 /*
650 * XXX This ugly block can be largely eliminated if 657 * XXX This ugly block can be largely eliminated if
651 * locking is pushed down into the file systems. 658 * locking is pushed down into the file systems.
652 * 659 *
@@ -696,33 +703,26 @@ retry: @@ -696,33 +703,26 @@ retry:
696 KASSERT(mutex_owned(vp->v_interlock)); 703 KASSERT(mutex_owned(vp->v_interlock));
697 KASSERT((vp->v_iflag & VI_INACTPEND) == 0); 704 KASSERT((vp->v_iflag & VI_INACTPEND) == 0);
698 vp->v_iflag &= ~VI_INACTNOW; 705 vp->v_iflag &= ~VI_INACTNOW;
699 vp->v_iflag |= VI_INACTPEND; 706 vp->v_iflag |= VI_INACTPEND;
700 mutex_enter(&vrele_lock); 707 mutex_enter(&vrele_lock);
701 TAILQ_INSERT_TAIL(&vrele_list, vp, v_freelist); 708 TAILQ_INSERT_TAIL(&vrele_list, vp, v_freelist);
702 if (++vrele_pending > (desiredvnodes >> 8)) 709 if (++vrele_pending > (desiredvnodes >> 8))
703 cv_signal(&vrele_cv);  710 cv_signal(&vrele_cv);
704 mutex_exit(&vrele_lock); 711 mutex_exit(&vrele_lock);
705 mutex_exit(vp->v_interlock); 712 mutex_exit(vp->v_interlock);
706 return; 713 return;
707 } 714 }
708 715
709#ifdef DIAGNOSTIC 
710 if ((vp->v_type == VBLK || vp->v_type == VCHR) && 
711 vp->v_specnode != NULL && vp->v_specnode->sn_opencnt != 0) { 
712 vprint("vrelel: missing VOP_CLOSE()", vp); 
713 } 
714#endif 
715 
716 /* 716 /*
717 * The vnode can gain another reference while being 717 * The vnode can gain another reference while being
718 * deactivated. If VOP_INACTIVE() indicates that 718 * deactivated. If VOP_INACTIVE() indicates that
719 * the described file has been deleted, then recycle 719 * the described file has been deleted, then recycle
720 * the vnode irrespective of additional references. 720 * the vnode irrespective of additional references.
721 * Another thread may be waiting to re-use the on-disk 721 * Another thread may be waiting to re-use the on-disk
722 * inode. 722 * inode.
723 * 723 *
724 * Note that VOP_INACTIVE() will drop the vnode lock. 724 * Note that VOP_INACTIVE() will drop the vnode lock.
725 */ 725 */
726 VOP_INACTIVE(vp, &recycle); 726 VOP_INACTIVE(vp, &recycle);
727 mutex_enter(vp->v_interlock); 727 mutex_enter(vp->v_interlock);
728 vp->v_iflag &= ~VI_INACTNOW; 728 vp->v_iflag &= ~VI_INACTNOW;