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...@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 Regards, Gu spin_unlock(sbi-orphan_inode_lock); } @@ -561,14 +556,12 @@ static int __add_dirty_inode(struct inode *inode, struct dir_inode_entry *new) { struct f2fs_sb_info *sbi = F2FS_SB(inode-i_sb); struct list_head *head = sbi-dir_inode_list; - struct list_head *this; + struct dir_inode_entry *entry; - list_for_each(this, head) { - struct dir_inode_entry *entry; - entry = list_entry(this, struct dir_inode_entry, list); + list_for_each_entry(entry, head, list) if (unlikely(entry-inode == inode)) return -EEXIST; - } + list_add_tail(new-list, head); stat_inc_dirty_dir(sbi); return 0; @@ -618,7 +611,8 @@ void add_dirty_dir_inode(struct inode *inode) void remove_dirty_dir_inode(struct inode *inode) { struct f2fs_sb_info *sbi = F2FS_SB(inode-i_sb); - struct list_head *this, *head; + struct list_head *head; + struct dir_inode_entry *entry; if (!S_ISDIR(inode-i_mode)) return; @@ -630,9 +624,7 @@ void remove_dirty_dir_inode(struct inode *inode) } head = sbi-dir_inode_list; - list_for_each(this, head) { - struct dir_inode_entry *entry; - entry = list_entry(this, struct dir_inode_entry, list); + list_for_each_entry(entry, head, list) { if (entry-inode == inode) { list_del(entry-list); stat_dec_dirty_dir(sbi); @@ -654,15 +646,14 @@ done: struct inode *check_dirty_dir_inode(struct f2fs_sb_info *sbi, nid_t ino) { - struct list_head *this, *head; + struct list_head *head; struct inode *inode = NULL; + struct dir_inode_entry *entry; spin_lock(sbi-dir_inode_lock); head = sbi-dir_inode_list; - list_for_each(this, head) { - struct dir_inode_entry *entry; - entry = list_entry(this, struct dir_inode_entry, list); + list_for_each_entry(entry, head, list) { if (entry-inode-i_ino == ino) { inode = entry-inode; break; diff --git a/fs/f2fs/node.c b/fs/f2fs/node.c index 0021056..afda3d3 100644 --- a/fs/f2fs/node.c +++ b/fs/f2fs/node.c @@ -1452,7 +1452,6 @@ bool alloc_nid(struct f2fs_sb_info *sbi, nid_t *nid) { struct f2fs_nm_info *nm_i = NM_I(sbi); struct free_nid *i = NULL; - struct list_head *this; retry: if (unlikely(sbi-total_valid_node_count + 1 = nm_i-max_nid)) return false; @@ -1462,11 +1461,9 @@ retry: /* We should not use stale free nids created by build_free_nids */ if (nm_i-fcnt !on_build_free_nids(nm_i)) { f2fs_bug_on(list_empty(nm_i-free_nid_list)); - list_for_each(this, nm_i-free_nid_list) { -
Re: [f2fs-dev] [PATCH 2/2] f2fs: use list_for_each_entry{_safe} for simplyfying code
Hi Yu, On 04/01/2014 09:45 AM, Chao Yu wrote: Hi Gu, -Original Message- From: Gu Zheng [mailto:guz.f...@cn.fujitsu.com] Sent: Monday, March 31, 2014 6:07 PM To: Chao Yu Cc: ???; linux-f2fs-devel@lists.sourceforge.net; linux-fsde...@vger.kernel.org; linux-ker...@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...@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] -- ___ Linux-f2fs-devel mailing list Linux-f2fs-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
[f2fs-dev] [PATCH 2/2] f2fs: use list_for_each_entry{_safe} for simplyfying code
This patch use list_for_each_entry{_safe} instead of list_for_each{_safe} for simplfying code. Signed-off-by: Chao Yu chao2...@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); spin_unlock(sbi-orphan_inode_lock); } @@ -561,14 +556,12 @@ static int __add_dirty_inode(struct inode *inode, struct dir_inode_entry *new) { struct f2fs_sb_info *sbi = F2FS_SB(inode-i_sb); struct list_head *head = sbi-dir_inode_list; - struct list_head *this; + struct dir_inode_entry *entry; - list_for_each(this, head) { - struct dir_inode_entry *entry; - entry = list_entry(this, struct dir_inode_entry, list); + list_for_each_entry(entry, head, list) if (unlikely(entry-inode == inode)) return -EEXIST; - } + list_add_tail(new-list, head); stat_inc_dirty_dir(sbi); return 0; @@ -618,7 +611,8 @@ void add_dirty_dir_inode(struct inode *inode) void remove_dirty_dir_inode(struct inode *inode) { struct f2fs_sb_info *sbi = F2FS_SB(inode-i_sb); - struct list_head *this, *head; + struct list_head *head; + struct dir_inode_entry *entry; if (!S_ISDIR(inode-i_mode)) return; @@ -630,9 +624,7 @@ void remove_dirty_dir_inode(struct inode *inode) } head = sbi-dir_inode_list; - list_for_each(this, head) { - struct dir_inode_entry *entry; - entry = list_entry(this, struct dir_inode_entry, list); + list_for_each_entry(entry, head, list) { if (entry-inode == inode) { list_del(entry-list); stat_dec_dirty_dir(sbi); @@ -654,15 +646,14 @@ done: struct inode *check_dirty_dir_inode(struct f2fs_sb_info *sbi, nid_t ino) { - struct list_head *this, *head; + struct list_head *head; struct inode *inode = NULL; + struct dir_inode_entry *entry; spin_lock(sbi-dir_inode_lock); head = sbi-dir_inode_list; - list_for_each(this, head) { - struct dir_inode_entry *entry; - entry = list_entry(this, struct dir_inode_entry, list); + list_for_each_entry(entry, head, list) { if (entry-inode-i_ino == ino) { inode = entry-inode; break; diff --git a/fs/f2fs/node.c b/fs/f2fs/node.c index 0021056..afda3d3 100644 --- a/fs/f2fs/node.c +++ b/fs/f2fs/node.c @@ -1452,7 +1452,6 @@ bool alloc_nid(struct f2fs_sb_info *sbi, nid_t *nid) { struct f2fs_nm_info *nm_i = NM_I(sbi); struct free_nid *i = NULL; - struct list_head *this; retry: if (unlikely(sbi-total_valid_node_count + 1 = nm_i-max_nid)) return false; @@ -1462,11 +1461,9 @@ retry: /* We should not use stale free nids created by build_free_nids */ if (nm_i-fcnt !on_build_free_nids(nm_i)) { f2fs_bug_on(list_empty(nm_i-free_nid_list)); - list_for_each(this, nm_i-free_nid_list) { - i = list_entry(this, struct free_nid, list); + list_for_each_entry(i, nm_i-free_nid_list, list) if (i-state == NID_NEW) break; - }