[OpenAFS-devel] afs_dentry_iput patch (linux)

Chas Williams chas@cmf.nrl.navy.mil
Mon, 29 Jan 2001 10:26:38 -0500


ok, here is a more 'complete' patch for the 'bad iput' problem that
i was seeing.  i also 'fixed' a couple other problems.

. afs_dentry_iput is essentially osi_iput, so it now calls osi_iput
  it wasnt locking before checking i_count (and osi_iput didnt either)
  osi_iput now AFS_GLOCK's.  to do this afs_delete_inode was split
  into afs_delete_inode (which does the locking) and osi_clear_inode
  before osi_iput called afs_delete_inode, which since it wanted
  to hold AFS_GLOCK, osi_iput couldnt call AFS_GLOCK.  this seems
  to have cured the problem i saw (running multiple 'ls -R &' would
  cause a bad iput w/o fail) and i hope it fixes it otherwise.
  its possible vc->lock should be held inside osi_clear_inode.

. removed osi_notify_change (i added during 2.4.0 development) and
  now i just call inode_change_ok/inode_setattr.  the 2.4.0 
  notify_change checks the dentry parent (which doenst exist for
  the cache inodes)

. moved the locks around inside writepage, writepage_sync, commit_write.
  the guy next door complained about pauses while running afs.  i think
  this addresses those issues.  apparently you only need to lock_kernel()
  during commit_write().  in order to get lock_kernel() inside AFS_GLOCK()
  the AFS_GLOCK() was moved out of writepage_sync and into writepage
  and commit_write

my testing of this mostly consists of rebuilding afs from scratch (which
seemed to work fine)  this set of patches does not address 2.4.1 issues.
i wonder if the ifdef AFS_LINUX24_ENV should be changed to be 
if LINUX_VERSION >= KERNEL_VERSION(2,4,0) in preperation for the 
2.4.1 kernel changes.

Index: osi_alloc.c
===================================================================
RCS file: /afs/cmf/project/cvsroot/openafs/src/afs/LINUX/osi_alloc.c,v
retrieving revision 1.1.1.3
retrieving revision 1.5
diff -u -u -r1.1.1.3 -r1.5
Index: osi_cred.c
===================================================================
RCS file: /afs/cmf/project/cvsroot/openafs/src/afs/LINUX/osi_cred.c,v
retrieving revision 1.1.1.3
retrieving revision 1.3
diff -u -u -r1.1.1.3 -r1.3
Index: osi_file.c
===================================================================
RCS file: /afs/cmf/project/cvsroot/openafs/src/afs/LINUX/osi_file.c,v
retrieving revision 1.1.1.3
retrieving revision 1.5
diff -u -u -r1.1.1.3 -r1.5
--- osi_file.c	2001/01/23 21:24:04	1.1.1.3
+++ osi_file.c	2001/01/28 19:35:00	1.5
@@ -104,33 +104,6 @@
       return 0;
   }
 
-#if defined(AFS_LINUX24_ENV)
-int osi_notify_change(struct dentry * dentry, struct iattr * attr)
-{
-    struct inode *inode = dentry->d_inode;
-    int error;
-    time_t now = CURRENT_TIME;
-    unsigned int ia_valid = attr->ia_valid;
-
-    attr->ia_ctime = now;
-    if (!(ia_valid & ATTR_ATIME_SET))
-	attr->ia_atime = now;
-    if (!(ia_valid & ATTR_MTIME_SET))
-	attr->ia_mtime = now;
-
-    lock_kernel();
-    if (inode && inode->i_op && inode->i_op->setattr)
-	error = inode->i_op->setattr(dentry, attr);
-    else {
-	error = inode_change_ok(inode, attr);
-	if (!error)
-	    inode_setattr(inode, attr);
-    }
-    unlock_kernel();
-    return error;
-}
-#endif
-
 osi_UFSTruncate(afile, asize)
     register struct osi_file *afile;
     afs_int32 asize; {
@@ -150,13 +123,18 @@
     MObtainWriteLock(&afs_xosi,321);    
     AFS_GUNLOCK();
     down(&inode->i_sem);
-#if defined(AFS_LINUX24_ENV)
-    newattrs.ia_size = asize;
-    newattrs.ia_valid = ATTR_SIZE | ATTR_CTIME;
-    code = osi_notify_change(&afile->dentry, &newattrs);
-#else
     inode->i_size = newattrs.ia_size = asize;
     newattrs.ia_valid = ATTR_SIZE | ATTR_CTIME;
+#if defined(AFS_LINUX24_ENV)
+    newattrs.ia_ctime = CURRENT_TIME;
+
+    /* avoid notify_change() since it wants to update dentry->d_parent */
+    lock_kernel();
+    code = inode_change_ok(inode, &newattrs);
+    if (!code)
+	inode_setattr(inode, &newattrs);
+    unlock_kernel();
+#else
     if (inode->i_sb->s_op && inode->i_sb->s_op->notify_change) {
 	code = inode->i_sb->s_op->notify_change(&afile->dentry, &newattrs);
     }
Index: osi_misc.c
===================================================================
RCS file: /afs/cmf/project/cvsroot/openafs/src/afs/LINUX/osi_misc.c,v
retrieving revision 1.1.1.3
retrieving revision 1.6
diff -u -u -r1.1.1.3 -r1.6
--- osi_misc.c	2001/01/23 21:24:06	1.1.1.3
+++ osi_misc.c	2001/01/29 14:25:36	1.6
@@ -15,6 +15,9 @@
 #include "../afs/sysincludes.h"
 #include "../afs/afsincludes.h"
 #include "../afs/afs_stats.h"
+#if defined(AFS_LINUX24_ENV)
+#include "../h/smp_lock.h"
+#endif
 
 char *crash_addr = 0; /* Induce an oops by writing here. */
 
@@ -313,16 +316,37 @@
     }
 }
 
+void osi_clear_inode(struct inode *ip)
+{
+    cred_t *credp = crref();
+    struct vcache *vc = (struct vcache*)ip;
+
+#if defined(AFS_LINUX24_ENV)
+    if (atomic_read(&ip->i_count) > 1)
+#else
+    if (ip->i_count > 1)
+#endif
+        printf("afs_put_inode: ino %d (0x%x) has count %d\n", ip->i_ino, ip);
+
+    afs_InactiveVCache(vc, credp);
+#if defined(AFS_LINUX24_ENV)
+    atomic_set(&ip->i_count, 0);
+#else
+    ip->i_count = 0;
+#endif
+    ip->i_nlink = 0; /* iput checks this after calling this routine. */
+    crfree(credp);
+}
+
 /* iput an inode. Since we still have a separate inode pool, we don't want
  * to call iput on AFS inodes, since they would then end up on Linux's
  * inode_unsed list.
  */
 void osi_iput(struct inode *ip)
 {
-    extern void afs_delete_inode(struct inode *ip);
     extern struct vfs *afs_globalVFS;
 
-    
+    AFS_GLOCK();
 #if defined(AFS_LINUX24_ENV)
     if (atomic_read(&ip->i_count) == 0 || atomic_read(&ip->i_count) & 0xffff0000) {
 #else
@@ -343,9 +367,11 @@
 	ip->i_count --;
 	if (!ip->i_count)
 #endif
-	    afs_delete_inode(ip);
+	    osi_clear_inode(ip);
+        AFS_GUNLOCK();
     }
     else { 
+        AFS_GUNLOCK();
 	iput(ip);
     }
 }
Index: osi_module.c
===================================================================
RCS file: /afs/cmf/project/cvsroot/openafs/src/afs/LINUX/osi_module.c,v
retrieving revision 1.1.1.3
retrieving revision 1.3
diff -u -u -r1.1.1.3 -r1.3
Index: osi_sleep.c
===================================================================
RCS file: /afs/cmf/project/cvsroot/openafs/src/afs/LINUX/osi_sleep.c,v
retrieving revision 1.1.1.3
retrieving revision 1.3
diff -u -u -r1.1.1.3 -r1.3
Index: osi_vfs.h
===================================================================
RCS file: /afs/cmf/project/cvsroot/openafs/src/afs/LINUX/osi_vfs.h,v
retrieving revision 1.1.1.3
retrieving revision 1.4
diff -u -u -r1.1.1.3 -r1.4
Index: osi_vfsops.c
===================================================================
RCS file: /afs/cmf/project/cvsroot/openafs/src/afs/LINUX/osi_vfsops.c,v
retrieving revision 1.1.1.3
retrieving revision 1.5
diff -u -u -r1.1.1.3 -r1.5
--- osi_vfsops.c	2001/01/23 21:24:08	1.1.1.3
+++ osi_vfsops.c	2001/01/28 13:46:32	1.5
@@ -231,36 +231,18 @@
  * If we use the common inode pool, we'll need to set i_nlink to 0 here.
  * That will trigger the call to delete routine.
  */
+
 void afs_delete_inode(struct inode *ip)
 {
-    cred_t *credp = crref();
     struct vcache *vc = (struct vcache*)ip;
 
     AFS_GLOCK();
-#if defined(AFS_LINUX24_ENV)
-    lock_kernel();
-    if (atomic_read(&ip->i_count) > 1)
-#else
-    if (ip->i_count > 1)
-#endif
-	printf("afs_put_inode: ino %d (0x%x) has count %d\n", ip->i_ino, ip);
-
     ObtainWriteLock(&vc->lock, 504);
-    afs_InactiveVCache(vc, credp);
-#if defined(AFS_LINUX24_ENV)
-    atomic_set(&ip->i_count, 0);
-#else
-    ip->i_count = 0;
-#endif
-    ip->i_nlink = 0; /* iput checks this after calling this routine. */
+    osi_clear_inode(ip);
     ReleaseWriteLock(&vc->lock);
-
-#ifdef AFS_LINUX24_ENV
-    unlock_kernel();
-#endif
     AFS_GUNLOCK();
-    crfree(credp);
 }
+
 
 /* afs_put_super
  * Called from unmount to release super_block. */
Index: osi_vnodeops.c
===================================================================
RCS file: /afs/cmf/project/cvsroot/openafs/src/afs/LINUX/osi_vnodeops.c,v
retrieving revision 1.1.1.3
retrieving revision 1.11
diff -u -u -r1.1.1.3 -r1.11
--- osi_vnodeops.c	2001/01/23 21:24:09	1.1.1.3
+++ osi_vnodeops.c	2001/01/29 13:12:15	1.11
@@ -161,7 +161,6 @@
 
     AFS_GLOCK();
     AFS_STATCNT(afs_readdir);
-
     code = afs_InitReq(&treq, credp);
     crfree(credp);
     if (code) {
@@ -709,27 +708,7 @@
 /* afs_dentry_iput */
 static void afs_dentry_iput(struct dentry *dp, struct inode *ip)
 {
-#if defined(AFS_LINUX24_ENV)
-    if (atomic_read(&ip->i_count) == 0 || atomic_read(&ip->i_count) & 0xffff0000) {
-#else
-    if (ip->i_count == 0 || ip->i_count & 0xffff0000) {
-#endif
-	osi_Panic("Bad refCount %d on inode 0x%x\n",
-#if defined(AFS_LINUX24_ENV)
-		  atomic_read(&ip->i_count), ip);
-#else
-		  ip->i_count, ip);
-#endif
-    }
-#if defined(AFS_LINUX24_ENV)
-    atomic_dec(&ip->i_count);
-    if (!atomic_read(&ip->i_count)) {
-#else
-    ip->i_count --;
-    if (!ip->i_count) {
-#endif
-	afs_delete_inode(ip);
-    }
+    osi_iput(ip);
 }
 
 #if defined(AFS_LINUX24_ENV)
@@ -824,7 +803,6 @@
     const char *comp = dp->d_name.name;
     AFS_GLOCK();
     code = afs_lookup((struct vcache *)dip, comp, &vcp, credp);
-
     if (vcp) {
 	struct inode *ip = (struct inode*)vcp;
 	/* Reset ops if symlink or directory. */
@@ -956,7 +934,6 @@
     vattr.va_mask = ATTR_MODE;
     vattr.va_mode = mode;
     code = afs_mkdir((struct vcache*)dip, name, &vattr, &tvcp, credp);
-
     if (tvcp) {
 	tvcp->v.v_op = &afs_dir_iops;
 #if defined(AFS_LINUX24_ENV)
@@ -1096,6 +1073,7 @@
     }
     else {
 	name[code] = '\0';
+
 	res = lookup_dentry(name, basep, follow);
     }
 
@@ -1140,11 +1118,11 @@
     setup_uio(&tuio, &iovec, (char*)address, pp->offset, PAGESIZE,
 	      UIO_READ, AFS_UIOSYS);
 #endif
-#ifdef AFS_LINUX24_ENV
+#if defined(AFS_LINUX24_ENV)
     lock_kernel();
 #endif
     code = afs_rdwr((struct vcache*)ip, &tuio, UIO_READ, 0, credp);
-#ifdef AFS_LINUX24_ENV
+#if defined(AFS_LINUX24_ENV)
     unlock_kernel();
 #endif
 
@@ -1153,6 +1131,9 @@
 	    memset((void*)(address+(PAGESIZE-tuio.uio_resid)), 0,
 		   tuio.uio_resid);
 #if defined(AFS_LINUX24_ENV)
+#ifndef __powerpc__
+        flush_dcache_page(pp);
+#endif
         SetPageUptodate(pp);
 #else
 	set_bit(PG_uptodate, &pp->flags);
@@ -1163,8 +1144,8 @@
     UnlockPage(pp);
 #else
     clear_bit(PG_locked, &pp->flags);
-#endif
     wake_up(&pp->wait);
+#endif
     free_page(address);
 
     crfree(credp);
@@ -1198,7 +1179,9 @@
     if (pp->index >= end_index+1 || !offset)
 	return -EIO;
 do_it:
+    AFS_GLOCK();
     status = afs_linux_writepage_sync(inode, pp, 0, offset);
+    AFS_GUNLOCK();
     SetPageUptodate(pp);
     UnlockPage(pp);
     /* kunmap(pp); */
@@ -1262,8 +1245,6 @@
     int f_flags = 0;
 
     credp = crref();
-    AFS_GLOCK();
-    lock_kernel();
     afs_Trace4(afs_iclSetp, CM_TRACE_UPDATEPAGE, ICL_TYPE_POINTER, vcp,
               ICL_TYPE_POINTER, pp,
               ICL_TYPE_INT32, atomic_read(&pp->count),
@@ -1282,8 +1263,6 @@
               ICL_TYPE_INT32, atomic_read(&pp->count),
               ICL_TYPE_INT32, code);
 
-    unlock_kernel();
-    AFS_GUNLOCK();
     crfree(credp);
 
     return code;
@@ -1307,13 +1286,13 @@
 			 unsigned long offset,
 			 unsigned int count, int sync)
 {
-    struct vcache *vcp = (struct vcache *)FILE_INODE(fp);
+    struct vcache *vcp = (struct vcache *) FILE_INODE(fp);
     u8 *page_addr = (u8*) afs_linux_page_address(pp);
     int code = 0;
     cred_t *credp;
     uio_t tuio;
     struct iovec iovec;
-    
+
     set_bit(PG_locked, &pp->flags);
 
     credp = crref();
@@ -1348,9 +1327,11 @@
 {
     long status;
 
-    /* lock_kernel(); */
+    AFS_GLOCK();
+    lock_kernel();
     status = afs_linux_updatepage(file, page, offset, to-offset);
-    /* unlock_kernel(); */
+    unlock_kernel();
+    AFS_GUNLOCK();
     kunmap(page);
 
     return status;