[OpenAFS-devel] porting openafs to another linux - problems w ith vRefcount type.. [grand.central.org #638]

Nickolai Zeldovich kolya@MIT.EDU
Fri, 05 Oct 2001 18:52:45 -0400


> Not all decrements should become VN_RELE's. VN_RELE calls into the
> kernel, and may do stuff when the refcount drops to 0.

Point taken.  I was originally thinking that making them VN_RELE's
might potentially fix some vnode leaks, but you're probably right..

Here's a patch that should take care of the atomic_t lossage on
Linux.  The only weird case is afs_pioctl.c, with the nested
#ifdef's.  As far as I can tell, the VREFCOUNT_INC() is only
used on AIX.  Could someone check what VNOP_HOLD() does on AIX,
and if it's safe to call on vref=0 vnodes?  If so, we should
probably garbage-collect VREFCOUNT_INC, and collapse the multiple
#ifdef's into a single one for DARWIN/FBSD.

-- kolya

--- src/afs/afs.h	2001/08/06 23:41:26	1.9
+++ src/afs/afs.h	2001/10/05 22:43:18
@@ -522,6 +522,18 @@
 #define vrefCount   v.v_usecount
 #endif /* AFS_FBSD_ENV */
 
+#ifdef AFS_LINUX24_ENV
+#define VREFCOUNT(v)		atomic_read(&((vnode_t *) v)->v_count)
+#define VREFCOUNT_SET(v, c)	atomic_set(&((vnode_t *) v)->v_count, c)
+#define VREFCOUNT_DEC(v)	atomic_dec(&((vnode_t *) v)->v_count)
+#define VREFCOUNT_INC(v)	atomic_inc(&((vnode_t *) v)->v_count)
+#else
+#define VREFCOUNT(v)		((v)->vrefCount)
+#define VREFCOUNT_SET(v, c)	(v)->vrefCount = c;
+#define VREFCOUNT_DEC(v)	(v)->vrefCount--;
+#define VREFCOUNT_INC(v)	(v)->vrefCount++;
+#endif
+
 #define	AFS_MAXDV   0x7fffffff	    /* largest dataversion number */
 #define	AFS_NOTRUNC 0x7fffffff	    /* largest dataversion number */
 
--- src/afs/afs_callback.c	2001/09/11 19:28:56	1.7
+++ src/afs/afs_callback.c	2001/10/05 22:43:18
@@ -157,7 +157,7 @@
     a_result->DataVersion = hgetlo(tvc->m.DataVersion);
     a_result->callback = afs_data_pointer_to_int32(tvc->callback);		/* XXXX Now a pointer; change it XXXX */
     a_result->cbExpires = tvc->cbExpires;
-    a_result->refCount = tvc->vrefCount;
+    a_result->refCount = VREFCOUNT(tvc);
     a_result->opens = tvc->opens;
     a_result->writers = tvc->execsOrWriters;
     a_result->mvstat = tvc->mvstat;

--- src/afs/afs_pioctl.c	2001/08/08 00:03:28	1.22
+++ src/afs/afs_pioctl.c	2001/10/05 22:43:18
@@ -2560,13 +2560,13 @@
     for(i = 0; i < VCSIZE; i++) {
 	for(tvc = afs_vhashT[i]; tvc; tvc=tvc->hnext) {
 	    if (tvc->fid.Fid.Volume == volume && tvc->fid.Cell == cell) {
-#if	defined(AFS_SGI_ENV) || defined(AFS_ALPHA_ENV)  || defined(AFS_SUN5_ENV)  || defined(AFS_HPUX_ENV)
+#if	defined(AFS_SGI_ENV) || defined(AFS_ALPHA_ENV)  || defined(AFS_SUN5_ENV)  || defined(AFS_HPUX_ENV) || defined(AFS_LINUX20_ENV)
 		VN_HOLD((struct vnode *)tvc);
 #else
 #if defined(AFS_DARWIN_ENV) || defined(AFS_FBSD_ENV)
 		osi_vnhold(tvc, 0);
 #else
-		tvc->vrefCount++;
+		VREFCOUNT_INC(tvc);
 #endif
 #endif
 		ReleaseReadLock(&afs_xvcache);

--- src/afs/afs_util.c	2001/07/12 19:58:15	1.5
+++ src/afs/afs_util.c	2001/10/05 22:43:18
@@ -180,9 +180,9 @@
 	for(i=0;i<VCSIZE;i++) {
 	    for(tvc = afs_vhashT[i]; tvc; tvc=tvc->hnext) {
 #ifdef	AFS_OSF_ENV
-		if (tvc->vrefCount > 1)
+		if (VREFCOUNT(tvc) > 1)
 #else	/* AFS_OSF_ENV */
-		if (tvc->vrefCount)
+		if (VREFCOUNT(tvc))
 #endif
 		    afs_warn("Stat cache entry at %x is held\n", tvc);
 		if (CheckLock(&tvc->lock))

--- src/afs/afs_vcache.c	2001/08/08 00:03:28	1.9
+++ src/afs/afs_vcache.c	2001/10/05 22:43:18
@@ -213,7 +213,7 @@
     /* This should put it back on the vnode free list since usecount is 1 */
     afs_vcount--;
     vSetType(avc, VREG);
-    if (avc->vrefCount > 0) {
+    if (VREFCOUNT(avc) > 0) {
         VN_UNLOCK((struct vnode *)avc);
         AFS_RELE((struct vnode *)avc);
     } else {
@@ -582,7 +582,7 @@
 	    if (tvc == afs_globalVp)
 		continue;
 
-	    if ( tvc->vrefCount && tvc->opens == 0 ) {
+	    if ( VREFCOUNT(tvc) && tvc->opens == 0 ) {
 		struct inode *ip = (struct inode*)tvc;
 		if (list_empty(&ip->i_dentry)) {
 		    vn --;
@@ -656,10 +656,10 @@
 		 refpanic ("Exceeded pool of AFS vnodes(VLRU cycle?)");
 	    else if (QNext(uq) != tq)
 		 refpanic ("VLRU inconsistent");
-	    else if (tvc->vrefCount < 1) 
+	    else if (VREFCOUNT(tvc) < 1) 
 		 refpanic ("refcnt 0 on VLRU");
 
-	    if ( tvc->vrefCount == 1   &&   tvc->opens == 0 
+	    if ( VREFCOUNT(tvc) == 1   &&   tvc->opens == 0 
 		&& (tvc->states & CUnlinkedDel) == 0) {
 		code = afs_FlushVCache(tvc, &fv_slept);
 		if (code == 0) {
@@ -711,7 +711,7 @@
 
 #ifdef AFS_DARWIN_ENV
 	   if (tvc->opens == 0 && ((tvc->states & CUnlinkedDel) == 0) &&
-                tvc->vrefCount == 1 && UBCINFOEXISTS(&tvc->v)) {
+                VREFCOUNT(tvc) == 1 && UBCINFOEXISTS(&tvc->v)) {
                osi_VM_TryReclaim(tvc, &fv_slept);
                if (fv_slept) {
                   uq = VLRU.prev;
@@ -720,7 +720,7 @@
                }
             }
 #endif
-	   if (tvc->vrefCount == 0 && tvc->opens == 0
+	   if (VREFCOUNT(tvc) == 0 && tvc->opens == 0
 	       && (tvc->states & CUnlinkedDel) == 0) {
 		code = afs_FlushVCache(tvc, &fv_slept);
 		if (code == 0) {
@@ -815,7 +815,7 @@
     /* Hold it for the LRU (should make count 2) */
     VN_HOLD((struct vnode *)tvc);
 #else	/* AFS_OSF_ENV */
-    tvc->vrefCount = 1;	/* us */
+    VREFCOUNT_SET(tvc, 1);	/* us */
 #endif	/* AFS_OSF_ENV */
 #ifdef	AFS_AIX32_ENV
     LOCK_INIT(&tvc->pvmlock, "vcache pvmlock");
@@ -1081,7 +1081,7 @@
 		/*
 		 * That's because if we come in via the CUnlinkedDel bit state path we'll be have 0 refcnt
 		 */
-		osi_Assert(tvc->vrefCount > 0);
+		osi_Assert(VREFCOUNT(tvc) > 0);
 		AFS_RWLOCK((vnode_t *)tvc, VRWLOCK_WRITE);
 #endif
 		ObtainWriteLock(&tvc->lock,52);
@@ -1136,7 +1136,7 @@
 		AFS_FAST_RELE(tvc);
 		if (didCore) {
 #ifdef	AFS_GFS_ENV
-		    tvc->vrefCount--;
+		    VREFCOUNT_DEC(tvc);
 #else
 		    AFS_RELE((struct vnode *)tvc);
 #endif
@@ -1145,7 +1145,7 @@
 		}
 	    }	       
 #ifdef AFS_DARWIN_ENV
-            if (tvc->vrefCount == 1 && UBCINFOEXISTS(&tvc->v)) {
+            if (VREFCOUNT(tvc) == 1 && UBCINFOEXISTS(&tvc->v)) {
 		if (tvc->opens) panic("flushactive open, hasubc, but refcnt 1");
 		osi_VM_TryReclaim(tvc,0);
 	    }
@@ -2694,7 +2694,7 @@
 		    vms_delete(tvc->segid);
 		    AFS_GLOCK();
 		    tvc->segid = tvc->vmh = NULL;
-		    if (tvc->vrefCount) osi_Panic("flushVcache: vm race");
+		    if (VREFCOUNT(tvc)) osi_Panic("flushVcache: vm race");
 		}
 		if (tvc->credp) {
 		    crfree(tvc->credp);

--- src/afs/LINUX/osi_vfs.hin	2001/10/01 22:59:01	1.1
+++ src/afs/LINUX/osi_vfs.hin	2001/10/05 22:43:18
@@ -57,7 +57,11 @@
 #define FSYNC O_SYNC
 
 #define VTOI(V)  ((struct inode*)V)
+#ifdef AFS_LINUX24_ENV
+#define VN_HOLD(V) atomic_inc(&((vnode_t*)V)->i_count)
+#else
 #define VN_HOLD(V) ((vnode_t*)V)->i_count++;
+#endif
 #define VN_RELE(V) osi_iput((struct inode *)V);
 #define VFS_STATFS(V, S) ((V)->s_op->statfs)((V), (S), sizeof(*(S)))
 
--- src/afs/LINUX/osi_vm.c	2001/07/12 19:58:21	1.8
+++ src/afs/LINUX/osi_vm.c	2001/10/05 22:43:18
@@ -43,7 +43,7 @@
 {
     struct inode *ip = (struct inode*)avc;
 
-    if (avc->vrefCount != 0)
+    if (VREFCOUNT(avc) != 0)
 	return EBUSY;
 
     if (avc->opens != 0)

--- src/afs/VNOPS/afs_vnop_lookup.c	2001/09/27 17:37:49	1.16
+++ src/afs/VNOPS/afs_vnop_lookup.c	2001/10/05 22:43:18
@@ -969,7 +969,7 @@
 	*avcp = tvc;
 	code = (tvc ? 0 : ENOENT);
 	hit = 1;
-	if (tvc && !tvc->vrefCount) {
+	if (tvc && !VREFCOUNT(tvc)) {
 	    osi_Panic("TT1");
 	}
 	if (code) {
@@ -1005,7 +1005,7 @@
 	code = 0;
 	*avcp = tvc = adp;
 	hit = 1;
-	if (adp && !adp->vrefCount) {
+	if (adp && !VREFCOUNT(adp)) {
 	    osi_Panic("TT2");
 	}
 	goto done;
@@ -1251,7 +1251,7 @@
 	    }
 	}
 	*avcp = tvc;
-	if (tvc && !tvc->vrefCount) {
+	if (tvc && !VREFCOUNT(tvc)) {
 	    osi_Panic("TT3");
 	}
 	code = 0;

--- src/afs/VNOPS/afs_vnop_remove.c	2001/07/12 19:58:22	1.4
+++ src/afs/VNOPS/afs_vnop_remove.c	2001/10/05 22:43:19
@@ -317,12 +317,12 @@
     osi_dnlc_remove ( adp, aname, tvc);
     if (tvc) afs_symhint_inval(tvc);
 
-    Tadp1 = adp; Tadpr = adp->vrefCount; Ttvc = tvc; Tnam = aname; Tnam1 = 0;
-    if (tvc) Ttvcr = tvc->vrefCount;
+    Tadp1 = adp; Tadpr = VREFCOUNT(adp); Ttvc = tvc; Tnam = aname; Tnam1 = 0;
+    if (tvc) Ttvcr = VREFCOUNT(tvc);
 #ifdef	AFS_AIX_ENV
-    if (tvc && (tvc->vrefCount > 2) && tvc->opens > 0 && !(tvc->states & CUnlinked)) {
+    if (tvc && (VREFCOUNT(tvc) > 2) && tvc->opens > 0 && !(tvc->states & CUnlinked)) {
 #else
-    if (tvc && (tvc->vrefCount > 1) && tvc->opens > 0 && !(tvc->states & CUnlinked)) {
+    if (tvc && (VREFCOUNT(tvc) > 1) && tvc->opens > 0 && !(tvc->states & CUnlinked)) {
 #endif
 	char *unlname = newname();
 
--- src/afs/VNOPS/afs_vnop_write.c	2001/09/20 05:08:20	1.10
+++ src/afs/VNOPS/afs_vnop_write.c	2001/10/05 22:43:19
@@ -909,7 +909,7 @@
 	ReleaseWriteLock(&avc->lock);
     }
 #ifdef	AFS_OSF_ENV
-    if ((avc->vrefCount <= 2) && (avc->states & CUnlinked)) {
+    if ((VREFCOUNT(avc) <= 2) && (avc->states & CUnlinked)) {
 	afs_remunlink(avc, 1);	/* ignore any return code */
     }
 #endif