[PATCH/RFC] Replacing iget4/read_inode2 with icreate

[PATCH/RFC] Replacing iget4/read_inode2 with icreate

[Jan Harkes]

This patch replaces iget4/read_inode2 with icreate. It is pretty much
the XFS icreate + a fix for the init race. Modified both Coda and
ReiserFS to use icreate and do their own inode initialization.

I tested it for Coda and it seems to be stable. Can't reproduce the race
on the SMP box here. I'd like to know if I made the right ReiserFS
changes.

Jan

Patch is against 2.5.11,

diff -urN orig/fs/Makefile icreate3/fs/Makefile
--- orig/fs/Makefile	Mon Apr 29 14:10:14 2002
+++ icreate3/fs/Makefile	Mon Apr 29 15:28:02 2002
@@ -7,7 +7,7 @@
 
 O_TARGET := fs.o
 
-export-objs :=	filesystems.o open.o dcache.o buffer.o bio.o
+export-objs :=	filesystems.o open.o dcache.o buffer.o bio.o inode.o
 mod-subdirs :=	nls
 
 obj-y :=	open.o read_write.o devices.o file_table.o buffer.o \
diff -urN orig/fs/coda/cnode.c icreate3/fs/coda/cnode.c
--- orig/fs/coda/cnode.c	Tue Apr  2 14:36:09 2002
+++ icreate3/fs/coda/cnode.c	Mon Apr 29 17:06:37 2002
@@ -25,7 +25,7 @@
 	return 1;
 }
 
-static int coda_inocmp(struct inode *inode, unsigned long ino, void *opaque)
+static int coda_inocmp(struct inode *inode, void *opaque)
 {
 	return (coda_fideq((ViceFid *)opaque, &(ITOC(inode)->c_fid)));
 }
@@ -55,26 +55,30 @@
                 init_special_inode(inode, inode->i_mode, attr->va_rdev);
 }
 
+int coda_init_inode(struct inode *inode, void *data)
+{
+    ITOC(inode)->c_fid = *(ViceFid *)data;
+    return 0;
+}
+
 struct inode * coda_iget(struct super_block * sb, ViceFid * fid,
 			 struct coda_vattr * attr)
 {
 	struct inode *inode;
 	struct coda_inode_info *cii;
+	struct coda_sb_info *sbi = coda_sbp(sb);
 	ino_t ino = coda_f2i(fid);
 
-	inode = iget4(sb, ino, coda_inocmp, fid);
+	inode = icreate(sb, ino, coda_inocmp, coda_init_inode, fid);
 
 	if (!inode)
 		return ERR_PTR(-ENOMEM);
 
-	/* check if the inode is already initialized */
-	cii = ITOC(inode);
-	if (coda_isnullfid(&cii->c_fid))
-		/* new, empty inode found... initializing */
-		cii->c_fid = *fid;
-
-	/* we shouldnt see inode collisions anymore */
-	if (!coda_fideq(fid, &cii->c_fid)) BUG();
+	if (inode->i_state & I_NEW) {
+		cii = ITOC(inode);
+		list_add(&cii->c_cilist, &sbi->sbi_cihead);
+		unlock_new_inode(inode);
+	}
 
 	/* always replace the attributes, type might have changed */
 	coda_fill_inode(inode, attr);
@@ -126,12 +130,16 @@
 	insert_inode_hash(inode);
 }
 
+int coda_fail_inode(struct inode *inode, void *opaque)
+{
+    return -1;
+}
+
 /* convert a fid to an inode. */
 struct inode *coda_fid_to_inode(ViceFid *fid, struct super_block *sb) 
 {
 	ino_t nr;
 	struct inode *inode;
-	struct coda_inode_info *cii;
 
 	if ( !sb ) {
 		printk("coda_fid_to_inode: no sb!\n");
@@ -139,24 +147,13 @@
 	}
 
 	nr = coda_f2i(fid);
-	inode = iget4(sb, nr, coda_inocmp, fid);
-	if ( !inode ) {
-		printk("coda_fid_to_inode: null from iget, sb %p, nr %ld.\n",
-		       sb, (long)nr);
-		return NULL;
-	}
-
-	cii = ITOC(inode);
-
-	/* The inode could already be purged due to memory pressure */
-	if (coda_isnullfid(&cii->c_fid)) {
-		inode->i_nlink = 0;
-		iput(inode);
+	inode = icreate(sb, nr, coda_inocmp, coda_fail_inode, fid);
+	if ( !inode )
 		return NULL;
-	}
 
-	/* we shouldn't see inode collisions anymore */
-	if ( !coda_fideq(fid, &cii->c_fid) ) BUG();
+	/* we should never see newly created inodes because we intentionally
+	 * fail in the initialization callback */
+	BUG_ON(inode->i_state & I_NEW);
 
         return inode;
 }
@@ -165,13 +162,14 @@
 int coda_cnode_makectl(struct inode **inode, struct super_block *sb)
 {
     int error = 0;
+    ViceFid nullfid = {0,0,0};
 
-    *inode = iget(sb, CTL_INO);
-    if ( *inode ) {
+    *inode = icreate(sb, CTL_INO, NULL, NULL, NULL);
+    if ( *inode && ((*inode)->i_state & I_NEW) ) {
 	(*inode)->i_op = &coda_ioctl_inode_operations;
 	(*inode)->i_fop = &coda_ioctl_operations;
 	(*inode)->i_mode = 0444;
-	error = 0;
+	unlock_new_inode(*inode);
     } else { 
 	error = -ENOMEM;
     }
diff -urN orig/fs/coda/inode.c icreate3/fs/coda/inode.c
--- orig/fs/coda/inode.c	Mon Apr 29 14:09:02 2002
+++ icreate3/fs/coda/inode.c	Mon Apr 29 15:55:16 2002
@@ -229,16 +229,9 @@
 	kfree(sbi);
 }
 
-/* all filling in of inodes postponed until lookup */
 static void coda_read_inode(struct inode *inode)
 {
-	struct coda_sb_info *sbi = coda_sbp(inode->i_sb);
-	struct coda_inode_info *cii;
-
-        if (!sbi) BUG();
-
-	cii = ITOC(inode);
-	list_add(&cii->c_cilist, &sbi->sbi_cihead);
+	make_bad_inode(inode);
 }
 
 static void coda_clear_inode(struct inode *inode)
diff -urN orig/fs/inode.c icreate3/fs/inode.c
--- orig/fs/inode.c	Mon Apr 29 14:09:02 2002
+++ icreate3/fs/inode.c	Mon Apr 29 18:49:57 2002
@@ -17,6 +17,7 @@
 #include <linux/swapctl.h>
 #include <linux/prefetch.h>
 #include <linux/locks.h>
+#include <linux/module.h>
 
 /*
  * New inode.c implementation.
@@ -793,7 +794,7 @@
  * by hand after calling find_inode now! This simplifies iunique and won't
  * add any additional branch in the common code.
  */
-static struct inode * find_inode(struct super_block * sb, unsigned long ino, struct list_head *head, find_inode_t find_actor, void *opaque)
+static struct inode * find_inode(struct super_block * sb, unsigned long ino, struct list_head *head, int (*test)(struct inode *, void *), void *data)
 {
 	struct list_head *tmp;
 	struct inode * inode;
@@ -809,7 +810,7 @@
 			continue;
 		if (inode->i_sb != sb)
 			continue;
-		if (find_actor && !find_actor(inode, ino, opaque))
+		if (test && !test(inode, data))
 			continue;
 		break;
 	}
@@ -842,53 +843,59 @@
 	return inode;
 }
 
+void unlock_new_inode(struct inode *inode)
+{
+	BUG_ON(!(inode->i_state & I_NEW));
+	/*
+	 * This is special!  We do not need the spinlock
+	 * when clearing I_LOCK, because we're guaranteed
+	 * that nobody else tries to do anything about the
+	 * state of the inode when it is locked, as we
+	 * just created it (so there can be no old holders
+	 * that haven't tested I_LOCK).
+	 */
+	inode->i_state &= ~(I_LOCK|I_NEW);
+	wake_up(&inode->i_wait);
+}
+
+
 /*
  * This is called without the inode lock held.. Be careful.
  *
  * We no longer cache the sb_flags in i_flags - see fs.h
  *	-- rmk@arm.uk.linux.org
  */
-static struct inode * get_new_inode(struct super_block *sb, unsigned long ino, struct list_head *head, find_inode_t find_actor, void *opaque)
+static struct inode * get_new_inode(struct super_block *sb, unsigned long ino, struct list_head *head, int (*test)(struct inode *, void *), int (*set)(struct inode *, void *), void *data)
 {
 	struct inode * inode;
+	int err = 0;
 
 	inode = alloc_inode(sb);
 	if (inode) {
 		struct inode * old;
+		inode->i_state = I_LOCK|I_NEW;
 
 		spin_lock(&inode_lock);
 		/* We released the lock, so.. */
-		old = find_inode(sb, ino, head, find_actor, opaque);
+		old = find_inode(sb, ino, head, test, data);
 		if (!old) {
-			inodes_stat.nr_inodes++;
-			list_add(&inode->i_list, &inode_in_use);
-			list_add(&inode->i_hash, head);
 			inode->i_ino = ino;
-			inode->i_state = I_LOCK;
+			if (!set || (err = set(inode, data)) == 0) {
+			    inodes_stat.nr_inodes++;
+			    list_add(&inode->i_list, &inode_in_use);
+			    list_add(&inode->i_hash, head);
+			}
 			spin_unlock(&inode_lock);
 
-			/* reiserfs specific hack right here.  We don't
-			** want this to last, and are looking for VFS changes
-			** that will allow us to get rid of it.
-			** -- mason@suse.com 
-			*/
-			if (sb->s_op->read_inode2) {
-				sb->s_op->read_inode2(inode, opaque) ;
-			} else {
-				sb->s_op->read_inode(inode);
+			/* failed to initialize? */
+			if (err) {
+			    destroy_inode(inode);
+			    return NULL;
 			}
 
-			/*
-			 * This is special!  We do not need the spinlock
-			 * when clearing I_LOCK, because we're guaranteed
-			 * that nobody else tries to do anything about the
-			 * state of the inode when it is locked, as we
-			 * just created it (so there can be no old holders
-			 * that haven't tested I_LOCK).
+			/* Return the locked inode with I_NEW set, the
+			 * caller is responsible for filling in the contents
 			 */
-			inode->i_state &= ~I_LOCK;
-			wake_up(&inode->i_wait);
-
 			return inode;
 		}
 
@@ -968,14 +975,18 @@
 	return inode;
 }
 
-
-struct inode *iget4(struct super_block *sb, unsigned long ino, find_inode_t find_actor, void *opaque)
+/*
+ * This is iget4 without the read_inode portion of get_new_inode
+ * the filesystem gets back a new locked and hashed inode and gets
+ * to fill it in before unlocking it via unlock_new_inode().
+ */
+struct inode *icreate(struct super_block *sb, unsigned long ino, int (*test)(struct inode *, void *), int (*set)(struct inode *, void *), void *data)
 {
 	struct list_head * head = inode_hashtable + hash(sb,ino);
 	struct inode * inode;
 
 	spin_lock(&inode_lock);
-	inode = find_inode(sb, ino, head, find_actor, opaque);
+	inode = find_inode(sb, ino, head, test, data);
 	if (inode) {
 		__iget(inode);
 		spin_unlock(&inode_lock);
@@ -984,12 +995,11 @@
 	}
 	spin_unlock(&inode_lock);
 
-	/*
-	 * get_new_inode() will do the right thing, re-trying the search
-	 * in case it had to block at any point.
-	 */
-	return get_new_inode(sb, ino, head, find_actor, opaque);
+	return get_new_inode(sb, ino, head, test, set, data);
 }
+
+EXPORT_SYMBOL(icreate);
+EXPORT_SYMBOL(unlock_new_inode);
 
 /**
  *	insert_inode_hash - hash an inode
diff -urN orig/fs/reiserfs/inode.c icreate3/fs/reiserfs/inode.c
--- orig/fs/reiserfs/inode.c	Mon Apr 29 14:10:15 2002
+++ icreate3/fs/reiserfs/inode.c	Mon Apr 29 17:15:22 2002
@@ -33,7 +33,7 @@
     lock_kernel() ; 
 
     /* The = 0 happens when we abort creating a new inode for some reason like lack of space.. */
-    if (INODE_PKEY(inode)->k_objectid != 0) { /* also handles bad_inode case */
+    if (!(inode->i_state & I_NEW) && INODE_PKEY(inode)->k_objectid != 0) { /* also handles bad_inode case */
 	down (&inode->i_sem); 
 
 	journal_begin(&th, inode->i_sb, jbegin_count) ;
@@ -1134,19 +1134,20 @@
 
 /* looks for stat data in the tree, and fills up the fields of in-core
    inode stat data fields */
-void reiserfs_read_inode2 (struct inode * inode, void *p)
+int reiserfs_init_inode (struct inode * inode, void *p)
+{
+    struct reiserfs_iget4_args *args = (struct reiserfs_iget4_args *)p ;
+    INODE_PKEY(inode)->k_dir_id = cpu_to_le32(args->objectid);
+    return 0;
+}
+
+void reiserfs_init_inode2 (struct inode * inode, struct reiserfs_iget4_args *args)
 {
     INITIALIZE_PATH (path_to_sd);
     struct cpu_key key;
-    struct reiserfs_iget4_args *args = (struct reiserfs_iget4_args *)p ;
     unsigned long dirino;
     int retval;
 
-    if (!p) {
-	reiserfs_make_bad_inode(inode) ;
-	return;
-    }
-
     dirino = args->objectid ;
 
     /* set version 1, version 2 could be used too, because stat data
@@ -1216,8 +1217,7 @@
  * inode numbers (objectids) are distinguished by parent directory ids.
  *
  */
-static int reiserfs_find_actor( struct inode *inode, 
-				unsigned long inode_no, void *opaque )
+static int reiserfs_find_actor( struct inode *inode, void *opaque )
 {
     struct reiserfs_iget4_args *args;
 
@@ -1232,10 +1232,15 @@
     struct reiserfs_iget4_args args ;
 
     args.objectid = key->on_disk_key.k_dir_id ;
-    inode = iget4 (s, key->on_disk_key.k_objectid, 
-		   reiserfs_find_actor, (void *)(&args));
+    inode = icreate (s, key->on_disk_key.k_objectid, 
+		     reiserfs_find_actor, reiserfs_init_inode, (void *)(&args));
     if (!inode) 
 	return ERR_PTR(-ENOMEM) ;
+
+    if (inode->i_state & I_NEW) {
+	reiserfs_init_inode2(inode, &args);
+	unlock_new_inode(inode);
+    }
 
     if (comp_short_keys (INODE_PKEY (inode), key) || is_bad_inode (inode)) {
 	/* either due to i/o error or a stale NFS handle */
diff -urN orig/fs/reiserfs/super.c icreate3/fs/reiserfs/super.c
--- orig/fs/reiserfs/super.c	Mon Apr 29 14:09:03 2002
+++ icreate3/fs/reiserfs/super.c	Mon Apr 29 16:18:28 2002
@@ -485,7 +485,6 @@
   alloc_inode: reiserfs_alloc_inode,
   destroy_inode: reiserfs_destroy_inode,
   read_inode: reiserfs_read_inode,
-  read_inode2: reiserfs_read_inode2,
   write_inode: reiserfs_write_inode,
   dirty_inode: reiserfs_dirty_inode,
   delete_inode: reiserfs_delete_inode,
@@ -1065,10 +1064,15 @@
 	s->s_flags |= MS_RDONLY ;
     }
     args.objectid = REISERFS_ROOT_PARENT_OBJECTID ;
-    root_inode = iget4 (s, REISERFS_ROOT_OBJECTID, 0, (void *)(&args));
+    root_inode = icreate (s, REISERFS_ROOT_OBJECTID, 0, reiserfs_init_inode, (void *)(&args));
     if (!root_inode) {
 	printk ("reiserfs_fill_super: get root inode failed\n");
 	goto error;
+    }
+
+    if (root_inode->i_state & I_NEW) {
+	reiserfs_init_inode2(root_inode, &args);
+	unlock_new_inode(root_inode);
     }
 
     s->s_root = d_alloc_root(root_inode);  
diff -urN orig/include/linux/fs.h icreate3/include/linux/fs.h
--- orig/include/linux/fs.h	Mon Apr 29 14:11:21 2002
+++ icreate3/include/linux/fs.h	Mon Apr 29 18:52:01 2002
@@ -852,13 +852,6 @@
 
 	void (*read_inode) (struct inode *);
   
-  	/* reiserfs kludge.  reiserfs needs 64 bits of information to
-    	** find an inode.  We are using the read_inode2 call to get
-   	** that information.  We don't like this, and are waiting on some
-   	** VFS changes for the real solution.
-   	** iget4 calls read_inode2, iff it is defined
-   	*/
-    	void (*read_inode2) (struct inode *, void *) ;
    	void (*dirty_inode) (struct inode *);
 	void (*write_inode) (struct inode *, int);
 	void (*put_inode) (struct inode *);
@@ -906,6 +899,7 @@
 #define I_LOCK			8
 #define I_FREEING		16
 #define I_CLEAR			32
+#define I_NEW			64
 
 #define I_DIRTY (I_DIRTY_SYNC | I_DIRTY_DATASYNC | I_DIRTY_PAGES)
 
@@ -1442,11 +1436,21 @@
 extern struct inode * igrab(struct inode *);
 extern ino_t iunique(struct super_block *, ino_t);
 
-typedef int (*find_inode_t)(struct inode *, unsigned long, void *);
-extern struct inode * iget4(struct super_block *, unsigned long, find_inode_t, void *);
+extern struct inode * icreate(struct super_block *, unsigned long, int (*test)(struct inode *, void *), int (*set)(struct inode *, void *), void *data);
+extern void unlock_new_inode(struct inode *);
+
 static inline struct inode *iget(struct super_block *sb, unsigned long ino)
 {
-	return iget4(sb, ino, NULL, NULL);
+	struct inode *inode;
+
+	inode = icreate(sb, ino, NULL, NULL, NULL);
+
+	if (inode && (inode->i_state & I_NEW)) {
+		sb->s_op->read_inode(inode);
+		unlock_new_inode(inode);
+	}
+
+	return inode;
 }
 
 extern void clear_inode(struct inode *);
diff -urN orig/include/linux/reiserfs_fs.h icreate3/include/linux/reiserfs_fs.h
--- orig/include/linux/reiserfs_fs.h	Mon Apr 29 14:10:33 2002
+++ icreate3/include/linux/reiserfs_fs.h	Mon Apr 29 16:17:31 2002
@@ -1819,7 +1819,8 @@
 /* inode.c */
 
 void reiserfs_read_inode (struct inode * inode) ;
-void reiserfs_read_inode2(struct inode * inode, void *p) ;
+int reiserfs_init_inode(struct inode * inode, void *p) ;
+void reiserfs_init_inode2(struct inode * inode, struct reiserfs_iget4_args *args) ;
 void reiserfs_delete_inode (struct inode * inode);
 void reiserfs_write_inode (struct inode * inode, int) ;
 struct dentry *reiserfs_fh_to_dentry(struct super_block *sb, __u32 *data,
diff -urN orig/kernel/ksyms.c icreate3/kernel/ksyms.c
--- orig/kernel/ksyms.c	Mon Apr 29 14:11:22 2002
+++ icreate3/kernel/ksyms.c	Mon Apr 29 16:02:01 2002
@@ -137,7 +137,6 @@
 EXPORT_SYMBOL(fget);
 EXPORT_SYMBOL(igrab);
 EXPORT_SYMBOL(iunique);
-EXPORT_SYMBOL(iget4);
 EXPORT_SYMBOL(iput);
 EXPORT_SYMBOL(inode_init_once);
 EXPORT_SYMBOL(force_delete);

Re: [PATCH/RFC] Replacing iget4/read_inode2 with icreate

[Christoph Hellwig]

On Mon, Apr 29, 2002 at 07:24:29PM -0400, Jan Harkes wrote:
> 
> This patch replaces iget4/read_inode2 with icreate. It is pretty much
> the XFS icreate + a fix for the init race. Modified both Coda and
> ReiserFS to use icreate and do their own inode initialization.
> 
> I tested it for Coda and it seems to be stable. Can't reproduce the race
> on the SMP box here. I'd like to know if I made the right ReiserFS
> changes.

Patch looks good to me, but _please_ read Documentation/CodingStyle and
use one tab indents for core code.

Re: [PATCH/RFC] Replacing iget4/read_inode2 with icreate

[Jan Harkes]

On Tue, Apr 30, 2002 at 07:12:36AM +0100, Christoph Hellwig wrote:
> Patch looks good to me, but _please_ read Documentation/CodingStyle and
> use one tab indents for core code.

I've pretty much consistently used one tab indents in both the core VFS
and the Coda parts. Reiserfs uses 4 space tabs all over, so I 'emulated'
their formatting. Some places might have slipped by I'll double check.

Jan

Re: [PATCH/RFC] Replacing iget4/read_inode2 with icreate

[Jan Harkes]

On Tue, Apr 30, 2002 at 10:52:30AM -0400, Jan Harkes wrote:
> On Tue, Apr 30, 2002 at 07:12:36AM +0100, Christoph Hellwig wrote:
> > Patch looks good to me, but _please_ read Documentation/CodingStyle and
> > use one tab indents for core code.
> 
> I've pretty much consistently used one tab indents in both the core VFS
> and the Coda parts. Reiserfs uses 4 space tabs all over, so I 'emulated'
> their formatting. Some places might have slipped by I'll double check.

Ok, my bad. More slipped through than I expected. This is a new version
that fixes the bad Coda and VFS indenting. Reiser needs to be pulled
through 'indent', it just looks silly if the few lines that I change are
correctly indented and the rest is not.

Jan


Against 2.5.11

diff -urN orig/fs/Makefile icreate4/fs/Makefile
--- orig/fs/Makefile	Mon Apr 29 14:10:14 2002
+++ icreate4/fs/Makefile	Tue Apr 30 11:31:00 2002
@@ -7,7 +7,7 @@
 
 O_TARGET := fs.o
 
-export-objs :=	filesystems.o open.o dcache.o buffer.o bio.o
+export-objs :=	filesystems.o open.o dcache.o buffer.o bio.o inode.o
 mod-subdirs :=	nls
 
 obj-y :=	open.o read_write.o devices.o file_table.o buffer.o \
diff -urN orig/fs/coda/cnode.c icreate4/fs/coda/cnode.c
--- orig/fs/coda/cnode.c	Tue Apr  2 14:36:09 2002
+++ icreate4/fs/coda/cnode.c	Tue Apr 30 11:34:57 2002
@@ -25,7 +25,7 @@
 	return 1;
 }
 
-static int coda_inocmp(struct inode *inode, unsigned long ino, void *opaque)
+static int coda_inocmp(struct inode *inode, void *opaque)
 {
 	return (coda_fideq((ViceFid *)opaque, &(ITOC(inode)->c_fid)));
 }
@@ -55,26 +55,30 @@
                 init_special_inode(inode, inode->i_mode, attr->va_rdev);
 }
 
+int coda_init_inode(struct inode *inode, void *data)
+{
+	ITOC(inode)->c_fid = *(ViceFid *)data;
+	return 0;
+}
+
 struct inode * coda_iget(struct super_block * sb, ViceFid * fid,
 			 struct coda_vattr * attr)
 {
 	struct inode *inode;
 	struct coda_inode_info *cii;
+	struct coda_sb_info *sbi = coda_sbp(sb);
 	ino_t ino = coda_f2i(fid);
 
-	inode = iget4(sb, ino, coda_inocmp, fid);
+	inode = icreate(sb, ino, coda_inocmp, coda_init_inode, fid);
 
 	if (!inode)
 		return ERR_PTR(-ENOMEM);
 
-	/* check if the inode is already initialized */
-	cii = ITOC(inode);
-	if (coda_isnullfid(&cii->c_fid))
-		/* new, empty inode found... initializing */
-		cii->c_fid = *fid;
-
-	/* we shouldnt see inode collisions anymore */
-	if (!coda_fideq(fid, &cii->c_fid)) BUG();
+	if (inode->i_state & I_NEW) {
+		cii = ITOC(inode);
+		list_add(&cii->c_cilist, &sbi->sbi_cihead);
+		unlock_new_inode(inode);
+	}
 
 	/* always replace the attributes, type might have changed */
 	coda_fill_inode(inode, attr);
@@ -126,12 +130,16 @@
 	insert_inode_hash(inode);
 }
 
+int coda_fail_inode(struct inode *inode, void *opaque)
+{
+	return -1;
+}
+
 /* convert a fid to an inode. */
 struct inode *coda_fid_to_inode(ViceFid *fid, struct super_block *sb) 
 {
 	ino_t nr;
 	struct inode *inode;
-	struct coda_inode_info *cii;
 
 	if ( !sb ) {
 		printk("coda_fid_to_inode: no sb!\n");
@@ -139,24 +147,13 @@
 	}
 
 	nr = coda_f2i(fid);
-	inode = iget4(sb, nr, coda_inocmp, fid);
-	if ( !inode ) {
-		printk("coda_fid_to_inode: null from iget, sb %p, nr %ld.\n",
-		       sb, (long)nr);
-		return NULL;
-	}
-
-	cii = ITOC(inode);
-
-	/* The inode could already be purged due to memory pressure */
-	if (coda_isnullfid(&cii->c_fid)) {
-		inode->i_nlink = 0;
-		iput(inode);
+	inode = icreate(sb, nr, coda_inocmp, coda_fail_inode, fid);
+	if ( !inode )
 		return NULL;
-	}
 
-	/* we shouldn't see inode collisions anymore */
-	if ( !coda_fideq(fid, &cii->c_fid) ) BUG();
+	/* we should never see newly created inodes because we intentionally
+	 * fail in the initialization callback */
+	BUG_ON(inode->i_state & I_NEW);
 
         return inode;
 }
@@ -164,18 +161,19 @@
 /* the CONTROL inode is made without asking attributes from Venus */
 int coda_cnode_makectl(struct inode **inode, struct super_block *sb)
 {
-    int error = 0;
+	int error = -ENOMEM;
+	ViceFid nullfid = {0,0,0};
+
+	*inode = icreate(sb, CTL_INO, NULL, NULL, NULL);
 
-    *inode = iget(sb, CTL_INO);
-    if ( *inode ) {
+	if ( *inode && ((*inode)->i_state & I_NEW) ) {
 	(*inode)->i_op = &coda_ioctl_inode_operations;
-	(*inode)->i_fop = &coda_ioctl_operations;
-	(*inode)->i_mode = 0444;
-	error = 0;
-    } else { 
-	error = -ENOMEM;
-    }
-    
-    return error;
+		(*inode)->i_fop = &coda_ioctl_operations;
+		(*inode)->i_mode = 0444;
+		unlock_new_inode(*inode);
+		error = 0;
+	}
+
+	return error;
 }
 
diff -urN orig/fs/coda/inode.c icreate4/fs/coda/inode.c
--- orig/fs/coda/inode.c	Mon Apr 29 14:09:02 2002
+++ icreate4/fs/coda/inode.c	Tue Apr 30 11:31:00 2002
@@ -229,16 +229,9 @@
 	kfree(sbi);
 }
 
-/* all filling in of inodes postponed until lookup */
 static void coda_read_inode(struct inode *inode)
 {
-	struct coda_sb_info *sbi = coda_sbp(inode->i_sb);
-	struct coda_inode_info *cii;
-
-        if (!sbi) BUG();
-
-	cii = ITOC(inode);
-	list_add(&cii->c_cilist, &sbi->sbi_cihead);
+	make_bad_inode(inode);
 }
 
 static void coda_clear_inode(struct inode *inode)
diff -urN orig/fs/inode.c icreate4/fs/inode.c
--- orig/fs/inode.c	Mon Apr 29 14:09:02 2002
+++ icreate4/fs/inode.c	Tue Apr 30 11:31:00 2002
@@ -17,6 +17,7 @@
 #include <linux/swapctl.h>
 #include <linux/prefetch.h>
 #include <linux/locks.h>
+#include <linux/module.h>
 
 /*
  * New inode.c implementation.
@@ -793,7 +794,7 @@
  * by hand after calling find_inode now! This simplifies iunique and won't
  * add any additional branch in the common code.
  */
-static struct inode * find_inode(struct super_block * sb, unsigned long ino, struct list_head *head, find_inode_t find_actor, void *opaque)
+static struct inode * find_inode(struct super_block * sb, unsigned long ino, struct list_head *head, int (*test)(struct inode *, void *), void *data)
 {
 	struct list_head *tmp;
 	struct inode * inode;
@@ -809,7 +810,7 @@
 			continue;
 		if (inode->i_sb != sb)
 			continue;
-		if (find_actor && !find_actor(inode, ino, opaque))
+		if (test && !test(inode, data))
 			continue;
 		break;
 	}
@@ -842,53 +843,59 @@
 	return inode;
 }
 
+void unlock_new_inode(struct inode *inode)
+{
+	BUG_ON(!(inode->i_state & I_NEW));
+	/*
+	 * This is special!  We do not need the spinlock
+	 * when clearing I_LOCK, because we're guaranteed
+	 * that nobody else tries to do anything about the
+	 * state of the inode when it is locked, as we
+	 * just created it (so there can be no old holders
+	 * that haven't tested I_LOCK).
+	 */
+	inode->i_state &= ~(I_LOCK|I_NEW);
+	wake_up(&inode->i_wait);
+}
+
+
 /*
  * This is called without the inode lock held.. Be careful.
  *
  * We no longer cache the sb_flags in i_flags - see fs.h
  *	-- rmk@arm.uk.linux.org
  */
-static struct inode * get_new_inode(struct super_block *sb, unsigned long ino, struct list_head *head, find_inode_t find_actor, void *opaque)
+static struct inode * get_new_inode(struct super_block *sb, unsigned long ino, struct list_head *head, int (*test)(struct inode *, void *), int (*set)(struct inode *, void *), void *data)
 {
 	struct inode * inode;
+	int err = 0;
 
 	inode = alloc_inode(sb);
 	if (inode) {
 		struct inode * old;
+		inode->i_state = I_LOCK|I_NEW;
 
 		spin_lock(&inode_lock);
 		/* We released the lock, so.. */
-		old = find_inode(sb, ino, head, find_actor, opaque);
+		old = find_inode(sb, ino, head, test, data);
 		if (!old) {
-			inodes_stat.nr_inodes++;
-			list_add(&inode->i_list, &inode_in_use);
-			list_add(&inode->i_hash, head);
 			inode->i_ino = ino;
-			inode->i_state = I_LOCK;
+			if (!set || (err = set(inode, data)) == 0) {
+				inodes_stat.nr_inodes++;
+				list_add(&inode->i_list, &inode_in_use);
+				list_add(&inode->i_hash, head);
+			}
 			spin_unlock(&inode_lock);
 
-			/* reiserfs specific hack right here.  We don't
-			** want this to last, and are looking for VFS changes
-			** that will allow us to get rid of it.
-			** -- mason@suse.com 
-			*/
-			if (sb->s_op->read_inode2) {
-				sb->s_op->read_inode2(inode, opaque) ;
-			} else {
-				sb->s_op->read_inode(inode);
+			/* failed to initialize? */
+			if (err) {
+				destroy_inode(inode);
+				return NULL;
 			}
 
-			/*
-			 * This is special!  We do not need the spinlock
-			 * when clearing I_LOCK, because we're guaranteed
-			 * that nobody else tries to do anything about the
-			 * state of the inode when it is locked, as we
-			 * just created it (so there can be no old holders
-			 * that haven't tested I_LOCK).
+			/* Return the locked inode with I_NEW set, the
+			 * caller is responsible for filling in the contents
 			 */
-			inode->i_state &= ~I_LOCK;
-			wake_up(&inode->i_wait);
-
 			return inode;
 		}
 
@@ -968,14 +975,18 @@
 	return inode;
 }
 
-
-struct inode *iget4(struct super_block *sb, unsigned long ino, find_inode_t find_actor, void *opaque)
+/*
+ * This is iget4 without the read_inode portion of get_new_inode
+ * the filesystem gets back a new locked and hashed inode and gets
+ * to fill it in before unlocking it via unlock_new_inode().
+ */
+struct inode *icreate(struct super_block *sb, unsigned long ino, int (*test)(struct inode *, void *), int (*set)(struct inode *, void *), void *data)
 {
 	struct list_head * head = inode_hashtable + hash(sb,ino);
 	struct inode * inode;
 
 	spin_lock(&inode_lock);
-	inode = find_inode(sb, ino, head, find_actor, opaque);
+	inode = find_inode(sb, ino, head, test, data);
 	if (inode) {
 		__iget(inode);
 		spin_unlock(&inode_lock);
@@ -984,12 +995,11 @@
 	}
 	spin_unlock(&inode_lock);
 
-	/*
-	 * get_new_inode() will do the right thing, re-trying the search
-	 * in case it had to block at any point.
-	 */
-	return get_new_inode(sb, ino, head, find_actor, opaque);
+	return get_new_inode(sb, ino, head, test, set, data);
 }
+
+EXPORT_SYMBOL(icreate);
+EXPORT_SYMBOL(unlock_new_inode);
 
 /**
  *	insert_inode_hash - hash an inode
diff -urN orig/fs/reiserfs/inode.c icreate4/fs/reiserfs/inode.c
--- orig/fs/reiserfs/inode.c	Mon Apr 29 14:10:15 2002
+++ icreate4/fs/reiserfs/inode.c	Tue Apr 30 11:38:06 2002
@@ -33,7 +33,7 @@
     lock_kernel() ; 
 
     /* The = 0 happens when we abort creating a new inode for some reason like lack of space.. */
-    if (INODE_PKEY(inode)->k_objectid != 0) { /* also handles bad_inode case */
+    if (!(inode->i_state & I_NEW) && INODE_PKEY(inode)->k_objectid != 0) { /* also handles bad_inode case */
 	down (&inode->i_sem); 
 
 	journal_begin(&th, inode->i_sb, jbegin_count) ;
@@ -1134,19 +1134,20 @@
 
 /* looks for stat data in the tree, and fills up the fields of in-core
    inode stat data fields */
-void reiserfs_read_inode2 (struct inode * inode, void *p)
+int reiserfs_init_inode (struct inode * inode, void *p)
+{
+	struct reiserfs_iget4_args *args = (struct reiserfs_iget4_args *)p ;
+	INODE_PKEY(inode)->k_dir_id = cpu_to_le32(args->objectid);
+	return 0;
+}
+
+void reiserfs_init_inode2 (struct inode * inode, struct reiserfs_iget4_args *args)
 {
     INITIALIZE_PATH (path_to_sd);
     struct cpu_key key;
-    struct reiserfs_iget4_args *args = (struct reiserfs_iget4_args *)p ;
     unsigned long dirino;
     int retval;
 
-    if (!p) {
-	reiserfs_make_bad_inode(inode) ;
-	return;
-    }
-
     dirino = args->objectid ;
 
     /* set version 1, version 2 could be used too, because stat data
@@ -1216,8 +1217,7 @@
  * inode numbers (objectids) are distinguished by parent directory ids.
  *
  */
-static int reiserfs_find_actor( struct inode *inode, 
-				unsigned long inode_no, void *opaque )
+static int reiserfs_find_actor( struct inode *inode, void *opaque )
 {
     struct reiserfs_iget4_args *args;
 
@@ -1232,10 +1232,15 @@
     struct reiserfs_iget4_args args ;
 
     args.objectid = key->on_disk_key.k_dir_id ;
-    inode = iget4 (s, key->on_disk_key.k_objectid, 
-		   reiserfs_find_actor, (void *)(&args));
+    inode = icreate(s, key->on_disk_key.k_objectid, 
+		    reiserfs_find_actor, reiserfs_init_inode, (void *)(&args));
     if (!inode) 
 	return ERR_PTR(-ENOMEM) ;
+
+    if (inode->i_state & I_NEW) {
+	reiserfs_init_inode2(inode, &args);
+	unlock_new_inode(inode);
+    }
 
     if (comp_short_keys (INODE_PKEY (inode), key) || is_bad_inode (inode)) {
 	/* either due to i/o error or a stale NFS handle */
diff -urN orig/fs/reiserfs/super.c icreate4/fs/reiserfs/super.c
--- orig/fs/reiserfs/super.c	Mon Apr 29 14:09:03 2002
+++ icreate4/fs/reiserfs/super.c	Tue Apr 30 11:38:35 2002
@@ -485,7 +485,6 @@
   alloc_inode: reiserfs_alloc_inode,
   destroy_inode: reiserfs_destroy_inode,
   read_inode: reiserfs_read_inode,
-  read_inode2: reiserfs_read_inode2,
   write_inode: reiserfs_write_inode,
   dirty_inode: reiserfs_dirty_inode,
   delete_inode: reiserfs_delete_inode,
@@ -1065,10 +1064,15 @@
 	s->s_flags |= MS_RDONLY ;
     }
     args.objectid = REISERFS_ROOT_PARENT_OBJECTID ;
-    root_inode = iget4 (s, REISERFS_ROOT_OBJECTID, 0, (void *)(&args));
+    root_inode = icreate (s, REISERFS_ROOT_OBJECTID, 0, reiserfs_init_inode, (void *)(&args));
     if (!root_inode) {
 	printk ("reiserfs_fill_super: get root inode failed\n");
 	goto error;
+    }
+
+    if (root_inode->i_state & I_NEW) {
+	reiserfs_init_inode2(root_inode, &args);
+	unlock_new_inode(root_inode);
     }
 
     s->s_root = d_alloc_root(root_inode);  
diff -urN orig/include/linux/fs.h icreate4/include/linux/fs.h
--- orig/include/linux/fs.h	Mon Apr 29 14:11:21 2002
+++ icreate4/include/linux/fs.h	Tue Apr 30 11:31:00 2002
@@ -852,13 +852,6 @@
 
 	void (*read_inode) (struct inode *);
   
-  	/* reiserfs kludge.  reiserfs needs 64 bits of information to
-    	** find an inode.  We are using the read_inode2 call to get
-   	** that information.  We don't like this, and are waiting on some
-   	** VFS changes for the real solution.
-   	** iget4 calls read_inode2, iff it is defined
-   	*/
-    	void (*read_inode2) (struct inode *, void *) ;
    	void (*dirty_inode) (struct inode *);
 	void (*write_inode) (struct inode *, int);
 	void (*put_inode) (struct inode *);
@@ -906,6 +899,7 @@
 #define I_LOCK			8
 #define I_FREEING		16
 #define I_CLEAR			32
+#define I_NEW			64
 
 #define I_DIRTY (I_DIRTY_SYNC | I_DIRTY_DATASYNC | I_DIRTY_PAGES)
 
@@ -1442,11 +1436,21 @@
 extern struct inode * igrab(struct inode *);
 extern ino_t iunique(struct super_block *, ino_t);
 
-typedef int (*find_inode_t)(struct inode *, unsigned long, void *);
-extern struct inode * iget4(struct super_block *, unsigned long, find_inode_t, void *);
+extern struct inode * icreate(struct super_block *, unsigned long, int (*test)(struct inode *, void *), int (*set)(struct inode *, void *), void *data);
+extern void unlock_new_inode(struct inode *);
+
 static inline struct inode *iget(struct super_block *sb, unsigned long ino)
 {
-	return iget4(sb, ino, NULL, NULL);
+	struct inode *inode;
+
+	inode = icreate(sb, ino, NULL, NULL, NULL);
+
+	if (inode && (inode->i_state & I_NEW)) {
+		sb->s_op->read_inode(inode);
+		unlock_new_inode(inode);
+	}
+
+	return inode;
 }
 
 extern void clear_inode(struct inode *);
diff -urN orig/include/linux/reiserfs_fs.h icreate4/include/linux/reiserfs_fs.h
--- orig/include/linux/reiserfs_fs.h	Mon Apr 29 14:10:33 2002
+++ icreate4/include/linux/reiserfs_fs.h	Tue Apr 30 11:31:00 2002
@@ -1819,7 +1819,8 @@
 /* inode.c */
 
 void reiserfs_read_inode (struct inode * inode) ;
-void reiserfs_read_inode2(struct inode * inode, void *p) ;
+int reiserfs_init_inode(struct inode * inode, void *p) ;
+void reiserfs_init_inode2(struct inode * inode, struct reiserfs_iget4_args *args) ;
 void reiserfs_delete_inode (struct inode * inode);
 void reiserfs_write_inode (struct inode * inode, int) ;
 struct dentry *reiserfs_fh_to_dentry(struct super_block *sb, __u32 *data,
diff -urN orig/kernel/ksyms.c icreate4/kernel/ksyms.c
--- orig/kernel/ksyms.c	Mon Apr 29 14:11:22 2002
+++ icreate4/kernel/ksyms.c	Tue Apr 30 11:31:00 2002
@@ -137,7 +137,6 @@
 EXPORT_SYMBOL(fget);
 EXPORT_SYMBOL(igrab);
 EXPORT_SYMBOL(iunique);
-EXPORT_SYMBOL(iget4);
 EXPORT_SYMBOL(iput);
 EXPORT_SYMBOL(inode_init_once);
 EXPORT_SYMBOL(force_delete);

Re: [PATCH/RFC] Replacing iget4/read_inode2 with icreate

[Steve Lord]

On Mon, 2002-04-29 at 18:24, Jan Harkes wrote:
> 
> This patch replaces iget4/read_inode2 with icreate. It is pretty much
> the XFS icreate + a fix for the init race. Modified both Coda and
> ReiserFS to use icreate and do their own inode initialization.
> 
> I tested it for Coda and it seems to be stable. Can't reproduce the race
> on the SMP box here. I'd like to know if I made the right ReiserFS
> changes.
> 
> Jan
> 
>

May be a little soon to remove iget4 - you left one caller, nfs is still
there in __nfs_fhget. This is the code which will get removed once
filesystems have switched to the new method I think. I am booting an
XFS kernel with this code now.

Steve



-- 

Steve Lord                                      voice: +1-651-683-3511
Principal Engineer, Filesystem Software         email: lord@sgi.com

Re: [PATCH/RFC] Replacing iget4/read_inode2 with icreate

[Steve Lord]

On Tue, 2002-04-30 at 10:54, Steve Lord wrote:

> 
> May be a little soon to remove iget4 - you left one caller, nfs is still
> there in __nfs_fhget. This is the code which will get removed once
> filesystems have switched to the new method I think. I am booting an
> XFS kernel with this code now.
> 
> Steve

Quick followup, XFS still functions with this patch, so thumbs up from
this end.

Steve

 

Re: [PATCH/RFC] Replacing iget4/read_inode2 with icreate

[Jan Harkes]

On Tue, Apr 30, 2002 at 10:54:06AM -0500, Steve Lord wrote:
> May be a little soon to remove iget4 - you left one caller, nfs is still
> there in __nfs_fhget. This is the code which will get removed once
> filesystems have switched to the new method I think. I am booting an
> XFS kernel with this code now.

Actually, it looks like NFS uses this itself because 'v3' has 64-bit
inode numbers.

Fix should be simple, something like the following. In fact NFS seems to
use it's own test to track newly created inodes, which can be replaced
with checking inode->i_state & I_NEW.

Jan


--- orig/fs/nfs/inode.c	Mon Apr 29 14:09:02 2002
+++ icreate5/fs/nfs/inode.c	Tue Apr 30 12:12:39 2002
@@ -592,7 +592,7 @@
  * i_ino.
  */
 static int
-nfs_find_actor(struct inode *inode, unsigned long ino, void *opaque)
+nfs_find_actor(struct inode *inode, void *opaque)
 {
 	struct nfs_find_desc	*desc = (struct nfs_find_desc *)opaque;
 	struct nfs_fh		*fh = desc->fh;
@@ -610,6 +610,18 @@
 	return 1;
 }
 
+static int
+nfs_init_locked(struct inode *inode, void *opaque)
+{
+	struct nfs_find_desc	*desc = (struct nfs_find_desc *)opaque;
+	struct nfs_fh		*fh = desc->fh;
+	struct nfs_fattr	*fattr = desc->fattr;
+
+	NFS_FILEID(inode) = fattr->fileid;
+	memcpy(NFS_FH(inode), fh, sizeof(struct nfs_fh));
+	return 0;
+}
+
 /*
  * This is our own version of iget that looks up inodes by file handle
  * instead of inode number.  We use this technique instead of using
@@ -652,10 +664,10 @@
 
 	ino = nfs_fattr_to_ino_t(fattr);
 
-	if (!(inode = iget4(sb, ino, nfs_find_actor, &desc)))
+	if (!(inode = icreate(sb, ino, nfs_find_actor, nfs_init_locked, &desc)))
 		goto out_no_inode;
 
-	if (NFS_NEW(inode)) {
+	if (inode->i_state & I_NEW) {
 		__u64		new_size, new_mtime;
 		loff_t		new_isize;
 		time_t		new_atime;
@@ -711,6 +723,8 @@
 		NFS_ATTRTIMEO(inode) = NFS_MINATTRTIMEO(inode);
 		NFS_ATTRTIMEO_UPDATE(inode) = jiffies;
 		memset(NFS_COOKIEVERF(inode), 0, sizeof(NFS_COOKIEVERF(inode)));
+
+		unlock_new_inode(inode);
 	} else
 		nfs_refresh_inode(inode, fattr);
 	dprintk("NFS: __nfs_fhget(%s/%Ld ct=%d)\n",

Re: [PATCH/RFC] Replacing iget4/read_inode2 with icreate

[Steve Lord]

On Tue, 2002-04-30 at 11:14, Jan Harkes wrote:
> On Tue, Apr 30, 2002 at 10:54:06AM -0500, Steve Lord wrote:
> > May be a little soon to remove iget4 - you left one caller, nfs is still
> > there in __nfs_fhget. This is the code which will get removed once
> > filesystems have switched to the new method I think. I am booting an
> > XFS kernel with this code now.
> 
> Actually, it looks like NFS uses this itself because 'v3' has 64-bit
> inode numbers.

Should have realized that when I saw the code in the client not the
server.

> 
> Fix should be simple, something like the following. In fact NFS seems to
> use it's own test to track newly created inodes, which can be replaced
> with checking inode->i_state & I_NEW.
> 

NFS is up and running with the combo of the two patches now, I only
have V2 servers setup here though.

Thanks,

Steve

Re: [PATCH/RFC] Replacing iget4/read_inode2 with icreate

[Chris Mason]

Your reiserfs bits work, but I prefer reiserfs_read_locked_inode over
reiserfs_init_inode2 (we've got init_inode and reiserfs_init_inode it
got confusing....

Below is a slightly updated version (reiserfs parts only), the only
difference is the name change, and it is against the reiserfs patch the
namesys guys posted yesterday there was a trivial reject in
fs/reiserfs/super.c.

These work for me under light testing on 2.5.11 + Viro's fastwalk fixes
+ reiserfs update.

-chris

===== fs/reiserfs/inode.c 1.51 vs edited =====
--- 1.51/fs/reiserfs/inode.c	Fri Apr 26 11:09:07 2002
+++ edited/fs/reiserfs/inode.c	Tue Apr 30 09:31:28 2002
@@ -30,7 +30,7 @@
     lock_kernel() ; 
 
     /* The = 0 happens when we abort creating a new inode for some reason like lack of space.. */
-    if (INODE_PKEY(inode)->k_objectid != 0) { /* also handles bad_inode case */
+    if (!(inode->i_state & I_NEW) && INODE_PKEY(inode)->k_objectid != 0) { /* also handles bad_inode case */
 	down (&inode->i_sem); 
 
 	journal_begin(&th, inode->i_sb, jbegin_count) ;
@@ -1118,21 +1118,22 @@
 }
 

+int reiserfs_init_inode (struct inode * inode, void *p)
+{
+    struct reiserfs_iget4_args *args = (struct reiserfs_iget4_args *)p ;
+    INODE_PKEY(inode)->k_dir_id = cpu_to_le32(args->objectid);
+    return 0;
+}
+
 /* looks for stat data in the tree, and fills up the fields of in-core
    inode stat data fields */
-void reiserfs_read_inode2 (struct inode * inode, void *p)
+void reiserfs_read_locked_inode (struct inode * inode, struct reiserfs_iget4_args *args)
 {
     INITIALIZE_PATH (path_to_sd);
     struct cpu_key key;
-    struct reiserfs_iget4_args *args = (struct reiserfs_iget4_args *)p ;
     unsigned long dirino;
     int retval;
 
-    if (!p) {
-	reiserfs_make_bad_inode(inode) ;
-	return;
-    }
-
     dirino = args->objectid ;
 
     /* set version 1, version 2 could be used too, because stat data
@@ -1202,8 +1203,7 @@
  * inode numbers (objectids) are distinguished by parent directory ids.
  *
  */
-static int reiserfs_find_actor( struct inode *inode, 
-				unsigned long inode_no, void *opaque )
+static int reiserfs_find_actor( struct inode *inode, void *opaque )
 {
     struct reiserfs_iget4_args *args;
 
@@ -1218,10 +1218,15 @@
     struct reiserfs_iget4_args args ;
 
     args.objectid = key->on_disk_key.k_dir_id ;
-    inode = iget4 (s, key->on_disk_key.k_objectid, 
-		   reiserfs_find_actor, (void *)(&args));
+    inode = icreate (s, key->on_disk_key.k_objectid, 
+		     reiserfs_find_actor, reiserfs_init_inode, (void *)(&args));
     if (!inode) 
 	return ERR_PTR(-ENOMEM) ;
+
+    if (inode->i_state & I_NEW) {
+	reiserfs_read_locked_inode(inode, &args);
+	unlock_new_inode(inode);
+    }
 
     if (comp_short_keys (INODE_PKEY (inode), key) || is_bad_inode (inode)) {
 	/* either due to i/o error or a stale NFS handle */
===== fs/reiserfs/super.c 1.43 vs edited =====
--- 1.43/fs/reiserfs/super.c	Mon Apr 29 08:55:12 2002
+++ edited/fs/reiserfs/super.c	Tue Apr 30 09:31:49 2002
@@ -471,7 +471,6 @@
   alloc_inode: reiserfs_alloc_inode,
   destroy_inode: reiserfs_destroy_inode,
   read_inode: reiserfs_read_inode,
-  read_inode2: reiserfs_read_inode2,
   write_inode: reiserfs_write_inode,
   dirty_inode: reiserfs_dirty_inode,
   delete_inode: reiserfs_delete_inode,
@@ -1148,10 +1147,16 @@
 	s->s_flags |= MS_RDONLY ;
     }
     args.objectid = REISERFS_ROOT_PARENT_OBJECTID ;
-    root_inode = iget4 (s, REISERFS_ROOT_OBJECTID, 0, (void *)(&args));
+    root_inode = icreate (s, REISERFS_ROOT_OBJECTID, 0, reiserfs_init_inode, 
+                         (void *)(&args));
     if (!root_inode) {
 	printk ("jmacd-10: reiserfs_fill_super: get root inode failed\n");
 	goto error;
+    }
+
+    if (root_inode->i_state & I_NEW) {
+        reiserfs_read_locked_inode(root_inode, &args);
+	unlock_new_inode(root_inode);
     }
 
     s->s_root = d_alloc_root(root_inode);  
===== include/linux/reiserfs_fs.h 1.32 vs edited =====
--- 1.32/include/linux/reiserfs_fs.h	Fri Apr 26 12:03:26 2002
+++ edited/include/linux/reiserfs_fs.h	Tue Apr 30 10:28:52 2002
@@ -1822,7 +1822,8 @@
 /* inode.c */
 
 void reiserfs_read_inode (struct inode * inode) ;
-void reiserfs_read_inode2(struct inode * inode, void *p) ;
+int reiserfs_init_inode(struct inode * inode, void *p) ;
+void reiserfs_read_locked_inode(struct inode * inode, struct reiserfs_iget4_args *args) ;
 void reiserfs_delete_inode (struct inode * inode);
 void reiserfs_write_inode (struct inode * inode, int) ;
 struct dentry *reiserfs_fh_to_dentry(struct super_block *sb, __u32 *data,

Re: [PATCH/RFC] Replacing iget4/read_inode2 with icreate

[Jan Harkes]

On Tue, Apr 30, 2002 at 12:40:33PM -0400, Chris Mason wrote:
> Your reiserfs bits work, but I prefer reiserfs_read_locked_inode over
> reiserfs_init_inode2 (we've got init_inode and reiserfs_init_inode it
> got confusing....

reiserfs_init_inode could be renamed to reiserfs_init_locked_inode which
matches better what it is doing. It would also nicely complement the
reiserfs_read_locked_inode name.

I've merged the naming changes.

> These work for me under light testing on 2.5.11 + Viro's fastwalk fixes
> + reiserfs update.

Nice, so now we have all existing users of iget4 converted to icreate. I
just did an exhaustive grep on the tree and iget4 only shows up in a few
comments (and as struct reiserfs_iget4_args). 

I've also changed these. Should there be a description of the iget4 ->
icreate change in Documentation/filesystems? Wasn't there some place
where all VFS changes were documented?

Jan

Re: [PATCH/RFC] Replacing iget4/read_inode2 with icreate

[Jan Harkes]

On Tue, Apr 30, 2002 at 01:03:46PM -0400, Jan Harkes wrote:
> Nice, so now we have all existing users of iget4 converted to icreate. I
> just did an exhaustive grep on the tree and iget4 only shows up in a few
> comments (and as struct reiserfs_iget4_args). 
> 
> I've also changed these. Should there be a description of the iget4 ->
> icreate change in Documentation/filesystems? Wasn't there some place
> where all VFS changes were documented?

Thought I had already sent the latest version of the patch before I went
home...

This is the 6th iteration of the patch and it pulls everything together
as well as adding a bit to Documentation/filesystems/porting.

Jan

diff -urN orig/Documentation/filesystems/Locking icreate6/Documentation/filesystems/Locking
--- orig/Documentation/filesystems/Locking	Mon Apr 29 14:11:18 2002
+++ icreate6/Documentation/filesystems/Locking	Tue Apr 30 13:00:44 2002
@@ -115,7 +115,7 @@
 remount_fs:	yes	yes	maybe		(see below)
 umount_begin:	yes	no	maybe		(see below)
 
-->read_inode() is not a method - it's a callback used in iget()/iget4().
+->read_inode() is not a method - it's a callback used in iget().
 rules for mount_sem are not too nice - it is going to die and be replaced
 by better scheme anyway.
 
diff -urN orig/Documentation/filesystems/porting icreate6/Documentation/filesystems/porting
--- orig/Documentation/filesystems/porting	Mon Apr 29 14:11:18 2002
+++ icreate6/Documentation/filesystems/porting	Tue Apr 30 16:18:08 2002
@@ -146,3 +146,33 @@
 
 It is planned that this will be required for exporting once the code
 settles down a bit.
+
+---
+[mandatory]
+
+iget4() and the read_inode2 callback have been superceded by icreate()
+which has the following prototype,
+
+    struct inode *icreate(struct super_block *sb, unsigned long ino,
+			  void (*test)(struct inode *, void *),
+			  void (*set)(struct inode *, void *),
+			  void *data);
+
+'test' is an additional function that can be used when the inode
+number is not sufficient to identify the actual file object. 'set'
+should be a non-blocking function that initializes those parts of a
+newly created inode to allow the test function to succeed. 'data' is
+passed as an opaque value to both test and set functions.
+
+When the inode has been created by icreate(), it will be returned with
+the I_NEW flag set and will still be locked. read_inode has not been
+called so the file system still has to finalize the initialization. Once
+the inode is initialized is must be unlocked by calling unlock_new_inode().
+
+e.g.
+	inode = icreate(sb, ino, NULL, NULL, NULL);
+	if (inode->i_state & I_NEW) {
+		read_inode_from_disk(inode);
+		unlock_new_inode(inode);
+	}
+
diff -urN orig/fs/Makefile icreate6/fs/Makefile
--- orig/fs/Makefile	Mon Apr 29 14:10:14 2002
+++ icreate6/fs/Makefile	Tue Apr 30 11:31:00 2002
@@ -7,7 +7,7 @@
 
 O_TARGET := fs.o
 
-export-objs :=	filesystems.o open.o dcache.o buffer.o bio.o
+export-objs :=	filesystems.o open.o dcache.o buffer.o bio.o inode.o
 mod-subdirs :=	nls
 
 obj-y :=	open.o read_write.o devices.o file_table.o buffer.o \
diff -urN orig/fs/coda/cnode.c icreate6/fs/coda/cnode.c
--- orig/fs/coda/cnode.c	Tue Apr  2 14:36:09 2002
+++ icreate6/fs/coda/cnode.c	Tue Apr 30 11:34:57 2002
@@ -25,7 +25,7 @@
 	return 1;
 }
 
-static int coda_inocmp(struct inode *inode, unsigned long ino, void *opaque)
+static int coda_inocmp(struct inode *inode, void *opaque)
 {
 	return (coda_fideq((ViceFid *)opaque, &(ITOC(inode)->c_fid)));
 }
@@ -55,26 +55,30 @@
                 init_special_inode(inode, inode->i_mode, attr->va_rdev);
 }
 
+int coda_init_inode(struct inode *inode, void *data)
+{
+	ITOC(inode)->c_fid = *(ViceFid *)data;
+	return 0;
+}
+
 struct inode * coda_iget(struct super_block * sb, ViceFid * fid,
 			 struct coda_vattr * attr)
 {
 	struct inode *inode;
 	struct coda_inode_info *cii;
+	struct coda_sb_info *sbi = coda_sbp(sb);
 	ino_t ino = coda_f2i(fid);
 
-	inode = iget4(sb, ino, coda_inocmp, fid);
+	inode = icreate(sb, ino, coda_inocmp, coda_init_inode, fid);
 
 	if (!inode)
 		return ERR_PTR(-ENOMEM);
 
-	/* check if the inode is already initialized */
-	cii = ITOC(inode);
-	if (coda_isnullfid(&cii->c_fid))
-		/* new, empty inode found... initializing */
-		cii->c_fid = *fid;
-
-	/* we shouldnt see inode collisions anymore */
-	if (!coda_fideq(fid, &cii->c_fid)) BUG();
+	if (inode->i_state & I_NEW) {
+		cii = ITOC(inode);
+		list_add(&cii->c_cilist, &sbi->sbi_cihead);
+		unlock_new_inode(inode);
+	}
 
 	/* always replace the attributes, type might have changed */
 	coda_fill_inode(inode, attr);
@@ -126,12 +130,16 @@
 	insert_inode_hash(inode);
 }
 
+int coda_fail_inode(struct inode *inode, void *opaque)
+{
+	return -1;
+}
+
 /* convert a fid to an inode. */
 struct inode *coda_fid_to_inode(ViceFid *fid, struct super_block *sb) 
 {
 	ino_t nr;
 	struct inode *inode;
-	struct coda_inode_info *cii;
 
 	if ( !sb ) {
 		printk("coda_fid_to_inode: no sb!\n");
@@ -139,24 +147,13 @@
 	}
 
 	nr = coda_f2i(fid);
-	inode = iget4(sb, nr, coda_inocmp, fid);
-	if ( !inode ) {
-		printk("coda_fid_to_inode: null from iget, sb %p, nr %ld.\n",
-		       sb, (long)nr);
-		return NULL;
-	}
-
-	cii = ITOC(inode);
-
-	/* The inode could already be purged due to memory pressure */
-	if (coda_isnullfid(&cii->c_fid)) {
-		inode->i_nlink = 0;
-		iput(inode);
+	inode = icreate(sb, nr, coda_inocmp, coda_fail_inode, fid);
+	if ( !inode )
 		return NULL;
-	}
 
-	/* we shouldn't see inode collisions anymore */
-	if ( !coda_fideq(fid, &cii->c_fid) ) BUG();
+	/* we should never see newly created inodes because we intentionally
+	 * fail in the initialization callback */
+	BUG_ON(inode->i_state & I_NEW);
 
         return inode;
 }
@@ -164,18 +161,19 @@
 /* the CONTROL inode is made without asking attributes from Venus */
 int coda_cnode_makectl(struct inode **inode, struct super_block *sb)
 {
-    int error = 0;
+	int error = -ENOMEM;
+	ViceFid nullfid = {0,0,0};
+
+	*inode = icreate(sb, CTL_INO, NULL, NULL, NULL);
 
-    *inode = iget(sb, CTL_INO);
-    if ( *inode ) {
+	if ( *inode && ((*inode)->i_state & I_NEW) ) {
 	(*inode)->i_op = &coda_ioctl_inode_operations;
-	(*inode)->i_fop = &coda_ioctl_operations;
-	(*inode)->i_mode = 0444;
-	error = 0;
-    } else { 
-	error = -ENOMEM;
-    }
-    
-    return error;
+		(*inode)->i_fop = &coda_ioctl_operations;
+		(*inode)->i_mode = 0444;
+		unlock_new_inode(*inode);
+		error = 0;
+	}
+
+	return error;
 }
 
diff -urN orig/fs/coda/inode.c icreate6/fs/coda/inode.c
--- orig/fs/coda/inode.c	Mon Apr 29 14:09:02 2002
+++ icreate6/fs/coda/inode.c	Tue Apr 30 11:31:00 2002
@@ -229,16 +229,9 @@
 	kfree(sbi);
 }
 
-/* all filling in of inodes postponed until lookup */
 static void coda_read_inode(struct inode *inode)
 {
-	struct coda_sb_info *sbi = coda_sbp(inode->i_sb);
-	struct coda_inode_info *cii;
-
-        if (!sbi) BUG();
-
-	cii = ITOC(inode);
-	list_add(&cii->c_cilist, &sbi->sbi_cihead);
+	make_bad_inode(inode);
 }
 
 static void coda_clear_inode(struct inode *inode)
diff -urN orig/fs/inode.c icreate6/fs/inode.c
--- orig/fs/inode.c	Mon Apr 29 14:09:02 2002
+++ icreate6/fs/inode.c	Tue Apr 30 12:59:20 2002
@@ -17,6 +17,7 @@
 #include <linux/swapctl.h>
 #include <linux/prefetch.h>
 #include <linux/locks.h>
+#include <linux/module.h>
 
 /*
  * New inode.c implementation.
@@ -793,7 +794,7 @@
  * by hand after calling find_inode now! This simplifies iunique and won't
  * add any additional branch in the common code.
  */
-static struct inode * find_inode(struct super_block * sb, unsigned long ino, struct list_head *head, find_inode_t find_actor, void *opaque)
+static struct inode * find_inode(struct super_block * sb, unsigned long ino, struct list_head *head, int (*test)(struct inode *, void *), void *data)
 {
 	struct list_head *tmp;
 	struct inode * inode;
@@ -809,7 +810,7 @@
 			continue;
 		if (inode->i_sb != sb)
 			continue;
-		if (find_actor && !find_actor(inode, ino, opaque))
+		if (test && !test(inode, data))
 			continue;
 		break;
 	}
@@ -842,53 +843,59 @@
 	return inode;
 }
 
+void unlock_new_inode(struct inode *inode)
+{
+	BUG_ON(!(inode->i_state & I_NEW));
+	/*
+	 * This is special!  We do not need the spinlock
+	 * when clearing I_LOCK, because we're guaranteed
+	 * that nobody else tries to do anything about the
+	 * state of the inode when it is locked, as we
+	 * just created it (so there can be no old holders
+	 * that haven't tested I_LOCK).
+	 */
+	inode->i_state &= ~(I_LOCK|I_NEW);
+	wake_up(&inode->i_wait);
+}
+
+
 /*
  * This is called without the inode lock held.. Be careful.
  *
  * We no longer cache the sb_flags in i_flags - see fs.h
  *	-- rmk@arm.uk.linux.org
  */
-static struct inode * get_new_inode(struct super_block *sb, unsigned long ino, struct list_head *head, find_inode_t find_actor, void *opaque)
+static struct inode * get_new_inode(struct super_block *sb, unsigned long ino, struct list_head *head, int (*test)(struct inode *, void *), int (*set)(struct inode *, void *), void *data)
 {
 	struct inode * inode;
+	int err = 0;
 
 	inode = alloc_inode(sb);
 	if (inode) {
 		struct inode * old;
+		inode->i_state = I_LOCK|I_NEW;
 
 		spin_lock(&inode_lock);
 		/* We released the lock, so.. */
-		old = find_inode(sb, ino, head, find_actor, opaque);
+		old = find_inode(sb, ino, head, test, data);
 		if (!old) {
-			inodes_stat.nr_inodes++;
-			list_add(&inode->i_list, &inode_in_use);
-			list_add(&inode->i_hash, head);
 			inode->i_ino = ino;
-			inode->i_state = I_LOCK;
+			if (!set || (err = set(inode, data)) == 0) {
+				inodes_stat.nr_inodes++;
+				list_add(&inode->i_list, &inode_in_use);
+				list_add(&inode->i_hash, head);
+			}
 			spin_unlock(&inode_lock);
 
-			/* reiserfs specific hack right here.  We don't
-			** want this to last, and are looking for VFS changes
-			** that will allow us to get rid of it.
-			** -- mason@suse.com 
-			*/
-			if (sb->s_op->read_inode2) {
-				sb->s_op->read_inode2(inode, opaque) ;
-			} else {
-				sb->s_op->read_inode(inode);
+			/* failed to initialize? */
+			if (err) {
+				destroy_inode(inode);
+				return NULL;
 			}
 
-			/*
-			 * This is special!  We do not need the spinlock
-			 * when clearing I_LOCK, because we're guaranteed
-			 * that nobody else tries to do anything about the
-			 * state of the inode when it is locked, as we
-			 * just created it (so there can be no old holders
-			 * that haven't tested I_LOCK).
+			/* Return the locked inode with I_NEW set, the
+			 * caller is responsible for filling in the contents
 			 */
-			inode->i_state &= ~I_LOCK;
-			wake_up(&inode->i_wait);
-
 			return inode;
 		}
 
@@ -968,14 +975,18 @@
 	return inode;
 }
 
-
-struct inode *iget4(struct super_block *sb, unsigned long ino, find_inode_t find_actor, void *opaque)
+/*
+ * This is iget without the read_inode portion of get_new_inode
+ * the filesystem gets back a new locked and hashed inode and gets
+ * to fill it in before unlocking it via unlock_new_inode().
+ */
+struct inode *icreate(struct super_block *sb, unsigned long ino, int (*test)(struct inode *, void *), int (*set)(struct inode *, void *), void *data)
 {
 	struct list_head * head = inode_hashtable + hash(sb,ino);
 	struct inode * inode;
 
 	spin_lock(&inode_lock);
-	inode = find_inode(sb, ino, head, find_actor, opaque);
+	inode = find_inode(sb, ino, head, test, data);
 	if (inode) {
 		__iget(inode);
 		spin_unlock(&inode_lock);
@@ -984,12 +995,11 @@
 	}
 	spin_unlock(&inode_lock);
 
-	/*
-	 * get_new_inode() will do the right thing, re-trying the search
-	 * in case it had to block at any point.
-	 */
-	return get_new_inode(sb, ino, head, find_actor, opaque);
+	return get_new_inode(sb, ino, head, test, set, data);
 }
+
+EXPORT_SYMBOL(icreate);
+EXPORT_SYMBOL(unlock_new_inode);
 
 /**
  *	insert_inode_hash - hash an inode
diff -urN orig/fs/nfs/inode.c icreate6/fs/nfs/inode.c
--- orig/fs/nfs/inode.c	Mon Apr 29 14:09:02 2002
+++ icreate6/fs/nfs/inode.c	Tue Apr 30 12:40:22 2002
@@ -592,7 +592,7 @@
  * i_ino.
  */
 static int
-nfs_find_actor(struct inode *inode, unsigned long ino, void *opaque)
+nfs_find_actor(struct inode *inode, void *opaque)
 {
 	struct nfs_find_desc	*desc = (struct nfs_find_desc *)opaque;
 	struct nfs_fh		*fh = desc->fh;
@@ -610,6 +610,18 @@
 	return 1;
 }
 
+static int
+nfs_init_locked(struct inode *inode, void *opaque)
+{
+	struct nfs_find_desc	*desc = (struct nfs_find_desc *)opaque;
+	struct nfs_fh		*fh = desc->fh;
+	struct nfs_fattr	*fattr = desc->fattr;
+
+	NFS_FILEID(inode) = fattr->fileid;
+	memcpy(NFS_FH(inode), fh, sizeof(struct nfs_fh));
+	return 0;
+}
+
 /*
  * This is our own version of iget that looks up inodes by file handle
  * instead of inode number.  We use this technique instead of using
@@ -652,18 +664,15 @@
 
 	ino = nfs_fattr_to_ino_t(fattr);
 
-	if (!(inode = iget4(sb, ino, nfs_find_actor, &desc)))
+	if (!(inode = icreate(sb, ino, nfs_find_actor, nfs_init_locked, &desc)))
 		goto out_no_inode;
 
-	if (NFS_NEW(inode)) {
+	if (inode->i_state & I_NEW) {
 		__u64		new_size, new_mtime;
 		loff_t		new_isize;
 		time_t		new_atime;
 
 		/* We can't support UPDATE_ATIME(), since the server will reset it */
-		NFS_FLAGS(inode) &= ~NFS_INO_NEW;
-		NFS_FILEID(inode) = fattr->fileid;
-		memcpy(NFS_FH(inode), fh, sizeof(struct nfs_fh));
 		inode->i_flags |= S_NOATIME;
 		inode->i_mode = fattr->mode;
 		/* Why so? Because we want revalidate for devices/FIFOs, and
@@ -711,6 +720,8 @@
 		NFS_ATTRTIMEO(inode) = NFS_MINATTRTIMEO(inode);
 		NFS_ATTRTIMEO_UPDATE(inode) = jiffies;
 		memset(NFS_COOKIEVERF(inode), 0, sizeof(NFS_COOKIEVERF(inode)));
+
+		unlock_new_inode(inode);
 	} else
 		nfs_refresh_inode(inode, fattr);
 	dprintk("NFS: __nfs_fhget(%s/%Ld ct=%d)\n",
@@ -1230,7 +1241,6 @@
 	nfsi = (struct nfs_inode *)kmem_cache_alloc(nfs_inode_cachep, SLAB_KERNEL);
 	if (!nfsi)
 		return NULL;
-	nfsi->flags = NFS_INO_NEW;
 	nfsi->mm_cred = NULL;
 	return &nfsi->vfs_inode;
 }
diff -urN orig/fs/reiserfs/inode.c icreate6/fs/reiserfs/inode.c
--- orig/fs/reiserfs/inode.c	Mon Apr 29 14:10:15 2002
+++ icreate6/fs/reiserfs/inode.c	Tue Apr 30 12:58:13 2002
@@ -33,7 +33,7 @@
     lock_kernel() ; 
 
     /* The = 0 happens when we abort creating a new inode for some reason like lack of space.. */
-    if (INODE_PKEY(inode)->k_objectid != 0) { /* also handles bad_inode case */
+    if (!(inode->i_state & I_NEW) && INODE_PKEY(inode)->k_objectid != 0) { /* also handles bad_inode case */
 	down (&inode->i_sem); 
 
 	journal_begin(&th, inode->i_sb, jbegin_count) ;
@@ -1134,19 +1134,20 @@
 
 /* looks for stat data in the tree, and fills up the fields of in-core
    inode stat data fields */
-void reiserfs_read_inode2 (struct inode * inode, void *p)
+int reiserfs_init_locked_inode (struct inode * inode, void *p)
+{
+	struct reiserfs_icreate_args *args = (struct reiserfs_icreate_args *)p ;
+	INODE_PKEY(inode)->k_dir_id = cpu_to_le32(args->objectid);
+	return 0;
+}
+
+void reiserfs_read_locked_inode (struct inode * inode, struct reiserfs_icreate_args *args)
 {
     INITIALIZE_PATH (path_to_sd);
     struct cpu_key key;
-    struct reiserfs_iget4_args *args = (struct reiserfs_iget4_args *)p ;
     unsigned long dirino;
     int retval;
 
-    if (!p) {
-	reiserfs_make_bad_inode(inode) ;
-	return;
-    }
-
     dirino = args->objectid ;
 
     /* set version 1, version 2 could be used too, because stat data
@@ -1204,22 +1205,21 @@
 }
 
 /**
- * reiserfs_find_actor() - "find actor" reiserfs supplies to iget4().
+ * reiserfs_find_actor() - "find actor" reiserfs supplies to icreate().
  *
  * @inode:    inode from hash table to check
  * @inode_no: inode number we are looking for
- * @opaque:   "cookie" passed to iget4(). This is &reiserfs_iget4_args.
+ * @opaque:   "cookie" passed to icreate(). This is &reiserfs_icreate_args.
  *
- * This function is called by iget4() to distinguish reiserfs inodes
+ * This function is called by icreate() to distinguish reiserfs inodes
  * having the same inode numbers. Such inodes can only exist due to some
  * error condition. One of them should be bad. Inodes with identical
  * inode numbers (objectids) are distinguished by parent directory ids.
  *
  */
-static int reiserfs_find_actor( struct inode *inode, 
-				unsigned long inode_no, void *opaque )
+static int reiserfs_find_actor( struct inode *inode, void *opaque )
 {
-    struct reiserfs_iget4_args *args;
+    struct reiserfs_icreate_args *args;
 
     args = opaque;
     /* args is already in CPU order */
@@ -1229,13 +1229,18 @@
 struct inode * reiserfs_iget (struct super_block * s, const struct cpu_key * key)
 {
     struct inode * inode;
-    struct reiserfs_iget4_args args ;
+    struct reiserfs_icreate_args args ;
 
     args.objectid = key->on_disk_key.k_dir_id ;
-    inode = iget4 (s, key->on_disk_key.k_objectid, 
-		   reiserfs_find_actor, (void *)(&args));
+    inode = icreate(s, key->on_disk_key.k_objectid, 
+		    reiserfs_find_actor, reiserfs_init_locked_inode, (void *)(&args));
     if (!inode) 
 	return ERR_PTR(-ENOMEM) ;
+
+    if (inode->i_state & I_NEW) {
+	reiserfs_read_locked_inode(inode, &args);
+	unlock_new_inode(inode);
+    }
 
     if (comp_short_keys (INODE_PKEY (inode), key) || is_bad_inode (inode)) {
 	/* either due to i/o error or a stale NFS handle */
diff -urN orig/fs/reiserfs/super.c icreate6/fs/reiserfs/super.c
--- orig/fs/reiserfs/super.c	Mon Apr 29 14:09:03 2002
+++ icreate6/fs/reiserfs/super.c	Tue Apr 30 12:58:18 2002
@@ -485,7 +485,6 @@
   alloc_inode: reiserfs_alloc_inode,
   destroy_inode: reiserfs_destroy_inode,
   read_inode: reiserfs_read_inode,
-  read_inode2: reiserfs_read_inode2,
   write_inode: reiserfs_write_inode,
   dirty_inode: reiserfs_dirty_inode,
   delete_inode: reiserfs_delete_inode,
@@ -1002,7 +1001,7 @@
     int old_format = 0;
     unsigned long blocks;
     int jinit_done = 0 ;
-    struct reiserfs_iget4_args args ;
+    struct reiserfs_icreate_args args ;
     struct reiserfs_super_block * rs;
     char *jdev_name;
     struct reiserfs_sb_info *sbi;
@@ -1065,10 +1064,15 @@
 	s->s_flags |= MS_RDONLY ;
     }
     args.objectid = REISERFS_ROOT_PARENT_OBJECTID ;
-    root_inode = iget4 (s, REISERFS_ROOT_OBJECTID, 0, (void *)(&args));
+    root_inode = icreate (s, REISERFS_ROOT_OBJECTID, 0, reiserfs_init_locked_inode, (void *)(&args));
     if (!root_inode) {
 	printk ("reiserfs_fill_super: get root inode failed\n");
 	goto error;
+    }
+
+    if (root_inode->i_state & I_NEW) {
+	reiserfs_read_locked_inode(root_inode, &args);
+	unlock_new_inode(root_inode);
     }
 
     s->s_root = d_alloc_root(root_inode);  
diff -urN orig/include/linux/fs.h icreate6/include/linux/fs.h
--- orig/include/linux/fs.h	Mon Apr 29 14:11:21 2002
+++ icreate6/include/linux/fs.h	Tue Apr 30 11:31:00 2002
@@ -852,13 +852,6 @@
 
 	void (*read_inode) (struct inode *);
   
-  	/* reiserfs kludge.  reiserfs needs 64 bits of information to
-    	** find an inode.  We are using the read_inode2 call to get
-   	** that information.  We don't like this, and are waiting on some
-   	** VFS changes for the real solution.
-   	** iget4 calls read_inode2, iff it is defined
-   	*/
-    	void (*read_inode2) (struct inode *, void *) ;
    	void (*dirty_inode) (struct inode *);
 	void (*write_inode) (struct inode *, int);
 	void (*put_inode) (struct inode *);
@@ -906,6 +899,7 @@
 #define I_LOCK			8
 #define I_FREEING		16
 #define I_CLEAR			32
+#define I_NEW			64
 
 #define I_DIRTY (I_DIRTY_SYNC | I_DIRTY_DATASYNC | I_DIRTY_PAGES)
 
@@ -1442,11 +1436,21 @@
 extern struct inode * igrab(struct inode *);
 extern ino_t iunique(struct super_block *, ino_t);
 
-typedef int (*find_inode_t)(struct inode *, unsigned long, void *);
-extern struct inode * iget4(struct super_block *, unsigned long, find_inode_t, void *);
+extern struct inode * icreate(struct super_block *, unsigned long, int (*test)(struct inode *, void *), int (*set)(struct inode *, void *), void *data);
+extern void unlock_new_inode(struct inode *);
+
 static inline struct inode *iget(struct super_block *sb, unsigned long ino)
 {
-	return iget4(sb, ino, NULL, NULL);
+	struct inode *inode;
+
+	inode = icreate(sb, ino, NULL, NULL, NULL);
+
+	if (inode && (inode->i_state & I_NEW)) {
+		sb->s_op->read_inode(inode);
+		unlock_new_inode(inode);
+	}
+
+	return inode;
 }
 
 extern void clear_inode(struct inode *);
diff -urN orig/include/linux/nfs_fs.h icreate6/include/linux/nfs_fs.h
--- orig/include/linux/nfs_fs.h	Tue Apr 30 12:10:17 2002
+++ icreate6/include/linux/nfs_fs.h	Tue Apr 30 12:40:58 2002
@@ -170,7 +170,6 @@
 #define NFS_INO_REVALIDATING	0x0004		/* revalidating attrs */
 #define NFS_IS_SNAPSHOT		0x0010		/* a snapshot file */
 #define NFS_INO_FLUSH		0x0020		/* inode is due for flushing */
-#define NFS_INO_NEW		0x0040		/* hadn't been filled yet */
 
 static inline struct nfs_inode *NFS_I(struct inode *inode)
 {
@@ -208,7 +207,6 @@
 #define NFS_FLAGS(inode)		(NFS_I(inode)->flags)
 #define NFS_REVALIDATING(inode)		(NFS_FLAGS(inode) & NFS_INO_REVALIDATING)
 #define NFS_STALE(inode)		(NFS_FLAGS(inode) & NFS_INO_STALE)
-#define NFS_NEW(inode)			(NFS_FLAGS(inode) & NFS_INO_NEW)
 
 #define NFS_FILEID(inode)		(NFS_I(inode)->fileid)
 
diff -urN orig/include/linux/reiserfs_fs.h icreate6/include/linux/reiserfs_fs.h
--- orig/include/linux/reiserfs_fs.h	Mon Apr 29 14:10:33 2002
+++ icreate6/include/linux/reiserfs_fs.h	Tue Apr 30 12:58:05 2002
@@ -1564,7 +1564,7 @@
 #define B_I_POS_UNFM_POINTER(bh,ih,pos) le32_to_cpu(*(((unp_t *)B_I_PITEM(bh,ih)) + (pos)))
 #define PUT_B_I_POS_UNFM_POINTER(bh,ih,pos, val) do {*(((unp_t *)B_I_PITEM(bh,ih)) + (pos)) = cpu_to_le32(val); } while (0)
 
-struct reiserfs_iget4_args {
+struct reiserfs_icreate_args {
     __u32 objectid ;
 } ;
 
@@ -1819,7 +1819,8 @@
 /* inode.c */
 
 void reiserfs_read_inode (struct inode * inode) ;
-void reiserfs_read_inode2(struct inode * inode, void *p) ;
+int reiserfs_init_locked_inode(struct inode * inode, void *p) ;
+void reiserfs_read_locked_inode(struct inode * inode, struct reiserfs_icreate_args *args) ;
 void reiserfs_delete_inode (struct inode * inode);
 void reiserfs_write_inode (struct inode * inode, int) ;
 struct dentry *reiserfs_fh_to_dentry(struct super_block *sb, __u32 *data,
diff -urN orig/kernel/ksyms.c icreate6/kernel/ksyms.c
--- orig/kernel/ksyms.c	Mon Apr 29 14:11:22 2002
+++ icreate6/kernel/ksyms.c	Tue Apr 30 11:31:00 2002
@@ -137,7 +137,6 @@
 EXPORT_SYMBOL(fget);
 EXPORT_SYMBOL(igrab);
 EXPORT_SYMBOL(iunique);
-EXPORT_SYMBOL(iget4);
 EXPORT_SYMBOL(iput);
 EXPORT_SYMBOL(inode_init_once);
 EXPORT_SYMBOL(force_delete);

Re: [PATCH/RFC] Replacing iget4/read_inode2 with icreate

[Alexander Viro]

On Tue, 30 Apr 2002, Jan Harkes wrote:

> On Tue, Apr 30, 2002 at 01:03:46PM -0400, Jan Harkes wrote:
> Thought I had already sent the latest version of the patch before I went
> home...
> 
> This is the 6th iteration of the patch and it pulls everything together
> as well as adding a bit to Documentation/filesystems/porting.

Why are you passing ino?  If anything, it can be derived from the
last argument (and set by set())...

I would probably add two helpers - icreate() (as in your variant, but
without ino) and icreate_light() - with ino, but without last 3 arguments.

And then switched callers of iget() to icreate_liget()...

When some filesystem stops using iget() we can drop its ->read_inode().
When all of them are converted we can kill the method itself.

Re: [PATCH/RFC] Replacing iget4/read_inode2 with icreate

[Jan Harkes]

On Tue, Apr 30, 2002 at 11:25:43PM -0400, Alexander Viro wrote:
> On Tue, 30 Apr 2002, Jan Harkes wrote:
> > On Tue, Apr 30, 2002 at 01:03:46PM -0400, Jan Harkes wrote:
> > Thought I had already sent the latest version of the patch before I went
> > home...
> > 
> > This is the 6th iteration of the patch and it pulls everything together
> > as well as adding a bit to Documentation/filesystems/porting.
> 
> Why are you passing ino?  If anything, it can be derived from the
> last argument (and set by set())...

Ehh, the filesystem might know how to hash the last argument to an
inode number, but how else would we pick the right chain out of the
inode_hashtable? If we only pass the opaque data, the VFS will have to
pass all inodes associated with a superblock to the test function.

Adding another callback that can hash the last argument, or a length so
that the VFS can do a checksum of the data seems a bit overkill if we
can just as well pass ino.

> When some filesystem stops using iget() we can drop its ->read_inode().
> When all of them are converted we can kill the method itself.

They can use icreate, by passing NULL for the last three arguments and
checking for I_NEW the way I've described in "porting".

Jan

Re: [PATCH/RFC] Replacing iget4/read_inode2 with icreate

[Kai Henningsen]

jaharkes@cs.cmu.edu (Jan Harkes)  wrote on 30.04.02 in <20020501024236.GA1878@mentor.odyssey.cs.cmu.edu>:

> +iget4() and the read_inode2 callback have been superceded by icreate()
> +which has the following prototype,
> +
> +    struct inode *icreate(struct super_block *sb, unsigned long ino,
> +			  void (*test)(struct inode *, void *),

Surely test doesn't return a void?!

MfG Kai