Filipe David Borba Manana
2013-Aug-19 18:08 UTC
[RFC PATCH] Btrfs: fix memory leak of orphan block rsv
When adding orphans to an inode''s root, we start a transaction for
that root that when ended in several places such as for example
extent-tree.c:btrfs_remove_block_group(), inode.c:btrfs_unlink() and
inode.c:btrfs_evict_node(), doesn''t result in a commit, that is,
inode.c:btrfs_orphan_commit_root() doesn''t get called (via
transaction.c:commit_fs_roots()).
The respective inode will end up being called by inode.c:btrfs_evict_node()
(via the VFS). So my likely dirty hack, after some debugging, of
freeing the orphan block rsv in btrfs_evict_inode() if its not being
used by other tasks, seems to fix the leak without failing xfstests
so far. But this is unlikely to be a correct solution without causing
issues elsewhere.
This commit also changes calls to btrfs_orphan_del() which grabbed the
root''s orphan_block_rsv without acquiring the orphan_lock first
(confront
with btrfs_orphan_commit_root(), which acquires the lock before getting
and setting the root''s orphan_block_rsv).
This issue is simple to reproduce and observe if kmemleak is enabled.
Two simple ways to reproduce it:
** 1
$ mkfs.btrfs -f /dev/loop0
$ mount /dev/loop0 /mnt/btrfs
$ btrfs balance start /mnt/btrfs
$ umount /mnt/btrfs
** 2
$ mkfs.btrfs -f /dev/loop0
$ mount /dev/loop0 /mnt/btrfs
$ touch /mnt/btrfs/foobar
$ rm -f /mnt/btrfs/foobar
$ umount /mnt/btrfs
After a while, kmemleak reports the leak:
$ cat /sys/kernel/debug/kmemleak
unreferenced object 0xffff880402b13e00 (size 128):
comm "btrfs", pid 19621, jiffies 4341648183 (age 70057.844s)
hex dump (first 32 bytes):
00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 ................
00 fc c6 b1 04 88 ff ff 04 00 04 00 ad 4e ad de .............N..
backtrace:
[<ffffffff817275a6>] kmemleak_alloc+0x26/0x50
[<ffffffff8117832b>] kmem_cache_alloc_trace+0xeb/0x1d0
[<ffffffffa04db499>] btrfs_alloc_block_rsv+0x39/0x70 [btrfs]
[<ffffffffa04f8bad>] btrfs_orphan_add+0x13d/0x1b0 [btrfs]
[<ffffffffa04e2b13>] btrfs_remove_block_group+0x143/0x500 [btrfs]
[<ffffffffa0518158>] btrfs_relocate_chunk.isra.63+0x618/0x790 [btrfs]
[<ffffffffa051bc27>] btrfs_balance+0x8f7/0xe90 [btrfs]
[<ffffffffa05240a0>] btrfs_ioctl_balance+0x250/0x550 [btrfs]
[<ffffffffa05269ca>] btrfs_ioctl+0xdfa/0x25f0 [btrfs]
[<ffffffff8119c936>] do_vfs_ioctl+0x96/0x570
[<ffffffff8119cea1>] SyS_ioctl+0x91/0xb0
[<ffffffff81750242>] system_call_fastpath+0x16/0x1b
[<ffffffffffffffff>] 0xffffffffffffffff
This affects btrfs-next, revision be8e3cd00d7293dd177e3f8a4a1645ce09ca3acb
(Btrfs: separate out tests into their own directory).
Signed-off-by: Filipe David Borba Manana <fdmanana@gmail.com>
---
fs/btrfs/ctree.h | 1 +
fs/btrfs/extent-tree.c | 1 +
fs/btrfs/inode.c | 36 +++++++++++++++++++++++++++++-------
3 files changed, 31 insertions(+), 7 deletions(-)
diff --git a/fs/btrfs/ctree.h b/fs/btrfs/ctree.h
index 7c93d9f..da3ac13 100644
--- a/fs/btrfs/ctree.h
+++ b/fs/btrfs/ctree.h
@@ -1153,6 +1153,7 @@ struct btrfs_block_rsv {
u64 reserved;
struct btrfs_space_info *space_info;
spinlock_t lock;
+ atomic_t count;
unsigned short full;
unsigned short type;
unsigned short failfast;
diff --git a/fs/btrfs/extent-tree.c b/fs/btrfs/extent-tree.c
index 32639f2..3ecee8e 100644
--- a/fs/btrfs/extent-tree.c
+++ b/fs/btrfs/extent-tree.c
@@ -4500,6 +4500,7 @@ struct btrfs_block_rsv *btrfs_alloc_block_rsv(struct
btrfs_root *root,
btrfs_init_block_rsv(block_rsv, type);
block_rsv->space_info = __find_space_info(fs_info,
BTRFS_BLOCK_GROUP_METADATA);
+ atomic_set(&block_rsv->count, 0);
return block_rsv;
}
diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c
index b67b81a..459ffe2 100644
--- a/fs/btrfs/inode.c
+++ b/fs/btrfs/inode.c
@@ -2870,8 +2870,13 @@ void btrfs_orphan_commit_root(struct btrfs_trans_handle
*trans,
return;
}
- block_rsv = root->orphan_block_rsv;
- root->orphan_block_rsv = NULL;
+ if (root->orphan_block_rsv &&
+ atomic_dec_and_test(&root->orphan_block_rsv->count)) {
+ block_rsv = root->orphan_block_rsv;
+ root->orphan_block_rsv = NULL;
+ } else {
+ block_rsv = NULL;
+ }
spin_unlock(&root->orphan_lock);
if (root->orphan_item_inserted &&
@@ -2918,6 +2923,7 @@ int btrfs_orphan_add(struct btrfs_trans_handle *trans,
struct inode *inode)
btrfs_free_block_rsv(root, block_rsv);
block_rsv = NULL;
}
+ atomic_inc(&root->orphan_block_rsv->count);
if (!test_and_set_bit(BTRFS_INODE_HAS_ORPHAN_ITEM,
&BTRFS_I(inode)->runtime_flags)) {
@@ -2991,6 +2997,9 @@ static int btrfs_orphan_del(struct btrfs_trans_handle
*trans,
int ret = 0;
spin_lock(&root->orphan_lock);
+ if (trans)
+ trans->block_rsv = root->orphan_block_rsv;
+
if (test_and_clear_bit(BTRFS_INODE_HAS_ORPHAN_ITEM,
&BTRFS_I(inode)->runtime_flags))
delete_item = 1;
@@ -4520,12 +4529,10 @@ void btrfs_evict_inode(struct inode *inode)
* Errors here aren''t a big deal, it just means we leave orphan items
* in the tree. They will be cleaned up on the next mount.
*/
- if (ret == 0) {
- trans->block_rsv = root->orphan_block_rsv;
+ if (ret == 0)
btrfs_orphan_del(trans, inode);
- } else {
+ else
btrfs_orphan_del(NULL, inode);
- }
trans->block_rsv = &root->fs_info->trans_block_rsv;
if (!(root == root->fs_info->tree_root ||
@@ -4535,6 +4542,22 @@ void btrfs_evict_inode(struct inode *inode)
btrfs_end_transaction(trans, root);
btrfs_btree_balance_dirty(root);
no_delete:
+
+ spin_lock(&root->orphan_lock);
+ if (root->orphan_block_rsv &&
+ atomic_dec_and_test(&root->orphan_block_rsv->count)) {
+ rsv = root->orphan_block_rsv;
+ root->orphan_block_rsv = NULL;
+ } else {
+ rsv = NULL;
+ }
+ spin_unlock(&root->orphan_lock);
+
+ if (rsv) {
+ WARN_ON(rsv->size > 0);
+ btrfs_free_block_rsv(root, rsv);
+ }
+
btrfs_remove_delayed_node(inode);
clear_inode(inode);
return;
@@ -7650,7 +7673,6 @@ static int btrfs_truncate(struct inode *inode)
}
if (ret == 0 && inode->i_nlink > 0) {
- trans->block_rsv = root->orphan_block_rsv;
ret = btrfs_orphan_del(trans, inode);
if (ret)
err = ret;
--
1.7.9.5
--
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
Filipe David Borba Manana
2013-Aug-19 18:31 UTC
[PATCH v2] Btrfs: fix memory leak of orphan block rsv
When adding orphans to an inode''s root, we start a transaction for
that root that when ended in several places such as for example
extent-tree.c:btrfs_remove_block_group(), inode.c:btrfs_unlink() and
inode.c:btrfs_evict_node(), doesn''t result in a commit, that is,
inode.c:btrfs_orphan_commit_root() doesn''t get called (via
transaction.c:commit_fs_roots()).
The respective inode will end up being called by inode.c:btrfs_evict_node()
(via the VFS). So my likely dirty hack, after some debugging, of
freeing the orphan block rsv in btrfs_evict_inode() if it''s not being
used by other tasks, seems to fix the leak for the cases listed below and
without failing xfstests.
This commit also changes calls to btrfs_orphan_del() which grabbed the
root''s orphan_block_rsv without acquiring the orphan_lock first
(confront
with btrfs_orphan_commit_root(), which acquires the lock before getting
and setting the root''s orphan_block_rsv).
This issue is simple to reproduce and observe if kmemleak is enabled.
There are at least two simple ways to reproduce it:
** 1
$ mkfs.btrfs -f /dev/loop0
$ mount /dev/loop0 /mnt/btrfs
$ btrfs balance start /mnt/btrfs
$ umount /mnt/btrfs
** 2
$ mkfs.btrfs -f /dev/loop0
$ mount /dev/loop0 /mnt/btrfs
$ touch /mnt/btrfs/foobar
$ rm -f /mnt/btrfs/foobar
$ umount /mnt/btrfs
After a while, kmemleak reports the leak:
$ cat /sys/kernel/debug/kmemleak
unreferenced object 0xffff880402b13e00 (size 128):
comm "btrfs", pid 19621, jiffies 4341648183 (age 70057.844s)
hex dump (first 32 bytes):
00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 ................
00 fc c6 b1 04 88 ff ff 04 00 04 00 ad 4e ad de .............N..
backtrace:
[<ffffffff817275a6>] kmemleak_alloc+0x26/0x50
[<ffffffff8117832b>] kmem_cache_alloc_trace+0xeb/0x1d0
[<ffffffffa04db499>] btrfs_alloc_block_rsv+0x39/0x70 [btrfs]
[<ffffffffa04f8bad>] btrfs_orphan_add+0x13d/0x1b0 [btrfs]
[<ffffffffa04e2b13>] btrfs_remove_block_group+0x143/0x500 [btrfs]
[<ffffffffa0518158>] btrfs_relocate_chunk.isra.63+0x618/0x790 [btrfs]
[<ffffffffa051bc27>] btrfs_balance+0x8f7/0xe90 [btrfs]
[<ffffffffa05240a0>] btrfs_ioctl_balance+0x250/0x550 [btrfs]
[<ffffffffa05269ca>] btrfs_ioctl+0xdfa/0x25f0 [btrfs]
[<ffffffff8119c936>] do_vfs_ioctl+0x96/0x570
[<ffffffff8119cea1>] SyS_ioctl+0x91/0xb0
[<ffffffff81750242>] system_call_fastpath+0x16/0x1b
[<ffffffffffffffff>] 0xffffffffffffffff
This affects btrfs-next, revision be8e3cd00d7293dd177e3f8a4a1645ce09ca3acb
(Btrfs: separate out tests into their own directory).
Signed-off-by: Filipe David Borba Manana <fdmanana@gmail.com>
---
V2: removed atomic_t member in struct btrfs_block_rsv, as suggested by
Josef Bacik, and use instead the condition reserved == 0 to decide
when to free the block.
fs/btrfs/inode.c | 35 ++++++++++++++++++++++++++++-------
1 file changed, 28 insertions(+), 7 deletions(-)
diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c
index b67b81a..905de1e2 100644
--- a/fs/btrfs/inode.c
+++ b/fs/btrfs/inode.c
@@ -2870,8 +2870,13 @@ void btrfs_orphan_commit_root(struct btrfs_trans_handle
*trans,
return;
}
- block_rsv = root->orphan_block_rsv;
- root->orphan_block_rsv = NULL;
+ if (root->orphan_block_rsv &&
+ root->orphan_block_rsv->reserved == 0) {
+ block_rsv = root->orphan_block_rsv;
+ root->orphan_block_rsv = NULL;
+ } else {
+ block_rsv = NULL;
+ }
spin_unlock(&root->orphan_lock);
if (root->orphan_item_inserted &&
@@ -2991,6 +2996,9 @@ static int btrfs_orphan_del(struct btrfs_trans_handle
*trans,
int ret = 0;
spin_lock(&root->orphan_lock);
+ if (trans)
+ trans->block_rsv = root->orphan_block_rsv;
+
if (test_and_clear_bit(BTRFS_INODE_HAS_ORPHAN_ITEM,
&BTRFS_I(inode)->runtime_flags))
delete_item = 1;
@@ -4520,12 +4528,10 @@ void btrfs_evict_inode(struct inode *inode)
* Errors here aren''t a big deal, it just means we leave orphan items
* in the tree. They will be cleaned up on the next mount.
*/
- if (ret == 0) {
- trans->block_rsv = root->orphan_block_rsv;
+ if (ret == 0)
btrfs_orphan_del(trans, inode);
- } else {
+ else
btrfs_orphan_del(NULL, inode);
- }
trans->block_rsv = &root->fs_info->trans_block_rsv;
if (!(root == root->fs_info->tree_root ||
@@ -4535,6 +4541,22 @@ void btrfs_evict_inode(struct inode *inode)
btrfs_end_transaction(trans, root);
btrfs_btree_balance_dirty(root);
no_delete:
+
+ spin_lock(&root->orphan_lock);
+ if (root->orphan_block_rsv &&
+ root->orphan_block_rsv->reserved == 0) {
+ rsv = root->orphan_block_rsv;
+ root->orphan_block_rsv = NULL;
+ } else {
+ rsv = NULL;
+ }
+ spin_unlock(&root->orphan_lock);
+
+ if (rsv) {
+ WARN_ON(rsv->size > 0);
+ btrfs_free_block_rsv(root, rsv);
+ }
+
btrfs_remove_delayed_node(inode);
clear_inode(inode);
return;
@@ -7650,7 +7672,6 @@ static int btrfs_truncate(struct inode *inode)
}
if (ret == 0 && inode->i_nlink > 0) {
- trans->block_rsv = root->orphan_block_rsv;
ret = btrfs_orphan_del(trans, inode);
if (ret)
err = ret;
--
1.7.9.5
--
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
Filipe David Borba Manana
2013-Aug-19 20:15 UTC
[PATCH v3] Btrfs: fix memory leak of orphan block rsv
This issue is simple to reproduce and observe if kmemleak is enabled.
Two simple ways to reproduce it:
** 1
$ mkfs.btrfs -f /dev/loop0
$ mount /dev/loop0 /mnt/btrfs
$ btrfs balance start /mnt/btrfs
$ umount /mnt/btrfs
** 2
$ mkfs.btrfs -f /dev/loop0
$ mount /dev/loop0 /mnt/btrfs
$ touch /mnt/btrfs/foobar
$ rm -f /mnt/btrfs/foobar
$ umount /mnt/btrfs
After a while, kmemleak reports the leak:
$ cat /sys/kernel/debug/kmemleak
unreferenced object 0xffff880402b13e00 (size 128):
comm "btrfs", pid 19621, jiffies 4341648183 (age 70057.844s)
hex dump (first 32 bytes):
00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 ................
00 fc c6 b1 04 88 ff ff 04 00 04 00 ad 4e ad de .............N..
backtrace:
[<ffffffff817275a6>] kmemleak_alloc+0x26/0x50
[<ffffffff8117832b>] kmem_cache_alloc_trace+0xeb/0x1d0
[<ffffffffa04db499>] btrfs_alloc_block_rsv+0x39/0x70 [btrfs]
[<ffffffffa04f8bad>] btrfs_orphan_add+0x13d/0x1b0 [btrfs]
[<ffffffffa04e2b13>] btrfs_remove_block_group+0x143/0x500 [btrfs]
[<ffffffffa0518158>] btrfs_relocate_chunk.isra.63+0x618/0x790 [btrfs]
[<ffffffffa051bc27>] btrfs_balance+0x8f7/0xe90 [btrfs]
[<ffffffffa05240a0>] btrfs_ioctl_balance+0x250/0x550 [btrfs]
[<ffffffffa05269ca>] btrfs_ioctl+0xdfa/0x25f0 [btrfs]
[<ffffffff8119c936>] do_vfs_ioctl+0x96/0x570
[<ffffffff8119cea1>] SyS_ioctl+0x91/0xb0
[<ffffffff81750242>] system_call_fastpath+0x16/0x1b
[<ffffffffffffffff>] 0xffffffffffffffff
This affects btrfs-next, revision be8e3cd00d7293dd177e3f8a4a1645ce09ca3acb
(Btrfs: separate out tests into their own directory).
Signed-off-by: Filipe David Borba Manana <fdmanana@gmail.com>
---
V2: removed atomic_t member in struct btrfs_block_rsv, as suggested by
Josef Bacik, and use instead the condition reserved == 0 to decide
when to free the block.
V3: simplified patch, just kfree() (and not btrfs_free_block_rsv) the
root''s orphan_block_rsv when free''ing the root. Thanks
Josef for
the suggestion.
fs/btrfs/disk-io.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/fs/btrfs/disk-io.c b/fs/btrfs/disk-io.c
index 3b12c26..c7d2fd8 100644
--- a/fs/btrfs/disk-io.c
+++ b/fs/btrfs/disk-io.c
@@ -3434,6 +3434,7 @@ static void free_fs_root(struct btrfs_root *root)
free_anon_bdev(root->anon_dev);
free_extent_buffer(root->node);
free_extent_buffer(root->commit_root);
+ kfree(root->orphan_block_rsv);
kfree(root->free_ino_ctl);
kfree(root->free_ino_pinned);
kfree(root->name);
--
1.7.9.5
--
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
Filipe David Borba Manana
2013-Aug-19 20:30 UTC
[PATCH v4] Btrfs: fix memory leak of orphan block rsv
This issue is simple to reproduce and observe if kmemleak is enabled.
Two simple ways to reproduce it:
** 1
$ mkfs.btrfs -f /dev/loop0
$ mount /dev/loop0 /mnt/btrfs
$ btrfs balance start /mnt/btrfs
$ umount /mnt/btrfs
** 2
$ mkfs.btrfs -f /dev/loop0
$ mount /dev/loop0 /mnt/btrfs
$ touch /mnt/btrfs/foobar
$ rm -f /mnt/btrfs/foobar
$ umount /mnt/btrfs
After a while, kmemleak reports the leak:
$ cat /sys/kernel/debug/kmemleak
unreferenced object 0xffff880402b13e00 (size 128):
comm "btrfs", pid 19621, jiffies 4341648183 (age 70057.844s)
hex dump (first 32 bytes):
00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 ................
00 fc c6 b1 04 88 ff ff 04 00 04 00 ad 4e ad de .............N..
backtrace:
[<ffffffff817275a6>] kmemleak_alloc+0x26/0x50
[<ffffffff8117832b>] kmem_cache_alloc_trace+0xeb/0x1d0
[<ffffffffa04db499>] btrfs_alloc_block_rsv+0x39/0x70 [btrfs]
[<ffffffffa04f8bad>] btrfs_orphan_add+0x13d/0x1b0 [btrfs]
[<ffffffffa04e2b13>] btrfs_remove_block_group+0x143/0x500 [btrfs]
[<ffffffffa0518158>] btrfs_relocate_chunk.isra.63+0x618/0x790 [btrfs]
[<ffffffffa051bc27>] btrfs_balance+0x8f7/0xe90 [btrfs]
[<ffffffffa05240a0>] btrfs_ioctl_balance+0x250/0x550 [btrfs]
[<ffffffffa05269ca>] btrfs_ioctl+0xdfa/0x25f0 [btrfs]
[<ffffffff8119c936>] do_vfs_ioctl+0x96/0x570
[<ffffffff8119cea1>] SyS_ioctl+0x91/0xb0
[<ffffffff81750242>] system_call_fastpath+0x16/0x1b
[<ffffffffffffffff>] 0xffffffffffffffff
This affects btrfs-next, revision be8e3cd00d7293dd177e3f8a4a1645ce09ca3acb
(Btrfs: separate out tests into their own directory).
Signed-off-by: Filipe David Borba Manana <fdmanana@gmail.com>
---
V2: removed atomic_t member in struct btrfs_block_rsv, as suggested by
Josef Bacik, and use instead the condition reserved == 0 to decide
when to free the block.
V3: simplified patch, just kfree() (and not btrfs_free_block_rsv) the
root''s orphan_block_rsv when free''ing the root. Thanks
Josef for
the suggestion.
V4: use btrfs_free_block_rsv() instead of kfree(). The error I was getting
in xfstests when using btrfs_free_block_rsv() was unrelated, Josef just
pointed it to me (separate issue).
fs/btrfs/disk-io.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/fs/btrfs/disk-io.c b/fs/btrfs/disk-io.c
index 3b12c26..c4b90a0 100644
--- a/fs/btrfs/disk-io.c
+++ b/fs/btrfs/disk-io.c
@@ -3428,6 +3428,8 @@ void btrfs_drop_and_free_fs_root(struct btrfs_fs_info
*fs_info,
static void free_fs_root(struct btrfs_root *root)
{
+ btrfs_free_block_rsv(root, root->orphan_block_rsv);
+ root->orphan_block_rsv = NULL;
iput(root->cache_inode);
WARN_ON(!RB_EMPTY_ROOT(&root->inode_tree));
if (root->anon_dev)
--
1.7.9.5
--
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
Filipe David Borba Manana
2013-Aug-19 20:42 UTC
[PATCH v5] Btrfs: fix memory leak of orphan block rsv
This issue is simple to reproduce and observe if kmemleak is enabled.
Two simple ways to reproduce it:
** 1
$ mkfs.btrfs -f /dev/loop0
$ mount /dev/loop0 /mnt/btrfs
$ btrfs balance start /mnt/btrfs
$ umount /mnt/btrfs
** 2
$ mkfs.btrfs -f /dev/loop0
$ mount /dev/loop0 /mnt/btrfs
$ touch /mnt/btrfs/foobar
$ rm -f /mnt/btrfs/foobar
$ umount /mnt/btrfs
After a while, kmemleak reports the leak:
$ cat /sys/kernel/debug/kmemleak
unreferenced object 0xffff880402b13e00 (size 128):
comm "btrfs", pid 19621, jiffies 4341648183 (age 70057.844s)
hex dump (first 32 bytes):
00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 ................
00 fc c6 b1 04 88 ff ff 04 00 04 00 ad 4e ad de .............N..
backtrace:
[<ffffffff817275a6>] kmemleak_alloc+0x26/0x50
[<ffffffff8117832b>] kmem_cache_alloc_trace+0xeb/0x1d0
[<ffffffffa04db499>] btrfs_alloc_block_rsv+0x39/0x70 [btrfs]
[<ffffffffa04f8bad>] btrfs_orphan_add+0x13d/0x1b0 [btrfs]
[<ffffffffa04e2b13>] btrfs_remove_block_group+0x143/0x500 [btrfs]
[<ffffffffa0518158>] btrfs_relocate_chunk.isra.63+0x618/0x790 [btrfs]
[<ffffffffa051bc27>] btrfs_balance+0x8f7/0xe90 [btrfs]
[<ffffffffa05240a0>] btrfs_ioctl_balance+0x250/0x550 [btrfs]
[<ffffffffa05269ca>] btrfs_ioctl+0xdfa/0x25f0 [btrfs]
[<ffffffff8119c936>] do_vfs_ioctl+0x96/0x570
[<ffffffff8119cea1>] SyS_ioctl+0x91/0xb0
[<ffffffff81750242>] system_call_fastpath+0x16/0x1b
[<ffffffffffffffff>] 0xffffffffffffffff
This affects btrfs-next, revision be8e3cd00d7293dd177e3f8a4a1645ce09ca3acb
(Btrfs: separate out tests into their own directory).
Signed-off-by: Filipe David Borba Manana <fdmanana@gmail.com>
---
V2: removed atomic_t member in struct btrfs_block_rsv, as suggested by
Josef Bacik, and use instead the condition reserved == 0 to decide
when to free the block.
V3: simplified patch, just kfree() (and not btrfs_free_block_rsv) the
root''s orphan_block_rsv when free''ing the root. Thanks
Josef for
the suggestion.
V4: use btrfs_free_block_rsv() instead of kfree(). The error I was getting
in xfstests when using btrfs_free_block_rsv() was unrelated, Josef just
pointed it to me (separate issue).
V5: move the free call below the iput() call, so that btrfs_evict_node()
can process the orphan_block_rsv first to do some needed cleanup before
we free it.
fs/btrfs/disk-io.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/fs/btrfs/disk-io.c b/fs/btrfs/disk-io.c
index 3b12c26..9e66543 100644
--- a/fs/btrfs/disk-io.c
+++ b/fs/btrfs/disk-io.c
@@ -3430,6 +3430,8 @@ static void free_fs_root(struct btrfs_root *root)
{
iput(root->cache_inode);
WARN_ON(!RB_EMPTY_ROOT(&root->inode_tree));
+ btrfs_free_block_rsv(root, root->orphan_block_rsv);
+ root->orphan_block_rsv = NULL;
if (root->anon_dev)
free_anon_bdev(root->anon_dev);
free_extent_buffer(root->node);
--
1.7.9.5
--
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
Filipe David Borba Manana
2013-Aug-19 23:52 UTC
[PATCH v6] Btrfs: fix memory leak of orphan block rsv
This issue is simple to reproduce and observe if kmemleak is enabled.
Two simple ways to reproduce it:
** 1
$ mkfs.btrfs -f /dev/loop0
$ mount /dev/loop0 /mnt/btrfs
$ btrfs balance start /mnt/btrfs
$ umount /mnt/btrfs
** 2
$ mkfs.btrfs -f /dev/loop0
$ mount /dev/loop0 /mnt/btrfs
$ touch /mnt/btrfs/foobar
$ rm -f /mnt/btrfs/foobar
$ umount /mnt/btrfs
After a while, kmemleak reports the leak:
$ cat /sys/kernel/debug/kmemleak
unreferenced object 0xffff880402b13e00 (size 128):
comm "btrfs", pid 19621, jiffies 4341648183 (age 70057.844s)
hex dump (first 32 bytes):
00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 ................
00 fc c6 b1 04 88 ff ff 04 00 04 00 ad 4e ad de .............N..
backtrace:
[<ffffffff817275a6>] kmemleak_alloc+0x26/0x50
[<ffffffff8117832b>] kmem_cache_alloc_trace+0xeb/0x1d0
[<ffffffffa04db499>] btrfs_alloc_block_rsv+0x39/0x70 [btrfs]
[<ffffffffa04f8bad>] btrfs_orphan_add+0x13d/0x1b0 [btrfs]
[<ffffffffa04e2b13>] btrfs_remove_block_group+0x143/0x500 [btrfs]
[<ffffffffa0518158>] btrfs_relocate_chunk.isra.63+0x618/0x790 [btrfs]
[<ffffffffa051bc27>] btrfs_balance+0x8f7/0xe90 [btrfs]
[<ffffffffa05240a0>] btrfs_ioctl_balance+0x250/0x550 [btrfs]
[<ffffffffa05269ca>] btrfs_ioctl+0xdfa/0x25f0 [btrfs]
[<ffffffff8119c936>] do_vfs_ioctl+0x96/0x570
[<ffffffff8119cea1>] SyS_ioctl+0x91/0xb0
[<ffffffff81750242>] system_call_fastpath+0x16/0x1b
[<ffffffffffffffff>] 0xffffffffffffffff
This affects btrfs-next, revision be8e3cd00d7293dd177e3f8a4a1645ce09ca3acb
(Btrfs: separate out tests into their own directory).
Signed-off-by: Filipe David Borba Manana <fdmanana@gmail.com>
---
V2: removed atomic_t member in struct btrfs_block_rsv, as suggested by
Josef Bacik, and use instead the condition reserved == 0 to decide
when to free the block.
V3: simplified patch, just kfree() (and not btrfs_free_block_rsv) the
root''s orphan_block_rsv when free''ing the root. Thanks
Josef for
the suggestion.
V4: use btrfs_free_block_rsv() instead of kfree(). The error I was getting
in xfstests when using btrfs_free_block_rsv() was unrelated, Josef just
pointed it to me (separate issue).
V5: move the free call below the iput() call, so that btrfs_evict_node()
can process the orphan_block_rsv first to do some needed cleanup before
we free it.
V6: free the root''s orphan_block_rsv in close_ctree() too. After a
balance
the orphan_block_rsv of the tree of tree roots was being leaked, because
free_fs_root() is only called for filesystem trees.
fs/btrfs/disk-io.c | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/fs/btrfs/disk-io.c b/fs/btrfs/disk-io.c
index 3b12c26..5d17163 100644
--- a/fs/btrfs/disk-io.c
+++ b/fs/btrfs/disk-io.c
@@ -3430,6 +3430,8 @@ static void free_fs_root(struct btrfs_root *root)
{
iput(root->cache_inode);
WARN_ON(!RB_EMPTY_ROOT(&root->inode_tree));
+ btrfs_free_block_rsv(root, root->orphan_block_rsv);
+ root->orphan_block_rsv = NULL;
if (root->anon_dev)
free_anon_bdev(root->anon_dev);
free_extent_buffer(root->node);
@@ -3582,6 +3584,9 @@ int close_ctree(struct btrfs_root *root)
btrfs_free_stripe_hash_table(fs_info);
+ btrfs_free_block_rsv(root, root->orphan_block_rsv);
+ root->orphan_block_rsv = NULL;
+
return 0;
}
--
1.7.9.5
--
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
Alex Lyakas
2013-Oct-23 13:33 UTC
Re: [PATCH v6] Btrfs: fix memory leak of orphan block rsv
Hi Filipe, On Tue, Aug 20, 2013 at 2:52 AM, Filipe David Borba Manana <fdmanana@gmail.com> wrote:> > This issue is simple to reproduce and observe if kmemleak is enabled. > Two simple ways to reproduce it: > > ** 1 > > $ mkfs.btrfs -f /dev/loop0 > $ mount /dev/loop0 /mnt/btrfs > $ btrfs balance start /mnt/btrfs > $ umount /mnt/btrfs > > ** 2 > > $ mkfs.btrfs -f /dev/loop0 > $ mount /dev/loop0 /mnt/btrfs > $ touch /mnt/btrfs/foobar > $ rm -f /mnt/btrfs/foobar > $ umount /mnt/btrfsI tried the second repro script on kernel 3.8.13, and kmemleak does not report a leak (even if I force the kmemleak scan). I did not try the balance-repro script, though. Am I missing something? Thanks, Alex.> > > After a while, kmemleak reports the leak: > > $ cat /sys/kernel/debug/kmemleak > unreferenced object 0xffff880402b13e00 (size 128): > comm "btrfs", pid 19621, jiffies 4341648183 (age 70057.844s) > hex dump (first 32 bytes): > 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 ................ > 00 fc c6 b1 04 88 ff ff 04 00 04 00 ad 4e ad de .............N.. > backtrace: > [<ffffffff817275a6>] kmemleak_alloc+0x26/0x50 > [<ffffffff8117832b>] kmem_cache_alloc_trace+0xeb/0x1d0 > [<ffffffffa04db499>] btrfs_alloc_block_rsv+0x39/0x70 [btrfs] > [<ffffffffa04f8bad>] btrfs_orphan_add+0x13d/0x1b0 [btrfs] > [<ffffffffa04e2b13>] btrfs_remove_block_group+0x143/0x500 [btrfs] > [<ffffffffa0518158>] btrfs_relocate_chunk.isra.63+0x618/0x790 [btrfs] > [<ffffffffa051bc27>] btrfs_balance+0x8f7/0xe90 [btrfs] > [<ffffffffa05240a0>] btrfs_ioctl_balance+0x250/0x550 [btrfs] > [<ffffffffa05269ca>] btrfs_ioctl+0xdfa/0x25f0 [btrfs] > [<ffffffff8119c936>] do_vfs_ioctl+0x96/0x570 > [<ffffffff8119cea1>] SyS_ioctl+0x91/0xb0 > [<ffffffff81750242>] system_call_fastpath+0x16/0x1b > [<ffffffffffffffff>] 0xffffffffffffffff > > This affects btrfs-next, revision be8e3cd00d7293dd177e3f8a4a1645ce09ca3acb > (Btrfs: separate out tests into their own directory). > > Signed-off-by: Filipe David Borba Manana <fdmanana@gmail.com> > --- > > V2: removed atomic_t member in struct btrfs_block_rsv, as suggested by > Josef Bacik, and use instead the condition reserved == 0 to decide > when to free the block. > V3: simplified patch, just kfree() (and not btrfs_free_block_rsv) the > root''s orphan_block_rsv when free''ing the root. Thanks Josef for > the suggestion. > V4: use btrfs_free_block_rsv() instead of kfree(). The error I was getting > in xfstests when using btrfs_free_block_rsv() was unrelated, Josef just > pointed it to me (separate issue). > V5: move the free call below the iput() call, so that btrfs_evict_node() > can process the orphan_block_rsv first to do some needed cleanup before > we free it. > V6: free the root''s orphan_block_rsv in close_ctree() too. After a balance > the orphan_block_rsv of the tree of tree roots was being leaked, because > free_fs_root() is only called for filesystem trees. > > fs/btrfs/disk-io.c | 5 +++++ > 1 file changed, 5 insertions(+) > > diff --git a/fs/btrfs/disk-io.c b/fs/btrfs/disk-io.c > index 3b12c26..5d17163 100644 > --- a/fs/btrfs/disk-io.c > +++ b/fs/btrfs/disk-io.c > @@ -3430,6 +3430,8 @@ static void free_fs_root(struct btrfs_root *root) > { > iput(root->cache_inode); > WARN_ON(!RB_EMPTY_ROOT(&root->inode_tree)); > + btrfs_free_block_rsv(root, root->orphan_block_rsv); > + root->orphan_block_rsv = NULL; > if (root->anon_dev) > free_anon_bdev(root->anon_dev); > free_extent_buffer(root->node); > @@ -3582,6 +3584,9 @@ int close_ctree(struct btrfs_root *root) > > btrfs_free_stripe_hash_table(fs_info); > > + btrfs_free_block_rsv(root, root->orphan_block_rsv); > + root->orphan_block_rsv = NULL; > + > return 0; > } > > -- > 1.7.9.5 > > -- > 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-- 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
Filipe David Manana
2013-Oct-23 13:35 UTC
Re: [PATCH v6] Btrfs: fix memory leak of orphan block rsv
On Wed, Oct 23, 2013 at 2:33 PM, Alex Lyakas <alex.btrfs@zadarastorage.com> wrote:> Hi Filipe, > > > On Tue, Aug 20, 2013 at 2:52 AM, Filipe David Borba Manana > <fdmanana@gmail.com> wrote: >> >> This issue is simple to reproduce and observe if kmemleak is enabled. >> Two simple ways to reproduce it: >> >> ** 1 >> >> $ mkfs.btrfs -f /dev/loop0 >> $ mount /dev/loop0 /mnt/btrfs >> $ btrfs balance start /mnt/btrfs >> $ umount /mnt/btrfs >> >> ** 2 >> >> $ mkfs.btrfs -f /dev/loop0 >> $ mount /dev/loop0 /mnt/btrfs >> $ touch /mnt/btrfs/foobar >> $ rm -f /mnt/btrfs/foobar >> $ umount /mnt/btrfs > > > I tried the second repro script on kernel 3.8.13, and kmemleak does > not report a leak (even if I force the kmemleak scan). I did not try > the balance-repro script, though. Am I missing something?Maybe it''s not an issue on 3.8.13 and older releases. This was on btrfs-next from August 19. thanks for testing> > Thanks, > Alex. > > >> >> >> After a while, kmemleak reports the leak: >> >> $ cat /sys/kernel/debug/kmemleak >> unreferenced object 0xffff880402b13e00 (size 128): >> comm "btrfs", pid 19621, jiffies 4341648183 (age 70057.844s) >> hex dump (first 32 bytes): >> 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 ................ >> 00 fc c6 b1 04 88 ff ff 04 00 04 00 ad 4e ad de .............N.. >> backtrace: >> [<ffffffff817275a6>] kmemleak_alloc+0x26/0x50 >> [<ffffffff8117832b>] kmem_cache_alloc_trace+0xeb/0x1d0 >> [<ffffffffa04db499>] btrfs_alloc_block_rsv+0x39/0x70 [btrfs] >> [<ffffffffa04f8bad>] btrfs_orphan_add+0x13d/0x1b0 [btrfs] >> [<ffffffffa04e2b13>] btrfs_remove_block_group+0x143/0x500 [btrfs] >> [<ffffffffa0518158>] btrfs_relocate_chunk.isra.63+0x618/0x790 [btrfs] >> [<ffffffffa051bc27>] btrfs_balance+0x8f7/0xe90 [btrfs] >> [<ffffffffa05240a0>] btrfs_ioctl_balance+0x250/0x550 [btrfs] >> [<ffffffffa05269ca>] btrfs_ioctl+0xdfa/0x25f0 [btrfs] >> [<ffffffff8119c936>] do_vfs_ioctl+0x96/0x570 >> [<ffffffff8119cea1>] SyS_ioctl+0x91/0xb0 >> [<ffffffff81750242>] system_call_fastpath+0x16/0x1b >> [<ffffffffffffffff>] 0xffffffffffffffff >> >> This affects btrfs-next, revision be8e3cd00d7293dd177e3f8a4a1645ce09ca3acb >> (Btrfs: separate out tests into their own directory). >> >> Signed-off-by: Filipe David Borba Manana <fdmanana@gmail.com> >> --- >> >> V2: removed atomic_t member in struct btrfs_block_rsv, as suggested by >> Josef Bacik, and use instead the condition reserved == 0 to decide >> when to free the block. >> V3: simplified patch, just kfree() (and not btrfs_free_block_rsv) the >> root''s orphan_block_rsv when free''ing the root. Thanks Josef for >> the suggestion. >> V4: use btrfs_free_block_rsv() instead of kfree(). The error I was getting >> in xfstests when using btrfs_free_block_rsv() was unrelated, Josef just >> pointed it to me (separate issue). >> V5: move the free call below the iput() call, so that btrfs_evict_node() >> can process the orphan_block_rsv first to do some needed cleanup before >> we free it. >> V6: free the root''s orphan_block_rsv in close_ctree() too. After a balance >> the orphan_block_rsv of the tree of tree roots was being leaked, because >> free_fs_root() is only called for filesystem trees. >> >> fs/btrfs/disk-io.c | 5 +++++ >> 1 file changed, 5 insertions(+) >> >> diff --git a/fs/btrfs/disk-io.c b/fs/btrfs/disk-io.c >> index 3b12c26..5d17163 100644 >> --- a/fs/btrfs/disk-io.c >> +++ b/fs/btrfs/disk-io.c >> @@ -3430,6 +3430,8 @@ static void free_fs_root(struct btrfs_root *root) >> { >> iput(root->cache_inode); >> WARN_ON(!RB_EMPTY_ROOT(&root->inode_tree)); >> + btrfs_free_block_rsv(root, root->orphan_block_rsv); >> + root->orphan_block_rsv = NULL; >> if (root->anon_dev) >> free_anon_bdev(root->anon_dev); >> free_extent_buffer(root->node); >> @@ -3582,6 +3584,9 @@ int close_ctree(struct btrfs_root *root) >> >> btrfs_free_stripe_hash_table(fs_info); >> >> + btrfs_free_block_rsv(root, root->orphan_block_rsv); >> + root->orphan_block_rsv = NULL; >> + >> return 0; >> } >> >> -- >> 1.7.9.5 >> >> -- >> 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-- Filipe David Manana, "Reasonable men adapt themselves to the world. Unreasonable men adapt the world to themselves. That''s why all progress depends on unreasonable men." -- 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
Alex Lyakas
2013-Oct-23 14:14 UTC
Re: [PATCH v6] Btrfs: fix memory leak of orphan block rsv
Hello, On Wed, Oct 23, 2013 at 4:35 PM, Filipe David Manana <fdmanana@gmail.com> wrote:> On Wed, Oct 23, 2013 at 2:33 PM, Alex Lyakas > <alex.btrfs@zadarastorage.com> wrote: >> Hi Filipe, >> >> >> On Tue, Aug 20, 2013 at 2:52 AM, Filipe David Borba Manana >> <fdmanana@gmail.com> wrote: >>> >>> This issue is simple to reproduce and observe if kmemleak is enabled. >>> Two simple ways to reproduce it: >>> >>> ** 1 >>> >>> $ mkfs.btrfs -f /dev/loop0 >>> $ mount /dev/loop0 /mnt/btrfs >>> $ btrfs balance start /mnt/btrfs >>> $ umount /mnt/btrfsSo here it seems that the leak can only happen in case the block-group has a free-space inode. This is what the orphan item is added for. Yes, here kmemleak reports. But: if space_cache option is disabled (and nospace_cache) enabled, it seems that btrfs still creates the FREE_SPACE inodes, although they are empty because in cache_save_setup: inode = lookup_free_space_inode(root, block_group, path); if (IS_ERR(inode) && PTR_ERR(inode) != -ENOENT) { ret = PTR_ERR(inode); btrfs_release_path(path); goto out; } if (IS_ERR(inode)) { ... ret = create_free_space_inode(root, trans, block_group, path); and only later it actually sets BTRFS_DC_WRITTEN if space_cache option is disabled. Amazing! Although this is a different issue, do you know perhaps why these empty inodes are needed? Thanks! Alex.>>> >>> ** 2 >>> >>> $ mkfs.btrfs -f /dev/loop0 >>> $ mount /dev/loop0 /mnt/btrfs >>> $ touch /mnt/btrfs/foobar >>> $ rm -f /mnt/btrfs/foobar >>> $ umount /mnt/btrfs >> >> >> I tried the second repro script on kernel 3.8.13, and kmemleak does >> not report a leak (even if I force the kmemleak scan). I did not try >> the balance-repro script, though. Am I missing something? > > Maybe it''s not an issue on 3.8.13 and older releases. > This was on btrfs-next from August 19. > > thanks for testing > >> >> Thanks, >> Alex. >> >> >>> >>> >>> After a while, kmemleak reports the leak: >>> >>> $ cat /sys/kernel/debug/kmemleak >>> unreferenced object 0xffff880402b13e00 (size 128): >>> comm "btrfs", pid 19621, jiffies 4341648183 (age 70057.844s) >>> hex dump (first 32 bytes): >>> 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 ................ >>> 00 fc c6 b1 04 88 ff ff 04 00 04 00 ad 4e ad de .............N.. >>> backtrace: >>> [<ffffffff817275a6>] kmemleak_alloc+0x26/0x50 >>> [<ffffffff8117832b>] kmem_cache_alloc_trace+0xeb/0x1d0 >>> [<ffffffffa04db499>] btrfs_alloc_block_rsv+0x39/0x70 [btrfs] >>> [<ffffffffa04f8bad>] btrfs_orphan_add+0x13d/0x1b0 [btrfs] >>> [<ffffffffa04e2b13>] btrfs_remove_block_group+0x143/0x500 [btrfs] >>> [<ffffffffa0518158>] btrfs_relocate_chunk.isra.63+0x618/0x790 [btrfs] >>> [<ffffffffa051bc27>] btrfs_balance+0x8f7/0xe90 [btrfs] >>> [<ffffffffa05240a0>] btrfs_ioctl_balance+0x250/0x550 [btrfs] >>> [<ffffffffa05269ca>] btrfs_ioctl+0xdfa/0x25f0 [btrfs] >>> [<ffffffff8119c936>] do_vfs_ioctl+0x96/0x570 >>> [<ffffffff8119cea1>] SyS_ioctl+0x91/0xb0 >>> [<ffffffff81750242>] system_call_fastpath+0x16/0x1b >>> [<ffffffffffffffff>] 0xffffffffffffffff >>> >>> This affects btrfs-next, revision be8e3cd00d7293dd177e3f8a4a1645ce09ca3acb >>> (Btrfs: separate out tests into their own directory). >>> >>> Signed-off-by: Filipe David Borba Manana <fdmanana@gmail.com> >>> --- >>> >>> V2: removed atomic_t member in struct btrfs_block_rsv, as suggested by >>> Josef Bacik, and use instead the condition reserved == 0 to decide >>> when to free the block. >>> V3: simplified patch, just kfree() (and not btrfs_free_block_rsv) the >>> root''s orphan_block_rsv when free''ing the root. Thanks Josef for >>> the suggestion. >>> V4: use btrfs_free_block_rsv() instead of kfree(). The error I was getting >>> in xfstests when using btrfs_free_block_rsv() was unrelated, Josef just >>> pointed it to me (separate issue). >>> V5: move the free call below the iput() call, so that btrfs_evict_node() >>> can process the orphan_block_rsv first to do some needed cleanup before >>> we free it. >>> V6: free the root''s orphan_block_rsv in close_ctree() too. After a balance >>> the orphan_block_rsv of the tree of tree roots was being leaked, because >>> free_fs_root() is only called for filesystem trees. >>> >>> fs/btrfs/disk-io.c | 5 +++++ >>> 1 file changed, 5 insertions(+) >>> >>> diff --git a/fs/btrfs/disk-io.c b/fs/btrfs/disk-io.c >>> index 3b12c26..5d17163 100644 >>> --- a/fs/btrfs/disk-io.c >>> +++ b/fs/btrfs/disk-io.c >>> @@ -3430,6 +3430,8 @@ static void free_fs_root(struct btrfs_root *root) >>> { >>> iput(root->cache_inode); >>> WARN_ON(!RB_EMPTY_ROOT(&root->inode_tree)); >>> + btrfs_free_block_rsv(root, root->orphan_block_rsv); >>> + root->orphan_block_rsv = NULL; >>> if (root->anon_dev) >>> free_anon_bdev(root->anon_dev); >>> free_extent_buffer(root->node); >>> @@ -3582,6 +3584,9 @@ int close_ctree(struct btrfs_root *root) >>> >>> btrfs_free_stripe_hash_table(fs_info); >>> >>> + btrfs_free_block_rsv(root, root->orphan_block_rsv); >>> + root->orphan_block_rsv = NULL; >>> + >>> return 0; >>> } >>> >>> -- >>> 1.7.9.5 >>> >>> -- >>> 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 > > > > -- > Filipe David Manana, > > "Reasonable men adapt themselves to the world. > Unreasonable men adapt the world to themselves. > That''s why all progress depends on unreasonable men."-- 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
Filipe David Manana
2013-Oct-23 14:26 UTC
Re: [PATCH v6] Btrfs: fix memory leak of orphan block rsv
On Wed, Oct 23, 2013 at 3:14 PM, Alex Lyakas <alex.btrfs@zadarastorage.com> wrote:> Hello, > > On Wed, Oct 23, 2013 at 4:35 PM, Filipe David Manana <fdmanana@gmail.com> wrote: >> On Wed, Oct 23, 2013 at 2:33 PM, Alex Lyakas >> <alex.btrfs@zadarastorage.com> wrote: >>> Hi Filipe, >>> >>> >>> On Tue, Aug 20, 2013 at 2:52 AM, Filipe David Borba Manana >>> <fdmanana@gmail.com> wrote: >>>> >>>> This issue is simple to reproduce and observe if kmemleak is enabled. >>>> Two simple ways to reproduce it: >>>> >>>> ** 1 >>>> >>>> $ mkfs.btrfs -f /dev/loop0 >>>> $ mount /dev/loop0 /mnt/btrfs >>>> $ btrfs balance start /mnt/btrfs >>>> $ umount /mnt/btrfs > > So here it seems that the leak can only happen in case the block-group > has a free-space inode. This is what the orphan item is added for. > Yes, here kmemleak reports. > But: if space_cache option is disabled (and nospace_cache) enabled, it > seems that btrfs still creates the FREE_SPACE inodes, although they > are empty because in cache_save_setup: > > inode = lookup_free_space_inode(root, block_group, path); > if (IS_ERR(inode) && PTR_ERR(inode) != -ENOENT) { > ret = PTR_ERR(inode); > btrfs_release_path(path); > goto out; > } > > if (IS_ERR(inode)) { > ... > ret = create_free_space_inode(root, trans, block_group, path); > > and only later it actually sets BTRFS_DC_WRITTEN if space_cache option > is disabled. Amazing! > Although this is a different issue, do you know perhaps why these > empty inodes are needed?Don''t know if they are needed. But you have a point, it seems odd to create the free space cache inode if mount option nospace_cache was supplied. Thanks Alex. Testing the following patch: diff --git a/fs/btrfs/extent-tree.c b/fs/btrfs/extent-tree.c index c43ee8a..eb1b7da 100644 --- a/fs/btrfs/extent-tree.c +++ b/fs/btrfs/extent-tree.c @@ -3162,6 +3162,9 @@ static int cache_save_setup(struct btrfs_block_group_cache *block_group, int retries = 0; int ret = 0; + if (!btrfs_test_opt(root, SPACE_CACHE)) + return 0; + /* * If this block group is smaller than 100 megs don''t bother caching the * block group.> > Thanks! > Alex. > > > >>>> >>>> ** 2 >>>> >>>> $ mkfs.btrfs -f /dev/loop0 >>>> $ mount /dev/loop0 /mnt/btrfs >>>> $ touch /mnt/btrfs/foobar >>>> $ rm -f /mnt/btrfs/foobar >>>> $ umount /mnt/btrfs >>> >>> >>> I tried the second repro script on kernel 3.8.13, and kmemleak does >>> not report a leak (even if I force the kmemleak scan). I did not try >>> the balance-repro script, though. Am I missing something? >> >> Maybe it''s not an issue on 3.8.13 and older releases. >> This was on btrfs-next from August 19. >> >> thanks for testing >> >>> >>> Thanks, >>> Alex. >>> >>> >>>> >>>> >>>> After a while, kmemleak reports the leak: >>>> >>>> $ cat /sys/kernel/debug/kmemleak >>>> unreferenced object 0xffff880402b13e00 (size 128): >>>> comm "btrfs", pid 19621, jiffies 4341648183 (age 70057.844s) >>>> hex dump (first 32 bytes): >>>> 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 ................ >>>> 00 fc c6 b1 04 88 ff ff 04 00 04 00 ad 4e ad de .............N.. >>>> backtrace: >>>> [<ffffffff817275a6>] kmemleak_alloc+0x26/0x50 >>>> [<ffffffff8117832b>] kmem_cache_alloc_trace+0xeb/0x1d0 >>>> [<ffffffffa04db499>] btrfs_alloc_block_rsv+0x39/0x70 [btrfs] >>>> [<ffffffffa04f8bad>] btrfs_orphan_add+0x13d/0x1b0 [btrfs] >>>> [<ffffffffa04e2b13>] btrfs_remove_block_group+0x143/0x500 [btrfs] >>>> [<ffffffffa0518158>] btrfs_relocate_chunk.isra.63+0x618/0x790 [btrfs] >>>> [<ffffffffa051bc27>] btrfs_balance+0x8f7/0xe90 [btrfs] >>>> [<ffffffffa05240a0>] btrfs_ioctl_balance+0x250/0x550 [btrfs] >>>> [<ffffffffa05269ca>] btrfs_ioctl+0xdfa/0x25f0 [btrfs] >>>> [<ffffffff8119c936>] do_vfs_ioctl+0x96/0x570 >>>> [<ffffffff8119cea1>] SyS_ioctl+0x91/0xb0 >>>> [<ffffffff81750242>] system_call_fastpath+0x16/0x1b >>>> [<ffffffffffffffff>] 0xffffffffffffffff >>>> >>>> This affects btrfs-next, revision be8e3cd00d7293dd177e3f8a4a1645ce09ca3acb >>>> (Btrfs: separate out tests into their own directory). >>>> >>>> Signed-off-by: Filipe David Borba Manana <fdmanana@gmail.com> >>>> --- >>>> >>>> V2: removed atomic_t member in struct btrfs_block_rsv, as suggested by >>>> Josef Bacik, and use instead the condition reserved == 0 to decide >>>> when to free the block. >>>> V3: simplified patch, just kfree() (and not btrfs_free_block_rsv) the >>>> root''s orphan_block_rsv when free''ing the root. Thanks Josef for >>>> the suggestion. >>>> V4: use btrfs_free_block_rsv() instead of kfree(). The error I was getting >>>> in xfstests when using btrfs_free_block_rsv() was unrelated, Josef just >>>> pointed it to me (separate issue). >>>> V5: move the free call below the iput() call, so that btrfs_evict_node() >>>> can process the orphan_block_rsv first to do some needed cleanup before >>>> we free it. >>>> V6: free the root''s orphan_block_rsv in close_ctree() too. After a balance >>>> the orphan_block_rsv of the tree of tree roots was being leaked, because >>>> free_fs_root() is only called for filesystem trees. >>>> >>>> fs/btrfs/disk-io.c | 5 +++++ >>>> 1 file changed, 5 insertions(+) >>>> >>>> diff --git a/fs/btrfs/disk-io.c b/fs/btrfs/disk-io.c >>>> index 3b12c26..5d17163 100644 >>>> --- a/fs/btrfs/disk-io.c >>>> +++ b/fs/btrfs/disk-io.c >>>> @@ -3430,6 +3430,8 @@ static void free_fs_root(struct btrfs_root *root) >>>> { >>>> iput(root->cache_inode); >>>> WARN_ON(!RB_EMPTY_ROOT(&root->inode_tree)); >>>> + btrfs_free_block_rsv(root, root->orphan_block_rsv); >>>> + root->orphan_block_rsv = NULL; >>>> if (root->anon_dev) >>>> free_anon_bdev(root->anon_dev); >>>> free_extent_buffer(root->node); >>>> @@ -3582,6 +3584,9 @@ int close_ctree(struct btrfs_root *root) >>>> >>>> btrfs_free_stripe_hash_table(fs_info); >>>> >>>> + btrfs_free_block_rsv(root, root->orphan_block_rsv); >>>> + root->orphan_block_rsv = NULL; >>>> + >>>> return 0; >>>> } >>>> >>>> -- >>>> 1.7.9.5 >>>> >>>> -- >>>> 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 >> >> >> >> -- >> Filipe David Manana, >> >> "Reasonable men adapt themselves to the world. >> Unreasonable men adapt the world to themselves. >> That''s why all progress depends on unreasonable men."-- Filipe David Manana, "Reasonable men adapt themselves to the world. Unreasonable men adapt the world to themselves. That''s why all progress depends on unreasonable men." -- 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
Alex Lyakas
2013-Nov-04 12:16 UTC
Re: [PATCH v6] Btrfs: fix memory leak of orphan block rsv
Hi Filipe, any luck with this patch?:) Alex. On Wed, Oct 23, 2013 at 5:26 PM, Filipe David Manana <fdmanana@gmail.com> wrote:> On Wed, Oct 23, 2013 at 3:14 PM, Alex Lyakas > <alex.btrfs@zadarastorage.com> wrote: >> Hello, >> >> On Wed, Oct 23, 2013 at 4:35 PM, Filipe David Manana <fdmanana@gmail.com> wrote: >>> On Wed, Oct 23, 2013 at 2:33 PM, Alex Lyakas >>> <alex.btrfs@zadarastorage.com> wrote: >>>> Hi Filipe, >>>> >>>> >>>> On Tue, Aug 20, 2013 at 2:52 AM, Filipe David Borba Manana >>>> <fdmanana@gmail.com> wrote: >>>>> >>>>> This issue is simple to reproduce and observe if kmemleak is enabled. >>>>> Two simple ways to reproduce it: >>>>> >>>>> ** 1 >>>>> >>>>> $ mkfs.btrfs -f /dev/loop0 >>>>> $ mount /dev/loop0 /mnt/btrfs >>>>> $ btrfs balance start /mnt/btrfs >>>>> $ umount /mnt/btrfs >> >> So here it seems that the leak can only happen in case the block-group >> has a free-space inode. This is what the orphan item is added for. >> Yes, here kmemleak reports. >> But: if space_cache option is disabled (and nospace_cache) enabled, it >> seems that btrfs still creates the FREE_SPACE inodes, although they >> are empty because in cache_save_setup: >> >> inode = lookup_free_space_inode(root, block_group, path); >> if (IS_ERR(inode) && PTR_ERR(inode) != -ENOENT) { >> ret = PTR_ERR(inode); >> btrfs_release_path(path); >> goto out; >> } >> >> if (IS_ERR(inode)) { >> ... >> ret = create_free_space_inode(root, trans, block_group, path); >> >> and only later it actually sets BTRFS_DC_WRITTEN if space_cache option >> is disabled. Amazing! >> Although this is a different issue, do you know perhaps why these >> empty inodes are needed? > > Don''t know if they are needed. But you have a point, it seems odd to > create the free space cache inode if mount option nospace_cache was > supplied. Thanks Alex. Testing the following patch: > > diff --git a/fs/btrfs/extent-tree.c b/fs/btrfs/extent-tree.c > index c43ee8a..eb1b7da 100644 > --- a/fs/btrfs/extent-tree.c > +++ b/fs/btrfs/extent-tree.c > @@ -3162,6 +3162,9 @@ static int cache_save_setup(struct > btrfs_block_group_cache *block_group, > int retries = 0; > int ret = 0; > > + if (!btrfs_test_opt(root, SPACE_CACHE)) > + return 0; > + > /* > * If this block group is smaller than 100 megs don''t bother caching the > * block group. > > >> >> Thanks! >> Alex. >> >> >> >>>>> >>>>> ** 2 >>>>> >>>>> $ mkfs.btrfs -f /dev/loop0 >>>>> $ mount /dev/loop0 /mnt/btrfs >>>>> $ touch /mnt/btrfs/foobar >>>>> $ rm -f /mnt/btrfs/foobar >>>>> $ umount /mnt/btrfs >>>> >>>> >>>> I tried the second repro script on kernel 3.8.13, and kmemleak does >>>> not report a leak (even if I force the kmemleak scan). I did not try >>>> the balance-repro script, though. Am I missing something? >>> >>> Maybe it''s not an issue on 3.8.13 and older releases. >>> This was on btrfs-next from August 19. >>> >>> thanks for testing >>> >>>> >>>> Thanks, >>>> Alex. >>>> >>>> >>>>> >>>>> >>>>> After a while, kmemleak reports the leak: >>>>> >>>>> $ cat /sys/kernel/debug/kmemleak >>>>> unreferenced object 0xffff880402b13e00 (size 128): >>>>> comm "btrfs", pid 19621, jiffies 4341648183 (age 70057.844s) >>>>> hex dump (first 32 bytes): >>>>> 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 ................ >>>>> 00 fc c6 b1 04 88 ff ff 04 00 04 00 ad 4e ad de .............N.. >>>>> backtrace: >>>>> [<ffffffff817275a6>] kmemleak_alloc+0x26/0x50 >>>>> [<ffffffff8117832b>] kmem_cache_alloc_trace+0xeb/0x1d0 >>>>> [<ffffffffa04db499>] btrfs_alloc_block_rsv+0x39/0x70 [btrfs] >>>>> [<ffffffffa04f8bad>] btrfs_orphan_add+0x13d/0x1b0 [btrfs] >>>>> [<ffffffffa04e2b13>] btrfs_remove_block_group+0x143/0x500 [btrfs] >>>>> [<ffffffffa0518158>] btrfs_relocate_chunk.isra.63+0x618/0x790 [btrfs] >>>>> [<ffffffffa051bc27>] btrfs_balance+0x8f7/0xe90 [btrfs] >>>>> [<ffffffffa05240a0>] btrfs_ioctl_balance+0x250/0x550 [btrfs] >>>>> [<ffffffffa05269ca>] btrfs_ioctl+0xdfa/0x25f0 [btrfs] >>>>> [<ffffffff8119c936>] do_vfs_ioctl+0x96/0x570 >>>>> [<ffffffff8119cea1>] SyS_ioctl+0x91/0xb0 >>>>> [<ffffffff81750242>] system_call_fastpath+0x16/0x1b >>>>> [<ffffffffffffffff>] 0xffffffffffffffff >>>>> >>>>> This affects btrfs-next, revision be8e3cd00d7293dd177e3f8a4a1645ce09ca3acb >>>>> (Btrfs: separate out tests into their own directory). >>>>> >>>>> Signed-off-by: Filipe David Borba Manana <fdmanana@gmail.com> >>>>> --- >>>>> >>>>> V2: removed atomic_t member in struct btrfs_block_rsv, as suggested by >>>>> Josef Bacik, and use instead the condition reserved == 0 to decide >>>>> when to free the block. >>>>> V3: simplified patch, just kfree() (and not btrfs_free_block_rsv) the >>>>> root''s orphan_block_rsv when free''ing the root. Thanks Josef for >>>>> the suggestion. >>>>> V4: use btrfs_free_block_rsv() instead of kfree(). The error I was getting >>>>> in xfstests when using btrfs_free_block_rsv() was unrelated, Josef just >>>>> pointed it to me (separate issue). >>>>> V5: move the free call below the iput() call, so that btrfs_evict_node() >>>>> can process the orphan_block_rsv first to do some needed cleanup before >>>>> we free it. >>>>> V6: free the root''s orphan_block_rsv in close_ctree() too. After a balance >>>>> the orphan_block_rsv of the tree of tree roots was being leaked, because >>>>> free_fs_root() is only called for filesystem trees. >>>>> >>>>> fs/btrfs/disk-io.c | 5 +++++ >>>>> 1 file changed, 5 insertions(+) >>>>> >>>>> diff --git a/fs/btrfs/disk-io.c b/fs/btrfs/disk-io.c >>>>> index 3b12c26..5d17163 100644 >>>>> --- a/fs/btrfs/disk-io.c >>>>> +++ b/fs/btrfs/disk-io.c >>>>> @@ -3430,6 +3430,8 @@ static void free_fs_root(struct btrfs_root *root) >>>>> { >>>>> iput(root->cache_inode); >>>>> WARN_ON(!RB_EMPTY_ROOT(&root->inode_tree)); >>>>> + btrfs_free_block_rsv(root, root->orphan_block_rsv); >>>>> + root->orphan_block_rsv = NULL; >>>>> if (root->anon_dev) >>>>> free_anon_bdev(root->anon_dev); >>>>> free_extent_buffer(root->node); >>>>> @@ -3582,6 +3584,9 @@ int close_ctree(struct btrfs_root *root) >>>>> >>>>> btrfs_free_stripe_hash_table(fs_info); >>>>> >>>>> + btrfs_free_block_rsv(root, root->orphan_block_rsv); >>>>> + root->orphan_block_rsv = NULL; >>>>> + >>>>> return 0; >>>>> } >>>>> >>>>> -- >>>>> 1.7.9.5 >>>>> >>>>> -- >>>>> 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 >>> >>> >>> >>> -- >>> Filipe David Manana, >>> >>> "Reasonable men adapt themselves to the world. >>> Unreasonable men adapt the world to themselves. >>> That''s why all progress depends on unreasonable men." > > > > -- > Filipe David Manana, > > "Reasonable men adapt themselves to the world. > Unreasonable men adapt the world to themselves. > That''s why all progress depends on unreasonable men."-- 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
Filipe David Manana
2013-Nov-06 12:19 UTC
Re: [PATCH v6] Btrfs: fix memory leak of orphan block rsv
On Mon, Nov 4, 2013 at 12:16 PM, Alex Lyakas <alex.btrfs@zadarastorage.com> wrote:> Hi Filipe, > any luck with this patch?:)Hey Alex, I haven''t digged further, but I remember I couldn''t reproduce your issue (with latest btrfs-next of that day) of getting the free space inodes created even when mount option nospace_cache is given. What kernel were you using?> > Alex. > > On Wed, Oct 23, 2013 at 5:26 PM, Filipe David Manana <fdmanana@gmail.com> wrote: >> On Wed, Oct 23, 2013 at 3:14 PM, Alex Lyakas >> <alex.btrfs@zadarastorage.com> wrote: >>> Hello, >>> >>> On Wed, Oct 23, 2013 at 4:35 PM, Filipe David Manana <fdmanana@gmail.com> wrote: >>>> On Wed, Oct 23, 2013 at 2:33 PM, Alex Lyakas >>>> <alex.btrfs@zadarastorage.com> wrote: >>>>> Hi Filipe, >>>>> >>>>> >>>>> On Tue, Aug 20, 2013 at 2:52 AM, Filipe David Borba Manana >>>>> <fdmanana@gmail.com> wrote: >>>>>> >>>>>> This issue is simple to reproduce and observe if kmemleak is enabled. >>>>>> Two simple ways to reproduce it: >>>>>> >>>>>> ** 1 >>>>>> >>>>>> $ mkfs.btrfs -f /dev/loop0 >>>>>> $ mount /dev/loop0 /mnt/btrfs >>>>>> $ btrfs balance start /mnt/btrfs >>>>>> $ umount /mnt/btrfs >>> >>> So here it seems that the leak can only happen in case the block-group >>> has a free-space inode. This is what the orphan item is added for. >>> Yes, here kmemleak reports. >>> But: if space_cache option is disabled (and nospace_cache) enabled, it >>> seems that btrfs still creates the FREE_SPACE inodes, although they >>> are empty because in cache_save_setup: >>> >>> inode = lookup_free_space_inode(root, block_group, path); >>> if (IS_ERR(inode) && PTR_ERR(inode) != -ENOENT) { >>> ret = PTR_ERR(inode); >>> btrfs_release_path(path); >>> goto out; >>> } >>> >>> if (IS_ERR(inode)) { >>> ... >>> ret = create_free_space_inode(root, trans, block_group, path); >>> >>> and only later it actually sets BTRFS_DC_WRITTEN if space_cache option >>> is disabled. Amazing! >>> Although this is a different issue, do you know perhaps why these >>> empty inodes are needed? >> >> Don''t know if they are needed. But you have a point, it seems odd to >> create the free space cache inode if mount option nospace_cache was >> supplied. Thanks Alex. Testing the following patch: >> >> diff --git a/fs/btrfs/extent-tree.c b/fs/btrfs/extent-tree.c >> index c43ee8a..eb1b7da 100644 >> --- a/fs/btrfs/extent-tree.c >> +++ b/fs/btrfs/extent-tree.c >> @@ -3162,6 +3162,9 @@ static int cache_save_setup(struct >> btrfs_block_group_cache *block_group, >> int retries = 0; >> int ret = 0; >> >> + if (!btrfs_test_opt(root, SPACE_CACHE)) >> + return 0; >> + >> /* >> * If this block group is smaller than 100 megs don''t bother caching the >> * block group. >> >> >>> >>> Thanks! >>> Alex. >>> >>> >>> >>>>>> >>>>>> ** 2 >>>>>> >>>>>> $ mkfs.btrfs -f /dev/loop0 >>>>>> $ mount /dev/loop0 /mnt/btrfs >>>>>> $ touch /mnt/btrfs/foobar >>>>>> $ rm -f /mnt/btrfs/foobar >>>>>> $ umount /mnt/btrfs >>>>> >>>>> >>>>> I tried the second repro script on kernel 3.8.13, and kmemleak does >>>>> not report a leak (even if I force the kmemleak scan). I did not try >>>>> the balance-repro script, though. Am I missing something? >>>> >>>> Maybe it''s not an issue on 3.8.13 and older releases. >>>> This was on btrfs-next from August 19. >>>> >>>> thanks for testing >>>> >>>>> >>>>> Thanks, >>>>> Alex. >>>>> >>>>> >>>>>> >>>>>> >>>>>> After a while, kmemleak reports the leak: >>>>>> >>>>>> $ cat /sys/kernel/debug/kmemleak >>>>>> unreferenced object 0xffff880402b13e00 (size 128): >>>>>> comm "btrfs", pid 19621, jiffies 4341648183 (age 70057.844s) >>>>>> hex dump (first 32 bytes): >>>>>> 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 ................ >>>>>> 00 fc c6 b1 04 88 ff ff 04 00 04 00 ad 4e ad de .............N.. >>>>>> backtrace: >>>>>> [<ffffffff817275a6>] kmemleak_alloc+0x26/0x50 >>>>>> [<ffffffff8117832b>] kmem_cache_alloc_trace+0xeb/0x1d0 >>>>>> [<ffffffffa04db499>] btrfs_alloc_block_rsv+0x39/0x70 [btrfs] >>>>>> [<ffffffffa04f8bad>] btrfs_orphan_add+0x13d/0x1b0 [btrfs] >>>>>> [<ffffffffa04e2b13>] btrfs_remove_block_group+0x143/0x500 [btrfs] >>>>>> [<ffffffffa0518158>] btrfs_relocate_chunk.isra.63+0x618/0x790 [btrfs] >>>>>> [<ffffffffa051bc27>] btrfs_balance+0x8f7/0xe90 [btrfs] >>>>>> [<ffffffffa05240a0>] btrfs_ioctl_balance+0x250/0x550 [btrfs] >>>>>> [<ffffffffa05269ca>] btrfs_ioctl+0xdfa/0x25f0 [btrfs] >>>>>> [<ffffffff8119c936>] do_vfs_ioctl+0x96/0x570 >>>>>> [<ffffffff8119cea1>] SyS_ioctl+0x91/0xb0 >>>>>> [<ffffffff81750242>] system_call_fastpath+0x16/0x1b >>>>>> [<ffffffffffffffff>] 0xffffffffffffffff >>>>>> >>>>>> This affects btrfs-next, revision be8e3cd00d7293dd177e3f8a4a1645ce09ca3acb >>>>>> (Btrfs: separate out tests into their own directory). >>>>>> >>>>>> Signed-off-by: Filipe David Borba Manana <fdmanana@gmail.com> >>>>>> --- >>>>>> >>>>>> V2: removed atomic_t member in struct btrfs_block_rsv, as suggested by >>>>>> Josef Bacik, and use instead the condition reserved == 0 to decide >>>>>> when to free the block. >>>>>> V3: simplified patch, just kfree() (and not btrfs_free_block_rsv) the >>>>>> root''s orphan_block_rsv when free''ing the root. Thanks Josef for >>>>>> the suggestion. >>>>>> V4: use btrfs_free_block_rsv() instead of kfree(). The error I was getting >>>>>> in xfstests when using btrfs_free_block_rsv() was unrelated, Josef just >>>>>> pointed it to me (separate issue). >>>>>> V5: move the free call below the iput() call, so that btrfs_evict_node() >>>>>> can process the orphan_block_rsv first to do some needed cleanup before >>>>>> we free it. >>>>>> V6: free the root''s orphan_block_rsv in close_ctree() too. After a balance >>>>>> the orphan_block_rsv of the tree of tree roots was being leaked, because >>>>>> free_fs_root() is only called for filesystem trees. >>>>>> >>>>>> fs/btrfs/disk-io.c | 5 +++++ >>>>>> 1 file changed, 5 insertions(+) >>>>>> >>>>>> diff --git a/fs/btrfs/disk-io.c b/fs/btrfs/disk-io.c >>>>>> index 3b12c26..5d17163 100644 >>>>>> --- a/fs/btrfs/disk-io.c >>>>>> +++ b/fs/btrfs/disk-io.c >>>>>> @@ -3430,6 +3430,8 @@ static void free_fs_root(struct btrfs_root *root) >>>>>> { >>>>>> iput(root->cache_inode); >>>>>> WARN_ON(!RB_EMPTY_ROOT(&root->inode_tree)); >>>>>> + btrfs_free_block_rsv(root, root->orphan_block_rsv); >>>>>> + root->orphan_block_rsv = NULL; >>>>>> if (root->anon_dev) >>>>>> free_anon_bdev(root->anon_dev); >>>>>> free_extent_buffer(root->node); >>>>>> @@ -3582,6 +3584,9 @@ int close_ctree(struct btrfs_root *root) >>>>>> >>>>>> btrfs_free_stripe_hash_table(fs_info); >>>>>> >>>>>> + btrfs_free_block_rsv(root, root->orphan_block_rsv); >>>>>> + root->orphan_block_rsv = NULL; >>>>>> + >>>>>> return 0; >>>>>> } >>>>>> >>>>>> -- >>>>>> 1.7.9.5 >>>>>> >>>>>> -- >>>>>> 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 >>>> >>>> >>>> >>>> -- >>>> Filipe David Manana, >>>> >>>> "Reasonable men adapt themselves to the world. >>>> Unreasonable men adapt the world to themselves. >>>> That''s why all progress depends on unreasonable men." >> >> >> >> -- >> Filipe David Manana, >> >> "Reasonable men adapt themselves to the world. >> Unreasonable men adapt the world to themselves. >> That''s why all progress depends on unreasonable men."-- Filipe David Manana, "Reasonable men adapt themselves to the world. Unreasonable men adapt the world to themselves. That''s why all progress depends on unreasonable men." -- 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
Possibly Parallel Threads
- [PATCH] Btrfs: protect orphan block rsv with spin_lock
- [PATCH] Btrfs: fix sync fs to actually wait for all data to be persisted
- [PATCH] Btrfs: optimize key searches in btrfs_search_slot
- [PATCH] Btrfs: fix very slow inode eviction and fs unmount
- [PATCH 4/5] fstests/MAINTAINERS: add some specific reviewers