* [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.