* [PATCH 1/3] Btrfs: insert orphan roots into fs radix tree
@ 2013-09-25 13:47 Miao Xie
2013-09-25 13:47 ` [PATCH 2/3] Btrfs: fix oops caused by the space balance and dead roots Miao Xie
2013-09-25 13:47 ` [PATCH 3/3] Btrfs: fix BUG_ON() casued by the reserved space migration Miao Xie
0 siblings, 2 replies; 5+ messages in thread
From: Miao Xie @ 2013-09-25 13:47 UTC (permalink / raw)
To: linux-btrfs
Now we don't drop all the deleted snapshots/subvolumes before the space
balance. It means we have to relocate the space which is held by the dead
snapshots/subvolumes. So we must into them into fs radix tree, or we would
forget to commit the change of them when doing transaction commit, and it
would corrupt the metadata.
Signed-off-by: Miao Xie <miaox@cn.fujitsu.com>
---
fs/btrfs/root-tree.c | 8 +++-----
1 file changed, 3 insertions(+), 5 deletions(-)
diff --git a/fs/btrfs/root-tree.c b/fs/btrfs/root-tree.c
index 0b1f4ef..ec71ea4 100644
--- a/fs/btrfs/root-tree.c
+++ b/fs/btrfs/root-tree.c
@@ -299,11 +299,6 @@ int btrfs_find_orphan_roots(struct btrfs_root *tree_root)
continue;
}
- if (btrfs_root_refs(&root->root_item) == 0) {
- btrfs_add_dead_root(root);
- continue;
- }
-
err = btrfs_init_fs_root(root);
if (err) {
btrfs_free_fs_root(root);
@@ -318,6 +313,9 @@ int btrfs_find_orphan_roots(struct btrfs_root *tree_root)
btrfs_free_fs_root(root);
break;
}
+
+ if (btrfs_root_refs(&root->root_item) == 0)
+ btrfs_add_dead_root(root);
}
btrfs_free_path(path);
--
1.8.3.1
^ permalink raw reply related [flat|nested] 5+ messages in thread
* [PATCH 2/3] Btrfs: fix oops caused by the space balance and dead roots
2013-09-25 13:47 [PATCH 1/3] Btrfs: insert orphan roots into fs radix tree Miao Xie
@ 2013-09-25 13:47 ` Miao Xie
2013-09-25 14:11 ` Josef Bacik
2013-09-25 13:47 ` [PATCH 3/3] Btrfs: fix BUG_ON() casued by the reserved space migration Miao Xie
1 sibling, 1 reply; 5+ messages in thread
From: Miao Xie @ 2013-09-25 13:47 UTC (permalink / raw)
To: linux-btrfs
When doing space balance and subvolume destroy at the same time, we met
the following oops:
kernel BUG at fs/btrfs/relocation.c:2247!
RIP: 0010: [<ffffffffa04cec16>] prepare_to_merge+0x154/0x1f0 [btrfs]
Call Trace:
[<ffffffffa04b5ab7>] relocate_block_group+0x466/0x4e6 [btrfs]
[<ffffffffa04b5c7a>] btrfs_relocate_block_group+0x143/0x275 [btrfs]
[<ffffffffa0495c56>] btrfs_relocate_chunk.isra.27+0x5c/0x5a2 [btrfs]
[<ffffffffa0459871>] ? btrfs_item_key_to_cpu+0x15/0x31 [btrfs]
[<ffffffffa048b46a>] ? btrfs_get_token_64+0x7e/0xcd [btrfs]
[<ffffffffa04a3467>] ? btrfs_tree_read_unlock_blocking+0xb2/0xb7 [btrfs]
[<ffffffffa049907d>] btrfs_balance+0x9c7/0xb6f [btrfs]
[<ffffffffa049ef84>] btrfs_ioctl_balance+0x234/0x2ac [btrfs]
[<ffffffffa04a1e8e>] btrfs_ioctl+0xd87/0x1ef9 [btrfs]
[<ffffffff81122f53>] ? path_openat+0x234/0x4db
[<ffffffff813c3b78>] ? __do_page_fault+0x31d/0x391
[<ffffffff810f8ab6>] ? vma_link+0x74/0x94
[<ffffffff811250f5>] vfs_ioctl+0x1d/0x39
[<ffffffff811258c8>] do_vfs_ioctl+0x32d/0x3e2
[<ffffffff811259d4>] SyS_ioctl+0x57/0x83
[<ffffffff813c3bfa>] ? do_page_fault+0xe/0x10
[<ffffffff813c73c2>] system_call_fastpath+0x16/0x1b
It is because we returned the error number if the reference of the root was 0
when doing space relocation. It was not right here, because though the root
was dead(refs == 0), but the space it held still need be relocated, or we
could not remove the block group. So in this case, we should return the root
no matter it is dead or not.
Signed-off-by: Miao Xie <miaox@cn.fujitsu.com>
---
fs/btrfs/disk-io.c | 9 +++++----
fs/btrfs/disk-io.h | 13 +++++++++++--
fs/btrfs/relocation.c | 2 +-
3 files changed, 17 insertions(+), 7 deletions(-)
diff --git a/fs/btrfs/disk-io.c b/fs/btrfs/disk-io.c
index 4cbb00a..65eddf7 100644
--- a/fs/btrfs/disk-io.c
+++ b/fs/btrfs/disk-io.c
@@ -1560,8 +1560,9 @@ int btrfs_insert_fs_root(struct btrfs_fs_info *fs_info,
return ret;
}
-struct btrfs_root *btrfs_read_fs_root_no_name(struct btrfs_fs_info *fs_info,
- struct btrfs_key *location)
+struct btrfs_root *btrfs_get_fs_root(struct btrfs_fs_info *fs_info,
+ struct btrfs_key *location,
+ bool check_ref)
{
struct btrfs_root *root;
int ret;
@@ -1585,7 +1586,7 @@ struct btrfs_root *btrfs_read_fs_root_no_name(struct btrfs_fs_info *fs_info,
again:
root = btrfs_lookup_fs_root(fs_info, location->objectid);
if (root) {
- if (btrfs_root_refs(&root->root_item) == 0)
+ if (check_ref && btrfs_root_refs(&root->root_item) == 0)
return ERR_PTR(-ENOENT);
return root;
}
@@ -1594,7 +1595,7 @@ again:
if (IS_ERR(root))
return root;
- if (btrfs_root_refs(&root->root_item) == 0) {
+ if (check_ref && btrfs_root_refs(&root->root_item) == 0) {
ret = -ENOENT;
goto fail;
}
diff --git a/fs/btrfs/disk-io.h b/fs/btrfs/disk-io.h
index b71acd6e..5ce2a7d 100644
--- a/fs/btrfs/disk-io.h
+++ b/fs/btrfs/disk-io.h
@@ -68,8 +68,17 @@ struct btrfs_root *btrfs_read_fs_root(struct btrfs_root *tree_root,
int btrfs_init_fs_root(struct btrfs_root *root);
int btrfs_insert_fs_root(struct btrfs_fs_info *fs_info,
struct btrfs_root *root);
-struct btrfs_root *btrfs_read_fs_root_no_name(struct btrfs_fs_info *fs_info,
- struct btrfs_key *location);
+
+struct btrfs_root *btrfs_get_fs_root(struct btrfs_fs_info *fs_info,
+ struct btrfs_key *key,
+ bool check_ref);
+static inline struct btrfs_root *
+btrfs_read_fs_root_no_name(struct btrfs_fs_info *fs_info,
+ struct btrfs_key *location)
+{
+ return btrfs_get_fs_root(fs_info, location, true);
+}
+
int btrfs_cleanup_fs_roots(struct btrfs_fs_info *fs_info);
void btrfs_btree_balance_dirty(struct btrfs_root *root);
void btrfs_btree_balance_dirty_nodelay(struct btrfs_root *root);
diff --git a/fs/btrfs/relocation.c b/fs/btrfs/relocation.c
index aacc212..1ba09d1 100644
--- a/fs/btrfs/relocation.c
+++ b/fs/btrfs/relocation.c
@@ -588,7 +588,7 @@ static struct btrfs_root *read_fs_root(struct btrfs_fs_info *fs_info,
else
key.offset = (u64)-1;
- return btrfs_read_fs_root_no_name(fs_info, &key);
+ return btrfs_get_fs_root(fs_info, &key, false);
}
#ifdef BTRFS_COMPAT_EXTENT_TREE_V0
--
1.8.3.1
^ permalink raw reply related [flat|nested] 5+ messages in thread
* [PATCH 3/3] Btrfs: fix BUG_ON() casued by the reserved space migration
2013-09-25 13:47 [PATCH 1/3] Btrfs: insert orphan roots into fs radix tree Miao Xie
2013-09-25 13:47 ` [PATCH 2/3] Btrfs: fix oops caused by the space balance and dead roots Miao Xie
@ 2013-09-25 13:47 ` Miao Xie
1 sibling, 0 replies; 5+ messages in thread
From: Miao Xie @ 2013-09-25 13:47 UTC (permalink / raw)
To: linux-btrfs
When we did space balance and snapshot creation at the same time, we might
meet the following oops:
kernel BUG at fs/btrfs/inode.c:3038!
[SNIP]
Call Trace:
[<ffffffffa0411ec7>] btrfs_orphan_cleanup+0x293/0x407 [btrfs]
[<ffffffffa042dc45>] btrfs_mksubvol.isra.28+0x259/0x373 [btrfs]
[<ffffffffa042de85>] btrfs_ioctl_snap_create_transid+0x126/0x156 [btrfs]
[<ffffffffa042dff1>] btrfs_ioctl_snap_create_v2+0xd0/0x121 [btrfs]
[<ffffffffa0430b2c>] btrfs_ioctl+0x414/0x1854 [btrfs]
[<ffffffff813b60b7>] ? __do_page_fault+0x305/0x379
[<ffffffff811215a9>] vfs_ioctl+0x1d/0x39
[<ffffffff81121d7c>] do_vfs_ioctl+0x32d/0x3e2
[<ffffffff81057fe7>] ? finish_task_switch+0x80/0xb8
[<ffffffff81121e88>] SyS_ioctl+0x57/0x83
[<ffffffff813b39ff>] ? do_device_not_available+0x12/0x14
[<ffffffff813b99c2>] system_call_fastpath+0x16/0x1b
[SNIP]
RIP [<ffffffffa040da40>] btrfs_orphan_add+0xc3/0x126 [btrfs]
The reason of the problem is that the relocation root creation stole
the reserved space, which was reserved for orphan item deletion.
There are several ways to fix this problem, one is to increasing
the reserved space size of the space balace, and then we can use
that space to create the relocation tree for each fs/file trees.
But it is hard to calculate the suitable size because we doesn't
know how many fs/file trees we need relocate.
We fixed this problem by reserving the space for relocation root creation
actively since the space it need is very small (one tree block, used for
root node copy), then we use that reserved space to create the
relocation tree. If we don't reserve space for relocation tree creation,
we will use the reserved space of the balance.
Signed-off-by: Miao Xie <miaox@cn.fujitsu.com>
---
fs/btrfs/relocation.c | 6 ++++--
fs/btrfs/transaction.c | 24 +++++++++++++++++++++++-
fs/btrfs/transaction.h | 1 +
3 files changed, 28 insertions(+), 3 deletions(-)
diff --git a/fs/btrfs/relocation.c b/fs/btrfs/relocation.c
index 1ba09d1..0b5fc05 100644
--- a/fs/btrfs/relocation.c
+++ b/fs/btrfs/relocation.c
@@ -1383,6 +1383,7 @@ int btrfs_init_reloc_root(struct btrfs_trans_handle *trans,
{
struct btrfs_root *reloc_root;
struct reloc_control *rc = root->fs_info->reloc_ctl;
+ struct btrfs_block_rsv *rsv;
int clear_rsv = 0;
int ret;
@@ -1396,13 +1397,14 @@ int btrfs_init_reloc_root(struct btrfs_trans_handle *trans,
root->root_key.objectid == BTRFS_TREE_RELOC_OBJECTID)
return 0;
- if (!trans->block_rsv) {
+ if (!trans->reloc_reserved) {
+ rsv = trans->block_rsv;
trans->block_rsv = rc->block_rsv;
clear_rsv = 1;
}
reloc_root = create_reloc_root(trans, root, root->root_key.objectid);
if (clear_rsv)
- trans->block_rsv = NULL;
+ trans->block_rsv = rsv;
ret = __add_reloc_root(reloc_root);
BUG_ON(ret < 0);
diff --git a/fs/btrfs/transaction.c b/fs/btrfs/transaction.c
index cac4a3f..012e003 100644
--- a/fs/btrfs/transaction.c
+++ b/fs/btrfs/transaction.c
@@ -353,6 +353,17 @@ static int may_wait_transaction(struct btrfs_root *root, int type)
return 0;
}
+static inline bool need_reserve_reloc_root(struct btrfs_root *root)
+{
+ if (!root->fs_info->reloc_ctl ||
+ !root->ref_cows ||
+ root->root_key.objectid == BTRFS_TREE_RELOC_OBJECTID ||
+ root->reloc_root)
+ return false;
+
+ return true;
+}
+
static struct btrfs_trans_handle *
start_transaction(struct btrfs_root *root, u64 num_items, unsigned int type,
enum btrfs_reserve_flush_enum flush)
@@ -360,8 +371,9 @@ start_transaction(struct btrfs_root *root, u64 num_items, unsigned int type,
struct btrfs_trans_handle *h;
struct btrfs_transaction *cur_trans;
u64 num_bytes = 0;
- int ret;
u64 qgroup_reserved = 0;
+ bool reloc_reserved = false;
+ int ret;
if (test_bit(BTRFS_FS_STATE_ERROR, &root->fs_info->fs_state))
return ERR_PTR(-EROFS);
@@ -390,6 +402,14 @@ start_transaction(struct btrfs_root *root, u64 num_items, unsigned int type,
}
num_bytes = btrfs_calc_trans_metadata_size(root, num_items);
+ /*
+ * Do the reservation for the relocation root creation
+ */
+ if (unlikely(need_reserve_reloc_root(root))) {
+ num_bytes += root->nodesize;
+ reloc_reserved = true;
+ }
+
ret = btrfs_block_rsv_add(root,
&root->fs_info->trans_block_rsv,
num_bytes, flush);
@@ -451,6 +471,7 @@ again:
h->delayed_ref_elem.seq = 0;
h->type = type;
h->allocating_chunk = false;
+ h->reloc_reserved = false;
INIT_LIST_HEAD(&h->qgroup_ref_list);
INIT_LIST_HEAD(&h->new_bgs);
@@ -466,6 +487,7 @@ again:
h->transid, num_bytes, 1);
h->block_rsv = &root->fs_info->trans_block_rsv;
h->bytes_reserved = num_bytes;
+ h->reloc_reserved = reloc_reserved;
}
h->qgroup_reserved = qgroup_reserved;
diff --git a/fs/btrfs/transaction.h b/fs/btrfs/transaction.h
index 5c2af84..150e5dc 100644
--- a/fs/btrfs/transaction.h
+++ b/fs/btrfs/transaction.h
@@ -92,6 +92,7 @@ struct btrfs_trans_handle {
short aborted;
short adding_csums;
bool allocating_chunk;
+ bool reloc_reserved;
unsigned int type;
/*
* this root is only needed to validate that the root passed to
--
1.8.3.1
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH 2/3] Btrfs: fix oops caused by the space balance and dead roots
2013-09-25 13:47 ` [PATCH 2/3] Btrfs: fix oops caused by the space balance and dead roots Miao Xie
@ 2013-09-25 14:11 ` Josef Bacik
2013-09-26 9:29 ` Miao Xie
0 siblings, 1 reply; 5+ messages in thread
From: Josef Bacik @ 2013-09-25 14:11 UTC (permalink / raw)
To: Miao Xie; +Cc: linux-btrfs
On Wed, Sep 25, 2013 at 09:47:44PM +0800, Miao Xie wrote:
> When doing space balance and subvolume destroy at the same time, we met
> the following oops:
>
> kernel BUG at fs/btrfs/relocation.c:2247!
> RIP: 0010: [<ffffffffa04cec16>] prepare_to_merge+0x154/0x1f0 [btrfs]
> Call Trace:
> [<ffffffffa04b5ab7>] relocate_block_group+0x466/0x4e6 [btrfs]
> [<ffffffffa04b5c7a>] btrfs_relocate_block_group+0x143/0x275 [btrfs]
> [<ffffffffa0495c56>] btrfs_relocate_chunk.isra.27+0x5c/0x5a2 [btrfs]
> [<ffffffffa0459871>] ? btrfs_item_key_to_cpu+0x15/0x31 [btrfs]
> [<ffffffffa048b46a>] ? btrfs_get_token_64+0x7e/0xcd [btrfs]
> [<ffffffffa04a3467>] ? btrfs_tree_read_unlock_blocking+0xb2/0xb7 [btrfs]
> [<ffffffffa049907d>] btrfs_balance+0x9c7/0xb6f [btrfs]
> [<ffffffffa049ef84>] btrfs_ioctl_balance+0x234/0x2ac [btrfs]
> [<ffffffffa04a1e8e>] btrfs_ioctl+0xd87/0x1ef9 [btrfs]
> [<ffffffff81122f53>] ? path_openat+0x234/0x4db
> [<ffffffff813c3b78>] ? __do_page_fault+0x31d/0x391
> [<ffffffff810f8ab6>] ? vma_link+0x74/0x94
> [<ffffffff811250f5>] vfs_ioctl+0x1d/0x39
> [<ffffffff811258c8>] do_vfs_ioctl+0x32d/0x3e2
> [<ffffffff811259d4>] SyS_ioctl+0x57/0x83
> [<ffffffff813c3bfa>] ? do_page_fault+0xe/0x10
> [<ffffffff813c73c2>] system_call_fastpath+0x16/0x1b
>
Can you put this into an xfstest please so I can reproduce and verify your patch
fixes the problem? Thanks,
Josef
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH 2/3] Btrfs: fix oops caused by the space balance and dead roots
2013-09-25 14:11 ` Josef Bacik
@ 2013-09-26 9:29 ` Miao Xie
0 siblings, 0 replies; 5+ messages in thread
From: Miao Xie @ 2013-09-26 9:29 UTC (permalink / raw)
To: Josef Bacik; +Cc: linux-btrfs
On Wed, 25 Sep 2013 10:11:25 -0400, Josef Bacik wrote:
> On Wed, Sep 25, 2013 at 09:47:44PM +0800, Miao Xie wrote:
>> When doing space balance and subvolume destroy at the same time, we met
>> the following oops:
>>
>> kernel BUG at fs/btrfs/relocation.c:2247!
>> RIP: 0010: [<ffffffffa04cec16>] prepare_to_merge+0x154/0x1f0 [btrfs]
>> Call Trace:
>> [<ffffffffa04b5ab7>] relocate_block_group+0x466/0x4e6 [btrfs]
>> [<ffffffffa04b5c7a>] btrfs_relocate_block_group+0x143/0x275 [btrfs]
>> [<ffffffffa0495c56>] btrfs_relocate_chunk.isra.27+0x5c/0x5a2 [btrfs]
>> [<ffffffffa0459871>] ? btrfs_item_key_to_cpu+0x15/0x31 [btrfs]
>> [<ffffffffa048b46a>] ? btrfs_get_token_64+0x7e/0xcd [btrfs]
>> [<ffffffffa04a3467>] ? btrfs_tree_read_unlock_blocking+0xb2/0xb7 [btrfs]
>> [<ffffffffa049907d>] btrfs_balance+0x9c7/0xb6f [btrfs]
>> [<ffffffffa049ef84>] btrfs_ioctl_balance+0x234/0x2ac [btrfs]
>> [<ffffffffa04a1e8e>] btrfs_ioctl+0xd87/0x1ef9 [btrfs]
>> [<ffffffff81122f53>] ? path_openat+0x234/0x4db
>> [<ffffffff813c3b78>] ? __do_page_fault+0x31d/0x391
>> [<ffffffff810f8ab6>] ? vma_link+0x74/0x94
>> [<ffffffff811250f5>] vfs_ioctl+0x1d/0x39
>> [<ffffffff811258c8>] do_vfs_ioctl+0x32d/0x3e2
>> [<ffffffff811259d4>] SyS_ioctl+0x57/0x83
>> [<ffffffff813c3bfa>] ? do_page_fault+0xe/0x10
>> [<ffffffff813c73c2>] system_call_fastpath+0x16/0x1b
>>
>
> Can you put this into an xfstest please so I can reproduce and verify your patch
> fixes the problem? Thanks,
No problem, I will send out a test case soon.
Thanks
Miao
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2013-09-26 9:28 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-09-25 13:47 [PATCH 1/3] Btrfs: insert orphan roots into fs radix tree Miao Xie
2013-09-25 13:47 ` [PATCH 2/3] Btrfs: fix oops caused by the space balance and dead roots Miao Xie
2013-09-25 14:11 ` Josef Bacik
2013-09-26 9:29 ` Miao Xie
2013-09-25 13:47 ` [PATCH 3/3] Btrfs: fix BUG_ON() casued by the reserved space migration Miao Xie
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).