* [PATCH 1/4] Btrfs: MOD_LOG_KEY_REMOVE_WHILE_MOVING never change node's nritems
@ 2012-10-19 9:50 Liu Bo
2012-10-19 9:50 ` [PATCH 2/4] Btrfs: reorder tree mod log operations in deleting a pointer Liu Bo
` (2 more replies)
0 siblings, 3 replies; 6+ messages in thread
From: Liu Bo @ 2012-10-19 9:50 UTC (permalink / raw)
To: linux-btrfs; +Cc: list.btrfs
Key MOD_LOG_KEY_REMOVE_WHILE_MOVING means that we're doing memmove inside
an extent buffer node, and the node's number of items remains unchanged
(unless we are inserting a single pointer, but we have MOD_LOG_KEY_ADD for that).
So we don't need to increase node's number of items during rewinding,
otherwise we may get an node larger than leafsize and cause general protection
errors later.
Here is the details,
- If we do memory move for inserting a single pointer, we need to
add node's nritems by one, and we honor MOD_LOG_KEY_ADD for adding.
- If we do memory move for deleting a single pointer, we need to
decrease node's nritems by one, and we honor MOD_LOG_KEY_REMOVE for
deleting.
- If we do memory move for balance left/right, we need to decrease
node's nritems, and we honor MOD_LOG_KEY_REMOVE for balaning.
Signed-off-by: Liu Bo <bo.li.liu@oracle.com>
---
fs/btrfs/ctree.c | 4 ++--
1 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/fs/btrfs/ctree.c b/fs/btrfs/ctree.c
index b334362..2858c2f 100644
--- a/fs/btrfs/ctree.c
+++ b/fs/btrfs/ctree.c
@@ -1142,13 +1142,13 @@ __tree_mod_log_rewind(struct extent_buffer *eb, u64 time_seq,
switch (tm->op) {
case MOD_LOG_KEY_REMOVE_WHILE_FREEING:
BUG_ON(tm->slot < n);
- case MOD_LOG_KEY_REMOVE_WHILE_MOVING:
case MOD_LOG_KEY_REMOVE:
+ n++;
+ case MOD_LOG_KEY_REMOVE_WHILE_MOVING:
btrfs_set_node_key(eb, &tm->key, tm->slot);
btrfs_set_node_blockptr(eb, tm->slot, tm->blockptr);
btrfs_set_node_ptr_generation(eb, tm->slot,
tm->generation);
- n++;
break;
case MOD_LOG_KEY_REPLACE:
BUG_ON(tm->slot >= n);
--
1.7.7.6
^ permalink raw reply related [flat|nested] 6+ messages in thread* [PATCH 2/4] Btrfs: reorder tree mod log operations in deleting a pointer 2012-10-19 9:50 [PATCH 1/4] Btrfs: MOD_LOG_KEY_REMOVE_WHILE_MOVING never change node's nritems Liu Bo @ 2012-10-19 9:50 ` Liu Bo 2012-10-19 9:50 ` [PATCH 3/4] Btrfs: kill unnecessary arguments in del_ptr Liu Bo 2012-10-19 9:50 ` [PATCH 4/4] Btrfs: cleanup unused arguments Liu Bo 2 siblings, 0 replies; 6+ messages in thread From: Liu Bo @ 2012-10-19 9:50 UTC (permalink / raw) To: linux-btrfs; +Cc: list.btrfs Since we don't use MOD_LOG_KEY_REMOVE_WHILE_MOVING to add nritems during rewinding, we should insert a MOD_LOG_KEY_REMOVE operation first. Signed-off-by: Liu Bo <bo.li.liu@oracle.com> --- fs/btrfs/ctree.c | 10 ++++++---- 1 files changed, 6 insertions(+), 4 deletions(-) diff --git a/fs/btrfs/ctree.c b/fs/btrfs/ctree.c index 2858c2f..bbea4fb 100644 --- a/fs/btrfs/ctree.c +++ b/fs/btrfs/ctree.c @@ -4569,6 +4569,12 @@ static void del_ptr(struct btrfs_trans_handle *trans, struct btrfs_root *root, u32 nritems; int ret; + if (tree_mod_log && level) { + ret = tree_mod_log_insert_key(root->fs_info, parent, slot, + MOD_LOG_KEY_REMOVE); + BUG_ON(ret < 0); + } + nritems = btrfs_header_nritems(parent); if (slot != nritems - 1) { if (tree_mod_log && level) @@ -4579,10 +4585,6 @@ static void del_ptr(struct btrfs_trans_handle *trans, struct btrfs_root *root, btrfs_node_key_ptr_offset(slot + 1), sizeof(struct btrfs_key_ptr) * (nritems - slot - 1)); - } else if (tree_mod_log && level) { - ret = tree_mod_log_insert_key(root->fs_info, parent, slot, - MOD_LOG_KEY_REMOVE); - BUG_ON(ret < 0); } nritems--; -- 1.7.7.6 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH 3/4] Btrfs: kill unnecessary arguments in del_ptr 2012-10-19 9:50 [PATCH 1/4] Btrfs: MOD_LOG_KEY_REMOVE_WHILE_MOVING never change node's nritems Liu Bo 2012-10-19 9:50 ` [PATCH 2/4] Btrfs: reorder tree mod log operations in deleting a pointer Liu Bo @ 2012-10-19 9:50 ` Liu Bo 2012-10-19 9:50 ` [PATCH 4/4] Btrfs: cleanup unused arguments Liu Bo 2 siblings, 0 replies; 6+ messages in thread From: Liu Bo @ 2012-10-19 9:50 UTC (permalink / raw) To: linux-btrfs; +Cc: list.btrfs The argument 'tree_mod_log' is not necessary since all of callers enable it. Signed-off-by: Liu Bo <bo.li.liu@oracle.com> --- fs/btrfs/ctree.c | 16 +++++++--------- 1 files changed, 7 insertions(+), 9 deletions(-) diff --git a/fs/btrfs/ctree.c b/fs/btrfs/ctree.c index bbea4fb..235831f 100644 --- a/fs/btrfs/ctree.c +++ b/fs/btrfs/ctree.c @@ -38,8 +38,7 @@ static int balance_node_right(struct btrfs_trans_handle *trans, struct extent_buffer *dst_buf, struct extent_buffer *src_buf); static void del_ptr(struct btrfs_trans_handle *trans, struct btrfs_root *root, - struct btrfs_path *path, int level, int slot, - int tree_mod_log); + struct btrfs_path *path, int level, int slot); static void tree_mod_log_free_eb(struct btrfs_fs_info *fs_info, struct extent_buffer *eb); struct extent_buffer *read_old_tree_block(struct btrfs_root *root, u64 bytenr, @@ -1789,7 +1788,7 @@ static noinline int balance_level(struct btrfs_trans_handle *trans, if (btrfs_header_nritems(right) == 0) { clean_tree_block(trans, root, right); btrfs_tree_unlock(right); - del_ptr(trans, root, path, level + 1, pslot + 1, 1); + del_ptr(trans, root, path, level + 1, pslot + 1); root_sub_used(root, right->len); btrfs_free_tree_block(trans, root, right, 0, 1); free_extent_buffer_stale(right); @@ -1833,7 +1832,7 @@ static noinline int balance_level(struct btrfs_trans_handle *trans, if (btrfs_header_nritems(mid) == 0) { clean_tree_block(trans, root, mid); btrfs_tree_unlock(mid); - del_ptr(trans, root, path, level + 1, pslot, 1); + del_ptr(trans, root, path, level + 1, pslot); root_sub_used(root, mid->len); btrfs_free_tree_block(trans, root, mid, 0, 1); free_extent_buffer_stale(mid); @@ -4562,14 +4561,13 @@ int btrfs_insert_item(struct btrfs_trans_handle *trans, struct btrfs_root * empty a node. */ static void del_ptr(struct btrfs_trans_handle *trans, struct btrfs_root *root, - struct btrfs_path *path, int level, int slot, - int tree_mod_log) + struct btrfs_path *path, int level, int slot) { struct extent_buffer *parent = path->nodes[level]; u32 nritems; int ret; - if (tree_mod_log && level) { + if (level) { ret = tree_mod_log_insert_key(root->fs_info, parent, slot, MOD_LOG_KEY_REMOVE); BUG_ON(ret < 0); @@ -4577,7 +4575,7 @@ static void del_ptr(struct btrfs_trans_handle *trans, struct btrfs_root *root, nritems = btrfs_header_nritems(parent); if (slot != nritems - 1) { - if (tree_mod_log && level) + if (level) tree_mod_log_eb_move(root->fs_info, parent, slot, slot + 1, nritems - slot - 1); memmove_extent_buffer(parent, @@ -4618,7 +4616,7 @@ static noinline void btrfs_del_leaf(struct btrfs_trans_handle *trans, struct extent_buffer *leaf) { WARN_ON(btrfs_header_generation(leaf) != trans->transid); - del_ptr(trans, root, path, 1, path->slots[1], 1); + del_ptr(trans, root, path, 1, path->slots[1]); /* * btrfs_free_extent is expensive, we want to make sure we -- 1.7.7.6 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH 4/4] Btrfs: cleanup unused arguments 2012-10-19 9:50 [PATCH 1/4] Btrfs: MOD_LOG_KEY_REMOVE_WHILE_MOVING never change node's nritems Liu Bo 2012-10-19 9:50 ` [PATCH 2/4] Btrfs: reorder tree mod log operations in deleting a pointer Liu Bo 2012-10-19 9:50 ` [PATCH 3/4] Btrfs: kill unnecessary arguments in del_ptr Liu Bo @ 2012-10-19 9:50 ` Liu Bo 2012-10-19 12:14 ` Jan Schmidt 2 siblings, 1 reply; 6+ messages in thread From: Liu Bo @ 2012-10-19 9:50 UTC (permalink / raw) To: linux-btrfs; +Cc: list.btrfs 'disk_key' is not used at all. Signed-off-by: Liu Bo <bo.li.liu@oracle.com> --- fs/btrfs/ctree.c | 3 +-- 1 files changed, 1 insertions(+), 2 deletions(-) diff --git a/fs/btrfs/ctree.c b/fs/btrfs/ctree.c index 235831f..7f649d2 100644 --- a/fs/btrfs/ctree.c +++ b/fs/btrfs/ctree.c @@ -772,8 +772,7 @@ tree_mod_log_eb_move(struct btrfs_fs_info *fs_info, struct extent_buffer *dst, static noinline void tree_mod_log_set_node_key(struct btrfs_fs_info *fs_info, - struct extent_buffer *eb, - struct btrfs_disk_key *disk_key, int slot, int atomic) + struct extent_buffer *eb, int slot, int atomic) { int ret; -- 1.7.7.6 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH 4/4] Btrfs: cleanup unused arguments 2012-10-19 9:50 ` [PATCH 4/4] Btrfs: cleanup unused arguments Liu Bo @ 2012-10-19 12:14 ` Jan Schmidt 2012-10-19 12:44 ` Liu Bo 0 siblings, 1 reply; 6+ messages in thread From: Jan Schmidt @ 2012-10-19 12:14 UTC (permalink / raw) To: Liu Bo; +Cc: linux-btrfs Hi liubo, Patches 3 and 4 are looking good. I'm still trying to sort out on the other two, as I cannot reproduce your bug with your script, unfortunately. After applying all 4 patches, the result doesn't compile, here: fs/btrfs/ctree.c: In function 'balance_level': fs/btrfs/ctree.c:1799: warning: passing argument 3 of 'tree_mod_log_set_node_key' makes integer from pointer without a cast fs/btrfs/ctree.c:774: note: expected 'int' but argument is of type 'struct btrfs_disk_key *' fs/btrfs/ctree.c:1799: error: too many arguments to function 'tree_mod_log_set_node_key' fs/btrfs/ctree.c:1844: warning: passing argument 3 of 'tree_mod_log_set_node_key' makes integer from pointer without a cast fs/btrfs/ctree.c:774: note: expected 'int' but argument is of type 'struct btrfs_disk_key *' fs/btrfs/ctree.c:1844: error: too many arguments to function 'tree_mod_log_set_node_key' fs/btrfs/ctree.c: In function 'push_nodes_for_insert': fs/btrfs/ctree.c:1943: warning: passing argument 3 of 'tree_mod_log_set_node_key' makes integer from pointer without a cast fs/btrfs/ctree.c:774: note: expected 'int' but argument is of type 'struct btrfs_disk_key *' fs/btrfs/ctree.c:1943: error: too many arguments to function 'tree_mod_log_set_node_key' fs/btrfs/ctree.c:1996: warning: passing argument 3 of 'tree_mod_log_set_node_key' makes integer from pointer without a cast fs/btrfs/ctree.c:774: note: expected 'int' but argument is of type 'struct btrfs_disk_key *' fs/btrfs/ctree.c:1996: error: too many arguments to function 'tree_mod_log_set_node_key' fs/btrfs/ctree.c: In function 'fixup_low_keys': fs/btrfs/ctree.c:2880: warning: passing argument 3 of 'tree_mod_log_set_node_key' makes integer from pointer without a cast fs/btrfs/ctree.c:774: note: expected 'int' but argument is of type 'struct btrfs_disk_key *' fs/btrfs/ctree.c:2880: error: too many arguments to function 'tree_mod_log_set_node_key' CC [M] fs/btrfs/sysfs.o make[2]: *** [fs/btrfs/ctree.o] Error 1 make[2]: *** Waiting for unfinished jobs.... make[1]: *** [fs/btrfs] Error 2 make[1]: *** Waiting for unfinished jobs.... make: *** [fs] Error 2 -Jan ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 4/4] Btrfs: cleanup unused arguments 2012-10-19 12:14 ` Jan Schmidt @ 2012-10-19 12:44 ` Liu Bo 0 siblings, 0 replies; 6+ messages in thread From: Liu Bo @ 2012-10-19 12:44 UTC (permalink / raw) To: Jan Schmidt; +Cc: linux-btrfs On 10/19/2012 08:14 PM, Jan Schmidt wrote: > Hi liubo, > > Patches 3 and 4 are looking good. I'm still trying to sort out on the other two, > as I cannot reproduce your bug with your script, unfortunately. > > After applying all 4 patches, the result doesn't compile, here: > > fs/btrfs/ctree.c: In function 'balance_level': > fs/btrfs/ctree.c:1799: warning: passing argument 3 of > 'tree_mod_log_set_node_key' makes integer from pointer without a cast > fs/btrfs/ctree.c:774: note: expected 'int' but argument is of type 'struct > btrfs_disk_key *' > fs/btrfs/ctree.c:1799: error: too many arguments to function > 'tree_mod_log_set_node_key' > fs/btrfs/ctree.c:1844: warning: passing argument 3 of > 'tree_mod_log_set_node_key' makes integer from pointer without a cast > fs/btrfs/ctree.c:774: note: expected 'int' but argument is of type 'struct > btrfs_disk_key *' > fs/btrfs/ctree.c:1844: error: too many arguments to function > 'tree_mod_log_set_node_key' > fs/btrfs/ctree.c: In function 'push_nodes_for_insert': > fs/btrfs/ctree.c:1943: warning: passing argument 3 of > 'tree_mod_log_set_node_key' makes integer from pointer without a cast > fs/btrfs/ctree.c:774: note: expected 'int' but argument is of type 'struct > btrfs_disk_key *' > fs/btrfs/ctree.c:1943: error: too many arguments to function > 'tree_mod_log_set_node_key' > fs/btrfs/ctree.c:1996: warning: passing argument 3 of > 'tree_mod_log_set_node_key' makes integer from pointer without a cast > fs/btrfs/ctree.c:774: note: expected 'int' but argument is of type 'struct > btrfs_disk_key *' > fs/btrfs/ctree.c:1996: error: too many arguments to function > 'tree_mod_log_set_node_key' > fs/btrfs/ctree.c: In function 'fixup_low_keys': > fs/btrfs/ctree.c:2880: warning: passing argument 3 of > 'tree_mod_log_set_node_key' makes integer from pointer without a cast > fs/btrfs/ctree.c:774: note: expected 'int' but argument is of type 'struct > btrfs_disk_key *' > fs/btrfs/ctree.c:2880: error: too many arguments to function > 'tree_mod_log_set_node_key' > CC [M] fs/btrfs/sysfs.o > make[2]: *** [fs/btrfs/ctree.o] Error 1 > make[2]: *** Waiting for unfinished jobs.... > make[1]: *** [fs/btrfs] Error 2 > make[1]: *** Waiting for unfinished jobs.... > make: *** [fs] Error 2 > oh, I'm so sorry, I forgot to format new patches and sent the wrong version. Sorry for the trouble. thanks, liubo > -Jan > -- > 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
end of thread, other threads:[~2012-10-19 12:45 UTC | newest] Thread overview: 6+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2012-10-19 9:50 [PATCH 1/4] Btrfs: MOD_LOG_KEY_REMOVE_WHILE_MOVING never change node's nritems Liu Bo 2012-10-19 9:50 ` [PATCH 2/4] Btrfs: reorder tree mod log operations in deleting a pointer Liu Bo 2012-10-19 9:50 ` [PATCH 3/4] Btrfs: kill unnecessary arguments in del_ptr Liu Bo 2012-10-19 9:50 ` [PATCH 4/4] Btrfs: cleanup unused arguments Liu Bo 2012-10-19 12:14 ` Jan Schmidt 2012-10-19 12:44 ` Liu Bo
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).