| Message ID | e8249a59dd7a59869dfaa4fbcb76424f458e21c7.1672016104.git.wqu@suse.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | btrfs: add error reports for possible run_one_delayed_ref() | expand |
On Mon, Dec 26, 2022 at 09:00:40AM +0800, Qu Wenruo wrote: > Currently we have a btrfs_debug() for run_one_delayed_ref() failure, but > if end users hit such problem, there will be no chance that > btrfs_debug() is enabled. > > This can lead to very little useful info for debug. > > This patch will: > > - Add extra info for error reporting > Including: > * logical bytenr > * num_bytes > * type > * action > * ref_mod > > - Replace the btrfs_debug() with btrfs_err() > > - Move the error reporting into run_one_delayed_ref() > This is to avoid use-after-free, the @node can be freed in the caller. > > This error should only be triggered at most once. > > As if run_one_delayed_ref() failed, we trigger the error message, then > causing the call chain to error out: > > btrfs_run_delayed_refs() > `- btrfs_run_delayed_refs() > `- btrfs_run_delayed_refs_for_head() > `- run_one_delayed_ref() > > And we will abort the current transaction in btrfs_run_delayed_refs(). > If we have to run delayed refs for the abort transaction, > run_one_delayed_ref() will just cleanup the refs and do nothing, thus no > new error messages would be output. > > Signed-off-by: Qu Wenruo <wqu@suse.com> Added to misc-next, thanks.
On 12/26/22 09:00, Qu Wenruo wrote: > Currently we have a btrfs_debug() for run_one_delayed_ref() failure, but > if end users hit such problem, there will be no chance that > btrfs_debug() is enabled. > > This can lead to very little useful info for debug. > > This patch will: > > - Add extra info for error reporting > Including: > * logical bytenr > * num_bytes > * type > * action > * ref_mod > > - Replace the btrfs_debug() with btrfs_err() > > - Move the error reporting into run_one_delayed_ref() > This is to avoid use-after-free, the @node can be freed in the caller. > > This error should only be triggered at most once. > > As if run_one_delayed_ref() failed, we trigger the error message, then > causing the call chain to error out: > > btrfs_run_delayed_refs() > `- btrfs_run_delayed_refs() > `- btrfs_run_delayed_refs_for_head() > `- run_one_delayed_ref() > > And we will abort the current transaction in btrfs_run_delayed_refs(). > If we have to run delayed refs for the abort transaction, > run_one_delayed_ref() will just cleanup the refs and do nothing, thus no > new error messages would be output. > > Signed-off-by: Qu Wenruo <wqu@suse.com> Reviewed-by: Anand Jain <anand.jain@oracle.com>
diff --git a/fs/btrfs/extent-tree.c b/fs/btrfs/extent-tree.c index eaa1fb2850d7..d1a4e51f8fbc 100644 --- a/fs/btrfs/extent-tree.c +++ b/fs/btrfs/extent-tree.c @@ -1713,6 +1713,11 @@ static int run_one_delayed_ref(struct btrfs_trans_handle *trans, BUG(); if (ret && insert_reserved) btrfs_pin_extent(trans, node->bytenr, node->num_bytes, 1); + if (ret < 0) + btrfs_err(trans->fs_info, +"failed to run delayed ref for logical %llu num_bytes %llu type %u action %u ref_mod %d: %d", + node->bytenr, node->num_bytes, node->type, + node->action, node->ref_mod, ret); return ret; } @@ -1954,8 +1959,6 @@ static int btrfs_run_delayed_refs_for_head(struct btrfs_trans_handle *trans, if (ret) { unselect_delayed_ref_head(delayed_refs, locked_ref); btrfs_put_delayed_ref(ref); - btrfs_debug(fs_info, "run_one_delayed_ref returned %d", - ret); return ret; }
Currently we have a btrfs_debug() for run_one_delayed_ref() failure, but if end users hit such problem, there will be no chance that btrfs_debug() is enabled. This can lead to very little useful info for debug. This patch will: - Add extra info for error reporting Including: * logical bytenr * num_bytes * type * action * ref_mod - Replace the btrfs_debug() with btrfs_err() - Move the error reporting into run_one_delayed_ref() This is to avoid use-after-free, the @node can be freed in the caller. This error should only be triggered at most once. As if run_one_delayed_ref() failed, we trigger the error message, then causing the call chain to error out: btrfs_run_delayed_refs() `- btrfs_run_delayed_refs() `- btrfs_run_delayed_refs_for_head() `- run_one_delayed_ref() And we will abort the current transaction in btrfs_run_delayed_refs(). If we have to run delayed refs for the abort transaction, run_one_delayed_ref() will just cleanup the refs and do nothing, thus no new error messages would be output. Signed-off-by: Qu Wenruo <wqu@suse.com> --- fs/btrfs/extent-tree.c | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-)