Re: [PATCH 02/17] btrfs: stop tracking failed reads in the I/O tree

Qu Wenruo
2022-09-01T08:55:58Z

On 2022/9/1 15:42, Christoph Hellwig wrote:
> There is a separate I/O failure tree to track the fail reads, so remove
> the extra EXTENT_DAMAGED bit in the I/O tree.
>
> Signed-off-by: Christoph Hellwig <hch@lst.de>

Reviewed-by: Qu Wenruo <wqu@suse.com>

Reducing extent flags is already a good thing.

Thanks,
Qu
> ---
> fs/btrfs/extent-io-tree.h | 1 -
> fs/btrfs/extent_io.c | 16 +---------------
> fs/btrfs/tests/extent-io-tests.c | 1 -
> include/trace/events/btrfs.h | 1 -
> 4 files changed, 1 insertion(+), 18 deletions(-)
>
> diff --git a/fs/btrfs/extent-io-tree.h b/fs/btrfs/extent-io-tree.h
> index ec2f8b8e6faa7..e218bb56d86ac 100644
> --- a/fs/btrfs/extent-io-tree.h
> +++ b/fs/btrfs/extent-io-tree.h
> @@ -17,7 +17,6 @@ struct io_failure_record;
> #define EXTENT_NODATASUM (1U << 7)
> #define EXTENT_CLEAR_META_RESV (1U << 8)
> #define EXTENT_NEED_WAIT (1U << 9)
> -#define EXTENT_DAMAGED (1U << 10)
> #define EXTENT_NORESERVE (1U << 11)
> #define EXTENT_QGROUP_RESERVED (1U << 12)
> #define EXTENT_CLEAR_DATA_RESV (1U << 13)
> diff --git a/fs/btrfs/extent_io.c b/fs/btrfs/extent_io.c
> index 591c191a58bc9..6ac76534d2c9e 100644
> --- a/fs/btrfs/extent_io.c
> +++ b/fs/btrfs/extent_io.c
> @@ -2280,23 +2280,13 @@ int free_io_failure(struct extent_io_tree *failure_tree,
> struct io_failure_record *rec)
> {
> int ret;
> - int err = 0;
>
> set_state_failrec(failure_tree, rec->start, NULL);
> ret = clear_extent_bits(failure_tree, rec->start,
> rec->start + rec->len - 1,
> EXTENT_LOCKED | EXTENT_DIRTY);
> - if (ret)
> - err = ret;
> -
> - ret = clear_extent_bits(io_tree, rec->start,
> - rec->start + rec->len - 1,
> - EXTENT_DAMAGED);
> - if (ret && !err)
> - err = ret;
> -
> kfree(rec);
> - return err;
> + return ret;
> }
>
> /*
> @@ -2521,7 +2511,6 @@ static struct io_failure_record *btrfs_get_io_failure_record(struct inode *inode
> u64 start = bbio->file_offset + bio_offset;
> struct io_failure_record *failrec;
> struct extent_io_tree *failure_tree = &BTRFS_I(inode)->io_failure_tree;
> - struct extent_io_tree *tree = &BTRFS_I(inode)->io_tree;
> const u32 sectorsize = fs_info->sectorsize;
> int ret;
>
> @@ -2573,9 +2562,6 @@ static struct io_failure_record *btrfs_get_io_failure_record(struct inode *inode
> EXTENT_LOCKED | EXTENT_DIRTY);
> if (ret >= 0) {
> ret = set_state_failrec(failure_tree, start, failrec);
> - /* Set the bits in the inode's tree */
> - ret = set_extent_bits(tree, start, start + sectorsize - 1,
> - EXTENT_DAMAGED);
> } else if (ret < 0) {
> kfree(failrec);
> return ERR_PTR(ret);
> diff --git a/fs/btrfs/tests/extent-io-tests.c b/fs/btrfs/tests/extent-io-tests.c
> index a232b15b8021f..ba4b7601e8c0a 100644
> --- a/fs/btrfs/tests/extent-io-tests.c
> +++ b/fs/btrfs/tests/extent-io-tests.c
> @@ -80,7 +80,6 @@ static void extent_flag_to_str(const struct extent_state *state, char *dest)
> PRINT_ONE_FLAG(state, dest, cur, NODATASUM);
> PRINT_ONE_FLAG(state, dest, cur, CLEAR_META_RESV);
> PRINT_ONE_FLAG(state, dest, cur, NEED_WAIT);
> - PRINT_ONE_FLAG(state, dest, cur, DAMAGED);
> PRINT_ONE_FLAG(state, dest, cur, NORESERVE);
> PRINT_ONE_FLAG(state, dest, cur, QGROUP_RESERVED);
> PRINT_ONE_FLAG(state, dest, cur, CLEAR_DATA_RESV);
> diff --git a/include/trace/events/btrfs.h b/include/trace/events/btrfs.h
> index 73df80d462dc8..f8a4118b16574 100644
> --- a/include/trace/events/btrfs.h
> +++ b/include/trace/events/btrfs.h
> @@ -154,7 +154,6 @@ FLUSH_STATES
> { EXTENT_NODATASUM, "NODATASUM"}, \
> { EXTENT_CLEAR_META_RESV, "CLEAR_META_RESV"}, \
> { EXTENT_NEED_WAIT, "NEED_WAIT"}, \
> - { EXTENT_DAMAGED, "DAMAGED"}, \
> { EXTENT_NORESERVE, "NORESERVE"}, \
> { EXTENT_QGROUP_RESERVED, "QGROUP_RESERVED"}, \
> { EXTENT_CLEAR_DATA_RESV, "CLEAR_DATA_RESV"}, \