btrfs-progs: extent buffer related refactor and cleanup

PATCH Applied Qu Wenruo 3 participants 8 messages Mar 30, 2018 – Apr 9, 2018 Btrfs file system

lore

Share & tools

lore

Qu Wenruo

author

2018-03-30 05:49 UTC #0 lore raw

v2 cover btrfs-progs: extent buffer related refactor and cleanup

The patchset can be fetched from github:
https://github.com/adam900710/btrfs-progs/tree/eb_cleanup

Just like kernel cleanup and refactors, this patchset will embed btrfs_fs_info structure into extent_buffer.

And fixes several possible NULL pointer dereference (although not utilized in btrfs-progs yet).

Changelog:
v2:
Embarrassingly, I forgot to install reiserfsprogs in my development machine, so the 3rd patch lacks one call site in convert/source-reiserfs.c.

Qu Wenruo (5):
btrfs-progs: extent_io: Fix NULL pointer dereference in
free_extent_buffer_final()
btrfs-progs: extent_io: Init eb->lru to avoid NULL pointer dereference
btrfs-progs: extent_io: Refactor alloc_extent_buffer() to follow
kernel parameters
btrfs-progs: Unify btrfs_leaf_free_psace() parameter with kernel
btrfs-progs: print-tree: Remove btrfs_root parameter

 btrfs-corrupt-block.c     |  2 +-
 check/main.c              |  2 +-
 check/mode-lowmem.c       |  2 +-
 cmds-inspect-dump-tree.c  | 31 ++++++++++------------
 convert/source-reiserfs.c |  3 +--
 ctree.c                   | 65 +++++++++++++++++++++++++----------------------
 ctree.h                   |  3 ++-
 disk-io.c                 |  3 +--
 extent-tree.c             |  8 +++---
 extent_io.c               | 17 ++++++++-----
 extent_io.h               |  3 ++-
 print-tree.c              | 20 ++++++++-------
 print-tree.h              |  4 +--
 13 files changed, 85 insertions(+), 78 deletions(-)

Signature · 1 line

2.16.3

Qu Wenruo

author

2018-03-30 05:49 UTC #1 lore raw

PATCH v2 1/5 btrfs-progs: extent_io: Fix NULL pointer dereference in free_extent_buffer_final()

In free_extent_buffer_final() we access eb->tree->cache_size in BUG_ON().
However eb->tree can be NULL if it's a cloned extent buffer.

Currently the cloned extent buffer is only used in backref.c, paths_from_inode() function.
Thankfully that function is not used yet (but could be pretty useful to convert inode number to path, so I'd like to keep such function).

Anyway, check eb->tree before accessing its member.

Signed-off-by Qu Wenruo <[email protected]>

---
 extent_io.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

extent_io.c +1 -1

index eda1fb6f5897..986ad5c0577c 100644
@@ -587,7 +587,7 @@ static void free_extent_buffer_final(struct extent_buffer *eb)
 	struct extent_io_tree *tree = eb->tree;
 
 	BUG_ON(eb->refs);
-	BUG_ON(tree->cache_size < eb->len);
+	BUG_ON(tree && tree->cache_size < eb->len);
 	list_del_init(&eb->lru);
 	if (!(eb->flags & EXTENT_BUFFER_DUMMY)) {
 		remove_cache_extent(&tree->cache, &eb->cache_node);

Signature · 1 line

2.16.3

Qu Wenruo

author

2018-03-30 05:49 UTC #2 lore raw

PATCH v2 2/5 btrfs-progs: extent_io: Init eb->lru to avoid NULL pointer dereference

In reply to Qu Wenruo

eb->lru is not initialized in __alloc_extent_buffer(), so in the following call chain, it could call NULL pointer dereference:

btrfs_clone_extent_buffer()
|- __alloc_extent_buffer()
|- Now eb->lru is NULL (not initialized)

free_extent_buffer_final()
|- list_del_init(&eb->lru)

Thankfully, current btrfs-progs won't trigger such bug as the only btrfs_clone_extent_buffer() user is paths_from_inode(), which is not used by anyone.
(But due to the usefulness of that function in future offline scrub, I'd like to keep this dead code)

Anyway, initialize eb->lru in __alloc_extent_bufer() bring no harm.

Signed-off-by Qu Wenruo <[email protected]>

---
 extent_io.c | 1 +
 1 file changed, 1 insertion(+)

extent_io.c +1 -0

index 986ad5c0577c..3117782335ab 100644
@@ -564,6 +564,7 @@ static struct extent_buffer *__alloc_extent_buffer(struct extent_io_tree *tree,
 	eb->cache_node.start = bytenr;
 	eb->cache_node.size = blocksize;
 	INIT_LIST_HEAD(&eb->recow);
+	INIT_LIST_HEAD(&eb->lru);
 
 	return eb;
 }

Signature · 1 line

2.16.3

Qu Wenruo

author

2018-03-30 05:49 UTC #3 lore raw

PATCH v2 3/5 btrfs-progs: extent_io: Refactor alloc_extent_buffer() to follow kernel parameters

In reply to Qu Wenruo

Instead of using the internal struct extent_io_tree, use struct fs_info.

This does not only unify the interface between kernel and btrfs-progs, but also makes later btrfs_print_tree() use less parameters.

Signed-off-by Qu Wenruo <[email protected]>

---
 convert/source-reiserfs.c |  3 +--
 disk-io.c                 |  3 +--
 extent_io.c               | 14 +++++++++-----
 extent_io.h               |  3 ++-
 4 files changed, 13 insertions(+), 10 deletions(-)

convert/source-reiserfs.c +1 -2

index eeb68d962c5d..e0b3b685f764 100644
@@ -350,8 +350,7 @@ static int convert_direct(struct btrfs_trans_handle *trans,
 	if (ret)
 		return ret;
 
-	eb = alloc_extent_buffer(&root->fs_info->extent_cache, key.objectid,
-				 sectorsize);
+	eb = alloc_extent_buffer(root->fs_info, key.objectid, sectorsize);
 
 	if (!eb)
 		return -ENOMEM;

disk-io.c +1 -2

index 76958aef239e..610963357675 100644
@@ -184,8 +184,7 @@ struct extent_buffer *btrfs_find_tree_block(struct btrfs_fs_info *fs_info,
 struct extent_buffer* btrfs_find_create_tree_block(
 		struct btrfs_fs_info *fs_info, u64 bytenr)
 {
-	return alloc_extent_buffer(&fs_info->extent_cache, bytenr,
-			fs_info->nodesize);
+	return alloc_extent_buffer(fs_info, bytenr, fs_info->nodesize);
 }
 
 void readahead_tree_block(struct btrfs_fs_info *fs_info, u64 bytenr,

extent_io.c +9 -5

index 3117782335ab..198492699438 100644
@@ -545,7 +545,7 @@ out:
 	return ret;
 }
 
-static struct extent_buffer *__alloc_extent_buffer(struct extent_io_tree *tree,
+static struct extent_buffer *__alloc_extent_buffer(struct btrfs_fs_info *info,
 						   u64 bytenr, u32 blocksize)
 {
 	struct extent_buffer *eb;

@@ -558,11 +558,12 @@ static struct extent_buffer *__alloc_extent_buffer(struct extent_io_tree *tree,
 	eb->len = blocksize;
 	eb->refs = 1;
 	eb->flags = 0;
-	eb->tree = tree;
 	eb->fd = -1;
 	eb->dev_bytenr = (u64)-1;
 	eb->cache_node.start = bytenr;
 	eb->cache_node.size = blocksize;
+	eb->fs_info = info;
+	eb->tree = &info->extent_cache;
 	INIT_LIST_HEAD(&eb->recow);
 	INIT_LIST_HEAD(&eb->lru);

@@ -573,9 +574,11 @@ struct extent_buffer *btrfs_clone_extent_buffer(struct extent_buffer *src)
 {
 	struct extent_buffer *new;
 
-	new = __alloc_extent_buffer(NULL, src->start, src->len);
+	new = __alloc_extent_buffer(src->fs_info, src->start, src->len);
 	if (!new)
 		return NULL;
+	/* cloned eb is not linked into fs_info->extent_cache */
+	new->tree = NULL;
 
 	copy_extent_buffer(new, src, 0, 0, src->len);
 	new->flags |= EXTENT_BUFFER_DUMMY;

@@ -665,10 +668,11 @@ static void trim_extent_buffer_cache(struct extent_io_tree *tree)
 	}
 }
 
-struct extent_buffer *alloc_extent_buffer(struct extent_io_tree *tree,
+struct extent_buffer *alloc_extent_buffer(struct btrfs_fs_info *fs_info,
 					  u64 bytenr, u32 blocksize)
 {
 	struct extent_buffer *eb;
+	struct extent_io_tree *tree = &fs_info->extent_cache;
 	struct cache_extent *cache;
 
 	cache = lookup_cache_extent(&tree->cache, bytenr, blocksize);

@@ -685,7 +689,7 @@ struct extent_buffer *alloc_extent_buffer(struct extent_io_tree *tree,
 					  cache_node);
 			free_extent_buffer(eb);
 		}
-		eb = __alloc_extent_buffer(tree, bytenr, blocksize);
+		eb = __alloc_extent_buffer(fs_info, bytenr, blocksize);
 		if (!eb)
 			return NULL;
 		ret = insert_cache_extent(&tree->cache, &eb->cache_node);

extent_io.h +2 -1

index 17a4a8298635..f8f73089ab40 100644
@@ -98,6 +98,7 @@ struct extent_buffer {
 	int refs;
 	u32 flags;
 	int fd;
+	struct btrfs_fs_info *fs_info;
 	char data[] __attribute__((aligned(8)));
 };

@@ -145,7 +146,7 @@ struct extent_buffer *find_extent_buffer(struct extent_io_tree *tree,
 					 u64 bytenr, u32 blocksize);
 struct extent_buffer *find_first_extent_buffer(struct extent_io_tree *tree,
 					       u64 start);
-struct extent_buffer *alloc_extent_buffer(struct extent_io_tree *tree,
+struct extent_buffer *alloc_extent_buffer(struct btrfs_fs_info *fs_info,
 					  u64 bytenr, u32 blocksize);
 struct extent_buffer *btrfs_clone_extent_buffer(struct extent_buffer *src);
 void free_extent_buffer(struct extent_buffer *eb);

Signature · 1 line

2.16.3

Qu Wenruo

author

2018-03-30 05:49 UTC #4 lore raw

PATCH v2 4/5 btrfs-progs: Unify btrfs_leaf_free_psace() parameter with kernel

In reply to Qu Wenruo

Instead of struct btrfs_root, use struct btrfs_fs_info, since nodesize is now a per-fs setting, and with the need to pass a @root, caller don't need to wonder which root should be passed.

Signed-off-by Qu Wenruo <[email protected]>

---
 btrfs-corrupt-block.c |  2 +-
 check/main.c          |  2 +-
 check/mode-lowmem.c   |  2 +-
 ctree.c               | 49 ++++++++++++++++++++++++++-----------------------
 ctree.h               |  3 ++-
 print-tree.c          |  2 +-
 6 files changed, 32 insertions(+), 28 deletions(-)

btrfs-corrupt-block.c +1 -1

index 59ee1b45922e..da0ec8c51e5a 100644
@@ -726,7 +726,7 @@ out:
 static void shift_items(struct btrfs_root *root, struct extent_buffer *eb)
 {
 	int nritems = btrfs_header_nritems(eb);
-	int shift_space = btrfs_leaf_free_space(root, eb) / 2;
+	int shift_space = btrfs_leaf_free_space(root->fs_info, eb) / 2;
 	int slot = nritems / 2;
 	int i = 0;
 	unsigned int data_end = btrfs_item_offset_nr(eb, nritems - 1);

check/main.c +1 -1

index 891a6d797756..7fde46e459e4 100644
@@ -5959,7 +5959,7 @@ static int run_next_block(struct btrfs_root *root,
 		goto out;
 
 	if (btrfs_is_leaf(buf)) {
-		btree_space_waste += btrfs_leaf_free_space(root, buf);
+		btree_space_waste += btrfs_leaf_free_space(fs_info, buf);
 		for (i = 0; i < nritems; i++) {
 			struct btrfs_file_extent_item *fi;

check/mode-lowmem.c +1 -1

index dac3201b7d99..bfe45abab6cc 100644
@@ -390,7 +390,7 @@ static void account_bytes(struct btrfs_root *root, struct btrfs_path *path,
 		total_extent_tree_bytes += eb->len;
 
 	if (level == 0) {
-		btree_space_waste += btrfs_leaf_free_space(root, eb);
+		btree_space_waste += btrfs_leaf_free_space(root->fs_info, eb);
 	} else {
 		free_nrs = (BTRFS_NODEPTRS_PER_BLOCK(root->fs_info) -
 			    btrfs_header_nritems(eb));

ctree.c +26 ~~-23~~

154 lines not on this page · raw message

ctree.h +2 -1

index fa861ba0b4c3..3d13720faf69 100644
@@ -2662,7 +2662,8 @@ static inline int btrfs_next_item(struct btrfs_root *root,
 }
 
 int btrfs_prev_leaf(struct btrfs_root *root, struct btrfs_path *path);
-int btrfs_leaf_free_space(struct btrfs_root *root, struct extent_buffer *leaf);
+int btrfs_leaf_free_space(struct btrfs_fs_info *fs_info,
+			  struct extent_buffer *leaf);
 void btrfs_fixup_low_keys(struct btrfs_root *root, struct btrfs_path *path,
 			  struct btrfs_disk_key *key, int level);
 int btrfs_set_item_key_safe(struct btrfs_root *root, struct btrfs_path *path,

print-tree.c +1 -1

index a2f6bfc027c9..418ea5b4e7d4 100644
@@ -1190,7 +1190,7 @@ void btrfs_print_leaf(struct btrfs_root *root, struct extent_buffer *eb)
 
 	printf("leaf %llu items %d free space %d generation %llu owner ",
 		(unsigned long long)btrfs_header_bytenr(eb), nr,
-		btrfs_leaf_free_space(root, eb),
+		btrfs_leaf_free_space(root->fs_info, eb),
 		(unsigned long long)btrfs_header_generation(eb));
 	print_objectid(stdout, btrfs_header_owner(eb), 0);
 	printf("\n");

Signature · 1 line

2.16.3

Qu Wenruo

author

2018-03-30 05:49 UTC #5 lore raw

PATCH v2 5/5 btrfs-progs: print-tree: Remove btrfs_root parameter

In reply to Qu Wenruo

Just like kernel cleanup made by David, btrfs_print_leaf() and btrfs_print_tree() doesn't need btrfs_root parameter at all.

With previous patches as preparation, now we can remove the btrfs_root parameter.

Signed-off-by Qu Wenruo <[email protected]>

---
 cmds-inspect-dump-tree.c | 31 ++++++++++++++-----------------
 ctree.c                  | 16 ++++++++--------
 extent-tree.c            |  8 ++++----
 print-tree.c             | 20 +++++++++++---------
 print-tree.h             |  4 ++--
 5 files changed, 39 insertions(+), 40 deletions(-)

cmds-inspect-dump-tree.c +14 ~~-17~~

index 0802b31e9596..b0cd49b32664 100644
@@ -33,8 +33,9 @@
 #include "utils.h"
 #include "help.h"
 
-static void print_extents(struct btrfs_root *root, struct extent_buffer *eb)
+static void print_extents(struct extent_buffer *eb)
 {
+	struct btrfs_fs_info *fs_info = eb->fs_info;
 	struct extent_buffer *next;
 	int i;
 	u32 nr;

@@ -43,13 +44,13 @@ static void print_extents(struct btrfs_root *root, struct extent_buffer *eb)
 		return;
 
 	if (btrfs_is_leaf(eb)) {
-		btrfs_print_leaf(root, eb);
+		btrfs_print_leaf(eb);
 		return;
 	}
 
 	nr = btrfs_header_nritems(eb);
 	for (i = 0; i < nr; i++) {
-		next = read_tree_block(root->fs_info,
+		next = read_tree_block(fs_info,
 				btrfs_node_blockptr(eb, i),
 				btrfs_node_ptr_generation(eb, i));
 		if (!extent_buffer_uptodate(next))

@@ -68,7 +69,7 @@ static void print_extents(struct btrfs_root *root, struct extent_buffer *eb)
 				btrfs_header_level(eb));
 			goto out;
 		}
-		print_extents(root, next);
+		print_extents(next);
 		free_extent_buffer(next);
 	}

@@ -331,7 +332,7 @@ int cmd_inspect_dump_tree(int argc, char **argv)
 				(unsigned long long)block_only);
 			goto close_root;
 		}
-		btrfs_print_tree(root, leaf, follow);
+		btrfs_print_tree(leaf, follow);
 		free_extent_buffer(leaf);
 		goto close_root;
 	}

@@ -358,20 +359,17 @@ int cmd_inspect_dump_tree(int argc, char **argv)
 		} else {
 			if (info->tree_root->node) {
 				printf("root tree\n");
-				btrfs_print_tree(info->tree_root,
-						 info->tree_root->node, 1);
+				btrfs_print_tree(info->tree_root->node, 1);
 			}
 
 			if (info->chunk_root->node) {
 				printf("chunk tree\n");
-				btrfs_print_tree(info->chunk_root,
-						 info->chunk_root->node, 1);
+				btrfs_print_tree(info->chunk_root->node, 1);
 			}
 
 			if (info->log_root_tree) {
 				printf("log root tree\n");
-				btrfs_print_tree(info->log_root_tree,
-						info->log_root_tree->node, 1);
+				btrfs_print_tree(info->log_root_tree->node, 1);
 			}
 		}
 	}

@@ -391,7 +389,7 @@ again:
 			goto close_root;
 		}
 		printf("root tree\n");
-		btrfs_print_tree(info->tree_root, info->tree_root->node, 1);
+		btrfs_print_tree(info->tree_root->node, 1);
 		goto close_root;
 	}

@@ -401,7 +399,7 @@ again:
 			goto close_root;
 		}
 		printf("chunk tree\n");
-		btrfs_print_tree(info->chunk_root, info->chunk_root->node, 1);
+		btrfs_print_tree(info->chunk_root->node, 1);
 		goto close_root;
 	}

@@ -411,8 +409,7 @@ again:
 			goto close_root;
 		}
 		printf("log root tree\n");
-		btrfs_print_tree(info->log_root_tree, info->log_root_tree->node,
-				 1);
+		btrfs_print_tree(info->log_root_tree->node, 1);
 		goto close_root;
 	}

@@ -548,7 +545,7 @@ again:
 				printf(" tree ");
 				btrfs_print_key(&disk_key);
 				printf("\n");
-				print_extents(tree_root_scan, buf);
+				print_extents(buf);
 			} else if (!skip) {
 				printf(" tree ");
 				btrfs_print_key(&disk_key);

@@ -558,7 +555,7 @@ again:
 					       btrfs_header_level(buf));
 				} else {
 					printf(" \n");
-					btrfs_print_tree(tree_root_scan, buf, 1);
+					btrfs_print_tree(buf, 1);
 				}
 			}
 			free_extent_buffer(buf);

ctree.c +8 -8

index 20848f0f7024..d1c41925d71c 100644
@@ -2315,7 +2315,7 @@ split:
 
 	ret = 0;
 	if (btrfs_leaf_free_space(root->fs_info, leaf) < 0) {
-		btrfs_print_leaf(root, leaf);
+		btrfs_print_leaf(leaf);
 		BUG();
 	}
 	kfree(buf);

@@ -2411,7 +2411,7 @@ int btrfs_truncate_item(struct btrfs_root *root, struct btrfs_path *path,
 
 	ret = 0;
 	if (btrfs_leaf_free_space(root->fs_info, leaf) < 0) {
-		btrfs_print_leaf(root, leaf);
+		btrfs_print_leaf(leaf);
 		BUG();
 	}
 	return ret;

@@ -2436,7 +2436,7 @@ int btrfs_extend_item(struct btrfs_root *root, struct btrfs_path *path,
 	data_end = leaf_data_end(root->fs_info, leaf);
 
 	if (btrfs_leaf_free_space(root->fs_info, leaf) < data_size) {
-		btrfs_print_leaf(root, leaf);
+		btrfs_print_leaf(leaf);
 		BUG();
 	}
 	slot = path->slots[0];

@@ -2444,7 +2444,7 @@ int btrfs_extend_item(struct btrfs_root *root, struct btrfs_path *path,
 
 	BUG_ON(slot < 0);
 	if (slot >= nritems) {
-		btrfs_print_leaf(root, leaf);
+		btrfs_print_leaf(leaf);
 		printk("slot %d too large, nritems %d\n", slot, nritems);
 		BUG_ON(1);
 	}

@@ -2473,7 +2473,7 @@ int btrfs_extend_item(struct btrfs_root *root, struct btrfs_path *path,
 
 	ret = 0;
 	if (btrfs_leaf_free_space(root->fs_info, leaf) < 0) {
-		btrfs_print_leaf(root, leaf);
+		btrfs_print_leaf(leaf);
 		BUG();
 	}
 	return ret;

@@ -2522,7 +2522,7 @@ int btrfs_insert_empty_items(struct btrfs_trans_handle *trans,
 	data_end = leaf_data_end(root->fs_info, leaf);
 
 	if (btrfs_leaf_free_space(root->fs_info, leaf) < total_size) {
-		btrfs_print_leaf(root, leaf);
+		btrfs_print_leaf(leaf);
 		printk("not enough freespace need %u have %d\n",
 		       total_size, btrfs_leaf_free_space(root->fs_info, leaf));
 		BUG();

@@ -2535,7 +2535,7 @@ int btrfs_insert_empty_items(struct btrfs_trans_handle *trans,
 		unsigned int old_data = btrfs_item_end_nr(leaf, slot);
 
 		if (old_data < data_end) {
-			btrfs_print_leaf(root, leaf);
+			btrfs_print_leaf(leaf);
 			printk("slot %d old_data %d data_end %d\n",
 			       slot, old_data, data_end);
 			BUG_ON(1);

@@ -2583,7 +2583,7 @@ int btrfs_insert_empty_items(struct btrfs_trans_handle *trans,
 	}
 
 	if (btrfs_leaf_free_space(root->fs_info, leaf) < 0) {
-		btrfs_print_leaf(root, leaf);
+		btrfs_print_leaf(leaf);
 		BUG();
 	}

extent-tree.c +4 -4

index e2ae74a7fe66..ea205ccf4c30 100644
@@ -1070,7 +1070,7 @@ again:
 		printf("Size is %u, needs to be %u, slot %d\n",
 		       (unsigned)item_size,
 		       (unsigned)sizeof(*ei), path->slots[0]);
-		btrfs_print_leaf(root, leaf);
+		btrfs_print_leaf(leaf);
 		return -EINVAL;
 	}
 	BUG_ON(item_size < sizeof(*ei));

@@ -1587,7 +1587,7 @@ again:
 	}
 
 	if (ret != 0) {
-		btrfs_print_leaf(root, path->nodes[0]);
+		btrfs_print_leaf(path->nodes[0]);
 		printk("failed to find block number %Lu\n",
 			(unsigned long long)bytenr);
 		BUG();

@@ -2273,7 +2273,7 @@ static int __free_extent(struct btrfs_trans_handle *trans,
 				printk(KERN_ERR "umm, got %d back from search"
 				       ", was looking for %llu\n", ret,
 				       (unsigned long long)bytenr);
-				btrfs_print_leaf(extent_root, path->nodes[0]);
+				btrfs_print_leaf(path->nodes[0]);
 			}
 			BUG_ON(ret);
 			extent_slot = path->slots[0];

@@ -2311,7 +2311,7 @@ static int __free_extent(struct btrfs_trans_handle *trans,
 			printk(KERN_ERR "umm, got %d back from search"
 			       ", was looking for %llu\n", ret,
 			       (unsigned long long)bytenr);
-			btrfs_print_leaf(extent_root, path->nodes[0]);
+			btrfs_print_leaf(path->nodes[0]);
 		}
 		BUG_ON(ret);
 		extent_slot = path->slots[0];

print-tree.c +11 -9

index 418ea5b4e7d4..a1a7954abdae 100644
@@ -1173,8 +1173,9 @@ static void header_flags_to_str(u64 flags, char *ret)
 	}
 }
 
-void btrfs_print_leaf(struct btrfs_root *root, struct extent_buffer *eb)
+void btrfs_print_leaf(struct extent_buffer *eb)
 {
+	struct btrfs_fs_info *fs_info = eb->fs_info;
 	struct btrfs_item *item;
 	struct btrfs_disk_key disk_key;
 	char flags_str[128];

@@ -1190,7 +1191,7 @@ void btrfs_print_leaf(struct btrfs_root *root, struct extent_buffer *eb)
 
 	printf("leaf %llu items %d free space %d generation %llu owner ",
 		(unsigned long long)btrfs_header_bytenr(eb), nr,
-		btrfs_leaf_free_space(root->fs_info, eb),
+		btrfs_leaf_free_space(fs_info, eb),
 		(unsigned long long)btrfs_header_generation(eb));
 	print_objectid(stdout, btrfs_header_owner(eb), 0);
 	printf("\n");

@@ -1290,7 +1291,7 @@ void btrfs_print_leaf(struct btrfs_root *root, struct extent_buffer *eb)
 			printf("\t\tcsum item\n");
 			break;
 		case BTRFS_EXTENT_CSUM_KEY:
-			print_extent_csum(eb, root->fs_info, item_size,
+			print_extent_csum(eb, fs_info, item_size,
 					offset);
 			break;
 		case BTRFS_EXTENT_DATA_KEY:

@@ -1351,10 +1352,11 @@ void btrfs_print_leaf(struct btrfs_root *root, struct extent_buffer *eb)
 	}
 }
 
-void btrfs_print_tree(struct btrfs_root *root, struct extent_buffer *eb, int follow)
+void btrfs_print_tree(struct extent_buffer *eb, int follow)
 {
 	u32 i;
 	u32 nr;
+	struct btrfs_fs_info *fs_info = eb->fs_info;
 	struct btrfs_disk_key disk_key;
 	struct btrfs_key key;
 	struct extent_buffer *next;

@@ -1363,13 +1365,13 @@ void btrfs_print_tree(struct btrfs_root *root, struct extent_buffer *eb, int fol
 		return;
 	nr = btrfs_header_nritems(eb);
 	if (btrfs_is_leaf(eb)) {
-		btrfs_print_leaf(root, eb);
+		btrfs_print_leaf(eb);
 		return;
 	}
 	printf("node %llu level %d items %d free %u generation %llu owner ",
 	       (unsigned long long)eb->start,
 	        btrfs_header_level(eb), nr,
-		(u32)BTRFS_NODEPTRS_PER_BLOCK(root->fs_info) - nr,
+		(u32)BTRFS_NODEPTRS_PER_BLOCK(fs_info) - nr,
 		(unsigned long long)btrfs_header_generation(eb));
 	print_objectid(stdout, btrfs_header_owner(eb), 0);
 	printf("\n");

@@ -1383,7 +1385,7 @@ void btrfs_print_tree(struct btrfs_root *root, struct extent_buffer *eb, int fol
 		btrfs_print_key(&disk_key);
 		printf(" block %llu (%llu) gen %llu\n",
 		       (unsigned long long)blocknr,
-		       (unsigned long long)blocknr / root->fs_info->nodesize,
+		       (unsigned long long)blocknr / fs_info->nodesize,
 		       (unsigned long long)btrfs_node_ptr_generation(eb, i));
 		fflush(stdout);
 	}

@@ -1391,7 +1393,7 @@ void btrfs_print_tree(struct btrfs_root *root, struct extent_buffer *eb, int fol
 		return;
 
 	for (i = 0; i < nr; i++) {
-		next = read_tree_block(root->fs_info,
+		next = read_tree_block(fs_info,
 				btrfs_node_blockptr(eb, i),
 				btrfs_node_ptr_generation(eb, i));
 		if (!extent_buffer_uptodate(next)) {

@@ -1411,7 +1413,7 @@ void btrfs_print_tree(struct btrfs_root *root, struct extent_buffer *eb, int fol
 			free_extent_buffer(next);
 			continue;
 		}
-		btrfs_print_tree(root, next, 1);
+		btrfs_print_tree(next, 1);
 		free_extent_buffer(next);
 	}

print-tree.h +2 -2

index df58b846788b..62667d7f6195 100644
@@ -19,8 +19,8 @@
 #ifndef __PRINT_TREE_H__
 #define __PRINT_TREE_H__
 
-void btrfs_print_leaf(struct btrfs_root *root, struct extent_buffer *l);
-void btrfs_print_tree(struct btrfs_root *root, struct extent_buffer *t, int follow);
+void btrfs_print_leaf(struct extent_buffer *l);
+void btrfs_print_tree(struct extent_buffer *t, int follow);
 void btrfs_print_key(struct btrfs_disk_key *disk_key);
 void print_chunk_item(struct extent_buffer *eb, struct btrfs_chunk *chunk);
 void print_extent_item(struct extent_buffer *eb, int slot, int metadata);

Signature · 1 line

2.16.3

Lu Fengqi

Reviewed-by on the v2 series

2018-03-30 07:01 UTC #6 lore raw

In reply to Qu Wenruo

Quoted 15 lines · Qu Wenruo

The patchset can be fetched from github:
https://github.com/adam900710/btrfs-progs/tree/eb_cleanup
Just like kernel cleanup and refactors, this patchset will embed btrfs_fs_info structure into extent_buffer.
And fixes several possible NULL pointer dereference (although not utilized in btrfs-progs yet).
Changelog:
v2:
Embarrassingly, I forgot to install reiserfsprogs in my development machine, so the 3rd patch lacks one call site in convert/source-reiserfs.c.

Quoted 8 lines

Qu Wenruo (5):
btrfs-progs: extent_io: Fix NULL pointer dereference in
free_extent_buffer_final()
btrfs-progs: extent_io: Init eb->lru to avoid NULL pointer dereference
btrfs-progs: extent_io: Refactor alloc_extent_buffer() to follow
kernel parameters
btrfs-progs: Unify btrfs_leaf_free_psace() parameter with kernel
btrfs-progs: print-tree: Remove btrfs_root parameter

The patchset looks good to me.

Reviewed-by Lu Fengqi <[email protected]>

Quoted 25 lines

btrfs-corrupt-block.c     |  2 +-
check/main.c              |  2 +-
check/mode-lowmem.c       |  2 +-
cmds-inspect-dump-tree.c  | 31 ++++++++++------------
convert/source-reiserfs.c |  3 +--
ctree.c                   | 65 +++++++++++++++++++++++++----------------------
ctree.h                   |  3 ++-
disk-io.c                 |  3 +--
extent-tree.c             |  8 +++---
extent_io.c               | 17 ++++++++-----
extent_io.h               |  3 ++-
print-tree.c              | 20 ++++++++-------
print-tree.h              |  4 +--
13 files changed, 85 insertions(+), 78 deletions(-)

Signature · 6 lines

2.16.3

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

Signature · 2 lines

Thanks,
Lu

David Sterba

Applied

2018-04-09 14:57 UTC #7 lore raw

In reply to Qu Wenruo

Quoted 15 lines · Qu Wenruo

The patchset can be fetched from github:
https://github.com/adam900710/btrfs-progs/tree/eb_cleanup
Just like kernel cleanup and refactors, this patchset will embed btrfs_fs_info structure into extent_buffer.
And fixes several possible NULL pointer dereference (although not utilized in btrfs-progs yet).
Changelog:
v2:
Embarrassingly, I forgot to install reiserfsprogs in my development machine, so the 3rd patch lacks one call site in convert/source-reiserfs.c.

Quoted 8 lines

Qu Wenruo (5):
btrfs-progs: extent_io: Fix NULL pointer dereference in
free_extent_buffer_final()
btrfs-progs: extent_io: Init eb->lru to avoid NULL pointer dereference
btrfs-progs: extent_io: Refactor alloc_extent_buffer() to follow
kernel parameters
btrfs-progs: Unify btrfs_leaf_free_psace() parameter with kernel
btrfs-progs: print-tree: Remove btrfs_root parameter

Applied, thanks.