* [PATCH] Btrfs: fix panic in balance due to EIO
@ 2016-07-12 0:37 Liu Bo
2016-07-12 15:05 ` Chris Mason
2016-07-12 17:29 ` [PATCH v2] " Liu Bo
0 siblings, 2 replies; 5+ messages in thread
From: Liu Bo @ 2016-07-12 0:37 UTC (permalink / raw)
To: linux-btrfs; +Cc: David Sterba
During build_backref_tree(), if we fail to read a btree node,
we can eventually run into BUG_ON(cache->nr_nodes) that we put
in backref_cache_cleanup(), meaning we have at least one
memory leak.
This frees the backref_node that we allocate at the very beginning of build_backref_tree().
Signed-off-by: Liu Bo <bo.li.liu@oracle.com>
---
fs/btrfs/relocation.c | 7 +++++++
1 file changed, 7 insertions(+)
diff --git a/fs/btrfs/relocation.c b/fs/btrfs/relocation.c
index 0477dca..f00267a 100644
--- a/fs/btrfs/relocation.c
+++ b/fs/btrfs/relocation.c
@@ -1135,6 +1135,8 @@ out:
btrfs_free_path(path1);
btrfs_free_path(path2);
if (err) {
+ int orig_free = 0;
+
while (!list_empty(&useless)) {
lower = list_entry(useless.next,
struct backref_node, list);
@@ -1171,8 +1173,13 @@ out:
lower = list_entry(useless.next,
struct backref_node, list);
list_del_init(&lower->list);
+ if (lower == node)
+ orig_free = 1;
free_backref_node(cache, lower);
}
+
+ if (!orig_free)
+ free_backref_node(cache, node);
return ERR_PTR(err);
}
ASSERT(!node || !node->detached);
--
2.5.5
^ permalink raw reply related [flat|nested] 5+ messages in thread* Re: [PATCH] Btrfs: fix panic in balance due to EIO
2016-07-12 0:37 [PATCH] Btrfs: fix panic in balance due to EIO Liu Bo
@ 2016-07-12 15:05 ` Chris Mason
2016-07-12 17:08 ` Liu Bo
2016-07-12 17:29 ` [PATCH v2] " Liu Bo
1 sibling, 1 reply; 5+ messages in thread
From: Chris Mason @ 2016-07-12 15:05 UTC (permalink / raw)
To: Liu Bo, linux-btrfs; +Cc: David Sterba
On 07/11/2016 08:37 PM, Liu Bo wrote:
> During build_backref_tree(), if we fail to read a btree node,
> we can eventually run into BUG_ON(cache->nr_nodes) that we put
> in backref_cache_cleanup(), meaning we have at least one
> memory leak.
>
> This frees the backref_node that we allocate at the very beginning of build_backref_tree().
>
> Signed-off-by: Liu Bo <bo.li.liu@oracle.com>
> ---
> fs/btrfs/relocation.c | 7 +++++++
> 1 file changed, 7 insertions(+)
>
> diff --git a/fs/btrfs/relocation.c b/fs/btrfs/relocation.c
> index 0477dca..f00267a 100644
> --- a/fs/btrfs/relocation.c
> +++ b/fs/btrfs/relocation.c
> @@ -1135,6 +1135,8 @@ out:
> btrfs_free_path(path1);
> btrfs_free_path(path2);
> if (err) {
> + int orig_free = 0;
> +
> while (!list_empty(&useless)) {
> lower = list_entry(useless.next,
> struct backref_node, list);
> @@ -1171,8 +1173,13 @@ out:
> lower = list_entry(useless.next,
> struct backref_node, list);
> list_del_init(&lower->list);
> + if (lower == node)
> + orig_free = 1;
> free_backref_node(cache, lower);
> }
> +
> + if (!orig_free)
> + free_backref_node(cache, node);
> return ERR_PTR(err);
> }
> ASSERT(!node || !node->detached);
Instead of doing the orig_free set and test
...
if (lower == node)
node = NULL
free_backref_node(cache, lower)
}
free_backref_node(cache, node);
return ERR_PTR(err);
Your patch isn't wrong, but having node NULL after it was free'd makes
us less likely to make mistakes as the code changes.
-chris
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH] Btrfs: fix panic in balance due to EIO
2016-07-12 15:05 ` Chris Mason
@ 2016-07-12 17:08 ` Liu Bo
0 siblings, 0 replies; 5+ messages in thread
From: Liu Bo @ 2016-07-12 17:08 UTC (permalink / raw)
To: Chris Mason; +Cc: linux-btrfs, David Sterba
On Tue, Jul 12, 2016 at 11:05:48AM -0400, Chris Mason wrote:
>
>
> On 07/11/2016 08:37 PM, Liu Bo wrote:
> > During build_backref_tree(), if we fail to read a btree node,
> > we can eventually run into BUG_ON(cache->nr_nodes) that we put
> > in backref_cache_cleanup(), meaning we have at least one
> > memory leak.
> >
> > This frees the backref_node that we allocate at the very beginning of build_backref_tree().
> >
> > Signed-off-by: Liu Bo <bo.li.liu@oracle.com>
> > ---
> > fs/btrfs/relocation.c | 7 +++++++
> > 1 file changed, 7 insertions(+)
> >
> > diff --git a/fs/btrfs/relocation.c b/fs/btrfs/relocation.c
> > index 0477dca..f00267a 100644
> > --- a/fs/btrfs/relocation.c
> > +++ b/fs/btrfs/relocation.c
> > @@ -1135,6 +1135,8 @@ out:
> > btrfs_free_path(path1);
> > btrfs_free_path(path2);
> > if (err) {
> > + int orig_free = 0;
> > +
> > while (!list_empty(&useless)) {
> > lower = list_entry(useless.next,
> > struct backref_node, list);
> > @@ -1171,8 +1173,13 @@ out:
> > lower = list_entry(useless.next,
> > struct backref_node, list);
> > list_del_init(&lower->list);
> > + if (lower == node)
> > + orig_free = 1;
> > free_backref_node(cache, lower);
> > }
> > +
> > + if (!orig_free)
> > + free_backref_node(cache, node);
> > return ERR_PTR(err);
> > }
> > ASSERT(!node || !node->detached);
>
> Instead of doing the orig_free set and test
>
> ...
> if (lower == node)
> node = NULL
> free_backref_node(cache, lower)
> }
> free_backref_node(cache, node);
> return ERR_PTR(err);
>
> Your patch isn't wrong, but having node NULL after it was free'd makes us
> less likely to make mistakes as the code changes.
Golden rule indeed.
Thanks,
-liubo
^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH v2] Btrfs: fix panic in balance due to EIO
2016-07-12 0:37 [PATCH] Btrfs: fix panic in balance due to EIO Liu Bo
2016-07-12 15:05 ` Chris Mason
@ 2016-07-12 17:29 ` Liu Bo
2016-07-12 18:01 ` Chris Mason
1 sibling, 1 reply; 5+ messages in thread
From: Liu Bo @ 2016-07-12 17:29 UTC (permalink / raw)
To: linux-btrfs; +Cc: David Sterba, Chris Mason
During build_backref_tree(), if we fail to read a btree node,
we can eventually run into BUG_ON(cache->nr_nodes) that we put
in backref_cache_cleanup(), meaning we have at least one
memory leak.
This frees the backref_node that we's allocated at the very
beginning of build_backref_tree().
Signed-off-by: Liu Bo <bo.li.liu@oracle.com>
---
v2: Set node to NULL if we know it'll be freed.
fs/btrfs/relocation.c | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/fs/btrfs/relocation.c b/fs/btrfs/relocation.c
index 9732919..535314a 100644
--- a/fs/btrfs/relocation.c
+++ b/fs/btrfs/relocation.c
@@ -1171,8 +1171,12 @@ out:
lower = list_entry(useless.next,
struct backref_node, list);
list_del_init(&lower->list);
+ if (lower == node)
+ node = NULL;
free_backref_node(cache, lower);
}
+
+ free_backref_node(cache, node);
return ERR_PTR(err);
}
ASSERT(!node || !node->detached);
--
2.5.5
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH v2] Btrfs: fix panic in balance due to EIO
2016-07-12 17:29 ` [PATCH v2] " Liu Bo
@ 2016-07-12 18:01 ` Chris Mason
0 siblings, 0 replies; 5+ messages in thread
From: Chris Mason @ 2016-07-12 18:01 UTC (permalink / raw)
To: Liu Bo, linux-btrfs; +Cc: David Sterba
On 07/12/2016 01:29 PM, Liu Bo wrote:
> During build_backref_tree(), if we fail to read a btree node,
> we can eventually run into BUG_ON(cache->nr_nodes) that we put
> in backref_cache_cleanup(), meaning we have at least one
> memory leak.
>
> This frees the backref_node that we's allocated at the very
> beginning of build_backref_tree().
>
> Signed-off-by: Liu Bo <bo.li.liu@oracle.com>
Signed-off-by: Chris Mason <clm@fb.com>
-chris
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2016-07-12 18:01 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2016-07-12 0:37 [PATCH] Btrfs: fix panic in balance due to EIO Liu Bo
2016-07-12 15:05 ` Chris Mason
2016-07-12 17:08 ` Liu Bo
2016-07-12 17:29 ` [PATCH v2] " Liu Bo
2016-07-12 18:01 ` Chris Mason
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).