[PATCH] iget_locked [5/6]

This patch starts taking i_ino dependencies out of the VFS. The FS
provided test and set callbacks become responsible for testing and
setting inode->i_ino.

Because most filesystems are based on 32-bit unique inode numbers
several functions are duplicated to keep iget_locked as a fast path. We
can avoid unnecessary pointer dereferences and function calls for this
specific case.

Documentation/filesystems/porting

@@ -175,8 +175,10 @@ 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 it must be unlocked by calling unlock_new_inode().
 
-There is also a simpler iget_locked function that just takes the
-superblock and inode number as arguments.
+The filesystem is responsible for setting (and possibly testing) i_ino
+when appropriate. There is also a simpler iget_locked function that
+just takes the superblock and inode number as arguments and does the
+test and set for you.
 
 e.g.
        inode = iget_locked(sb, ino);

fs/coda/cnode.c

@@ -83,6 +83,7 @@ struct inode * coda_iget(struct super_block * sb, ViceFid * fid,
 
 	if (inode->i_state & I_NEW) {
 		cii = ITOC(inode);
+		inode->i_ino = ino;
 		list_add(&cii->c_cilist, &sbi->sbi_cihead);
 		unlock_new_inode(inode);
 	}

fs/inode.c

@@ -453,7 +453,32 @@ int shrink_icache_memory(int priority, int gfp_mask)
  * 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, int (*test)(struct inode *, void *), void *data)
+static struct inode * find_inode(struct super_block * sb, struct list_head *head, int (*test)(struct inode *, void *), void *data)
+{
+	struct list_head *tmp;
+	struct inode * inode;
+
+	tmp = head;
+	for (;;) {
+		tmp = tmp->next;
+		inode = NULL;
+		if (tmp == head)
+			break;
+		inode = list_entry(tmp, struct inode, i_hash);
+		if (inode->i_sb != sb)
+			continue;
+		if (!test(inode, data))
+			continue;
+		break;
+	}
+	return inode;
+}
+
+/*
+ * find_inode_fast is the fast path version of find_inode, see the comment at
+ * iget_locked for details.
+ */
+static struct inode * find_inode_fast(struct super_block * sb, struct list_head *head, unsigned long ino)
 {
 	struct list_head *tmp;
 	struct inode * inode;
@@ -469,8 +494,6 @@ static struct inode * find_inode(struct super_block * sb, unsigned long ino, str
 			continue;
 		if (inode->i_sb != sb)
 			continue;
-		if (test && !test(inode, data))
-			continue;
 		break;
 	}
 	return inode;
@@ -523,10 +546,9 @@ void unlock_new_inode(struct inode *inode)
  * 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, int (*test)(struct inode *, void *), int (*set)(struct inode *, void *), void *data)
+static struct inode * get_new_inode(struct super_block *sb, 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) {
@@ -534,10 +556,9 @@ static struct inode * get_new_inode(struct super_block *sb, unsigned long ino, s
 
 		spin_lock(&inode_lock);
 		/* We released the lock, so.. */
-		old = find_inode(sb, ino, head, test, data);
+		old = find_inode(sb, head, test, data);
 		if (!old) {
-			inode->i_ino = ino;
-			if (set && set(inode, data))
+			if (set(inode, data))
 				goto set_failed;
 
 			inodes_stat.nr_inodes++;
@@ -571,6 +592,49 @@ static struct inode * get_new_inode(struct super_block *sb, unsigned long ino, s
 	return NULL;
 }
 
+/*
+ * get_new_inode_fast is the fast path version of get_new_inode, see the
+ * comment at iget_locked for details.
+ */
+static struct inode * get_new_inode_fast(struct super_block *sb, struct list_head *head, unsigned long ino)
+{
+	struct inode * inode;
+
+	inode = alloc_inode(sb);
+	if (inode) {
+		struct inode * old;
+
+		spin_lock(&inode_lock);
+		/* We released the lock, so.. */
+		old = find_inode_fast(sb, head, ino);
+		if (!old) {
+			inode->i_ino = ino;
+			inodes_stat.nr_inodes++;
+			list_add(&inode->i_list, &inode_in_use);
+			list_add(&inode->i_hash, head);
+			inode->i_state = I_LOCK|I_NEW;
+			spin_unlock(&inode_lock);
+
+			/* Return the locked inode with I_NEW set, the
+			 * caller is responsible for filling in the contents
+			 */
+			return inode;
+		}
+
+		/*
+		 * Uhhuh, somebody else created the same inode under
+		 * us. Use the old inode instead of the one we just
+		 * allocated.
+		 */
+		__iget(old);
+		spin_unlock(&inode_lock);
+		destroy_inode(inode);
+		inode = old;
+		wait_on_inode(inode);
+	}
+	return inode;
+}
+
 static inline unsigned long hash(struct super_block *sb, unsigned long i_ino)
 {
 	unsigned long tmp = i_ino + ((unsigned long) sb / L1_CACHE_BYTES);
@@ -605,7 +669,8 @@ ino_t iunique(struct super_block *sb, ino_t max_reserved)
 retry:
 	if (counter > max_reserved) {
 		head = inode_hashtable + hash(sb,counter);
-		inode = find_inode(sb, res = counter++, head, NULL, NULL);
+		res = counter++;
+		inode = find_inode_fast(sb, head, res);
 		if (!inode) {
 			spin_unlock(&inode_lock);
 			return res;
@@ -644,7 +709,7 @@ struct inode *iget5_locked(struct super_block *sb, unsigned long ino, int (*test
 	struct inode * inode;
 
 	spin_lock(&inode_lock);
-	inode = find_inode(sb, ino, head, test, data);
+	inode = find_inode(sb, head, test, data);
 	if (inode) {
 		__iget(inode);
 		spin_unlock(&inode_lock);
@@ -657,12 +722,36 @@ struct inode *iget5_locked(struct super_block *sb, unsigned long ino, int (*test
 	 * 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, test, set, data);
+	return get_new_inode(sb, head, test, set, data);
 }
 
+/*
+ * Because most filesystems are based on 32-bit unique inode numbers some
+ * functions are duplicated to keep iget_locked as a fast path. We can avoid
+ * unnecessary pointer dereferences and function calls for this specific
+ * case. The duplicated functions (find_inode_fast and get_new_inode_fast)
+ * have the same pre- and post-conditions as their original counterparts.
+ */
 struct inode *iget_locked(struct super_block *sb, unsigned long ino)
 {
-	return iget5_locked(sb, ino, NULL, NULL, NULL);
+	struct list_head * head = inode_hashtable + hash(sb, ino);
+	struct inode * inode;
+
+	spin_lock(&inode_lock);
+	inode = find_inode_fast(sb, head, ino);
+	if (inode) {
+		__iget(inode);
+		spin_unlock(&inode_lock);
+		wait_on_inode(inode);
+		return inode;
+	}
+	spin_unlock(&inode_lock);
+
+	/*
+	 * get_new_inode_fast() will do the right thing, re-trying the search
+	 * in case it had to block at any point.
+	 */
+	return get_new_inode_fast(sb, head, ino);
 }
 
 EXPORT_SYMBOL(iget5_locked);

fs/nfs/inode.c

@@ -661,6 +661,8 @@ __nfs_fhget(struct super_block *sb, struct nfs_fh *fh, struct nfs_fattr *fattr)
 		loff_t		new_isize;
 		time_t		new_atime;
 
+		inode->i_ino = ino;
+
 		/* We can't support UPDATE_ATIME(), since the server will reset it */
 		inode->i_flags |= S_NOATIME;
 		inode->i_mode = fattr->mode;

fs/reiserfs/inode.c

@@ -1136,7 +1136,8 @@ static void reiserfs_make_bad_inode(struct inode *inode) {
 int reiserfs_init_locked_inode (struct inode * inode, void *p)
 {
     struct reiserfs_iget_args *args = (struct reiserfs_iget_args *)p ;
-    INODE_PKEY(inode)->k_dir_id = cpu_to_le32(args->objectid);
+    inode->i_ino = args->objectid;
+    INODE_PKEY(inode)->k_dir_id = cpu_to_le32(args->dirid);
     return 0;
 }
 
@@ -1149,7 +1150,7 @@ void reiserfs_read_locked_inode (struct inode * inode, struct reiserfs_iget_args
     unsigned long dirino;
     int retval;
 
-    dirino = args->objectid ;
+    dirino = args->dirid ;
 
     /* set version 1, version 2 could be used too, because stat data
        key is the same in both versions */
@@ -1223,7 +1224,8 @@ int reiserfs_find_actor( struct inode *inode, void *opaque )
 
     args = opaque;
     /* args is already in CPU order */
-    return le32_to_cpu(INODE_PKEY(inode)->k_dir_id) == args -> objectid;
+    return (inode->i_ino == args->objectid) &&
+	(le32_to_cpu(INODE_PKEY(inode)->k_dir_id) == args->dirid);
 }
 
 struct inode * reiserfs_iget (struct super_block * s, const struct cpu_key * key)
@@ -1231,7 +1233,8 @@ struct inode * reiserfs_iget (struct super_block * s, const struct cpu_key * key
     struct inode * inode;
     struct reiserfs_iget_args args ;
 
-    args.objectid = key->on_disk_key.k_dir_id ;
+    args.objectid = key->on_disk_key.k_objectid ;
+    args.dirid = key->on_disk_key.k_dir_id ;
     inode = iget5_locked (s, key->on_disk_key.k_objectid, 
 		   reiserfs_find_actor, reiserfs_init_locked_inode, (void *)(&args));
     if (!inode) 

fs/reiserfs/super.c

@@ -1067,7 +1067,8 @@ static int reiserfs_fill_super(struct super_block *s, void *data, int silent)
         printk("clm-7000: Detected readonly device, marking FS readonly\n") ;
 	s->s_flags |= MS_RDONLY ;
     }
-    args.objectid = REISERFS_ROOT_PARENT_OBJECTID ;
+    args.objectid = REISERFS_ROOT_OBJECTID ;
+    args.dirid = REISERFS_ROOT_PARENT_OBJECTID ;
     root_inode = iget5_locked (s, REISERFS_ROOT_OBJECTID, reiserfs_find_actor, reiserfs_init_locked_inode, (void *)(&args));
     if (!root_inode) {
 	printk ("reiserfs_fill_super: get root inode failed\n");

include/linux/reiserfs_fs.h

@@ -1566,6 +1566,7 @@ extern struct item_operations * item_ops [TYPE_ANY + 1];
 
 struct reiserfs_iget_args {
     __u32 objectid ;
+    __u32 dirid ;
 } ;
 
 /***************************************************************************/