All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] Btrfs: do not use extent commit root for sending
@ 2014-01-12 13:38 Wang Shilong
  2014-01-12 13:38 ` [PATCH 1/3] Btrfs: fix missing skinny metadata check in scrub_stripe() Wang Shilong
                   ` (2 more replies)
  0 siblings, 3 replies; 6+ messages in thread
From: Wang Shilong @ 2014-01-12 13:38 UTC (permalink / raw)
  To: linux-btrfs

From: Wang Shilong <wangsl.fnst@cn.fujitsu.com>

Now we have kicked off transaction from btrfs send, it is not safe
that we use extent commit root to search.

I happended to catch this problem when running sending and snapshot
in my desktop.

Signed-off-by: Wang Shilong <wangsl.fnst@cn.fujitsu.com>
---
 fs/btrfs/send.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/fs/btrfs/send.c b/fs/btrfs/send.c
index 591063d..e159df1 100644
--- a/fs/btrfs/send.c
+++ b/fs/btrfs/send.c
@@ -1225,7 +1225,7 @@ static int find_extent_clone(struct send_ctx *sctx,
 
 	extent_item_pos = logical - found_key.objectid;
 	ret = iterate_extent_inodes(sctx->send_root->fs_info,
-					found_key.objectid, extent_item_pos, 1,
+					found_key.objectid, extent_item_pos, 0,
 					__iterate_backrefs, backref_ctx);
 
 	if (ret < 0)
-- 
1.8.4


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [PATCH 1/3] Btrfs: fix missing skinny metadata check in scrub_stripe()
  2014-01-12 13:38 [PATCH] Btrfs: do not use extent commit root for sending Wang Shilong
@ 2014-01-12 13:38 ` Wang Shilong
  2014-01-12 13:38 ` [PATCH 2/3] Btrfs: fix to search previous metadata extent item since skinny metadata Wang Shilong
  2014-01-12 13:38 ` [PATCH 3/3] Btrfs: fix missing inline refs when walking backrefs Wang Shilong
  2 siblings, 0 replies; 6+ messages in thread
From: Wang Shilong @ 2014-01-12 13:38 UTC (permalink / raw)
  To: linux-btrfs

From: Wang Shilong <wangsl.fnst@cn.fujitsu.com>

Check if we support skinny metadata firstly and fix to use
right type to search.

Signed-off-by: Wang Shilong <wangsl.fnst@cn.fujitsu.com>
---
 fs/btrfs/scrub.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/fs/btrfs/scrub.c b/fs/btrfs/scrub.c
index 7806e2c..e0677e4 100644
--- a/fs/btrfs/scrub.c
+++ b/fs/btrfs/scrub.c
@@ -2373,8 +2373,11 @@ static noinline_for_stack int scrub_stripe(struct scrub_ctx *sctx,
 			scrub_blocked_if_needed(fs_info);
 		}
 
+		if (btrfs_fs_incompat(fs_info, SKINNY_METADATA))
+			key.type = BTRFS_METADATA_ITEM_KEY;
+		else
+			key.type = BTRFS_EXTENT_ITEM_KEY;
 		key.objectid = logical;
-		key.type = BTRFS_EXTENT_ITEM_KEY;
 		key.offset = (u64)-1;
 
 		ret = btrfs_search_slot(NULL, root, &key, path, 0, 0);
-- 
1.8.4


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [PATCH 2/3] Btrfs: fix to search previous metadata extent item since skinny metadata
  2014-01-12 13:38 [PATCH] Btrfs: do not use extent commit root for sending Wang Shilong
  2014-01-12 13:38 ` [PATCH 1/3] Btrfs: fix missing skinny metadata check in scrub_stripe() Wang Shilong
@ 2014-01-12 13:38 ` Wang Shilong
  2014-01-12 13:38 ` [PATCH 3/3] Btrfs: fix missing inline refs when walking backrefs Wang Shilong
  2 siblings, 0 replies; 6+ messages in thread
From: Wang Shilong @ 2014-01-12 13:38 UTC (permalink / raw)
  To: linux-btrfs

From: Wang Shilong <wangsl.fnst@cn.fujitsu.com>

There is a bug that using btrfs_previous_item() to search metadata extent item.
This is because in btrfs_previous_item(), we need type match, however, since
skinny metada was introduced by josef, we may mix this two types. So just
use btrfs_previous_item() is not working right.

To keep btrfs_previous_item() like normal tree search, i introduce another
function btrfs_previous_extent_item().

Signed-off-by: Wang Shilong <wangsl.fnst@cn.fujitsu.com>
---
 fs/btrfs/backref.c |  3 +--
 fs/btrfs/ctree.c   | 43 +++++++++++++++++++++++++++++++++++++++++++
 fs/btrfs/ctree.h   |  2 ++
 fs/btrfs/scrub.c   |  3 +--
 4 files changed, 47 insertions(+), 4 deletions(-)

diff --git a/fs/btrfs/backref.c b/fs/btrfs/backref.c
index 835b6c9..964679c 100644
--- a/fs/btrfs/backref.c
+++ b/fs/btrfs/backref.c
@@ -1303,8 +1303,7 @@ int extent_from_logical(struct btrfs_fs_info *fs_info, u64 logical,
 	ret = btrfs_search_slot(NULL, fs_info->extent_root, &key, path, 0, 0);
 	if (ret < 0)
 		return ret;
-	ret = btrfs_previous_item(fs_info->extent_root, path,
-					0, BTRFS_EXTENT_ITEM_KEY);
+	ret = btrfs_previous_extent_item(fs_info->extent_root, path, 0);
 	if (ret < 0)
 		return ret;
 
diff --git a/fs/btrfs/ctree.c b/fs/btrfs/ctree.c
index 9e9de68..30f5b11 100644
--- a/fs/btrfs/ctree.c
+++ b/fs/btrfs/ctree.c
@@ -5955,3 +5955,46 @@ int btrfs_previous_item(struct btrfs_root *root,
 	}
 	return 1;
 }
+
+/*
+ * search in extent tree to find a previous Metadata/Data extent item with
+ * min objecitd.
+ *
+ * returns 0 if something is found, 1 if nothing was found and < 0 on error
+ */
+int btrfs_previous_extent_item(struct btrfs_root *root,
+			struct btrfs_path *path, u64 min_objectid)
+{
+	struct btrfs_key found_key;
+	struct extent_buffer *leaf;
+	u32 nritems;
+	int ret;
+
+	while (1) {
+		if (path->slots[0] == 0) {
+			btrfs_set_path_blocking(path);
+			ret = btrfs_prev_leaf(root, path);
+			if (ret != 0)
+				return ret;
+		} else {
+			path->slots[0]--;
+		}
+		leaf = path->nodes[0];
+		nritems = btrfs_header_nritems(leaf);
+		if (nritems == 0)
+			return 1;
+		if (path->slots[0] == nritems)
+			path->slots[0]--;
+
+		btrfs_item_key_to_cpu(leaf, &found_key, path->slots[0]);
+		if (found_key.objectid < min_objectid)
+			break;
+		if (found_key.type == BTRFS_EXTENT_ITEM_KEY ||
+		    found_key.type == BTRFS_METADATA_ITEM_KEY)
+			return 0;
+		if (found_key.objectid == min_objectid &&
+		    found_key.type < BTRFS_EXTENT_ITEM_KEY)
+			break;
+	}
+	return 1;
+}
diff --git a/fs/btrfs/ctree.h b/fs/btrfs/ctree.h
index 3cebb4a..81f8190 100644
--- a/fs/btrfs/ctree.h
+++ b/fs/btrfs/ctree.h
@@ -3331,6 +3331,8 @@ int btrfs_comp_cpu_keys(struct btrfs_key *k1, struct btrfs_key *k2);
 int btrfs_previous_item(struct btrfs_root *root,
 			struct btrfs_path *path, u64 min_objectid,
 			int type);
+int btrfs_previous_extent_item(struct btrfs_root *root,
+			struct btrfs_path *path, u64 min_objectid);
 void btrfs_set_item_key_safe(struct btrfs_root *root, struct btrfs_path *path,
 			     struct btrfs_key *new_key);
 struct extent_buffer *btrfs_root_node(struct btrfs_root *root);
diff --git a/fs/btrfs/scrub.c b/fs/btrfs/scrub.c
index e0677e4..51c342b 100644
--- a/fs/btrfs/scrub.c
+++ b/fs/btrfs/scrub.c
@@ -2385,8 +2385,7 @@ static noinline_for_stack int scrub_stripe(struct scrub_ctx *sctx,
 			goto out;
 
 		if (ret > 0) {
-			ret = btrfs_previous_item(root, path, 0,
-						  BTRFS_EXTENT_ITEM_KEY);
+			ret = btrfs_previous_extent_item(root, path, 0);
 			if (ret < 0)
 				goto out;
 			if (ret > 0) {
-- 
1.8.4


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [PATCH 3/3] Btrfs: fix missing inline refs when walking backrefs
  2014-01-12 13:38 [PATCH] Btrfs: do not use extent commit root for sending Wang Shilong
  2014-01-12 13:38 ` [PATCH 1/3] Btrfs: fix missing skinny metadata check in scrub_stripe() Wang Shilong
  2014-01-12 13:38 ` [PATCH 2/3] Btrfs: fix to search previous metadata extent item since skinny metadata Wang Shilong
@ 2014-01-12 13:38 ` Wang Shilong
  2014-01-12 15:36   ` Filipe David Manana
  2 siblings, 1 reply; 6+ messages in thread
From: Wang Shilong @ 2014-01-12 13:38 UTC (permalink / raw)
  To: linux-btrfs

From: Wang Shilong <wangsl.fnst@cn.fujitsu.com>

If @slot=0, we may have an expected item in the previous leaf,
So we should handle that case, otherwise, we will miss inline refs
,fix it.

Signed-off-by: Wang Shilong <wangsl.fnst@cn.fujitsu.com>
---
 fs/btrfs/backref.c | 13 ++++++-------
 1 file changed, 6 insertions(+), 7 deletions(-)

diff --git a/fs/btrfs/backref.c b/fs/btrfs/backref.c
index 964679c..4ccd726 100644
--- a/fs/btrfs/backref.c
+++ b/fs/btrfs/backref.c
@@ -893,14 +893,13 @@ again:
 		spin_unlock(&delayed_refs->lock);
 	}
 
-	if (path->slots[0]) {
-		struct extent_buffer *leaf;
-		int slot;
+	ret = btrfs_previous_extent_item(fs_info->extent_root, path,
+					 key.objectid);
+	if (ret < 0)
+		goto out;
 
-		path->slots[0]--;
-		leaf = path->nodes[0];
-		slot = path->slots[0];
-		btrfs_item_key_to_cpu(leaf, &key, slot);
+	if (ret == 0) {
+		btrfs_item_key_to_cpu(path->nodes[0], &key, path->slots[0]);
 		if (key.objectid == bytenr &&
 		    (key.type == BTRFS_EXTENT_ITEM_KEY ||
 		     key.type == BTRFS_METADATA_ITEM_KEY)) {
-- 
1.8.4


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH 3/3] Btrfs: fix missing inline refs when walking backrefs
  2014-01-12 13:38 ` [PATCH 3/3] Btrfs: fix missing inline refs when walking backrefs Wang Shilong
@ 2014-01-12 15:36   ` Filipe David Manana
  2014-01-13  1:27     ` Wang Shilong
  0 siblings, 1 reply; 6+ messages in thread
From: Filipe David Manana @ 2014-01-12 15:36 UTC (permalink / raw)
  To: Wang Shilong; +Cc: linux-btrfs@vger.kernel.org

On Sun, Jan 12, 2014 at 1:38 PM, Wang Shilong <wangshilong1991@gmail.com> wrote:
> From: Wang Shilong <wangsl.fnst@cn.fujitsu.com>
>
> If @slot=0, we may have an expected item in the previous leaf,
> So we should handle that case, otherwise, we will miss inline refs
> ,fix it.

Hi Shilong.

How can this happen exactly?

So the search key, regardless of having an BTRFS_EXTENT_ITEM_KEY or  a
BTRFS_METADATA_ITEM_KEY type, always has an offset set to (u64)-1.
This means the btrfs_search_slot call will always return 1 (not found,
as expected) or an error.

And btrfs_search_slot, when it doesn't find a key and if that key
should be the first item in a leaf, it makes the path point to the
previous leaf with a "path->slots[0] ==
btrfs_header_nritems(path->nodes[0])" (see [1]), otherwise to some
value > 0. The only cases I can imagine getting "path->slots[0] == 0"
is if we hit the first, i.e. left most, leaf in the btree or if the
btree consists only of 1 leaf (no nodes) - in these cases there's
isn't a previous leaf.

Did I miss something here?

thanks

[1] - https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/fs/btrfs/ctree.c?id=refs/tags/v3.13-rc8#n2629

>
> Signed-off-by: Wang Shilong <wangsl.fnst@cn.fujitsu.com>
> ---
>  fs/btrfs/backref.c | 13 ++++++-------
>  1 file changed, 6 insertions(+), 7 deletions(-)
>
> diff --git a/fs/btrfs/backref.c b/fs/btrfs/backref.c
> index 964679c..4ccd726 100644
> --- a/fs/btrfs/backref.c
> +++ b/fs/btrfs/backref.c
> @@ -893,14 +893,13 @@ again:
>                 spin_unlock(&delayed_refs->lock);
>         }
>
> -       if (path->slots[0]) {
> -               struct extent_buffer *leaf;
> -               int slot;
> +       ret = btrfs_previous_extent_item(fs_info->extent_root, path,
> +                                        key.objectid);
> +       if (ret < 0)
> +               goto out;
>
> -               path->slots[0]--;
> -               leaf = path->nodes[0];
> -               slot = path->slots[0];
> -               btrfs_item_key_to_cpu(leaf, &key, slot);
> +       if (ret == 0) {
> +               btrfs_item_key_to_cpu(path->nodes[0], &key, path->slots[0]);
>                 if (key.objectid == bytenr &&
>                     (key.type == BTRFS_EXTENT_ITEM_KEY ||
>                      key.type == BTRFS_METADATA_ITEM_KEY)) {
> --
> 1.8.4
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html



-- 
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."

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH 3/3] Btrfs: fix missing inline refs when walking backrefs
  2014-01-12 15:36   ` Filipe David Manana
@ 2014-01-13  1:27     ` Wang Shilong
  0 siblings, 0 replies; 6+ messages in thread
From: Wang Shilong @ 2014-01-13  1:27 UTC (permalink / raw)
  To: fdmanana; +Cc: Wang Shilong, linux-btrfs@vger.kernel.org

Hi Filipe,

On 01/12/2014 11:36 PM, Filipe David Manana wrote:
> On Sun, Jan 12, 2014 at 1:38 PM, Wang Shilong <wangshilong1991@gmail.com> wrote:
>> From: Wang Shilong <wangsl.fnst@cn.fujitsu.com>
>>
>> If @slot=0, we may have an expected item in the previous leaf,
>> So we should handle that case, otherwise, we will miss inline refs
>> ,fix it.
> Hi Shilong.
>
> How can this happen exactly?
>
> So the search key, regardless of having an BTRFS_EXTENT_ITEM_KEY or  a
> BTRFS_METADATA_ITEM_KEY type, always has an offset set to (u64)-1.
> This means the btrfs_search_slot call will always return 1 (not found,
> as expected) or an error.
>
> And btrfs_search_slot, when it doesn't find a key and if that key
> should be the first item in a leaf, it makes the path point to the
> previous leaf with a "path->slots[0] ==
> btrfs_header_nritems(path->nodes[0])" (see [1]), otherwise to some
> value > 0. The only cases I can imagine getting "path->slots[0] == 0"
> is if we hit the first, i.e. left most, leaf in the btree or if the
> btree consists only of 1 leaf (no nodes) - in these cases there's
> isn't a previous leaf.
>
> Did I miss something here?
>
> thanks
>
> [1] - https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/fs/btrfs/ctree.c?id=refs/tags/v3.13-rc8#n2629
You are right, i was missing something when i made this patch.
Really thanks for you reviewing and correcting me!:-)

Thanks,
Wang
>
>> Signed-off-by: Wang Shilong <wangsl.fnst@cn.fujitsu.com>
>> ---
>>   fs/btrfs/backref.c | 13 ++++++-------
>>   1 file changed, 6 insertions(+), 7 deletions(-)
>>
>> diff --git a/fs/btrfs/backref.c b/fs/btrfs/backref.c
>> index 964679c..4ccd726 100644
>> --- a/fs/btrfs/backref.c
>> +++ b/fs/btrfs/backref.c
>> @@ -893,14 +893,13 @@ again:
>>                  spin_unlock(&delayed_refs->lock);
>>          }
>>
>> -       if (path->slots[0]) {
>> -               struct extent_buffer *leaf;
>> -               int slot;
>> +       ret = btrfs_previous_extent_item(fs_info->extent_root, path,
>> +                                        key.objectid);
>> +       if (ret < 0)
>> +               goto out;
>>
>> -               path->slots[0]--;
>> -               leaf = path->nodes[0];
>> -               slot = path->slots[0];
>> -               btrfs_item_key_to_cpu(leaf, &key, slot);
>> +       if (ret == 0) {
>> +               btrfs_item_key_to_cpu(path->nodes[0], &key, path->slots[0]);
>>                  if (key.objectid == bytenr &&
>>                      (key.type == BTRFS_EXTENT_ITEM_KEY ||
>>                       key.type == BTRFS_METADATA_ITEM_KEY)) {
>> --
>> 1.8.4
>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
>> the body of a message to majordomo@vger.kernel.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>
>


^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2014-01-13  1:28 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2014-01-12 13:38 [PATCH] Btrfs: do not use extent commit root for sending Wang Shilong
2014-01-12 13:38 ` [PATCH 1/3] Btrfs: fix missing skinny metadata check in scrub_stripe() Wang Shilong
2014-01-12 13:38 ` [PATCH 2/3] Btrfs: fix to search previous metadata extent item since skinny metadata Wang Shilong
2014-01-12 13:38 ` [PATCH 3/3] Btrfs: fix missing inline refs when walking backrefs Wang Shilong
2014-01-12 15:36   ` Filipe David Manana
2014-01-13  1:27     ` Wang Shilong

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.