All of lore.kernel.org
 help / color / mirror / Atom feed
From: Josef Bacik <josef@toxicpanda.com>
To: linux-btrfs@vger.kernel.org, kernel-team@fb.com
Subject: [PATCH v7 00/38] Cleanup error handling in relocation
Date: Wed, 16 Dec 2020 11:26:16 -0500	[thread overview]
Message-ID: <cover.1608135849.git.josef@toxicpanda.com> (raw)

v6->v7:
- Broke up the series into 3 series, 1 for cosmetic things, 1 for all the major
  issues (including those reported on v6 of this set), and this new set which is
  solely the error handling related patches for relocation.  It's still a lot of
  patches, sorry about that.

v5->v6:
- Reworked "btrfs: handle errors from select_reloc_root()" because Zygo reported
  hitting an ASSERT(ret != -ENOENT) during his testing.  This was because I
  changed select_reloc_root() to return -ENOENT if we happened to race with
  somebody else who failed to init the reloc root, however we had an ASSERT() to
  check for this because it indicated corruption.  I modified that patch to move
  the ASSERT() to where the problem actually is, so select_reloc_root() can
  return whatever error and it'll pass it along.  I also removed Qu's
  reviewed-by for the patch because of the change.

v4->v5:
- Dropped "btrfs: fix error handling in commit_fs_roots" as it was merged.
- Fixed an ASSERT() that happened during relocation recovery that Zygo reported,
  I moved the error condition out of another condition which broke recovery if
  we had deleted subvols pending with relocation.

v3->v4:
- Squashed the __add_reloc_root error handling patches in
  btrfs_recover_relocation as they were small and in the same function.
- Squashed the record_root_in_trans failure handling patches for
  select_reloc_root as they were small and in the same function.
- Added a new patch to address an existing error handling problem with subvol
  creation.
- Fixed up the various cases that Qu noticed where I got things wrong, cleaning
  up a leaked root extent ref, a leaked inode item, and where I accidentally
  stopped dealing with errors from btrfs_drop_subtree.
- Reworked a bunch of the ASSERT()'s to do ASSERT(0) in their respective if
  statements.
- Added reviewed-bys.

v2->v3:
- A lot of extra patches fixing various things that I encountered while
  debugging the corruption problem that was uncovered by these patches.
- Fixed the panic that Zygo was seeing and other issues.
- Fixed up the comments from Nikolay and Filipe.

A slight note, the first set of patches could probably be taken now, and in fact

  btrfs: fix error handling in commit_fs_roots

Was sent earlier this week and is very important and needs to be reviewed and
merged ASAP.  The following are safe and could be merged outside of the rest of
this series

  btrfs: allow error injection for btrfs_search_slot and btrfs_cow_block
  btrfs: fix lockdep splat in btrfs_recover_relocation
  btrfs: keep track of the root owner for relocation reads
  btrfs: noinline btrfs_should_cancel_balance
  btrfs: do not cleanup upper nodes in btrfs_backref_cleanup_node
  btrfs: pass down the tree block level through ref-verify
  btrfs: make sure owner is set in ref-verify
  btrfs: don't clear ret in btrfs_start_dirty_block_groups

The rest obviously are all around the actual error handling.

v1->v2:
- fixed a bug where I accidentally dropped reading flags in relocate_block_group
  when I dropped the extra checks that we handle in the tree checker.

--- Original message ---
Hello,

Relocation is the last place that is not able to handle errors at all, which
results in all sorts of lovely panics if you encounter corruptions or IO errors.
I'm going to start cleaning up relocation, but before I move code around I want
the error handling to be somewhat sane, so I'm not changing behavior and error
handling at the same time.

These patches are purely about error handling, there is no behavior changing
other than returning errors up the chain properly.  There is a lot of room for
follow up cleanups, which will happen next.  However I wanted to get this series
done today and out so we could get it merged ASAP, and then the follow up
cleanups can happen later as they are less important and less critical.

The only exception to the above is the patch to add the error injection sites
for btrfs_cow_block and btrfs_search_slot, and a lockdep fix that I discovered
while running my tests, those are the first two patches in the series.

I tested this with my error injection stress test, where I keep track of all
stack traces that have been tested and only inject errors when we have a new
stack trace, which means I should have covered all of the various error
conditions.  With this patchset I'm no longer panicing while stressing the error
conditions.  Thanks,

Josef

Josef Bacik (38):
  btrfs: convert BUG_ON()'s in relocate_tree_block
  btrfs: return an error from btrfs_record_root_in_trans
  btrfs: handle errors from select_reloc_root()
  btrfs: convert BUG_ON()'s in select_reloc_root() to proper errors
  btrfs: check record_root_in_trans related failures in
    select_reloc_root
  btrfs: do proper error handling in record_reloc_root_in_trans
  btrfs: handle btrfs_record_root_in_trans failure in
    btrfs_rename_exchange
  btrfs: handle btrfs_record_root_in_trans failure in btrfs_rename
  btrfs: handle btrfs_record_root_in_trans failure in
    btrfs_delete_subvolume
  btrfs: handle btrfs_record_root_in_trans failure in
    btrfs_recover_log_trees
  btrfs: handle btrfs_record_root_in_trans failure in create_subvol
  btrfs: btrfs: handle btrfs_record_root_in_trans failure in
    relocate_tree_block
  btrfs: handle btrfs_record_root_in_trans failure in start_transaction
  btrfs: handle record_root_in_trans failure in qgroup_account_snapshot
  btrfs: handle record_root_in_trans failure in
    btrfs_record_root_in_trans
  btrfs: handle record_root_in_trans failure in create_pending_snapshot
  btrfs: do not panic in __add_reloc_root
  btrfs: have proper error handling in btrfs_init_reloc_root
  btrfs: do proper error handling in create_reloc_root
  btrfs: validate ->reloc_root after recording root in trans
  btrfs: handle btrfs_update_reloc_root failure in commit_fs_roots
  btrfs: change insert_dirty_subvol to return errors
  btrfs: handle btrfs_update_reloc_root failure in insert_dirty_subvol
  btrfs: handle btrfs_update_reloc_root failure in prepare_to_merge
  btrfs: do proper error handling in btrfs_update_reloc_root
  btrfs: convert logic BUG_ON()'s in replace_path to ASSERT()'s
  btrfs: handle btrfs_cow_block errors in replace_path
  btrfs: handle btrfs_search_slot failure in replace_path
  btrfs: handle errors in reference count manipulation in replace_path
  btrfs: handle extent reference errors in do_relocation
  btrfs: check for BTRFS_BLOCK_FLAG_FULL_BACKREF being set improperly
  btrfs: remove the extent item sanity checks in relocate_block_group
  btrfs: do proper error handling in create_reloc_inode
  btrfs: handle __add_reloc_root failures in btrfs_recover_relocation
  btrfs: cleanup error handling in prepare_to_merge
  btrfs: handle extent corruption with select_one_root properly
  btrfs: do proper error handling in merge_reloc_roots
  btrfs: check return value of btrfs_commit_transaction in relocation

 fs/btrfs/inode.c        |  20 +-
 fs/btrfs/ioctl.c        |   7 +-
 fs/btrfs/relocation.c   | 414 +++++++++++++++++++++++++++++++---------
 fs/btrfs/transaction.c  |  37 ++--
 fs/btrfs/tree-checker.c |   5 +
 fs/btrfs/tree-log.c     |   8 +-
 6 files changed, 382 insertions(+), 109 deletions(-)

-- 
2.26.2


             reply	other threads:[~2020-12-16 16:27 UTC|newest]

Thread overview: 45+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-12-16 16:26 Josef Bacik [this message]
2020-12-16 16:26 ` [PATCH v7 01/38] btrfs: convert BUG_ON()'s in relocate_tree_block Josef Bacik
2020-12-16 16:26 ` [PATCH v7 02/38] btrfs: return an error from btrfs_record_root_in_trans Josef Bacik
2021-02-26 19:03   ` David Sterba
2020-12-16 16:26 ` [PATCH v7 03/38] btrfs: handle errors from select_reloc_root() Josef Bacik
2021-02-26 18:30   ` David Sterba
2021-03-11 18:10     ` Josef Bacik
2021-04-09 19:24       ` David Sterba
2020-12-16 16:26 ` [PATCH v7 04/38] btrfs: convert BUG_ON()'s in select_reloc_root() to proper errors Josef Bacik
2020-12-16 16:26 ` [PATCH v7 05/38] btrfs: check record_root_in_trans related failures in select_reloc_root Josef Bacik
2020-12-16 16:26 ` [PATCH v7 06/38] btrfs: do proper error handling in record_reloc_root_in_trans Josef Bacik
2020-12-16 16:26 ` [PATCH v7 07/38] btrfs: handle btrfs_record_root_in_trans failure in btrfs_rename_exchange Josef Bacik
2020-12-16 16:26 ` [PATCH v7 08/38] btrfs: handle btrfs_record_root_in_trans failure in btrfs_rename Josef Bacik
2020-12-16 16:26 ` [PATCH v7 09/38] btrfs: handle btrfs_record_root_in_trans failure in btrfs_delete_subvolume Josef Bacik
2020-12-16 16:26 ` [PATCH v7 10/38] btrfs: handle btrfs_record_root_in_trans failure in btrfs_recover_log_trees Josef Bacik
2020-12-16 16:26 ` [PATCH v7 11/38] btrfs: handle btrfs_record_root_in_trans failure in create_subvol Josef Bacik
2020-12-16 16:26 ` [PATCH v7 12/38] btrfs: btrfs: handle btrfs_record_root_in_trans failure in relocate_tree_block Josef Bacik
2020-12-16 16:26 ` [PATCH v7 13/38] btrfs: handle btrfs_record_root_in_trans failure in start_transaction Josef Bacik
2020-12-16 16:26 ` [PATCH v7 14/38] btrfs: handle record_root_in_trans failure in qgroup_account_snapshot Josef Bacik
2020-12-16 16:26 ` [PATCH v7 15/38] btrfs: handle record_root_in_trans failure in btrfs_record_root_in_trans Josef Bacik
2020-12-16 16:26 ` [PATCH v7 16/38] btrfs: handle record_root_in_trans failure in create_pending_snapshot Josef Bacik
2020-12-16 16:26 ` [PATCH v7 17/38] btrfs: do not panic in __add_reloc_root Josef Bacik
2020-12-16 16:26 ` [PATCH v7 18/38] btrfs: have proper error handling in btrfs_init_reloc_root Josef Bacik
2020-12-16 16:26 ` [PATCH v7 19/38] btrfs: do proper error handling in create_reloc_root Josef Bacik
2020-12-16 16:26 ` [PATCH v7 20/38] btrfs: validate ->reloc_root after recording root in trans Josef Bacik
2020-12-16 16:26 ` [PATCH v7 21/38] btrfs: handle btrfs_update_reloc_root failure in commit_fs_roots Josef Bacik
2020-12-16 16:26 ` [PATCH v7 22/38] btrfs: change insert_dirty_subvol to return errors Josef Bacik
2020-12-16 16:26 ` [PATCH v7 23/38] btrfs: handle btrfs_update_reloc_root failure in insert_dirty_subvol Josef Bacik
2020-12-16 16:26 ` [PATCH v7 24/38] btrfs: handle btrfs_update_reloc_root failure in prepare_to_merge Josef Bacik
2020-12-16 16:26 ` [PATCH v7 25/38] btrfs: do proper error handling in btrfs_update_reloc_root Josef Bacik
2020-12-16 16:26 ` [PATCH v7 26/38] btrfs: convert logic BUG_ON()'s in replace_path to ASSERT()'s Josef Bacik
2020-12-16 16:26 ` [PATCH v7 27/38] btrfs: handle btrfs_cow_block errors in replace_path Josef Bacik
2020-12-16 16:26 ` [PATCH v7 28/38] btrfs: handle btrfs_search_slot failure " Josef Bacik
2020-12-16 16:26 ` [PATCH v7 29/38] btrfs: handle errors in reference count manipulation " Josef Bacik
2020-12-16 16:26 ` [PATCH v7 30/38] btrfs: handle extent reference errors in do_relocation Josef Bacik
2020-12-16 16:26 ` [PATCH v7 31/38] btrfs: check for BTRFS_BLOCK_FLAG_FULL_BACKREF being set improperly Josef Bacik
2020-12-16 16:26 ` [PATCH v7 32/38] btrfs: remove the extent item sanity checks in relocate_block_group Josef Bacik
2020-12-16 16:26 ` [PATCH v7 33/38] btrfs: do proper error handling in create_reloc_inode Josef Bacik
2020-12-16 16:26 ` [PATCH v7 34/38] btrfs: handle __add_reloc_root failures in btrfs_recover_relocation Josef Bacik
2020-12-16 16:26 ` [PATCH v7 35/38] btrfs: cleanup error handling in prepare_to_merge Josef Bacik
2020-12-16 16:26 ` [PATCH v7 36/38] btrfs: handle extent corruption with select_one_root properly Josef Bacik
2020-12-16 16:26 ` [PATCH v7 37/38] btrfs: do proper error handling in merge_reloc_roots Josef Bacik
2020-12-16 16:26 ` [PATCH v7 38/38] btrfs: check return value of btrfs_commit_transaction in relocation Josef Bacik
2020-12-16 19:56 ` [PATCH v7 00/38] Cleanup error handling " Zygo Blaxell
2020-12-18 16:25   ` Zygo Blaxell

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=cover.1608135849.git.josef@toxicpanda.com \
    --to=josef@toxicpanda.com \
    --cc=kernel-team@fb.com \
    --cc=linux-btrfs@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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.