>Number:         5013
>Category:       kern
>Synopsis:       lfs_fastvget locks inodes, then they are forgotten
>Confidential:   no
>Severity:       serious
>Priority:       medium
>Responsible:    kern-bug-people (Kernel Bug People)
>State:          open
>Class:          sw-bug
>Submitter-Id:   net
>Arrival-Date:   Tue Feb 17 22:05:01 1998
>Last-Modified:
>Originator:     Konrad Schroder
>Organization:
						Konrad Schroder
						perseant@hhhh.org
>Release:        1.3, plus patches for PR #s 4641, 4812, 4965.
>Environment:
System: NetBSD inle 1.3 NetBSD 1.3 (INLE) #101: Tue Feb 17 19:14:12 PST 1998 perseant@inle:/usr/src/sys/arch/i386/compile/INLE i386

>Description:
        lfs_fastvget has two mechanisms by which it retrieves vnodes: it
        either pulls them out of the hash list using ufs_ihashlookup(),
        (using an unlocked access to prevent deadlocking) or creates
        new, locked, vnodes using getnewvnode().

        lfs_fastvget does not make any distinction between these two
        types of inodes; so lfs_markv (the only caller of lfs_fastvget)
        does not VOP_UNLOCK the vnodes when it is done with them, as it
        has no way of knkowing whether they are locked or not, and if
        so, by whom.
>How-To-Repeat:
        Make an LFS, and create/delete/modify files on it until the
        cleaner cleans a few segments.  (You will need the patches
        mentioned above to get this far.)  The process writing to the
        LFS will hang, as will any other process that tries to access
        *any* filesystem.
>Fix:
        My solution simply has lfs_fastvget return whether the retrieved
        vnode should be VOP_UNLOCKed by the caller or not.  A patch follows.

*** lfs_extern.h.dist	Tue Feb 17 18:57:12 1998
--- lfs_extern.h	Tue Feb 17 18:57:31 1998
***************
*** 106,110 ****
  
  /* lfs_syscalls.c */
! int lfs_fastvget __P((struct mount *, ino_t, daddr_t, struct vnode **, struct dinode *));
  struct buf *lfs_fakebuf __P((struct vnode *, int, size_t, caddr_t));
  
--- 106,110 ----
  
  /* lfs_syscalls.c */
! int lfs_fastvget __P((struct mount *, ino_t, daddr_t, struct vnode **, struct dinode *, int *));
  struct buf *lfs_fakebuf __P((struct vnode *, int, size_t, caddr_t));
  
*** lfs_syscalls.c.dist	Mon Jan 12 18:25:35 1998
--- lfs_syscalls.c	Tue Feb 17 19:13:26 1998
***************
*** 108,112 ****
  	daddr_t b_daddr, v_daddr;
  	u_long bsize;
! 	int cnt, error;
  
  	if ((error = suser(p->p_ucred, &p->p_acflag)) != 0)
--- 108,112 ----
  	daddr_t b_daddr, v_daddr;
  	u_long bsize;
! 	int cnt, error, lfs_fastvget_unlock;
  
  	if ((error = suser(p->p_ucred, &p->p_acflag)) != 0)
***************
*** 150,153 ****
--- 150,155 ----
  
  				lfs_writeinode(fs, sp, ip);
+ 				if(lfs_fastvget_unlock)
+ 					VOP_UNLOCK(vp);
  				lfs_vunref(vp);
  			}
***************
*** 176,180 ****
  			if (lfs_fastvget(mntp, blkp->bi_inode, v_daddr, &vp,
  			    blkp->bi_lbn == LFS_UNUSED_LBN ? 
! 			    blkp->bi_bp : NULL)) {
  #ifdef DIAGNOSTIC
  				printf("lfs_markv: VFS_VGET failed (%d)\n",
--- 178,182 ----
  			if (lfs_fastvget(mntp, blkp->bi_inode, v_daddr, &vp,
  			    blkp->bi_lbn == LFS_UNUSED_LBN ? 
! 			    blkp->bi_bp : NULL, &lfs_fastvget_unlock)) {
  #ifdef DIAGNOSTIC
  				printf("lfs_markv: VFS_VGET failed (%d)\n",
***************
*** 449,453 ****
   */
  int
! lfs_fastvget(mp, ino, daddr, vpp, dinp)
  	struct mount *mp;
  	ino_t ino;
--- 451,455 ----
   */
  int
! lfs_fastvget(mp, ino, daddr, vpp, dinp, need_unlock)
  	struct mount *mp;
  	ino_t ino;
***************
*** 455,458 ****
--- 457,461 ----
  	struct vnode **vpp;
  	struct dinode *dinp;
+ 	int *need_unlock;
  {
  	register struct inode *ip;
***************
*** 465,468 ****
--- 468,472 ----
  	ump = VFSTOUFS(mp);
  	dev = ump->um_dev;
+ 	*need_unlock = 0;
  	/*
  	 * This is playing fast and loose.  Someone may have the inode
***************
*** 557,560 ****
--- 561,565 ----
  	VREF(ip->i_devvp);
  	*vpp = vp;
+ 	*need_unlock = 1;
  	return (0);
  }
>Audit-Trail:
>Unformatted: