[PATCH 1/2] btrfs: don't WARN_ON() if we abort a transaction with -EROFS

[PATCH 1/2] btrfs: don't WARN_ON() if we abort a transaction with -EROFS

[Josef Bacik]

If we got some sort of corruption via a read and call
btrfs_handle_fs_error() we'll set BTRFS_FS_STATE_ERROR on the fs and
complain.  If a subsequent trans handle trips over this it'll get -EROFS
and then abort.  However at that point we're not aborting for the
original reason, we're aborting because we've been flipped read only.
We do not need to WARN_ON() here.

Signed-off-by: Josef Bacik <josef@toxicpanda.com>
---
 fs/btrfs/ctree.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/fs/btrfs/ctree.h b/fs/btrfs/ctree.h
index b70c2024296f..f8ae4849f235 100644
--- a/fs/btrfs/ctree.h
+++ b/fs/btrfs/ctree.h
@@ -3279,7 +3279,7 @@ do {								\
 	/* Report first abort since mount */			\
 	if (!test_and_set_bit(BTRFS_FS_STATE_TRANS_ABORTED,	\
 			&((trans)->fs_info->fs_state))) {	\
-		if ((errno) != -EIO) {				\
+		if ((errno) != -EIO && (errno) != -EROFS) {	\
 			WARN(1, KERN_DEBUG				\
 			"BTRFS: Transaction aborted (error %d)\n",	\
 			(errno));					\
-- 
2.24.1

[PATCH 2/2] btrfs: document special case error codes for fs errors

[Josef Bacik]

We've had some discussions about what to do in certain scenarios for
error codes, specifically -EUCLEAN and -EROFS.  Document these near the
error handling code so its clear what their intentions are.

Signed-off-by: Josef Bacik <josef@toxicpanda.com>
---
 fs/btrfs/super.c | 15 +++++++++++++++
 1 file changed, 15 insertions(+)

diff --git a/fs/btrfs/super.c b/fs/btrfs/super.c
index 58f890f73650..688d1ab95b2b 100644
--- a/fs/btrfs/super.c
+++ b/fs/btrfs/super.c
@@ -67,6 +67,21 @@ static struct file_system_type btrfs_root_fs_type;
 
 static int btrfs_remount(struct super_block *sb, int *flags, char *data);
 
+/*
+ * Generally the error codes correspond to their respective errors, but there's
+ * a few special cases.
+ *
+ * -EUCLEAN: Any sort of corruption that we encounter.  The tree-checker for
+ *  instance will return -EUCLEAN if any of the blocks are corrupted in a way
+ *  that is problematic.  We want to reserve -EUCLEAN for these sort of
+ *  corruptions.
+ *
+ * -EROFS: If we check BTRFS_FS_STATE_ERROR and fail out with a return error, we
+ *  need to use -EROFS for this case.  We will have no idea of the original
+ *  failure, that will have been reported at the time we tripped over the error.
+ *  Each subsequent error that doesn't have any context of the original error
+ *  should use -EROFS when handling BTRFS_FS_STATE_ERROR.
+ */
 const char * __attribute_const__ btrfs_decode_error(int errno)
 {
 	char *errstr = "unknown";
-- 
2.24.1

Re: [PATCH 1/2] btrfs: don't WARN_ON() if we abort a transaction with -EROFS

[David Sterba]

On Tue, Jul 21, 2020 at 11:24:27AM -0400, Josef Bacik wrote:
> If we got some sort of corruption via a read and call
> btrfs_handle_fs_error() we'll set BTRFS_FS_STATE_ERROR on the fs and
> complain.  If a subsequent trans handle trips over this it'll get -EROFS
> and then abort.  However at that point we're not aborting for the
> original reason, we're aborting because we've been flipped read only.
> We do not need to WARN_ON() here.
> 
> Signed-off-by: Josef Bacik <josef@toxicpanda.com>

1 and 2 added to misc-next, thanks.