* [PATCH 1/8] btrfs: remove BUG_ON()'s in add_new_free_space()
2023-06-30 15:03 [PATCH 0/8] btrfs: remove a couple BUG_ON()s and cleanups fdmanana
@ 2023-06-30 15:03 ` fdmanana
2023-06-30 15:03 ` [PATCH 2/8] btrfs: update documentation for add_new_free_space() fdmanana
` (7 subsequent siblings)
8 siblings, 0 replies; 12+ messages in thread
From: fdmanana @ 2023-06-30 15:03 UTC (permalink / raw)
To: linux-btrfs
From: Filipe Manana <fdmanana@suse.com>
At add_new_free_space() we have these BUG_ON()'s that are there to deal
with any failure to add free space to the in memory free space cache.
Such failures are mostly -ENOMEM that should be very rare. However there's
no need to have these BUG_ON()'s, we can just return any error to the
caller and all callers and their upper call chain are already dealing with
errors.
So just make add_new_free_space() return any errors, while removing the
BUG_ON()'s, and returning the total amount of added free space to an
optional u64 pointer argument.
Reported-by: syzbot+3ba856e07b7127889d8c@syzkaller.appspotmail.com
Link: https://lore.kernel.org/linux-btrfs/000000000000e9cb8305ff4e8327@google.com/
Signed-off-by: Filipe Manana <fdmanana@suse.com>
---
fs/btrfs/block-group.c | 51 +++++++++++++++++++++++++-------------
fs/btrfs/block-group.h | 4 +--
fs/btrfs/free-space-tree.c | 27 ++++++++++++++------
3 files changed, 56 insertions(+), 26 deletions(-)
diff --git a/fs/btrfs/block-group.c b/fs/btrfs/block-group.c
index f53297726238..aab6667cc580 100644
--- a/fs/btrfs/block-group.c
+++ b/fs/btrfs/block-group.c
@@ -499,12 +499,16 @@ static void fragment_free_space(struct btrfs_block_group *block_group)
* used yet since their free space will be released as soon as the transaction
* commits.
*/
-u64 add_new_free_space(struct btrfs_block_group *block_group, u64 start, u64 end)
+int add_new_free_space(struct btrfs_block_group *block_group, u64 start, u64 end,
+ u64 *total_added_ret)
{
struct btrfs_fs_info *info = block_group->fs_info;
- u64 extent_start, extent_end, size, total_added = 0;
+ u64 extent_start, extent_end, size;
int ret;
+ if (total_added_ret)
+ *total_added_ret = 0;
+
while (start < end) {
ret = find_first_extent_bit(&info->excluded_extents, start,
&extent_start, &extent_end,
@@ -517,10 +521,12 @@ u64 add_new_free_space(struct btrfs_block_group *block_group, u64 start, u64 end
start = extent_end + 1;
} else if (extent_start > start && extent_start < end) {
size = extent_start - start;
- total_added += size;
ret = btrfs_add_free_space_async_trimmed(block_group,
start, size);
- BUG_ON(ret); /* -ENOMEM or logic error */
+ if (ret)
+ return ret;
+ if (total_added_ret)
+ *total_added_ret += size;
start = extent_end + 1;
} else {
break;
@@ -529,13 +535,15 @@ u64 add_new_free_space(struct btrfs_block_group *block_group, u64 start, u64 end
if (start < end) {
size = end - start;
- total_added += size;
ret = btrfs_add_free_space_async_trimmed(block_group, start,
size);
- BUG_ON(ret); /* -ENOMEM or logic error */
+ if (ret)
+ return ret;
+ if (total_added_ret)
+ *total_added_ret += size;
}
- return total_added;
+ return 0;
}
/*
@@ -779,8 +787,13 @@ static int load_extent_tree_free(struct btrfs_caching_control *caching_ctl)
if (key.type == BTRFS_EXTENT_ITEM_KEY ||
key.type == BTRFS_METADATA_ITEM_KEY) {
- total_found += add_new_free_space(block_group, last,
- key.objectid);
+ u64 space_added;
+
+ ret = add_new_free_space(block_group, last, key.objectid,
+ &space_added);
+ if (ret)
+ goto out;
+ total_found += space_added;
if (key.type == BTRFS_METADATA_ITEM_KEY)
last = key.objectid +
fs_info->nodesize;
@@ -795,11 +808,10 @@ static int load_extent_tree_free(struct btrfs_caching_control *caching_ctl)
}
path->slots[0]++;
}
- ret = 0;
-
- total_found += add_new_free_space(block_group, last,
- block_group->start + block_group->length);
+ ret = add_new_free_space(block_group, last,
+ block_group->start + block_group->length,
+ NULL);
out:
btrfs_free_path(path);
return ret;
@@ -2293,9 +2305,11 @@ static int read_one_block_group(struct btrfs_fs_info *info,
btrfs_free_excluded_extents(cache);
} else if (cache->used == 0) {
cache->cached = BTRFS_CACHE_FINISHED;
- add_new_free_space(cache, cache->start,
- cache->start + cache->length);
+ ret = add_new_free_space(cache, cache->start,
+ cache->start + cache->length, NULL);
btrfs_free_excluded_extents(cache);
+ if (ret)
+ goto error;
}
ret = btrfs_add_block_group_cache(info, cache);
@@ -2739,9 +2753,12 @@ struct btrfs_block_group *btrfs_make_block_group(struct btrfs_trans_handle *tran
return ERR_PTR(ret);
}
- add_new_free_space(cache, chunk_offset, chunk_offset + size);
-
+ ret = add_new_free_space(cache, chunk_offset, chunk_offset + size, NULL);
btrfs_free_excluded_extents(cache);
+ if (ret) {
+ btrfs_put_block_group(cache);
+ return ERR_PTR(ret);
+ }
/*
* Ensure the corresponding space_info object is created and
diff --git a/fs/btrfs/block-group.h b/fs/btrfs/block-group.h
index 0ed31112d932..a6e7d66b9d80 100644
--- a/fs/btrfs/block-group.h
+++ b/fs/btrfs/block-group.h
@@ -289,8 +289,8 @@ int btrfs_cache_block_group(struct btrfs_block_group *cache, bool wait);
void btrfs_put_caching_control(struct btrfs_caching_control *ctl);
struct btrfs_caching_control *btrfs_get_caching_control(
struct btrfs_block_group *cache);
-u64 add_new_free_space(struct btrfs_block_group *block_group,
- u64 start, u64 end);
+int add_new_free_space(struct btrfs_block_group *block_group,
+ u64 start, u64 end, u64 *total_added_ret);
struct btrfs_trans_handle *btrfs_start_trans_remove_block_group(
struct btrfs_fs_info *fs_info,
const u64 chunk_offset);
diff --git a/fs/btrfs/free-space-tree.c b/fs/btrfs/free-space-tree.c
index 045ddce32eca..53b4ad1abdbc 100644
--- a/fs/btrfs/free-space-tree.c
+++ b/fs/btrfs/free-space-tree.c
@@ -1515,9 +1515,15 @@ static int load_free_space_bitmaps(struct btrfs_caching_control *caching_ctl,
if (prev_bit == 0 && bit == 1) {
extent_start = offset;
} else if (prev_bit == 1 && bit == 0) {
- total_found += add_new_free_space(block_group,
- extent_start,
- offset);
+ u64 space_added;
+
+ ret = add_new_free_space(block_group,
+ extent_start,
+ offset,
+ &space_added);
+ if (ret)
+ goto out;
+ total_found += space_added;
if (total_found > CACHING_CTL_WAKE_UP) {
total_found = 0;
wake_up(&caching_ctl->wait);
@@ -1529,8 +1535,9 @@ static int load_free_space_bitmaps(struct btrfs_caching_control *caching_ctl,
}
}
if (prev_bit == 1) {
- total_found += add_new_free_space(block_group, extent_start,
- end);
+ ret = add_new_free_space(block_group, extent_start, end, NULL);
+ if (ret)
+ goto out;
extent_count++;
}
@@ -1569,6 +1576,8 @@ static int load_free_space_extents(struct btrfs_caching_control *caching_ctl,
end = block_group->start + block_group->length;
while (1) {
+ u64 space_added;
+
ret = btrfs_next_item(root, path);
if (ret < 0)
goto out;
@@ -1583,8 +1592,12 @@ static int load_free_space_extents(struct btrfs_caching_control *caching_ctl,
ASSERT(key.type == BTRFS_FREE_SPACE_EXTENT_KEY);
ASSERT(key.objectid < end && key.objectid + key.offset <= end);
- total_found += add_new_free_space(block_group, key.objectid,
- key.objectid + key.offset);
+ ret = add_new_free_space(block_group, key.objectid,
+ key.objectid + key.offset,
+ &space_added);
+ if (ret)
+ goto out;
+ total_found += space_added;
if (total_found > CACHING_CTL_WAKE_UP) {
total_found = 0;
wake_up(&caching_ctl->wait);
--
2.34.1
^ permalink raw reply related [flat|nested] 12+ messages in thread* [PATCH 2/8] btrfs: update documentation for add_new_free_space()
2023-06-30 15:03 [PATCH 0/8] btrfs: remove a couple BUG_ON()s and cleanups fdmanana
2023-06-30 15:03 ` [PATCH 1/8] btrfs: remove BUG_ON()'s in add_new_free_space() fdmanana
@ 2023-06-30 15:03 ` fdmanana
2023-06-30 15:03 ` [PATCH 3/8] btrfs: rename add_new_free_space() to btrfs_add_new_free_space() fdmanana
` (6 subsequent siblings)
8 siblings, 0 replies; 12+ messages in thread
From: fdmanana @ 2023-06-30 15:03 UTC (permalink / raw)
To: linux-btrfs
From: Filipe Manana <fdmanana@suse.com>
The documentation for add_new_free_space() is stale and no longer correct:
1) It's no longer used only when caching a block group. It's also called
when creating a block group (btrfs_make_block_group()), when reading
a block group at mount time (read_one_block_group()) and when reading
the free space tree for a block group (typically the first time we
attempt to allocate from the block group);
2) It has nothing to do with pinned extents. It only deals with the
excluded extents io tree, which is used to track the locations of
super blocks in order to make sure we never add the location of a
super block to the free space cache of a block group.
So update the documention and also add a description of the arguments
and return values.
Signed-off-by: Filipe Manana <fdmanana@suse.com>
---
fs/btrfs/block-group.c | 15 +++++++++++----
1 file changed, 11 insertions(+), 4 deletions(-)
diff --git a/fs/btrfs/block-group.c b/fs/btrfs/block-group.c
index aab6667cc580..ec4c6edb556b 100644
--- a/fs/btrfs/block-group.c
+++ b/fs/btrfs/block-group.c
@@ -494,10 +494,17 @@ static void fragment_free_space(struct btrfs_block_group *block_group)
#endif
/*
- * This is only called by btrfs_cache_block_group, since we could have freed
- * extents we need to check the pinned_extents for any extents that can't be
- * used yet since their free space will be released as soon as the transaction
- * commits.
+ * Add a free space range to the in memory free space cache of a block group.
+ * This checks if the range contains super block locations and any such
+ * locations are not added to the free space cache.
+ *
+ * @block_group: The target block group.
+ * @start: Start offset of the range.
+ * @end: End offset of the range (exclusive).
+ * @total_added_ret: Optional pointer to return the total amound of space
+ * added to the block group's free space cache.
+ *
+ * Returns 0 on success or < 0 on error.
*/
int add_new_free_space(struct btrfs_block_group *block_group, u64 start, u64 end,
u64 *total_added_ret)
--
2.34.1
^ permalink raw reply related [flat|nested] 12+ messages in thread* [PATCH 3/8] btrfs: rename add_new_free_space() to btrfs_add_new_free_space()
2023-06-30 15:03 [PATCH 0/8] btrfs: remove a couple BUG_ON()s and cleanups fdmanana
2023-06-30 15:03 ` [PATCH 1/8] btrfs: remove BUG_ON()'s in add_new_free_space() fdmanana
2023-06-30 15:03 ` [PATCH 2/8] btrfs: update documentation for add_new_free_space() fdmanana
@ 2023-06-30 15:03 ` fdmanana
2023-06-30 15:03 ` [PATCH 4/8] btrfs: make btrfs_destroy_marked_extents() return void fdmanana
` (5 subsequent siblings)
8 siblings, 0 replies; 12+ messages in thread
From: fdmanana @ 2023-06-30 15:03 UTC (permalink / raw)
To: linux-btrfs
From: Filipe Manana <fdmanana@suse.com>
Since add_new_free_space() is exported, used outside block-group.c, rename
it to include the 'btrfs_' prefix.
Signed-off-by: Filipe Manana <fdmanana@suse.com>
---
fs/btrfs/block-group.c | 20 ++++++++++----------
fs/btrfs/block-group.h | 4 ++--
fs/btrfs/free-space-tree.c | 16 ++++++++--------
3 files changed, 20 insertions(+), 20 deletions(-)
diff --git a/fs/btrfs/block-group.c b/fs/btrfs/block-group.c
index ec4c6edb556b..ac098d62169d 100644
--- a/fs/btrfs/block-group.c
+++ b/fs/btrfs/block-group.c
@@ -506,8 +506,8 @@ static void fragment_free_space(struct btrfs_block_group *block_group)
*
* Returns 0 on success or < 0 on error.
*/
-int add_new_free_space(struct btrfs_block_group *block_group, u64 start, u64 end,
- u64 *total_added_ret)
+int btrfs_add_new_free_space(struct btrfs_block_group *block_group, u64 start,
+ u64 end, u64 *total_added_ret)
{
struct btrfs_fs_info *info = block_group->fs_info;
u64 extent_start, extent_end, size;
@@ -796,8 +796,8 @@ static int load_extent_tree_free(struct btrfs_caching_control *caching_ctl)
key.type == BTRFS_METADATA_ITEM_KEY) {
u64 space_added;
- ret = add_new_free_space(block_group, last, key.objectid,
- &space_added);
+ ret = btrfs_add_new_free_space(block_group, last,
+ key.objectid, &space_added);
if (ret)
goto out;
total_found += space_added;
@@ -816,9 +816,9 @@ static int load_extent_tree_free(struct btrfs_caching_control *caching_ctl)
path->slots[0]++;
}
- ret = add_new_free_space(block_group, last,
- block_group->start + block_group->length,
- NULL);
+ ret = btrfs_add_new_free_space(block_group, last,
+ block_group->start + block_group->length,
+ NULL);
out:
btrfs_free_path(path);
return ret;
@@ -2312,8 +2312,8 @@ static int read_one_block_group(struct btrfs_fs_info *info,
btrfs_free_excluded_extents(cache);
} else if (cache->used == 0) {
cache->cached = BTRFS_CACHE_FINISHED;
- ret = add_new_free_space(cache, cache->start,
- cache->start + cache->length, NULL);
+ ret = btrfs_add_new_free_space(cache, cache->start,
+ cache->start + cache->length, NULL);
btrfs_free_excluded_extents(cache);
if (ret)
goto error;
@@ -2760,7 +2760,7 @@ struct btrfs_block_group *btrfs_make_block_group(struct btrfs_trans_handle *tran
return ERR_PTR(ret);
}
- ret = add_new_free_space(cache, chunk_offset, chunk_offset + size, NULL);
+ ret = btrfs_add_new_free_space(cache, chunk_offset, chunk_offset + size, NULL);
btrfs_free_excluded_extents(cache);
if (ret) {
btrfs_put_block_group(cache);
diff --git a/fs/btrfs/block-group.h b/fs/btrfs/block-group.h
index a6e7d66b9d80..2267be0aa66a 100644
--- a/fs/btrfs/block-group.h
+++ b/fs/btrfs/block-group.h
@@ -289,8 +289,8 @@ int btrfs_cache_block_group(struct btrfs_block_group *cache, bool wait);
void btrfs_put_caching_control(struct btrfs_caching_control *ctl);
struct btrfs_caching_control *btrfs_get_caching_control(
struct btrfs_block_group *cache);
-int add_new_free_space(struct btrfs_block_group *block_group,
- u64 start, u64 end, u64 *total_added_ret);
+int btrfs_add_new_free_space(struct btrfs_block_group *block_group,
+ u64 start, u64 end, u64 *total_added_ret);
struct btrfs_trans_handle *btrfs_start_trans_remove_block_group(
struct btrfs_fs_info *fs_info,
const u64 chunk_offset);
diff --git a/fs/btrfs/free-space-tree.c b/fs/btrfs/free-space-tree.c
index 53b4ad1abdbc..c0e734082dcc 100644
--- a/fs/btrfs/free-space-tree.c
+++ b/fs/btrfs/free-space-tree.c
@@ -1517,10 +1517,10 @@ static int load_free_space_bitmaps(struct btrfs_caching_control *caching_ctl,
} else if (prev_bit == 1 && bit == 0) {
u64 space_added;
- ret = add_new_free_space(block_group,
- extent_start,
- offset,
- &space_added);
+ ret = btrfs_add_new_free_space(block_group,
+ extent_start,
+ offset,
+ &space_added);
if (ret)
goto out;
total_found += space_added;
@@ -1535,7 +1535,7 @@ static int load_free_space_bitmaps(struct btrfs_caching_control *caching_ctl,
}
}
if (prev_bit == 1) {
- ret = add_new_free_space(block_group, extent_start, end, NULL);
+ ret = btrfs_add_new_free_space(block_group, extent_start, end, NULL);
if (ret)
goto out;
extent_count++;
@@ -1592,9 +1592,9 @@ static int load_free_space_extents(struct btrfs_caching_control *caching_ctl,
ASSERT(key.type == BTRFS_FREE_SPACE_EXTENT_KEY);
ASSERT(key.objectid < end && key.objectid + key.offset <= end);
- ret = add_new_free_space(block_group, key.objectid,
- key.objectid + key.offset,
- &space_added);
+ ret = btrfs_add_new_free_space(block_group, key.objectid,
+ key.objectid + key.offset,
+ &space_added);
if (ret)
goto out;
total_found += space_added;
--
2.34.1
^ permalink raw reply related [flat|nested] 12+ messages in thread* [PATCH 4/8] btrfs: make btrfs_destroy_marked_extents() return void
2023-06-30 15:03 [PATCH 0/8] btrfs: remove a couple BUG_ON()s and cleanups fdmanana
` (2 preceding siblings ...)
2023-06-30 15:03 ` [PATCH 3/8] btrfs: rename add_new_free_space() to btrfs_add_new_free_space() fdmanana
@ 2023-06-30 15:03 ` fdmanana
2023-06-30 15:03 ` [PATCH 5/8] btrfs: make btrfs_destroy_pinned_extent() " fdmanana
` (4 subsequent siblings)
8 siblings, 0 replies; 12+ messages in thread
From: fdmanana @ 2023-06-30 15:03 UTC (permalink / raw)
To: linux-btrfs
From: Filipe Manana <fdmanana@suse.com>
Currently btrfs_destroy_marked_extents() is returning the value of the
last call to find_first_extent_bit(), which returns a value of 1 meaning
no more ranges found the dirty pages io tree. This value is useless to the
single caller of btrfs_destroy_marked_extents(), which ignores any return
value from btrfs_destroy_marked_extents(). This is because it's only used
in the transaction abort path, where we can't even deal with any errors
since we are in a critical situation already and cleanup of resources is
done in a best effort fashion.
So make btrfs_destroy_marked_extents() return void.
Signed-off-by: Filipe Manana <fdmanana@suse.com>
---
fs/btrfs/disk-io.c | 17 +++++------------
1 file changed, 5 insertions(+), 12 deletions(-)
diff --git a/fs/btrfs/disk-io.c b/fs/btrfs/disk-io.c
index 7513388b0567..cfff53cbb72a 100644
--- a/fs/btrfs/disk-io.c
+++ b/fs/btrfs/disk-io.c
@@ -4700,21 +4700,16 @@ static void btrfs_destroy_all_delalloc_inodes(struct btrfs_fs_info *fs_info)
spin_unlock(&fs_info->delalloc_root_lock);
}
-static int btrfs_destroy_marked_extents(struct btrfs_fs_info *fs_info,
- struct extent_io_tree *dirty_pages,
- int mark)
+static void btrfs_destroy_marked_extents(struct btrfs_fs_info *fs_info,
+ struct extent_io_tree *dirty_pages,
+ int mark)
{
- int ret;
struct extent_buffer *eb;
u64 start = 0;
u64 end;
- while (1) {
- ret = find_first_extent_bit(dirty_pages, start, &start, &end,
- mark, NULL);
- if (ret)
- break;
-
+ while (!find_first_extent_bit(dirty_pages, start, &start, &end,
+ mark, NULL)) {
clear_extent_bits(dirty_pages, start, end, mark);
while (start <= end) {
eb = find_extent_buffer(fs_info, start);
@@ -4730,8 +4725,6 @@ static int btrfs_destroy_marked_extents(struct btrfs_fs_info *fs_info,
free_extent_buffer_stale(eb);
}
}
-
- return ret;
}
static int btrfs_destroy_pinned_extent(struct btrfs_fs_info *fs_info,
--
2.34.1
^ permalink raw reply related [flat|nested] 12+ messages in thread* [PATCH 5/8] btrfs: make btrfs_destroy_pinned_extent() return void
2023-06-30 15:03 [PATCH 0/8] btrfs: remove a couple BUG_ON()s and cleanups fdmanana
` (3 preceding siblings ...)
2023-06-30 15:03 ` [PATCH 4/8] btrfs: make btrfs_destroy_marked_extents() return void fdmanana
@ 2023-06-30 15:03 ` fdmanana
2023-06-30 15:03 ` [PATCH 6/8] btrfs: make find_first_extent_bit() return a boolean fdmanana
` (3 subsequent siblings)
8 siblings, 0 replies; 12+ messages in thread
From: fdmanana @ 2023-06-30 15:03 UTC (permalink / raw)
To: linux-btrfs
From: Filipe Manana <fdmanana@suse.com>
Currently btrfs_destroy_pinned_extent() is always returning 0 no matter
what and its caller ignores its return value (as well everything up in
the call chain). This is because this is called in the transaction abort
path, where we can't even deal with any errors since we are in a critical
situation already and cleanup of resources is done in a best effort
fashion.
So make btrfs_destroy_pinned_extent() return void.
Signed-off-by: Filipe Manana <fdmanana@suse.com>
---
fs/btrfs/disk-io.c | 12 ++++--------
1 file changed, 4 insertions(+), 8 deletions(-)
diff --git a/fs/btrfs/disk-io.c b/fs/btrfs/disk-io.c
index cfff53cbb72a..894096d42efc 100644
--- a/fs/btrfs/disk-io.c
+++ b/fs/btrfs/disk-io.c
@@ -4727,12 +4727,11 @@ static void btrfs_destroy_marked_extents(struct btrfs_fs_info *fs_info,
}
}
-static int btrfs_destroy_pinned_extent(struct btrfs_fs_info *fs_info,
- struct extent_io_tree *unpin)
+static void btrfs_destroy_pinned_extent(struct btrfs_fs_info *fs_info,
+ struct extent_io_tree *unpin)
{
u64 start;
u64 end;
- int ret;
while (1) {
struct extent_state *cached_state = NULL;
@@ -4744,9 +4743,8 @@ static int btrfs_destroy_pinned_extent(struct btrfs_fs_info *fs_info,
* the same extent range.
*/
mutex_lock(&fs_info->unused_bg_unpin_mutex);
- ret = find_first_extent_bit(unpin, 0, &start, &end,
- EXTENT_DIRTY, &cached_state);
- if (ret) {
+ if (find_first_extent_bit(unpin, 0, &start, &end,
+ EXTENT_DIRTY, &cached_state)) {
mutex_unlock(&fs_info->unused_bg_unpin_mutex);
break;
}
@@ -4757,8 +4755,6 @@ static int btrfs_destroy_pinned_extent(struct btrfs_fs_info *fs_info,
mutex_unlock(&fs_info->unused_bg_unpin_mutex);
cond_resched();
}
-
- return 0;
}
static void btrfs_cleanup_bg_io(struct btrfs_block_group *cache)
--
2.34.1
^ permalink raw reply related [flat|nested] 12+ messages in thread* [PATCH 6/8] btrfs: make find_first_extent_bit() return a boolean
2023-06-30 15:03 [PATCH 0/8] btrfs: remove a couple BUG_ON()s and cleanups fdmanana
` (4 preceding siblings ...)
2023-06-30 15:03 ` [PATCH 5/8] btrfs: make btrfs_destroy_pinned_extent() " fdmanana
@ 2023-06-30 15:03 ` fdmanana
2023-06-30 15:03 ` [PATCH 7/8] btrfs: open code trivial btrfs_add_excluded_extent() fdmanana
` (2 subsequent siblings)
8 siblings, 0 replies; 12+ messages in thread
From: fdmanana @ 2023-06-30 15:03 UTC (permalink / raw)
To: linux-btrfs
From: Filipe Manana <fdmanana@suse.com>
Currently find_first_extent_bit() returns a 0 if it found a range in the
given io tree and 1 if it didn't find any. There's no need to return any
errors, so make the return value a boolean and invert the logic to make
more sense: return true if it found a range and false if it didn't find
any range.
Signed-off-by: Filipe Manana <fdmanana@suse.com>
---
fs/btrfs/block-group.c | 9 ++++-----
fs/btrfs/dev-replace.c | 6 +++---
fs/btrfs/disk-io.c | 10 +++++-----
fs/btrfs/extent-io-tree.c | 14 +++++++-------
fs/btrfs/extent-io-tree.h | 6 +++---
fs/btrfs/extent-tree.c | 5 ++---
fs/btrfs/free-space-cache.c | 7 +++----
fs/btrfs/relocation.c | 10 ++++++----
fs/btrfs/transaction.c | 8 ++++----
fs/btrfs/volumes.c | 6 +++---
10 files changed, 40 insertions(+), 41 deletions(-)
diff --git a/fs/btrfs/block-group.c b/fs/btrfs/block-group.c
index ac098d62169d..ba4e66de3372 100644
--- a/fs/btrfs/block-group.c
+++ b/fs/btrfs/block-group.c
@@ -517,11 +517,10 @@ int btrfs_add_new_free_space(struct btrfs_block_group *block_group, u64 start,
*total_added_ret = 0;
while (start < end) {
- ret = find_first_extent_bit(&info->excluded_extents, start,
- &extent_start, &extent_end,
- EXTENT_DIRTY | EXTENT_UPTODATE,
- NULL);
- if (ret)
+ if (!find_first_extent_bit(&info->excluded_extents, start,
+ &extent_start, &extent_end,
+ EXTENT_DIRTY | EXTENT_UPTODATE,
+ NULL))
break;
if (extent_start <= start) {
diff --git a/fs/btrfs/dev-replace.c b/fs/btrfs/dev-replace.c
index 5e86bea0a950..661f02c8c038 100644
--- a/fs/btrfs/dev-replace.c
+++ b/fs/btrfs/dev-replace.c
@@ -792,9 +792,9 @@ static int btrfs_set_target_alloc_state(struct btrfs_device *srcdev,
lockdep_assert_held(&srcdev->fs_info->chunk_mutex);
- while (!find_first_extent_bit(&srcdev->alloc_state, start,
- &found_start, &found_end,
- CHUNK_ALLOCATED, &cached_state)) {
+ while (find_first_extent_bit(&srcdev->alloc_state, start,
+ &found_start, &found_end,
+ CHUNK_ALLOCATED, &cached_state)) {
ret = set_extent_bit(&tgtdev->alloc_state, found_start,
found_end, CHUNK_ALLOCATED, NULL);
if (ret)
diff --git a/fs/btrfs/disk-io.c b/fs/btrfs/disk-io.c
index 894096d42efc..fd84906a6ea4 100644
--- a/fs/btrfs/disk-io.c
+++ b/fs/btrfs/disk-io.c
@@ -4212,7 +4212,7 @@ static void warn_about_uncommitted_trans(struct btrfs_fs_info *fs_info)
u64 found_end;
found = true;
- while (!find_first_extent_bit(&trans->dirty_pages, cur,
+ while (find_first_extent_bit(&trans->dirty_pages, cur,
&found_start, &found_end, EXTENT_DIRTY, &cached)) {
dirty_bytes += found_end + 1 - found_start;
cur = found_end + 1;
@@ -4708,8 +4708,8 @@ static void btrfs_destroy_marked_extents(struct btrfs_fs_info *fs_info,
u64 start = 0;
u64 end;
- while (!find_first_extent_bit(dirty_pages, start, &start, &end,
- mark, NULL)) {
+ while (find_first_extent_bit(dirty_pages, start, &start, &end,
+ mark, NULL)) {
clear_extent_bits(dirty_pages, start, end, mark);
while (start <= end) {
eb = find_extent_buffer(fs_info, start);
@@ -4743,8 +4743,8 @@ static void btrfs_destroy_pinned_extent(struct btrfs_fs_info *fs_info,
* the same extent range.
*/
mutex_lock(&fs_info->unused_bg_unpin_mutex);
- if (find_first_extent_bit(unpin, 0, &start, &end,
- EXTENT_DIRTY, &cached_state)) {
+ if (!find_first_extent_bit(unpin, 0, &start, &end,
+ EXTENT_DIRTY, &cached_state)) {
mutex_unlock(&fs_info->unused_bg_unpin_mutex);
break;
}
diff --git a/fs/btrfs/extent-io-tree.c b/fs/btrfs/extent-io-tree.c
index a2315a4b8b75..ff8e117a1ace 100644
--- a/fs/btrfs/extent-io-tree.c
+++ b/fs/btrfs/extent-io-tree.c
@@ -831,15 +831,15 @@ static struct extent_state *find_first_extent_bit_state(struct extent_io_tree *t
*
* Note: If there are multiple bits set in @bits, any of them will match.
*
- * Return 0 if we find something, and update @start_ret and @end_ret.
- * Return 1 if we found nothing.
+ * Return true if we find something, and update @start_ret and @end_ret.
+ * Return false if we found nothing.
*/
-int find_first_extent_bit(struct extent_io_tree *tree, u64 start,
- u64 *start_ret, u64 *end_ret, u32 bits,
- struct extent_state **cached_state)
+bool find_first_extent_bit(struct extent_io_tree *tree, u64 start,
+ u64 *start_ret, u64 *end_ret, u32 bits,
+ struct extent_state **cached_state)
{
struct extent_state *state;
- int ret = 1;
+ bool ret = false;
spin_lock(&tree->lock);
if (cached_state && *cached_state) {
@@ -863,7 +863,7 @@ int find_first_extent_bit(struct extent_io_tree *tree, u64 start,
cache_state_if_flags(state, cached_state, 0);
*start_ret = state->start;
*end_ret = state->end;
- ret = 0;
+ ret = true;
}
out:
spin_unlock(&tree->lock);
diff --git a/fs/btrfs/extent-io-tree.h b/fs/btrfs/extent-io-tree.h
index fbd3b275ab1c..28c23a23d121 100644
--- a/fs/btrfs/extent-io-tree.h
+++ b/fs/btrfs/extent-io-tree.h
@@ -182,9 +182,9 @@ int convert_extent_bit(struct extent_io_tree *tree, u64 start, u64 end,
u32 bits, u32 clear_bits,
struct extent_state **cached_state);
-int find_first_extent_bit(struct extent_io_tree *tree, u64 start,
- u64 *start_ret, u64 *end_ret, u32 bits,
- struct extent_state **cached_state);
+bool find_first_extent_bit(struct extent_io_tree *tree, u64 start,
+ u64 *start_ret, u64 *end_ret, u32 bits,
+ struct extent_state **cached_state);
void find_first_clear_extent_bit(struct extent_io_tree *tree, u64 start,
u64 *start_ret, u64 *end_ret, u32 bits);
int find_contiguous_extent_bit(struct extent_io_tree *tree, u64 start,
diff --git a/fs/btrfs/extent-tree.c b/fs/btrfs/extent-tree.c
index a6efbfd7b24c..4c140636b456 100644
--- a/fs/btrfs/extent-tree.c
+++ b/fs/btrfs/extent-tree.c
@@ -2740,9 +2740,8 @@ int btrfs_finish_extent_commit(struct btrfs_trans_handle *trans)
struct extent_state *cached_state = NULL;
mutex_lock(&fs_info->unused_bg_unpin_mutex);
- ret = find_first_extent_bit(unpin, 0, &start, &end,
- EXTENT_DIRTY, &cached_state);
- if (ret) {
+ if (!find_first_extent_bit(unpin, 0, &start, &end,
+ EXTENT_DIRTY, &cached_state)) {
mutex_unlock(&fs_info->unused_bg_unpin_mutex);
break;
}
diff --git a/fs/btrfs/free-space-cache.c b/fs/btrfs/free-space-cache.c
index 880800418075..bfc01352351c 100644
--- a/fs/btrfs/free-space-cache.c
+++ b/fs/btrfs/free-space-cache.c
@@ -1219,10 +1219,9 @@ static noinline_for_stack int write_pinned_extent_entries(
start = block_group->start;
while (start < block_group->start + block_group->length) {
- ret = find_first_extent_bit(unpin, start,
- &extent_start, &extent_end,
- EXTENT_DIRTY, NULL);
- if (ret)
+ if (!find_first_extent_bit(unpin, start,
+ &extent_start, &extent_end,
+ EXTENT_DIRTY, NULL))
return 0;
/* This pinned extent is out of our range */
diff --git a/fs/btrfs/relocation.c b/fs/btrfs/relocation.c
index 25a3361caedc..9db2e6fa2cb2 100644
--- a/fs/btrfs/relocation.c
+++ b/fs/btrfs/relocation.c
@@ -3469,6 +3469,8 @@ int find_next_extent(struct reloc_control *rc, struct btrfs_path *path,
last = rc->block_group->start + rc->block_group->length;
while (1) {
+ bool block_found;
+
cond_resched();
if (rc->search_start >= last) {
ret = 1;
@@ -3519,11 +3521,11 @@ int find_next_extent(struct reloc_control *rc, struct btrfs_path *path,
goto next;
}
- ret = find_first_extent_bit(&rc->processed_blocks,
- key.objectid, &start, &end,
- EXTENT_DIRTY, NULL);
+ block_found = find_first_extent_bit(&rc->processed_blocks,
+ key.objectid, &start, &end,
+ EXTENT_DIRTY, NULL);
- if (ret == 0 && start <= key.objectid) {
+ if (block_found && start <= key.objectid) {
btrfs_release_path(path);
rc->search_start = end + 1;
} else {
diff --git a/fs/btrfs/transaction.c b/fs/btrfs/transaction.c
index cf306351b148..4743882fa94b 100644
--- a/fs/btrfs/transaction.c
+++ b/fs/btrfs/transaction.c
@@ -1054,8 +1054,8 @@ int btrfs_write_marked_extents(struct btrfs_fs_info *fs_info,
u64 start = 0;
u64 end;
- while (!find_first_extent_bit(dirty_pages, start, &start, &end,
- mark, &cached_state)) {
+ while (find_first_extent_bit(dirty_pages, start, &start, &end,
+ mark, &cached_state)) {
bool wait_writeback = false;
err = convert_extent_bit(dirty_pages, start, end,
@@ -1108,8 +1108,8 @@ static int __btrfs_wait_marked_extents(struct btrfs_fs_info *fs_info,
u64 start = 0;
u64 end;
- while (!find_first_extent_bit(dirty_pages, start, &start, &end,
- EXTENT_NEED_WAIT, &cached_state)) {
+ while (find_first_extent_bit(dirty_pages, start, &start, &end,
+ EXTENT_NEED_WAIT, &cached_state)) {
/*
* Ignore -ENOMEM errors returned by clear_extent_bit().
* When committing the transaction, we'll remove any entries
diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c
index 8137c04f31c9..2e2c01de8188 100644
--- a/fs/btrfs/volumes.c
+++ b/fs/btrfs/volumes.c
@@ -1427,9 +1427,9 @@ static bool contains_pending_extent(struct btrfs_device *device, u64 *start,
lockdep_assert_held(&device->fs_info->chunk_mutex);
- if (!find_first_extent_bit(&device->alloc_state, *start,
- &physical_start, &physical_end,
- CHUNK_ALLOCATED, NULL)) {
+ if (find_first_extent_bit(&device->alloc_state, *start,
+ &physical_start, &physical_end,
+ CHUNK_ALLOCATED, NULL)) {
if (in_range(physical_start, *start, len) ||
in_range(*start, physical_start,
--
2.34.1
^ permalink raw reply related [flat|nested] 12+ messages in thread* [PATCH 7/8] btrfs: open code trivial btrfs_add_excluded_extent()
2023-06-30 15:03 [PATCH 0/8] btrfs: remove a couple BUG_ON()s and cleanups fdmanana
` (5 preceding siblings ...)
2023-06-30 15:03 ` [PATCH 6/8] btrfs: make find_first_extent_bit() return a boolean fdmanana
@ 2023-06-30 15:03 ` fdmanana
2023-06-30 15:03 ` [PATCH 8/8] btrfs: move btrfs_free_excluded_extents() into block-group.c fdmanana
2023-07-12 16:04 ` [PATCH 0/8] btrfs: remove a couple BUG_ON()s and cleanups David Sterba
8 siblings, 0 replies; 12+ messages in thread
From: fdmanana @ 2023-06-30 15:03 UTC (permalink / raw)
To: linux-btrfs
From: Filipe Manana <fdmanana@suse.com>
The code for btrfs_add_excluded_extent() is trivial, it's just a
set_extent_bit() call. However it's defined in extent-tree.c but it is
only used (twice) in block-group.c. So open code it in block-group.c,
reducing the need to export a trivial function.
Also since the only caller btrfs_add_excluded_extent() is prepared to
deal with errors, stop ignoring errors from the set_extent_bit() call.
Signed-off-by: Filipe Manana <fdmanana@suse.com>
---
fs/btrfs/block-group.c | 10 ++++++----
fs/btrfs/extent-tree.c | 9 ---------
fs/btrfs/extent-tree.h | 2 --
3 files changed, 6 insertions(+), 15 deletions(-)
diff --git a/fs/btrfs/block-group.c b/fs/btrfs/block-group.c
index ba4e66de3372..7485660a1529 100644
--- a/fs/btrfs/block-group.c
+++ b/fs/btrfs/block-group.c
@@ -2091,8 +2091,9 @@ static int exclude_super_stripes(struct btrfs_block_group *cache)
if (cache->start < BTRFS_SUPER_INFO_OFFSET) {
stripe_len = BTRFS_SUPER_INFO_OFFSET - cache->start;
cache->bytes_super += stripe_len;
- ret = btrfs_add_excluded_extent(fs_info, cache->start,
- stripe_len);
+ ret = set_extent_bit(&fs_info->excluded_extents, cache->start,
+ cache->start + stripe_len - 1,
+ EXTENT_UPTODATE, NULL);
if (ret)
return ret;
}
@@ -2117,8 +2118,9 @@ static int exclude_super_stripes(struct btrfs_block_group *cache)
cache->start + cache->length - logical[nr]);
cache->bytes_super += len;
- ret = btrfs_add_excluded_extent(fs_info, logical[nr],
- len);
+ ret = set_extent_bit(&fs_info->excluded_extents, logical[nr],
+ logical[nr] + len - 1,
+ EXTENT_UPTODATE, NULL);
if (ret) {
kfree(logical);
return ret;
diff --git a/fs/btrfs/extent-tree.c b/fs/btrfs/extent-tree.c
index 4c140636b456..701fa08cffb6 100644
--- a/fs/btrfs/extent-tree.c
+++ b/fs/btrfs/extent-tree.c
@@ -69,15 +69,6 @@ static int block_group_bits(struct btrfs_block_group *cache, u64 bits)
return (cache->flags & bits) == bits;
}
-int btrfs_add_excluded_extent(struct btrfs_fs_info *fs_info,
- u64 start, u64 num_bytes)
-{
- u64 end = start + num_bytes - 1;
- set_extent_bit(&fs_info->excluded_extents, start, end,
- EXTENT_UPTODATE, NULL);
- return 0;
-}
-
void btrfs_free_excluded_extents(struct btrfs_block_group *cache)
{
struct btrfs_fs_info *fs_info = cache->fs_info;
diff --git a/fs/btrfs/extent-tree.h b/fs/btrfs/extent-tree.h
index 429d5c570061..3b2f265f4653 100644
--- a/fs/btrfs/extent-tree.h
+++ b/fs/btrfs/extent-tree.h
@@ -96,8 +96,6 @@ int btrfs_get_extent_inline_ref_type(const struct extent_buffer *eb,
enum btrfs_inline_ref_type is_data);
u64 hash_extent_data_ref(u64 root_objectid, u64 owner, u64 offset);
-int btrfs_add_excluded_extent(struct btrfs_fs_info *fs_info,
- u64 start, u64 num_bytes);
void btrfs_free_excluded_extents(struct btrfs_block_group *cache);
int btrfs_run_delayed_refs(struct btrfs_trans_handle *trans, unsigned long count);
void btrfs_cleanup_ref_head_accounting(struct btrfs_fs_info *fs_info,
--
2.34.1
^ permalink raw reply related [flat|nested] 12+ messages in thread* [PATCH 8/8] btrfs: move btrfs_free_excluded_extents() into block-group.c
2023-06-30 15:03 [PATCH 0/8] btrfs: remove a couple BUG_ON()s and cleanups fdmanana
` (6 preceding siblings ...)
2023-06-30 15:03 ` [PATCH 7/8] btrfs: open code trivial btrfs_add_excluded_extent() fdmanana
@ 2023-06-30 15:03 ` fdmanana
[not found] ` <20230707221740.GH2565448@zen>
2023-07-12 16:04 ` [PATCH 0/8] btrfs: remove a couple BUG_ON()s and cleanups David Sterba
8 siblings, 1 reply; 12+ messages in thread
From: fdmanana @ 2023-06-30 15:03 UTC (permalink / raw)
To: linux-btrfs
From: Filipe Manana <fdmanana@suse.com>
The function btrfs_free_excluded_extents() is only used by block-group.c,
so move it into block-group.c and make it static. Also removed unnecessary
variables that are used only once.
Signed-off-by: Filipe Manana <fdmanana@suse.com>
---
fs/btrfs/block-group.c | 6 ++++++
fs/btrfs/extent-tree.c | 12 ------------
fs/btrfs/extent-tree.h | 1 -
3 files changed, 6 insertions(+), 13 deletions(-)
diff --git a/fs/btrfs/block-group.c b/fs/btrfs/block-group.c
index 7485660a1529..796e4be167a0 100644
--- a/fs/btrfs/block-group.c
+++ b/fs/btrfs/block-group.c
@@ -823,6 +823,12 @@ static int load_extent_tree_free(struct btrfs_caching_control *caching_ctl)
return ret;
}
+static inline void btrfs_free_excluded_extents(const struct btrfs_block_group *bg)
+{
+ clear_extent_bits(&bg->fs_info->excluded_extents, bg->start,
+ bg->start + bg->length - 1, EXTENT_UPTODATE);
+}
+
static noinline void caching_thread(struct btrfs_work *work)
{
struct btrfs_block_group *block_group;
diff --git a/fs/btrfs/extent-tree.c b/fs/btrfs/extent-tree.c
index 701fa08cffb6..b0fcc2a042ad 100644
--- a/fs/btrfs/extent-tree.c
+++ b/fs/btrfs/extent-tree.c
@@ -69,18 +69,6 @@ static int block_group_bits(struct btrfs_block_group *cache, u64 bits)
return (cache->flags & bits) == bits;
}
-void btrfs_free_excluded_extents(struct btrfs_block_group *cache)
-{
- struct btrfs_fs_info *fs_info = cache->fs_info;
- u64 start, end;
-
- start = cache->start;
- end = start + cache->length - 1;
-
- clear_extent_bits(&fs_info->excluded_extents, start, end,
- EXTENT_UPTODATE);
-}
-
/* simple helper to search for an existing data extent at a given offset */
int btrfs_lookup_data_extent(struct btrfs_fs_info *fs_info, u64 start, u64 len)
{
diff --git a/fs/btrfs/extent-tree.h b/fs/btrfs/extent-tree.h
index 3b2f265f4653..b9e148adcd28 100644
--- a/fs/btrfs/extent-tree.h
+++ b/fs/btrfs/extent-tree.h
@@ -96,7 +96,6 @@ int btrfs_get_extent_inline_ref_type(const struct extent_buffer *eb,
enum btrfs_inline_ref_type is_data);
u64 hash_extent_data_ref(u64 root_objectid, u64 owner, u64 offset);
-void btrfs_free_excluded_extents(struct btrfs_block_group *cache);
int btrfs_run_delayed_refs(struct btrfs_trans_handle *trans, unsigned long count);
void btrfs_cleanup_ref_head_accounting(struct btrfs_fs_info *fs_info,
struct btrfs_delayed_ref_root *delayed_refs,
--
2.34.1
^ permalink raw reply related [flat|nested] 12+ messages in thread* Re: [PATCH 0/8] btrfs: remove a couple BUG_ON()s and cleanups
2023-06-30 15:03 [PATCH 0/8] btrfs: remove a couple BUG_ON()s and cleanups fdmanana
` (7 preceding siblings ...)
2023-06-30 15:03 ` [PATCH 8/8] btrfs: move btrfs_free_excluded_extents() into block-group.c fdmanana
@ 2023-07-12 16:04 ` David Sterba
8 siblings, 0 replies; 12+ messages in thread
From: David Sterba @ 2023-07-12 16:04 UTC (permalink / raw)
To: fdmanana; +Cc: linux-btrfs
On Fri, Jun 30, 2023 at 04:03:43PM +0100, fdmanana@kernel.org wrote:
> From: Filipe Manana <fdmanana@suse.com>
>
> The first patch removes a couple unnecessary BUG_ON()'s, since all
> callers are able to properly deal with errors, which are triggered by
> syzbot with error injection (-ENOMEM). The rest are just some followup
> cleanups. More details in the changelogs.
>
> Filipe Manana (8):
> btrfs: remove BUG_ON()'s in add_new_free_space()
> btrfs: update documentation for add_new_free_space()
> btrfs: rename add_new_free_space() to btrfs_add_new_free_space()
> btrfs: make btrfs_destroy_marked_extents() return void
> btrfs: make btrfs_destroy_pinned_extent() return void
> btrfs: make find_first_extent_bit() return a boolean
> btrfs: open code trivial btrfs_add_excluded_extent()
> btrfs: move btrfs_free_excluded_extents() into block-group.c
Nice cleanups, added to misc-next, thanks.
^ permalink raw reply [flat|nested] 12+ messages in thread