Re: [PATCH 4/4] btrfs-progs: print-tree: Use breadth-first search for btrfs_print_tree()
On 5.09.2018 09:29, Qu Wenruo wrote:
> btrfs_print_tree() uses depth-first search to print a subtree, it works
> fine until we have 3 level tree.
>
> In that case, leaves and nodes will be printed in a depth-first order,
> making it pretty hard to locate level 1 nodes.
>
> This patch will use breadth-first search for btrfs_print_tree().
> It will use btrfs_path::lowest_level to indicate current level, and
> print out tree blocks level by level (breadth-first).
>
> Signed-off-by: Qu Wenruo
Reviewed-by: Nikolay Borisov
> ---
> print-tree.c | 99 ++--
> 1 file changed, 73 insertions(+), 26 deletions(-)
>
> diff --git a/print-tree.c b/print-tree.c
> index 31f6fa12522f..0509ec3da46e 100644
> --- a/print-tree.c
> +++ b/print-tree.c
> @@ -1381,6 +1381,78 @@ void btrfs_print_leaf(struct extent_buffer *eb)
> }
> }
>
> +/* Helper function to reach the most left tree block at @path->lowest_level
> */
> +static int search_leftmost_tree_block(struct btrfs_fs_info *fs_info,
> + struct btrfs_path *path, int root_level)
> +{
> + int i;
> + int ret = 0;
> +
> + /* Release all nodes expect path->nodes[root_level] */
> + for (i = 0; i < root_level; i++) {
> + path->slots[i] = 0;
> + if (!path->nodes[i])
> + continue;
> + free_extent_buffer(path->nodes[i]);
> + }
> +
> + /* Reach the leftmost tree block by always reading out slot 0 */
> + for (i = root_level; i > path->lowest_level; i--) {
> + struct extent_buffer *eb;
> +
> + path->slots[i] = 0;
> + eb = read_node_slot(fs_info, path->nodes[i], 0);
> + if (!extent_buffer_uptodate(eb)) {
> + ret = -EIO;
> + goto out;
> + }
> + path->nodes[i - 1] = eb;
> + }
> +out:
> + return ret;
> +}
> +
> +static void bfs_print_children(struct extent_buffer *root_eb)
> +{
> + struct btrfs_fs_info *fs_info = root_eb->fs_info;
> + struct btrfs_path path;
> + int root_level = btrfs_header_level(root_eb);
> + int cur_level;
> + int ret;
> +
> + if (root_level < 1)
> + return;
> +
> + btrfs_init_path(&path);
> + /* For path */
> + extent_buffer_get(root_eb);
> + path.nodes[root_level] = root_eb;
> +
> + for (cur_level = root_level - 1; cur_level >= 0; cur_level--) {
> + path.lowest_level = cur_level;
> +
> + /* Use the leftmost tree block as start point */
> + ret = search_leftmost_tree_block(fs_info, &path, root_level);
So what you do here is really get the leftmost item at until level
'cur_level'.
> + if (ret < 0)
> + goto out;
> +
> + /* Print all sibling tree blocks */
> + while (1) {
> + btrfs_print_tree(path.nodes[cur_level], 0);
Then you print the block.
> + ret = btrfs_next_sibling_tree_block(fs_info, &path);
And this just loads the next block at level 'cur_level', representing
the "breadth" portion.
> + if (ret < 0)
> + goto out;
> + if (ret > 0) {
> + ret = 0;
> + break;
> + }
> + }
> + }
> +out:
> + btrfs_release_path(&path);
> + return;
> +}
> +
> void btrfs_print_tree(struct extent_buffer *eb, int follow)
> {
> u32 i;
> @@ -1389,7 +1461,6 @@ void btrfs_print_tree(struct extent_buffer *eb, int
> follow)
> struct btrfs_fs_info *fs_info = eb->fs_info;
> struct btrfs_disk_key disk_key;
> struct btrfs_key key;
> - struct extent_buffer *next;
>
> if (!eb)
> return;
> @@ -1431,30 +1502,6 @@ void btrfs_print_tree(struct extent_buffer *eb, int
> follow)
> if (follow && !fs_info)
> return;
>
> - for (i = 0; i < nr; i++) {
> - next = read_tree_block(fs_info,
> - btrfs_node_blockptr(eb, i),
> - btrfs_node_ptr_generation(eb, i));
> - if (!extent_buffer_uptodate(next)) {
> - fprintf(stderr, "failed to read %llu in tree %llu\n",
> - (unsigned long long)btrfs_node_blockptr(eb, i),
> - (unsigned long
[PATCH 4/4] btrfs-progs: print-tree: Use breadth-first search for btrfs_print_tree()
btrfs_print_tree() uses depth-first search to print a subtree, it works
fine until we have 3 level tree.
In that case, leaves and nodes will be printed in a depth-first order,
making it pretty hard to locate level 1 nodes.
This patch will use breadth-first search for btrfs_print_tree().
It will use btrfs_path::lowest_level to indicate current level, and
print out tree blocks level by level (breadth-first).
Signed-off-by: Qu Wenruo
---
print-tree.c | 99 ++--
1 file changed, 73 insertions(+), 26 deletions(-)
diff --git a/print-tree.c b/print-tree.c
index 31f6fa12522f..0509ec3da46e 100644
--- a/print-tree.c
+++ b/print-tree.c
@@ -1381,6 +1381,78 @@ void btrfs_print_leaf(struct extent_buffer *eb)
}
}
+/* Helper function to reach the most left tree block at @path->lowest_level */
+static int search_leftmost_tree_block(struct btrfs_fs_info *fs_info,
+ struct btrfs_path *path, int root_level)
+{
+ int i;
+ int ret = 0;
+
+ /* Release all nodes expect path->nodes[root_level] */
+ for (i = 0; i < root_level; i++) {
+ path->slots[i] = 0;
+ if (!path->nodes[i])
+ continue;
+ free_extent_buffer(path->nodes[i]);
+ }
+
+ /* Reach the leftmost tree block by always reading out slot 0 */
+ for (i = root_level; i > path->lowest_level; i--) {
+ struct extent_buffer *eb;
+
+ path->slots[i] = 0;
+ eb = read_node_slot(fs_info, path->nodes[i], 0);
+ if (!extent_buffer_uptodate(eb)) {
+ ret = -EIO;
+ goto out;
+ }
+ path->nodes[i - 1] = eb;
+ }
+out:
+ return ret;
+}
+
+static void bfs_print_children(struct extent_buffer *root_eb)
+{
+ struct btrfs_fs_info *fs_info = root_eb->fs_info;
+ struct btrfs_path path;
+ int root_level = btrfs_header_level(root_eb);
+ int cur_level;
+ int ret;
+
+ if (root_level < 1)
+ return;
+
+ btrfs_init_path(&path);
+ /* For path */
+ extent_buffer_get(root_eb);
+ path.nodes[root_level] = root_eb;
+
+ for (cur_level = root_level - 1; cur_level >= 0; cur_level--) {
+ path.lowest_level = cur_level;
+
+ /* Use the leftmost tree block as start point */
+ ret = search_leftmost_tree_block(fs_info, &path, root_level);
+ if (ret < 0)
+ goto out;
+
+ /* Print all sibling tree blocks */
+ while (1) {
+ btrfs_print_tree(path.nodes[cur_level], 0);
+ ret = btrfs_next_sibling_tree_block(fs_info, &path);
+ if (ret < 0)
+ goto out;
+ if (ret > 0) {
+ ret = 0;
+ break;
+ }
+ }
+ }
+out:
+ btrfs_release_path(&path);
+ return;
+}
+
void btrfs_print_tree(struct extent_buffer *eb, int follow)
{
u32 i;
@@ -1389,7 +1461,6 @@ void btrfs_print_tree(struct extent_buffer *eb, int
follow)
struct btrfs_fs_info *fs_info = eb->fs_info;
struct btrfs_disk_key disk_key;
struct btrfs_key key;
- struct extent_buffer *next;
if (!eb)
return;
@@ -1431,30 +1502,6 @@ void btrfs_print_tree(struct extent_buffer *eb, int
follow)
if (follow && !fs_info)
return;
- for (i = 0; i < nr; i++) {
- next = read_tree_block(fs_info,
- btrfs_node_blockptr(eb, i),
- btrfs_node_ptr_generation(eb, i));
- if (!extent_buffer_uptodate(next)) {
- fprintf(stderr, "failed to read %llu in tree %llu\n",
- (unsigned long long)btrfs_node_blockptr(eb, i),
- (unsigned long long)btrfs_header_owner(eb));
- continue;
- }
- if (btrfs_header_level(next) != btrfs_header_level(eb) - 1) {
- warning(
-"eb corrupted: parent bytenr %llu slot %d level %d child bytenr %llu level has
%d expect %d, skipping the slot",
- btrfs_header_bytenr(eb), i,
- btrfs_header_level(eb),
- btrfs_header_bytenr(next),
- btrfs_header_level(next),
- btrfs_header_level(eb) - 1);
- free_extent_buffer(next);
- continue;
- }
- btrfs_print_tree(next, 1);
- free_extent_buffer(next);
- }
-
+ bfs_print_children(eb);
return;
}
--
2.18.0
Re: [PATCH] btrfs-progs: print-tree: remove dead code from btrfs_print_tree
On Mon, Apr 09, 2018 at 02:40:44PM +0800, Lu Fengqi wrote:
> Since the out label has been deleted, this free_extent_buffer will never
> be executed.
>
> Fixes: f37ae8d275c2 ("btrfs-progs: print-tree: Enhance warning on tree block
> level mismatch and error handling")
> Signed-off-by: Lu Fengqi
Applied, thanks.
--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 3/3] btrfs-progs: print-tree: Enhance btrfs_print_tree() check to avoid out-of-boundary memory access
On Thu, May 10, 2018 at 09:50:01AM +0800, Qu Wenruo wrote: > For btrfs_print_tree(), if nr_items is corrupted, it can easily go > beyond extent buffer boundary. > > Add extra nr_item check, and only print as many valid slots as possible. > > Signed-off-by: Qu Wenruo Applied, thanks. -- To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 3/3] btrfs-progs: print-tree: Enhance btrfs_print_tree() check to avoid out-of-boundary memory access
For btrfs_print_tree(), if nr_items is corrupted, it can easily go
beyond extent buffer boundary.
Add extra nr_item check, and only print as many valid slots as possible.
Signed-off-by: Qu Wenruo
---
changelog:
v2:
Use better loop condition suggested by Su.
---
print-tree.c | 10 +-
1 file changed, 9 insertions(+), 1 deletion(-)
diff --git a/print-tree.c b/print-tree.c
index 31a851ef4413..1c2533a9678d 100644
--- a/print-tree.c
+++ b/print-tree.c
@@ -1364,6 +1364,7 @@ void btrfs_print_tree(struct extent_buffer *eb, int
follow)
{
u32 i;
u32 nr;
+ u32 ptr_num;
struct btrfs_fs_info *fs_info = eb->fs_info;
struct btrfs_disk_key disk_key;
struct btrfs_key key;
@@ -1376,6 +1377,11 @@ void btrfs_print_tree(struct extent_buffer *eb, int
follow)
btrfs_print_leaf(eb);
return;
}
+ /* We are crossing eb boundary, this node must be corrupted */
+ if (nr > BTRFS_NODEPTRS_PER_EXTENT_BUFFER(eb))
+ warning(
+ "node nr_items corrupted, has %u limit %u, continue print
anyway",
+ nr, BTRFS_NODEPTRS_PER_EXTENT_BUFFER(eb));
printf("node %llu level %d items %d free %u generation %llu owner ",
(unsigned long long)eb->start,
btrfs_header_level(eb), nr,
@@ -1385,8 +1391,10 @@ void btrfs_print_tree(struct extent_buffer *eb, int
follow)
printf("\n");
print_uuids(eb);
fflush(stdout);
- for (i = 0; i < nr; i++) {
+ ptr_num = BTRFS_NODEPTRS_PER_EXTENT_BUFFER(eb);
+ for (i = 0; i < nr && i < ptr_num; i++) {
u64 blocknr = btrfs_node_blockptr(eb, i);
+
btrfs_node_key(eb, &disk_key, i);
btrfs_disk_key_to_cpu(&key, &disk_key);
printf("\t");
--
2.17.0
--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 3/3] btrfs-progs: print-tree: Enhance btrfs_print_tree() check to avoid out-of-boundary memory access
On Mon, Apr 30, 2018 at 11:51:19AM +0800, Qu Wenruo wrote:
> >> btrfs_print_leaf(eb);
> >> return;
> >> }
> >> + /* We are crossing eb boundary, this node must be corrupted */
> >> + if (nr > BTRFS_NODEPTRS_PER_EXTENT_BUFFER(eb))
> >> + warning(
> >> + "node nr_items corrupted, has %u limit %u, continue print
> >> anyway",
> >> + nr, BTRFS_NODEPTRS_PER_EXTENT_BUFFER(eb));
> >> printf("node %llu level %d items %d free %u generation %llu
> >> owner ",
> >> (unsigned long long)eb->start,
> >> btrfs_header_level(eb), nr,
> >> @@ -1386,7 +1391,11 @@ void btrfs_print_tree(struct extent_buffer *eb,
> >> int follow)
> >> print_uuids(eb);
> >> fflush(stdout);
> >>
> >> - u64 blocknr = btrfs_node_blockptr(eb, i);
> >> + u64 blocknr;
> >> +
> >> + if (i > BTRFS_NODEPTRS_PER_EXTENT_BUFFER(eb))
> >> + break;
> >
> > Should it be i >= BTRFS_NODEPTRS_PER_EXTENT_BUFFER(eb)?
>
> BTRFS_NODEPTRS_PER_EXTENT_BUFFER() provides the maximum valid number.
> So it 's >=.
>
> >
> > Here BTRFS_NODEPTRS_PER_EXTENT_BUFFER() is called during iterations.
> > The judement can be calculated in advance like:
> >
> > ptr_num = BTRFS_NODEPTRS_PER_EXTENT_BUFFER(eb);
> > ...
> > for (i = 0; i < nr && i < ptr_num ; i++) {
>
> Indeed looks better.
Please resend this patch with the suggested updates, thanks.
--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 3/3] btrfs-progs: print-tree: Enhance btrfs_print_tree() check to avoid out-of-boundary memory access
On 2018年04月30日 11:49, Su Yue wrote:
>
>
> On 04/30/2018 11:15 AM, Qu Wenruo wrote:
>> For btrfs_print_tree(), if nr_items is corrupted, it can easily go
>> beyond extent buffer boundary.
>>
>> Add extra nr_item check, and only print as many valid slots as possible.
>>
>
> Make sense.
>
>> Signed-off-by: Qu Wenruo
>> ---
>> print-tree.c | 11 ++-
>> 1 file changed, 10 insertions(+), 1 deletion(-)
>>
>> diff --git a/print-tree.c b/print-tree.c
>> index 31a851ef4413..55db80bebb2a 100644
>> --- a/print-tree.c
>> +++ b/print-tree.c
>> @@ -1376,6 +1376,11 @@ void btrfs_print_tree(struct extent_buffer *eb,
>> int follow)
>> btrfs_print_leaf(eb);
>> return;
>> }
>> + /* We are crossing eb boundary, this node must be corrupted */
>> + if (nr > BTRFS_NODEPTRS_PER_EXTENT_BUFFER(eb))
>> + warning(
>> + "node nr_items corrupted, has %u limit %u, continue print
>> anyway",
>> + nr, BTRFS_NODEPTRS_PER_EXTENT_BUFFER(eb));
>> printf("node %llu level %d items %d free %u generation %llu
>> owner ",
>> (unsigned long long)eb->start,
>> btrfs_header_level(eb), nr,
>> @@ -1386,7 +1391,11 @@ void btrfs_print_tree(struct extent_buffer *eb,
>> int follow)
>> print_uuids(eb);
>> fflush(stdout);
>>
>> - u64 blocknr = btrfs_node_blockptr(eb, i);
>> + u64 blocknr;
>> +
>> + if (i > BTRFS_NODEPTRS_PER_EXTENT_BUFFER(eb))
>> + break;
>
> Should it be i >= BTRFS_NODEPTRS_PER_EXTENT_BUFFER(eb)?
BTRFS_NODEPTRS_PER_EXTENT_BUFFER() provides the maximum valid number.
So it 's >=.
>
> Here BTRFS_NODEPTRS_PER_EXTENT_BUFFER() is called during iterations.
> The judement can be calculated in advance like:
>
> ptr_num = BTRFS_NODEPTRS_PER_EXTENT_BUFFER(eb);
> ...
> for (i = 0; i < nr && i < ptr_num ; i++) {
Indeed looks better.
Thanks,
Qu
>
> Thanks,
> Su
>
>> + blocknr = btrfs_node_blockptr(eb, i);
>> btrfs_node_key(eb, &disk_key, i);
>> btrfs_disk_key_to_cpu(&key, &disk_key);
>> printf("\t");
>>
>
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
signature.asc
Description: OpenPGP digital signature
Re: [PATCH 3/3] btrfs-progs: print-tree: Enhance btrfs_print_tree() check to avoid out-of-boundary memory access
On 04/30/2018 11:15 AM, Qu Wenruo wrote:
For btrfs_print_tree(), if nr_items is corrupted, it can easily go
beyond extent buffer boundary.
Add extra nr_item check, and only print as many valid slots as possible.
Make sense.
Signed-off-by: Qu Wenruo
---
print-tree.c | 11 ++-
1 file changed, 10 insertions(+), 1 deletion(-)
diff --git a/print-tree.c b/print-tree.c
index 31a851ef4413..55db80bebb2a 100644
--- a/print-tree.c
+++ b/print-tree.c
@@ -1376,6 +1376,11 @@ void btrfs_print_tree(struct extent_buffer *eb, int
follow)
btrfs_print_leaf(eb);
return;
}
+ /* We are crossing eb boundary, this node must be corrupted */
+ if (nr > BTRFS_NODEPTRS_PER_EXTENT_BUFFER(eb))
+ warning(
+ "node nr_items corrupted, has %u limit %u, continue print
anyway",
+ nr, BTRFS_NODEPTRS_PER_EXTENT_BUFFER(eb));
printf("node %llu level %d items %d free %u generation %llu owner ",
(unsigned long long)eb->start,
btrfs_header_level(eb), nr,
@@ -1386,7 +1391,11 @@ void btrfs_print_tree(struct extent_buffer *eb, int
follow)
print_uuids(eb);
fflush(stdout);
- u64 blocknr = btrfs_node_blockptr(eb, i);
+ u64 blocknr;
+
+ if (i > BTRFS_NODEPTRS_PER_EXTENT_BUFFER(eb))
+ break;
Should it be i >= BTRFS_NODEPTRS_PER_EXTENT_BUFFER(eb)?
Here BTRFS_NODEPTRS_PER_EXTENT_BUFFER() is called during iterations.
The judement can be calculated in advance like:
ptr_num = BTRFS_NODEPTRS_PER_EXTENT_BUFFER(eb);
...
for (i = 0; i < nr && i < ptr_num ; i++) {
Thanks,
Su
+ blocknr = btrfs_node_blockptr(eb, i);
btrfs_node_key(eb, &disk_key, i);
btrfs_disk_key_to_cpu(&key, &disk_key);
printf("\t");
--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 3/3] btrfs-progs: print-tree: Enhance btrfs_print_tree() check to avoid out-of-boundary memory access
For btrfs_print_tree(), if nr_items is corrupted, it can easily go
beyond extent buffer boundary.
Add extra nr_item check, and only print as many valid slots as possible.
Signed-off-by: Qu Wenruo
---
print-tree.c | 11 ++-
1 file changed, 10 insertions(+), 1 deletion(-)
diff --git a/print-tree.c b/print-tree.c
index 31a851ef4413..55db80bebb2a 100644
--- a/print-tree.c
+++ b/print-tree.c
@@ -1376,6 +1376,11 @@ void btrfs_print_tree(struct extent_buffer *eb, int
follow)
btrfs_print_leaf(eb);
return;
}
+ /* We are crossing eb boundary, this node must be corrupted */
+ if (nr > BTRFS_NODEPTRS_PER_EXTENT_BUFFER(eb))
+ warning(
+ "node nr_items corrupted, has %u limit %u, continue print
anyway",
+ nr, BTRFS_NODEPTRS_PER_EXTENT_BUFFER(eb));
printf("node %llu level %d items %d free %u generation %llu owner ",
(unsigned long long)eb->start,
btrfs_header_level(eb), nr,
@@ -1386,7 +1391,11 @@ void btrfs_print_tree(struct extent_buffer *eb, int
follow)
print_uuids(eb);
fflush(stdout);
for (i = 0; i < nr; i++) {
- u64 blocknr = btrfs_node_blockptr(eb, i);
+ u64 blocknr;
+
+ if (i > BTRFS_NODEPTRS_PER_EXTENT_BUFFER(eb))
+ break;
+ blocknr = btrfs_node_blockptr(eb, i);
btrfs_node_key(eb, &disk_key, i);
btrfs_disk_key_to_cpu(&key, &disk_key);
printf("\t");
--
2.17.0
--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] btrfs-progs: print-tree: remove dead code from btrfs_print_tree
Since the out label has been deleted, this free_extent_buffer will never
be executed.
Fixes: f37ae8d275c2 ("btrfs-progs: print-tree: Enhance warning on tree block
level mismatch and error handling")
Signed-off-by: Lu Fengqi
---
print-tree.c | 2 --
1 file changed, 2 deletions(-)
diff --git a/print-tree.c b/print-tree.c
index a2f6bfc027c9..44e9b40a9874 100644
--- a/print-tree.c
+++ b/print-tree.c
@@ -1416,6 +1416,4 @@ void btrfs_print_tree(struct btrfs_root *root, struct
extent_buffer *eb, int fol
}
return;
-
- free_extent_buffer(next);
}
--
2.16.3
--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 2/4] btrfs: get fs_info from eb in btrfs_print_tree, remove argument
Signed-off-by: David Sterba
---
fs/btrfs/print-tree.c | 5 +++--
fs/btrfs/print-tree.h | 2 +-
2 files changed, 4 insertions(+), 3 deletions(-)
diff --git a/fs/btrfs/print-tree.c b/fs/btrfs/print-tree.c
index 569393ffd0fd..3823a6a64ded 100644
--- a/fs/btrfs/print-tree.c
+++ b/fs/btrfs/print-tree.c
@@ -319,8 +319,9 @@ void btrfs_print_leaf(struct extent_buffer *l)
}
}
-void btrfs_print_tree(struct btrfs_fs_info *fs_info, struct extent_buffer *c)
+void btrfs_print_tree(struct extent_buffer *c)
{
+ struct btrfs_fs_info *fs_info = c->fs_info;
int i; u32 nr;
struct btrfs_key key;
int level;
@@ -360,7 +361,7 @@ void btrfs_print_tree(struct btrfs_fs_info *fs_info, struct
extent_buffer *c)
if (btrfs_header_level(next) !=
level - 1)
BUG();
- btrfs_print_tree(fs_info, next);
+ btrfs_print_tree(next);
free_extent_buffer(next);
}
}
diff --git a/fs/btrfs/print-tree.h b/fs/btrfs/print-tree.h
index 689a52ee0cd1..3afd508ed8c5 100644
--- a/fs/btrfs/print-tree.h
+++ b/fs/btrfs/print-tree.h
@@ -19,5 +19,5 @@
#ifndef __PRINT_TREE_
#define __PRINT_TREE_
void btrfs_print_leaf(struct extent_buffer *l);
-void btrfs_print_tree(struct btrfs_fs_info *fs_info, struct extent_buffer *c);
+void btrfs_print_tree(struct extent_buffer *c);
#endif
--
2.13.0
--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] delete obsolete function btrfs_print_tree()
On Mon, Apr 04, 2016 at 05:02:38PM +0100, Filipe Manana wrote: > It's not serious if it doesn't have all the proper error handling > and etc, it's just something for debugging purposes. I'm slowly trying to remove static checker warnings so that we can detect real bugs. People sometimes leave little messages for me in their code because they know I will review the new warning: foo = kmalloc(); /* error handling deliberately left out */ It make me quite annoyed because it's like "Oh. No if we added error handling that would take 40 extra bytes of memory! Such a waste!" But we could use instead use___GFP_NOFAIL instead. Or BUG_ON(!foo)". I have gotten distracted. What was the question again? regards, dan carpenter -- To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] delete obsolete function btrfs_print_tree()
On 04/04/16 18:02, Filipe Manana wrote: > I use this function frequently during development, and there's a good > reason to use it instead of the user space tool btrfs-debug-tree. Good to know, that's why I asked. Printing unwritten extents makes sense. -h -- To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] delete obsolete function btrfs_print_tree()
On Mon, Apr 4, 2016 at 4:54 PM, Holger Hoffstätte wrote: > On 04/04/16 15:56, David Sterba wrote: >> On Fri, Mar 25, 2016 at 03:53:17PM +0100, Holger Hoffstätte wrote: >>> Dan Carpenter's static checker recently found missing IS_ERR handling >>> in print-tree.c:btrfs_print_tree(). While looking into this I found that >>> this function is no longer called anywhere and was moved to btrfs-progs >>> long ago. It can simply be removed. >> >> I'm not sure, the function could be used for debugging, and it's hard to > > ..but is it? So far nobody has complained. I will complain. I use this function frequently during development, and there's a good reason to use it instead of the user space tool btrfs-debug-tree. > >> say if we'll ever need it. Printing the whole tree to the system log >> would produce a lot of text so some manual filtering would be required, >> the function could serve as a template. > > The original problem of missing error handling from btrfs_read_tree_block() > remains as well. I don't remember if that also was true for the btrfs-progs > counterpart, but in in any case I didn't really know what to do there. > Print an error? silently ignore the stripe? abort? When I realized that the > function was not called anywhere, deleting it seemed more effective. > > Under what circumstances would the in-kernel function be more > practical or useful than the userland tool? The user land tool requires the btree nodes to be on disk. With the in kernel function we can print nodes that are not yet on disk, very useful during development. So no, we should not delete it in my opinion. It's not serious if it doesn't have all the proper error handling and etc, it's just something for debugging purposes. It does the same, won't disturb > or wedge the kernel further, is up-to-date and can be scripted. > I agree that in-place filtering (while iterating) would be nice to have, > but that's also a whole different problem and would IMHO also be better > suited for userland. > > When in doubt cut it out. When in doubt leave it alone. > > Holger > > -- > To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in > the body of a message to majord...@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 majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] delete obsolete function btrfs_print_tree()
On 04/04/16 15:56, David Sterba wrote: > On Fri, Mar 25, 2016 at 03:53:17PM +0100, Holger Hoffstätte wrote: >> Dan Carpenter's static checker recently found missing IS_ERR handling >> in print-tree.c:btrfs_print_tree(). While looking into this I found that >> this function is no longer called anywhere and was moved to btrfs-progs >> long ago. It can simply be removed. > > I'm not sure, the function could be used for debugging, and it's hard to ..but is it? So far nobody has complained. > say if we'll ever need it. Printing the whole tree to the system log > would produce a lot of text so some manual filtering would be required, > the function could serve as a template. The original problem of missing error handling from btrfs_read_tree_block() remains as well. I don't remember if that also was true for the btrfs-progs counterpart, but in in any case I didn't really know what to do there. Print an error? silently ignore the stripe? abort? When I realized that the function was not called anywhere, deleting it seemed more effective. Under what circumstances would the in-kernel function be more practical or useful than the userland tool? It does the same, won't disturb or wedge the kernel further, is up-to-date and can be scripted. I agree that in-place filtering (while iterating) would be nice to have, but that's also a whole different problem and would IMHO also be better suited for userland. When in doubt cut it out. Holger -- To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] delete obsolete function btrfs_print_tree()
On Fri, Mar 25, 2016 at 03:53:17PM +0100, Holger Hoffstätte wrote: > Dan Carpenter's static checker recently found missing IS_ERR handling > in print-tree.c:btrfs_print_tree(). While looking into this I found that > this function is no longer called anywhere and was moved to btrfs-progs > long ago. It can simply be removed. I'm not sure, the function could be used for debugging, and it's hard to say if we'll ever need it. Printing the whole tree to the system log would produce a lot of text so some manual filtering would be required, the function could serve as a template. The function is not that big that it would save bytes, but putting it under the debug config would help a bit. -- To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] delete obsolete function btrfs_print_tree()
Dan Carpenter's static checker recently found missing IS_ERR handling
in print-tree.c:btrfs_print_tree(). While looking into this I found that
this function is no longer called anywhere and was moved to btrfs-progs
long ago. It can simply be removed.
Reported-by: Dan Carpenter
Signed-off-by: Holger Hoffstätte
---
fs/btrfs/print-tree.c | 38 --
fs/btrfs/print-tree.h | 1 -
2 files changed, 39 deletions(-)
diff --git a/fs/btrfs/print-tree.c b/fs/btrfs/print-tree.c
index 147dc6c..dc28db8 100644
--- a/fs/btrfs/print-tree.c
+++ b/fs/btrfs/print-tree.c
@@ -328,41 +328,3 @@ void btrfs_print_leaf(struct btrfs_root *root, struct
extent_buffer *l)
};
}
}
-
-void btrfs_print_tree(struct btrfs_root *root, struct extent_buffer *c)
-{
- int i; u32 nr;
- struct btrfs_key key;
- int level;
-
- if (!c)
- return;
- nr = btrfs_header_nritems(c);
- level = btrfs_header_level(c);
- if (level == 0) {
- btrfs_print_leaf(root, c);
- return;
- }
- btrfs_info(root->fs_info, "node %llu level %d total ptrs %d free spc
%u",
- btrfs_header_bytenr(c), level, nr,
- (u32)BTRFS_NODEPTRS_PER_BLOCK(root) - nr);
- for (i = 0; i < nr; i++) {
- btrfs_node_key_to_cpu(c, &key, i);
- printk(KERN_INFO "\tkey %d (%llu %u %llu) block %llu\n",
- i, key.objectid, key.type, key.offset,
- btrfs_node_blockptr(c, i));
- }
- for (i = 0; i < nr; i++) {
- struct extent_buffer *next = read_tree_block(root,
- btrfs_node_blockptr(c, i),
- btrfs_node_ptr_generation(c, i));
- if (btrfs_is_leaf(next) &&
- level != 1)
- BUG();
- if (btrfs_header_level(next) !=
- level - 1)
- BUG();
- btrfs_print_tree(root, next);
- free_extent_buffer(next);
- }
-}
diff --git a/fs/btrfs/print-tree.h b/fs/btrfs/print-tree.h
index 7faddfa..9dd56b9 100644
--- a/fs/btrfs/print-tree.h
+++ b/fs/btrfs/print-tree.h
@@ -19,5 +19,4 @@
#ifndef __PRINT_TREE_
#define __PRINT_TREE_
void btrfs_print_leaf(struct btrfs_root *root, struct extent_buffer *l);
-void btrfs_print_tree(struct btrfs_root *root, struct extent_buffer *c);
#endif
--
2.7.4
signature.asc
Description: OpenPGP digital signature
Re: btrfs_print_tree?
On Sun, Jul 01, 2012 at 06:16:55PM +0800, Jeff Liu wrote: > On 07/01/2012 05:49 PM, Zhi Yong Wu wrote: > > > On Sun, Jul 1, 2012 at 5:41 PM, Mike Fleetwood > > No, i also did as this, but didn't find out who will invoke this > > function. From above output, we only saw that it invokes itself one > > time. > > Looks this is a helper routine exported to btrfs-progs previously, it is > used by debug-tree, quick-test, etc... > > But this function has been implemented at btrfs-progs now, maybe it > could be safely removed from kernel, not sure. :) Please do not remove that function, it could be helpful for debugging prints. david -- To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: btrfs_print_tree?
On Sun, Jul 1, 2012 at 6:16 PM, Jeff Liu wrote: > On 07/01/2012 05:49 PM, Zhi Yong Wu wrote: > >> On Sun, Jul 1, 2012 at 5:41 PM, Mike Fleetwood >> wrote: >>> On 1 July 2012 05:53, Zhi Yong Wu wrote: >>>> HI, >>>> >>>> Do anyone know where btrfs_print_tree is invoked? thanks. >>>> >>>> -- >>>> Regards, >>>> >>>> Zhi Yong Wu >>> >>> Is this the answer you are after? >>> >>> $ grep -r btrfs_print_tree fs/btrfs/ >>> fs/btrfs/print-tree.c:void btrfs_print_tree(struct btrfs_root *root, >>> struct extent_buffer *c) >>> fs/btrfs/print-tree.c: btrfs_print_tree(root, next); >>> fs/btrfs/print-tree.h:void btrfs_print_tree(struct btrfs_root *root, >>> struct extent_buffer *t); >> No, i also did as this, but didn't find out who will invoke this >> function. From above output, we only saw that it invokes itself one >> time. > > Looks this is a helper routine exported to btrfs-progs previously, it is > used by debug-tree, quick-test, etc... > > But this function has been implemented at btrfs-progs now, maybe it > could be safely removed from kernel, not sure. :) Great, thanks both. > > Thanks, > -Jeff > >> >>> >>> Mike >> >> >> > > -- Regards, Zhi Yong Wu -- To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: btrfs_print_tree?
-BEGIN PGP SIGNED MESSAGE- Hash: SHA1 On 01.07.2012 12:16, Jeff Liu wrote: > On 07/01/2012 05:49 PM, Zhi Yong Wu wrote: > >> On Sun, Jul 1, 2012 at 5:41 PM, Mike Fleetwood >> wrote: >>> On 1 July 2012 05:53, Zhi Yong Wu wrote: >>>> HI, >>>> >>>> Do anyone know where btrfs_print_tree is invoked? thanks. >>>> >>>> -- >>>> Regards, >>>> >>>> Zhi Yong Wu >>> >>> Is this the answer you are after? >>> >>> $ grep -r btrfs_print_tree fs/btrfs/ >>> fs/btrfs/print-tree.c:void btrfs_print_tree(struct btrfs_root *root, >>> struct extent_buffer *c) >>> fs/btrfs/print-tree.c: btrfs_print_tree(root, next); >>> fs/btrfs/print-tree.h:void btrfs_print_tree(struct btrfs_root *root, >>> struct extent_buffer *t); >> No, i also did as this, but didn't find out who will invoke this >> function. From above output, we only saw that it invokes itself one >> time. > > Looks this is a helper routine exported to btrfs-progs previously, it is > used by debug-tree, quick-test, etc... > > But this function has been implemented at btrfs-progs now, maybe it > could be safely removed from kernel, not sure. :) > > Thanks, > -Jeff Looks exactly the same to me. Thanks, Andreas > > >> >>> >>> Mike >> >> >> > > > -- > To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in > the body of a message to majord...@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html -BEGIN PGP SIGNATURE- Version: GnuPG v1.4.9 (MingW32) Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/ iQEcBAEBAgAGBQJP8CVUAAoJEIW3W1BiBxU7RMMH/A90pUdpILSWJndkJiddmbqW 17SyQMldS/xSFVHvT8ot92jyKqYhk6MFGwdkGgCX8RXCO5G5PzbxOGHxtF3j872V J3fPp9qxTpNSEhy7uDsnyMvDFgNgXWNxz61G6ISZrubd0M0rTRdd77DrYLOqgwiQ uTOwQ8WGPB+jOPb6+8URcPzyD82uUejIQKS4svmffPSQMY8gJEz5408HFB6SIyf/ +5/UUXwRYOlbNAVjiOVVhHWvrm1MDI1hD0S8duy2/TsS/cEt/d6xrWIJ9Ky2468y Q62PRf6hu3GI3yxfNrEDn5i6vRVI/0EMzJHna56r1NApqa1jIIJFVbo53UyjnGw= =3Dqd -END PGP SIGNATURE- -- To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: btrfs_print_tree?
On 07/01/2012 05:49 PM, Zhi Yong Wu wrote: > On Sun, Jul 1, 2012 at 5:41 PM, Mike Fleetwood > wrote: >> On 1 July 2012 05:53, Zhi Yong Wu wrote: >>> HI, >>> >>> Do anyone know where btrfs_print_tree is invoked? thanks. >>> >>> -- >>> Regards, >>> >>> Zhi Yong Wu >> >> Is this the answer you are after? >> >> $ grep -r btrfs_print_tree fs/btrfs/ >> fs/btrfs/print-tree.c:void btrfs_print_tree(struct btrfs_root *root, >> struct extent_buffer *c) >> fs/btrfs/print-tree.c: btrfs_print_tree(root, next); >> fs/btrfs/print-tree.h:void btrfs_print_tree(struct btrfs_root *root, >> struct extent_buffer *t); > No, i also did as this, but didn't find out who will invoke this > function. From above output, we only saw that it invokes itself one > time. Looks this is a helper routine exported to btrfs-progs previously, it is used by debug-tree, quick-test, etc... But this function has been implemented at btrfs-progs now, maybe it could be safely removed from kernel, not sure. :) Thanks, -Jeff > >> >> Mike > > > -- To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: btrfs_print_tree?
-BEGIN PGP SIGNED MESSAGE- Hash: SHA1 On 01.07.2012 11:49, Zhi Yong Wu wrote: > On Sun, Jul 1, 2012 at 5:41 PM, Mike Fleetwood > wrote: >> On 1 July 2012 05:53, Zhi Yong Wu wrote: >>> HI, >>> >>> Do anyone know where btrfs_print_tree is invoked? thanks. >>> >>> -- >>> Regards, >>> >>> Zhi Yong Wu >> >> Is this the answer you are after? >> >> $ grep -r btrfs_print_tree fs/btrfs/ >> fs/btrfs/print-tree.c:void btrfs_print_tree(struct btrfs_root *root, >> struct extent_buffer *c) >> fs/btrfs/print-tree.c: btrfs_print_tree(root, next); >> fs/btrfs/print-tree.h:void btrfs_print_tree(struct btrfs_root *root, >> struct extent_buffer *t); > No, i also did as this, but didn't find out who will invoke this > function. From above output, we only saw that it invokes itself one > time. Is this function unused then? Andreas > > >> >> Mike > > > -BEGIN PGP SIGNATURE- Version: GnuPG v1.4.9 (MingW32) Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/ iQEcBAEBAgAGBQJP8CNFAAoJEIW3W1BiBxU7C40H/0ecIgo1AagGyh/q4tLjXxAZ PILqpKZzGiX6Qc+qWnaHrNyCJU1ntppO73k/SmhMpx+xycw/qHbqKtCpw/W8aQfn ihKOP9amgCUR/X+1W7hn190r0c1teWnXV/bYoD81fksWYuZmG9XzBOF4u9iLrtIb 0hpeBtFPr7jizKv3puOVKBpEV3ZMUdiqwc+G5GxUSlOssqKdF38gHGvj1k3j1qTX 0JRndsaeZq0smA0RJnYa9nX7gvg8CnmUXIJ/yLgg8Rnn5LhJQoq2eK4fgE+I0OCy SjlzSQG78W9CnWmE6SitfVeb/QUyLdCZ/mtwPxqeKd9p06hcLCU4R89rWRnNWTQ= =bq50 -END PGP SIGNATURE- -- To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: btrfs_print_tree?
On Sun, Jul 1, 2012 at 5:41 PM, Mike Fleetwood wrote: > On 1 July 2012 05:53, Zhi Yong Wu wrote: >> HI, >> >> Do anyone know where btrfs_print_tree is invoked? thanks. >> >> -- >> Regards, >> >> Zhi Yong Wu > > Is this the answer you are after? > > $ grep -r btrfs_print_tree fs/btrfs/ > fs/btrfs/print-tree.c:void btrfs_print_tree(struct btrfs_root *root, > struct extent_buffer *c) > fs/btrfs/print-tree.c: btrfs_print_tree(root, next); > fs/btrfs/print-tree.h:void btrfs_print_tree(struct btrfs_root *root, > struct extent_buffer *t); No, i also did as this, but didn't find out who will invoke this function. From above output, we only saw that it invokes itself one time. > > Mike -- Regards, Zhi Yong Wu -- To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: btrfs_print_tree?
On 1 July 2012 05:53, Zhi Yong Wu wrote: > HI, > > Do anyone know where btrfs_print_tree is invoked? thanks. > > -- > Regards, > > Zhi Yong Wu Is this the answer you are after? $ grep -r btrfs_print_tree fs/btrfs/ fs/btrfs/print-tree.c:void btrfs_print_tree(struct btrfs_root *root, struct extent_buffer *c) fs/btrfs/print-tree.c: btrfs_print_tree(root, next); fs/btrfs/print-tree.h:void btrfs_print_tree(struct btrfs_root *root, struct extent_buffer *t); Mike -- To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
btrfs_print_tree?
HI, Do anyone know where btrfs_print_tree is invoked? thanks. -- Regards, Zhi Yong Wu -- To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] Changed type field to be %x for tree header (line in btrfs_print_tree)
Chris Mason wrote: > [ use %x for key type printks ] > > I've gone back and forth on this, since the type used to be a mask. > But, right now all the #defines are in decimal, I'd rather keep the > output the same way. > > -chris Then, perhaps, one should change the %x in print-tree.c:btrfs_print_tree() to a %u? Alan -- To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] Changed type field to be %x for tree header (line in btrfs_print_tree)
[ use %x for key type printks ] I've gone back and forth on this, since the type used to be a mask. But, right now all the #defines are in decimal, I'd rather keep the output the same way. -chris -- To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] Changed type field to be %x for tree header (line in btrfs_print_tree)
This makes it easier to visually match keys - see:
print-tree.c:btrfs_print_tree():329:
printf("\tkey %d (%llu %x %llu) block %llu (%llu) gen %llu\n",
I'm not sure if this breaks any other parsers?
Signed-off-by: Alan D. Brunelle <[EMAIL PROTECTED]>
---
debug-tree.c |2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/debug-tree.c b/debug-tree.c
index fa5c90c..9185dc2 100644
--- a/debug-tree.c
+++ b/debug-tree.c
@@ -190,7 +190,7 @@ int main(int ac, char **av)
break;
}
if (!skip && !extent_only) {
-printf("tree %llu %u %llu\n",
+printf("tree %llu %x %llu\n",
(unsigned long long)found_key.objectid,
found_key.type,
(unsigned long long)found_key.offset);
--
1.5.4.3
