lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <51D26C6D.6070601@gmail.com>
Date:	Tue, 02 Jul 2013 02:00:13 -0400
From:	Dong Fang <yp.fangdong@...il.com>
To:	Gu Zheng <guz.fnst@...fujitsu.com>
CC:	viro@...iv.linux.org.uk, linux-fsdevel@...r.kernel.org,
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH] vfs: remove the unnecessrary code of fs/inode.c

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@...il.com>
>
>
> Reviewed-by: Gu Zheng <guz.fnst@...fujitsu.com>
>
> 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-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ