[PATCH 0/9] Parameter cleanups in extent state helpers

[PATCH 0/9] Parameter cleanups in extent state helpers

[David Sterba]

This is motivated by the gfp parameter passed to all the helpers and how
to get rid of it. Most of the values are GFP_NOFS with some exceptions.
One of them (GFP_NOFAIL) can be removed the other one (GFP_NOWAIT) is
transformed to existing parameters.

Module code size is lower for the whole series and stack is reduced in
about 50 functions by 8 bytes.

David Sterba (9):
  btrfs: open code set_extent_defrag
  btrfs: open code set_extent_delalloc
  btrfs: open code set_extent_new
  btrfs: open code set_extent_dirty
  btrfs: open code set_extent_bits_nowait
  btrfs: open code set_extent_bits
  btrfs: drop NOFAIL from set_extent_bit allocation masks
  btrfs: pass NOWAIT for set/clear extent bits as another bit
  btrfs: drop gfp from parameter extent state helpers

 fs/btrfs/block-group.c           |  6 ++--
 fs/btrfs/defrag.c                |  3 +-
 fs/btrfs/dev-replace.c           |  4 +--
 fs/btrfs/extent-io-tree.c        | 37 ++++++++++++-------
 fs/btrfs/extent-io-tree.h        | 62 ++++++++------------------------
 fs/btrfs/extent-tree.c           | 27 +++++++-------
 fs/btrfs/extent_io.c             |  3 +-
 fs/btrfs/extent_map.c            | 10 +++---
 fs/btrfs/file-item.c             | 10 +++---
 fs/btrfs/inode.c                 |  9 +++--
 fs/btrfs/relocation.c            | 10 +++---
 fs/btrfs/tests/extent-io-tests.c | 16 ++++-----
 fs/btrfs/zoned.c                 |  4 +--
 13 files changed, 91 insertions(+), 110 deletions(-)

-- 
2.40.0

[PATCH 1/9] btrfs: open code set_extent_defrag

[David Sterba]

The helper is used only once.

Signed-off-by: David Sterba <dsterba@suse.com>
---
 fs/btrfs/defrag.c         | 4 +++-
 fs/btrfs/extent-io-tree.h | 8 --------
 2 files changed, 3 insertions(+), 9 deletions(-)

diff --git a/fs/btrfs/defrag.c b/fs/btrfs/defrag.c
index 8065341d831a..4e7a1e0a0441 100644
--- a/fs/btrfs/defrag.c
+++ b/fs/btrfs/defrag.c
@@ -1040,7 +1040,9 @@ static int defrag_one_locked_target(struct btrfs_inode *inode,
 	clear_extent_bit(&inode->io_tree, start, start + len - 1,
 			 EXTENT_DELALLOC | EXTENT_DO_ACCOUNTING |
 			 EXTENT_DEFRAG, cached_state);
-	set_extent_defrag(&inode->io_tree, start, start + len - 1, cached_state);
+	set_extent_bit(&inode->io_tree, start, start + len - 1,
+		       EXTENT_DELALLOC | EXTENT_DEFRAG,
+		       cached_state, GFP_NOFS);
 
 	/* Update the page status */
 	for (i = start_index - first_index; i <= last_index - first_index; i++) {
diff --git a/fs/btrfs/extent-io-tree.h b/fs/btrfs/extent-io-tree.h
index 21766e49ec02..ea344e5ca24f 100644
--- a/fs/btrfs/extent-io-tree.h
+++ b/fs/btrfs/extent-io-tree.h
@@ -202,14 +202,6 @@ static inline int set_extent_delalloc(struct extent_io_tree *tree, u64 start,
 			      cached_state, GFP_NOFS);
 }
 
-static inline int set_extent_defrag(struct extent_io_tree *tree, u64 start,
-		u64 end, struct extent_state **cached_state)
-{
-	return set_extent_bit(tree, start, end,
-			      EXTENT_DELALLOC | EXTENT_DEFRAG,
-			      cached_state, GFP_NOFS);
-}
-
 static inline int set_extent_new(struct extent_io_tree *tree, u64 start,
 		u64 end)
 {
-- 
2.40.0

[PATCH 2/9] btrfs: open code set_extent_delalloc

[David Sterba]

The helper is used once in fs code and a few times in the self test
code.

Signed-off-by: David Sterba <dsterba@suse.com>
---
 fs/btrfs/extent-io-tree.h        | 9 ---------
 fs/btrfs/inode.c                 | 4 ++--
 fs/btrfs/tests/extent-io-tests.c | 6 +++---
 3 files changed, 5 insertions(+), 14 deletions(-)

diff --git a/fs/btrfs/extent-io-tree.h b/fs/btrfs/extent-io-tree.h
index ea344e5ca24f..e5289d67b6b7 100644
--- a/fs/btrfs/extent-io-tree.h
+++ b/fs/btrfs/extent-io-tree.h
@@ -193,15 +193,6 @@ int convert_extent_bit(struct extent_io_tree *tree, u64 start, u64 end,
 		       u32 bits, u32 clear_bits,
 		       struct extent_state **cached_state);
 
-static inline int set_extent_delalloc(struct extent_io_tree *tree, u64 start,
-				      u64 end, u32 extra_bits,
-				      struct extent_state **cached_state)
-{
-	return set_extent_bit(tree, start, end,
-			      EXTENT_DELALLOC | extra_bits,
-			      cached_state, GFP_NOFS);
-}
-
 static inline int set_extent_new(struct extent_io_tree *tree, u64 start,
 		u64 end)
 {
diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c
index 2e83fb225052..6144a2b89db2 100644
--- a/fs/btrfs/inode.c
+++ b/fs/btrfs/inode.c
@@ -2922,8 +2922,8 @@ int btrfs_set_extent_delalloc(struct btrfs_inode *inode, u64 start, u64 end,
 			return ret;
 	}
 
-	return set_extent_delalloc(&inode->io_tree, start, end, extra_bits,
-				   cached_state);
+	return set_extent_bit(&inode->io_tree, start, end,
+			      EXTENT_DELALLOC | extra_bits, cached_state, GFP_NOFS);
 }
 
 /* see btrfs_writepage_start_hook for details on why this is required */
diff --git a/fs/btrfs/tests/extent-io-tests.c b/fs/btrfs/tests/extent-io-tests.c
index dfc5c7fa6038..b9de94a50868 100644
--- a/fs/btrfs/tests/extent-io-tests.c
+++ b/fs/btrfs/tests/extent-io-tests.c
@@ -159,7 +159,7 @@ static int test_find_delalloc(u32 sectorsize)
 	 * |--- delalloc ---|
 	 * |---  search  ---|
 	 */
-	set_extent_delalloc(tmp, 0, sectorsize - 1, 0, NULL);
+	set_extent_bit(tmp, 0, sectorsize - 1, EXTENT_DELALLOC, NULL, GFP_NOFS);
 	start = 0;
 	end = start + PAGE_SIZE - 1;
 	found = find_lock_delalloc_range(inode, locked_page, &start,
@@ -190,7 +190,7 @@ static int test_find_delalloc(u32 sectorsize)
 		test_err("couldn't find the locked page");
 		goto out_bits;
 	}
-	set_extent_delalloc(tmp, sectorsize, max_bytes - 1, 0, NULL);
+	set_extent_bit(tmp, sectorsize, max_bytes - 1, EXTENT_DELALLOC, NULL, GFP_NOFS);
 	start = test_start;
 	end = start + PAGE_SIZE - 1;
 	found = find_lock_delalloc_range(inode, locked_page, &start,
@@ -245,7 +245,7 @@ static int test_find_delalloc(u32 sectorsize)
 	 *
 	 * We are re-using our test_start from above since it works out well.
 	 */
-	set_extent_delalloc(tmp, max_bytes, total_dirty - 1, 0, NULL);
+	set_extent_bit(tmp, max_bytes, total_dirty - 1, EXTENT_DELALLOC, NULL, GFP_NOFS);
 	start = test_start;
 	end = start + PAGE_SIZE - 1;
 	found = find_lock_delalloc_range(inode, locked_page, &start,
-- 
2.40.0

[PATCH 3/9] btrfs: open code set_extent_new

[David Sterba]

The helper is used only once.

Signed-off-by: David Sterba <dsterba@suse.com>
---
 fs/btrfs/extent-io-tree.h | 6 ------
 fs/btrfs/extent-tree.c    | 5 +++--
 2 files changed, 3 insertions(+), 8 deletions(-)

diff --git a/fs/btrfs/extent-io-tree.h b/fs/btrfs/extent-io-tree.h
index e5289d67b6b7..293e49377104 100644
--- a/fs/btrfs/extent-io-tree.h
+++ b/fs/btrfs/extent-io-tree.h
@@ -193,12 +193,6 @@ int convert_extent_bit(struct extent_io_tree *tree, u64 start, u64 end,
 		       u32 bits, u32 clear_bits,
 		       struct extent_state **cached_state);
 
-static inline int set_extent_new(struct extent_io_tree *tree, u64 start,
-		u64 end)
-{
-	return set_extent_bit(tree, start, end, EXTENT_NEW, NULL, GFP_NOFS);
-}
-
 int find_first_extent_bit(struct extent_io_tree *tree, u64 start,
 			  u64 *start_ret, u64 *end_ret, u32 bits,
 			  struct extent_state **cached_state);
diff --git a/fs/btrfs/extent-tree.c b/fs/btrfs/extent-tree.c
index 5de5b577e7fd..5c7c72042193 100644
--- a/fs/btrfs/extent-tree.c
+++ b/fs/btrfs/extent-tree.c
@@ -4832,8 +4832,9 @@ btrfs_init_new_buffer(struct btrfs_trans_handle *trans, struct btrfs_root *root,
 			set_extent_dirty(&root->dirty_log_pages, buf->start,
 					buf->start + buf->len - 1, GFP_NOFS);
 		else
-			set_extent_new(&root->dirty_log_pages, buf->start,
-					buf->start + buf->len - 1);
+			set_extent_bit(&root->dirty_log_pages, buf->start,
+				       buf->start + buf->len - 1,
+				       EXTENT_NEW, NULL, GFP_NOFS);
 	} else {
 		buf->log_index = -1;
 		set_extent_dirty(&trans->transaction->dirty_pages, buf->start,
-- 
2.40.0

[PATCH 4/9] btrfs: open code set_extent_dirty

[David Sterba]

The helper is used a few times, that it's setting the DIRTY extent bit
is still clear.

Signed-off-by: David Sterba <dsterba@suse.com>
---
 fs/btrfs/block-group.c    |  7 ++++---
 fs/btrfs/extent-io-tree.h |  6 ------
 fs/btrfs/extent-tree.c    | 15 +++++++++------
 3 files changed, 13 insertions(+), 15 deletions(-)

diff --git a/fs/btrfs/block-group.c b/fs/btrfs/block-group.c
index 590b03560265..ec463f8d83ec 100644
--- a/fs/btrfs/block-group.c
+++ b/fs/btrfs/block-group.c
@@ -3521,9 +3521,10 @@ int btrfs_update_block_group(struct btrfs_trans_handle *trans,
 			spin_unlock(&cache->lock);
 			spin_unlock(&space_info->lock);
 
-			set_extent_dirty(&trans->transaction->pinned_extents,
-					 bytenr, bytenr + num_bytes - 1,
-					 GFP_NOFS | __GFP_NOFAIL);
+			set_extent_bit(&trans->transaction->pinned_extents,
+				       bytenr, bytenr + num_bytes - 1,
+				       EXTENT_DIRTY, NULL,
+				       GFP_NOFS | __GFP_NOFAIL);
 		}
 
 		spin_lock(&trans->transaction->dirty_bgs_lock);
diff --git a/fs/btrfs/extent-io-tree.h b/fs/btrfs/extent-io-tree.h
index 293e49377104..0fc54546a63d 100644
--- a/fs/btrfs/extent-io-tree.h
+++ b/fs/btrfs/extent-io-tree.h
@@ -175,12 +175,6 @@ static inline int clear_extent_uptodate(struct extent_io_tree *tree, u64 start,
 				  cached_state, GFP_NOFS, NULL);
 }
 
-static inline int set_extent_dirty(struct extent_io_tree *tree, u64 start,
-		u64 end, gfp_t mask)
-{
-	return set_extent_bit(tree, start, end, EXTENT_DIRTY, NULL, mask);
-}
-
 static inline int clear_extent_dirty(struct extent_io_tree *tree, u64 start,
 				     u64 end, struct extent_state **cached)
 {
diff --git a/fs/btrfs/extent-tree.c b/fs/btrfs/extent-tree.c
index 5c7c72042193..47cfb694cdbf 100644
--- a/fs/btrfs/extent-tree.c
+++ b/fs/btrfs/extent-tree.c
@@ -2507,8 +2507,9 @@ static int pin_down_extent(struct btrfs_trans_handle *trans,
 	spin_unlock(&cache->lock);
 	spin_unlock(&cache->space_info->lock);
 
-	set_extent_dirty(&trans->transaction->pinned_extents, bytenr,
-			 bytenr + num_bytes - 1, GFP_NOFS | __GFP_NOFAIL);
+	set_extent_bit(&trans->transaction->pinned_extents, bytenr,
+		       bytenr + num_bytes - 1, EXTENT_DIRTY, NULL,
+		       GFP_NOFS | __GFP_NOFAIL);
 	return 0;
 }
 
@@ -4829,16 +4830,18 @@ btrfs_init_new_buffer(struct btrfs_trans_handle *trans, struct btrfs_root *root,
 		 * EXTENT bit to differentiate dirty pages.
 		 */
 		if (buf->log_index == 0)
-			set_extent_dirty(&root->dirty_log_pages, buf->start,
-					buf->start + buf->len - 1, GFP_NOFS);
+			set_extent_bit(&root->dirty_log_pages, buf->start,
+				       buf->start + buf->len - 1,
+				       EXTENT_DIRTY, NULL, GFP_NOFS);
 		else
 			set_extent_bit(&root->dirty_log_pages, buf->start,
 				       buf->start + buf->len - 1,
 				       EXTENT_NEW, NULL, GFP_NOFS);
 	} else {
 		buf->log_index = -1;
-		set_extent_dirty(&trans->transaction->dirty_pages, buf->start,
-			 buf->start + buf->len - 1, GFP_NOFS);
+		set_extent_bit(&trans->transaction->dirty_pages, buf->start,
+			       buf->start + buf->len - 1, EXTENT_DIRTY, NULL,
+			       GFP_NOFS);
 	}
 	/* this returns a buffer locked for blocking */
 	return buf;
-- 
2.40.0

[PATCH 5/9] btrfs: open code set_extent_bits_nowait

[David Sterba]

The helper only passes GFP_NOWAIT as gfp flags and is used two times.

Signed-off-by: David Sterba <dsterba@suse.com>
---
 fs/btrfs/extent-io-tree.h | 6 ------
 fs/btrfs/extent_map.c     | 5 +++--
 fs/btrfs/zoned.c          | 4 ++--
 3 files changed, 5 insertions(+), 10 deletions(-)

diff --git a/fs/btrfs/extent-io-tree.h b/fs/btrfs/extent-io-tree.h
index 0fc54546a63d..ef9d54cdee5c 100644
--- a/fs/btrfs/extent-io-tree.h
+++ b/fs/btrfs/extent-io-tree.h
@@ -156,12 +156,6 @@ int set_record_extent_bits(struct extent_io_tree *tree, u64 start, u64 end,
 int set_extent_bit(struct extent_io_tree *tree, u64 start, u64 end,
 		   u32 bits, struct extent_state **cached_state, gfp_t mask);
 
-static inline int set_extent_bits_nowait(struct extent_io_tree *tree, u64 start,
-					 u64 end, u32 bits)
-{
-	return set_extent_bit(tree, start, end, bits, NULL, GFP_NOWAIT);
-}
-
 static inline int set_extent_bits(struct extent_io_tree *tree, u64 start,
 		u64 end, u32 bits)
 {
diff --git a/fs/btrfs/extent_map.c b/fs/btrfs/extent_map.c
index 138afa955370..918ce12ea412 100644
--- a/fs/btrfs/extent_map.c
+++ b/fs/btrfs/extent_map.c
@@ -364,8 +364,9 @@ static void extent_map_device_set_bits(struct extent_map *em, unsigned bits)
 		struct btrfs_io_stripe *stripe = &map->stripes[i];
 		struct btrfs_device *device = stripe->dev;
 
-		set_extent_bits_nowait(&device->alloc_state, stripe->physical,
-				 stripe->physical + stripe_size - 1, bits);
+		set_extent_bit(&device->alloc_state, stripe->physical,
+			       stripe->physical + stripe_size - 1, bits, NULL,
+			       GFP_NOWAIT);
 	}
 }
 
diff --git a/fs/btrfs/zoned.c b/fs/btrfs/zoned.c
index bda301a55cbe..b82a350c4c59 100644
--- a/fs/btrfs/zoned.c
+++ b/fs/btrfs/zoned.c
@@ -1611,8 +1611,8 @@ void btrfs_redirty_list_add(struct btrfs_transaction *trans,
 	memzero_extent_buffer(eb, 0, eb->len);
 	set_bit(EXTENT_BUFFER_NO_CHECK, &eb->bflags);
 	set_extent_buffer_dirty(eb);
-	set_extent_bits_nowait(&trans->dirty_pages, eb->start,
-			       eb->start + eb->len - 1, EXTENT_DIRTY);
+	set_extent_bit(&trans->dirty_pages, eb->start, eb->start + eb->len - 1,
+			EXTENT_DIRTY, NULL, GFP_NOWAIT);
 }
 
 bool btrfs_use_zone_append(struct btrfs_bio *bbio)
-- 
2.40.0

[PATCH 6/9] btrfs: open code set_extent_bits

[David Sterba]

This helper calls set_extent_bit with two more parameters set to default
values, but otherwise it's purpose is not clear.

Signed-off-by: David Sterba <dsterba@suse.com>
---
 fs/btrfs/dev-replace.c           |  4 ++--
 fs/btrfs/extent-io-tree.h        |  6 ------
 fs/btrfs/extent-tree.c           | 10 +++++-----
 fs/btrfs/file-item.c             | 10 +++++-----
 fs/btrfs/relocation.c            | 11 ++++++-----
 fs/btrfs/tests/extent-io-tests.c | 11 ++++++-----
 6 files changed, 24 insertions(+), 28 deletions(-)

diff --git a/fs/btrfs/dev-replace.c b/fs/btrfs/dev-replace.c
index 78696d331639..3a0fc57d5db9 100644
--- a/fs/btrfs/dev-replace.c
+++ b/fs/btrfs/dev-replace.c
@@ -795,8 +795,8 @@ static int btrfs_set_target_alloc_state(struct btrfs_device *srcdev,
 	while (!find_first_extent_bit(&srcdev->alloc_state, start,
 				      &found_start, &found_end,
 				      CHUNK_ALLOCATED, &cached_state)) {
-		ret = set_extent_bits(&tgtdev->alloc_state, found_start,
-				      found_end, CHUNK_ALLOCATED);
+		ret = set_extent_bit(&tgtdev->alloc_state, found_start,
+				     found_end, CHUNK_ALLOCATED, NULL, GFP_NOFS);
 		if (ret)
 			break;
 		start = found_end + 1;
diff --git a/fs/btrfs/extent-io-tree.h b/fs/btrfs/extent-io-tree.h
index ef9d54cdee5c..5a53a4558366 100644
--- a/fs/btrfs/extent-io-tree.h
+++ b/fs/btrfs/extent-io-tree.h
@@ -156,12 +156,6 @@ int set_record_extent_bits(struct extent_io_tree *tree, u64 start, u64 end,
 int set_extent_bit(struct extent_io_tree *tree, u64 start, u64 end,
 		   u32 bits, struct extent_state **cached_state, gfp_t mask);
 
-static inline int set_extent_bits(struct extent_io_tree *tree, u64 start,
-		u64 end, u32 bits)
-{
-	return set_extent_bit(tree, start, end, bits, NULL, GFP_NOFS);
-}
-
 static inline int clear_extent_uptodate(struct extent_io_tree *tree, u64 start,
 		u64 end, struct extent_state **cached_state)
 {
diff --git a/fs/btrfs/extent-tree.c b/fs/btrfs/extent-tree.c
index 47cfb694cdbf..03b2a7c508b9 100644
--- a/fs/btrfs/extent-tree.c
+++ b/fs/btrfs/extent-tree.c
@@ -73,8 +73,8 @@ int btrfs_add_excluded_extent(struct btrfs_fs_info *fs_info,
 			      u64 start, u64 num_bytes)
 {
 	u64 end = start + num_bytes - 1;
-	set_extent_bits(&fs_info->excluded_extents, start, end,
-			EXTENT_UPTODATE);
+	set_extent_bit(&fs_info->excluded_extents, start, end,
+		       EXTENT_UPTODATE, NULL, GFP_NOFS);
 	return 0;
 }
 
@@ -5981,9 +5981,9 @@ static int btrfs_trim_free_extents(struct btrfs_device *device, u64 *trimmed)
 		ret = btrfs_issue_discard(device->bdev, start, len,
 					  &bytes);
 		if (!ret)
-			set_extent_bits(&device->alloc_state, start,
-					start + bytes - 1,
-					CHUNK_TRIMMED);
+			set_extent_bit(&device->alloc_state, start,
+				       start + bytes - 1,
+				       CHUNK_TRIMMED, NULL, GFP_NOFS);
 		mutex_unlock(&fs_info->chunk_mutex);
 
 		if (ret)
diff --git a/fs/btrfs/file-item.c b/fs/btrfs/file-item.c
index e74b9804bcde..1e364a7b74aa 100644
--- a/fs/btrfs/file-item.c
+++ b/fs/btrfs/file-item.c
@@ -94,8 +94,8 @@ int btrfs_inode_set_file_extent_range(struct btrfs_inode *inode, u64 start,
 
 	if (btrfs_fs_incompat(inode->root->fs_info, NO_HOLES))
 		return 0;
-	return set_extent_bits(&inode->file_extent_tree, start, start + len - 1,
-			       EXTENT_DIRTY);
+	return set_extent_bit(&inode->file_extent_tree, start, start + len - 1,
+			      EXTENT_DIRTY, NULL, GFP_NOFS);
 }
 
 /*
@@ -438,9 +438,9 @@ blk_status_t btrfs_lookup_bio_sums(struct btrfs_bio *bbio)
 			    BTRFS_DATA_RELOC_TREE_OBJECTID) {
 				u64 file_offset = bbio->file_offset + bio_offset;
 
-				set_extent_bits(&inode->io_tree, file_offset,
-						file_offset + sectorsize - 1,
-						EXTENT_NODATASUM);
+				set_extent_bit(&inode->io_tree, file_offset,
+					       file_offset + sectorsize - 1,
+					       EXTENT_NODATASUM, NULL, GFP_NOFS);
 			} else {
 				btrfs_warn_rl(fs_info,
 			"csum hole found for disk bytenr range [%llu, %llu)",
diff --git a/fs/btrfs/relocation.c b/fs/btrfs/relocation.c
index 38cfbd38a819..1ed8b132fe2a 100644
--- a/fs/btrfs/relocation.c
+++ b/fs/btrfs/relocation.c
@@ -174,8 +174,9 @@ static void mark_block_processed(struct reloc_control *rc,
 	    in_range(node->bytenr, rc->block_group->start,
 		     rc->block_group->length)) {
 		blocksize = rc->extent_root->fs_info->nodesize;
-		set_extent_bits(&rc->processed_blocks, node->bytenr,
-				node->bytenr + blocksize - 1, EXTENT_DIRTY);
+		set_extent_bit(&rc->processed_blocks, node->bytenr,
+			       node->bytenr + blocksize - 1, EXTENT_DIRTY,
+			       NULL, GFP_NOFS);
 	}
 	node->processed = 1;
 }
@@ -3051,9 +3052,9 @@ static int relocate_one_page(struct inode *inode, struct file_ra_state *ra,
 			u64 boundary_end = boundary_start +
 					   fs_info->sectorsize - 1;
 
-			set_extent_bits(&BTRFS_I(inode)->io_tree,
-					boundary_start, boundary_end,
-					EXTENT_BOUNDARY);
+			set_extent_bit(&BTRFS_I(inode)->io_tree,
+				       boundary_start, boundary_end,
+				       EXTENT_BOUNDARY, NULL, GFP_NOFS);
 		}
 		unlock_extent(&BTRFS_I(inode)->io_tree, clamped_start, clamped_end,
 			      &cached_state);
diff --git a/fs/btrfs/tests/extent-io-tests.c b/fs/btrfs/tests/extent-io-tests.c
index b9de94a50868..acaaddf7181a 100644
--- a/fs/btrfs/tests/extent-io-tests.c
+++ b/fs/btrfs/tests/extent-io-tests.c
@@ -503,8 +503,8 @@ static int test_find_first_clear_extent_bit(void)
 	 * Set 1M-4M alloc/discard and 32M-64M thus leaving a hole between
 	 * 4M-32M
 	 */
-	set_extent_bits(&tree, SZ_1M, SZ_4M - 1,
-			CHUNK_TRIMMED | CHUNK_ALLOCATED);
+	set_extent_bit(&tree, SZ_1M, SZ_4M - 1,
+		       CHUNK_TRIMMED | CHUNK_ALLOCATED, NULL, GFP_NOFS);
 
 	find_first_clear_extent_bit(&tree, SZ_512K, &start, &end,
 				    CHUNK_TRIMMED | CHUNK_ALLOCATED);
@@ -516,8 +516,8 @@ static int test_find_first_clear_extent_bit(void)
 	}
 
 	/* Now add 32M-64M so that we have a hole between 4M-32M */
-	set_extent_bits(&tree, SZ_32M, SZ_64M - 1,
-			CHUNK_TRIMMED | CHUNK_ALLOCATED);
+	set_extent_bit(&tree, SZ_32M, SZ_64M - 1,
+		       CHUNK_TRIMMED | CHUNK_ALLOCATED, NULL, GFP_NOFS);
 
 	/*
 	 * Request first hole starting at 12M, we should get 4M-32M
@@ -548,7 +548,8 @@ static int test_find_first_clear_extent_bit(void)
 	 * Set 64M-72M with CHUNK_ALLOC flag, then search for CHUNK_TRIMMED flag
 	 * being unset in this range, we should get the entry in range 64M-72M
 	 */
-	set_extent_bits(&tree, SZ_64M, SZ_64M + SZ_8M - 1, CHUNK_ALLOCATED);
+	set_extent_bit(&tree, SZ_64M, SZ_64M + SZ_8M - 1, CHUNK_ALLOCATED, NULL,
+		       GFP_NOFS);
 	find_first_clear_extent_bit(&tree, SZ_64M + SZ_1M, &start, &end,
 				    CHUNK_TRIMMED);
 
-- 
2.40.0

[PATCH 7/9] btrfs: drop NOFAIL from set_extent_bit allocation masks

[David Sterba]

The __GFP_NOFAIL passed to set_extent_bit first appeared in 2010
(commit f0486c68e4bd9a ("Btrfs: Introduce contexts for metadata
reservation")), without any explanation why it would be needed.

Meanwhile we've updated the semantics of set_extent_bit to handle failed
allocations and do unlock, sleep and retry if needed.  The use of the
NOFAIL flag is also an outlier, we never want any of the set/clear
extent bit helpers to fail, they're used for many critical changes like
extent locking, besides the extent state bit changes.

Signed-off-by: David Sterba <dsterba@suse.com>
---
 fs/btrfs/block-group.c | 3 +--
 fs/btrfs/extent-tree.c | 3 +--
 2 files changed, 2 insertions(+), 4 deletions(-)

diff --git a/fs/btrfs/block-group.c b/fs/btrfs/block-group.c
index ec463f8d83ec..202e2aa949c5 100644
--- a/fs/btrfs/block-group.c
+++ b/fs/btrfs/block-group.c
@@ -3523,8 +3523,7 @@ int btrfs_update_block_group(struct btrfs_trans_handle *trans,
 
 			set_extent_bit(&trans->transaction->pinned_extents,
 				       bytenr, bytenr + num_bytes - 1,
-				       EXTENT_DIRTY, NULL,
-				       GFP_NOFS | __GFP_NOFAIL);
+				       EXTENT_DIRTY, NULL, GFP_NOFS);
 		}
 
 		spin_lock(&trans->transaction->dirty_bgs_lock);
diff --git a/fs/btrfs/extent-tree.c b/fs/btrfs/extent-tree.c
index 03b2a7c508b9..6e319100e3a3 100644
--- a/fs/btrfs/extent-tree.c
+++ b/fs/btrfs/extent-tree.c
@@ -2508,8 +2508,7 @@ static int pin_down_extent(struct btrfs_trans_handle *trans,
 	spin_unlock(&cache->space_info->lock);
 
 	set_extent_bit(&trans->transaction->pinned_extents, bytenr,
-		       bytenr + num_bytes - 1, EXTENT_DIRTY, NULL,
-		       GFP_NOFS | __GFP_NOFAIL);
+		       bytenr + num_bytes - 1, EXTENT_DIRTY, NULL, GFP_NOFS);
 	return 0;
 }
 
-- 
2.40.0

[PATCH 8/9] btrfs: pass NOWAIT for set/clear extent bits as another bit

[David Sterba]

The only flags we now pass to set_extent_bit/__clear_extent_bit are
GFP_NOFS and GFP_NOWAIT (a few functions handling mappings). This
requires an extra parameter to be passed everywhere but is almost always
the same.

Encode the GFP_NOWAIT as an artificial extent bit and extract the
real bits and gfp mask in the lowest level helpers. Now the passed
gfp mask is not actually used and can be removed.

Signed-off-by: David Sterba <dsterba@suse.com>
---
 fs/btrfs/extent-io-tree.c | 12 ++++++++++++
 fs/btrfs/extent-io-tree.h |  9 +++++++++
 fs/btrfs/extent_map.c     |  7 ++++---
 fs/btrfs/zoned.c          |  2 +-
 4 files changed, 26 insertions(+), 4 deletions(-)

diff --git a/fs/btrfs/extent-io-tree.c b/fs/btrfs/extent-io-tree.c
index 29a225836e28..83e40c02f62e 100644
--- a/fs/btrfs/extent-io-tree.c
+++ b/fs/btrfs/extent-io-tree.c
@@ -532,6 +532,16 @@ static struct extent_state *clear_state_bit(struct extent_io_tree *tree,
 	return next;
 }
 
+/*
+ * Detect if extent bits request NOWAIT semantics and set the gfp mask accordingly,
+ * unset the EXTENT_NOWAIT bit.
+ */
+static void set_gfp_mask_from_bits(u32 *bits, gfp_t *mask)
+{
+	*mask = (*bits & EXTENT_NOWAIT ? GFP_NOWAIT : GFP_NOFS);
+	*bits &= EXTENT_NOWAIT - 1;
+}
+
 /*
  * Clear some bits on a range in the tree.  This may require splitting or
  * inserting elements in the tree, so the gfp mask is used to indicate which
@@ -557,6 +567,7 @@ int __clear_extent_bit(struct extent_io_tree *tree, u64 start, u64 end,
 	int wake;
 	int delete = (bits & EXTENT_CLEAR_ALL_BITS);
 
+	set_gfp_mask_from_bits(&bits, &mask);
 	btrfs_debug_check_extent_io_range(tree, start, end);
 	trace_btrfs_clear_extent_bit(tree, start, end - start + 1, bits);
 
@@ -979,6 +990,7 @@ static int __set_extent_bit(struct extent_io_tree *tree, u64 start, u64 end,
 	u64 last_end;
 	u32 exclusive_bits = (bits & EXTENT_LOCKED);
 
+	set_gfp_mask_from_bits(&bits, &mask);
 	btrfs_debug_check_extent_io_range(tree, start, end);
 	trace_btrfs_set_extent_bit(tree, start, end - start + 1, bits);
 
diff --git a/fs/btrfs/extent-io-tree.h b/fs/btrfs/extent-io-tree.h
index 5a53a4558366..d7f5afeb5ce7 100644
--- a/fs/btrfs/extent-io-tree.h
+++ b/fs/btrfs/extent-io-tree.h
@@ -43,6 +43,15 @@ enum {
 	 * want the extent states to go away.
 	 */
 	ENUM_BIT(EXTENT_CLEAR_ALL_BITS),
+
+	/*
+	 * This must be last.
+	 *
+	 * Bit not representing a state but a request for NOWAIT semantics,
+	 * e.g. when allocating memory, and must be masked out from the other
+	 * bits.
+	 */
+	ENUM_BIT(EXTENT_NOWAIT)
 };
 
 #define EXTENT_DO_ACCOUNTING    (EXTENT_CLEAR_META_RESV | \
diff --git a/fs/btrfs/extent_map.c b/fs/btrfs/extent_map.c
index 918ce12ea412..4c8c87524d62 100644
--- a/fs/btrfs/extent_map.c
+++ b/fs/btrfs/extent_map.c
@@ -365,8 +365,8 @@ static void extent_map_device_set_bits(struct extent_map *em, unsigned bits)
 		struct btrfs_device *device = stripe->dev;
 
 		set_extent_bit(&device->alloc_state, stripe->physical,
-			       stripe->physical + stripe_size - 1, bits, NULL,
-			       GFP_NOWAIT);
+			       stripe->physical + stripe_size - 1,
+			       bits | EXTENT_NOWAIT, NULL, GFP_NOWAIT);
 	}
 }
 
@@ -381,7 +381,8 @@ static void extent_map_device_clear_bits(struct extent_map *em, unsigned bits)
 		struct btrfs_device *device = stripe->dev;
 
 		__clear_extent_bit(&device->alloc_state, stripe->physical,
-				   stripe->physical + stripe_size - 1, bits,
+				   stripe->physical + stripe_size - 1,
+				   bits | EXTENT_NOWAIT,
 				   NULL, GFP_NOWAIT, NULL);
 	}
 }
diff --git a/fs/btrfs/zoned.c b/fs/btrfs/zoned.c
index b82a350c4c59..fb90e2b20614 100644
--- a/fs/btrfs/zoned.c
+++ b/fs/btrfs/zoned.c
@@ -1612,7 +1612,7 @@ void btrfs_redirty_list_add(struct btrfs_transaction *trans,
 	set_bit(EXTENT_BUFFER_NO_CHECK, &eb->bflags);
 	set_extent_buffer_dirty(eb);
 	set_extent_bit(&trans->dirty_pages, eb->start, eb->start + eb->len - 1,
-			EXTENT_DIRTY, NULL, GFP_NOWAIT);
+			EXTENT_DIRTY | EXTENT_NOWAIT, NULL, GFP_NOWAIT);
 }
 
 bool btrfs_use_zone_append(struct btrfs_bio *bbio)
-- 
2.40.0

[PATCH 9/9] btrfs: drop gfp from parameter extent state helpers

[David Sterba]

Now that all extent state bit helpers effectively take the GFP_NOFS mask
(and GFP_NOWAIT is encoded in the bits) we can remove the parameter.
This reduces stack consumption in many functions and simplifies a lot of
code.

Net effect on module on a release build:

   text    data     bss     dec     hex filename
1250432   20985   16088 1287505  13a551 pre/btrfs.ko
1247074   20985   16088 1284147  139833 post/btrfs.ko

DELTA: -3358

Signed-off-by: David Sterba <dsterba@suse.com>
---
 fs/btrfs/block-group.c           |  2 +-
 fs/btrfs/defrag.c                |  3 +--
 fs/btrfs/dev-replace.c           |  2 +-
 fs/btrfs/extent-io-tree.c        | 25 +++++++++++++------------
 fs/btrfs/extent-io-tree.h        | 12 +++++-------
 fs/btrfs/extent-tree.c           | 14 ++++++--------
 fs/btrfs/extent_io.c             |  3 +--
 fs/btrfs/extent_map.c            |  4 ++--
 fs/btrfs/file-item.c             |  4 ++--
 fs/btrfs/inode.c                 |  7 +++----
 fs/btrfs/relocation.c            |  5 ++---
 fs/btrfs/tests/extent-io-tests.c | 13 ++++++-------
 fs/btrfs/zoned.c                 |  2 +-
 13 files changed, 44 insertions(+), 52 deletions(-)

diff --git a/fs/btrfs/block-group.c b/fs/btrfs/block-group.c
index 202e2aa949c5..618ba7670e66 100644
--- a/fs/btrfs/block-group.c
+++ b/fs/btrfs/block-group.c
@@ -3523,7 +3523,7 @@ int btrfs_update_block_group(struct btrfs_trans_handle *trans,
 
 			set_extent_bit(&trans->transaction->pinned_extents,
 				       bytenr, bytenr + num_bytes - 1,
-				       EXTENT_DIRTY, NULL, GFP_NOFS);
+				       EXTENT_DIRTY, NULL);
 		}
 
 		spin_lock(&trans->transaction->dirty_bgs_lock);
diff --git a/fs/btrfs/defrag.c b/fs/btrfs/defrag.c
index 4e7a1e0a0441..f2ff4cbe8656 100644
--- a/fs/btrfs/defrag.c
+++ b/fs/btrfs/defrag.c
@@ -1041,8 +1041,7 @@ static int defrag_one_locked_target(struct btrfs_inode *inode,
 			 EXTENT_DELALLOC | EXTENT_DO_ACCOUNTING |
 			 EXTENT_DEFRAG, cached_state);
 	set_extent_bit(&inode->io_tree, start, start + len - 1,
-		       EXTENT_DELALLOC | EXTENT_DEFRAG,
-		       cached_state, GFP_NOFS);
+		       EXTENT_DELALLOC | EXTENT_DEFRAG, cached_state);
 
 	/* Update the page status */
 	for (i = start_index - first_index; i <= last_index - first_index; i++) {
diff --git a/fs/btrfs/dev-replace.c b/fs/btrfs/dev-replace.c
index 3a0fc57d5db9..dc3f30c79320 100644
--- a/fs/btrfs/dev-replace.c
+++ b/fs/btrfs/dev-replace.c
@@ -796,7 +796,7 @@ static int btrfs_set_target_alloc_state(struct btrfs_device *srcdev,
 				      &found_start, &found_end,
 				      CHUNK_ALLOCATED, &cached_state)) {
 		ret = set_extent_bit(&tgtdev->alloc_state, found_start,
-				     found_end, CHUNK_ALLOCATED, NULL, GFP_NOFS);
+				     found_end, CHUNK_ALLOCATED, NULL);
 		if (ret)
 			break;
 		start = found_end + 1;
diff --git a/fs/btrfs/extent-io-tree.c b/fs/btrfs/extent-io-tree.c
index 83e40c02f62e..a2315a4b8b75 100644
--- a/fs/btrfs/extent-io-tree.c
+++ b/fs/btrfs/extent-io-tree.c
@@ -556,7 +556,7 @@ static void set_gfp_mask_from_bits(u32 *bits, gfp_t *mask)
  */
 int __clear_extent_bit(struct extent_io_tree *tree, u64 start, u64 end,
 		       u32 bits, struct extent_state **cached_state,
-		       gfp_t mask, struct extent_changeset *changeset)
+		       struct extent_changeset *changeset)
 {
 	struct extent_state *state;
 	struct extent_state *cached;
@@ -566,6 +566,7 @@ int __clear_extent_bit(struct extent_io_tree *tree, u64 start, u64 end,
 	int clear = 0;
 	int wake;
 	int delete = (bits & EXTENT_CLEAR_ALL_BITS);
+	gfp_t mask;
 
 	set_gfp_mask_from_bits(&bits, &mask);
 	btrfs_debug_check_extent_io_range(tree, start, end);
@@ -964,7 +965,8 @@ bool btrfs_find_delalloc_range(struct extent_io_tree *tree, u64 *start,
 
 /*
  * Set some bits on a range in the tree.  This may require allocations or
- * sleeping, so the gfp mask is used to indicate what is allowed.
+ * sleeping. By default all allocations use GFP_NOFS, use EXTENT_NOWAIT for
+ * GFP_NOWAIT.
  *
  * If any of the exclusive bits are set, this will fail with -EEXIST if some
  * part of the range already has the desired bits set.  The extent_state of the
@@ -979,7 +981,7 @@ static int __set_extent_bit(struct extent_io_tree *tree, u64 start, u64 end,
 			    u32 bits, u64 *failed_start,
 			    struct extent_state **failed_state,
 			    struct extent_state **cached_state,
-			    struct extent_changeset *changeset, gfp_t mask)
+			    struct extent_changeset *changeset)
 {
 	struct extent_state *state;
 	struct extent_state *prealloc = NULL;
@@ -989,6 +991,7 @@ static int __set_extent_bit(struct extent_io_tree *tree, u64 start, u64 end,
 	u64 last_start;
 	u64 last_end;
 	u32 exclusive_bits = (bits & EXTENT_LOCKED);
+	gfp_t mask;
 
 	set_gfp_mask_from_bits(&bits, &mask);
 	btrfs_debug_check_extent_io_range(tree, start, end);
@@ -1200,10 +1203,10 @@ static int __set_extent_bit(struct extent_io_tree *tree, u64 start, u64 end,
 }
 
 int set_extent_bit(struct extent_io_tree *tree, u64 start, u64 end,
-		   u32 bits, struct extent_state **cached_state, gfp_t mask)
+		   u32 bits, struct extent_state **cached_state)
 {
 	return __set_extent_bit(tree, start, end, bits, NULL, NULL,
-				cached_state, NULL, mask);
+				cached_state, NULL);
 }
 
 /*
@@ -1699,8 +1702,7 @@ int set_record_extent_bits(struct extent_io_tree *tree, u64 start, u64 end,
 	 */
 	ASSERT(!(bits & EXTENT_LOCKED));
 
-	return __set_extent_bit(tree, start, end, bits, NULL, NULL, NULL,
-				changeset, GFP_NOFS);
+	return __set_extent_bit(tree, start, end, bits, NULL, NULL, NULL, changeset);
 }
 
 int clear_record_extent_bits(struct extent_io_tree *tree, u64 start, u64 end,
@@ -1712,8 +1714,7 @@ int clear_record_extent_bits(struct extent_io_tree *tree, u64 start, u64 end,
 	 */
 	ASSERT(!(bits & EXTENT_LOCKED));
 
-	return __clear_extent_bit(tree, start, end, bits, NULL, GFP_NOFS,
-				  changeset);
+	return __clear_extent_bit(tree, start, end, bits, NULL, changeset);
 }
 
 int try_lock_extent(struct extent_io_tree *tree, u64 start, u64 end,
@@ -1723,7 +1724,7 @@ int try_lock_extent(struct extent_io_tree *tree, u64 start, u64 end,
 	u64 failed_start;
 
 	err = __set_extent_bit(tree, start, end, EXTENT_LOCKED, &failed_start,
-			       NULL, cached, NULL, GFP_NOFS);
+			       NULL, cached, NULL);
 	if (err == -EEXIST) {
 		if (failed_start > start)
 			clear_extent_bit(tree, start, failed_start - 1,
@@ -1745,7 +1746,7 @@ int lock_extent(struct extent_io_tree *tree, u64 start, u64 end,
 	u64 failed_start;
 
 	err = __set_extent_bit(tree, start, end, EXTENT_LOCKED, &failed_start,
-			       &failed_state, cached_state, NULL, GFP_NOFS);
+			       &failed_state, cached_state, NULL);
 	while (err == -EEXIST) {
 		if (failed_start != start)
 			clear_extent_bit(tree, start, failed_start - 1,
@@ -1755,7 +1756,7 @@ int lock_extent(struct extent_io_tree *tree, u64 start, u64 end,
 				&failed_state);
 		err = __set_extent_bit(tree, start, end, EXTENT_LOCKED,
 				       &failed_start, &failed_state,
-				       cached_state, NULL, GFP_NOFS);
+				       cached_state, NULL);
 	}
 	return err;
 }
diff --git a/fs/btrfs/extent-io-tree.h b/fs/btrfs/extent-io-tree.h
index d7f5afeb5ce7..fbd3b275ab1c 100644
--- a/fs/btrfs/extent-io-tree.h
+++ b/fs/btrfs/extent-io-tree.h
@@ -136,22 +136,20 @@ int test_range_bit(struct extent_io_tree *tree, u64 start, u64 end,
 int clear_record_extent_bits(struct extent_io_tree *tree, u64 start, u64 end,
 			     u32 bits, struct extent_changeset *changeset);
 int __clear_extent_bit(struct extent_io_tree *tree, u64 start, u64 end,
-		       u32 bits, struct extent_state **cached, gfp_t mask,
+		       u32 bits, struct extent_state **cached,
 		       struct extent_changeset *changeset);
 
 static inline int clear_extent_bit(struct extent_io_tree *tree, u64 start,
 				   u64 end, u32 bits,
 				   struct extent_state **cached)
 {
-	return __clear_extent_bit(tree, start, end, bits, cached,
-				  GFP_NOFS, NULL);
+	return __clear_extent_bit(tree, start, end, bits, cached, NULL);
 }
 
 static inline int unlock_extent(struct extent_io_tree *tree, u64 start, u64 end,
 				struct extent_state **cached)
 {
-	return __clear_extent_bit(tree, start, end, EXTENT_LOCKED, cached,
-				  GFP_NOFS, NULL);
+	return __clear_extent_bit(tree, start, end, EXTENT_LOCKED, cached, NULL);
 }
 
 static inline int clear_extent_bits(struct extent_io_tree *tree, u64 start,
@@ -163,13 +161,13 @@ static inline int clear_extent_bits(struct extent_io_tree *tree, u64 start,
 int set_record_extent_bits(struct extent_io_tree *tree, u64 start, u64 end,
 			   u32 bits, struct extent_changeset *changeset);
 int set_extent_bit(struct extent_io_tree *tree, u64 start, u64 end,
-		   u32 bits, struct extent_state **cached_state, gfp_t mask);
+		   u32 bits, struct extent_state **cached_state);
 
 static inline int clear_extent_uptodate(struct extent_io_tree *tree, u64 start,
 		u64 end, struct extent_state **cached_state)
 {
 	return __clear_extent_bit(tree, start, end, EXTENT_UPTODATE,
-				  cached_state, GFP_NOFS, NULL);
+				  cached_state, NULL);
 }
 
 static inline int clear_extent_dirty(struct extent_io_tree *tree, u64 start,
diff --git a/fs/btrfs/extent-tree.c b/fs/btrfs/extent-tree.c
index 6e319100e3a3..71aaf28fafc9 100644
--- a/fs/btrfs/extent-tree.c
+++ b/fs/btrfs/extent-tree.c
@@ -74,7 +74,7 @@ int btrfs_add_excluded_extent(struct btrfs_fs_info *fs_info,
 {
 	u64 end = start + num_bytes - 1;
 	set_extent_bit(&fs_info->excluded_extents, start, end,
-		       EXTENT_UPTODATE, NULL, GFP_NOFS);
+		       EXTENT_UPTODATE, NULL);
 	return 0;
 }
 
@@ -2508,7 +2508,7 @@ static int pin_down_extent(struct btrfs_trans_handle *trans,
 	spin_unlock(&cache->space_info->lock);
 
 	set_extent_bit(&trans->transaction->pinned_extents, bytenr,
-		       bytenr + num_bytes - 1, EXTENT_DIRTY, NULL, GFP_NOFS);
+		       bytenr + num_bytes - 1, EXTENT_DIRTY, NULL);
 	return 0;
 }
 
@@ -4831,16 +4831,15 @@ btrfs_init_new_buffer(struct btrfs_trans_handle *trans, struct btrfs_root *root,
 		if (buf->log_index == 0)
 			set_extent_bit(&root->dirty_log_pages, buf->start,
 				       buf->start + buf->len - 1,
-				       EXTENT_DIRTY, NULL, GFP_NOFS);
+				       EXTENT_DIRTY, NULL);
 		else
 			set_extent_bit(&root->dirty_log_pages, buf->start,
 				       buf->start + buf->len - 1,
-				       EXTENT_NEW, NULL, GFP_NOFS);
+				       EXTENT_NEW, NULL);
 	} else {
 		buf->log_index = -1;
 		set_extent_bit(&trans->transaction->dirty_pages, buf->start,
-			       buf->start + buf->len - 1, EXTENT_DIRTY, NULL,
-			       GFP_NOFS);
+			       buf->start + buf->len - 1, EXTENT_DIRTY, NULL);
 	}
 	/* this returns a buffer locked for blocking */
 	return buf;
@@ -5981,8 +5980,7 @@ static int btrfs_trim_free_extents(struct btrfs_device *device, u64 *trimmed)
 					  &bytes);
 		if (!ret)
 			set_extent_bit(&device->alloc_state, start,
-				       start + bytes - 1,
-				       CHUNK_TRIMMED, NULL, GFP_NOFS);
+				       start + bytes - 1, CHUNK_TRIMMED, NULL);
 		mutex_unlock(&fs_info->chunk_mutex);
 
 		if (ret)
diff --git a/fs/btrfs/extent_io.c b/fs/btrfs/extent_io.c
index 4e592df58df4..fcd0563f7a15 100644
--- a/fs/btrfs/extent_io.c
+++ b/fs/btrfs/extent_io.c
@@ -2452,8 +2452,7 @@ static int try_release_extent_state(struct extent_io_tree *tree,
 		 * The delalloc new bit will be cleared by ordered extent
 		 * completion.
 		 */
-		ret = __clear_extent_bit(tree, start, end, clear_bits, NULL,
-					 mask, NULL);
+		ret = __clear_extent_bit(tree, start, end, clear_bits, NULL, NULL);
 
 		/* if clear_extent_bit failed for enomem reasons,
 		 * we can't allow the release to continue.
diff --git a/fs/btrfs/extent_map.c b/fs/btrfs/extent_map.c
index 4c8c87524d62..4d86d4739217 100644
--- a/fs/btrfs/extent_map.c
+++ b/fs/btrfs/extent_map.c
@@ -366,7 +366,7 @@ static void extent_map_device_set_bits(struct extent_map *em, unsigned bits)
 
 		set_extent_bit(&device->alloc_state, stripe->physical,
 			       stripe->physical + stripe_size - 1,
-			       bits | EXTENT_NOWAIT, NULL, GFP_NOWAIT);
+			       bits | EXTENT_NOWAIT, NULL);
 	}
 }
 
@@ -383,7 +383,7 @@ static void extent_map_device_clear_bits(struct extent_map *em, unsigned bits)
 		__clear_extent_bit(&device->alloc_state, stripe->physical,
 				   stripe->physical + stripe_size - 1,
 				   bits | EXTENT_NOWAIT,
-				   NULL, GFP_NOWAIT, NULL);
+				   NULL, NULL);
 	}
 }
 
diff --git a/fs/btrfs/file-item.c b/fs/btrfs/file-item.c
index 1e364a7b74aa..594b69d94241 100644
--- a/fs/btrfs/file-item.c
+++ b/fs/btrfs/file-item.c
@@ -95,7 +95,7 @@ int btrfs_inode_set_file_extent_range(struct btrfs_inode *inode, u64 start,
 	if (btrfs_fs_incompat(inode->root->fs_info, NO_HOLES))
 		return 0;
 	return set_extent_bit(&inode->file_extent_tree, start, start + len - 1,
-			      EXTENT_DIRTY, NULL, GFP_NOFS);
+			      EXTENT_DIRTY, NULL);
 }
 
 /*
@@ -440,7 +440,7 @@ blk_status_t btrfs_lookup_bio_sums(struct btrfs_bio *bbio)
 
 				set_extent_bit(&inode->io_tree, file_offset,
 					       file_offset + sectorsize - 1,
-					       EXTENT_NODATASUM, NULL, GFP_NOFS);
+					       EXTENT_NODATASUM, NULL);
 			} else {
 				btrfs_warn_rl(fs_info,
 			"csum hole found for disk bytenr range [%llu, %llu)",
diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c
index 6144a2b89db2..d730d883d41a 100644
--- a/fs/btrfs/inode.c
+++ b/fs/btrfs/inode.c
@@ -2888,8 +2888,7 @@ static int btrfs_find_new_delalloc_bytes(struct btrfs_inode *inode,
 
 		ret = set_extent_bit(&inode->io_tree, search_start,
 				     search_start + em_len - 1,
-				     EXTENT_DELALLOC_NEW, cached_state,
-				     GFP_NOFS);
+				     EXTENT_DELALLOC_NEW, cached_state);
 next:
 		search_start = extent_map_end(em);
 		free_extent_map(em);
@@ -2923,7 +2922,7 @@ int btrfs_set_extent_delalloc(struct btrfs_inode *inode, u64 start, u64 end,
 	}
 
 	return set_extent_bit(&inode->io_tree, start, end,
-			      EXTENT_DELALLOC | extra_bits, cached_state, GFP_NOFS);
+			      EXTENT_DELALLOC | extra_bits, cached_state);
 }
 
 /* see btrfs_writepage_start_hook for details on why this is required */
@@ -5000,7 +4999,7 @@ int btrfs_truncate_block(struct btrfs_inode *inode, loff_t from, loff_t len,
 
 	if (only_release_metadata)
 		set_extent_bit(&inode->io_tree, block_start, block_end,
-			       EXTENT_NORESERVE, NULL, GFP_NOFS);
+			       EXTENT_NORESERVE, NULL);
 
 out_unlock:
 	if (ret) {
diff --git a/fs/btrfs/relocation.c b/fs/btrfs/relocation.c
index 1ed8b132fe2a..0bda67ad349e 100644
--- a/fs/btrfs/relocation.c
+++ b/fs/btrfs/relocation.c
@@ -175,8 +175,7 @@ static void mark_block_processed(struct reloc_control *rc,
 		     rc->block_group->length)) {
 		blocksize = rc->extent_root->fs_info->nodesize;
 		set_extent_bit(&rc->processed_blocks, node->bytenr,
-			       node->bytenr + blocksize - 1, EXTENT_DIRTY,
-			       NULL, GFP_NOFS);
+			       node->bytenr + blocksize - 1, EXTENT_DIRTY, NULL);
 	}
 	node->processed = 1;
 }
@@ -3054,7 +3053,7 @@ static int relocate_one_page(struct inode *inode, struct file_ra_state *ra,
 
 			set_extent_bit(&BTRFS_I(inode)->io_tree,
 				       boundary_start, boundary_end,
-				       EXTENT_BOUNDARY, NULL, GFP_NOFS);
+				       EXTENT_BOUNDARY, NULL);
 		}
 		unlock_extent(&BTRFS_I(inode)->io_tree, clamped_start, clamped_end,
 			      &cached_state);
diff --git a/fs/btrfs/tests/extent-io-tests.c b/fs/btrfs/tests/extent-io-tests.c
index acaaddf7181a..f6bc6d738555 100644
--- a/fs/btrfs/tests/extent-io-tests.c
+++ b/fs/btrfs/tests/extent-io-tests.c
@@ -159,7 +159,7 @@ static int test_find_delalloc(u32 sectorsize)
 	 * |--- delalloc ---|
 	 * |---  search  ---|
 	 */
-	set_extent_bit(tmp, 0, sectorsize - 1, EXTENT_DELALLOC, NULL, GFP_NOFS);
+	set_extent_bit(tmp, 0, sectorsize - 1, EXTENT_DELALLOC, NULL);
 	start = 0;
 	end = start + PAGE_SIZE - 1;
 	found = find_lock_delalloc_range(inode, locked_page, &start,
@@ -190,7 +190,7 @@ static int test_find_delalloc(u32 sectorsize)
 		test_err("couldn't find the locked page");
 		goto out_bits;
 	}
-	set_extent_bit(tmp, sectorsize, max_bytes - 1, EXTENT_DELALLOC, NULL, GFP_NOFS);
+	set_extent_bit(tmp, sectorsize, max_bytes - 1, EXTENT_DELALLOC, NULL);
 	start = test_start;
 	end = start + PAGE_SIZE - 1;
 	found = find_lock_delalloc_range(inode, locked_page, &start,
@@ -245,7 +245,7 @@ static int test_find_delalloc(u32 sectorsize)
 	 *
 	 * We are re-using our test_start from above since it works out well.
 	 */
-	set_extent_bit(tmp, max_bytes, total_dirty - 1, EXTENT_DELALLOC, NULL, GFP_NOFS);
+	set_extent_bit(tmp, max_bytes, total_dirty - 1, EXTENT_DELALLOC, NULL);
 	start = test_start;
 	end = start + PAGE_SIZE - 1;
 	found = find_lock_delalloc_range(inode, locked_page, &start,
@@ -504,7 +504,7 @@ static int test_find_first_clear_extent_bit(void)
 	 * 4M-32M
 	 */
 	set_extent_bit(&tree, SZ_1M, SZ_4M - 1,
-		       CHUNK_TRIMMED | CHUNK_ALLOCATED, NULL, GFP_NOFS);
+		       CHUNK_TRIMMED | CHUNK_ALLOCATED, NULL);
 
 	find_first_clear_extent_bit(&tree, SZ_512K, &start, &end,
 				    CHUNK_TRIMMED | CHUNK_ALLOCATED);
@@ -517,7 +517,7 @@ static int test_find_first_clear_extent_bit(void)
 
 	/* Now add 32M-64M so that we have a hole between 4M-32M */
 	set_extent_bit(&tree, SZ_32M, SZ_64M - 1,
-		       CHUNK_TRIMMED | CHUNK_ALLOCATED, NULL, GFP_NOFS);
+		       CHUNK_TRIMMED | CHUNK_ALLOCATED, NULL);
 
 	/*
 	 * Request first hole starting at 12M, we should get 4M-32M
@@ -548,8 +548,7 @@ static int test_find_first_clear_extent_bit(void)
 	 * Set 64M-72M with CHUNK_ALLOC flag, then search for CHUNK_TRIMMED flag
 	 * being unset in this range, we should get the entry in range 64M-72M
 	 */
-	set_extent_bit(&tree, SZ_64M, SZ_64M + SZ_8M - 1, CHUNK_ALLOCATED, NULL,
-		       GFP_NOFS);
+	set_extent_bit(&tree, SZ_64M, SZ_64M + SZ_8M - 1, CHUNK_ALLOCATED, NULL);
 	find_first_clear_extent_bit(&tree, SZ_64M + SZ_1M, &start, &end,
 				    CHUNK_TRIMMED);
 
diff --git a/fs/btrfs/zoned.c b/fs/btrfs/zoned.c
index fb90e2b20614..98d6b8cc3874 100644
--- a/fs/btrfs/zoned.c
+++ b/fs/btrfs/zoned.c
@@ -1612,7 +1612,7 @@ void btrfs_redirty_list_add(struct btrfs_transaction *trans,
 	set_bit(EXTENT_BUFFER_NO_CHECK, &eb->bflags);
 	set_extent_buffer_dirty(eb);
 	set_extent_bit(&trans->dirty_pages, eb->start, eb->start + eb->len - 1,
-			EXTENT_DIRTY | EXTENT_NOWAIT, NULL, GFP_NOWAIT);
+			EXTENT_DIRTY | EXTENT_NOWAIT, NULL);
 }
 
 bool btrfs_use_zone_append(struct btrfs_bio *bbio)
-- 
2.40.0

Re: [PATCH 1/9] btrfs: open code set_extent_defrag

[Christoph Hellwig]

Looks good:

Reviewed-by: Christoph Hellwig <hch@lst.de>

Re: [PATCH 2/9] btrfs: open code set_extent_delalloc

[Christoph Hellwig]

Looks good:

Reviewed-by: Christoph Hellwig <hch@lst.de>

Re: [PATCH 3/9] btrfs: open code set_extent_new

[Christoph Hellwig]

Looks good:

Reviewed-by: Christoph Hellwig <hch@lst.de>

Re: [PATCH 4/9] btrfs: open code set_extent_dirty

[Christoph Hellwig]

Looks good:

Reviewed-by: Christoph Hellwig <hch@lst.de>

Re: [PATCH 5/9] btrfs: open code set_extent_bits_nowait

[Christoph Hellwig]

The change itself looks ok to me:

Reviewed-by: Christoph Hellwig <hch@lst.de>

.. but:

> diff --git a/fs/btrfs/zoned.c b/fs/btrfs/zoned.c
> index bda301a55cbe..b82a350c4c59 100644
> --- a/fs/btrfs/zoned.c
> +++ b/fs/btrfs/zoned.c
> @@ -1611,8 +1611,8 @@ void btrfs_redirty_list_add(struct btrfs_transaction *trans,
>  	memzero_extent_buffer(eb, 0, eb->len);
>  	set_bit(EXTENT_BUFFER_NO_CHECK, &eb->bflags);
>  	set_extent_buffer_dirty(eb);
> -	set_extent_bits_nowait(&trans->dirty_pages, eb->start,
> -			       eb->start + eb->len - 1, EXTENT_DIRTY);
> +	set_extent_bit(&trans->dirty_pages, eb->start, eb->start + eb->len - 1,
> +			EXTENT_DIRTY, NULL, GFP_NOWAIT);

.. there is no point in even using GFP_NOWAIT here, as we are always
called in a context that can sleep, set_extent_buffer_dirty relies on
that as well as it calls lock_page.

Re: [PATCH 6/9] btrfs: open code set_extent_bits

[Christoph Hellwig]

On Thu, May 25, 2023 at 01:04:32AM +0200, David Sterba wrote:
> This helper calls set_extent_bit with two more parameters set to default
> values, but otherwise it's purpose is not clear.

... and the naming is confusing as f*%$ given that it doesn't set any
more or less bits thatn set_extent_bit.  Good riddance.

Reviewed-by: Christoph Hellwig <hch@lst.de>

Re: [PATCH 7/9] btrfs: drop NOFAIL from set_extent_bit allocation masks

[Christoph Hellwig]

On Thu, May 25, 2023 at 01:04:34AM +0200, David Sterba wrote:
> The __GFP_NOFAIL passed to set_extent_bit first appeared in 2010
> (commit f0486c68e4bd9a ("Btrfs: Introduce contexts for metadata
> reservation")), without any explanation why it would be needed.
> 
> Meanwhile we've updated the semantics of set_extent_bit to handle failed
> allocations and do unlock, sleep and retry if needed.  The use of the
> NOFAIL flag is also an outlier, we never want any of the set/clear
> extent bit helpers to fail, they're used for many critical changes like
> extent locking, besides the extent state bit changes.

Given how many of the callers do not check the return value, and that
the trees store essential information, I think the right thing here
is to always use __GFP_NOFAIL unless GFP_NOWAIT is passed.  In practice
this won't make a difference as currently small slab allocations never
fail, but that's an undocumented assumption.

Re: [PATCH 0/9] Parameter cleanups in extent state helpers

[Qu Wenruo]

On 2023/5/25 07:04, David Sterba wrote:
> This is motivated by the gfp parameter passed to all the helpers and how
> to get rid of it. Most of the values are GFP_NOFS with some exceptions.
> One of them (GFP_NOFAIL) can be removed the other one (GFP_NOWAIT) is
> transformed to existing parameters.

The patchset gives me some inspiration on a wild idea to completely
remove the memory allocation inside set/clear_bits().

The core idea is to change extent io tree to have "hole" extent_state,
so that every set/clear_bits() call will only need to allocate 2 new
extent states, thus we can pre-allocate them.


Currently the extent io tree behaves much like no-hole, we only insert
new extent_state if there is any bit set for a certain range.
Thus the following case will need as many new extent states as the holes:

           |<------- Range to set bits ------------->|
           |///|   |////|    |////|   ....     |/////|
                 ^         ^                 ^
                 |         |                 \- hole N
                 hole 1    hole 2

But if we keep hole state extents, the above case will only need to
allocate 0 new extent state:

           |<------- Range to set bits ------------->|
           |///|000|////|0000|////|   .... 0000|/////|

Although we may still need to allocate up to 2 new extent states even
with hole states.
The most straightforward example is:

                |<- Range to clear bits ---->|
           |//////////////////////////////////////////|


But this greatly limits the new allocation needed, thus can make the
set/clear_bits() to return void and avoid memory allocation at all (all
pre-allocated by callers).

Obviously there would be no free lunch, the overall memory usage would
increase, especially for fragmented extent io tree.

Thus I'm not sure if the hole extent state idea is really a good idea or
the existing NOWAIT/NOFAIL allocation is more acceptable...

Thanks,
Qu
>
> Module code size is lower for the whole series and stack is reduced in
> about 50 functions by 8 bytes.
>
> David Sterba (9):
>    btrfs: open code set_extent_defrag
>    btrfs: open code set_extent_delalloc
>    btrfs: open code set_extent_new
>    btrfs: open code set_extent_dirty
>    btrfs: open code set_extent_bits_nowait
>    btrfs: open code set_extent_bits
>    btrfs: drop NOFAIL from set_extent_bit allocation masks
>    btrfs: pass NOWAIT for set/clear extent bits as another bit
>    btrfs: drop gfp from parameter extent state helpers
>
>   fs/btrfs/block-group.c           |  6 ++--
>   fs/btrfs/defrag.c                |  3 +-
>   fs/btrfs/dev-replace.c           |  4 +--
>   fs/btrfs/extent-io-tree.c        | 37 ++++++++++++-------
>   fs/btrfs/extent-io-tree.h        | 62 ++++++++------------------------
>   fs/btrfs/extent-tree.c           | 27 +++++++-------
>   fs/btrfs/extent_io.c             |  3 +-
>   fs/btrfs/extent_map.c            | 10 +++---
>   fs/btrfs/file-item.c             | 10 +++---
>   fs/btrfs/inode.c                 |  9 +++--
>   fs/btrfs/relocation.c            | 10 +++---
>   fs/btrfs/tests/extent-io-tests.c | 16 ++++-----
>   fs/btrfs/zoned.c                 |  4 +--
>   13 files changed, 91 insertions(+), 110 deletions(-)
>

Re: [PATCH 1/9] btrfs: open code set_extent_defrag

[Qu Wenruo]

On 2023/5/25 07:04, David Sterba wrote:
> The helper is used only once.
>
> Signed-off-by: David Sterba <dsterba@suse.com>

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

Thanks,
Qu
> ---
>   fs/btrfs/defrag.c         | 4 +++-
>   fs/btrfs/extent-io-tree.h | 8 --------
>   2 files changed, 3 insertions(+), 9 deletions(-)
>
> diff --git a/fs/btrfs/defrag.c b/fs/btrfs/defrag.c
> index 8065341d831a..4e7a1e0a0441 100644
> --- a/fs/btrfs/defrag.c
> +++ b/fs/btrfs/defrag.c
> @@ -1040,7 +1040,9 @@ static int defrag_one_locked_target(struct btrfs_inode *inode,
>   	clear_extent_bit(&inode->io_tree, start, start + len - 1,
>   			 EXTENT_DELALLOC | EXTENT_DO_ACCOUNTING |
>   			 EXTENT_DEFRAG, cached_state);
> -	set_extent_defrag(&inode->io_tree, start, start + len - 1, cached_state);
> +	set_extent_bit(&inode->io_tree, start, start + len - 1,
> +		       EXTENT_DELALLOC | EXTENT_DEFRAG,
> +		       cached_state, GFP_NOFS);
>
>   	/* Update the page status */
>   	for (i = start_index - first_index; i <= last_index - first_index; i++) {
> diff --git a/fs/btrfs/extent-io-tree.h b/fs/btrfs/extent-io-tree.h
> index 21766e49ec02..ea344e5ca24f 100644
> --- a/fs/btrfs/extent-io-tree.h
> +++ b/fs/btrfs/extent-io-tree.h
> @@ -202,14 +202,6 @@ static inline int set_extent_delalloc(struct extent_io_tree *tree, u64 start,
>   			      cached_state, GFP_NOFS);
>   }
>
> -static inline int set_extent_defrag(struct extent_io_tree *tree, u64 start,
> -		u64 end, struct extent_state **cached_state)
> -{
> -	return set_extent_bit(tree, start, end,
> -			      EXTENT_DELALLOC | EXTENT_DEFRAG,
> -			      cached_state, GFP_NOFS);
> -}
> -
>   static inline int set_extent_new(struct extent_io_tree *tree, u64 start,
>   		u64 end)
>   {

Re: [PATCH 2/9] btrfs: open code set_extent_delalloc

[Qu Wenruo]

On 2023/5/25 07:04, David Sterba wrote:
> The helper is used once in fs code and a few times in the self test
> code.
>
> Signed-off-by: David Sterba <dsterba@suse.com>

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

Thanks,
Qu
> ---
>   fs/btrfs/extent-io-tree.h        | 9 ---------
>   fs/btrfs/inode.c                 | 4 ++--
>   fs/btrfs/tests/extent-io-tests.c | 6 +++---
>   3 files changed, 5 insertions(+), 14 deletions(-)
>
> diff --git a/fs/btrfs/extent-io-tree.h b/fs/btrfs/extent-io-tree.h
> index ea344e5ca24f..e5289d67b6b7 100644
> --- a/fs/btrfs/extent-io-tree.h
> +++ b/fs/btrfs/extent-io-tree.h
> @@ -193,15 +193,6 @@ int convert_extent_bit(struct extent_io_tree *tree, u64 start, u64 end,
>   		       u32 bits, u32 clear_bits,
>   		       struct extent_state **cached_state);
>
> -static inline int set_extent_delalloc(struct extent_io_tree *tree, u64 start,
> -				      u64 end, u32 extra_bits,
> -				      struct extent_state **cached_state)
> -{
> -	return set_extent_bit(tree, start, end,
> -			      EXTENT_DELALLOC | extra_bits,
> -			      cached_state, GFP_NOFS);
> -}
> -
>   static inline int set_extent_new(struct extent_io_tree *tree, u64 start,
>   		u64 end)
>   {
> diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c
> index 2e83fb225052..6144a2b89db2 100644
> --- a/fs/btrfs/inode.c
> +++ b/fs/btrfs/inode.c
> @@ -2922,8 +2922,8 @@ int btrfs_set_extent_delalloc(struct btrfs_inode *inode, u64 start, u64 end,
>   			return ret;
>   	}
>
> -	return set_extent_delalloc(&inode->io_tree, start, end, extra_bits,
> -				   cached_state);
> +	return set_extent_bit(&inode->io_tree, start, end,
> +			      EXTENT_DELALLOC | extra_bits, cached_state, GFP_NOFS);
>   }
>
>   /* see btrfs_writepage_start_hook for details on why this is required */
> diff --git a/fs/btrfs/tests/extent-io-tests.c b/fs/btrfs/tests/extent-io-tests.c
> index dfc5c7fa6038..b9de94a50868 100644
> --- a/fs/btrfs/tests/extent-io-tests.c
> +++ b/fs/btrfs/tests/extent-io-tests.c
> @@ -159,7 +159,7 @@ static int test_find_delalloc(u32 sectorsize)
>   	 * |--- delalloc ---|
>   	 * |---  search  ---|
>   	 */
> -	set_extent_delalloc(tmp, 0, sectorsize - 1, 0, NULL);
> +	set_extent_bit(tmp, 0, sectorsize - 1, EXTENT_DELALLOC, NULL, GFP_NOFS);
>   	start = 0;
>   	end = start + PAGE_SIZE - 1;
>   	found = find_lock_delalloc_range(inode, locked_page, &start,
> @@ -190,7 +190,7 @@ static int test_find_delalloc(u32 sectorsize)
>   		test_err("couldn't find the locked page");
>   		goto out_bits;
>   	}
> -	set_extent_delalloc(tmp, sectorsize, max_bytes - 1, 0, NULL);
> +	set_extent_bit(tmp, sectorsize, max_bytes - 1, EXTENT_DELALLOC, NULL, GFP_NOFS);
>   	start = test_start;
>   	end = start + PAGE_SIZE - 1;
>   	found = find_lock_delalloc_range(inode, locked_page, &start,
> @@ -245,7 +245,7 @@ static int test_find_delalloc(u32 sectorsize)
>   	 *
>   	 * We are re-using our test_start from above since it works out well.
>   	 */
> -	set_extent_delalloc(tmp, max_bytes, total_dirty - 1, 0, NULL);
> +	set_extent_bit(tmp, max_bytes, total_dirty - 1, EXTENT_DELALLOC, NULL, GFP_NOFS);
>   	start = test_start;
>   	end = start + PAGE_SIZE - 1;
>   	found = find_lock_delalloc_range(inode, locked_page, &start,

Re: [PATCH 3/9] btrfs: open code set_extent_new

[Qu Wenruo]

On 2023/5/25 07:04, David Sterba wrote:
> The helper is used only once.
>
> Signed-off-by: David Sterba <dsterba@suse.com>

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

Thanks,
Qu
> ---
>   fs/btrfs/extent-io-tree.h | 6 ------
>   fs/btrfs/extent-tree.c    | 5 +++--
>   2 files changed, 3 insertions(+), 8 deletions(-)
>
> diff --git a/fs/btrfs/extent-io-tree.h b/fs/btrfs/extent-io-tree.h
> index e5289d67b6b7..293e49377104 100644
> --- a/fs/btrfs/extent-io-tree.h
> +++ b/fs/btrfs/extent-io-tree.h
> @@ -193,12 +193,6 @@ int convert_extent_bit(struct extent_io_tree *tree, u64 start, u64 end,
>   		       u32 bits, u32 clear_bits,
>   		       struct extent_state **cached_state);
>
> -static inline int set_extent_new(struct extent_io_tree *tree, u64 start,
> -		u64 end)
> -{
> -	return set_extent_bit(tree, start, end, EXTENT_NEW, NULL, GFP_NOFS);
> -}
> -
>   int find_first_extent_bit(struct extent_io_tree *tree, u64 start,
>   			  u64 *start_ret, u64 *end_ret, u32 bits,
>   			  struct extent_state **cached_state);
> diff --git a/fs/btrfs/extent-tree.c b/fs/btrfs/extent-tree.c
> index 5de5b577e7fd..5c7c72042193 100644
> --- a/fs/btrfs/extent-tree.c
> +++ b/fs/btrfs/extent-tree.c
> @@ -4832,8 +4832,9 @@ btrfs_init_new_buffer(struct btrfs_trans_handle *trans, struct btrfs_root *root,
>   			set_extent_dirty(&root->dirty_log_pages, buf->start,
>   					buf->start + buf->len - 1, GFP_NOFS);
>   		else
> -			set_extent_new(&root->dirty_log_pages, buf->start,
> -					buf->start + buf->len - 1);
> +			set_extent_bit(&root->dirty_log_pages, buf->start,
> +				       buf->start + buf->len - 1,
> +				       EXTENT_NEW, NULL, GFP_NOFS);
>   	} else {
>   		buf->log_index = -1;
>   		set_extent_dirty(&trans->transaction->dirty_pages, buf->start,

Re: [PATCH 4/9] btrfs: open code set_extent_dirty

[Qu Wenruo]

On 2023/5/25 07:04, David Sterba wrote:
> The helper is used a few times, that it's setting the DIRTY extent bit
> is still clear.
>
> Signed-off-by: David Sterba <dsterba@suse.com>

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

Thanks,
Qu
> ---
>   fs/btrfs/block-group.c    |  7 ++++---
>   fs/btrfs/extent-io-tree.h |  6 ------
>   fs/btrfs/extent-tree.c    | 15 +++++++++------
>   3 files changed, 13 insertions(+), 15 deletions(-)
>
> diff --git a/fs/btrfs/block-group.c b/fs/btrfs/block-group.c
> index 590b03560265..ec463f8d83ec 100644
> --- a/fs/btrfs/block-group.c
> +++ b/fs/btrfs/block-group.c
> @@ -3521,9 +3521,10 @@ int btrfs_update_block_group(struct btrfs_trans_handle *trans,
>   			spin_unlock(&cache->lock);
>   			spin_unlock(&space_info->lock);
>
> -			set_extent_dirty(&trans->transaction->pinned_extents,
> -					 bytenr, bytenr + num_bytes - 1,
> -					 GFP_NOFS | __GFP_NOFAIL);
> +			set_extent_bit(&trans->transaction->pinned_extents,
> +				       bytenr, bytenr + num_bytes - 1,
> +				       EXTENT_DIRTY, NULL,
> +				       GFP_NOFS | __GFP_NOFAIL);
>   		}
>
>   		spin_lock(&trans->transaction->dirty_bgs_lock);
> diff --git a/fs/btrfs/extent-io-tree.h b/fs/btrfs/extent-io-tree.h
> index 293e49377104..0fc54546a63d 100644
> --- a/fs/btrfs/extent-io-tree.h
> +++ b/fs/btrfs/extent-io-tree.h
> @@ -175,12 +175,6 @@ static inline int clear_extent_uptodate(struct extent_io_tree *tree, u64 start,
>   				  cached_state, GFP_NOFS, NULL);
>   }
>
> -static inline int set_extent_dirty(struct extent_io_tree *tree, u64 start,
> -		u64 end, gfp_t mask)
> -{
> -	return set_extent_bit(tree, start, end, EXTENT_DIRTY, NULL, mask);
> -}
> -
>   static inline int clear_extent_dirty(struct extent_io_tree *tree, u64 start,
>   				     u64 end, struct extent_state **cached)
>   {
> diff --git a/fs/btrfs/extent-tree.c b/fs/btrfs/extent-tree.c
> index 5c7c72042193..47cfb694cdbf 100644
> --- a/fs/btrfs/extent-tree.c
> +++ b/fs/btrfs/extent-tree.c
> @@ -2507,8 +2507,9 @@ static int pin_down_extent(struct btrfs_trans_handle *trans,
>   	spin_unlock(&cache->lock);
>   	spin_unlock(&cache->space_info->lock);
>
> -	set_extent_dirty(&trans->transaction->pinned_extents, bytenr,
> -			 bytenr + num_bytes - 1, GFP_NOFS | __GFP_NOFAIL);
> +	set_extent_bit(&trans->transaction->pinned_extents, bytenr,
> +		       bytenr + num_bytes - 1, EXTENT_DIRTY, NULL,
> +		       GFP_NOFS | __GFP_NOFAIL);
>   	return 0;
>   }
>
> @@ -4829,16 +4830,18 @@ btrfs_init_new_buffer(struct btrfs_trans_handle *trans, struct btrfs_root *root,
>   		 * EXTENT bit to differentiate dirty pages.
>   		 */
>   		if (buf->log_index == 0)
> -			set_extent_dirty(&root->dirty_log_pages, buf->start,
> -					buf->start + buf->len - 1, GFP_NOFS);
> +			set_extent_bit(&root->dirty_log_pages, buf->start,
> +				       buf->start + buf->len - 1,
> +				       EXTENT_DIRTY, NULL, GFP_NOFS);
>   		else
>   			set_extent_bit(&root->dirty_log_pages, buf->start,
>   				       buf->start + buf->len - 1,
>   				       EXTENT_NEW, NULL, GFP_NOFS);
>   	} else {
>   		buf->log_index = -1;
> -		set_extent_dirty(&trans->transaction->dirty_pages, buf->start,
> -			 buf->start + buf->len - 1, GFP_NOFS);
> +		set_extent_bit(&trans->transaction->dirty_pages, buf->start,
> +			       buf->start + buf->len - 1, EXTENT_DIRTY, NULL,
> +			       GFP_NOFS);
>   	}
>   	/* this returns a buffer locked for blocking */
>   	return buf;

Re: [PATCH 5/9] btrfs: open code set_extent_bits_nowait

[Qu Wenruo]

On 2023/5/25 07:04, David Sterba wrote:
> The helper only passes GFP_NOWAIT as gfp flags and is used two times.
>
> Signed-off-by: David Sterba <dsterba@suse.com>

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

Thanks,
Qu
> ---
>   fs/btrfs/extent-io-tree.h | 6 ------
>   fs/btrfs/extent_map.c     | 5 +++--
>   fs/btrfs/zoned.c          | 4 ++--
>   3 files changed, 5 insertions(+), 10 deletions(-)
>
> diff --git a/fs/btrfs/extent-io-tree.h b/fs/btrfs/extent-io-tree.h
> index 0fc54546a63d..ef9d54cdee5c 100644
> --- a/fs/btrfs/extent-io-tree.h
> +++ b/fs/btrfs/extent-io-tree.h
> @@ -156,12 +156,6 @@ int set_record_extent_bits(struct extent_io_tree *tree, u64 start, u64 end,
>   int set_extent_bit(struct extent_io_tree *tree, u64 start, u64 end,
>   		   u32 bits, struct extent_state **cached_state, gfp_t mask);
>
> -static inline int set_extent_bits_nowait(struct extent_io_tree *tree, u64 start,
> -					 u64 end, u32 bits)
> -{
> -	return set_extent_bit(tree, start, end, bits, NULL, GFP_NOWAIT);
> -}
> -
>   static inline int set_extent_bits(struct extent_io_tree *tree, u64 start,
>   		u64 end, u32 bits)
>   {
> diff --git a/fs/btrfs/extent_map.c b/fs/btrfs/extent_map.c
> index 138afa955370..918ce12ea412 100644
> --- a/fs/btrfs/extent_map.c
> +++ b/fs/btrfs/extent_map.c
> @@ -364,8 +364,9 @@ static void extent_map_device_set_bits(struct extent_map *em, unsigned bits)
>   		struct btrfs_io_stripe *stripe = &map->stripes[i];
>   		struct btrfs_device *device = stripe->dev;
>
> -		set_extent_bits_nowait(&device->alloc_state, stripe->physical,
> -				 stripe->physical + stripe_size - 1, bits);
> +		set_extent_bit(&device->alloc_state, stripe->physical,
> +			       stripe->physical + stripe_size - 1, bits, NULL,
> +			       GFP_NOWAIT);
>   	}
>   }
>
> diff --git a/fs/btrfs/zoned.c b/fs/btrfs/zoned.c
> index bda301a55cbe..b82a350c4c59 100644
> --- a/fs/btrfs/zoned.c
> +++ b/fs/btrfs/zoned.c
> @@ -1611,8 +1611,8 @@ void btrfs_redirty_list_add(struct btrfs_transaction *trans,
>   	memzero_extent_buffer(eb, 0, eb->len);
>   	set_bit(EXTENT_BUFFER_NO_CHECK, &eb->bflags);
>   	set_extent_buffer_dirty(eb);
> -	set_extent_bits_nowait(&trans->dirty_pages, eb->start,
> -			       eb->start + eb->len - 1, EXTENT_DIRTY);
> +	set_extent_bit(&trans->dirty_pages, eb->start, eb->start + eb->len - 1,
> +			EXTENT_DIRTY, NULL, GFP_NOWAIT);
>   }
>
>   bool btrfs_use_zone_append(struct btrfs_bio *bbio)

Re: [PATCH 6/9] btrfs: open code set_extent_bits

[Qu Wenruo]

On 2023/5/25 07:04, David Sterba wrote:
> This helper calls set_extent_bit with two more parameters set to default
> values, but otherwise it's purpose is not clear.
>
> Signed-off-by: David Sterba <dsterba@suse.com>

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

Thanks,
Qu
> ---
>   fs/btrfs/dev-replace.c           |  4 ++--
>   fs/btrfs/extent-io-tree.h        |  6 ------
>   fs/btrfs/extent-tree.c           | 10 +++++-----
>   fs/btrfs/file-item.c             | 10 +++++-----
>   fs/btrfs/relocation.c            | 11 ++++++-----
>   fs/btrfs/tests/extent-io-tests.c | 11 ++++++-----
>   6 files changed, 24 insertions(+), 28 deletions(-)
>
> diff --git a/fs/btrfs/dev-replace.c b/fs/btrfs/dev-replace.c
> index 78696d331639..3a0fc57d5db9 100644
> --- a/fs/btrfs/dev-replace.c
> +++ b/fs/btrfs/dev-replace.c
> @@ -795,8 +795,8 @@ static int btrfs_set_target_alloc_state(struct btrfs_device *srcdev,
>   	while (!find_first_extent_bit(&srcdev->alloc_state, start,
>   				      &found_start, &found_end,
>   				      CHUNK_ALLOCATED, &cached_state)) {
> -		ret = set_extent_bits(&tgtdev->alloc_state, found_start,
> -				      found_end, CHUNK_ALLOCATED);
> +		ret = set_extent_bit(&tgtdev->alloc_state, found_start,
> +				     found_end, CHUNK_ALLOCATED, NULL, GFP_NOFS);
>   		if (ret)
>   			break;
>   		start = found_end + 1;
> diff --git a/fs/btrfs/extent-io-tree.h b/fs/btrfs/extent-io-tree.h
> index ef9d54cdee5c..5a53a4558366 100644
> --- a/fs/btrfs/extent-io-tree.h
> +++ b/fs/btrfs/extent-io-tree.h
> @@ -156,12 +156,6 @@ int set_record_extent_bits(struct extent_io_tree *tree, u64 start, u64 end,
>   int set_extent_bit(struct extent_io_tree *tree, u64 start, u64 end,
>   		   u32 bits, struct extent_state **cached_state, gfp_t mask);
>
> -static inline int set_extent_bits(struct extent_io_tree *tree, u64 start,
> -		u64 end, u32 bits)
> -{
> -	return set_extent_bit(tree, start, end, bits, NULL, GFP_NOFS);
> -}
> -
>   static inline int clear_extent_uptodate(struct extent_io_tree *tree, u64 start,
>   		u64 end, struct extent_state **cached_state)
>   {
> diff --git a/fs/btrfs/extent-tree.c b/fs/btrfs/extent-tree.c
> index 47cfb694cdbf..03b2a7c508b9 100644
> --- a/fs/btrfs/extent-tree.c
> +++ b/fs/btrfs/extent-tree.c
> @@ -73,8 +73,8 @@ int btrfs_add_excluded_extent(struct btrfs_fs_info *fs_info,
>   			      u64 start, u64 num_bytes)
>   {
>   	u64 end = start + num_bytes - 1;
> -	set_extent_bits(&fs_info->excluded_extents, start, end,
> -			EXTENT_UPTODATE);
> +	set_extent_bit(&fs_info->excluded_extents, start, end,
> +		       EXTENT_UPTODATE, NULL, GFP_NOFS);
>   	return 0;
>   }
>
> @@ -5981,9 +5981,9 @@ static int btrfs_trim_free_extents(struct btrfs_device *device, u64 *trimmed)
>   		ret = btrfs_issue_discard(device->bdev, start, len,
>   					  &bytes);
>   		if (!ret)
> -			set_extent_bits(&device->alloc_state, start,
> -					start + bytes - 1,
> -					CHUNK_TRIMMED);
> +			set_extent_bit(&device->alloc_state, start,
> +				       start + bytes - 1,
> +				       CHUNK_TRIMMED, NULL, GFP_NOFS);
>   		mutex_unlock(&fs_info->chunk_mutex);
>
>   		if (ret)
> diff --git a/fs/btrfs/file-item.c b/fs/btrfs/file-item.c
> index e74b9804bcde..1e364a7b74aa 100644
> --- a/fs/btrfs/file-item.c
> +++ b/fs/btrfs/file-item.c
> @@ -94,8 +94,8 @@ int btrfs_inode_set_file_extent_range(struct btrfs_inode *inode, u64 start,
>
>   	if (btrfs_fs_incompat(inode->root->fs_info, NO_HOLES))
>   		return 0;
> -	return set_extent_bits(&inode->file_extent_tree, start, start + len - 1,
> -			       EXTENT_DIRTY);
> +	return set_extent_bit(&inode->file_extent_tree, start, start + len - 1,
> +			      EXTENT_DIRTY, NULL, GFP_NOFS);
>   }
>
>   /*
> @@ -438,9 +438,9 @@ blk_status_t btrfs_lookup_bio_sums(struct btrfs_bio *bbio)
>   			    BTRFS_DATA_RELOC_TREE_OBJECTID) {
>   				u64 file_offset = bbio->file_offset + bio_offset;
>
> -				set_extent_bits(&inode->io_tree, file_offset,
> -						file_offset + sectorsize - 1,
> -						EXTENT_NODATASUM);
> +				set_extent_bit(&inode->io_tree, file_offset,
> +					       file_offset + sectorsize - 1,
> +					       EXTENT_NODATASUM, NULL, GFP_NOFS);
>   			} else {
>   				btrfs_warn_rl(fs_info,
>   			"csum hole found for disk bytenr range [%llu, %llu)",
> diff --git a/fs/btrfs/relocation.c b/fs/btrfs/relocation.c
> index 38cfbd38a819..1ed8b132fe2a 100644
> --- a/fs/btrfs/relocation.c
> +++ b/fs/btrfs/relocation.c
> @@ -174,8 +174,9 @@ static void mark_block_processed(struct reloc_control *rc,
>   	    in_range(node->bytenr, rc->block_group->start,
>   		     rc->block_group->length)) {
>   		blocksize = rc->extent_root->fs_info->nodesize;
> -		set_extent_bits(&rc->processed_blocks, node->bytenr,
> -				node->bytenr + blocksize - 1, EXTENT_DIRTY);
> +		set_extent_bit(&rc->processed_blocks, node->bytenr,
> +			       node->bytenr + blocksize - 1, EXTENT_DIRTY,
> +			       NULL, GFP_NOFS);
>   	}
>   	node->processed = 1;
>   }
> @@ -3051,9 +3052,9 @@ static int relocate_one_page(struct inode *inode, struct file_ra_state *ra,
>   			u64 boundary_end = boundary_start +
>   					   fs_info->sectorsize - 1;
>
> -			set_extent_bits(&BTRFS_I(inode)->io_tree,
> -					boundary_start, boundary_end,
> -					EXTENT_BOUNDARY);
> +			set_extent_bit(&BTRFS_I(inode)->io_tree,
> +				       boundary_start, boundary_end,
> +				       EXTENT_BOUNDARY, NULL, GFP_NOFS);
>   		}
>   		unlock_extent(&BTRFS_I(inode)->io_tree, clamped_start, clamped_end,
>   			      &cached_state);
> diff --git a/fs/btrfs/tests/extent-io-tests.c b/fs/btrfs/tests/extent-io-tests.c
> index b9de94a50868..acaaddf7181a 100644
> --- a/fs/btrfs/tests/extent-io-tests.c
> +++ b/fs/btrfs/tests/extent-io-tests.c
> @@ -503,8 +503,8 @@ static int test_find_first_clear_extent_bit(void)
>   	 * Set 1M-4M alloc/discard and 32M-64M thus leaving a hole between
>   	 * 4M-32M
>   	 */
> -	set_extent_bits(&tree, SZ_1M, SZ_4M - 1,
> -			CHUNK_TRIMMED | CHUNK_ALLOCATED);
> +	set_extent_bit(&tree, SZ_1M, SZ_4M - 1,
> +		       CHUNK_TRIMMED | CHUNK_ALLOCATED, NULL, GFP_NOFS);
>
>   	find_first_clear_extent_bit(&tree, SZ_512K, &start, &end,
>   				    CHUNK_TRIMMED | CHUNK_ALLOCATED);
> @@ -516,8 +516,8 @@ static int test_find_first_clear_extent_bit(void)
>   	}
>
>   	/* Now add 32M-64M so that we have a hole between 4M-32M */
> -	set_extent_bits(&tree, SZ_32M, SZ_64M - 1,
> -			CHUNK_TRIMMED | CHUNK_ALLOCATED);
> +	set_extent_bit(&tree, SZ_32M, SZ_64M - 1,
> +		       CHUNK_TRIMMED | CHUNK_ALLOCATED, NULL, GFP_NOFS);
>
>   	/*
>   	 * Request first hole starting at 12M, we should get 4M-32M
> @@ -548,7 +548,8 @@ static int test_find_first_clear_extent_bit(void)
>   	 * Set 64M-72M with CHUNK_ALLOC flag, then search for CHUNK_TRIMMED flag
>   	 * being unset in this range, we should get the entry in range 64M-72M
>   	 */
> -	set_extent_bits(&tree, SZ_64M, SZ_64M + SZ_8M - 1, CHUNK_ALLOCATED);
> +	set_extent_bit(&tree, SZ_64M, SZ_64M + SZ_8M - 1, CHUNK_ALLOCATED, NULL,
> +		       GFP_NOFS);
>   	find_first_clear_extent_bit(&tree, SZ_64M + SZ_1M, &start, &end,
>   				    CHUNK_TRIMMED);
>

Re: [PATCH 7/9] btrfs: drop NOFAIL from set_extent_bit allocation masks

[Qu Wenruo]

On 2023/5/25 07:04, David Sterba wrote:
> The __GFP_NOFAIL passed to set_extent_bit first appeared in 2010
> (commit f0486c68e4bd9a ("Btrfs: Introduce contexts for metadata
> reservation")), without any explanation why it would be needed.
>
> Meanwhile we've updated the semantics of set_extent_bit to handle failed
> allocations and do unlock, sleep and retry if needed.  The use of the
> NOFAIL flag is also an outlier, we never want any of the set/clear
> extent bit helpers to fail, they're used for many critical changes like
> extent locking, besides the extent state bit changes.

As I mentioned in the cover letter, if we really want to set/clear bits
to not fail, and can accept the extra memory usage (as high as twice the
number of extent states), then we should really consider the following
changes:

- Introduce hole extent_state
   For ranges without any bit set, there still needs to be an
   extent_state.

- Make callers to pre-allocate 2 extent_state and pass them as mandatory
   parameters to set/clear bits

- Make set/clear bits to use the preallocated 2 extent states

By this, we should be able to completely get rid of the memory
allocation inside the extent io tree set/clear calls.

Thanks,
Qu
>
> Signed-off-by: David Sterba <dsterba@suse.com>
> ---
>   fs/btrfs/block-group.c | 3 +--
>   fs/btrfs/extent-tree.c | 3 +--
>   2 files changed, 2 insertions(+), 4 deletions(-)
>
> diff --git a/fs/btrfs/block-group.c b/fs/btrfs/block-group.c
> index ec463f8d83ec..202e2aa949c5 100644
> --- a/fs/btrfs/block-group.c
> +++ b/fs/btrfs/block-group.c
> @@ -3523,8 +3523,7 @@ int btrfs_update_block_group(struct btrfs_trans_handle *trans,
>
>   			set_extent_bit(&trans->transaction->pinned_extents,
>   				       bytenr, bytenr + num_bytes - 1,
> -				       EXTENT_DIRTY, NULL,
> -				       GFP_NOFS | __GFP_NOFAIL);
> +				       EXTENT_DIRTY, NULL, GFP_NOFS);
>   		}
>
>   		spin_lock(&trans->transaction->dirty_bgs_lock);
> diff --git a/fs/btrfs/extent-tree.c b/fs/btrfs/extent-tree.c
> index 03b2a7c508b9..6e319100e3a3 100644
> --- a/fs/btrfs/extent-tree.c
> +++ b/fs/btrfs/extent-tree.c
> @@ -2508,8 +2508,7 @@ static int pin_down_extent(struct btrfs_trans_handle *trans,
>   	spin_unlock(&cache->space_info->lock);
>
>   	set_extent_bit(&trans->transaction->pinned_extents, bytenr,
> -		       bytenr + num_bytes - 1, EXTENT_DIRTY, NULL,
> -		       GFP_NOFS | __GFP_NOFAIL);
> +		       bytenr + num_bytes - 1, EXTENT_DIRTY, NULL, GFP_NOFS);
>   	return 0;
>   }
>

Re: [PATCH 5/9] btrfs: open code set_extent_bits_nowait

[David Sterba]

On Thu, May 25, 2023 at 02:02:33AM -0700, Christoph Hellwig wrote:
> The change itself looks ok to me:
> 
> Reviewed-by: Christoph Hellwig <hch@lst.de>
> 
> .. but:
> 
> > diff --git a/fs/btrfs/zoned.c b/fs/btrfs/zoned.c
> > index bda301a55cbe..b82a350c4c59 100644
> > --- a/fs/btrfs/zoned.c
> > +++ b/fs/btrfs/zoned.c
> > @@ -1611,8 +1611,8 @@ void btrfs_redirty_list_add(struct btrfs_transaction *trans,
> >  	memzero_extent_buffer(eb, 0, eb->len);
> >  	set_bit(EXTENT_BUFFER_NO_CHECK, &eb->bflags);
> >  	set_extent_buffer_dirty(eb);
> > -	set_extent_bits_nowait(&trans->dirty_pages, eb->start,
> > -			       eb->start + eb->len - 1, EXTENT_DIRTY);
> > +	set_extent_bit(&trans->dirty_pages, eb->start, eb->start + eb->len - 1,
> > +			EXTENT_DIRTY, NULL, GFP_NOWAIT);
> 
> .. there is no point in even using GFP_NOWAIT here, as we are always
> called in a context that can sleep, set_extent_buffer_dirty relies on
> that as well as it calls lock_page.

Right, this case of NOWAIT can be removed.

Re: [PATCH 7/9] btrfs: drop NOFAIL from set_extent_bit allocation masks

[David Sterba]

On Thu, May 25, 2023 at 06:38:51PM +0800, Qu Wenruo wrote:
> On 2023/5/25 07:04, David Sterba wrote:
> > The __GFP_NOFAIL passed to set_extent_bit first appeared in 2010
> > (commit f0486c68e4bd9a ("Btrfs: Introduce contexts for metadata
> > reservation")), without any explanation why it would be needed.
> >
> > Meanwhile we've updated the semantics of set_extent_bit to handle failed
> > allocations and do unlock, sleep and retry if needed.  The use of the
> > NOFAIL flag is also an outlier, we never want any of the set/clear
> > extent bit helpers to fail, they're used for many critical changes like
> > extent locking, besides the extent state bit changes.
> 
> As I mentioned in the cover letter, if we really want to set/clear bits
> to not fail, and can accept the extra memory usage (as high as twice the
> number of extent states), then we should really consider the following
> changes:
> 
> - Introduce hole extent_state
>    For ranges without any bit set, there still needs to be an
>    extent_state.
> 
> - Make callers to pre-allocate 2 extent_state and pass them as mandatory
>    parameters to set/clear bits
> 
> - Make set/clear bits to use the preallocated 2 extent states
> 
> By this, we should be able to completely get rid of the memory
> allocation inside the extent io tree set/clear calls.

I did not understand what you mean from the cover letter comment but now
I see it. The size of extent_state is 72 bytes on release build, which
is not that much, doubling that size is still in acceptable range even
if we'd never utilize the preallocated memory at some point. That we
dont't see any failures now is 1) GFP_NOFS does not fail 2) we're using
a slab cache and the objects get reused within the same allocated space
due to frequent state changes. Even if 1) wasn't true we'd still have to
hit a hard memory allocation error, i.e. no available pages for a slab
extension.

If the preallocation prevents any failure due to memory allocations then
we can keep the current way of dealing with extent bit change error
handling, i.e. none because we rely on the allocator.

Re: [PATCH 7/9] btrfs: drop NOFAIL from set_extent_bit allocation masks

[David Sterba]

On Thu, May 25, 2023 at 02:06:05AM -0700, Christoph Hellwig wrote:
> On Thu, May 25, 2023 at 01:04:34AM +0200, David Sterba wrote:
> > The __GFP_NOFAIL passed to set_extent_bit first appeared in 2010
> > (commit f0486c68e4bd9a ("Btrfs: Introduce contexts for metadata
> > reservation")), without any explanation why it would be needed.
> > 
> > Meanwhile we've updated the semantics of set_extent_bit to handle failed
> > allocations and do unlock, sleep and retry if needed.  The use of the
> > NOFAIL flag is also an outlier, we never want any of the set/clear
> > extent bit helpers to fail, they're used for many critical changes like
> > extent locking, besides the extent state bit changes.
> 
> Given how many of the callers do not check the return value, and that
> the trees store essential information, I think the right thing here
> is to always use __GFP_NOFAIL unless GFP_NOWAIT is passed.  In practice
> this won't make a difference as currently small slab allocations never
> fail, but that's an undocumented assumption.

Yeah, that "GFP_NOFS does not fail for < costly allocations" has been a
deal with the allocator but there was some pressure to stop doing that
eventually. Discussed at LSF/MM, __GFP_NOFAIL should be replaced by
proper error handling if possible and minimized. I'm not sure if adding
it for something that frequently done as the state bit changes is a good
idea wrt memory management but I agree there's practically no change how
things currently work.

Qu suggested another way how we could preallocate some memory that would
be used for extent holes and then reused for the cases where we need to
allocate to do insert/split. I'd like to try this approach first and also
check with MM guys that NOFAIL in this case is acceptable so we have
more options to choose from.

Re: [PATCH 7/9] btrfs: drop NOFAIL from set_extent_bit allocation masks

[Qu Wenruo]

On 2023/5/26 07:00, David Sterba wrote:
> On Thu, May 25, 2023 at 06:38:51PM +0800, Qu Wenruo wrote:
>> On 2023/5/25 07:04, David Sterba wrote:
>>> The __GFP_NOFAIL passed to set_extent_bit first appeared in 2010
>>> (commit f0486c68e4bd9a ("Btrfs: Introduce contexts for metadata
>>> reservation")), without any explanation why it would be needed.
>>>
>>> Meanwhile we've updated the semantics of set_extent_bit to handle failed
>>> allocations and do unlock, sleep and retry if needed.  The use of the
>>> NOFAIL flag is also an outlier, we never want any of the set/clear
>>> extent bit helpers to fail, they're used for many critical changes like
>>> extent locking, besides the extent state bit changes.
>>
>> As I mentioned in the cover letter, if we really want to set/clear bits
>> to not fail, and can accept the extra memory usage (as high as twice the
>> number of extent states), then we should really consider the following
>> changes:
>>
>> - Introduce hole extent_state
>>     For ranges without any bit set, there still needs to be an
>>     extent_state.
>>
>> - Make callers to pre-allocate 2 extent_state and pass them as mandatory
>>     parameters to set/clear bits
>>
>> - Make set/clear bits to use the preallocated 2 extent states
>>
>> By this, we should be able to completely get rid of the memory
>> allocation inside the extent io tree set/clear calls.
>
> I did not understand what you mean from the cover letter comment but now
> I see it. The size of extent_state is 72 bytes on release build, which
> is not that much, doubling that size is still in acceptable range even
> if we'd never utilize the preallocated memory at some point. That we
> dont't see any failures now is 1) GFP_NOFS does not fail 2) we're using
> a slab cache and the objects get reused within the same allocated space
> due to frequent state changes. Even if 1) wasn't true we'd still have to
> hit a hard memory allocation error, i.e. no available pages for a slab
> extension.

Well, I still remember an internal bugzilla that openSUSE on RPI4
triggered failure at the extent_state allocation.

>
> If the preallocation prevents any failure due to memory allocations then
> we can keep the current way of dealing with extent bit change error
> handling, i.e. none because we rely on the allocator.

The preallocation is not going to help much after the first allocation
is being used.

After that, all later states are allocated using GFP_ATOMIC, which has a
much smaller pool to fulfill from.
And considering how many call sites don't check the return value at all,
we'd either go forward changing all call sites, or really make the
set/clear bits bulletproof (and only fail out of the set/clear call).

Thanks,
Qu