[PATCH v3 1/2] block: Add new BLK_STS_UCLEAN status

[PATCH v3 1/2] block: Add new BLK_STS_UCLEAN status

[Qu Wenruo]

There are quite a lot of filesystems doing their verification work done
at endio hook or hook before submitting bio.

Normally such verification returns -EUCLEAN to indicate something
unexpected, and some of such verification either uses bio->bi_status or
users bio->bi_endio to return their value.

In such case, the missing of such corresponding BLK_STS_UCLEAN can lower
the severity just like:
  In endio function:
	return errno_to_blk_status(-EUCLEAN);
 	^^^ -EUCLEAN gets interpreted to BLK_STS_IOERR

  In the filesystem code:
	ret = blk_status_to_errno(bi->bi_status);
	^^^ BLK_STS_IOERR gets interpreted to -EIO;

This lowers the severity, making the filesystem layer to believe it's
just an ordinary error.

This patch will add a new BLK_STS_UCLEAN, to allow -EUCLEAN to be
converted to BLK_STS_UCLEAN, and then converted back to -EUCLEAN without
losing anything.

Signed-off-by: Qu Wenruo <wqu@suse.com>
---
Changlog:
v2:
- Use BLK_STS_UCLEAN to replace the previous stupid naming scheme.
v3:
- Use "filesystem is corrupted" as error string.
- Add explanation about why filesystem code needs such status.
---
 block/blk-core.c          | 1 +
 include/linux/blk_types.h | 3 +++
 2 files changed, 4 insertions(+)

diff --git a/block/blk-core.c b/block/blk-core.c
index a55389ba8779..427528950dcc 100644
--- a/block/blk-core.c
+++ b/block/blk-core.c
@@ -135,6 +135,7 @@ static const struct {
 	[BLK_STS_RESOURCE]	= { -ENOMEM,	"kernel resource" },
 	[BLK_STS_DEV_RESOURCE]	= { -EBUSY,	"device resource" },
 	[BLK_STS_AGAIN]		= { -EAGAIN,	"nonblocking retry" },
+	[BLK_STS_UCLEAN]	= { -EUCLEAN,	"filesystem is corrupted" },
 
 	/* device mapper special case, should not leak out: */
 	[BLK_STS_DM_REQUEUE]	= { -EREMCHG, "dm internal retry" },
diff --git a/include/linux/blk_types.h b/include/linux/blk_types.h
index 791fee35df88..df0c470147c1 100644
--- a/include/linux/blk_types.h
+++ b/include/linux/blk_types.h
@@ -63,6 +63,9 @@ typedef u8 __bitwise blk_status_t;
  */
 #define BLK_STS_DEV_RESOURCE	((__force blk_status_t)13)
 
+/* Normally filesystem layer generated error */
+#define BLK_STS_UCLEAN		((__force blk_status_t)14)
+
 /**
  * blk_path_error - returns true if error may be path related
  * @error: status the request was completed with
-- 
2.21.0

[PATCH v3 2/2] btrfs: Add error string for EUCLEAN

[Qu Wenruo]

Since we're going to support write time tree checker, it's possible that
transaction get aborted due to tree-checker, also due to new
BLK_STS_UCLEAN bit, we can distinguish real EIO error and EUCLEAN
error.

So adding new string for EUCLEAN to make the "unknown" reason to an easy
to read one.

Signed-off-by: Qu Wenruo <wqu@suse.com>
---
 fs/btrfs/super.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/fs/btrfs/super.c b/fs/btrfs/super.c
index 120e4340792a..c8d0646bfd2c 100644
--- a/fs/btrfs/super.c
+++ b/fs/btrfs/super.c
@@ -86,6 +86,9 @@ const char *btrfs_decode_error(int errno)
 	case -ENOENT:
 		errstr = "No such entry";
 		break;
+	case -EUCLEAN:
+		errstr = "Filesystem is corrupted";
+		break;
 	}
 
 	return errstr;
-- 
2.21.0

Re: [PATCH v3 1/2] block: Add new BLK_STS_UCLEAN status

[Christoph Hellwig]

Please keep linux-block and fsdevel in Cc.

Also as mentioned before UCLEAN is a horrible name, please use
the FSCORRUPTED one actually in use in the file systems that
actually describes what is going on.  Bonus points for also
moving the EFSCORRUPTED definition to common code.

Re: [PATCH v3 1/2] block: Add new BLK_STS_UCLEAN status

[Qu Wenruo]

[-- Attachment #1.1: Type: text/plain, Size: 560 bytes --]



On 2019/4/10 下午3:44, Christoph Hellwig wrote:
> Please keep linux-block and fsdevel in Cc.
> 
> Also as mentioned before UCLEAN is a horrible name, please use
> the FSCORRUPTED one actually in use in the file systems that
> actually describes what is going on.  Bonus points for also
> moving the EFSCORRUPTED definition to common code.

Then the problem is, if we're keeping EFSCORRUPTED, should it still be
the same value of EUCLEAN?

Or it's pretty common to have same error value while have two different
name for it?

Thanks,
Qu


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]