[PATCH 0/3] Misc cleanups to cached extent

[PATCH 0/3] Misc cleanups to cached extent

[Nikolay Borisov]

Here are a couple of cleanups I stumbled upon while looking at the freespace 
validation code. The first one simplifies btrfs_rmap_block that has an unused 
parameter. The next 2 patches cleanup the cache_extent apis since they provide
more than we are actually using (or have ever used).

Nikolay Borisov (3):
  btrfs-progs: Remove devid parameter from btrfs_rmap_block
  btrfs-progs: Remove add_cache_extent2
  btrfs-progs: Remove objectid argument from alloc_cache_extent

 check/main.c   |  2 +-
 extent-cache.c | 21 ++++-----------------
 extent-cache.h |  2 --
 extent-tree.c  | 10 +++++-----
 volumes.c      |  7 ++-----
 volumes.h      |  4 ++--
 6 files changed, 14 insertions(+), 32 deletions(-)

-- 
2.7.4

[PATCH 1/3] btrfs-progs: Remove devid parameter from btrfs_rmap_block

[Nikolay Borisov]

This parameter was introduced with the original implementation of the
function but has never really been used, so just remove it.

Signed-off-by: Nikolay Borisov <nborisov@suse.com>
---
 check/main.c  |  2 +-
 extent-tree.c | 10 +++++-----
 volumes.c     |  7 ++-----
 volumes.h     |  4 ++--
 4 files changed, 10 insertions(+), 13 deletions(-)

diff --git a/check/main.c b/check/main.c
index c4a1801fb0ef..a2b91df05569 100644
--- a/check/main.c
+++ b/check/main.c
@@ -5147,7 +5147,7 @@ static int check_cache_range(struct btrfs_root *root,
 	for (i = 0; i < BTRFS_SUPER_MIRROR_MAX; i++) {
 		bytenr = btrfs_sb_offset(i);
 		ret = btrfs_rmap_block(root->fs_info,
-				       cache->key.objectid, bytenr, 0,
+				       cache->key.objectid, bytenr,
 				       &logical, &nr, &stripe_len);
 		if (ret)
 			return ret;
diff --git a/extent-tree.c b/extent-tree.c
index 391f0a784710..1b8a4f8cb1c3 100644
--- a/extent-tree.c
+++ b/extent-tree.c
@@ -75,7 +75,7 @@ static int remove_sb_from_cache(struct btrfs_root *root,
 	free_space_cache = &fs_info->free_space_cache;
 	for (i = 0; i < BTRFS_SUPER_MIRROR_MAX; i++) {
 		bytenr = btrfs_sb_offset(i);
-		ret = btrfs_rmap_block(fs_info, cache->key.objectid, bytenr, 0,
+		ret = btrfs_rmap_block(fs_info, cache->key.objectid, bytenr,
 				       &logical, &nr, &stripe_len);
 		BUG_ON(ret);
 		while (nr--) {
@@ -699,7 +699,7 @@ static noinline int lookup_extent_data_ref(struct btrfs_trans_handle *trans,
 		if (key.objectid != bytenr ||
 		    key.type != BTRFS_EXTENT_DATA_REF_KEY)
 			goto fail;
-		
+
 		ref = btrfs_item_ptr(leaf, path->slots[0],
 				     struct btrfs_extent_data_ref);
 
@@ -1403,7 +1403,7 @@ int btrfs_inc_extent_ref(struct btrfs_trans_handle *trans,
 		err = ret;
 		goto out;
 	}
-	
+
 	leaf = path->nodes[0];
 	item = btrfs_item_ptr(leaf, path->slots[0], struct btrfs_extent_item);
 	refs = btrfs_extent_refs(leaf, item);
@@ -1666,7 +1666,7 @@ static int __btrfs_mod_ref(struct btrfs_trans_handle *trans,
 			bytenr = btrfs_file_extent_disk_bytenr(buf, fi);
 			if (bytenr == 0)
 				continue;
-			
+
 			num_bytes = btrfs_file_extent_disk_num_bytes(buf, fi);
 			key.offset -= btrfs_file_extent_offset(buf, fi);
 			ret = process_func(trans, root, bytenr, num_bytes,
@@ -4132,7 +4132,7 @@ int exclude_super_stripes(struct btrfs_root *root,
 		bytenr = btrfs_sb_offset(i);
 		ret = btrfs_rmap_block(root->fs_info,
 				       cache->key.objectid, bytenr,
-				       0, &logical, &nr, &stripe_len);
+				       &logical, &nr, &stripe_len);
 		if (ret)
 			return ret;
 
diff --git a/volumes.c b/volumes.c
index c6e34321ff24..ad3016dcdb20 100644
--- a/volumes.c
+++ b/volumes.c
@@ -1364,9 +1364,8 @@ int btrfs_next_bg(struct btrfs_fs_info *fs_info, u64 *logical,
 	return -ENOENT;
 }
 
-int btrfs_rmap_block(struct btrfs_fs_info *fs_info,
-		     u64 chunk_start, u64 physical, u64 devid,
-		     u64 **logical, int *naddrs, int *stripe_len)
+int btrfs_rmap_block(struct btrfs_fs_info *fs_info, u64 chunk_start,
+		     u64 physical, u64 **logical, int *naddrs, int *stripe_len)
 {
 	struct btrfs_mapping_tree *map_tree = &fs_info->mapping_tree;
 	struct cache_extent *ce;
@@ -1397,8 +1396,6 @@ int btrfs_rmap_block(struct btrfs_fs_info *fs_info,
 	buf = kzalloc(sizeof(u64) * map->num_stripes, GFP_NOFS);
 
 	for (i = 0; i < map->num_stripes; i++) {
-		if (devid && map->stripes[i].dev->devid != devid)
-			continue;
 		if (map->stripes[i].physical > physical ||
 		    map->stripes[i].physical + length <= physical)
 			continue;
diff --git a/volumes.h b/volumes.h
index 5bcaae7f03d2..b4ea93f0bec3 100644
--- a/volumes.h
+++ b/volumes.h
@@ -254,8 +254,8 @@ static inline int btrfs_next_bg_system(struct btrfs_fs_info *fs_info,
 			BTRFS_BLOCK_GROUP_SYSTEM);
 }
 int btrfs_rmap_block(struct btrfs_fs_info *fs_info,
-		     u64 chunk_start, u64 physical, u64 devid,
-		     u64 **logical, int *naddrs, int *stripe_len);
+		     u64 chunk_start, u64 physical, u64 **logical,
+		     int *naddrs, int *stripe_len);
 int btrfs_read_sys_array(struct btrfs_fs_info *fs_info);
 int btrfs_read_chunk_tree(struct btrfs_fs_info *fs_info);
 int btrfs_alloc_chunk(struct btrfs_trans_handle *trans,
-- 
2.7.4

[PATCH 2/3] btrfs-progs: Remove add_cache_extent2

[Nikolay Borisov]

Commit 17793e3e6a49 ("Btrfs-progs: extend the extent cache for the
device extent") extended the cache extent APIs to support objectid to
distinguish between phyisical extents with same dimensions but on
different devices. However, it seems that this particular function is
not used to allocate a device extent with accompanying objectid.
Instead such extents as processed solely by insert_device_extent_record.
Remove the unused code as a result.

Signed-off-by: Nikolay Borisov <nborisov@suse.com>
---
 extent-cache.c | 16 ++--------------
 extent-cache.h |  2 --
 2 files changed, 2 insertions(+), 16 deletions(-)

diff --git a/extent-cache.c b/extent-cache.c
index 38bed8b5db70..662c801e6760 100644
--- a/extent-cache.c
+++ b/extent-cache.c
@@ -107,10 +107,9 @@ alloc_cache_extent(u64 objectid, u64 start, u64 size)
 	return pe;
 }
 
-static int __add_cache_extent(struct cache_tree *tree,
-			      u64 objectid, u64 start, u64 size)
+int add_cache_extent(struct cache_tree *tree, u64 start, u64 size)
 {
-	struct cache_extent *pe = alloc_cache_extent(objectid, start, size);
+	struct cache_extent *pe = alloc_cache_extent(0, start, size);
 	int ret;
 
 	if (!pe) {
@@ -125,17 +124,6 @@ static int __add_cache_extent(struct cache_tree *tree,
 	return ret;
 }
 
-int add_cache_extent(struct cache_tree *tree, u64 start, u64 size)
-{
-	return __add_cache_extent(tree, 0, start, size);
-}
-
-int add_cache_extent2(struct cache_tree *tree,
-		      u64 objectid, u64 start, u64 size)
-{
-	return __add_cache_extent(tree, objectid, start, size);
-}
-
 int insert_cache_extent(struct cache_tree *tree, struct cache_extent *pe)
 {
 	return rb_insert(&tree->root, &pe->rb_node, cache_tree_comp_nodes);
diff --git a/extent-cache.h b/extent-cache.h
index 82db7fa3e463..33d018dd5fef 100644
--- a/extent-cache.h
+++ b/extent-cache.h
@@ -106,8 +106,6 @@ struct cache_extent *search_cache_extent2(struct cache_tree *tree,
  */
 struct cache_extent *lookup_cache_extent2(struct cache_tree *tree,
 					  u64 objectid, u64 start, u64 size);
-int add_cache_extent2(struct cache_tree *tree,
-		      u64 objectid, u64 start, u64 size);
 int insert_cache_extent2(struct cache_tree *tree, struct cache_extent *pe);
 
 /*
-- 
2.7.4

[PATCH 3/3] btrfs-progs: Remove objectid argument from alloc_cache_extent

[Nikolay Borisov]

This function is always called with objectid set to 0. So remove the
parameter and statically set the ->objectid to 0 when allocating a new
extent.

Signed-off-by: Nikolay Borisov <nborisov@suse.com>
---
 extent-cache.c | 7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/extent-cache.c b/extent-cache.c
index 662c801e6760..f458de261293 100644
--- a/extent-cache.c
+++ b/extent-cache.c
@@ -93,15 +93,14 @@ void cache_tree_init(struct cache_tree *tree)
 	tree->root = RB_ROOT;
 }
 
-static struct cache_extent *
-alloc_cache_extent(u64 objectid, u64 start, u64 size)
+static struct cache_extent *alloc_cache_extent(u64 start, u64 size)
 {
 	struct cache_extent *pe = malloc(sizeof(*pe));
 
 	if (!pe)
 		return pe;
 
-	pe->objectid = objectid;
+	pe->objectid = 0;
 	pe->start = start;
 	pe->size = size;
 	return pe;
@@ -109,7 +108,7 @@ alloc_cache_extent(u64 objectid, u64 start, u64 size)
 
 int add_cache_extent(struct cache_tree *tree, u64 start, u64 size)
 {
-	struct cache_extent *pe = alloc_cache_extent(0, start, size);
+	struct cache_extent *pe = alloc_cache_extent(start, size);
 	int ret;
 
 	if (!pe) {
-- 
2.7.4

Re: [PATCH 0/3] Misc cleanups to cached extent

[Qu Wenruo]

On 2018年05月04日 15:47, Nikolay Borisov wrote:
> Here are a couple of cleanups I stumbled upon while looking at the freespace 
> validation code. The first one simplifies btrfs_rmap_block that has an unused 
> parameter. The next 2 patches cleanup the cache_extent apis since they provide
> more than we are actually using (or have ever used).

Under all cases, only insert_cache_extent2() is used, so removing
add_cache_extent2() is completely fine.

> Nikolay Borisov (3):
>   btrfs-progs: Remove devid parameter from btrfs_rmap_block
>   btrfs-progs: Remove add_cache_extent2
>   btrfs-progs: Remove objectid argument from alloc_cache_extent

All of them looks good.

Reviewed-by: Qu Wenruo <wqu@suse.com>

Thanks,
Qu

> 
>  check/main.c   |  2 +-
>  extent-cache.c | 21 ++++-----------------
>  extent-cache.h |  2 --
>  extent-tree.c  | 10 +++++-----
>  volumes.c      |  7 ++-----
>  volumes.h      |  4 ++--
>  6 files changed, 14 insertions(+), 32 deletions(-)
> 

Re: [PATCH 0/3] Misc cleanups to cached extent

[David Sterba]

On Fri, May 04, 2018 at 10:47:24AM +0300, Nikolay Borisov wrote:
> Here are a couple of cleanups I stumbled upon while looking at the freespace 
> validation code. The first one simplifies btrfs_rmap_block that has an unused 
> parameter. The next 2 patches cleanup the cache_extent apis since they provide
> more than we are actually using (or have ever used).
> 
> Nikolay Borisov (3):
>   btrfs-progs: Remove devid parameter from btrfs_rmap_block
>   btrfs-progs: Remove add_cache_extent2
>   btrfs-progs: Remove objectid argument from alloc_cache_extent

Applied, thanks.