[1/5] btrfs: rename EXTENT_BUFFER_NO_CHECK to EXTENT_BUFFER_CANCELLED
Commit Message
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(-)
Comments
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.
@@ -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;
}
@@ -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);
@@ -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;
@@ -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,
};
@@ -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);