diff mbox series

[5/9] btrfs: open code set_extent_bits_nowait

Message ID a497320d91b1e08e0766f44844746e235478630e.1684967923.git.dsterba@suse.com (mailing list archive)
State New, archived
Headers show
Series Parameter cleanups in extent state helpers | expand

Commit Message

David Sterba May 24, 2023, 11:04 p.m. UTC 
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(-)

Comments

Christoph Hellwig May 25, 2023, 9:02 a.m. UTC | #1 
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.
Qu Wenruo May 25, 2023, 10:33 a.m. UTC | #2 
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)
David Sterba May 25, 2023, 10:45 p.m. UTC | #3 
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.
diff mbox series

Patch

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)