On Mon, Feb 22, 2021 at 07:17:49AM -0600, Goldwyn Rodrigues wrote:
> force_nocow can be calculated by btrfs_inode and does not need to be
> passed as an argument.
> 
> This simplifies run_delalloc_nocow() call from btrfs_run_delalloc_range()
> where should_nocow() checks for BTRFS_INODE_NODATASUM and
> BTRFS_INODE_PREALLOC flags or if EXTENT_DEFRAG flags are set.
> 
> should_nocow() has been re-arranged so EXTENT_DEFRAG has higher priority
> in checks.

Why is that? In the check sequence, test_range_bit is called first but
that's more expensive as it needs to iterate the range.

> Signed-off-by: Goldwyn Rodrigues <rgold...@suse.com>
> ---
>  fs/btrfs/inode.c | 28 +++++++++++++---------------
>  1 file changed, 13 insertions(+), 15 deletions(-)
> 
> diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c
> index 4f2f1e932751..2115d8cc6f18 100644
> --- a/fs/btrfs/inode.c
> +++ b/fs/btrfs/inode.c
> @@ -1516,7 +1516,7 @@ static int fallback_to_cow(struct btrfs_inode *inode, 
> struct page *locked_page,
>  static noinline int run_delalloc_nocow(struct btrfs_inode *inode,
>                                      struct page *locked_page,
>                                      const u64 start, const u64 end,
> -                                    int *page_started, int force,
> +                                    int *page_started,
>                                      unsigned long *nr_written)
>  {
>       struct btrfs_fs_info *fs_info = inode->root->fs_info;
> @@ -1530,6 +1530,7 @@ static noinline int run_delalloc_nocow(struct 
> btrfs_inode *inode,
>       u64 ino = btrfs_ino(inode);
>       bool nocow = false;
>       u64 disk_bytenr = 0;
> +     bool force = inode->flags & BTRFS_INODE_NODATACOW;
>  
>       path = btrfs_alloc_path();
>       if (!path) {
> @@ -1863,13 +1864,9 @@ static noinline int run_delalloc_nocow(struct 
> btrfs_inode *inode,
>       return ret;
>  }
>  
> -static inline int need_force_cow(struct btrfs_inode *inode, u64 start, u64 
> end)
> +static inline bool should_nocow(struct btrfs_inode *inode, u64 start, u64 
> end)
>  {
>  
> -     if (!(inode->flags & BTRFS_INODE_NODATACOW) &&
> -         !(inode->flags & BTRFS_INODE_PREALLOC))
> -             return 0;
> -
>       /*
>        * @defrag_bytes is a hint value, no spinlock held here,
>        * if is not zero, it means the file is defragging.
> @@ -1877,9 +1874,15 @@ static inline int need_force_cow(struct btrfs_inode 
> *inode, u64 start, u64 end)
>        */
>       if (inode->defrag_bytes &&
>           test_range_bit(&inode->io_tree, start, end, EXTENT_DEFRAG, 0, NULL))
> -             return 1;
> +             return false;
>  
> -     return 0;
> +     if (inode->flags & BTRFS_INODE_NODATACOW)
> +             return true;
> +
> +     if (inode->flags & BTRFS_INODE_PREALLOC)
> +             return true;

So here it needs to do test_range_bit first, while before the two checks
were fast path.

> +
> +     return false;
>  }
>  
>  /*
> @@ -1891,17 +1894,12 @@ int btrfs_run_delalloc_range(struct btrfs_inode 
> *inode, struct page *locked_page
>               struct writeback_control *wbc)
>  {
>       int ret;
> -     int force_cow = need_force_cow(inode, start, end);
>       const bool zoned = btrfs_is_zoned(inode->root->fs_info);
>  
> -     if (inode->flags & BTRFS_INODE_NODATACOW && !force_cow) {
> -             ASSERT(!zoned);
> -             ret = run_delalloc_nocow(inode, locked_page, start, end,
> -                                      page_started, 1, nr_written);
> -     } else if (inode->flags & BTRFS_INODE_PREALLOC && !force_cow) {
> +     if (should_nocow(inode, start, end)) {
>               ASSERT(!zoned);
>               ret = run_delalloc_nocow(inode, locked_page, start, end,
> -                                      page_started, 0, nr_written);
> +                                      page_started, nr_written);

Merging the two calls into one branch makes sense so that all the
reasons for 'nocow' are in one helper, so ok.

>       } else if (!inode_can_compress(inode) ||
>                  !inode_need_compress(inode, start, end)) {
>               if (zoned)
> -- 
> 2.30.1

Reply via email to