linux-btrfs.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [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).