[OpenAFS-devel] osi_vfsops.c / afs_put_inode (LINUX) - editing bug?

[chas williams - CONTRACTOR]

In message <43FAD0C0.1080401@pclella.cern.ch>,Rainer Toebbicke writes:
>The code in 1.4.1-rc7 looks a bit odd:
>
>     if (VREFCOUNT(vcp) == 2) {
>         if (VREFCOUNT(vcp) == 2)
>             afs_InactiveVCache(vcp, NULL);

this is the result of a poor patch that apparently didnt remove
everything cleanly.  at one point, i was checking outside AFS_GLOCK
to see if the refcount was 2 before taking AFS_GLOCK (since it 
typically isnt going to be 2).  this is racy on multiple cpus as
others have pointed out.  so the only choice is to always take
the lock.

>More seriously however, afs_InactiveVCache() through the subsequently 
>called afs_InvalidateAllSegments() needs the vnode to be locked. I'm 
>worried that given that this is an upcall from the kernel this might 
>not always be the case, e.g. when invoked through afs_linux_lookup().

yep, this is true.  but...

>(Besides I still believe this function can be called with the global 
>lock already held and thus deadlock, but this is another story.)

you are right about this.  i never wanted to constantly grab the
AFS_GLOCK in ->put_inode.  i now believe that this can simply be moved
entirely to ->d_iput.  when ->d_iput is called, the linux vfs layer has
no more references to the file -- it's "done".   i am using the attached
with no ill effect.  as you mentioned, it probably should grab a write
lock though.

Index: src/afs/LINUX/osi_vfsops.c
===================================================================
RCS file: /cvs/openafs/src/afs/LINUX/osi_vfsops.c,v
retrieving revision 1.29.2.12
diff -u -u -r1.29.2.12 osi_vfsops.c
--- src/afs/LINUX/osi_vfsops.c	29 Nov 2005 03:20:28 -0000	1.29.2.12
+++ src/afs/LINUX/osi_vfsops.c	24 Feb 2006 19:04:46 -0000
@@ -332,25 +332,6 @@
 #endif
 }
 
-/* afs_put_inode
- * Linux version of inactive.  When refcount == 2, we are about to
- * decrement to 1 and the only reference remaining should be for
- * the VLRU
- */
-
-static void
-afs_put_inode(struct inode *ip)
-{
-    struct vcache *vcp = VTOAFS(ip);
-
-    AFS_GLOCK();
-    if (VREFCOUNT(vcp) == 2) {
-	if (VREFCOUNT(vcp) == 2)
-	    afs_InactiveVCache(vcp, NULL);
-    }
-    AFS_GUNLOCK();
-}
-
 /* afs_put_super
  * Called from unmount to release super_block. */
 static void
@@ -436,7 +417,6 @@
   .destroy_inode =	afs_destroy_inode,
 #endif
   .clear_inode =	afs_clear_inode,
-  .put_inode =		afs_put_inode,
   .put_super =		afs_put_super,
   .statfs =		afs_statfs,
 #if !defined(AFS_LINUX24_ENV)
Index: src/afs/LINUX/osi_vnodeops.c
===================================================================
RCS file: /cvs/openafs/src/afs/LINUX/osi_vnodeops.c,v
retrieving revision 1.81.2.40
diff -u -u -r1.81.2.40 osi_vnodeops.c
--- src/afs/LINUX/osi_vnodeops.c	11 Jan 2006 21:38:30 -0000	1.81.2.40
+++ src/afs/LINUX/osi_vnodeops.c	24 Feb 2006 19:04:46 -0000
@@ -788,8 +788,7 @@
     struct vcache *vcp = VTOAFS(ip);
 
     AFS_GLOCK();
-    if (vcp->states & CUnlinked)
-	(void) afs_InactiveVCache(vcp, NULL);
+    (void) afs_InactiveVCache(vcp, NULL);
     AFS_GUNLOCK();
 
     iput(ip);