* [PATCH] Btrfs: don't leak reloc root nodes on error
@ 2016-09-02 19:25 Josef Bacik
2016-09-03 1:08 ` Liu Bo
0 siblings, 1 reply; 6+ messages in thread
From: Josef Bacik @ 2016-09-02 19:25 UTC (permalink / raw)
To: linux-btrfs, kernel-team
We don't track the reloc roots in any sort of normal way, so the only way the
root/commit_root nodes get free'd is if the relocation finishes successfully and
the reloc root is deleted. Fix this by free'ing them in free_reloc_roots.
Thanks,
Signed-off-by: Josef Bacik <jbacik@fb.com>
---
fs/btrfs/relocation.c | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/fs/btrfs/relocation.c b/fs/btrfs/relocation.c
index 7fc6ea7..62dfc2c 100644
--- a/fs/btrfs/relocation.c
+++ b/fs/btrfs/relocation.c
@@ -2349,6 +2349,10 @@ void free_reloc_roots(struct list_head *list)
while (!list_empty(list)) {
reloc_root = list_entry(list->next, struct btrfs_root,
root_list);
+ free_extent_buffer(reloc_root->node);
+ free_extent_buffer(reloc_root->commit_root);
+ reloc_root->node = NULL;
+ reloc_root->commit_root = NULL;
__del_reloc_root(reloc_root);
}
}
--
2.7.4
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH] Btrfs: don't leak reloc root nodes on error
2016-09-02 19:25 [PATCH] Btrfs: don't leak reloc root nodes on error Josef Bacik
@ 2016-09-03 1:08 ` Liu Bo
2016-09-05 15:20 ` David Sterba
0 siblings, 1 reply; 6+ messages in thread
From: Liu Bo @ 2016-09-03 1:08 UTC (permalink / raw)
To: Josef Bacik; +Cc: linux-btrfs
On Fri, Sep 02, 2016 at 03:25:43PM -0400, Josef Bacik wrote:
> We don't track the reloc roots in any sort of normal way, so the only way the
> root/commit_root nodes get free'd is if the relocation finishes successfully and
> the reloc root is deleted. Fix this by free'ing them in free_reloc_roots.
> Thanks,
Looks good.
>
> Signed-off-by: Josef Bacik <jbacik@fb.com>
> ---
> fs/btrfs/relocation.c | 4 ++++
> 1 file changed, 4 insertions(+)
>
> diff --git a/fs/btrfs/relocation.c b/fs/btrfs/relocation.c
> index 7fc6ea7..62dfc2c 100644
> --- a/fs/btrfs/relocation.c
> +++ b/fs/btrfs/relocation.c
> @@ -2349,6 +2349,10 @@ void free_reloc_roots(struct list_head *list)
> while (!list_empty(list)) {
> reloc_root = list_entry(list->next, struct btrfs_root,
> root_list);
> + free_extent_buffer(reloc_root->node);
> + free_extent_buffer(reloc_root->commit_root);
> + reloc_root->node = NULL;
> + reloc_root->commit_root = NULL;
What about reloc_root itself?
Thanks,
-liubo
> __del_reloc_root(reloc_root);
> }
> }
> --
> 2.7.4
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] Btrfs: don't leak reloc root nodes on error
2016-09-03 1:08 ` Liu Bo
@ 2016-09-05 15:20 ` David Sterba
2016-09-06 19:22 ` Liu Bo
0 siblings, 1 reply; 6+ messages in thread
From: David Sterba @ 2016-09-05 15:20 UTC (permalink / raw)
To: Liu Bo; +Cc: Josef Bacik, linux-btrfs
On Fri, Sep 02, 2016 at 06:08:35PM -0700, Liu Bo wrote:
> On Fri, Sep 02, 2016 at 03:25:43PM -0400, Josef Bacik wrote:
> > We don't track the reloc roots in any sort of normal way, so the only way the
> > root/commit_root nodes get free'd is if the relocation finishes successfully and
> > the reloc root is deleted. Fix this by free'ing them in free_reloc_roots.
> > Thanks,
>
> Looks good.
>
> >
> > Signed-off-by: Josef Bacik <jbacik@fb.com>
> > ---
> > fs/btrfs/relocation.c | 4 ++++
> > 1 file changed, 4 insertions(+)
> >
> > diff --git a/fs/btrfs/relocation.c b/fs/btrfs/relocation.c
> > index 7fc6ea7..62dfc2c 100644
> > --- a/fs/btrfs/relocation.c
> > +++ b/fs/btrfs/relocation.c
> > @@ -2349,6 +2349,10 @@ void free_reloc_roots(struct list_head *list)
> > while (!list_empty(list)) {
> > reloc_root = list_entry(list->next, struct btrfs_root,
> > root_list);
> > + free_extent_buffer(reloc_root->node);
> > + free_extent_buffer(reloc_root->commit_root);
> > + reloc_root->node = NULL;
> > + reloc_root->commit_root = NULL;
>
> What about reloc_root itself?
> > __del_reloc_root(reloc_root);
It's deleted at the end of __del_reloc_root
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] Btrfs: don't leak reloc root nodes on error
2016-09-05 15:20 ` David Sterba
@ 2016-09-06 19:22 ` Liu Bo
2016-09-21 14:00 ` David Sterba
0 siblings, 1 reply; 6+ messages in thread
From: Liu Bo @ 2016-09-06 19:22 UTC (permalink / raw)
To: dsterba; +Cc: Josef Bacik, linux-btrfs
On Mon, Sep 05, 2016 at 05:20:53PM +0200, David Sterba wrote:
> On Fri, Sep 02, 2016 at 06:08:35PM -0700, Liu Bo wrote:
> > On Fri, Sep 02, 2016 at 03:25:43PM -0400, Josef Bacik wrote:
> > > We don't track the reloc roots in any sort of normal way, so the only way the
> > > root/commit_root nodes get free'd is if the relocation finishes successfully and
> > > the reloc root is deleted. Fix this by free'ing them in free_reloc_roots.
> > > Thanks,
> >
> > Looks good.
> >
> > >
> > > Signed-off-by: Josef Bacik <jbacik@fb.com>
> > > ---
> > > fs/btrfs/relocation.c | 4 ++++
> > > 1 file changed, 4 insertions(+)
> > >
> > > diff --git a/fs/btrfs/relocation.c b/fs/btrfs/relocation.c
> > > index 7fc6ea7..62dfc2c 100644
> > > --- a/fs/btrfs/relocation.c
> > > +++ b/fs/btrfs/relocation.c
> > > @@ -2349,6 +2349,10 @@ void free_reloc_roots(struct list_head *list)
> > > while (!list_empty(list)) {
> > > reloc_root = list_entry(list->next, struct btrfs_root,
> > > root_list);
> > > + free_extent_buffer(reloc_root->node);
> > > + free_extent_buffer(reloc_root->commit_root);
> > > + reloc_root->node = NULL;
> > > + reloc_root->commit_root = NULL;
> >
> > What about reloc_root itself?
>
> > > __del_reloc_root(reloc_root);
>
> It's deleted at the end of __del_reloc_root
__del_reloc_root() is dealing with the associated rb_node, not
reloc_root.
I posted a patch to free reloc_root while free'ing its corresponding fs
root, but it'd be good if we can free it here.
Thanks,
-liubo
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] Btrfs: don't leak reloc root nodes on error
2016-09-06 19:22 ` Liu Bo
@ 2016-09-21 14:00 ` David Sterba
2016-09-21 22:45 ` [PATCH] Btrfs: don't leak reloc root nodes on errorg Liu Bo
0 siblings, 1 reply; 6+ messages in thread
From: David Sterba @ 2016-09-21 14:00 UTC (permalink / raw)
To: Liu Bo; +Cc: dsterba, Josef Bacik, linux-btrfs
On Tue, Sep 06, 2016 at 12:22:01PM -0700, Liu Bo wrote:
> On Mon, Sep 05, 2016 at 05:20:53PM +0200, David Sterba wrote:
> > On Fri, Sep 02, 2016 at 06:08:35PM -0700, Liu Bo wrote:
> > > On Fri, Sep 02, 2016 at 03:25:43PM -0400, Josef Bacik wrote:
> > > > We don't track the reloc roots in any sort of normal way, so the only way the
> > > > root/commit_root nodes get free'd is if the relocation finishes successfully and
> > > > the reloc root is deleted. Fix this by free'ing them in free_reloc_roots.
> > > > Thanks,
> > >
> > > Looks good.
> > >
> > > >
> > > > Signed-off-by: Josef Bacik <jbacik@fb.com>
> > > > ---
> > > > fs/btrfs/relocation.c | 4 ++++
> > > > 1 file changed, 4 insertions(+)
> > > >
> > > > diff --git a/fs/btrfs/relocation.c b/fs/btrfs/relocation.c
> > > > index 7fc6ea7..62dfc2c 100644
> > > > --- a/fs/btrfs/relocation.c
> > > > +++ b/fs/btrfs/relocation.c
> > > > @@ -2349,6 +2349,10 @@ void free_reloc_roots(struct list_head *list)
> > > > while (!list_empty(list)) {
> > > > reloc_root = list_entry(list->next, struct btrfs_root,
> > > > root_list);
> > > > + free_extent_buffer(reloc_root->node);
> > > > + free_extent_buffer(reloc_root->commit_root);
> > > > + reloc_root->node = NULL;
> > > > + reloc_root->commit_root = NULL;
> > >
> > > What about reloc_root itself?
> >
> > > > __del_reloc_root(reloc_root);
> >
> > It's deleted at the end of __del_reloc_root
>
> __del_reloc_root() is dealing with the associated rb_node, not
> reloc_root.
>
> I posted a patch to free reloc_root while free'ing its corresponding fs
> root, but it'd be good if we can free it here.
Can you please point me to the patch?
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] Btrfs: don't leak reloc root nodes on errorg
2016-09-21 14:00 ` David Sterba
@ 2016-09-21 22:45 ` Liu Bo
0 siblings, 0 replies; 6+ messages in thread
From: Liu Bo @ 2016-09-21 22:45 UTC (permalink / raw)
To: dsterba; +Cc: Josef Bacik, linux-btrfs
On Wed, Sep 21, 2016 at 04:00:05PM +0200, David Sterba wrote:
> On Tue, Sep 06, 2016 at 12:22:01PM -0700, Liu Bo wrote:
> > On Mon, Sep 05, 2016 at 05:20:53PM +0200, David Sterba wrote:
> > > On Fri, Sep 02, 2016 at 06:08:35PM -0700, Liu Bo wrote:
> > > > On Fri, Sep 02, 2016 at 03:25:43PM -0400, Josef Bacik wrote:
> > > > > We don't track the reloc roots in any sort of normal way, so the only way the
> > > > > root/commit_root nodes get free'd is if the relocation finishes successfully and
> > > > > the reloc root is deleted. Fix this by free'ing them in free_reloc_roots.
> > > > > Thanks,
> > > >
> > > > Looks good.
> > > >
> > > > >
> > > > > Signed-off-by: Josef Bacik <jbacik@fb.com>
> > > > > ---
> > > > > fs/btrfs/relocation.c | 4 ++++
> > > > > 1 file changed, 4 insertions(+)
> > > > >
> > > > > diff --git a/fs/btrfs/relocation.c b/fs/btrfs/relocation.c
> > > > > index 7fc6ea7..62dfc2c 100644
> > > > > --- a/fs/btrfs/relocation.c
> > > > > +++ b/fs/btrfs/relocation.c
> > > > > @@ -2349,6 +2349,10 @@ void free_reloc_roots(struct list_head *list)
> > > > > while (!list_empty(list)) {
> > > > > reloc_root = list_entry(list->next, struct btrfs_root,
> > > > > root_list);
> > > > > + free_extent_buffer(reloc_root->node);
> > > > > + free_extent_buffer(reloc_root->commit_root);
> > > > > + reloc_root->node = NULL;
> > > > > + reloc_root->commit_root = NULL;
> > > >
> > > > What about reloc_root itself?
> > >
> > > > > __del_reloc_root(reloc_root);
> > >
> > > It's deleted at the end of __del_reloc_root
> >
> > __del_reloc_root() is dealing with the associated rb_node, not
> > reloc_root.
> >
> > I posted a patch to free reloc_root while free'ing its corresponding fs
> > root, but it'd be good if we can free it here.
>
> Can you please point me to the patch?
Btrfs: fix memory leak of reloc_root
https://patchwork.kernel.org/patch/9238383/
Thanks,
-liubo
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2016-09-21 22:45 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-09-02 19:25 [PATCH] Btrfs: don't leak reloc root nodes on error Josef Bacik
2016-09-03 1:08 ` Liu Bo
2016-09-05 15:20 ` David Sterba
2016-09-06 19:22 ` Liu Bo
2016-09-21 14:00 ` David Sterba
2016-09-21 22:45 ` [PATCH] Btrfs: don't leak reloc root nodes on errorg Liu Bo
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.