Re: [PATCH 1/3] Btrfs: fix possible memory leak in find_parent_nodes()

2013-08-08 Thread Wang Shilong 
On 08/08/2013 07:02 PM, Jan Schmidt wrote:
  
 On Thu, August 08, 2013 at 07:04 (+0200), Wang Shilong wrote:
 Signed-off-by: Wang Shilong wangsl.f...@cn.fujitsu.com
 Reviewed-by: Miao Xie mi...@cn.fujitsu.com
 ---
  fs/btrfs/backref.c | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

 diff --git a/fs/btrfs/backref.c b/fs/btrfs/backref.c
 index cb73a12..54e7610 100644
 --- a/fs/btrfs/backref.c
 +++ b/fs/btrfs/backref.c
 @@ -911,7 +911,6 @@ again:
  
  while (!list_empty(prefs)) {
  ref = list_first_entry(prefs, struct __prelim_ref, list);
 -list_del(ref-list);
  WARN_ON(ref-count  0);
  if (ref-count  ref-root_id  ref-parent == 0) {
  /* no parent == root of tree */
 @@ -954,6 +953,7 @@ again:
  eie-next = ref-inode_list;
  }
  }
 +list_del(ref-list);
  kfree(ref);
  }
  

 
 I'm not convinced, you're not calling kfree() more often. Can you please add
 some patch description?

Yeah. i will add more description in V2.

Thanks
Wang
 
 -Jan
 

--
To unsubscribe from this list: send the line unsubscribe linux-btrfs in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 1/3] Btrfs: fix possible memory leak in find_parent_nodes()

2013-08-07 Thread Wang Shilong 
Signed-off-by: Wang Shilong wangsl.f...@cn.fujitsu.com
Reviewed-by: Miao Xie mi...@cn.fujitsu.com
---
 fs/btrfs/backref.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/fs/btrfs/backref.c b/fs/btrfs/backref.c
index cb73a12..54e7610 100644
--- a/fs/btrfs/backref.c
+++ b/fs/btrfs/backref.c
@@ -911,7 +911,6 @@ again:
 
while (!list_empty(prefs)) {
ref = list_first_entry(prefs, struct __prelim_ref, list);
-   list_del(ref-list);
WARN_ON(ref-count  0);
if (ref-count  ref-root_id  ref-parent == 0) {
/* no parent == root of tree */
@@ -954,6 +953,7 @@ again:
eie-next = ref-inode_list;
}
}
+   list_del(ref-list);
kfree(ref);
}
 
-- 
1.8.0.1

--
To unsubscribe from this list: send the line unsubscribe linux-btrfs in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 1/3] Btrfs: fix possible memory leak in find_parent_nodes()

2013-08-07 Thread Liu Bo 
On Thu, Aug 08, 2013 at 01:04:17PM +0800, Wang Shilong wrote:
 Signed-off-by: Wang Shilong wangsl.f...@cn.fujitsu.com
 Reviewed-by: Miao Xie mi...@cn.fujitsu.com

Sorry, I don't think I understand why it's a memory leak, some explanation
is needed here.

-liubo

 ---
  fs/btrfs/backref.c | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)
 
 diff --git a/fs/btrfs/backref.c b/fs/btrfs/backref.c
 index cb73a12..54e7610 100644
 --- a/fs/btrfs/backref.c
 +++ b/fs/btrfs/backref.c
 @@ -911,7 +911,6 @@ again:
  
   while (!list_empty(prefs)) {
   ref = list_first_entry(prefs, struct __prelim_ref, list);
 - list_del(ref-list);
   WARN_ON(ref-count  0);
   if (ref-count  ref-root_id  ref-parent == 0) {
   /* no parent == root of tree */
 @@ -954,6 +953,7 @@ again:
   eie-next = ref-inode_list;
   }
   }
 + list_del(ref-list);
   kfree(ref);
   }
  
 -- 
 1.8.0.1
 
 --
 To unsubscribe from this list: send the line unsubscribe linux-btrfs in
 the body of a message to majord...@vger.kernel.org
 More majordomo info at  http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line unsubscribe linux-btrfs in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 1/3] Btrfs: fix possible memory leak in find_parent_nodes()

2013-08-07 Thread Liu Bo 
On Thu, Aug 08, 2013 at 01:04:17PM +0800, Wang Shilong wrote:
 Signed-off-by: Wang Shilong wangsl.f...@cn.fujitsu.com
 Reviewed-by: Miao Xie mi...@cn.fujitsu.com

I think I know the whys :p, but still a log is preferred.

-liubo

 ---
  fs/btrfs/backref.c | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)
 
 diff --git a/fs/btrfs/backref.c b/fs/btrfs/backref.c
 index cb73a12..54e7610 100644
 --- a/fs/btrfs/backref.c
 +++ b/fs/btrfs/backref.c
 @@ -911,7 +911,6 @@ again:
  
   while (!list_empty(prefs)) {
   ref = list_first_entry(prefs, struct __prelim_ref, list);
 - list_del(ref-list);
   WARN_ON(ref-count  0);
   if (ref-count  ref-root_id  ref-parent == 0) {
   /* no parent == root of tree */
 @@ -954,6 +953,7 @@ again:
   eie-next = ref-inode_list;
   }
   }
 + list_del(ref-list);
   kfree(ref);
   }
  
 -- 
 1.8.0.1
 
 --
 To unsubscribe from this list: send the line unsubscribe linux-btrfs in
 the body of a message to majord...@vger.kernel.org
 More majordomo info at  http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line unsubscribe linux-btrfs in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 1/3] Btrfs: fix possible memory leak in find_parent_nodes()

2013-08-07 Thread Miao Xie 
On thu, 8 Aug 2013 13:22:12 +0800, Liu Bo wrote:
 On Thu, Aug 08, 2013 at 01:04:17PM +0800, Wang Shilong wrote:
 Signed-off-by: Wang Shilong wangsl.f...@cn.fujitsu.com
 Reviewed-by: Miao Xie mi...@cn.fujitsu.com
 
 I think I know the whys :p, but still a log is preferred.

Right, we need a explanation, we will update this patch soon.

Thanks
Miao

 
 -liubo
 
 ---
  fs/btrfs/backref.c | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

 diff --git a/fs/btrfs/backref.c b/fs/btrfs/backref.c
 index cb73a12..54e7610 100644
 --- a/fs/btrfs/backref.c
 +++ b/fs/btrfs/backref.c
 @@ -911,7 +911,6 @@ again:
  
  while (!list_empty(prefs)) {
  ref = list_first_entry(prefs, struct __prelim_ref, list);
 -list_del(ref-list);
  WARN_ON(ref-count  0);
  if (ref-count  ref-root_id  ref-parent == 0) {
  /* no parent == root of tree */
 @@ -954,6 +953,7 @@ again:
  eie-next = ref-inode_list;
  }
  }
 +list_del(ref-list);
  kfree(ref);
  }
  
 -- 
 1.8.0.1

 --
 To unsubscribe from this list: send the line unsubscribe linux-btrfs in
 the body of a message to majord...@vger.kernel.org
 More majordomo info at  http://vger.kernel.org/majordomo-info.html
 --
 To unsubscribe from this list: send the line unsubscribe linux-btrfs in
 the body of a message to majord...@vger.kernel.org
 More majordomo info at  http://vger.kernel.org/majordomo-info.html
 

--
To unsubscribe from this list: send the line unsubscribe linux-btrfs in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html