Re: [PATCH 2/2] f2fs: use list_for_each_entry{_safe} for simplyfying code

[Gu Zheng <guz.fnst@cn.fujitsu.com>]

Hi Yu,
On 04/01/2014 09:45 AM, Chao Yu wrote:

> Hi Gu,
> 
>> -----Original Message-----
>> From: Gu Zheng [mailto:guz.fnst@cn.fujitsu.com]
>> Sent: Monday, March 31, 2014 6:07 PM
>> To: Chao Yu
>> Cc: ???; linux-f2fs-devel@lists.sourceforge.net; linux-fsdevel@vger.kernel.org;
>> linux-kernel@vger.kernel.org
>> Subject: Re: [f2fs-dev] [PATCH 2/2] f2fs: use list_for_each_entry{_safe} for simplyfying code
>>
>> Hi Yu,
>> On 03/29/2014 11:33 AM, Chao Yu wrote:
>>
>>> This patch use list_for_each_entry{_safe} instead of list_for_each{_safe} for
>>> simplfying code.
>>>
>>> Signed-off-by: Chao Yu <chao2.yu@samsung.com>
>>> ---
>>>  fs/f2fs/checkpoint.c |   37 ++++++++++++++-----------------------
>>>  fs/f2fs/node.c       |   16 ++++++----------
>>>  fs/f2fs/recovery.c   |    6 ++----
>>>  fs/f2fs/segment.c    |    6 ++----
>>>  4 files changed, 24 insertions(+), 41 deletions(-)
>>>
>>> diff --git a/fs/f2fs/checkpoint.c b/fs/f2fs/checkpoint.c
>>> index d877f46..4aa521a 100644
>>> --- a/fs/f2fs/checkpoint.c
>>> +++ b/fs/f2fs/checkpoint.c
>>> @@ -308,16 +308,15 @@ void release_orphan_inode(struct f2fs_sb_info *sbi)
>>>
>>>  void add_orphan_inode(struct f2fs_sb_info *sbi, nid_t ino)
>>>  {
>>> -	struct list_head *head, *this;
>>> -	struct orphan_inode_entry *new = NULL, *orphan = NULL;
>>> +	struct list_head *head;
>>> +	struct orphan_inode_entry *new, *orphan;
>>>
>>>  	new = f2fs_kmem_cache_alloc(orphan_entry_slab, GFP_ATOMIC);
>>>  	new->ino = ino;
>>>
>>>  	spin_lock(&sbi->orphan_inode_lock);
>>>  	head = &sbi->orphan_inode_list;
>>> -	list_for_each(this, head) {
>>> -		orphan = list_entry(this, struct orphan_inode_entry, list);
>>> +	list_for_each_entry(orphan, head, list) {
>>>  		if (orphan->ino == ino) {
>>>  			spin_unlock(&sbi->orphan_inode_lock);
>>>  			kmem_cache_free(orphan_entry_slab, new);
>>> @@ -326,14 +325,10 @@ void add_orphan_inode(struct f2fs_sb_info *sbi, nid_t ino)
>>>
>>>  		if (orphan->ino > ino)
>>>  			break;
>>> -		orphan = NULL;
>>>  	}
>>>
>>> -	/* add new_oentry into list which is sorted by inode number */
>>> -	if (orphan)
>>> -		list_add(&new->list, this->prev);
>>> -	else
>>> -		list_add_tail(&new->list, head);
>>> +	/* add new orphan entry into list which is sorted by inode number */
>>> +	list_add_tail(&new->list, &orphan->list);
>>
>> It seems that the logic can not be changed here, otherwise the orphan list will not be in order
>> if
>> the new ino is bigger than all the in-list ones.
>> E.g.
>> ino:5
>> 1-->2-->3-->4
>> ==>
>> 1-->2-->3-->5-->4
> 
> As I checked, if new ino is bigger than all, it will break from list_for_each_entry because
> &orphan->list is pointing to head. So list_add_tail can add the new entry before head to make
> this list in order.

Oh...Yes, you are right.
Thanks for correcting me.

Regards,
Gu

> 
> Thanks.
> 
>>
>> Regards,
>> Gu
>>
> 
> [snip]
> 
> 



------------------------------------------------------------------------------