Re: [PATCH] vfs: remove the unnecessrary code of fs/inode.c

[Dong Fang <yp.fangdong@xxxxxxxxx>]

On 07/02/2013 12:15 AM, Gu Zheng wrote:
> On 07/01/2013 08:19 PM, Dong Fang wrote:
>
> > These functions, such as find_inode_fast() and find_inode(), iget_lock() and
> > iget5_lock(), insert_inode_locked() and insert_inode_locked4(), almost have
> > the same code.
>
> Maybe the title "[PATCH] vfs: remove the reduplicate code of fs/inode.c" is more
> suitable.

that's right, thanks for your advice

> > Signed-off-by: Dong Fang <yp.fangdong@xxxxxxxxx>
>
>
> Reviewed-by: Gu Zheng <guz.fnst@xxxxxxxxxxxxxx>
>
> Thanks,
> Gu
>
> > ---
> >   fs/inode.c |  134 ++++++++++++------------------------------------------------
> >   1 files changed, 26 insertions(+), 108 deletions(-)
> >
> > diff --git a/fs/inode.c b/fs/inode.c
> > index 00d5fc3..847eee9 100644
> > --- a/fs/inode.c
> > +++ b/fs/inode.c
> > @@ -790,6 +790,22 @@ void prune_icache_sb(struct super_block *sb, int nr_to_scan)
> >   }
> >
> >   static void __wait_on_freeing_inode(struct inode *inode);
> > +
> > +
> > +static int test_ino(struct inode *inode, void *data)
> > +{
> > +	unsigned long ino = *(unsigned long *) data;
> > +	return inode->i_ino == ino;
>
> Can be more concise:
> 	return inode->i_ino == *(unsigned long *) data;
> ,so does the new insert_inode_locked():
> 	
>
> > +}
> > +
> > +static int set_ino(struct inode *inode, void *data)
> > +{
> > +	inode->i_ino = *(unsigned long *) data;
> > +	return 0;
> > +}
> > +
> > +
> > +
> >   /*
> >    * Called with the inode lock held.
> >    */
> > @@ -829,28 +845,7 @@ repeat:
> >   static struct inode *find_inode_fast(struct super_block *sb,
> >   				struct hlist_head *head, unsigned long ino)
> >   {
> > -	struct inode *inode = NULL;
> > -
> > -repeat:
> > -	hlist_for_each_entry(inode, head, i_hash) {
> > -		spin_lock(&inode->i_lock);
> > -		if (inode->i_ino != ino) {
> > -			spin_unlock(&inode->i_lock);
> > -			continue;
> > -		}
> > -		if (inode->i_sb != sb) {
> > -			spin_unlock(&inode->i_lock);
> > -			continue;
> > -		}
> > -		if (inode->i_state & (I_FREEING|I_WILL_FREE)) {
> > -			__wait_on_freeing_inode(inode);
> > -			goto repeat;
> > -		}
> > -		__iget(inode);
> > -		spin_unlock(&inode->i_lock);
> > -		return inode;
> > -	}
> > -	return NULL;
> > +	return find_inode(sb, head, test_ino, (void *)&ino);
> >   }
> >
> >   /*
> > @@ -1073,50 +1068,7 @@ EXPORT_SYMBOL(iget5_locked);
> >    */
> >   struct inode *iget_locked(struct super_block *sb, unsigned long ino)
> >   {
> > -	struct hlist_head *head = inode_hashtable + hash(sb, ino);
> > -	struct inode *inode;
> > -
> > -	spin_lock(&inode_hash_lock);
> > -	inode = find_inode_fast(sb, head, ino);
> > -	spin_unlock(&inode_hash_lock);
> > -	if (inode) {
> > -		wait_on_inode(inode);
> > -		return inode;
> > -	}
> > -
> > -	inode = alloc_inode(sb);
> > -	if (inode) {
> > -		struct inode *old;
> > -
> > -		spin_lock(&inode_hash_lock);
> > -		/* We released the lock, so.. */
> > -		old = find_inode_fast(sb, head, ino);
> > -		if (!old) {
> > -			inode->i_ino = ino;
> > -			spin_lock(&inode->i_lock);
> > -			inode->i_state = I_NEW;
> > -			hlist_add_head(&inode->i_hash, head);
> > -			spin_unlock(&inode->i_lock);
> > -			inode_sb_list_add(inode);
> > -			spin_unlock(&inode_hash_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_hash_lock);
> > -		destroy_inode(inode);
> > -		inode = old;
> > -		wait_on_inode(inode);
> > -	}
> > -	return inode;
> > +	return iget5_locked(sb, ino, test_ino, set_ino, (void *)&ino);
> >   }
> >   EXPORT_SYMBOL(iget_locked);
> >
> > @@ -1281,48 +1233,6 @@ struct inode *ilookup(struct super_block *sb, unsigned long ino)
> >   }
> >   EXPORT_SYMBOL(ilookup);
> >
> > -int insert_inode_locked(struct inode *inode)
> > -{
> > -	struct super_block *sb = inode->i_sb;
> > -	ino_t ino = inode->i_ino;
> > -	struct hlist_head *head = inode_hashtable + hash(sb, ino);
> > -
> > -	while (1) {
> > -		struct inode *old = NULL;
> > -		spin_lock(&inode_hash_lock);
> > -		hlist_for_each_entry(old, head, i_hash) {
> > -			if (old->i_ino != ino)
> > -				continue;
> > -			if (old->i_sb != sb)
> > -				continue;
> > -			spin_lock(&old->i_lock);
> > -			if (old->i_state & (I_FREEING|I_WILL_FREE)) {
> > -				spin_unlock(&old->i_lock);
> > -				continue;
> > -			}
> > -			break;
> > -		}
> > -		if (likely(!old)) {
> > -			spin_lock(&inode->i_lock);
> > -			inode->i_state |= I_NEW;
> > -			hlist_add_head(&inode->i_hash, head);
> > -			spin_unlock(&inode->i_lock);
> > -			spin_unlock(&inode_hash_lock);
> > -			return 0;
> > -		}
> > -		__iget(old);
> > -		spin_unlock(&old->i_lock);
> > -		spin_unlock(&inode_hash_lock);
> > -		wait_on_inode(old);
> > -		if (unlikely(!inode_unhashed(old))) {
> > -			iput(old);
> > -			return -EBUSY;
> > -		}
> > -		iput(old);
> > -	}
> > -}
> > -EXPORT_SYMBOL(insert_inode_locked);
> > -
> >   int insert_inode_locked4(struct inode *inode, unsigned long hashval,
> >   		int (*test)(struct inode *, void *), void *data)
> >   {
> > @@ -1366,6 +1276,14 @@ int insert_inode_locked4(struct inode *inode, unsigned long hashval,
> >   }
> >   EXPORT_SYMBOL(insert_inode_locked4);
> >
> > +int insert_inode_locked(struct inode *inode)
> > +{
> > +	unsigned long ino = inode->i_ino;
> > +	return insert_inode_locked4(inode, ino, test_ino, (void *)&ino);
>
> return insert_inode_locked4(inode, inode->i_ino, test_ino, (void *)&ino);
>
> > +}
> > +EXPORT_SYMBOL(insert_inode_locked);
> > +
> > +
> >
> >   int generic_delete_inode(struct inode *inode)
> >   {

--
To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html