* [PATCH 01/16] btrfs: remove pointless out label from add_new_free_space_info()
2025-06-17 16:12 [PATCH 00/16] btrfs: free space tree optimization and cleanups fdmanana
@ 2025-06-17 16:12 ` fdmanana
2025-06-17 16:12 ` [PATCH 02/16] btrfs: remove pointless out label from update_free_space_extent_count() fdmanana
` (15 subsequent siblings)
16 siblings, 0 replies; 22+ messages in thread
From: fdmanana @ 2025-06-17 16:12 UTC (permalink / raw)
To: linux-btrfs
From: Filipe Manana <fdmanana@suse.com>
We can just return directly if btrfs_insert_empty_item() fails, there is
no need to release the path before returning, as all callers (or upper
in the call chain) will free the path if they get an error from the call
to add_new_free_space_info(), which is also a common pattern everywhere
in btrfs. Finally there's no need to set 'ret' to 0 if the call to
btrfs_insert_empty_item() didn't fail, since 'ret' is already 0.
Signed-off-by: Filipe Manana <fdmanana@suse.com>
---
fs/btrfs/free-space-tree.c | 7 ++-----
1 file changed, 2 insertions(+), 5 deletions(-)
diff --git a/fs/btrfs/free-space-tree.c b/fs/btrfs/free-space-tree.c
index f03f3610b713..6418b3b5d16a 100644
--- a/fs/btrfs/free-space-tree.c
+++ b/fs/btrfs/free-space-tree.c
@@ -82,18 +82,15 @@ static int add_new_free_space_info(struct btrfs_trans_handle *trans,
ret = btrfs_insert_empty_item(trans, root, path, &key, sizeof(*info));
if (ret)
- goto out;
+ return ret;
leaf = path->nodes[0];
info = btrfs_item_ptr(leaf, path->slots[0],
struct btrfs_free_space_info);
btrfs_set_free_space_extent_count(leaf, info, 0);
btrfs_set_free_space_flags(leaf, info, 0);
-
- ret = 0;
-out:
btrfs_release_path(path);
- return ret;
+ return 0;
}
EXPORT_FOR_TESTS
--
2.47.2
^ permalink raw reply related [flat|nested] 22+ messages in thread* [PATCH 02/16] btrfs: remove pointless out label from update_free_space_extent_count()
2025-06-17 16:12 [PATCH 00/16] btrfs: free space tree optimization and cleanups fdmanana
2025-06-17 16:12 ` [PATCH 01/16] btrfs: remove pointless out label from add_new_free_space_info() fdmanana
@ 2025-06-17 16:12 ` fdmanana
2025-06-17 16:12 ` [PATCH 03/16] btrfs: make extent_buffer_test_bit() return a boolean instead fdmanana
` (14 subsequent siblings)
16 siblings, 0 replies; 22+ messages in thread
From: fdmanana @ 2025-06-17 16:12 UTC (permalink / raw)
To: linux-btrfs
From: Filipe Manana <fdmanana@suse.com>
Just return directly, we don't need the label since all we do under it is
to return.
Signed-off-by: Filipe Manana <fdmanana@suse.com>
---
fs/btrfs/free-space-tree.c | 8 +++-----
1 file changed, 3 insertions(+), 5 deletions(-)
diff --git a/fs/btrfs/free-space-tree.c b/fs/btrfs/free-space-tree.c
index 6418b3b5d16a..29fada10158c 100644
--- a/fs/btrfs/free-space-tree.c
+++ b/fs/btrfs/free-space-tree.c
@@ -491,10 +491,9 @@ static int update_free_space_extent_count(struct btrfs_trans_handle *trans,
return 0;
info = search_free_space_info(trans, block_group, path, 1);
- if (IS_ERR(info)) {
- ret = PTR_ERR(info);
- goto out;
- }
+ if (IS_ERR(info))
+ return PTR_ERR(info);
+
flags = btrfs_free_space_flags(path->nodes[0], info);
extent_count = btrfs_free_space_extent_count(path->nodes[0], info);
@@ -510,7 +509,6 @@ static int update_free_space_extent_count(struct btrfs_trans_handle *trans,
ret = convert_free_space_to_extents(trans, block_group, path);
}
-out:
return ret;
}
--
2.47.2
^ permalink raw reply related [flat|nested] 22+ messages in thread* [PATCH 03/16] btrfs: make extent_buffer_test_bit() return a boolean instead
2025-06-17 16:12 [PATCH 00/16] btrfs: free space tree optimization and cleanups fdmanana
2025-06-17 16:12 ` [PATCH 01/16] btrfs: remove pointless out label from add_new_free_space_info() fdmanana
2025-06-17 16:12 ` [PATCH 02/16] btrfs: remove pointless out label from update_free_space_extent_count() fdmanana
@ 2025-06-17 16:12 ` fdmanana
2025-06-17 16:12 ` [PATCH 04/16] btrfs: make free_space_test_bit() " fdmanana
` (13 subsequent siblings)
16 siblings, 0 replies; 22+ messages in thread
From: fdmanana @ 2025-06-17 16:12 UTC (permalink / raw)
To: linux-btrfs
From: Filipe Manana <fdmanana@suse.com>
All the callers want is to determine if a bit is set and all of them call
the function and do a double negation (!!) on its result to get a boolean.
So change it to return a boolean and simplify callers.
Signed-off-by: Filipe Manana <fdmanana@suse.com>
---
fs/btrfs/extent_io.c | 4 ++--
fs/btrfs/extent_io.h | 4 ++--
fs/btrfs/free-space-tree.c | 2 +-
fs/btrfs/tests/extent-io-tests.c | 14 +++++++-------
4 files changed, 12 insertions(+), 12 deletions(-)
diff --git a/fs/btrfs/extent_io.c b/fs/btrfs/extent_io.c
index e9ba80a56172..c7811fe493f3 100644
--- a/fs/btrfs/extent_io.c
+++ b/fs/btrfs/extent_io.c
@@ -4108,8 +4108,8 @@ static inline void eb_bitmap_offset(const struct extent_buffer *eb,
* @start: offset of the bitmap item in the extent buffer
* @nr: bit number to test
*/
-int extent_buffer_test_bit(const struct extent_buffer *eb, unsigned long start,
- unsigned long nr)
+bool extent_buffer_test_bit(const struct extent_buffer *eb, unsigned long start,
+ unsigned long nr)
{
unsigned long i;
size_t offset;
diff --git a/fs/btrfs/extent_io.h b/fs/btrfs/extent_io.h
index 65bb87f1dce6..61130786b9a3 100644
--- a/fs/btrfs/extent_io.h
+++ b/fs/btrfs/extent_io.h
@@ -345,8 +345,8 @@ void memmove_extent_buffer(const struct extent_buffer *dst,
unsigned long len);
void memzero_extent_buffer(const struct extent_buffer *eb, unsigned long start,
unsigned long len);
-int extent_buffer_test_bit(const struct extent_buffer *eb, unsigned long start,
- unsigned long pos);
+bool extent_buffer_test_bit(const struct extent_buffer *eb, unsigned long start,
+ unsigned long pos);
void extent_buffer_bitmap_set(const struct extent_buffer *eb, unsigned long start,
unsigned long pos, unsigned long len);
void extent_buffer_bitmap_clear(const struct extent_buffer *eb,
diff --git a/fs/btrfs/free-space-tree.c b/fs/btrfs/free-space-tree.c
index 29fada10158c..b24c23312892 100644
--- a/fs/btrfs/free-space-tree.c
+++ b/fs/btrfs/free-space-tree.c
@@ -532,7 +532,7 @@ int free_space_test_bit(struct btrfs_block_group *block_group,
ptr = btrfs_item_ptr_offset(leaf, path->slots[0]);
i = div_u64(offset - found_start,
block_group->fs_info->sectorsize);
- return !!extent_buffer_test_bit(leaf, ptr, i);
+ return extent_buffer_test_bit(leaf, ptr, i);
}
static void free_space_set_bits(struct btrfs_trans_handle *trans,
diff --git a/fs/btrfs/tests/extent-io-tests.c b/fs/btrfs/tests/extent-io-tests.c
index 00da54f0164c..d2a6f769fd76 100644
--- a/fs/btrfs/tests/extent-io-tests.c
+++ b/fs/btrfs/tests/extent-io-tests.c
@@ -343,11 +343,11 @@ static int check_eb_bitmap(unsigned long *bitmap, struct extent_buffer *eb)
unsigned long i;
for (i = 0; i < eb->len * BITS_PER_BYTE; i++) {
- int bit, bit1;
+ bool bit_set, bit1_set;
- bit = !!test_bit(i, bitmap);
- bit1 = !!extent_buffer_test_bit(eb, 0, i);
- if (bit1 != bit) {
+ bit_set = test_bit(i, bitmap);
+ bit1_set = extent_buffer_test_bit(eb, 0, i);
+ if (bit1_set != bit_set) {
u8 has;
u8 expect;
@@ -360,9 +360,9 @@ static int check_eb_bitmap(unsigned long *bitmap, struct extent_buffer *eb)
return -EINVAL;
}
- bit1 = !!extent_buffer_test_bit(eb, i / BITS_PER_BYTE,
- i % BITS_PER_BYTE);
- if (bit1 != bit) {
+ bit1_set = extent_buffer_test_bit(eb, i / BITS_PER_BYTE,
+ i % BITS_PER_BYTE);
+ if (bit1_set != bit_set) {
u8 has;
u8 expect;
--
2.47.2
^ permalink raw reply related [flat|nested] 22+ messages in thread* [PATCH 04/16] btrfs: make free_space_test_bit() return a boolean instead
2025-06-17 16:12 [PATCH 00/16] btrfs: free space tree optimization and cleanups fdmanana
` (2 preceding siblings ...)
2025-06-17 16:12 ` [PATCH 03/16] btrfs: make extent_buffer_test_bit() return a boolean instead fdmanana
@ 2025-06-17 16:12 ` fdmanana
2025-06-17 16:13 ` [PATCH 05/16] btrfs: remove pointless out label from modify_free_space_bitmap() fdmanana
` (12 subsequent siblings)
16 siblings, 0 replies; 22+ messages in thread
From: fdmanana @ 2025-06-17 16:12 UTC (permalink / raw)
To: linux-btrfs
From: Filipe Manana <fdmanana@suse.com>
The function returns the result of another function that returns a boolean
(extent_buffer_test_bit()), and all the callers need is a boolean an not
an integer. So change its return type from int to bool, and modify the
callers to store results in booleans instead of integers, which also makes
them simpler.
Signed-off-by: Filipe Manana <fdmanana@suse.com>
---
fs/btrfs/free-space-tree.c | 37 ++++++++++++++++++-------------------
fs/btrfs/free-space-tree.h | 4 ++--
2 files changed, 20 insertions(+), 21 deletions(-)
diff --git a/fs/btrfs/free-space-tree.c b/fs/btrfs/free-space-tree.c
index b24c23312892..4cd1f46cd694 100644
--- a/fs/btrfs/free-space-tree.c
+++ b/fs/btrfs/free-space-tree.c
@@ -513,8 +513,8 @@ static int update_free_space_extent_count(struct btrfs_trans_handle *trans,
}
EXPORT_FOR_TESTS
-int free_space_test_bit(struct btrfs_block_group *block_group,
- struct btrfs_path *path, u64 offset)
+bool free_space_test_bit(struct btrfs_block_group *block_group,
+ struct btrfs_path *path, u64 offset)
{
struct extent_buffer *leaf;
struct btrfs_key key;
@@ -612,7 +612,8 @@ static int modify_free_space_bitmap(struct btrfs_trans_handle *trans,
struct btrfs_key key;
u64 end = start + size;
u64 cur_start, cur_size;
- int prev_bit, next_bit;
+ bool prev_bit_set = false;
+ bool next_bit_set = false;
int new_extents;
int ret;
@@ -631,7 +632,7 @@ static int modify_free_space_bitmap(struct btrfs_trans_handle *trans,
if (ret)
goto out;
- prev_bit = free_space_test_bit(block_group, path, prev_block);
+ prev_bit_set = free_space_test_bit(block_group, path, prev_block);
/* The previous block may have been in the previous bitmap. */
btrfs_item_key_to_cpu(path->nodes[0], &key, path->slots[0]);
@@ -648,8 +649,6 @@ static int modify_free_space_bitmap(struct btrfs_trans_handle *trans,
ret = btrfs_search_prev_slot(trans, root, &key, path, 0, 1);
if (ret)
goto out;
-
- prev_bit = -1;
}
/*
@@ -681,28 +680,26 @@ static int modify_free_space_bitmap(struct btrfs_trans_handle *trans,
goto out;
}
- next_bit = free_space_test_bit(block_group, path, end);
- } else {
- next_bit = -1;
+ next_bit_set = free_space_test_bit(block_group, path, end);
}
if (remove) {
new_extents = -1;
- if (prev_bit == 1) {
+ if (prev_bit_set) {
/* Leftover on the left. */
new_extents++;
}
- if (next_bit == 1) {
+ if (next_bit_set) {
/* Leftover on the right. */
new_extents++;
}
} else {
new_extents = 1;
- if (prev_bit == 1) {
+ if (prev_bit_set) {
/* Merging with neighbor on the left. */
new_extents--;
}
- if (next_bit == 1) {
+ if (next_bit_set) {
/* Merging with neighbor on the right. */
new_extents--;
}
@@ -1552,7 +1549,7 @@ static int load_free_space_bitmaps(struct btrfs_caching_control *caching_ctl,
struct btrfs_fs_info *fs_info;
struct btrfs_root *root;
struct btrfs_key key;
- int prev_bit = 0, bit;
+ bool prev_bit_set = false;
/* Initialize to silence GCC. */
u64 extent_start = 0;
u64 end, offset;
@@ -1583,10 +1580,12 @@ static int load_free_space_bitmaps(struct btrfs_caching_control *caching_ctl,
offset = key.objectid;
while (offset < key.objectid + key.offset) {
- bit = free_space_test_bit(block_group, path, offset);
- if (prev_bit == 0 && bit == 1) {
+ bool bit_set;
+
+ bit_set = free_space_test_bit(block_group, path, offset);
+ if (!prev_bit_set && bit_set) {
extent_start = offset;
- } else if (prev_bit == 1 && bit == 0) {
+ } else if (prev_bit_set && !bit_set) {
u64 space_added;
ret = btrfs_add_new_free_space(block_group,
@@ -1602,11 +1601,11 @@ static int load_free_space_bitmaps(struct btrfs_caching_control *caching_ctl,
}
extent_count++;
}
- prev_bit = bit;
+ prev_bit_set = bit_set;
offset += fs_info->sectorsize;
}
}
- if (prev_bit == 1) {
+ if (prev_bit_set) {
ret = btrfs_add_new_free_space(block_group, extent_start, end, NULL);
if (ret)
goto out;
diff --git a/fs/btrfs/free-space-tree.h b/fs/btrfs/free-space-tree.h
index e6c6d6f4f221..32e71d0c8dd4 100644
--- a/fs/btrfs/free-space-tree.h
+++ b/fs/btrfs/free-space-tree.h
@@ -53,8 +53,8 @@ int convert_free_space_to_bitmaps(struct btrfs_trans_handle *trans,
int convert_free_space_to_extents(struct btrfs_trans_handle *trans,
struct btrfs_block_group *block_group,
struct btrfs_path *path);
-int free_space_test_bit(struct btrfs_block_group *block_group,
- struct btrfs_path *path, u64 offset);
+bool free_space_test_bit(struct btrfs_block_group *block_group,
+ struct btrfs_path *path, u64 offset);
#endif
#endif
--
2.47.2
^ permalink raw reply related [flat|nested] 22+ messages in thread* [PATCH 05/16] btrfs: remove pointless out label from modify_free_space_bitmap()
2025-06-17 16:12 [PATCH 00/16] btrfs: free space tree optimization and cleanups fdmanana
` (3 preceding siblings ...)
2025-06-17 16:12 ` [PATCH 04/16] btrfs: make free_space_test_bit() " fdmanana
@ 2025-06-17 16:13 ` fdmanana
2025-06-17 16:13 ` [PATCH 06/16] btrfs: remove pointless out label from remove_free_space_extent() fdmanana
` (11 subsequent siblings)
16 siblings, 0 replies; 22+ messages in thread
From: fdmanana @ 2025-06-17 16:13 UTC (permalink / raw)
To: linux-btrfs
From: Filipe Manana <fdmanana@suse.com>
All we do under the label is to return, so there's no point in having it,
just return directly whenever we get an error.
Signed-off-by: Filipe Manana <fdmanana@suse.com>
---
fs/btrfs/free-space-tree.c | 16 ++++++----------
1 file changed, 6 insertions(+), 10 deletions(-)
diff --git a/fs/btrfs/free-space-tree.c b/fs/btrfs/free-space-tree.c
index 4cd1f46cd694..a4909393840a 100644
--- a/fs/btrfs/free-space-tree.c
+++ b/fs/btrfs/free-space-tree.c
@@ -630,7 +630,7 @@ static int modify_free_space_bitmap(struct btrfs_trans_handle *trans,
ret = btrfs_search_prev_slot(trans, root, &key, path, 0, 1);
if (ret)
- goto out;
+ return ret;
prev_bit_set = free_space_test_bit(block_group, path, prev_block);
@@ -639,7 +639,7 @@ static int modify_free_space_bitmap(struct btrfs_trans_handle *trans,
if (start >= key.objectid + key.offset) {
ret = free_space_next_bitmap(trans, root, path);
if (ret)
- goto out;
+ return ret;
}
} else {
key.objectid = start;
@@ -648,7 +648,7 @@ static int modify_free_space_bitmap(struct btrfs_trans_handle *trans,
ret = btrfs_search_prev_slot(trans, root, &key, path, 0, 1);
if (ret)
- goto out;
+ return ret;
}
/*
@@ -664,7 +664,7 @@ static int modify_free_space_bitmap(struct btrfs_trans_handle *trans,
break;
ret = free_space_next_bitmap(trans, root, path);
if (ret)
- goto out;
+ return ret;
}
/*
@@ -677,7 +677,7 @@ static int modify_free_space_bitmap(struct btrfs_trans_handle *trans,
if (end >= key.objectid + key.offset) {
ret = free_space_next_bitmap(trans, root, path);
if (ret)
- goto out;
+ return ret;
}
next_bit_set = free_space_test_bit(block_group, path, end);
@@ -706,11 +706,7 @@ static int modify_free_space_bitmap(struct btrfs_trans_handle *trans,
}
btrfs_release_path(path);
- ret = update_free_space_extent_count(trans, block_group, path,
- new_extents);
-
-out:
- return ret;
+ return update_free_space_extent_count(trans, block_group, path, new_extents);
}
static int remove_free_space_extent(struct btrfs_trans_handle *trans,
--
2.47.2
^ permalink raw reply related [flat|nested] 22+ messages in thread* [PATCH 06/16] btrfs: remove pointless out label from remove_free_space_extent()
2025-06-17 16:12 [PATCH 00/16] btrfs: free space tree optimization and cleanups fdmanana
` (4 preceding siblings ...)
2025-06-17 16:13 ` [PATCH 05/16] btrfs: remove pointless out label from modify_free_space_bitmap() fdmanana
@ 2025-06-17 16:13 ` fdmanana
2025-06-17 16:13 ` [PATCH 07/16] btrfs: remove pointless out label from add_free_space_extent() fdmanana
` (10 subsequent siblings)
16 siblings, 0 replies; 22+ messages in thread
From: fdmanana @ 2025-06-17 16:13 UTC (permalink / raw)
To: linux-btrfs
From: Filipe Manana <fdmanana@suse.com>
All we do under the label is to return, so there's no point in having it,
just return directly whenever we get an error.
Signed-off-by: Filipe Manana <fdmanana@suse.com>
---
fs/btrfs/free-space-tree.c | 14 +++++---------
1 file changed, 5 insertions(+), 9 deletions(-)
diff --git a/fs/btrfs/free-space-tree.c b/fs/btrfs/free-space-tree.c
index a4909393840a..cba097dbdebb 100644
--- a/fs/btrfs/free-space-tree.c
+++ b/fs/btrfs/free-space-tree.c
@@ -727,7 +727,7 @@ static int remove_free_space_extent(struct btrfs_trans_handle *trans,
ret = btrfs_search_prev_slot(trans, root, &key, path, -1, 1);
if (ret)
- goto out;
+ return ret;
btrfs_item_key_to_cpu(path->nodes[0], &key, path->slots[0]);
@@ -759,7 +759,7 @@ static int remove_free_space_extent(struct btrfs_trans_handle *trans,
/* Delete the existing key (cases 1-4). */
ret = btrfs_del_item(trans, root, path);
if (ret)
- goto out;
+ return ret;
/* Add a key for leftovers at the beginning (cases 3 and 4). */
if (start > found_start) {
@@ -770,7 +770,7 @@ static int remove_free_space_extent(struct btrfs_trans_handle *trans,
btrfs_release_path(path);
ret = btrfs_insert_empty_item(trans, root, path, &key, 0);
if (ret)
- goto out;
+ return ret;
new_extents++;
}
@@ -783,16 +783,12 @@ static int remove_free_space_extent(struct btrfs_trans_handle *trans,
btrfs_release_path(path);
ret = btrfs_insert_empty_item(trans, root, path, &key, 0);
if (ret)
- goto out;
+ return ret;
new_extents++;
}
btrfs_release_path(path);
- ret = update_free_space_extent_count(trans, block_group, path,
- new_extents);
-
-out:
- return ret;
+ return update_free_space_extent_count(trans, block_group, path, new_extents);
}
EXPORT_FOR_TESTS
--
2.47.2
^ permalink raw reply related [flat|nested] 22+ messages in thread* [PATCH 07/16] btrfs: remove pointless out label from add_free_space_extent()
2025-06-17 16:12 [PATCH 00/16] btrfs: free space tree optimization and cleanups fdmanana
` (5 preceding siblings ...)
2025-06-17 16:13 ` [PATCH 06/16] btrfs: remove pointless out label from remove_free_space_extent() fdmanana
@ 2025-06-17 16:13 ` fdmanana
2025-06-17 16:13 ` [PATCH 08/16] btrfs: remove pointless out label from load_free_space_bitmaps() fdmanana
` (9 subsequent siblings)
16 siblings, 0 replies; 22+ messages in thread
From: fdmanana @ 2025-06-17 16:13 UTC (permalink / raw)
To: linux-btrfs
From: Filipe Manana <fdmanana@suse.com>
All we do under the label is to return, so there's no point in having it,
just return directly whenever we get an error.
Signed-off-by: Filipe Manana <fdmanana@suse.com>
---
fs/btrfs/free-space-tree.c | 16 ++++++----------
1 file changed, 6 insertions(+), 10 deletions(-)
diff --git a/fs/btrfs/free-space-tree.c b/fs/btrfs/free-space-tree.c
index cba097dbdebb..1f76860ec61e 100644
--- a/fs/btrfs/free-space-tree.c
+++ b/fs/btrfs/free-space-tree.c
@@ -900,7 +900,7 @@ static int add_free_space_extent(struct btrfs_trans_handle *trans,
ret = btrfs_search_prev_slot(trans, root, &key, path, -1, 1);
if (ret)
- goto out;
+ return ret;
btrfs_item_key_to_cpu(path->nodes[0], &key, path->slots[0]);
@@ -923,7 +923,7 @@ static int add_free_space_extent(struct btrfs_trans_handle *trans,
if (found_end == start) {
ret = btrfs_del_item(trans, root, path);
if (ret)
- goto out;
+ return ret;
new_key.objectid = found_start;
new_key.offset += key.offset;
new_extents--;
@@ -940,7 +940,7 @@ static int add_free_space_extent(struct btrfs_trans_handle *trans,
ret = btrfs_search_prev_slot(trans, root, &key, path, -1, 1);
if (ret)
- goto out;
+ return ret;
btrfs_item_key_to_cpu(path->nodes[0], &key, path->slots[0]);
@@ -964,7 +964,7 @@ static int add_free_space_extent(struct btrfs_trans_handle *trans,
if (found_start == end) {
ret = btrfs_del_item(trans, root, path);
if (ret)
- goto out;
+ return ret;
new_key.offset += key.offset;
new_extents--;
}
@@ -974,14 +974,10 @@ static int add_free_space_extent(struct btrfs_trans_handle *trans,
/* Insert the new key (cases 1-4). */
ret = btrfs_insert_empty_item(trans, root, path, &new_key, 0);
if (ret)
- goto out;
+ return ret;
btrfs_release_path(path);
- ret = update_free_space_extent_count(trans, block_group, path,
- new_extents);
-
-out:
- return ret;
+ return update_free_space_extent_count(trans, block_group, path, new_extents);
}
EXPORT_FOR_TESTS
--
2.47.2
^ permalink raw reply related [flat|nested] 22+ messages in thread* [PATCH 08/16] btrfs: remove pointless out label from load_free_space_bitmaps()
2025-06-17 16:12 [PATCH 00/16] btrfs: free space tree optimization and cleanups fdmanana
` (6 preceding siblings ...)
2025-06-17 16:13 ` [PATCH 07/16] btrfs: remove pointless out label from add_free_space_extent() fdmanana
@ 2025-06-17 16:13 ` fdmanana
2025-06-17 16:13 ` [PATCH 09/16] btrfs: remove pointless out label from load_free_space_extents() fdmanana
` (8 subsequent siblings)
16 siblings, 0 replies; 22+ messages in thread
From: fdmanana @ 2025-06-17 16:13 UTC (permalink / raw)
To: linux-btrfs
From: Filipe Manana <fdmanana@suse.com>
All we do under the label is to return, so there's no point in having it,
just return directly whenever we get an error.
Signed-off-by: Filipe Manana <fdmanana@suse.com>
---
fs/btrfs/free-space-tree.c | 13 +++++--------
1 file changed, 5 insertions(+), 8 deletions(-)
diff --git a/fs/btrfs/free-space-tree.c b/fs/btrfs/free-space-tree.c
index 1f76860ec61e..0514b0a04572 100644
--- a/fs/btrfs/free-space-tree.c
+++ b/fs/btrfs/free-space-tree.c
@@ -1554,7 +1554,7 @@ static int load_free_space_bitmaps(struct btrfs_caching_control *caching_ctl,
while (1) {
ret = btrfs_next_item(root, path);
if (ret < 0)
- goto out;
+ return ret;
if (ret)
break;
@@ -1581,7 +1581,7 @@ static int load_free_space_bitmaps(struct btrfs_caching_control *caching_ctl,
offset,
&space_added);
if (ret)
- goto out;
+ return ret;
total_found += space_added;
if (total_found > CACHING_CTL_WAKE_UP) {
total_found = 0;
@@ -1596,7 +1596,7 @@ static int load_free_space_bitmaps(struct btrfs_caching_control *caching_ctl,
if (prev_bit_set) {
ret = btrfs_add_new_free_space(block_group, extent_start, end, NULL);
if (ret)
- goto out;
+ return ret;
extent_count++;
}
@@ -1606,13 +1606,10 @@ static int load_free_space_bitmaps(struct btrfs_caching_control *caching_ctl,
block_group->start, extent_count,
expected_extent_count);
DEBUG_WARN();
- ret = -EIO;
- goto out;
+ return -EIO;
}
- ret = 0;
-out:
- return ret;
+ return 0;
}
static int load_free_space_extents(struct btrfs_caching_control *caching_ctl,
--
2.47.2
^ permalink raw reply related [flat|nested] 22+ messages in thread* [PATCH 09/16] btrfs: remove pointless out label from load_free_space_extents()
2025-06-17 16:12 [PATCH 00/16] btrfs: free space tree optimization and cleanups fdmanana
` (7 preceding siblings ...)
2025-06-17 16:13 ` [PATCH 08/16] btrfs: remove pointless out label from load_free_space_bitmaps() fdmanana
@ 2025-06-17 16:13 ` fdmanana
2025-06-17 16:13 ` [PATCH 10/16] btrfs: add btrfs prefix to free space tree exported functions fdmanana
` (7 subsequent siblings)
16 siblings, 0 replies; 22+ messages in thread
From: fdmanana @ 2025-06-17 16:13 UTC (permalink / raw)
To: linux-btrfs
From: Filipe Manana <fdmanana@suse.com>
All we do under the label is to return, so there's no point in having it,
just return directly whenever we get an error.
Signed-off-by: Filipe Manana <fdmanana@suse.com>
---
fs/btrfs/free-space-tree.c | 11 ++++-------
1 file changed, 4 insertions(+), 7 deletions(-)
diff --git a/fs/btrfs/free-space-tree.c b/fs/btrfs/free-space-tree.c
index 0514b0a04572..d98a927ca079 100644
--- a/fs/btrfs/free-space-tree.c
+++ b/fs/btrfs/free-space-tree.c
@@ -1636,7 +1636,7 @@ static int load_free_space_extents(struct btrfs_caching_control *caching_ctl,
ret = btrfs_next_item(root, path);
if (ret < 0)
- goto out;
+ return ret;
if (ret)
break;
@@ -1652,7 +1652,7 @@ static int load_free_space_extents(struct btrfs_caching_control *caching_ctl,
key.objectid + key.offset,
&space_added);
if (ret)
- goto out;
+ return ret;
total_found += space_added;
if (total_found > CACHING_CTL_WAKE_UP) {
total_found = 0;
@@ -1667,13 +1667,10 @@ static int load_free_space_extents(struct btrfs_caching_control *caching_ctl,
block_group->start, extent_count,
expected_extent_count);
DEBUG_WARN();
- ret = -EIO;
- goto out;
+ return -EIO;
}
- ret = 0;
-out:
- return ret;
+ return 0;
}
int load_free_space_tree(struct btrfs_caching_control *caching_ctl)
--
2.47.2
^ permalink raw reply related [flat|nested] 22+ messages in thread* [PATCH 10/16] btrfs: add btrfs prefix to free space tree exported functions
2025-06-17 16:12 [PATCH 00/16] btrfs: free space tree optimization and cleanups fdmanana
` (8 preceding siblings ...)
2025-06-17 16:13 ` [PATCH 09/16] btrfs: remove pointless out label from load_free_space_extents() fdmanana
@ 2025-06-17 16:13 ` fdmanana
2025-06-17 16:13 ` [PATCH 11/16] btrfs: rename free_space_set_bits() and make it less confusing fdmanana
` (6 subsequent siblings)
16 siblings, 0 replies; 22+ messages in thread
From: fdmanana @ 2025-06-17 16:13 UTC (permalink / raw)
To: linux-btrfs
From: Filipe Manana <fdmanana@suse.com>
A few of the free space tree exported functions have a 'btrfs_' prefix in
their name, but most don't. Not only is this inconsistent, the preferred
style is to have such a prefix, to avoid potential collisions in the
future with other kernel code and offer a somewhat better readibility by
making it obvious in calls sites that we are calling btrfs specific code.
So add the 'btrfs_' prefix to all free space tree functions that are
missing it.
Signed-off-by: Filipe Manana <fdmanana@suse.com>
---
fs/btrfs/block-group.c | 10 +--
fs/btrfs/extent-tree.c | 4 +-
fs/btrfs/free-space-tree.c | 95 +++++++++++++-------------
fs/btrfs/free-space-tree.h | 52 +++++++-------
fs/btrfs/tests/free-space-tree-tests.c | 93 ++++++++++++-------------
5 files changed, 125 insertions(+), 129 deletions(-)
diff --git a/fs/btrfs/block-group.c b/fs/btrfs/block-group.c
index 5b0cb04b2b93..00e567a4cd16 100644
--- a/fs/btrfs/block-group.c
+++ b/fs/btrfs/block-group.c
@@ -877,7 +877,7 @@ static noinline void caching_thread(struct btrfs_work *work)
*/
if (btrfs_fs_compat_ro(fs_info, FREE_SPACE_TREE) &&
!(test_bit(BTRFS_FS_FREE_SPACE_TREE_UNTRUSTED, &fs_info->flags)))
- ret = load_free_space_tree(caching_ctl);
+ ret = btrfs_load_free_space_tree(caching_ctl);
else
ret = load_extent_tree_free(caching_ctl);
done:
@@ -1235,7 +1235,7 @@ int btrfs_remove_block_group(struct btrfs_trans_handle *trans,
* another task to attempt to create another block group with the same
* item key (and failing with -EEXIST and a transaction abort).
*/
- ret = remove_block_group_free_space(trans, block_group);
+ ret = btrfs_remove_block_group_free_space(trans, block_group);
if (ret)
goto out;
@@ -2372,7 +2372,7 @@ static int read_one_block_group(struct btrfs_fs_info *info,
cache->global_root_id = btrfs_stack_block_group_chunk_objectid(bgi);
cache->space_info = btrfs_find_space_info(info, cache->flags);
- set_free_space_tree_thresholds(cache);
+ btrfs_set_free_space_tree_thresholds(cache);
if (need_clear) {
/*
@@ -2791,7 +2791,7 @@ void btrfs_create_pending_block_groups(struct btrfs_trans_handle *trans)
block_group->length);
if (ret)
btrfs_abort_transaction(trans, ret);
- add_block_group_free_space(trans, block_group);
+ btrfs_add_block_group_free_space(trans, block_group);
/*
* If we restriped during balance, we may have added a new raid
@@ -2889,7 +2889,7 @@ struct btrfs_block_group *btrfs_make_block_group(struct btrfs_trans_handle *tran
set_bit(BLOCK_GROUP_FLAG_NEW, &cache->runtime_flags);
cache->length = size;
- set_free_space_tree_thresholds(cache);
+ btrfs_set_free_space_tree_thresholds(cache);
cache->flags = type;
cache->cached = BTRFS_CACHE_FINISHED;
cache->global_root_id = calculate_global_root_id(fs_info, cache->start);
diff --git a/fs/btrfs/extent-tree.c b/fs/btrfs/extent-tree.c
index f1ac6a8dd9f4..0c57aaa0f5f0 100644
--- a/fs/btrfs/extent-tree.c
+++ b/fs/btrfs/extent-tree.c
@@ -3000,7 +3000,7 @@ static int do_free_extent_accounting(struct btrfs_trans_handle *trans,
return ret;
}
- ret = add_to_free_space_tree(trans, bytenr, num_bytes);
+ ret = btrfs_add_to_free_space_tree(trans, bytenr, num_bytes);
if (ret) {
btrfs_abort_transaction(trans, ret);
return ret;
@@ -4784,7 +4784,7 @@ static int alloc_reserved_extent(struct btrfs_trans_handle *trans, u64 bytenr,
struct btrfs_fs_info *fs_info = trans->fs_info;
int ret;
- ret = remove_from_free_space_tree(trans, bytenr, num_bytes);
+ ret = btrfs_remove_from_free_space_tree(trans, bytenr, num_bytes);
if (ret)
return ret;
diff --git a/fs/btrfs/free-space-tree.c b/fs/btrfs/free-space-tree.c
index d98a927ca079..1794fdf06586 100644
--- a/fs/btrfs/free-space-tree.c
+++ b/fs/btrfs/free-space-tree.c
@@ -35,7 +35,7 @@ static struct btrfs_root *btrfs_free_space_root(
return btrfs_global_root(block_group->fs_info, &key);
}
-void set_free_space_tree_thresholds(struct btrfs_block_group *cache)
+void btrfs_set_free_space_tree_thresholds(struct btrfs_block_group *cache)
{
u32 bitmap_range;
size_t bitmap_size;
@@ -94,7 +94,7 @@ static int add_new_free_space_info(struct btrfs_trans_handle *trans,
}
EXPORT_FOR_TESTS
-struct btrfs_free_space_info *search_free_space_info(
+struct btrfs_free_space_info *btrfs_search_free_space_info(
struct btrfs_trans_handle *trans,
struct btrfs_block_group *block_group,
struct btrfs_path *path, int cow)
@@ -198,9 +198,9 @@ static void le_bitmap_set(unsigned long *map, unsigned int start, int len)
}
EXPORT_FOR_TESTS
-int convert_free_space_to_bitmaps(struct btrfs_trans_handle *trans,
- struct btrfs_block_group *block_group,
- struct btrfs_path *path)
+int btrfs_convert_free_space_to_bitmaps(struct btrfs_trans_handle *trans,
+ struct btrfs_block_group *block_group,
+ struct btrfs_path *path)
{
struct btrfs_fs_info *fs_info = trans->fs_info;
struct btrfs_root *root = btrfs_free_space_root(block_group);
@@ -278,7 +278,7 @@ int convert_free_space_to_bitmaps(struct btrfs_trans_handle *trans,
btrfs_release_path(path);
}
- info = search_free_space_info(trans, block_group, path, 1);
+ info = btrfs_search_free_space_info(trans, block_group, path, 1);
if (IS_ERR(info)) {
ret = PTR_ERR(info);
btrfs_abort_transaction(trans, ret);
@@ -340,9 +340,9 @@ int convert_free_space_to_bitmaps(struct btrfs_trans_handle *trans,
}
EXPORT_FOR_TESTS
-int convert_free_space_to_extents(struct btrfs_trans_handle *trans,
- struct btrfs_block_group *block_group,
- struct btrfs_path *path)
+int btrfs_convert_free_space_to_extents(struct btrfs_trans_handle *trans,
+ struct btrfs_block_group *block_group,
+ struct btrfs_path *path)
{
struct btrfs_fs_info *fs_info = trans->fs_info;
struct btrfs_root *root = btrfs_free_space_root(block_group);
@@ -425,7 +425,7 @@ int convert_free_space_to_extents(struct btrfs_trans_handle *trans,
btrfs_release_path(path);
}
- info = search_free_space_info(trans, block_group, path, 1);
+ info = btrfs_search_free_space_info(trans, block_group, path, 1);
if (IS_ERR(info)) {
ret = PTR_ERR(info);
btrfs_abort_transaction(trans, ret);
@@ -490,7 +490,7 @@ static int update_free_space_extent_count(struct btrfs_trans_handle *trans,
if (new_extents == 0)
return 0;
- info = search_free_space_info(trans, block_group, path, 1);
+ info = btrfs_search_free_space_info(trans, block_group, path, 1);
if (IS_ERR(info))
return PTR_ERR(info);
@@ -503,18 +503,18 @@ static int update_free_space_extent_count(struct btrfs_trans_handle *trans,
if (!(flags & BTRFS_FREE_SPACE_USING_BITMAPS) &&
extent_count > block_group->bitmap_high_thresh) {
- ret = convert_free_space_to_bitmaps(trans, block_group, path);
+ ret = btrfs_convert_free_space_to_bitmaps(trans, block_group, path);
} else if ((flags & BTRFS_FREE_SPACE_USING_BITMAPS) &&
extent_count < block_group->bitmap_low_thresh) {
- ret = convert_free_space_to_extents(trans, block_group, path);
+ ret = btrfs_convert_free_space_to_extents(trans, block_group, path);
}
return ret;
}
EXPORT_FOR_TESTS
-bool free_space_test_bit(struct btrfs_block_group *block_group,
- struct btrfs_path *path, u64 offset)
+bool btrfs_free_space_test_bit(struct btrfs_block_group *block_group,
+ struct btrfs_path *path, u64 offset)
{
struct extent_buffer *leaf;
struct btrfs_key key;
@@ -632,7 +632,7 @@ static int modify_free_space_bitmap(struct btrfs_trans_handle *trans,
if (ret)
return ret;
- prev_bit_set = free_space_test_bit(block_group, path, prev_block);
+ prev_bit_set = btrfs_free_space_test_bit(block_group, path, prev_block);
/* The previous block may have been in the previous bitmap. */
btrfs_item_key_to_cpu(path->nodes[0], &key, path->slots[0]);
@@ -680,7 +680,7 @@ static int modify_free_space_bitmap(struct btrfs_trans_handle *trans,
return ret;
}
- next_bit_set = free_space_test_bit(block_group, path, end);
+ next_bit_set = btrfs_free_space_test_bit(block_group, path, end);
}
if (remove) {
@@ -792,9 +792,9 @@ static int remove_free_space_extent(struct btrfs_trans_handle *trans,
}
EXPORT_FOR_TESTS
-int __remove_from_free_space_tree(struct btrfs_trans_handle *trans,
- struct btrfs_block_group *block_group,
- struct btrfs_path *path, u64 start, u64 size)
+int __btrfs_remove_from_free_space_tree(struct btrfs_trans_handle *trans,
+ struct btrfs_block_group *block_group,
+ struct btrfs_path *path, u64 start, u64 size)
{
struct btrfs_free_space_info *info;
u32 flags;
@@ -804,7 +804,7 @@ int __remove_from_free_space_tree(struct btrfs_trans_handle *trans,
if (ret)
return ret;
- info = search_free_space_info(NULL, block_group, path, 0);
+ info = btrfs_search_free_space_info(NULL, block_group, path, 0);
if (IS_ERR(info))
return PTR_ERR(info);
flags = btrfs_free_space_flags(path->nodes[0], info);
@@ -819,8 +819,8 @@ int __remove_from_free_space_tree(struct btrfs_trans_handle *trans,
}
}
-int remove_from_free_space_tree(struct btrfs_trans_handle *trans,
- u64 start, u64 size)
+int btrfs_remove_from_free_space_tree(struct btrfs_trans_handle *trans,
+ u64 start, u64 size)
{
struct btrfs_block_group *block_group;
struct btrfs_path *path;
@@ -845,8 +845,7 @@ int remove_from_free_space_tree(struct btrfs_trans_handle *trans,
}
mutex_lock(&block_group->free_space_lock);
- ret = __remove_from_free_space_tree(trans, block_group, path, start,
- size);
+ ret = __btrfs_remove_from_free_space_tree(trans, block_group, path, start, size);
mutex_unlock(&block_group->free_space_lock);
if (ret)
btrfs_abort_transaction(trans, ret);
@@ -981,9 +980,9 @@ static int add_free_space_extent(struct btrfs_trans_handle *trans,
}
EXPORT_FOR_TESTS
-int __add_to_free_space_tree(struct btrfs_trans_handle *trans,
- struct btrfs_block_group *block_group,
- struct btrfs_path *path, u64 start, u64 size)
+int __btrfs_add_to_free_space_tree(struct btrfs_trans_handle *trans,
+ struct btrfs_block_group *block_group,
+ struct btrfs_path *path, u64 start, u64 size)
{
struct btrfs_free_space_info *info;
u32 flags;
@@ -993,7 +992,7 @@ int __add_to_free_space_tree(struct btrfs_trans_handle *trans,
if (ret)
return ret;
- info = search_free_space_info(NULL, block_group, path, 0);
+ info = btrfs_search_free_space_info(NULL, block_group, path, 0);
if (IS_ERR(info))
return PTR_ERR(info);
flags = btrfs_free_space_flags(path->nodes[0], info);
@@ -1008,8 +1007,8 @@ int __add_to_free_space_tree(struct btrfs_trans_handle *trans,
}
}
-int add_to_free_space_tree(struct btrfs_trans_handle *trans,
- u64 start, u64 size)
+int btrfs_add_to_free_space_tree(struct btrfs_trans_handle *trans,
+ u64 start, u64 size)
{
struct btrfs_block_group *block_group;
struct btrfs_path *path;
@@ -1034,7 +1033,7 @@ int add_to_free_space_tree(struct btrfs_trans_handle *trans,
}
mutex_lock(&block_group->free_space_lock);
- ret = __add_to_free_space_tree(trans, block_group, path, start, size);
+ ret = __btrfs_add_to_free_space_tree(trans, block_group, path, start, size);
mutex_unlock(&block_group->free_space_lock);
if (ret)
btrfs_abort_transaction(trans, ret);
@@ -1114,11 +1113,11 @@ static int populate_free_space_tree(struct btrfs_trans_handle *trans,
break;
if (start < key.objectid) {
- ret = __add_to_free_space_tree(trans,
- block_group,
- path2, start,
- key.objectid -
- start);
+ ret = __btrfs_add_to_free_space_tree(trans,
+ block_group,
+ path2, start,
+ key.objectid -
+ start);
if (ret)
goto out_locked;
}
@@ -1137,8 +1136,8 @@ static int populate_free_space_tree(struct btrfs_trans_handle *trans,
goto out_locked;
}
if (start < end) {
- ret = __add_to_free_space_tree(trans, block_group, path2,
- start, end - start);
+ ret = __btrfs_add_to_free_space_tree(trans, block_group, path2,
+ start, end - start);
if (ret)
goto out_locked;
}
@@ -1424,8 +1423,8 @@ static int __add_block_group_free_space(struct btrfs_trans_handle *trans,
goto out;
}
- ret = __add_to_free_space_tree(trans, block_group, path,
- block_group->start, block_group->length);
+ ret = __btrfs_add_to_free_space_tree(trans, block_group, path,
+ block_group->start, block_group->length);
if (ret)
btrfs_abort_transaction(trans, ret);
@@ -1436,8 +1435,8 @@ static int __add_block_group_free_space(struct btrfs_trans_handle *trans,
return ret;
}
-int add_block_group_free_space(struct btrfs_trans_handle *trans,
- struct btrfs_block_group *block_group)
+int btrfs_add_block_group_free_space(struct btrfs_trans_handle *trans,
+ struct btrfs_block_group *block_group)
{
int ret;
@@ -1450,8 +1449,8 @@ int add_block_group_free_space(struct btrfs_trans_handle *trans,
return ret;
}
-int remove_block_group_free_space(struct btrfs_trans_handle *trans,
- struct btrfs_block_group *block_group)
+int btrfs_remove_block_group_free_space(struct btrfs_trans_handle *trans,
+ struct btrfs_block_group *block_group)
{
struct btrfs_root *root = btrfs_free_space_root(block_group);
struct btrfs_path *path;
@@ -1570,7 +1569,7 @@ static int load_free_space_bitmaps(struct btrfs_caching_control *caching_ctl,
while (offset < key.objectid + key.offset) {
bool bit_set;
- bit_set = free_space_test_bit(block_group, path, offset);
+ bit_set = btrfs_free_space_test_bit(block_group, path, offset);
if (!prev_bit_set && bit_set) {
extent_start = offset;
} else if (prev_bit_set && !bit_set) {
@@ -1673,7 +1672,7 @@ static int load_free_space_extents(struct btrfs_caching_control *caching_ctl,
return 0;
}
-int load_free_space_tree(struct btrfs_caching_control *caching_ctl)
+int btrfs_load_free_space_tree(struct btrfs_caching_control *caching_ctl)
{
struct btrfs_block_group *block_group;
struct btrfs_free_space_info *info;
@@ -1694,7 +1693,7 @@ int load_free_space_tree(struct btrfs_caching_control *caching_ctl)
path->search_commit_root = 1;
path->reada = READA_FORWARD;
- info = search_free_space_info(NULL, block_group, path, 0);
+ info = btrfs_search_free_space_info(NULL, block_group, path, 0);
if (IS_ERR(info))
return PTR_ERR(info);
diff --git a/fs/btrfs/free-space-tree.h b/fs/btrfs/free-space-tree.h
index 32e71d0c8dd4..3d9a5d4477fc 100644
--- a/fs/btrfs/free-space-tree.h
+++ b/fs/btrfs/free-space-tree.h
@@ -22,39 +22,39 @@ struct btrfs_trans_handle;
#define BTRFS_FREE_SPACE_BITMAP_SIZE 256
#define BTRFS_FREE_SPACE_BITMAP_BITS (BTRFS_FREE_SPACE_BITMAP_SIZE * BITS_PER_BYTE)
-void set_free_space_tree_thresholds(struct btrfs_block_group *block_group);
+void btrfs_set_free_space_tree_thresholds(struct btrfs_block_group *block_group);
int btrfs_create_free_space_tree(struct btrfs_fs_info *fs_info);
int btrfs_delete_free_space_tree(struct btrfs_fs_info *fs_info);
int btrfs_rebuild_free_space_tree(struct btrfs_fs_info *fs_info);
-int load_free_space_tree(struct btrfs_caching_control *caching_ctl);
-int add_block_group_free_space(struct btrfs_trans_handle *trans,
- struct btrfs_block_group *block_group);
-int remove_block_group_free_space(struct btrfs_trans_handle *trans,
- struct btrfs_block_group *block_group);
-int add_to_free_space_tree(struct btrfs_trans_handle *trans,
- u64 start, u64 size);
-int remove_from_free_space_tree(struct btrfs_trans_handle *trans,
- u64 start, u64 size);
+int btrfs_load_free_space_tree(struct btrfs_caching_control *caching_ctl);
+int btrfs_add_block_group_free_space(struct btrfs_trans_handle *trans,
+ struct btrfs_block_group *block_group);
+int btrfs_remove_block_group_free_space(struct btrfs_trans_handle *trans,
+ struct btrfs_block_group *block_group);
+int btrfs_add_to_free_space_tree(struct btrfs_trans_handle *trans,
+ u64 start, u64 size);
+int btrfs_remove_from_free_space_tree(struct btrfs_trans_handle *trans,
+ u64 start, u64 size);
#ifdef CONFIG_BTRFS_FS_RUN_SANITY_TESTS
struct btrfs_free_space_info *
-search_free_space_info(struct btrfs_trans_handle *trans,
- struct btrfs_block_group *block_group,
- struct btrfs_path *path, int cow);
-int __add_to_free_space_tree(struct btrfs_trans_handle *trans,
+btrfs_search_free_space_info(struct btrfs_trans_handle *trans,
struct btrfs_block_group *block_group,
- struct btrfs_path *path, u64 start, u64 size);
-int __remove_from_free_space_tree(struct btrfs_trans_handle *trans,
- struct btrfs_block_group *block_group,
- struct btrfs_path *path, u64 start, u64 size);
-int convert_free_space_to_bitmaps(struct btrfs_trans_handle *trans,
- struct btrfs_block_group *block_group,
- struct btrfs_path *path);
-int convert_free_space_to_extents(struct btrfs_trans_handle *trans,
- struct btrfs_block_group *block_group,
- struct btrfs_path *path);
-bool free_space_test_bit(struct btrfs_block_group *block_group,
- struct btrfs_path *path, u64 offset);
+ struct btrfs_path *path, int cow);
+int __btrfs_add_to_free_space_tree(struct btrfs_trans_handle *trans,
+ struct btrfs_block_group *block_group,
+ struct btrfs_path *path, u64 start, u64 size);
+int __btrfs_remove_from_free_space_tree(struct btrfs_trans_handle *trans,
+ struct btrfs_block_group *block_group,
+ struct btrfs_path *path, u64 start, u64 size);
+int btrfs_convert_free_space_to_bitmaps(struct btrfs_trans_handle *trans,
+ struct btrfs_block_group *block_group,
+ struct btrfs_path *path);
+int btrfs_convert_free_space_to_extents(struct btrfs_trans_handle *trans,
+ struct btrfs_block_group *block_group,
+ struct btrfs_path *path);
+bool btrfs_free_space_test_bit(struct btrfs_block_group *block_group,
+ struct btrfs_path *path, u64 offset);
#endif
#endif
diff --git a/fs/btrfs/tests/free-space-tree-tests.c b/fs/btrfs/tests/free-space-tree-tests.c
index b61972046feb..c8822edd32e2 100644
--- a/fs/btrfs/tests/free-space-tree-tests.c
+++ b/fs/btrfs/tests/free-space-tree-tests.c
@@ -32,7 +32,7 @@ static int __check_free_space_extents(struct btrfs_trans_handle *trans,
unsigned int i;
int ret;
- info = search_free_space_info(trans, cache, path, 0);
+ info = btrfs_search_free_space_info(trans, cache, path, 0);
if (IS_ERR(info)) {
test_err("could not find free space info");
ret = PTR_ERR(info);
@@ -57,7 +57,7 @@ static int __check_free_space_extents(struct btrfs_trans_handle *trans,
goto invalid;
offset = key.objectid;
while (offset < key.objectid + key.offset) {
- bit = free_space_test_bit(cache, path, offset);
+ bit = btrfs_free_space_test_bit(cache, path, offset);
if (prev_bit == 0 && bit == 1) {
extent_start = offset;
} else if (prev_bit == 1 && bit == 0) {
@@ -115,7 +115,7 @@ static int check_free_space_extents(struct btrfs_trans_handle *trans,
u32 flags;
int ret;
- info = search_free_space_info(trans, cache, path, 0);
+ info = btrfs_search_free_space_info(trans, cache, path, 0);
if (IS_ERR(info)) {
test_err("could not find free space info");
btrfs_release_path(path);
@@ -131,13 +131,13 @@ static int check_free_space_extents(struct btrfs_trans_handle *trans,
/* Flip it to the other format and check that for good measure. */
if (flags & BTRFS_FREE_SPACE_USING_BITMAPS) {
- ret = convert_free_space_to_extents(trans, cache, path);
+ ret = btrfs_convert_free_space_to_extents(trans, cache, path);
if (ret) {
test_err("could not convert to extents");
return ret;
}
} else {
- ret = convert_free_space_to_bitmaps(trans, cache, path);
+ ret = btrfs_convert_free_space_to_bitmaps(trans, cache, path);
if (ret) {
test_err("could not convert to bitmaps");
return ret;
@@ -170,9 +170,8 @@ static int test_remove_all(struct btrfs_trans_handle *trans,
const struct free_space_extent extents[] = {};
int ret;
- ret = __remove_from_free_space_tree(trans, cache, path,
- cache->start,
- cache->length);
+ ret = __btrfs_remove_from_free_space_tree(trans, cache, path,
+ cache->start, cache->length);
if (ret) {
test_err("could not remove free space");
return ret;
@@ -193,8 +192,8 @@ static int test_remove_beginning(struct btrfs_trans_handle *trans,
};
int ret;
- ret = __remove_from_free_space_tree(trans, cache, path,
- cache->start, alignment);
+ ret = __btrfs_remove_from_free_space_tree(trans, cache, path,
+ cache->start, alignment);
if (ret) {
test_err("could not remove free space");
return ret;
@@ -216,7 +215,7 @@ static int test_remove_end(struct btrfs_trans_handle *trans,
};
int ret;
- ret = __remove_from_free_space_tree(trans, cache, path,
+ ret = __btrfs_remove_from_free_space_tree(trans, cache, path,
cache->start + cache->length - alignment,
alignment);
if (ret) {
@@ -240,9 +239,9 @@ static int test_remove_middle(struct btrfs_trans_handle *trans,
};
int ret;
- ret = __remove_from_free_space_tree(trans, cache, path,
- cache->start + alignment,
- alignment);
+ ret = __btrfs_remove_from_free_space_tree(trans, cache, path,
+ cache->start + alignment,
+ alignment);
if (ret) {
test_err("could not remove free space");
return ret;
@@ -263,23 +262,22 @@ static int test_merge_left(struct btrfs_trans_handle *trans,
};
int ret;
- ret = __remove_from_free_space_tree(trans, cache, path,
- cache->start, cache->length);
+ ret = __btrfs_remove_from_free_space_tree(trans, cache, path,
+ cache->start, cache->length);
if (ret) {
test_err("could not remove free space");
return ret;
}
- ret = __add_to_free_space_tree(trans, cache, path, cache->start,
- alignment);
+ ret = __btrfs_add_to_free_space_tree(trans, cache, path, cache->start,
+ alignment);
if (ret) {
test_err("could not add free space");
return ret;
}
- ret = __add_to_free_space_tree(trans, cache, path,
- cache->start + alignment,
- alignment);
+ ret = __btrfs_add_to_free_space_tree(trans, cache, path,
+ cache->start + alignment, alignment);
if (ret) {
test_err("could not add free space");
return ret;
@@ -300,24 +298,23 @@ static int test_merge_right(struct btrfs_trans_handle *trans,
};
int ret;
- ret = __remove_from_free_space_tree(trans, cache, path,
- cache->start, cache->length);
+ ret = __btrfs_remove_from_free_space_tree(trans, cache, path,
+ cache->start, cache->length);
if (ret) {
test_err("could not remove free space");
return ret;
}
- ret = __add_to_free_space_tree(trans, cache, path,
- cache->start + 2 * alignment,
- alignment);
+ ret = __btrfs_add_to_free_space_tree(trans, cache, path,
+ cache->start + 2 * alignment,
+ alignment);
if (ret) {
test_err("could not add free space");
return ret;
}
- ret = __add_to_free_space_tree(trans, cache, path,
- cache->start + alignment,
- alignment);
+ ret = __btrfs_add_to_free_space_tree(trans, cache, path,
+ cache->start + alignment, alignment);
if (ret) {
test_err("could not add free space");
return ret;
@@ -338,29 +335,29 @@ static int test_merge_both(struct btrfs_trans_handle *trans,
};
int ret;
- ret = __remove_from_free_space_tree(trans, cache, path,
- cache->start, cache->length);
+ ret = __btrfs_remove_from_free_space_tree(trans, cache, path,
+ cache->start, cache->length);
if (ret) {
test_err("could not remove free space");
return ret;
}
- ret = __add_to_free_space_tree(trans, cache, path, cache->start,
- alignment);
+ ret = __btrfs_add_to_free_space_tree(trans, cache, path, cache->start,
+ alignment);
if (ret) {
test_err("could not add free space");
return ret;
}
- ret = __add_to_free_space_tree(trans, cache, path,
- cache->start + 2 * alignment, alignment);
+ ret = __btrfs_add_to_free_space_tree(trans, cache, path,
+ cache->start + 2 * alignment, alignment);
if (ret) {
test_err("could not add free space");
return ret;
}
- ret = __add_to_free_space_tree(trans, cache, path,
- cache->start + alignment, alignment);
+ ret = __btrfs_add_to_free_space_tree(trans, cache, path,
+ cache->start + alignment, alignment);
if (ret) {
test_err("could not add free space");
return ret;
@@ -383,29 +380,29 @@ static int test_merge_none(struct btrfs_trans_handle *trans,
};
int ret;
- ret = __remove_from_free_space_tree(trans, cache, path,
- cache->start, cache->length);
+ ret = __btrfs_remove_from_free_space_tree(trans, cache, path,
+ cache->start, cache->length);
if (ret) {
test_err("could not remove free space");
return ret;
}
- ret = __add_to_free_space_tree(trans, cache, path, cache->start,
- alignment);
+ ret = __btrfs_add_to_free_space_tree(trans, cache, path, cache->start,
+ alignment);
if (ret) {
test_err("could not add free space");
return ret;
}
- ret = __add_to_free_space_tree(trans, cache, path,
- cache->start + 4 * alignment, alignment);
+ ret = __btrfs_add_to_free_space_tree(trans, cache, path,
+ cache->start + 4 * alignment, alignment);
if (ret) {
test_err("could not add free space");
return ret;
}
- ret = __add_to_free_space_tree(trans, cache, path,
- cache->start + 2 * alignment, alignment);
+ ret = __btrfs_add_to_free_space_tree(trans, cache, path,
+ cache->start + 2 * alignment, alignment);
if (ret) {
test_err("could not add free space");
return ret;
@@ -483,14 +480,14 @@ static int run_test(test_func_t test_func, int bitmaps, u32 sectorsize,
goto out;
}
- ret = add_block_group_free_space(&trans, cache);
+ ret = btrfs_add_block_group_free_space(&trans, cache);
if (ret) {
test_err("could not add block group free space");
goto out;
}
if (bitmaps) {
- ret = convert_free_space_to_bitmaps(&trans, cache, path);
+ ret = btrfs_convert_free_space_to_bitmaps(&trans, cache, path);
if (ret) {
test_err("could not convert block group to bitmaps");
goto out;
@@ -501,7 +498,7 @@ static int run_test(test_func_t test_func, int bitmaps, u32 sectorsize,
if (ret)
goto out;
- ret = remove_block_group_free_space(&trans, cache);
+ ret = btrfs_remove_block_group_free_space(&trans, cache);
if (ret) {
test_err("could not remove block group free space");
goto out;
--
2.47.2
^ permalink raw reply related [flat|nested] 22+ messages in thread* [PATCH 11/16] btrfs: rename free_space_set_bits() and make it less confusing
2025-06-17 16:12 [PATCH 00/16] btrfs: free space tree optimization and cleanups fdmanana
` (9 preceding siblings ...)
2025-06-17 16:13 ` [PATCH 10/16] btrfs: add btrfs prefix to free space tree exported functions fdmanana
@ 2025-06-17 16:13 ` fdmanana
2025-06-17 16:13 ` [PATCH 12/16] btrfs: turn remove argument of modify_free_space_bitmap() to boolean fdmanana
` (5 subsequent siblings)
16 siblings, 0 replies; 22+ messages in thread
From: fdmanana @ 2025-06-17 16:13 UTC (permalink / raw)
To: linux-btrfs
From: Filipe Manana <fdmanana@suse.com>
The free_space_set_bits() is used both to set a range of bits or to clear
range of bits, depending on the 'bit' argument value. So the name is very
misleading since it's not used only to set bits. Furthermore the 'bit'
argument is an integer when a boolean is all that is needed plus it's name
is singular, which gives the idea the function operates on a single bit
and not on a range of bits.
So rename the function to free_space_modify_bits(), rename the 'bit'
argument to 'set_bits' and turn the argument to bool instead of int.
Signed-off-by: Filipe Manana <fdmanana@suse.com>
---
fs/btrfs/free-space-tree.c | 14 +++++++-------
1 file changed, 7 insertions(+), 7 deletions(-)
diff --git a/fs/btrfs/free-space-tree.c b/fs/btrfs/free-space-tree.c
index 1794fdf06586..a0b51543e83e 100644
--- a/fs/btrfs/free-space-tree.c
+++ b/fs/btrfs/free-space-tree.c
@@ -535,10 +535,10 @@ bool btrfs_free_space_test_bit(struct btrfs_block_group *block_group,
return extent_buffer_test_bit(leaf, ptr, i);
}
-static void free_space_set_bits(struct btrfs_trans_handle *trans,
- struct btrfs_block_group *block_group,
- struct btrfs_path *path, u64 *start, u64 *size,
- int bit)
+static void free_space_modify_bits(struct btrfs_trans_handle *trans,
+ struct btrfs_block_group *block_group,
+ struct btrfs_path *path, u64 *start, u64 *size,
+ bool set_bits)
{
struct btrfs_fs_info *fs_info = block_group->fs_info;
struct extent_buffer *leaf;
@@ -562,7 +562,7 @@ static void free_space_set_bits(struct btrfs_trans_handle *trans,
ptr = btrfs_item_ptr_offset(leaf, path->slots[0]);
first = (*start - found_start) >> fs_info->sectorsize_bits;
last = (end - found_start) >> fs_info->sectorsize_bits;
- if (bit)
+ if (set_bits)
extent_buffer_bitmap_set(leaf, ptr, first, last - first);
else
extent_buffer_bitmap_clear(leaf, ptr, first, last - first);
@@ -658,8 +658,8 @@ static int modify_free_space_bitmap(struct btrfs_trans_handle *trans,
cur_start = start;
cur_size = size;
while (1) {
- free_space_set_bits(trans, block_group, path, &cur_start, &cur_size,
- !remove);
+ free_space_modify_bits(trans, block_group, path, &cur_start,
+ &cur_size, !remove);
if (cur_size == 0)
break;
ret = free_space_next_bitmap(trans, root, path);
--
2.47.2
^ permalink raw reply related [flat|nested] 22+ messages in thread* [PATCH 12/16] btrfs: turn remove argument of modify_free_space_bitmap() to boolean
2025-06-17 16:12 [PATCH 00/16] btrfs: free space tree optimization and cleanups fdmanana
` (10 preceding siblings ...)
2025-06-17 16:13 ` [PATCH 11/16] btrfs: rename free_space_set_bits() and make it less confusing fdmanana
@ 2025-06-17 16:13 ` fdmanana
2025-06-17 16:13 ` [PATCH 13/16] btrfs: avoid double slot decrement at btrfs_convert_free_space_to_extents() fdmanana
` (4 subsequent siblings)
16 siblings, 0 replies; 22+ messages in thread
From: fdmanana @ 2025-06-17 16:13 UTC (permalink / raw)
To: linux-btrfs
From: Filipe Manana <fdmanana@suse.com>
The argument is used as a boolean, so switch its type from int to bool.
Signed-off-by: Filipe Manana <fdmanana@suse.com>
---
fs/btrfs/free-space-tree.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/fs/btrfs/free-space-tree.c b/fs/btrfs/free-space-tree.c
index a0b51543e83e..b85927232860 100644
--- a/fs/btrfs/free-space-tree.c
+++ b/fs/btrfs/free-space-tree.c
@@ -606,7 +606,7 @@ static int free_space_next_bitmap(struct btrfs_trans_handle *trans,
static int modify_free_space_bitmap(struct btrfs_trans_handle *trans,
struct btrfs_block_group *block_group,
struct btrfs_path *path,
- u64 start, u64 size, int remove)
+ u64 start, u64 size, bool remove)
{
struct btrfs_root *root = btrfs_free_space_root(block_group);
struct btrfs_key key;
@@ -812,7 +812,7 @@ int __btrfs_remove_from_free_space_tree(struct btrfs_trans_handle *trans,
if (flags & BTRFS_FREE_SPACE_USING_BITMAPS) {
return modify_free_space_bitmap(trans, block_group, path,
- start, size, 1);
+ start, size, true);
} else {
return remove_free_space_extent(trans, block_group, path,
start, size);
@@ -1000,7 +1000,7 @@ int __btrfs_add_to_free_space_tree(struct btrfs_trans_handle *trans,
if (flags & BTRFS_FREE_SPACE_USING_BITMAPS) {
return modify_free_space_bitmap(trans, block_group, path,
- start, size, 0);
+ start, size, false);
} else {
return add_free_space_extent(trans, block_group, path, start,
size);
--
2.47.2
^ permalink raw reply related [flat|nested] 22+ messages in thread* [PATCH 13/16] btrfs: avoid double slot decrement at btrfs_convert_free_space_to_extents()
2025-06-17 16:12 [PATCH 00/16] btrfs: free space tree optimization and cleanups fdmanana
` (11 preceding siblings ...)
2025-06-17 16:13 ` [PATCH 12/16] btrfs: turn remove argument of modify_free_space_bitmap() to boolean fdmanana
@ 2025-06-17 16:13 ` fdmanana
2025-06-17 16:13 ` [PATCH 14/16] btrfs: use fs_info from local variable in btrfs_convert_free_space_to_extents() fdmanana
` (3 subsequent siblings)
16 siblings, 0 replies; 22+ messages in thread
From: fdmanana @ 2025-06-17 16:13 UTC (permalink / raw)
To: linux-btrfs
From: Filipe Manana <fdmanana@suse.com>
There's no need to subtract 1 from path->slots[0] and then decrement the
slot, we can reduce the generated assembly code by decrementing the slot
and then use it.
Module size before:
$ size fs/btrfs/btrfs.ko
text data bss dec hex filename
1846220 162998 16136 2025354 1ee78a fs/btrfs/btrfs.ko
Module size after:
$ size fs/btrfs/btrfs.ko
text data bss dec hex filename
1846204 162998 16136 2025338 1ee77a fs/btrfs/btrfs.ko
Signed-off-by: Filipe Manana <fdmanana@suse.com>
---
fs/btrfs/free-space-tree.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/fs/btrfs/free-space-tree.c b/fs/btrfs/free-space-tree.c
index b85927232860..d5766e25f5dc 100644
--- a/fs/btrfs/free-space-tree.c
+++ b/fs/btrfs/free-space-tree.c
@@ -406,12 +406,12 @@ int btrfs_convert_free_space_to_extents(struct btrfs_trans_handle *trans,
data_size = free_space_bitmap_size(fs_info,
found_key.offset);
- ptr = btrfs_item_ptr_offset(leaf, path->slots[0] - 1);
+ path->slots[0]--;
+ ptr = btrfs_item_ptr_offset(leaf, path->slots[0]);
read_extent_buffer(leaf, bitmap_cursor, ptr,
data_size);
nr++;
- path->slots[0]--;
} else {
ASSERT(0);
}
--
2.47.2
^ permalink raw reply related [flat|nested] 22+ messages in thread* [PATCH 14/16] btrfs: use fs_info from local variable in btrfs_convert_free_space_to_extents()
2025-06-17 16:12 [PATCH 00/16] btrfs: free space tree optimization and cleanups fdmanana
` (12 preceding siblings ...)
2025-06-17 16:13 ` [PATCH 13/16] btrfs: avoid double slot decrement at btrfs_convert_free_space_to_extents() fdmanana
@ 2025-06-17 16:13 ` fdmanana
2025-06-17 16:13 ` [PATCH 15/16] btrfs: add and use helper to determine if using bitmaps in free space tree fdmanana
` (2 subsequent siblings)
16 siblings, 0 replies; 22+ messages in thread
From: fdmanana @ 2025-06-17 16:13 UTC (permalink / raw)
To: linux-btrfs
From: Filipe Manana <fdmanana@suse.com>
There's no need to dereference the block group to extract fs_info as we
have already stored fs_info in a local variable. So use the local variable
instead.
Signed-off-by: Filipe Manana <fdmanana@suse.com>
---
fs/btrfs/free-space-tree.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/fs/btrfs/free-space-tree.c b/fs/btrfs/free-space-tree.c
index d5766e25f5dc..c85ca7ce2683 100644
--- a/fs/btrfs/free-space-tree.c
+++ b/fs/btrfs/free-space-tree.c
@@ -438,16 +438,16 @@ int btrfs_convert_free_space_to_extents(struct btrfs_trans_handle *trans,
expected_extent_count = btrfs_free_space_extent_count(leaf, info);
btrfs_release_path(path);
- nrbits = block_group->length >> block_group->fs_info->sectorsize_bits;
+ nrbits = block_group->length >> fs_info->sectorsize_bits;
start_bit = find_next_bit_le(bitmap, nrbits, 0);
while (start_bit < nrbits) {
end_bit = find_next_zero_bit_le(bitmap, nrbits, start_bit);
ASSERT(start_bit < end_bit);
- key.objectid = start + start_bit * block_group->fs_info->sectorsize;
+ key.objectid = start + start_bit * fs_info->sectorsize;
key.type = BTRFS_FREE_SPACE_EXTENT_KEY;
- key.offset = (end_bit - start_bit) * block_group->fs_info->sectorsize;
+ key.offset = (end_bit - start_bit) * fs_info->sectorsize;
ret = btrfs_insert_empty_item(trans, root, path, &key, 0);
if (ret) {
--
2.47.2
^ permalink raw reply related [flat|nested] 22+ messages in thread* [PATCH 15/16] btrfs: add and use helper to determine if using bitmaps in free space tree
2025-06-17 16:12 [PATCH 00/16] btrfs: free space tree optimization and cleanups fdmanana
` (13 preceding siblings ...)
2025-06-17 16:13 ` [PATCH 14/16] btrfs: use fs_info from local variable in btrfs_convert_free_space_to_extents() fdmanana
@ 2025-06-17 16:13 ` fdmanana
2025-06-17 21:41 ` Boris Burkov
2025-06-17 16:13 ` [PATCH 16/16] btrfs: cache if we are using free space bitmaps for a block group fdmanana
2025-06-18 11:50 ` [PATCH 00/16] btrfs: free space tree optimization and cleanups David Sterba
16 siblings, 1 reply; 22+ messages in thread
From: fdmanana @ 2025-06-17 16:13 UTC (permalink / raw)
To: linux-btrfs
From: Filipe Manana <fdmanana@suse.com>
When adding and removing free space to the free space tree, we need to
lookup the respective block group's free info item in the free space tree,
check its flags for the BTRFS_FREE_SPACE_USING_BITMAPS bit and then
release the path.
Move these steps into a helper function and use it in both sites.
This will also help avoiding duplicate code in a subsequent change.
Signed-off-by: Filipe Manana <fdmanana@suse.com>
---
fs/btrfs/free-space-tree.c | 50 ++++++++++++++++++++------------------
1 file changed, 26 insertions(+), 24 deletions(-)
diff --git a/fs/btrfs/free-space-tree.c b/fs/btrfs/free-space-tree.c
index c85ca7ce2683..3c8bb95fa044 100644
--- a/fs/btrfs/free-space-tree.c
+++ b/fs/btrfs/free-space-tree.c
@@ -791,32 +791,40 @@ static int remove_free_space_extent(struct btrfs_trans_handle *trans,
return update_free_space_extent_count(trans, block_group, path, new_extents);
}
+static int use_bitmaps(struct btrfs_block_group *bg, struct btrfs_path *path)
+{
+ struct btrfs_free_space_info *info;
+ u32 flags;
+
+ info = btrfs_search_free_space_info(NULL, bg, path, 0);
+ if (IS_ERR(info))
+ return PTR_ERR(info);
+ flags = btrfs_free_space_flags(path->nodes[0], info);
+ btrfs_release_path(path);
+
+ return (flags & BTRFS_FREE_SPACE_USING_BITMAPS) ? 1 : 0;
+}
+
EXPORT_FOR_TESTS
int __btrfs_remove_from_free_space_tree(struct btrfs_trans_handle *trans,
struct btrfs_block_group *block_group,
struct btrfs_path *path, u64 start, u64 size)
{
- struct btrfs_free_space_info *info;
- u32 flags;
int ret;
ret = __add_block_group_free_space(trans, block_group, path);
if (ret)
return ret;
- info = btrfs_search_free_space_info(NULL, block_group, path, 0);
- if (IS_ERR(info))
- return PTR_ERR(info);
- flags = btrfs_free_space_flags(path->nodes[0], info);
- btrfs_release_path(path);
+ ret = use_bitmaps(block_group, path);
+ if (ret < 0)
+ return ret;
- if (flags & BTRFS_FREE_SPACE_USING_BITMAPS) {
+ if (ret)
return modify_free_space_bitmap(trans, block_group, path,
start, size, true);
- } else {
- return remove_free_space_extent(trans, block_group, path,
- start, size);
- }
+
+ return remove_free_space_extent(trans, block_group, path, start, size);
}
int btrfs_remove_from_free_space_tree(struct btrfs_trans_handle *trans,
@@ -984,27 +992,21 @@ int __btrfs_add_to_free_space_tree(struct btrfs_trans_handle *trans,
struct btrfs_block_group *block_group,
struct btrfs_path *path, u64 start, u64 size)
{
- struct btrfs_free_space_info *info;
- u32 flags;
int ret;
ret = __add_block_group_free_space(trans, block_group, path);
if (ret)
return ret;
- info = btrfs_search_free_space_info(NULL, block_group, path, 0);
- if (IS_ERR(info))
- return PTR_ERR(info);
- flags = btrfs_free_space_flags(path->nodes[0], info);
- btrfs_release_path(path);
+ ret = use_bitmaps(block_group, path);
+ if (ret < 0)
+ return ret;
- if (flags & BTRFS_FREE_SPACE_USING_BITMAPS) {
+ if (ret)
return modify_free_space_bitmap(trans, block_group, path,
start, size, false);
- } else {
- return add_free_space_extent(trans, block_group, path, start,
- size);
- }
+
+ return add_free_space_extent(trans, block_group, path, start, size);
}
int btrfs_add_to_free_space_tree(struct btrfs_trans_handle *trans,
--
2.47.2
^ permalink raw reply related [flat|nested] 22+ messages in thread* Re: [PATCH 15/16] btrfs: add and use helper to determine if using bitmaps in free space tree
2025-06-17 16:13 ` [PATCH 15/16] btrfs: add and use helper to determine if using bitmaps in free space tree fdmanana
@ 2025-06-17 21:41 ` Boris Burkov
0 siblings, 0 replies; 22+ messages in thread
From: Boris Burkov @ 2025-06-17 21:41 UTC (permalink / raw)
To: fdmanana; +Cc: linux-btrfs
On Tue, Jun 17, 2025 at 05:13:10PM +0100, fdmanana@kernel.org wrote:
> From: Filipe Manana <fdmanana@suse.com>
>
> When adding and removing free space to the free space tree, we need to
> lookup the respective block group's free info item in the free space tree,
> check its flags for the BTRFS_FREE_SPACE_USING_BITMAPS bit and then
> release the path.
>
> Move these steps into a helper function and use it in both sites.
> This will also help avoiding duplicate code in a subsequent change.
>
> Signed-off-by: Filipe Manana <fdmanana@suse.com>
> ---
> fs/btrfs/free-space-tree.c | 50 ++++++++++++++++++++------------------
> 1 file changed, 26 insertions(+), 24 deletions(-)
>
> diff --git a/fs/btrfs/free-space-tree.c b/fs/btrfs/free-space-tree.c
> index c85ca7ce2683..3c8bb95fa044 100644
> --- a/fs/btrfs/free-space-tree.c
> +++ b/fs/btrfs/free-space-tree.c
> @@ -791,32 +791,40 @@ static int remove_free_space_extent(struct btrfs_trans_handle *trans,
> return update_free_space_extent_count(trans, block_group, path, new_extents);
> }
>
> +static int use_bitmaps(struct btrfs_block_group *bg, struct btrfs_path *path)
I think 'using_bitmaps' makes more sense, given the name of the flag.
Ditto for the next patch and the caching variables.
> +{
> + struct btrfs_free_space_info *info;
> + u32 flags;
> +
> + info = btrfs_search_free_space_info(NULL, bg, path, 0);
> + if (IS_ERR(info))
> + return PTR_ERR(info);
> + flags = btrfs_free_space_flags(path->nodes[0], info);
> + btrfs_release_path(path);
> +
> + return (flags & BTRFS_FREE_SPACE_USING_BITMAPS) ? 1 : 0;
> +}
> +
> EXPORT_FOR_TESTS
> int __btrfs_remove_from_free_space_tree(struct btrfs_trans_handle *trans,
> struct btrfs_block_group *block_group,
> struct btrfs_path *path, u64 start, u64 size)
> {
> - struct btrfs_free_space_info *info;
> - u32 flags;
> int ret;
>
> ret = __add_block_group_free_space(trans, block_group, path);
> if (ret)
> return ret;
>
> - info = btrfs_search_free_space_info(NULL, block_group, path, 0);
> - if (IS_ERR(info))
> - return PTR_ERR(info);
> - flags = btrfs_free_space_flags(path->nodes[0], info);
> - btrfs_release_path(path);
> + ret = use_bitmaps(block_group, path);
> + if (ret < 0)
> + return ret;
>
> - if (flags & BTRFS_FREE_SPACE_USING_BITMAPS) {
> + if (ret)
> return modify_free_space_bitmap(trans, block_group, path,
> start, size, true);
> - } else {
> - return remove_free_space_extent(trans, block_group, path,
> - start, size);
> - }
> +
> + return remove_free_space_extent(trans, block_group, path, start, size);
> }
>
> int btrfs_remove_from_free_space_tree(struct btrfs_trans_handle *trans,
> @@ -984,27 +992,21 @@ int __btrfs_add_to_free_space_tree(struct btrfs_trans_handle *trans,
> struct btrfs_block_group *block_group,
> struct btrfs_path *path, u64 start, u64 size)
> {
> - struct btrfs_free_space_info *info;
> - u32 flags;
> int ret;
>
> ret = __add_block_group_free_space(trans, block_group, path);
> if (ret)
> return ret;
>
> - info = btrfs_search_free_space_info(NULL, block_group, path, 0);
> - if (IS_ERR(info))
> - return PTR_ERR(info);
> - flags = btrfs_free_space_flags(path->nodes[0], info);
> - btrfs_release_path(path);
> + ret = use_bitmaps(block_group, path);
> + if (ret < 0)
> + return ret;
>
> - if (flags & BTRFS_FREE_SPACE_USING_BITMAPS) {
> + if (ret)
> return modify_free_space_bitmap(trans, block_group, path,
> start, size, false);
> - } else {
> - return add_free_space_extent(trans, block_group, path, start,
> - size);
> - }
> +
> + return add_free_space_extent(trans, block_group, path, start, size);
> }
>
> int btrfs_add_to_free_space_tree(struct btrfs_trans_handle *trans,
> --
> 2.47.2
>
^ permalink raw reply [flat|nested] 22+ messages in thread
* [PATCH 16/16] btrfs: cache if we are using free space bitmaps for a block group
2025-06-17 16:12 [PATCH 00/16] btrfs: free space tree optimization and cleanups fdmanana
` (14 preceding siblings ...)
2025-06-17 16:13 ` [PATCH 15/16] btrfs: add and use helper to determine if using bitmaps in free space tree fdmanana
@ 2025-06-17 16:13 ` fdmanana
2025-06-17 21:52 ` Boris Burkov
2025-06-18 11:50 ` [PATCH 00/16] btrfs: free space tree optimization and cleanups David Sterba
16 siblings, 1 reply; 22+ messages in thread
From: fdmanana @ 2025-06-17 16:13 UTC (permalink / raw)
To: linux-btrfs
From: Filipe Manana <fdmanana@suse.com>
Every time we add free space to the free space tree or we remove free
space from the free space tree, we do a lookup for the block group's free
space info item in the free space tree. This takes time, navigating the
btree and we may block either on IO when reading extent buffers from disk
or on extent buffer lock contention due to concurrency.
Instead of doing this lookup everytime, cache the result in the block
structure and use it after the first lookup. This adds two boolean members
to the block group structure but doesn't increase the structure's size.
The following script that runs fs_mark was used to measure the time spent
on run_delayed_tree_ref(), since down that call chain we have calls to
add and remove free space to/from the free space tree (calls to
btrfs_add_to_free_space_tree() and btrfs_remove_from_free_space_tree()):
$ cat test.sh
#!/bin/bash
DEV=/dev/nullb0
MNT=/mnt
FILES=100000
THREADS=$(nproc --all)
echo "performance" | \
tee /sys/devices/system/cpu/cpu*/cpufreq/scaling_governor
umount $DEV &> /dev/null
mkfs.btrfs -f $DEV
mount -o ssd $DEV $MNT
OPTS="-S 0 -L 5 -n $FILES -s 0 -t $THREADS -k"
for ((i = 1; i <= $THREADS; i++)); do
OPTS="$OPTS -d $MNT/d$i"
done
fs_mark $OPTS
umount $MNT
This is a heavy metadata test as it's exercising only file creation, so a
lot of allocations of metadata extents, creating delayed refs for adding
new metadata extents and dropping existing ones due to COW. The results
of the times it took to execute run_delayed_tree_ref(), in nanoseconds,
are the following.
Before this change:
Range: 1868.000 - 6482857.000; Mean: 10231.430; Median: 7005.000; Stddev: 27993.173
Percentiles: 90th: 13342.000; 95th: 23279.000; 99th: 82448.000
1868.000 - 4222.038: 270696 ############
4222.038 - 9541.029: 1201327 #####################################################
9541.029 - 21559.383: 385436 #################
21559.383 - 48715.063: 64942 ###
48715.063 - 110073.800: 31454 #
110073.800 - 248714.944: 8218 |
248714.944 - 561977.042: 1030 |
561977.042 - 1269798.254: 295 |
1269798.254 - 2869132.711: 116 |
2869132.711 - 6482857.000: 28 |
After this change:
Range: 1554.000 - 4557014.000; Mean: 9168.164; Median: 6391.000; Stddev: 21467.060
Percentiles: 90th: 12478.000; 95th: 20964.000; 99th: 72234.000
1554.000 - 3453.820: 219004 ############
3453.820 - 7674.743: 980645 #####################################################
7674.743 - 17052.574: 552486 ##############################
17052.574 - 37887.762: 68558 ####
37887.762 - 84178.322: 31557 ##
84178.322 - 187024.331: 12102 #
187024.331 - 415522.355: 1364 |
415522.355 - 923187.626: 256 |
923187.626 - 2051092.468: 125 |
2051092.468 - 4557014.000: 21 |
Signed-off-by: Filipe Manana <fdmanana@suse.com>
---
fs/btrfs/block-group.h | 5 +++++
fs/btrfs/free-space-tree.c | 12 +++++++++++-
2 files changed, 16 insertions(+), 1 deletion(-)
diff --git a/fs/btrfs/block-group.h b/fs/btrfs/block-group.h
index aa176cc9a324..8a8f1fff7e5b 100644
--- a/fs/btrfs/block-group.h
+++ b/fs/btrfs/block-group.h
@@ -246,6 +246,11 @@ struct btrfs_block_group {
/* Lock for free space tree operations. */
struct mutex free_space_lock;
+ /* Protected by @free_space_lock. */
+ bool use_free_space_bitmaps;
+ /* Protected by @free_space_lock. */
+ bool use_free_space_bitmaps_cached;
+
/*
* Number of extents in this block group used for swap files.
* All accesses protected by the spinlock 'lock'.
diff --git a/fs/btrfs/free-space-tree.c b/fs/btrfs/free-space-tree.c
index 3c8bb95fa044..1bd07e91fd5a 100644
--- a/fs/btrfs/free-space-tree.c
+++ b/fs/btrfs/free-space-tree.c
@@ -287,6 +287,8 @@ int btrfs_convert_free_space_to_bitmaps(struct btrfs_trans_handle *trans,
leaf = path->nodes[0];
flags = btrfs_free_space_flags(leaf, info);
flags |= BTRFS_FREE_SPACE_USING_BITMAPS;
+ block_group->use_free_space_bitmaps = true;
+ block_group->use_free_space_bitmaps_cached = true;
btrfs_set_free_space_flags(leaf, info, flags);
expected_extent_count = btrfs_free_space_extent_count(leaf, info);
btrfs_release_path(path);
@@ -434,6 +436,8 @@ int btrfs_convert_free_space_to_extents(struct btrfs_trans_handle *trans,
leaf = path->nodes[0];
flags = btrfs_free_space_flags(leaf, info);
flags &= ~BTRFS_FREE_SPACE_USING_BITMAPS;
+ block_group->use_free_space_bitmaps = false;
+ block_group->use_free_space_bitmaps_cached = true;
btrfs_set_free_space_flags(leaf, info, flags);
expected_extent_count = btrfs_free_space_extent_count(leaf, info);
btrfs_release_path(path);
@@ -796,13 +800,19 @@ static int use_bitmaps(struct btrfs_block_group *bg, struct btrfs_path *path)
struct btrfs_free_space_info *info;
u32 flags;
+ if (bg->use_free_space_bitmaps_cached)
+ return bg->use_free_space_bitmaps;
+
info = btrfs_search_free_space_info(NULL, bg, path, 0);
if (IS_ERR(info))
return PTR_ERR(info);
flags = btrfs_free_space_flags(path->nodes[0], info);
btrfs_release_path(path);
- return (flags & BTRFS_FREE_SPACE_USING_BITMAPS) ? 1 : 0;
+ bg->use_free_space_bitmaps = (flags & BTRFS_FREE_SPACE_USING_BITMAPS);
+ bg->use_free_space_bitmaps_cached = true;
+
+ return bg->use_free_space_bitmaps;
}
EXPORT_FOR_TESTS
--
2.47.2
^ permalink raw reply related [flat|nested] 22+ messages in thread* Re: [PATCH 16/16] btrfs: cache if we are using free space bitmaps for a block group
2025-06-17 16:13 ` [PATCH 16/16] btrfs: cache if we are using free space bitmaps for a block group fdmanana
@ 2025-06-17 21:52 ` Boris Burkov
2025-06-17 21:59 ` Filipe Manana
0 siblings, 1 reply; 22+ messages in thread
From: Boris Burkov @ 2025-06-17 21:52 UTC (permalink / raw)
To: fdmanana; +Cc: linux-btrfs
On Tue, Jun 17, 2025 at 05:13:11PM +0100, fdmanana@kernel.org wrote:
> From: Filipe Manana <fdmanana@suse.com>
>
> Every time we add free space to the free space tree or we remove free
> space from the free space tree, we do a lookup for the block group's free
> space info item in the free space tree. This takes time, navigating the
> btree and we may block either on IO when reading extent buffers from disk
> or on extent buffer lock contention due to concurrency.
>
> Instead of doing this lookup everytime, cache the result in the block
> structure and use it after the first lookup. This adds two boolean members
> to the block group structure but doesn't increase the structure's size.
>
> The following script that runs fs_mark was used to measure the time spent
> on run_delayed_tree_ref(), since down that call chain we have calls to
> add and remove free space to/from the free space tree (calls to
> btrfs_add_to_free_space_tree() and btrfs_remove_from_free_space_tree()):
>
> $ cat test.sh
> #!/bin/bash
>
> DEV=/dev/nullb0
> MNT=/mnt
> FILES=100000
> THREADS=$(nproc --all)
>
> echo "performance" | \
> tee /sys/devices/system/cpu/cpu*/cpufreq/scaling_governor
>
> umount $DEV &> /dev/null
> mkfs.btrfs -f $DEV
> mount -o ssd $DEV $MNT
>
> OPTS="-S 0 -L 5 -n $FILES -s 0 -t $THREADS -k"
> for ((i = 1; i <= $THREADS; i++)); do
> OPTS="$OPTS -d $MNT/d$i"
> done
>
> fs_mark $OPTS
>
> umount $MNT
>
> This is a heavy metadata test as it's exercising only file creation, so a
> lot of allocations of metadata extents, creating delayed refs for adding
> new metadata extents and dropping existing ones due to COW. The results
> of the times it took to execute run_delayed_tree_ref(), in nanoseconds,
> are the following.
>
> Before this change:
>
> Range: 1868.000 - 6482857.000; Mean: 10231.430; Median: 7005.000; Stddev: 27993.173
> Percentiles: 90th: 13342.000; 95th: 23279.000; 99th: 82448.000
> 1868.000 - 4222.038: 270696 ############
> 4222.038 - 9541.029: 1201327 #####################################################
> 9541.029 - 21559.383: 385436 #################
> 21559.383 - 48715.063: 64942 ###
> 48715.063 - 110073.800: 31454 #
> 110073.800 - 248714.944: 8218 |
> 248714.944 - 561977.042: 1030 |
> 561977.042 - 1269798.254: 295 |
> 1269798.254 - 2869132.711: 116 |
> 2869132.711 - 6482857.000: 28 |
>
> After this change:
>
> Range: 1554.000 - 4557014.000; Mean: 9168.164; Median: 6391.000; Stddev: 21467.060
> Percentiles: 90th: 12478.000; 95th: 20964.000; 99th: 72234.000
> 1554.000 - 3453.820: 219004 ############
> 3453.820 - 7674.743: 980645 #####################################################
> 7674.743 - 17052.574: 552486 ##############################
> 17052.574 - 37887.762: 68558 ####
> 37887.762 - 84178.322: 31557 ##
> 84178.322 - 187024.331: 12102 #
> 187024.331 - 415522.355: 1364 |
> 415522.355 - 923187.626: 256 |
> 923187.626 - 2051092.468: 125 |
> 2051092.468 - 4557014.000: 21 |
>
> Signed-off-by: Filipe Manana <fdmanana@suse.com>
> ---
> fs/btrfs/block-group.h | 5 +++++
> fs/btrfs/free-space-tree.c | 12 +++++++++++-
> 2 files changed, 16 insertions(+), 1 deletion(-)
>
> diff --git a/fs/btrfs/block-group.h b/fs/btrfs/block-group.h
> index aa176cc9a324..8a8f1fff7e5b 100644
> --- a/fs/btrfs/block-group.h
> +++ b/fs/btrfs/block-group.h
> @@ -246,6 +246,11 @@ struct btrfs_block_group {
> /* Lock for free space tree operations. */
> struct mutex free_space_lock;
>
> + /* Protected by @free_space_lock. */
> + bool use_free_space_bitmaps;
> + /* Protected by @free_space_lock. */
> + bool use_free_space_bitmaps_cached;
> +
> /*
> * Number of extents in this block group used for swap files.
> * All accesses protected by the spinlock 'lock'.
> diff --git a/fs/btrfs/free-space-tree.c b/fs/btrfs/free-space-tree.c
> index 3c8bb95fa044..1bd07e91fd5a 100644
> --- a/fs/btrfs/free-space-tree.c
> +++ b/fs/btrfs/free-space-tree.c
> @@ -287,6 +287,8 @@ int btrfs_convert_free_space_to_bitmaps(struct btrfs_trans_handle *trans,
> leaf = path->nodes[0];
> flags = btrfs_free_space_flags(leaf, info);
> flags |= BTRFS_FREE_SPACE_USING_BITMAPS;
> + block_group->use_free_space_bitmaps = true;
> + block_group->use_free_space_bitmaps_cached = true;
> btrfs_set_free_space_flags(leaf, info, flags);
> expected_extent_count = btrfs_free_space_extent_count(leaf, info);
> btrfs_release_path(path);
> @@ -434,6 +436,8 @@ int btrfs_convert_free_space_to_extents(struct btrfs_trans_handle *trans,
> leaf = path->nodes[0];
> flags = btrfs_free_space_flags(leaf, info);
> flags &= ~BTRFS_FREE_SPACE_USING_BITMAPS;
> + block_group->use_free_space_bitmaps = false;
> + block_group->use_free_space_bitmaps_cached = true;
> btrfs_set_free_space_flags(leaf, info, flags);
> expected_extent_count = btrfs_free_space_extent_count(leaf, info);
> btrfs_release_path(path);
> @@ -796,13 +800,19 @@ static int use_bitmaps(struct btrfs_block_group *bg, struct btrfs_path *path)
> struct btrfs_free_space_info *info;
> u32 flags;
>
> + if (bg->use_free_space_bitmaps_cached)
> + return bg->use_free_space_bitmaps;
> +
I'm a little worried about what happens if the reader observes the
writes out of order.
i.e., say T1 is calling btrfs_convert_free_space_to_bitmaps() and T2 is
calling use_bitmaps(). Then if T2 observes use_free_space_bitmaps_cached
set to true but not use_free_space_bitmaps set to true, it will get the
wrong value.
Or is there some higher level locking that I missed protecting us?
Thanks,
Boris
> info = btrfs_search_free_space_info(NULL, bg, path, 0);
> if (IS_ERR(info))
> return PTR_ERR(info);
> flags = btrfs_free_space_flags(path->nodes[0], info);
> btrfs_release_path(path);
>
> - return (flags & BTRFS_FREE_SPACE_USING_BITMAPS) ? 1 : 0;
> + bg->use_free_space_bitmaps = (flags & BTRFS_FREE_SPACE_USING_BITMAPS);
> + bg->use_free_space_bitmaps_cached = true;
> +
> + return bg->use_free_space_bitmaps;
> }
>
> EXPORT_FOR_TESTS
> --
> 2.47.2
>
^ permalink raw reply [flat|nested] 22+ messages in thread* Re: [PATCH 16/16] btrfs: cache if we are using free space bitmaps for a block group
2025-06-17 21:52 ` Boris Burkov
@ 2025-06-17 21:59 ` Filipe Manana
2025-06-17 22:12 ` Boris Burkov
0 siblings, 1 reply; 22+ messages in thread
From: Filipe Manana @ 2025-06-17 21:59 UTC (permalink / raw)
To: Boris Burkov; +Cc: linux-btrfs
On Tue, Jun 17, 2025 at 10:51 PM Boris Burkov <boris@bur.io> wrote:
>
> On Tue, Jun 17, 2025 at 05:13:11PM +0100, fdmanana@kernel.org wrote:
> > From: Filipe Manana <fdmanana@suse.com>
> >
> > Every time we add free space to the free space tree or we remove free
> > space from the free space tree, we do a lookup for the block group's free
> > space info item in the free space tree. This takes time, navigating the
> > btree and we may block either on IO when reading extent buffers from disk
> > or on extent buffer lock contention due to concurrency.
> >
> > Instead of doing this lookup everytime, cache the result in the block
> > structure and use it after the first lookup. This adds two boolean members
> > to the block group structure but doesn't increase the structure's size.
> >
> > The following script that runs fs_mark was used to measure the time spent
> > on run_delayed_tree_ref(), since down that call chain we have calls to
> > add and remove free space to/from the free space tree (calls to
> > btrfs_add_to_free_space_tree() and btrfs_remove_from_free_space_tree()):
> >
> > $ cat test.sh
> > #!/bin/bash
> >
> > DEV=/dev/nullb0
> > MNT=/mnt
> > FILES=100000
> > THREADS=$(nproc --all)
> >
> > echo "performance" | \
> > tee /sys/devices/system/cpu/cpu*/cpufreq/scaling_governor
> >
> > umount $DEV &> /dev/null
> > mkfs.btrfs -f $DEV
> > mount -o ssd $DEV $MNT
> >
> > OPTS="-S 0 -L 5 -n $FILES -s 0 -t $THREADS -k"
> > for ((i = 1; i <= $THREADS; i++)); do
> > OPTS="$OPTS -d $MNT/d$i"
> > done
> >
> > fs_mark $OPTS
> >
> > umount $MNT
> >
> > This is a heavy metadata test as it's exercising only file creation, so a
> > lot of allocations of metadata extents, creating delayed refs for adding
> > new metadata extents and dropping existing ones due to COW. The results
> > of the times it took to execute run_delayed_tree_ref(), in nanoseconds,
> > are the following.
> >
> > Before this change:
> >
> > Range: 1868.000 - 6482857.000; Mean: 10231.430; Median: 7005.000; Stddev: 27993.173
> > Percentiles: 90th: 13342.000; 95th: 23279.000; 99th: 82448.000
> > 1868.000 - 4222.038: 270696 ############
> > 4222.038 - 9541.029: 1201327 #####################################################
> > 9541.029 - 21559.383: 385436 #################
> > 21559.383 - 48715.063: 64942 ###
> > 48715.063 - 110073.800: 31454 #
> > 110073.800 - 248714.944: 8218 |
> > 248714.944 - 561977.042: 1030 |
> > 561977.042 - 1269798.254: 295 |
> > 1269798.254 - 2869132.711: 116 |
> > 2869132.711 - 6482857.000: 28 |
> >
> > After this change:
> >
> > Range: 1554.000 - 4557014.000; Mean: 9168.164; Median: 6391.000; Stddev: 21467.060
> > Percentiles: 90th: 12478.000; 95th: 20964.000; 99th: 72234.000
> > 1554.000 - 3453.820: 219004 ############
> > 3453.820 - 7674.743: 980645 #####################################################
> > 7674.743 - 17052.574: 552486 ##############################
> > 17052.574 - 37887.762: 68558 ####
> > 37887.762 - 84178.322: 31557 ##
> > 84178.322 - 187024.331: 12102 #
> > 187024.331 - 415522.355: 1364 |
> > 415522.355 - 923187.626: 256 |
> > 923187.626 - 2051092.468: 125 |
> > 2051092.468 - 4557014.000: 21 |
> >
> > Signed-off-by: Filipe Manana <fdmanana@suse.com>
> > ---
> > fs/btrfs/block-group.h | 5 +++++
> > fs/btrfs/free-space-tree.c | 12 +++++++++++-
> > 2 files changed, 16 insertions(+), 1 deletion(-)
> >
> > diff --git a/fs/btrfs/block-group.h b/fs/btrfs/block-group.h
> > index aa176cc9a324..8a8f1fff7e5b 100644
> > --- a/fs/btrfs/block-group.h
> > +++ b/fs/btrfs/block-group.h
> > @@ -246,6 +246,11 @@ struct btrfs_block_group {
> > /* Lock for free space tree operations. */
> > struct mutex free_space_lock;
> >
> > + /* Protected by @free_space_lock. */
> > + bool use_free_space_bitmaps;
> > + /* Protected by @free_space_lock. */
> > + bool use_free_space_bitmaps_cached;
> > +
> > /*
> > * Number of extents in this block group used for swap files.
> > * All accesses protected by the spinlock 'lock'.
> > diff --git a/fs/btrfs/free-space-tree.c b/fs/btrfs/free-space-tree.c
> > index 3c8bb95fa044..1bd07e91fd5a 100644
> > --- a/fs/btrfs/free-space-tree.c
> > +++ b/fs/btrfs/free-space-tree.c
> > @@ -287,6 +287,8 @@ int btrfs_convert_free_space_to_bitmaps(struct btrfs_trans_handle *trans,
> > leaf = path->nodes[0];
> > flags = btrfs_free_space_flags(leaf, info);
> > flags |= BTRFS_FREE_SPACE_USING_BITMAPS;
> > + block_group->use_free_space_bitmaps = true;
> > + block_group->use_free_space_bitmaps_cached = true;
> > btrfs_set_free_space_flags(leaf, info, flags);
> > expected_extent_count = btrfs_free_space_extent_count(leaf, info);
> > btrfs_release_path(path);
> > @@ -434,6 +436,8 @@ int btrfs_convert_free_space_to_extents(struct btrfs_trans_handle *trans,
> > leaf = path->nodes[0];
> > flags = btrfs_free_space_flags(leaf, info);
> > flags &= ~BTRFS_FREE_SPACE_USING_BITMAPS;
> > + block_group->use_free_space_bitmaps = false;
> > + block_group->use_free_space_bitmaps_cached = true;
> > btrfs_set_free_space_flags(leaf, info, flags);
> > expected_extent_count = btrfs_free_space_extent_count(leaf, info);
> > btrfs_release_path(path);
> > @@ -796,13 +800,19 @@ static int use_bitmaps(struct btrfs_block_group *bg, struct btrfs_path *path)
> > struct btrfs_free_space_info *info;
> > u32 flags;
> >
> > + if (bg->use_free_space_bitmaps_cached)
> > + return bg->use_free_space_bitmaps;
> > +
>
> I'm a little worried about what happens if the reader observes the
> writes out of order.
>
> i.e., say T1 is calling btrfs_convert_free_space_to_bitmaps() and T2 is
> calling use_bitmaps(). Then if T2 observes use_free_space_bitmaps_cached
> set to true but not use_free_space_bitmaps set to true, it will get the
> wrong value.
>
> Or is there some higher level locking that I missed protecting us?
Yes, there is. It's the block group's free_space_lock mutex, taken at
any entry point that modifies the free space tree.
Thanks.
>
> Thanks,
> Boris
>
> > info = btrfs_search_free_space_info(NULL, bg, path, 0);
> > if (IS_ERR(info))
> > return PTR_ERR(info);
> > flags = btrfs_free_space_flags(path->nodes[0], info);
> > btrfs_release_path(path);
> >
> > - return (flags & BTRFS_FREE_SPACE_USING_BITMAPS) ? 1 : 0;
> > + bg->use_free_space_bitmaps = (flags & BTRFS_FREE_SPACE_USING_BITMAPS);
> > + bg->use_free_space_bitmaps_cached = true;
> > +
> > + return bg->use_free_space_bitmaps;
> > }
> >
> > EXPORT_FOR_TESTS
> > --
> > 2.47.2
> >
^ permalink raw reply [flat|nested] 22+ messages in thread* Re: [PATCH 16/16] btrfs: cache if we are using free space bitmaps for a block group
2025-06-17 21:59 ` Filipe Manana
@ 2025-06-17 22:12 ` Boris Burkov
0 siblings, 0 replies; 22+ messages in thread
From: Boris Burkov @ 2025-06-17 22:12 UTC (permalink / raw)
To: Filipe Manana; +Cc: linux-btrfs
On Tue, Jun 17, 2025 at 10:59:04PM +0100, Filipe Manana wrote:
> On Tue, Jun 17, 2025 at 10:51 PM Boris Burkov <boris@bur.io> wrote:
> >
> > On Tue, Jun 17, 2025 at 05:13:11PM +0100, fdmanana@kernel.org wrote:
> > > From: Filipe Manana <fdmanana@suse.com>
> > >
> > > Every time we add free space to the free space tree or we remove free
> > > space from the free space tree, we do a lookup for the block group's free
> > > space info item in the free space tree. This takes time, navigating the
> > > btree and we may block either on IO when reading extent buffers from disk
> > > or on extent buffer lock contention due to concurrency.
> > >
> > > Instead of doing this lookup everytime, cache the result in the block
> > > structure and use it after the first lookup. This adds two boolean members
> > > to the block group structure but doesn't increase the structure's size.
> > >
> > > The following script that runs fs_mark was used to measure the time spent
> > > on run_delayed_tree_ref(), since down that call chain we have calls to
> > > add and remove free space to/from the free space tree (calls to
> > > btrfs_add_to_free_space_tree() and btrfs_remove_from_free_space_tree()):
> > >
> > > $ cat test.sh
> > > #!/bin/bash
> > >
> > > DEV=/dev/nullb0
> > > MNT=/mnt
> > > FILES=100000
> > > THREADS=$(nproc --all)
> > >
> > > echo "performance" | \
> > > tee /sys/devices/system/cpu/cpu*/cpufreq/scaling_governor
> > >
> > > umount $DEV &> /dev/null
> > > mkfs.btrfs -f $DEV
> > > mount -o ssd $DEV $MNT
> > >
> > > OPTS="-S 0 -L 5 -n $FILES -s 0 -t $THREADS -k"
> > > for ((i = 1; i <= $THREADS; i++)); do
> > > OPTS="$OPTS -d $MNT/d$i"
> > > done
> > >
> > > fs_mark $OPTS
> > >
> > > umount $MNT
> > >
> > > This is a heavy metadata test as it's exercising only file creation, so a
> > > lot of allocations of metadata extents, creating delayed refs for adding
> > > new metadata extents and dropping existing ones due to COW. The results
> > > of the times it took to execute run_delayed_tree_ref(), in nanoseconds,
> > > are the following.
> > >
> > > Before this change:
> > >
> > > Range: 1868.000 - 6482857.000; Mean: 10231.430; Median: 7005.000; Stddev: 27993.173
> > > Percentiles: 90th: 13342.000; 95th: 23279.000; 99th: 82448.000
> > > 1868.000 - 4222.038: 270696 ############
> > > 4222.038 - 9541.029: 1201327 #####################################################
> > > 9541.029 - 21559.383: 385436 #################
> > > 21559.383 - 48715.063: 64942 ###
> > > 48715.063 - 110073.800: 31454 #
> > > 110073.800 - 248714.944: 8218 |
> > > 248714.944 - 561977.042: 1030 |
> > > 561977.042 - 1269798.254: 295 |
> > > 1269798.254 - 2869132.711: 116 |
> > > 2869132.711 - 6482857.000: 28 |
> > >
> > > After this change:
> > >
> > > Range: 1554.000 - 4557014.000; Mean: 9168.164; Median: 6391.000; Stddev: 21467.060
> > > Percentiles: 90th: 12478.000; 95th: 20964.000; 99th: 72234.000
> > > 1554.000 - 3453.820: 219004 ############
> > > 3453.820 - 7674.743: 980645 #####################################################
> > > 7674.743 - 17052.574: 552486 ##############################
> > > 17052.574 - 37887.762: 68558 ####
> > > 37887.762 - 84178.322: 31557 ##
> > > 84178.322 - 187024.331: 12102 #
> > > 187024.331 - 415522.355: 1364 |
> > > 415522.355 - 923187.626: 256 |
> > > 923187.626 - 2051092.468: 125 |
> > > 2051092.468 - 4557014.000: 21 |
> > >
> > > Signed-off-by: Filipe Manana <fdmanana@suse.com>
> > > ---
> > > fs/btrfs/block-group.h | 5 +++++
> > > fs/btrfs/free-space-tree.c | 12 +++++++++++-
> > > 2 files changed, 16 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/fs/btrfs/block-group.h b/fs/btrfs/block-group.h
> > > index aa176cc9a324..8a8f1fff7e5b 100644
> > > --- a/fs/btrfs/block-group.h
> > > +++ b/fs/btrfs/block-group.h
> > > @@ -246,6 +246,11 @@ struct btrfs_block_group {
> > > /* Lock for free space tree operations. */
> > > struct mutex free_space_lock;
> > >
> > > + /* Protected by @free_space_lock. */
> > > + bool use_free_space_bitmaps;
> > > + /* Protected by @free_space_lock. */
> > > + bool use_free_space_bitmaps_cached;
> > > +
> > > /*
> > > * Number of extents in this block group used for swap files.
> > > * All accesses protected by the spinlock 'lock'.
> > > diff --git a/fs/btrfs/free-space-tree.c b/fs/btrfs/free-space-tree.c
> > > index 3c8bb95fa044..1bd07e91fd5a 100644
> > > --- a/fs/btrfs/free-space-tree.c
> > > +++ b/fs/btrfs/free-space-tree.c
> > > @@ -287,6 +287,8 @@ int btrfs_convert_free_space_to_bitmaps(struct btrfs_trans_handle *trans,
> > > leaf = path->nodes[0];
> > > flags = btrfs_free_space_flags(leaf, info);
> > > flags |= BTRFS_FREE_SPACE_USING_BITMAPS;
> > > + block_group->use_free_space_bitmaps = true;
> > > + block_group->use_free_space_bitmaps_cached = true;
> > > btrfs_set_free_space_flags(leaf, info, flags);
> > > expected_extent_count = btrfs_free_space_extent_count(leaf, info);
> > > btrfs_release_path(path);
> > > @@ -434,6 +436,8 @@ int btrfs_convert_free_space_to_extents(struct btrfs_trans_handle *trans,
> > > leaf = path->nodes[0];
> > > flags = btrfs_free_space_flags(leaf, info);
> > > flags &= ~BTRFS_FREE_SPACE_USING_BITMAPS;
> > > + block_group->use_free_space_bitmaps = false;
> > > + block_group->use_free_space_bitmaps_cached = true;
> > > btrfs_set_free_space_flags(leaf, info, flags);
> > > expected_extent_count = btrfs_free_space_extent_count(leaf, info);
> > > btrfs_release_path(path);
> > > @@ -796,13 +800,19 @@ static int use_bitmaps(struct btrfs_block_group *bg, struct btrfs_path *path)
> > > struct btrfs_free_space_info *info;
> > > u32 flags;
> > >
> > > + if (bg->use_free_space_bitmaps_cached)
> > > + return bg->use_free_space_bitmaps;
> > > +
> >
> > I'm a little worried about what happens if the reader observes the
> > writes out of order.
> >
> > i.e., say T1 is calling btrfs_convert_free_space_to_bitmaps() and T2 is
> > calling use_bitmaps(). Then if T2 observes use_free_space_bitmaps_cached
> > set to true but not use_free_space_bitmaps set to true, it will get the
> > wrong value.
> >
> > Or is there some higher level locking that I missed protecting us?
>
> Yes, there is. It's the block group's free_space_lock mutex, taken at
> any entry point that modifies the free space tree.
>
> Thanks.
>
Ah, yup, looks good, thanks.
You can add
Reviewed-by: Boris Burkov <boris@bur.io>
to the whole series.
> >
> > Thanks,
> > Boris
> >
> > > info = btrfs_search_free_space_info(NULL, bg, path, 0);
> > > if (IS_ERR(info))
> > > return PTR_ERR(info);
> > > flags = btrfs_free_space_flags(path->nodes[0], info);
> > > btrfs_release_path(path);
> > >
> > > - return (flags & BTRFS_FREE_SPACE_USING_BITMAPS) ? 1 : 0;
> > > + bg->use_free_space_bitmaps = (flags & BTRFS_FREE_SPACE_USING_BITMAPS);
> > > + bg->use_free_space_bitmaps_cached = true;
> > > +
> > > + return bg->use_free_space_bitmaps;
> > > }
> > >
> > > EXPORT_FOR_TESTS
> > > --
> > > 2.47.2
> > >
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH 00/16] btrfs: free space tree optimization and cleanups
2025-06-17 16:12 [PATCH 00/16] btrfs: free space tree optimization and cleanups fdmanana
` (15 preceding siblings ...)
2025-06-17 16:13 ` [PATCH 16/16] btrfs: cache if we are using free space bitmaps for a block group fdmanana
@ 2025-06-18 11:50 ` David Sterba
16 siblings, 0 replies; 22+ messages in thread
From: David Sterba @ 2025-06-18 11:50 UTC (permalink / raw)
To: fdmanana; +Cc: linux-btrfs
On Tue, Jun 17, 2025 at 05:12:55PM +0100, fdmanana@kernel.org wrote:
> From: Filipe Manana <fdmanana@suse.com>
>
> A free space tree optimization (last patch in the series), and the rest
> mostly cleanups and preparatory work. Details in the change logs.
>
> Filipe Manana (16):
> btrfs: remove pointless out label from add_new_free_space_info()
> btrfs: remove pointless out label from update_free_space_extent_count()
> btrfs: make extent_buffer_test_bit() return a boolean instead
> btrfs: make free_space_test_bit() return a boolean instead
> btrfs: remove pointless out label from modify_free_space_bitmap()
> btrfs: remove pointless out label from remove_free_space_extent()
> btrfs: remove pointless out label from add_free_space_extent()
> btrfs: remove pointless out label from load_free_space_bitmaps()
> btrfs: remove pointless out label from load_free_space_extents()
> btrfs: add btrfs prefix to free space tree exported functions
> btrfs: rename free_space_set_bits() and make it less confusing
> btrfs: turn remove argument of modify_free_space_bitmap() to boolean
> btrfs: avoid double slot decrement at btrfs_convert_free_space_to_extents()
> btrfs: use fs_info from local variable in btrfs_convert_free_space_to_extents()
> btrfs: add and use helper to determine if using bitmaps in free space tree
> btrfs: cache if we are using free space bitmaps for a block group
Reviewed-by: David Sterba <dsterba@suse.com>
^ permalink raw reply [flat|nested] 22+ messages in thread