kern/93905: [possible patch] fdescfs vop_lookup and parent vnode locking

[Kostik Belousov]

>Number:         93905
>Category:       kern
>Synopsis:       [possible patch] fdescfs vop_lookup and parent vnode locking
>Confidential:   no
>Severity:       serious
>Priority:       medium
>Responsible:    freebsd-bugs
>State:          open
>Quarter:        
>Keywords:       
>Date-Required:
>Class:          sw-bug
>Submitter-Id:   current-users
>Arrival-Date:   Mon Feb 27 16:10:11 GMT 2006
>Closed-Date:
>Last-Modified:
>Originator:     Kostik Belousov
>Release:        6-STABLE
>Organization:
-
>Environment:
FreeBSD deviant.kiev.zoral.com.ua 6.1-PRERELEASE FreeBSD 6.1-PRERELEASE #3: Mon Feb 20 13:48:25 EET 2006     root at deviant.kiev.zoral.com.ua:/usr/obj/usr/src/sys/DEVIANT  i386

>Description:
> Is it correct that filesystem-specific vop_lookup is expected to              
> take the directory vnode (dvp) locked, and shall return with dvp locked ?     
> vnode_if.src specifies this behaviour as dependent on the flags in the cnp    
> arguments. My reading of the sources suggest that the statement in the first  
> sentence is true, and note from vnode_if.src applies to                       
> sys/kern/vfs_lookup.c:lookup() procedure itself.                              
>                                                                               
> If it is true, the fdescfs break this rule, dropping the lock on the dvp. See 
> sys/fs/fdescfs/fdesc_vnops.c, line 196. As result, having fdescfs mounted,    
> most accesses to files on it cause warning from the lockmgr,
> lockmgr: thread XXX unlocking unheld lock

> I did testing with the attached patch
> and do not see the warnings anymore. The system does not deadlock.
>How-To-Repeat:
mount the fdescfs on /dev/fd and do find /dev/fd on the kernel compiled
with INVARIANTS
>Fix:
Index: sys/fs/fdescfs/fdesc_vnops.c                                           
===================================================================           
RCS file: /usr/local/arch/ncvs/src/sys/fs/fdescfs/fdesc_vnops.c,v             
retrieving revision 1.100                                                     
diff -u -r1.100 fdesc_vnops.c                                                 
--- sys/fs/fdescfs/fdesc_vnops.c      10 Aug 2005 07:08:14 -0000      1.100   
+++ sys/fs/fdescfs/fdesc_vnops.c      3 Feb 2006 14:35:11 -0000               
@@ -193,11 +193,9 @@                                                          
              goto bad;                                                       
      }                                                                       
                                                                              
-     VOP_UNLOCK(dvp, 0, td);                                                 
      if (cnp->cn_namelen == 1 && *pname == '.') {                            
              *vpp = dvp;                                                     
              VREF(dvp);                                                      
-             vn_lock(dvp, LK_EXCLUSIVE | LK_RETRY, td);                      
              return (0);                                                     
      }                                                                       
                                                                              
@@ -228,12 +226,12 @@                                                         
      if (error)                                                              
              goto bad;                                                       
      VTOFDESC(fvp)->fd_fd = fd;                                              
-     vn_lock(fvp, LK_EXCLUSIVE | LK_RETRY, td);                              
+     if (fvp != dvp)                                                         
+             vn_lock(fvp, LK_EXCLUSIVE | LK_RETRY, td);                      
      *vpp = fvp;                                                             
      return (0);                                                             
                                                                              
 bad:                                                                         
-     vn_lock(dvp, LK_EXCLUSIVE | LK_RETRY, td);                              
      *vpp = NULL;                                                            
      return (error);                                                         
 }                                                                            
>Release-Note:
>Audit-Trail:
>Unformatted: