[PATCH] fs: kill get_new_inode{,_fast}

* [PATCH] fs: kill get_new_inode{,_fast}
@ 2010-10-23 17:44 Christoph Hellwig
  2010-10-24  0:07 ` Christian Stroetmann
  0 siblings, 1 reply; 2+ messages in thread
From: Christoph Hellwig @ 2010-10-23 17:44 UTC (permalink / raw)
  To: viro; +Cc: linux-fsdevel

There's very little point in these helpers.  One caller each
doing a tailcall with trivial amounts of code before it.

Merging into callers also makes use of the old vs inode variables
more obvious.

Also fix up kerneldoc comments to focus on what we are doing and
not how it's done.

Signed-off-by: Christoph Hellwig <hch@lst.de>

Index: linux-2.6/fs/inode.c
===================================================================

--- linux-2.6.orig/fs/inode.c	2010-10-23 19:05:01.114003338 +0200
+++ linux-2.6/fs/inode.c	2010-10-23 19:40:28.740005644 +0200
@@ -819,102 +819,6 @@ void unlock_new_inode(struct inode *inod
 EXPORT_SYMBOL(unlock_new_inode);
 
 /*
- * 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,
-				struct hlist_head *head,
-				int (*test)(struct inode *, void *),
-				int (*set)(struct inode *, void *),
-				void *data)
-{
-	struct inode *inode;
-
-	inode = alloc_inode(sb);
-	if (inode) {
-		struct inode *old;
-
-		spin_lock(&inode_lock);
-		/* We released the lock, so.. */
-		old = find_inode(sb, head, test, data);
-		if (!old) {
-			if (set(inode, data))
-				goto set_failed;
-
-			hlist_add_head(&inode->i_hash, head);
-			__inode_sb_list_add(inode);
-			inode->i_state = 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.
-		 */
-		spin_unlock(&inode_lock);
-		destroy_inode(inode);
-		inode = old;
-		wait_on_inode(inode);
-	}
-	return inode;
-
-set_failed:
-	spin_unlock(&inode_lock);
-	destroy_inode(inode);
-	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 hlist_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;
-			hlist_add_head(&inode->i_hash, head);
-			__inode_sb_list_add(inode);
-			inode->i_state = 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.
-		 */
-		spin_unlock(&inode_lock);
-		destroy_inode(inode);
-		inode = old;
-		wait_on_inode(inode);
-	}
-	return inode;
-}
-
-/*
  * search the inode cache for a matching inode number.
  * If we find one, then the inode number we are trying to
  * allocate is not unique and so we should not use it.
@@ -1147,15 +1051,14 @@ EXPORT_SYMBOL(ilookup);
  * @set:	callback used to initialize a new struct inode
  * @data:	opaque data pointer to pass to @test and @set
  *
- * iget5_locked() uses ifind() to search for the inode specified by @hashval
- * and @data in the inode cache and if present it is returned with an increased
- * reference count. This is a generalized version of iget_locked() for file
- * systems where the inode number is not sufficient for unique identification
- * of an inode.
+ * Search for the inode specified by @hashval and @data in the inode cache,
+ * and if present return it with an increased reference count.  If the inode
+ * is not in cache, allocate a new inode and returned it hashed and with the
+ * I_NEW flag set.  The file system gets to fill the inode in before unlocking
+ * it via unlock_new_inode().
  *
- * If the inode is not in cache, get_new_inode() is called to allocate a new
- * inode and this is returned locked, hashed, and with the I_NEW flag set. The
- * file system gets to fill it in before unlocking it via unlock_new_inode().
+ * This is a generalized version of iget_locked() for file systems where the
+ * inode number is not sufficient for unique identification of an inode.
  *
  * Note both @test and @set are called with the inode_lock held, so can't sleep.
  */
@@ -1164,16 +1067,48 @@ struct inode *iget5_locked(struct super_
 		int (*set)(struct inode *, void *), void *data)
 {
 	struct hlist_head *head = inode_hashtable + hash(sb, hashval);
-	struct inode *inode;
+	struct inode *inode, *old;
+
+	old = ifind(sb, head, test, data, 1);
+	if (old)
+		return old;
+
+	inode = alloc_inode(sb);
+	if (!inode)
+		return NULL;
+
+	spin_lock(&inode_lock);
+	/* We released the lock, so.. */
+	old = find_inode(sb, head, test, data);
+	if (old) {
+		/*
+		 * Uhhuh, somebody else created the same inode under
+		 * us. Use the old inode instead of the one we just
+		 * allocated.
+		 */
+		spin_unlock(&inode_lock);
+		destroy_inode(inode);
+		wait_on_inode(old);
+		return old;
+	}
+
+	if (set(inode, data))
+		goto set_failed;
+
+	hlist_add_head(&inode->i_hash, head);
+	__inode_sb_list_add(inode);
+	inode->i_state = I_NEW;
+	spin_unlock(&inode_lock);
 
-	inode = ifind(sb, head, test, data, 1);
-	if (inode)
-		return inode;
 	/*
-	 * get_new_inode() will do the right thing, re-trying the search
-	 * in case it had to block at any point.
+	 * Return the locked inode with I_NEW set, the caller is responsible for
+	 * filling in the contents
 	 */
-	return get_new_inode(sb, head, test, set, data);
+	return inode;
+set_failed:
+	spin_unlock(&inode_lock);
+	destroy_inode(inode);
+	return NULL;
 }
 EXPORT_SYMBOL(iget5_locked);
 
@@ -1182,29 +1117,51 @@ EXPORT_SYMBOL(iget5_locked);
  * @sb:		super block of file system
  * @ino:	inode number to get
  *
- * iget_locked() uses ifind_fast() to search for the inode specified by @ino in
- * the inode cache and if present it is returned with an increased reference
- * count. This is for file systems where the inode number is sufficient for
- * unique identification of an inode.
- *
- * If the inode is not in cache, get_new_inode_fast() is called to allocate a
- * new inode and this is returned locked, hashed, and with the I_NEW flag set.
- * The file system gets to fill it in before unlocking it via
+ * Search for the inode specified by @ino in the inode cache, and if present
+ * return it with an increased reference count.  If the inode is not in cache,
+ * allocate a new inode and returned it hashed and with the I_NEW flag set.
+ * The file system gets to fill the inode in before unlocking it via
  * unlock_new_inode().
  */
 struct inode *iget_locked(struct super_block *sb, unsigned long ino)
 {
 	struct hlist_head *head = inode_hashtable + hash(sb, ino);
-	struct inode *inode;
+	struct inode *inode, *old;
+
+	old = ifind_fast(sb, head, ino);
+	if (old)
+		return old;
+
+	inode = alloc_inode(sb);
+	if (!inode)
+		return NULL;
+
+	spin_lock(&inode_lock);
+	/* We released the lock, so.. */
+	old = find_inode_fast(sb, head, ino);
+	if (old) {
+		/*
+		 * Uhhuh, somebody else created the same inode under
+		 * us. Use the old inode instead of the one we just
+		 * allocated.
+		 */
+		spin_unlock(&inode_lock);
+		destroy_inode(inode);
+		wait_on_inode(old);
+		return old;
+	}
+
+	inode->i_ino = ino;
+	hlist_add_head(&inode->i_hash, head);
+	__inode_sb_list_add(inode);
+	inode->i_state = I_NEW;
+	spin_unlock(&inode_lock);
 
-	inode = ifind_fast(sb, head, ino);
-	if (inode)
-		return inode;
 	/*
-	 * get_new_inode_fast() will do the right thing, re-trying the search
-	 * in case it had to block at any point.
+	 * Return the locked inode with I_NEW set, the caller is responsible for
+	 * filling in the contents
 	 */
-	return get_new_inode_fast(sb, head, ino);
+	return inode;
 }
 EXPORT_SYMBOL(iget_locked);
 
Index: linux-2.6/fs/ocfs2/inode.c
===================================================================
--- linux-2.6.orig/fs/ocfs2/inode.c	2010-10-23 19:10:36.043010811 +0200
+++ linux-2.6/fs/ocfs2/inode.c	2010-10-23 19:10:48.961026523 +0200
@@ -183,7 +183,7 @@ bail:
  * here's how inodes get read from disk:
  * iget5_locked -> find_actor -> OCFS2_FIND_ACTOR
  * found? : return the in-memory inode
- * not found? : get_new_inode -> OCFS2_INIT_LOCKED_INODE
+ * not found? : iget5_locked -> OCFS2_INIT_LOCKED_INODE
  */
 
 static int ocfs2_find_actor(struct inode *inode, void *opaque)
Index: linux-2.6/fs/udf/inode.c
===================================================================
--- linux-2.6.orig/fs/udf/inode.c	2010-10-23 19:10:52.729253443 +0200
+++ linux-2.6/fs/udf/inode.c	2010-10-23 19:11:12.188005500 +0200
@@ -1065,9 +1065,9 @@ static void __udf_read_inode(struct inod
 
 	/*
 	 * Set defaults, but the inode is still incomplete!
-	 * Note: get_new_inode() sets the following on a new inode:
+	 * Note: iget_locked() sets the following on a new inode:
 	 *      i_sb = sb
-	 *      i_no = ino
+	 *      i_ino = ino
 	 *      i_flags = sb->s_flags
 	 *      i_state = 0
 	 * clean_inode(): zero fills and sets

^ permalink raw reply	[flat|nested] 2+ messages in thread