| Message ID | 20190406103759.6330-1-wqu@suse.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [v2,1/2] block: Add new BLK_STS_SELFTEST status | expand |
> + [BLK_STS_UCLEAN] = { -EUCLEAN, "structure needs cleaning" },
The subject line doesn't mention this new error code. That being said
while this sounds slightly less bad than the original name it still
sounds weird..
The various filesystems really use EFSCORRUPTED which is just mapped
to EUCLEAN, so maybe this really should be
[BLK_STS_FSCORRUPTED] =
{ -EUCLEAN, "file system corruption detected" },
But then again I really wonder why you need to pass this information
through a blk_status_t to start with. In general these kinds of error
should be passed through file system specific errno fields.
On 2019/4/7 下午3:04, Christoph Hellwig wrote: >> + [BLK_STS_UCLEAN] = { -EUCLEAN, "structure needs cleaning" }, > > The subject line doesn't mention this new error code. That being said > while this sounds slightly less bad than the original name it still > sounds weird.. > > The various filesystems really use EFSCORRUPTED which is just mapped > to EUCLEAN, so maybe this really should be > > [BLK_STS_FSCORRUPTED] = > { -EUCLEAN, "file system corruption detected" }, > > But then again I really wonder why you need to pass this information > through a blk_status_t to start with. In general these kinds of error > should be passed through file system specific errno fields. > For functions called in endio hook, we return blk_status_t. Or for case like hook before submitting bio, we set bio->bi_status to record it. Yes, it's possible to restore such info into fs specific structure, but why not reuse the bi_status we all use and love? Thanks, Qu
On Sun, Apr 07, 2019 at 03:08:42PM +0800, Qu Wenruo wrote: > > But then again I really wonder why you need to pass this information > > through a blk_status_t to start with. In general these kinds of error > > should be passed through file system specific errno fields. > > > > For functions called in endio hook, we return blk_status_t. > Or for case like hook before submitting bio, we set bio->bi_status to > record it. Ok. Good enough explanation, please add it to the changelog.
diff --git a/block/blk-core.c b/block/blk-core.c index 4673ebe42255..a51c0a13fd5e 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, "structure needs cleaning" }, /* 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
There are quite a lot of filesystems doing their verification work done at endio hook or hook before submitting bio. Normally such verification should return -EUCLEAN to indicate something unexpected. However there is no BLK_STS_* bit for that, this makes every selftest error to be interpreted to EIO, which lowers the severity. 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> --- changelog: v2: - Use BLK_STS_UCLEAN to replace the previous stupid naming scheme. --- block/blk-core.c | 1 + include/linux/blk_types.h | 3 +++ 2 files changed, 4 insertions(+)