On 24.03.20 г. 16:47 ч., Josef Bacik wrote:
> Dave reported a problem where we were panicing with generic/475 with
> misc-5.7. This is because we were doing IO after we had stopped all of
This doesn't seem correct. Before, the log tree would be freed in
btrfs_free_fs_roots which is called before btrfs_stop_all_workers and
only with transaction_kthread and cleaner_kthread stopped. I'd expect
that now btrfs_cleanup_transaction will be called from
btrfs_error_commit_super. Perhaps it's not "after we had stopped all of
the worker threads" but simply "We have stopped transaction/cleaner
kthreads" But then again I don't see how those 2 make any difference.
Could it be that this is not a full fix for the issue, and it can still
crash in case it's called from btrfs_error_commit_super ?
> the worker threads, because we do the log tree cleanup on roots at drop
> time. Cleaning up the log tree may need to do reads if we happened to
> have evicted the blocks from memory.
>
> Because of this simply add a helper to btrfs_cleanup_transaction() that
> will go through and drop all of the log roots. This gets run before we
> do the close_ctree() work, and thus we are allowed to do any reads that
> we would need. I ran this through many iterations of generic/475 with
> constrained memory and I did not see the issue.
>
> Signed-off-by: Josef Bacik <josef@xxxxxxxxxxxxxx>
>
> ---
> fs/btrfs/disk-io.c | 36 ++++++++++++++++++++++++++++++++----
> 1 file changed, 32 insertions(+), 4 deletions(-)
>
> diff --git a/fs/btrfs/disk-io.c b/fs/btrfs/disk-io.c
> index a6cb5cbbdb9f..d10c7be10f3b 100644
> --- a/fs/btrfs/disk-io.c
> +++ b/fs/btrfs/disk-io.c
> @@ -2036,9 +2036,6 @@ void btrfs_free_fs_roots(struct btrfs_fs_info *fs_info)
> for (i = 0; i < ret; i++)
> btrfs_drop_and_free_fs_root(fs_info, gang[i]);
> }
> -
> - if (test_bit(BTRFS_FS_STATE_ERROR, &fs_info->fs_state))
> - btrfs_free_log_root_tree(NULL, fs_info);
> }
>
> static void btrfs_init_scrub(struct btrfs_fs_info *fs_info)
> @@ -3888,7 +3885,7 @@ void btrfs_drop_and_free_fs_root(struct btrfs_fs_info *fs_info,
> spin_unlock(&fs_info->fs_roots_radix_lock);
>
> if (test_bit(BTRFS_FS_STATE_ERROR, &fs_info->fs_state)) {
> - btrfs_free_log(NULL, root);
> + ASSERT(root->log_root == NULL);
> if (root->reloc_root) {
> btrfs_put_root(root->reloc_root);
> root->reloc_root = NULL;
> @@ -4211,6 +4208,36 @@ static void btrfs_error_commit_super(struct btrfs_fs_info *fs_info)
> up_write(&fs_info->cleanup_work_sem);
> }
>
> +static void btrfs_drop_all_logs(struct btrfs_fs_info *fs_info)
> +{
> + struct btrfs_root *gang[8];
> + u64 root_objectid = 0;
> + int ret;
> +
> + spin_lock(&fs_info->fs_roots_radix_lock);
> + while ((ret = radix_tree_gang_lookup(&fs_info->fs_roots_radix,
> + (void **)gang, root_objectid,
> + ARRAY_SIZE(gang))) != 0) {
> + int i;
> +
> + for (i = 0; i < ret; i++)
> + gang[i] = btrfs_grab_root(gang[i]);
> + spin_unlock(&fs_info->fs_roots_radix_lock);
> +
> + for (i = 0; i < ret; i++) {
> + if (!gang[i])
> + continue;
> + root_objectid = gang[i]->root_key.objectid;
> + btrfs_free_log(NULL, gang[i]);
> + btrfs_put_root(gang[i]);
> + }
> + root_objectid++;
> + spin_lock(&fs_info->fs_roots_radix_lock);
> + }
> + spin_unlock(&fs_info->fs_roots_radix_lock);
> + btrfs_free_log_root_tree(NULL, fs_info);
> +}
> +
> static void btrfs_destroy_ordered_extents(struct btrfs_root *root)
> {
> struct btrfs_ordered_extent *ordered;
> @@ -4603,6 +4630,7 @@ static int btrfs_cleanup_transaction(struct btrfs_fs_info *fs_info)
> btrfs_destroy_delayed_inodes(fs_info);
> btrfs_assert_delayed_root_empty(fs_info);
> btrfs_destroy_all_delalloc_inodes(fs_info);
> + btrfs_drop_all_logs(fs_info);
> mutex_unlock(&fs_info->transaction_kthread_mutex);
>
> return 0;
>
