* [PATCH 0/3] Cleanups from pinned rework try
@ 2019-12-02 9:40 Nikolay Borisov
2019-12-02 9:40 ` [PATCH 1/3] btrfs: Remove WARN_ON in walk_log_tree Nikolay Borisov
` (3 more replies)
0 siblings, 4 replies; 6+ messages in thread
From: Nikolay Borisov @ 2019-12-02 9:40 UTC (permalink / raw)
To: linux-btrfs; +Cc: Nikolay Borisov
Here are 3 minor cleanups resulting from dwelling in pinned extents rework.
Those cleanup WARN_ONs and make it clear when btrfs_pin_reserved_extent is
called with active transaction. No functional changes in any of them.
Nikolay Borisov (3):
btrfs: Remove WARN_ON in walk_log_tree
btrfs: Remove redundant WARN_ON in walk_down_log_tree
btrfs: Ensure btrfs_pin_reserved_extent is always called with valid
transaction
fs/btrfs/tree-log.c | 60 ++++++++++++++++++---------------------------
1 file changed, 24 insertions(+), 36 deletions(-)
--
2.17.1
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH 1/3] btrfs: Remove WARN_ON in walk_log_tree
2019-12-02 9:40 [PATCH 0/3] Cleanups from pinned rework try Nikolay Borisov
@ 2019-12-02 9:40 ` Nikolay Borisov
2019-12-03 17:21 ` David Sterba
2019-12-02 9:40 ` [PATCH 2/3] btrfs: Remove redundant WARN_ON in walk_down_log_tree Nikolay Borisov
` (2 subsequent siblings)
3 siblings, 1 reply; 6+ messages in thread
From: Nikolay Borisov @ 2019-12-02 9:40 UTC (permalink / raw)
To: linux-btrfs; +Cc: Nikolay Borisov
The log_root passed to walk_log_tree is guaranteed to have its
root_key.objectid always be BTRFS_TREE_LOG_OBJECTID. This is by
merit that all log roots of an ordinary root are allocated in
alloc_log_tree which hard-codes objectid to be BTRFS_TREE_LOG_OBJECTID.
In case walk_log_tree is called for a log tree found by btrfs_read_fs_root
in btrfs_recover_log_trees, that function already ensures
found_key.objectid is BTRFS_TREE_LOG_OBJECTID.
No functional changes.
Signed-off-by: Nikolay Borisov <nborisov@suse.com>
---
fs/btrfs/tree-log.c | 2 --
1 file changed, 2 deletions(-)
diff --git a/fs/btrfs/tree-log.c b/fs/btrfs/tree-log.c
index a30057feff2a..33d329f22534 100644
--- a/fs/btrfs/tree-log.c
+++ b/fs/btrfs/tree-log.c
@@ -2893,8 +2893,6 @@ static int walk_log_tree(struct btrfs_trans_handle *trans,
clear_extent_buffer_dirty(next);
}
- WARN_ON(log->root_key.objectid !=
- BTRFS_TREE_LOG_OBJECTID);
ret = btrfs_pin_reserved_extent(fs_info, next->start,
next->len);
if (ret)
--
2.17.1
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH 2/3] btrfs: Remove redundant WARN_ON in walk_down_log_tree
2019-12-02 9:40 [PATCH 0/3] Cleanups from pinned rework try Nikolay Borisov
2019-12-02 9:40 ` [PATCH 1/3] btrfs: Remove WARN_ON in walk_log_tree Nikolay Borisov
@ 2019-12-02 9:40 ` Nikolay Borisov
2019-12-02 9:40 ` [PATCH 3/3] btrfs: Ensure btrfs_pin_reserved_extent is always called with valid transaction Nikolay Borisov
2019-12-03 17:42 ` [PATCH 0/3] Cleanups from pinned rework try David Sterba
3 siblings, 0 replies; 6+ messages in thread
From: Nikolay Borisov @ 2019-12-02 9:40 UTC (permalink / raw)
To: linux-btrfs; +Cc: Nikolay Borisov
level <0 and level >= BTRFS_MAX_LEVEL are already performed upon
extent buffer read by tree checker in btrfs_check_node.
go. As far as 'level <= 0' we are guaranteed that level is '> 0'
because the value of level _before_ reading 'next' is larger than 1
(otherwise we wouldn't have executed that code at all) this in turn
guarantees that 'level' after btrfs_read_buffer is 'level - 1' since
we verify this invariant in:
btrfs_read_buffer
btree_read_extent_buffer_pages
btrfs_verify_level_key
This guarantees that level can never be '<= 0' so the warn on is
never triggered.
Signed-off-by: Nikolay Borisov <nborisov@suse.com>
---
fs/btrfs/tree-log.c | 9 ---------
1 file changed, 9 deletions(-)
diff --git a/fs/btrfs/tree-log.c b/fs/btrfs/tree-log.c
index 33d329f22534..6ccf260b313e 100644
--- a/fs/btrfs/tree-log.c
+++ b/fs/btrfs/tree-log.c
@@ -2673,14 +2673,9 @@ static noinline int walk_down_log_tree(struct btrfs_trans_handle *trans,
u32 blocksize;
int ret = 0;
- WARN_ON(*level < 0);
- WARN_ON(*level >= BTRFS_MAX_LEVEL);
-
while (*level > 0) {
struct btrfs_key first_key;
- WARN_ON(*level < 0);
- WARN_ON(*level >= BTRFS_MAX_LEVEL);
cur = path->nodes[*level];
WARN_ON(btrfs_header_level(cur) != *level);
@@ -2747,7 +2742,6 @@ static noinline int walk_down_log_tree(struct btrfs_trans_handle *trans,
return ret;
}
- WARN_ON(*level <= 0);
if (path->nodes[*level-1])
free_extent_buffer(path->nodes[*level-1]);
path->nodes[*level-1] = next;
@@ -2755,9 +2749,6 @@ static noinline int walk_down_log_tree(struct btrfs_trans_handle *trans,
path->slots[*level] = 0;
cond_resched();
}
- WARN_ON(*level < 0);
- WARN_ON(*level >= BTRFS_MAX_LEVEL);
-
path->slots[*level] = btrfs_header_nritems(path->nodes[*level]);
cond_resched();
--
2.17.1
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH 3/3] btrfs: Ensure btrfs_pin_reserved_extent is always called with valid transaction
2019-12-02 9:40 [PATCH 0/3] Cleanups from pinned rework try Nikolay Borisov
2019-12-02 9:40 ` [PATCH 1/3] btrfs: Remove WARN_ON in walk_log_tree Nikolay Borisov
2019-12-02 9:40 ` [PATCH 2/3] btrfs: Remove redundant WARN_ON in walk_down_log_tree Nikolay Borisov
@ 2019-12-02 9:40 ` Nikolay Borisov
2019-12-03 17:42 ` [PATCH 0/3] Cleanups from pinned rework try David Sterba
3 siblings, 0 replies; 6+ messages in thread
From: Nikolay Borisov @ 2019-12-02 9:40 UTC (permalink / raw)
To: linux-btrfs; +Cc: Nikolay Borisov
btrfs_pin_reserved_extent only makes sense when called with a valid
transaction handle. At the moment all cases when it's called and
the transaction handle passed to its caller are from error context in
which 'btrfs_fs_info::pinned_extents' is not going to be processed e.g.
btrfs_Free_fs_roots <--called from open_ctree error label or
close_ctree, after all transaction processing has ceased.
btrfs_free_log_root_tree
free_log_tree
walk_log_tree
btrfs_drop_and_free_fs_root
btrfs_Free_log <-- called if fs is flagged BTRFS_FS_STATE_ERROR, all
IO is stopped.
free_log_tree
walk_log_tree
To make this explicit in the code rework the 3 call sites where
btrfs_pin_reserved_extent is called. This is in preparation to moving
pinned extent machinery to btrfs_transaction.
Signed-off-by: Nikolay Borisov <nborisov@suse.com>
---
fs/btrfs/tree-log.c | 49 ++++++++++++++++++++++-----------------------
1 file changed, 24 insertions(+), 25 deletions(-)
diff --git a/fs/btrfs/tree-log.c b/fs/btrfs/tree-log.c
index 6ccf260b313e..b76150cce0fe 100644
--- a/fs/btrfs/tree-log.c
+++ b/fs/btrfs/tree-log.c
@@ -2712,20 +2712,18 @@ static noinline int walk_down_log_tree(struct btrfs_trans_handle *trans,
free_extent_buffer(next);
return ret;
}
-
- if (trans) {
- btrfs_tree_lock(next);
- btrfs_set_lock_blocking_write(next);
- btrfs_clean_tree_block(next);
- btrfs_wait_tree_block_writeback(next);
- btrfs_tree_unlock(next);
- } else {
+ if (!trans) {
if (test_and_clear_bit(EXTENT_BUFFER_DIRTY, &next->bflags))
clear_extent_buffer_dirty(next);
+ return ret;
}
- WARN_ON(root_owner !=
- BTRFS_TREE_LOG_OBJECTID);
+ btrfs_tree_lock(next);
+ btrfs_set_lock_blocking_write(next);
+ btrfs_clean_tree_block(next);
+ btrfs_wait_tree_block_writeback(next);
+ btrfs_tree_unlock(next);
+ WARN_ON(root_owner != BTRFS_TREE_LOG_OBJECTID);
ret = btrfs_pin_reserved_extent(fs_info,
bytenr, blocksize);
if (ret) {
@@ -2791,18 +2789,18 @@ static noinline int walk_up_log_tree(struct btrfs_trans_handle *trans,
struct extent_buffer *next;
next = path->nodes[*level];
-
- if (trans) {
- btrfs_tree_lock(next);
- btrfs_set_lock_blocking_write(next);
- btrfs_clean_tree_block(next);
- btrfs_wait_tree_block_writeback(next);
- btrfs_tree_unlock(next);
- } else {
+ if (!trans) {
if (test_and_clear_bit(EXTENT_BUFFER_DIRTY, &next->bflags))
clear_extent_buffer_dirty(next);
+ return ret;
}
+ btrfs_tree_lock(next);
+ btrfs_set_lock_blocking_write(next);
+ btrfs_clean_tree_block(next);
+ btrfs_wait_tree_block_writeback(next);
+ btrfs_tree_unlock(next);
+
WARN_ON(root_owner != BTRFS_TREE_LOG_OBJECTID);
ret = btrfs_pin_reserved_extent(fs_info,
path->nodes[*level]->start,
@@ -2873,17 +2871,18 @@ static int walk_log_tree(struct btrfs_trans_handle *trans,
next = path->nodes[orig_level];
- if (trans) {
- btrfs_tree_lock(next);
- btrfs_set_lock_blocking_write(next);
- btrfs_clean_tree_block(next);
- btrfs_wait_tree_block_writeback(next);
- btrfs_tree_unlock(next);
- } else {
+ if (!trans) {
if (test_and_clear_bit(EXTENT_BUFFER_DIRTY, &next->bflags))
clear_extent_buffer_dirty(next);
+ goto out;
}
+ btrfs_tree_lock(next);
+ btrfs_set_lock_blocking_write(next);
+ btrfs_clean_tree_block(next);
+ btrfs_wait_tree_block_writeback(next);
+ btrfs_tree_unlock(next);
+
ret = btrfs_pin_reserved_extent(fs_info, next->start,
next->len);
if (ret)
--
2.17.1
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH 1/3] btrfs: Remove WARN_ON in walk_log_tree
2019-12-02 9:40 ` [PATCH 1/3] btrfs: Remove WARN_ON in walk_log_tree Nikolay Borisov
@ 2019-12-03 17:21 ` David Sterba
0 siblings, 0 replies; 6+ messages in thread
From: David Sterba @ 2019-12-03 17:21 UTC (permalink / raw)
To: Nikolay Borisov; +Cc: linux-btrfs
On Mon, Dec 02, 2019 at 11:40:13AM +0200, Nikolay Borisov wrote:
> The log_root passed to walk_log_tree is guaranteed to have its
> root_key.objectid always be BTRFS_TREE_LOG_OBJECTID. This is by
> merit that all log roots of an ordinary root are allocated in
> alloc_log_tree which hard-codes objectid to be BTRFS_TREE_LOG_OBJECTID.
>
> In case walk_log_tree is called for a log tree found by btrfs_read_fs_root
> in btrfs_recover_log_trees, that function already ensures
> found_key.objectid is BTRFS_TREE_LOG_OBJECTID.
Agreed, if anything the warning should have been an assert at the
beginning of the function.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 0/3] Cleanups from pinned rework try
2019-12-02 9:40 [PATCH 0/3] Cleanups from pinned rework try Nikolay Borisov
` (2 preceding siblings ...)
2019-12-02 9:40 ` [PATCH 3/3] btrfs: Ensure btrfs_pin_reserved_extent is always called with valid transaction Nikolay Borisov
@ 2019-12-03 17:42 ` David Sterba
3 siblings, 0 replies; 6+ messages in thread
From: David Sterba @ 2019-12-03 17:42 UTC (permalink / raw)
To: Nikolay Borisov; +Cc: linux-btrfs
On Mon, Dec 02, 2019 at 11:40:12AM +0200, Nikolay Borisov wrote:
> Here are 3 minor cleanups resulting from dwelling in pinned extents rework.
> Those cleanup WARN_ONs and make it clear when btrfs_pin_reserved_extent is
> called with active transaction. No functional changes in any of them.
>
> Nikolay Borisov (3):
> btrfs: Remove WARN_ON in walk_log_tree
> btrfs: Remove redundant WARN_ON in walk_down_log_tree
> btrfs: Ensure btrfs_pin_reserved_extent is always called with valid
> transaction
Added to misc-next, thanks.
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2019-12-03 17:42 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-12-02 9:40 [PATCH 0/3] Cleanups from pinned rework try Nikolay Borisov
2019-12-02 9:40 ` [PATCH 1/3] btrfs: Remove WARN_ON in walk_log_tree Nikolay Borisov
2019-12-03 17:21 ` David Sterba
2019-12-02 9:40 ` [PATCH 2/3] btrfs: Remove redundant WARN_ON in walk_down_log_tree Nikolay Borisov
2019-12-02 9:40 ` [PATCH 3/3] btrfs: Ensure btrfs_pin_reserved_extent is always called with valid transaction Nikolay Borisov
2019-12-03 17:42 ` [PATCH 0/3] Cleanups from pinned rework try David Sterba
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.