Skip to contentSkip to conversation

f2fs: use list_for_each rather than list_for_each_safe, in remove_orphan_inode()

PATCH In review Gu Zheng 2 participants 4 messages Jul 19, 2013 – Jul 23, 2013 F2FS file system

lore
Share & tools
lore

Gu Zheng

author
#0 lore raw

PATCH 1/1 f2fs: use list_for_each rather than list_for_each_safe, in remove_orphan_inode()

As we remove the target single node, so list_for_each is enought, in order to clean up, we use list_for_each_entry instead.

---
 fs/f2fs/checkpoint.c |    5 ++---
 1 files changed, 2 insertions(+), 3 deletions(-)
fs/f2fs/checkpoint.c +2 -3
index 290db04..87f7bc2 100644
@@ -237,13 +237,12 @@ out:
 
 void remove_orphan_inode(struct f2fs_sb_info *sbi, nid_t ino)
 {
-	struct list_head *this, *next, *head;
+	struct list_head *head;
 	struct orphan_inode_entry *orphan;
 
 	mutex_lock(&sbi->orphan_inode_mutex);
 	head = &sbi->orphan_inode_list;
-	list_for_each_safe(this, next, head) {
-		orphan = list_entry(this, struct orphan_inode_entry, list);
+	list_for_each_entry(orphan, head, list) {
 		if (orphan->ino == ino) {
 			list_del(&orphan->list);
 			kmem_cache_free(orphan_entry_slab, orphan);
Signature · 1 line
1.7.7
#1 lore raw
Quoted 18 lines · Gu Zheng

As we remove the target single node, so list_for_each is enought, in order to clean up, we use list_for_each_entry instead.

---
 fs/f2fs/checkpoint.c |    5 ++---
 1 files changed, 2 insertions(+), 3 deletions(-)
fs/f2fs/checkpoint.c +1 -1
index 290db04..87f7bc2 100644
@@ -237,13 +237,12 @@ out:
 
 void remove_orphan_inode(struct f2fs_sb_info *sbi, nid_t ino)
 {
-	struct list_head *this, *next, *head;
+	struct list_head *head;

fs/f2fs/checkpoint.c out: · line 248

Quoted 10 lines
fs/f2fs/checkpoint.c +1 -2
 	struct orphan_inode_entry *orphan;
 
 	mutex_lock(&sbi->orphan_inode_mutex);
 	head = &sbi->orphan_inode_list;
-	list_for_each_safe(this, next, head) {
-		orphan = list_entry(this, struct orphan_inode_entry, list);
+	list_for_each_entry(orphan, head, list) {
 		if (orphan->ino == ino) {
 			list_del(&orphan->list);
 			kmem_cache_free(orphan_entry_slab, orphan);

you have meant list_for_each_entry_safe, haven't you?

Signature · 8 lines
Nikola

------------------------------------------------------------------------------
See everything from the browser to the database with AppDynamics
Get end-to-end visibility with application monitoring from AppDynamics
Isolate bottlenecks and diagnose root cause in seconds.
Start your free trial of AppDynamics Pro today!
http://pubads.g.doubleclick.net/gampad/clk?id=48808831&iu=/4140/ostg.clktrk
author
#2 lore raw
Quoted 22 lines · Nikola Pajkovsky
Quoted 20 lines · Gu Zheng

As we remove the target single node, so list_for_each is enought, in order to clean up, we use list_for_each_entry instead.

---
 fs/f2fs/checkpoint.c |    5 ++---
 1 files changed, 2 insertions(+), 3 deletions(-)
fs/f2fs/checkpoint.c +1 -1
index 290db04..87f7bc2 100644
@@ -237,13 +237,12 @@ out:
 
 void remove_orphan_inode(struct f2fs_sb_info *sbi, nid_t ino)
 {
-	struct list_head *this, *next, *head;
+	struct list_head *head;
 	struct orphan_inode_entry *orphan;
 
Quoted 10 lines

fs/f2fs/checkpoint.c out: · line 248

Quoted 8 lines
fs/f2fs/checkpoint.c +1 -2
 	mutex_lock(&sbi->orphan_inode_mutex);
 	head = &sbi->orphan_inode_list;
-	list_for_each_safe(this, next, head) {
-		orphan = list_entry(this, struct orphan_inode_entry, list);
+	list_for_each_entry(orphan, head, list) {
 		if (orphan->ino == ino) {
 			list_del(&orphan->list);
 			kmem_cache_free(orphan_entry_slab, orphan);

you have meant list_for_each_entry_safe, haven't you?

No that, here list_for_each_entry is suitable, because we delete only one entry.

Thanks,
Gu

Quoted 1 line
#3 lore raw
Quoted 35 lines · Gu Zheng
Quoted 22 lines · Nikola Pajkovsky
Quoted 20 lines · Gu Zheng

As we remove the target single node, so list_for_each is enought, in order to clean up, we use list_for_each_entry instead.

---
 fs/f2fs/checkpoint.c |    5 ++---
 1 files changed, 2 insertions(+), 3 deletions(-)
fs/f2fs/checkpoint.c +1 -1
index 290db04..87f7bc2 100644
@@ -237,13 +237,12 @@ out:
 
 void remove_orphan_inode(struct f2fs_sb_info *sbi, nid_t ino)
 {
-	struct list_head *this, *next, *head;
+	struct list_head *head;
 	struct orphan_inode_entry *orphan;
 
Quoted 10 lines

fs/f2fs/checkpoint.c out: · line 248

Quoted 8 lines
fs/f2fs/checkpoint.c +1 -2
 	mutex_lock(&sbi->orphan_inode_mutex);
 	head = &sbi->orphan_inode_list;
-	list_for_each_safe(this, next, head) {
-		orphan = list_entry(this, struct orphan_inode_entry, list);
+	list_for_each_entry(orphan, head, list) {
 		if (orphan->ino == ino) {
 			list_del(&orphan->list);
 			kmem_cache_free(orphan_entry_slab, orphan);

you have meant list_for_each_entry_safe, haven't you?

Quoted 1 line

No that, here list_for_each_entry is suitable, because we delete only one entry.

yeah, you're correct.

Signature · 8 lines
Nikola

------------------------------------------------------------------------------
See everything from the browser to the database with AppDynamics
Get end-to-end visibility with application monitoring from AppDynamics
Isolate bottlenecks and diagnose root cause in seconds.
Start your free trial of AppDynamics Pro today!
http://pubads.g.doubleclick.net/gampad/clk?id=48808831&iu=/4140/ostg.clktrk