Skip to contentSkip to conversation

f2fs: fix removing cache entry within proper lock

PATCH NAK Namjae Jeon 2 participants 2 messages Dec 30, 2012 – Jan 2, 2013

Subsystems (2)
lore
Share & tools
lore
#0 lore raw

PATCH 3/4 f2fs: fix removing cache entry within proper lock

From: Namjae Jeon <[email protected]>

Cache entry for free NID, is not getting removed under proper lock in case of error in add_free_nid. So, free the cache entry first before releasing the spinlock.

---
 fs/f2fs/node.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
fs/f2fs/node.c +1 -1
index 5066bfd..09139fb 100644
@@ -1227,8 +1227,8 @@ retry:
 
 	spin_lock(&nm_i->free_nid_list_lock);
 	if (__lookup_free_nid_list(nid, &nm_i->free_nid_list)) {
-		spin_unlock(&nm_i->free_nid_list_lock);
 		kmem_cache_free(free_nid_slab, i);
+		spin_unlock(&nm_i->free_nid_list_lock);
 		return 0;
 	}
 	list_add_tail(&i->list, &nm_i->free_nid_list);
Signature · 1 line
1.7.9.5
#1 lore raw

NAK, we don't need to do this.
There is no relationship between kmem_cache_* and free_nid_list_lock. Moreover, IMO, it would be better unlock free_nid_list_lock as quickly as possible in order to reduce lock contention.

2012-12-30 (일), 14:52 +0900, Namjae Jeon:

Quoted 26 lines

From: Namjae Jeon <[email protected]>

Cache entry for free NID, is not getting removed under proper lock in case of error in add_free_nid. So, free the cache entry first before releasing the spinlock.

---
 fs/f2fs/node.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
fs/f2fs/node.c +1 -1
index 5066bfd..09139fb 100644
@@ -1227,8 +1227,8 @@ retry:
 
 	spin_lock(&nm_i->free_nid_list_lock);
 	if (__lookup_free_nid_list(nid, &nm_i->free_nid_list)) {
-		spin_unlock(&nm_i->free_nid_list_lock);
 		kmem_cache_free(free_nid_slab, i);
+		spin_unlock(&nm_i->free_nid_list_lock);
 		return 0;
 	}
 	list_add_tail(&i->list, &nm_i->free_nid_list);
Signature · 2 lines
Jaegeuk Kim
Samsung