| Message ID | 20231121-josef-generic-163-v1-1-049e37185841@wdc.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | btrfs: zoned: remove extent_buffer redirtying | expand |
Looks good:
Reviewed-by: Christoph Hellwig <hch@lst.de>
On Tue, Nov 21, 2023 at 08:32:30AM -0800, Johannes Thumshirn wrote: > EXTENT_BUFFER_CANCELLED better describes the state of the extent buffer, > namely its writeout has been cancelled. I've read the patches a few times and still can't see how the meaning of 'cancelled' fits. It's about cancelling write out yes, but I don't see anywhere explained why and why the eb is zeroed. This could be put next to the enum definition or to function that does the main part of the logic. You can also rename it to CANCELLED_WRITEOUT or use _ZONED_ in the name so it's clear that it has a special purpose etc, but as it is now I think it should be improved.
On 22.11.23 14:27, David Sterba wrote: > On Tue, Nov 21, 2023 at 08:32:30AM -0800, Johannes Thumshirn wrote: >> EXTENT_BUFFER_CANCELLED better describes the state of the extent buffer, >> namely its writeout has been cancelled. > > I've read the patches a few times and still can't see how the meaning of > 'cancelled' fits. It's about cancelling write out yes, but I don't see > anywhere explained why and why the eb is zeroed. This could be put next > to the enum definition or to function that does the main part of the > logic. You can also rename it to CANCELLED_WRITEOUT or use _ZONED_ in > the name so it's clear that it has a special purpose etc, but as it is > now I think it should be improved. > How about EXTENT_BUFFER_ZONED_ZEROOUT? It a) indicates the buffer is zeroed out and b) only for zoned mode.
diff --git a/fs/btrfs/disk-io.c b/fs/btrfs/disk-io.c index 5ac6789ca55f..ff6140e7eef7 100644 --- a/fs/btrfs/disk-io.c +++ b/fs/btrfs/disk-io.c @@ -254,7 +254,7 @@ blk_status_t btree_csum_one_bio(struct btrfs_bio *bbio) if (WARN_ON_ONCE(bbio->bio.bi_iter.bi_size != eb->len)) return BLK_STS_IOERR; - if (test_bit(EXTENT_BUFFER_NO_CHECK, &eb->bflags)) { + if (test_bit(EXTENT_BUFFER_CANCELLED, &eb->bflags)) { WARN_ON_ONCE(found_start != 0); return BLK_STS_OK; } diff --git a/fs/btrfs/extent-tree.c b/fs/btrfs/extent-tree.c index 0455935ff558..f6cbbec539fa 100644 --- a/fs/btrfs/extent-tree.c +++ b/fs/btrfs/extent-tree.c @@ -5041,7 +5041,7 @@ btrfs_init_new_buffer(struct btrfs_trans_handle *trans, struct btrfs_root *root, __btrfs_tree_lock(buf, nest); btrfs_clear_buffer_dirty(trans, buf); clear_bit(EXTENT_BUFFER_STALE, &buf->bflags); - clear_bit(EXTENT_BUFFER_NO_CHECK, &buf->bflags); + clear_bit(EXTENT_BUFFER_CANCELLED, &buf->bflags); set_extent_buffer_uptodate(buf); diff --git a/fs/btrfs/extent_io.c b/fs/btrfs/extent_io.c index 03cef28d9e37..74f984885719 100644 --- a/fs/btrfs/extent_io.c +++ b/fs/btrfs/extent_io.c @@ -4139,7 +4139,7 @@ static void __write_extent_buffer(const struct extent_buffer *eb, /* For unmapped (dummy) ebs, no need to check their uptodate status. */ const bool check_uptodate = !test_bit(EXTENT_BUFFER_UNMAPPED, &eb->bflags); - WARN_ON(test_bit(EXTENT_BUFFER_NO_CHECK, &eb->bflags)); + WARN_ON(test_bit(EXTENT_BUFFER_CANCELLED, &eb->bflags)); if (check_eb_range(eb, start, len)) return; diff --git a/fs/btrfs/extent_io.h b/fs/btrfs/extent_io.h index 2171057a4477..d5c9079dc578 100644 --- a/fs/btrfs/extent_io.h +++ b/fs/btrfs/extent_io.h @@ -28,7 +28,8 @@ enum { EXTENT_BUFFER_IN_TREE, /* write IO error */ EXTENT_BUFFER_WRITE_ERR, - EXTENT_BUFFER_NO_CHECK, + /* Indicate the extent buffer write out is cancelled (for zoned) */ + EXTENT_BUFFER_CANCELLED, /* Indicate that extent buffer pages a being read */ EXTENT_BUFFER_READING, }; diff --git a/fs/btrfs/zoned.c b/fs/btrfs/zoned.c index 188378ca19c7..89cd1664efe1 100644 --- a/fs/btrfs/zoned.c +++ b/fs/btrfs/zoned.c @@ -1725,7 +1725,7 @@ void btrfs_redirty_list_add(struct btrfs_transaction *trans, ASSERT(!test_bit(EXTENT_BUFFER_DIRTY, &eb->bflags)); memzero_extent_buffer(eb, 0, eb->len); - set_bit(EXTENT_BUFFER_NO_CHECK, &eb->bflags); + set_bit(EXTENT_BUFFER_CANCELLED, &eb->bflags); set_extent_buffer_dirty(eb); set_extent_bit(&trans->dirty_pages, eb->start, eb->start + eb->len - 1, EXTENT_DIRTY, NULL);
EXTENT_BUFFER_CANCELLED better describes the state of the extent buffer, namely its writeout has been cancelled. Signed-off-by: Johannes Thumshirn <johannes.thumshirn@wdc.com> --- fs/btrfs/disk-io.c | 2 +- fs/btrfs/extent-tree.c | 2 +- fs/btrfs/extent_io.c | 2 +- fs/btrfs/extent_io.h | 3 ++- fs/btrfs/zoned.c | 2 +- 5 files changed, 6 insertions(+), 5 deletions(-)