[1/5] btrfs: rename EXTENT_BUFFER_NO_CHECK to EXTENT_BUFFER_CANCELLED

Message ID 20231121-josef-generic-163-v1-1-049e37185841@wdc.com
State New
Headers
Series btrfs: zoned: remove extent_buffer redirtying |

Commit Message

Johannes Thumshirn Nov. 21, 2023, 4:32 p.m. UTC
  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

Christoph Hellwig Nov. 21, 2023, 5:02 p.m. UTC | #1
Looks good:

Reviewed-by: Christoph Hellwig <hch@lst.de>
  
David Sterba Nov. 22, 2023, 1:20 p.m. UTC | #2
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.
  
Johannes Thumshirn Nov. 22, 2023, 1:57 p.m. UTC | #3
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.
  

Patch

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);