diff mbox series

[v2,08/11] btrfs: cleanup duplicated parameters related to can_nocow_file_extent_args

Message ID 5f31db841be606ec0bf8693377647f572ac9f4e7.1714707707.git.wqu@suse.com (mailing list archive)
State New, archived
Headers show
Series btrfs: extent-map: unify the members with btrfs_ordered_extent | expand

Commit Message

Qu Wenruo May 3, 2024, 6:01 a.m. UTC 
The following functions and structures can be simplified using the
btrfs_file_extent structure:

- can_nocow_extent()
  No need to return ram_bytes/orig_block_len through the parameter list,
  the @file_extent parameter contains all needed info.

- can_nocow_file_extent_args
  The following members are no longer needed:

  * disk_bytenr
    This one is confusing as it's not really the
    btrfs_file_extent_item::disk_bytenr, but where the IO would be,
    thus it's file_extent::disk_bytenr + file_extent::offset now.

  * num_bytes
    Now file_extent::num_bytes.

  * extent_offset
    Now file_extent::offset.

  * disk_num_bytes
    Now file_extent::disk_num_bytes.

Signed-off-by: Qu Wenruo <wqu@suse.com>
---
 fs/btrfs/btrfs_inode.h |  3 +-
 fs/btrfs/file.c        |  2 +-
 fs/btrfs/inode.c       | 89 +++++++++++++++++++-----------------------
 3 files changed, 42 insertions(+), 52 deletions(-)

Comments

Filipe Manana May 20, 2024, 3:55 p.m. UTC | #1 
On Fri, May 3, 2024 at 7:02 AM Qu Wenruo <wqu@suse.com> wrote:
>
> The following functions and structures can be simplified using the
> btrfs_file_extent structure:
>
> - can_nocow_extent()
>   No need to return ram_bytes/orig_block_len through the parameter list,
>   the @file_extent parameter contains all needed info.
>
> - can_nocow_file_extent_args
>   The following members are no longer needed:
>
>   * disk_bytenr
>     This one is confusing as it's not really the
>     btrfs_file_extent_item::disk_bytenr, but where the IO would be,
>     thus it's file_extent::disk_bytenr + file_extent::offset now.
>
>   * num_bytes
>     Now file_extent::num_bytes.
>
>   * extent_offset
>     Now file_extent::offset.
>
>   * disk_num_bytes
>     Now file_extent::disk_num_bytes.
>
> Signed-off-by: Qu Wenruo <wqu@suse.com>
> ---
>  fs/btrfs/btrfs_inode.h |  3 +-
>  fs/btrfs/file.c        |  2 +-
>  fs/btrfs/inode.c       | 89 +++++++++++++++++++-----------------------
>  3 files changed, 42 insertions(+), 52 deletions(-)
>
> diff --git a/fs/btrfs/btrfs_inode.h b/fs/btrfs/btrfs_inode.h
> index f30afce4f6ca..bea343615ad1 100644
> --- a/fs/btrfs/btrfs_inode.h
> +++ b/fs/btrfs/btrfs_inode.h
> @@ -461,8 +461,7 @@ struct btrfs_file_extent {
>  };
>
>  noinline int can_nocow_extent(struct inode *inode, u64 offset, u64 *len,
> -                             u64 *orig_block_len,
> -                             u64 *ram_bytes, struct btrfs_file_extent *file_extent,
> +                             struct btrfs_file_extent *file_extent,
>                               bool nowait, bool strict);
>
>  void btrfs_del_delalloc_inode(struct btrfs_inode *inode);
> diff --git a/fs/btrfs/file.c b/fs/btrfs/file.c
> index 102b5c17ece1..6aaeb9ee048d 100644
> --- a/fs/btrfs/file.c
> +++ b/fs/btrfs/file.c
> @@ -1104,7 +1104,7 @@ int btrfs_check_nocow_lock(struct btrfs_inode *inode, loff_t pos,
>                                                    &cached_state);
>         }
>         ret = can_nocow_extent(&inode->vfs_inode, lockstart, &num_bytes,
> -                       NULL, NULL, NULL, nowait, false);
> +                              NULL, nowait, false);
>         if (ret <= 0)
>                 btrfs_drew_write_unlock(&root->snapshot_lock);
>         else
> diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c
> index 8bc1f165193a..89f284ae26a4 100644
> --- a/fs/btrfs/inode.c
> +++ b/fs/btrfs/inode.c
> @@ -1862,16 +1862,10 @@ struct can_nocow_file_extent_args {
>          */
>         bool free_path;
>
> -       /* Output fields. Only set when can_nocow_file_extent() returns 1. */
> -
> -       u64 disk_bytenr;
> -       u64 disk_num_bytes;
> -       u64 extent_offset;
> -
> -       /* Number of bytes that can be written to in NOCOW mode. */
> -       u64 num_bytes;
> -
> -       /* The expected file extent for the NOCOW write. */
> +       /*
> +        * Output fields. Only set when can_nocow_file_extent() returns 1.
> +        * The expected file extent for the NOCOW write.
> +        */
>         struct btrfs_file_extent file_extent;
>  };
>
> @@ -1894,6 +1888,7 @@ static int can_nocow_file_extent(struct btrfs_path *path,
>         struct btrfs_root *root = inode->root;
>         struct btrfs_file_extent_item *fi;
>         struct btrfs_root *csum_root;
> +       u64 io_start;
>         u64 extent_end;
>         u8 extent_type;
>         int can_nocow = 0;
> @@ -1906,11 +1901,6 @@ static int can_nocow_file_extent(struct btrfs_path *path,
>         if (extent_type == BTRFS_FILE_EXTENT_INLINE)
>                 goto out;
>
> -       /* Can't access these fields unless we know it's not an inline extent. */
> -       args->disk_bytenr = btrfs_file_extent_disk_bytenr(leaf, fi);
> -       args->disk_num_bytes = btrfs_file_extent_disk_num_bytes(leaf, fi);
> -       args->extent_offset = btrfs_file_extent_offset(leaf, fi);
> -
>         if (!(inode->flags & BTRFS_INODE_NODATACOW) &&
>             extent_type == BTRFS_FILE_EXTENT_REG)
>                 goto out;
> @@ -1926,7 +1916,7 @@ static int can_nocow_file_extent(struct btrfs_path *path,
>                 goto out;
>
>         /* An explicit hole, must COW. */
> -       if (args->disk_bytenr == 0)
> +       if (btrfs_file_extent_disk_num_bytes(leaf, fi) == 0)

No, this is not correct.
It's btrfs_file_extent_disk_bytenr() that we want, not
btrfs_file_extent_disk_num_bytes().
In fact a disk_num_bytes of 0 should ve invalid and never happen.

>                 goto out;
>
>         /* Compressed/encrypted/encoded extents must be COWed. */
> @@ -1951,8 +1941,8 @@ static int can_nocow_file_extent(struct btrfs_path *path,
>         btrfs_release_path(path);
>
>         ret = btrfs_cross_ref_exist(root, btrfs_ino(inode),
> -                                   key->offset - args->extent_offset,
> -                                   args->disk_bytenr, args->strict, path);
> +                                   key->offset - args->file_extent.offset,
> +                                   args->file_extent.disk_bytenr, args->strict, path);
>         WARN_ON_ONCE(ret > 0 && is_freespace_inode);
>         if (ret != 0)
>                 goto out;
> @@ -1973,21 +1963,18 @@ static int can_nocow_file_extent(struct btrfs_path *path,
>             atomic_read(&root->snapshot_force_cow))
>                 goto out;
>
> -       args->disk_bytenr += args->extent_offset;
> -       args->disk_bytenr += args->start - key->offset;
> -       args->num_bytes = min(args->end + 1, extent_end) - args->start;
> -
> -       args->file_extent.num_bytes = args->num_bytes;
> +       args->file_extent.num_bytes = min(args->end + 1, extent_end) - args->start;
>         args->file_extent.offset += args->start - key->offset;
> +       io_start = args->file_extent.disk_bytenr + args->file_extent.offset;
>
>         /*
>          * Force COW if csums exist in the range. This ensures that csums for a
>          * given extent are either valid or do not exist.
>          */
>
> -       csum_root = btrfs_csum_root(root->fs_info, args->disk_bytenr);
> -       ret = btrfs_lookup_csums_list(csum_root, args->disk_bytenr,
> -                                     args->disk_bytenr + args->num_bytes - 1,
> +       csum_root = btrfs_csum_root(root->fs_info, io_start);
> +       ret = btrfs_lookup_csums_list(csum_root, io_start,
> +                                     io_start + args->file_extent.num_bytes - 1,
>                                       NULL, nowait);
>         WARN_ON_ONCE(ret > 0 && is_freespace_inode);
>         if (ret != 0)
> @@ -2046,7 +2033,6 @@ static noinline int run_delalloc_nocow(struct btrfs_inode *inode,
>                 struct extent_buffer *leaf;
>                 struct extent_state *cached_state = NULL;
>                 u64 extent_end;
> -               u64 ram_bytes;
>                 u64 nocow_end;
>                 int extent_type;
>                 bool is_prealloc;
> @@ -2125,7 +2111,6 @@ static noinline int run_delalloc_nocow(struct btrfs_inode *inode,
>                         ret = -EUCLEAN;
>                         goto error;
>                 }
> -               ram_bytes = btrfs_file_extent_ram_bytes(leaf, fi);
>                 extent_end = btrfs_file_extent_end(path);
>
>                 /*
> @@ -2145,7 +2130,9 @@ static noinline int run_delalloc_nocow(struct btrfs_inode *inode,
>                         goto must_cow;
>
>                 ret = 0;
> -               nocow_bg = btrfs_inc_nocow_writers(fs_info, nocow_args.disk_bytenr);
> +               nocow_bg = btrfs_inc_nocow_writers(fs_info,
> +                               nocow_args.file_extent.disk_bytenr +
> +                               nocow_args.file_extent.offset);
>                 if (!nocow_bg) {
>  must_cow:
>                         /*
> @@ -2181,16 +2168,18 @@ static noinline int run_delalloc_nocow(struct btrfs_inode *inode,
>                         }
>                 }
>
> -               nocow_end = cur_offset + nocow_args.num_bytes - 1;
> +               nocow_end = cur_offset + nocow_args.file_extent.num_bytes - 1;
>                 lock_extent(&inode->io_tree, cur_offset, nocow_end, &cached_state);
>
>                 is_prealloc = extent_type == BTRFS_FILE_EXTENT_PREALLOC;
>                 if (is_prealloc) {
>                         struct extent_map *em;
>
> -                       em = create_io_em(inode, cur_offset, nocow_args.num_bytes,
> -                                         nocow_args.disk_num_bytes, /* orig_block_len */
> -                                         ram_bytes, BTRFS_COMPRESS_NONE,
> +                       em = create_io_em(inode, cur_offset,
> +                                         nocow_args.file_extent.num_bytes,
> +                                         nocow_args.file_extent.disk_num_bytes,
> +                                         nocow_args.file_extent.ram_bytes,
> +                                         BTRFS_COMPRESS_NONE,
>                                           &nocow_args.file_extent,
>                                           BTRFS_ORDERED_PREALLOC);
>                         if (IS_ERR(em)) {
> @@ -2203,9 +2192,16 @@ static noinline int run_delalloc_nocow(struct btrfs_inode *inode,
>                         free_extent_map(em);
>                 }
>
> +               /*
> +                * Check btrfs_create_dio_extent() for why we intentionally pass
> +                * incorrect value for NOCOW/PREALLOC OEs.
> +                */

If in the next version you remove that similar comment/rant about OEs
and disk_bytenr, also remove this one.

Everything else in this patch looks fine, thanks.


>                 ordered = btrfs_alloc_ordered_extent(inode, cur_offset,
> -                               nocow_args.num_bytes, nocow_args.num_bytes,
> -                               nocow_args.disk_bytenr, nocow_args.num_bytes, 0,
> +                               nocow_args.file_extent.num_bytes,
> +                               nocow_args.file_extent.num_bytes,
> +                               nocow_args.file_extent.disk_bytenr +
> +                               nocow_args.file_extent.offset,
> +                               nocow_args.file_extent.num_bytes, 0,
>                                 is_prealloc
>                                 ? (1 << BTRFS_ORDERED_PREALLOC)
>                                 : (1 << BTRFS_ORDERED_NOCOW),
> @@ -7144,8 +7140,7 @@ static bool btrfs_extent_readonly(struct btrfs_fs_info *fs_info, u64 bytenr)
>   *      any ordered extents.
>   */
>  noinline int can_nocow_extent(struct inode *inode, u64 offset, u64 *len,
> -                             u64 *orig_block_len,
> -                             u64 *ram_bytes, struct btrfs_file_extent *file_extent,
> +                             struct btrfs_file_extent *file_extent,
>                               bool nowait, bool strict)
>  {
>         struct btrfs_fs_info *fs_info = inode_to_fs_info(inode);
> @@ -7196,8 +7191,6 @@ noinline int can_nocow_extent(struct inode *inode, u64 offset, u64 *len,
>
>         fi = btrfs_item_ptr(leaf, path->slots[0], struct btrfs_file_extent_item);
>         found_type = btrfs_file_extent_type(leaf, fi);
> -       if (ram_bytes)
> -               *ram_bytes = btrfs_file_extent_ram_bytes(leaf, fi);
>
>         nocow_args.start = offset;
>         nocow_args.end = offset + *len - 1;
> @@ -7215,14 +7208,15 @@ noinline int can_nocow_extent(struct inode *inode, u64 offset, u64 *len,
>         }
>
>         ret = 0;
> -       if (btrfs_extent_readonly(fs_info, nocow_args.disk_bytenr))
> +       if (btrfs_extent_readonly(fs_info,
> +                               nocow_args.file_extent.disk_bytenr + nocow_args.file_extent.offset))
>                 goto out;
>
>         if (!(BTRFS_I(inode)->flags & BTRFS_INODE_NODATACOW) &&
>             found_type == BTRFS_FILE_EXTENT_PREALLOC) {
>                 u64 range_end;
>
> -               range_end = round_up(offset + nocow_args.num_bytes,
> +               range_end = round_up(offset + nocow_args.file_extent.num_bytes,
>                                      root->fs_info->sectorsize) - 1;
>                 ret = test_range_bit_exists(io_tree, offset, range_end, EXTENT_DELALLOC);
>                 if (ret) {
> @@ -7231,13 +7225,11 @@ noinline int can_nocow_extent(struct inode *inode, u64 offset, u64 *len,
>                 }
>         }
>
> -       if (orig_block_len)
> -               *orig_block_len = nocow_args.disk_num_bytes;
>         if (file_extent)
>                 memcpy(file_extent, &nocow_args.file_extent,
>                        sizeof(struct btrfs_file_extent));
>
> -       *len = nocow_args.num_bytes;
> +       *len = nocow_args.file_extent.num_bytes;
>         ret = 1;
>  out:
>         btrfs_free_path(path);
> @@ -7422,7 +7414,7 @@ static int btrfs_get_blocks_direct_write(struct extent_map **map,
>         struct btrfs_file_extent file_extent = { 0 };
>         struct extent_map *em = *map;
>         int type;
> -       u64 block_start, orig_block_len, ram_bytes;
> +       u64 block_start;
>         struct btrfs_block_group *bg;
>         bool can_nocow = false;
>         bool space_reserved = false;
> @@ -7450,7 +7442,6 @@ static int btrfs_get_blocks_direct_write(struct extent_map **map,
>                 block_start = extent_map_block_start(em) + (start - em->start);
>
>                 if (can_nocow_extent(inode, start, &len,
> -                                    &orig_block_len, &ram_bytes,
>                                      &file_extent, false, false) == 1) {
>                         bg = btrfs_inc_nocow_writers(fs_info, block_start);
>                         if (bg)
> @@ -7477,8 +7468,8 @@ static int btrfs_get_blocks_direct_write(struct extent_map **map,
>                 space_reserved = true;
>
>                 em2 = btrfs_create_dio_extent(BTRFS_I(inode), dio_data, start, len,
> -                                             orig_block_len,
> -                                             ram_bytes, type,
> +                                             file_extent.disk_num_bytes,
> +                                             file_extent.ram_bytes, type,
>                                               &file_extent);
>                 btrfs_dec_nocow_writers(bg);
>                 if (type == BTRFS_ORDERED_PREALLOC) {
> @@ -10709,7 +10700,7 @@ static int btrfs_swap_activate(struct swap_info_struct *sis, struct file *file,
>                 free_extent_map(em);
>                 em = NULL;
>
> -               ret = can_nocow_extent(inode, start, &len, NULL, NULL, NULL, false, true);
> +               ret = can_nocow_extent(inode, start, &len, NULL, false, true);
>                 if (ret < 0) {
>                         goto out;
>                 } else if (ret) {
> --
> 2.45.0
>
>
Qu Wenruo May 20, 2024, 10:13 p.m. UTC | #2 
在 2024/5/21 01:25, Filipe Manana 写道:
> On Fri, May 3, 2024 at 7:02 AM Qu Wenruo <wqu@suse.com> wrote:
[...]
>> @@ -1926,7 +1916,7 @@ static int can_nocow_file_extent(struct btrfs_path *path,
>>                  goto out;
>>
>>          /* An explicit hole, must COW. */
>> -       if (args->disk_bytenr == 0)
>> +       if (btrfs_file_extent_disk_num_bytes(leaf, fi) == 0)
>
> No, this is not correct.

All my bad, will fix it definitely.

> It's btrfs_file_extent_disk_bytenr() that we want, not
> btrfs_file_extent_disk_num_bytes().
> In fact a disk_num_bytes of 0 should ve invalid and never happen.

Thankfully for most cases, a explict hole has both its disk_num_bytes
and disk_bytenr as zeros, thus I didn't get any test case triggered:

	item 6 key (257 EXTENT_DATA 0) itemoff 15816 itemsize 53
		generation 7 type 1 (regular)
		extent data disk byte 0 nr 0
		extent data offset 0 nr 65536 ram 65536
		extent compression 0 (none)

But still I should fix that.

Thanks,
Qu

>
>>                  goto out;
>>
>>          /* Compressed/encrypted/encoded extents must be COWed. */
>> @@ -1951,8 +1941,8 @@ static int can_nocow_file_extent(struct btrfs_path *path,
>>          btrfs_release_path(path);
>>
>>          ret = btrfs_cross_ref_exist(root, btrfs_ino(inode),
>> -                                   key->offset - args->extent_offset,
>> -                                   args->disk_bytenr, args->strict, path);
>> +                                   key->offset - args->file_extent.offset,
>> +                                   args->file_extent.disk_bytenr, args->strict, path);
>>          WARN_ON_ONCE(ret > 0 && is_freespace_inode);
>>          if (ret != 0)
>>                  goto out;
>> @@ -1973,21 +1963,18 @@ static int can_nocow_file_extent(struct btrfs_path *path,
>>              atomic_read(&root->snapshot_force_cow))
>>                  goto out;
>>
>> -       args->disk_bytenr += args->extent_offset;
>> -       args->disk_bytenr += args->start - key->offset;
>> -       args->num_bytes = min(args->end + 1, extent_end) - args->start;
>> -
>> -       args->file_extent.num_bytes = args->num_bytes;
>> +       args->file_extent.num_bytes = min(args->end + 1, extent_end) - args->start;
>>          args->file_extent.offset += args->start - key->offset;
>> +       io_start = args->file_extent.disk_bytenr + args->file_extent.offset;
>>
>>          /*
>>           * Force COW if csums exist in the range. This ensures that csums for a
>>           * given extent are either valid or do not exist.
>>           */
>>
>> -       csum_root = btrfs_csum_root(root->fs_info, args->disk_bytenr);
>> -       ret = btrfs_lookup_csums_list(csum_root, args->disk_bytenr,
>> -                                     args->disk_bytenr + args->num_bytes - 1,
>> +       csum_root = btrfs_csum_root(root->fs_info, io_start);
>> +       ret = btrfs_lookup_csums_list(csum_root, io_start,
>> +                                     io_start + args->file_extent.num_bytes - 1,
>>                                        NULL, nowait);
>>          WARN_ON_ONCE(ret > 0 && is_freespace_inode);
>>          if (ret != 0)
>> @@ -2046,7 +2033,6 @@ static noinline int run_delalloc_nocow(struct btrfs_inode *inode,
>>                  struct extent_buffer *leaf;
>>                  struct extent_state *cached_state = NULL;
>>                  u64 extent_end;
>> -               u64 ram_bytes;
>>                  u64 nocow_end;
>>                  int extent_type;
>>                  bool is_prealloc;
>> @@ -2125,7 +2111,6 @@ static noinline int run_delalloc_nocow(struct btrfs_inode *inode,
>>                          ret = -EUCLEAN;
>>                          goto error;
>>                  }
>> -               ram_bytes = btrfs_file_extent_ram_bytes(leaf, fi);
>>                  extent_end = btrfs_file_extent_end(path);
>>
>>                  /*
>> @@ -2145,7 +2130,9 @@ static noinline int run_delalloc_nocow(struct btrfs_inode *inode,
>>                          goto must_cow;
>>
>>                  ret = 0;
>> -               nocow_bg = btrfs_inc_nocow_writers(fs_info, nocow_args.disk_bytenr);
>> +               nocow_bg = btrfs_inc_nocow_writers(fs_info,
>> +                               nocow_args.file_extent.disk_bytenr +
>> +                               nocow_args.file_extent.offset);
>>                  if (!nocow_bg) {
>>   must_cow:
>>                          /*
>> @@ -2181,16 +2168,18 @@ static noinline int run_delalloc_nocow(struct btrfs_inode *inode,
>>                          }
>>                  }
>>
>> -               nocow_end = cur_offset + nocow_args.num_bytes - 1;
>> +               nocow_end = cur_offset + nocow_args.file_extent.num_bytes - 1;
>>                  lock_extent(&inode->io_tree, cur_offset, nocow_end, &cached_state);
>>
>>                  is_prealloc = extent_type == BTRFS_FILE_EXTENT_PREALLOC;
>>                  if (is_prealloc) {
>>                          struct extent_map *em;
>>
>> -                       em = create_io_em(inode, cur_offset, nocow_args.num_bytes,
>> -                                         nocow_args.disk_num_bytes, /* orig_block_len */
>> -                                         ram_bytes, BTRFS_COMPRESS_NONE,
>> +                       em = create_io_em(inode, cur_offset,
>> +                                         nocow_args.file_extent.num_bytes,
>> +                                         nocow_args.file_extent.disk_num_bytes,
>> +                                         nocow_args.file_extent.ram_bytes,
>> +                                         BTRFS_COMPRESS_NONE,
>>                                            &nocow_args.file_extent,
>>                                            BTRFS_ORDERED_PREALLOC);
>>                          if (IS_ERR(em)) {
>> @@ -2203,9 +2192,16 @@ static noinline int run_delalloc_nocow(struct btrfs_inode *inode,
>>                          free_extent_map(em);
>>                  }
>>
>> +               /*
>> +                * Check btrfs_create_dio_extent() for why we intentionally pass
>> +                * incorrect value for NOCOW/PREALLOC OEs.
>> +                */
>
> If in the next version you remove that similar comment/rant about OEs
> and disk_bytenr, also remove this one.
>
> Everything else in this patch looks fine, thanks.
>
>
>>                  ordered = btrfs_alloc_ordered_extent(inode, cur_offset,
>> -                               nocow_args.num_bytes, nocow_args.num_bytes,
>> -                               nocow_args.disk_bytenr, nocow_args.num_bytes, 0,
>> +                               nocow_args.file_extent.num_bytes,
>> +                               nocow_args.file_extent.num_bytes,
>> +                               nocow_args.file_extent.disk_bytenr +
>> +                               nocow_args.file_extent.offset,
>> +                               nocow_args.file_extent.num_bytes, 0,
>>                                  is_prealloc
>>                                  ? (1 << BTRFS_ORDERED_PREALLOC)
>>                                  : (1 << BTRFS_ORDERED_NOCOW),
>> @@ -7144,8 +7140,7 @@ static bool btrfs_extent_readonly(struct btrfs_fs_info *fs_info, u64 bytenr)
>>    *      any ordered extents.
>>    */
>>   noinline int can_nocow_extent(struct inode *inode, u64 offset, u64 *len,
>> -                             u64 *orig_block_len,
>> -                             u64 *ram_bytes, struct btrfs_file_extent *file_extent,
>> +                             struct btrfs_file_extent *file_extent,
>>                                bool nowait, bool strict)
>>   {
>>          struct btrfs_fs_info *fs_info = inode_to_fs_info(inode);
>> @@ -7196,8 +7191,6 @@ noinline int can_nocow_extent(struct inode *inode, u64 offset, u64 *len,
>>
>>          fi = btrfs_item_ptr(leaf, path->slots[0], struct btrfs_file_extent_item);
>>          found_type = btrfs_file_extent_type(leaf, fi);
>> -       if (ram_bytes)
>> -               *ram_bytes = btrfs_file_extent_ram_bytes(leaf, fi);
>>
>>          nocow_args.start = offset;
>>          nocow_args.end = offset + *len - 1;
>> @@ -7215,14 +7208,15 @@ noinline int can_nocow_extent(struct inode *inode, u64 offset, u64 *len,
>>          }
>>
>>          ret = 0;
>> -       if (btrfs_extent_readonly(fs_info, nocow_args.disk_bytenr))
>> +       if (btrfs_extent_readonly(fs_info,
>> +                               nocow_args.file_extent.disk_bytenr + nocow_args.file_extent.offset))
>>                  goto out;
>>
>>          if (!(BTRFS_I(inode)->flags & BTRFS_INODE_NODATACOW) &&
>>              found_type == BTRFS_FILE_EXTENT_PREALLOC) {
>>                  u64 range_end;
>>
>> -               range_end = round_up(offset + nocow_args.num_bytes,
>> +               range_end = round_up(offset + nocow_args.file_extent.num_bytes,
>>                                       root->fs_info->sectorsize) - 1;
>>                  ret = test_range_bit_exists(io_tree, offset, range_end, EXTENT_DELALLOC);
>>                  if (ret) {
>> @@ -7231,13 +7225,11 @@ noinline int can_nocow_extent(struct inode *inode, u64 offset, u64 *len,
>>                  }
>>          }
>>
>> -       if (orig_block_len)
>> -               *orig_block_len = nocow_args.disk_num_bytes;
>>          if (file_extent)
>>                  memcpy(file_extent, &nocow_args.file_extent,
>>                         sizeof(struct btrfs_file_extent));
>>
>> -       *len = nocow_args.num_bytes;
>> +       *len = nocow_args.file_extent.num_bytes;
>>          ret = 1;
>>   out:
>>          btrfs_free_path(path);
>> @@ -7422,7 +7414,7 @@ static int btrfs_get_blocks_direct_write(struct extent_map **map,
>>          struct btrfs_file_extent file_extent = { 0 };
>>          struct extent_map *em = *map;
>>          int type;
>> -       u64 block_start, orig_block_len, ram_bytes;
>> +       u64 block_start;
>>          struct btrfs_block_group *bg;
>>          bool can_nocow = false;
>>          bool space_reserved = false;
>> @@ -7450,7 +7442,6 @@ static int btrfs_get_blocks_direct_write(struct extent_map **map,
>>                  block_start = extent_map_block_start(em) + (start - em->start);
>>
>>                  if (can_nocow_extent(inode, start, &len,
>> -                                    &orig_block_len, &ram_bytes,
>>                                       &file_extent, false, false) == 1) {
>>                          bg = btrfs_inc_nocow_writers(fs_info, block_start);
>>                          if (bg)
>> @@ -7477,8 +7468,8 @@ static int btrfs_get_blocks_direct_write(struct extent_map **map,
>>                  space_reserved = true;
>>
>>                  em2 = btrfs_create_dio_extent(BTRFS_I(inode), dio_data, start, len,
>> -                                             orig_block_len,
>> -                                             ram_bytes, type,
>> +                                             file_extent.disk_num_bytes,
>> +                                             file_extent.ram_bytes, type,
>>                                                &file_extent);
>>                  btrfs_dec_nocow_writers(bg);
>>                  if (type == BTRFS_ORDERED_PREALLOC) {
>> @@ -10709,7 +10700,7 @@ static int btrfs_swap_activate(struct swap_info_struct *sis, struct file *file,
>>                  free_extent_map(em);
>>                  em = NULL;
>>
>> -               ret = can_nocow_extent(inode, start, &len, NULL, NULL, NULL, false, true);
>> +               ret = can_nocow_extent(inode, start, &len, NULL, false, true);
>>                  if (ret < 0) {
>>                          goto out;
>>                  } else if (ret) {
>> --
>> 2.45.0
>>
>>
>
diff mbox series

Patch

diff --git a/fs/btrfs/btrfs_inode.h b/fs/btrfs/btrfs_inode.h
index f30afce4f6ca..bea343615ad1 100644
--- a/fs/btrfs/btrfs_inode.h
+++ b/fs/btrfs/btrfs_inode.h
@@ -461,8 +461,7 @@  struct btrfs_file_extent {
 };
 
 noinline int can_nocow_extent(struct inode *inode, u64 offset, u64 *len,
-			      u64 *orig_block_len,
-			      u64 *ram_bytes, struct btrfs_file_extent *file_extent,
+			      struct btrfs_file_extent *file_extent,
 			      bool nowait, bool strict);
 
 void btrfs_del_delalloc_inode(struct btrfs_inode *inode);
diff --git a/fs/btrfs/file.c b/fs/btrfs/file.c
index 102b5c17ece1..6aaeb9ee048d 100644
--- a/fs/btrfs/file.c
+++ b/fs/btrfs/file.c
@@ -1104,7 +1104,7 @@  int btrfs_check_nocow_lock(struct btrfs_inode *inode, loff_t pos,
 						   &cached_state);
 	}
 	ret = can_nocow_extent(&inode->vfs_inode, lockstart, &num_bytes,
-			NULL, NULL, NULL, nowait, false);
+			       NULL, nowait, false);
 	if (ret <= 0)
 		btrfs_drew_write_unlock(&root->snapshot_lock);
 	else
diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c
index 8bc1f165193a..89f284ae26a4 100644
--- a/fs/btrfs/inode.c
+++ b/fs/btrfs/inode.c
@@ -1862,16 +1862,10 @@  struct can_nocow_file_extent_args {
 	 */
 	bool free_path;
 
-	/* Output fields. Only set when can_nocow_file_extent() returns 1. */
-
-	u64 disk_bytenr;
-	u64 disk_num_bytes;
-	u64 extent_offset;
-
-	/* Number of bytes that can be written to in NOCOW mode. */
-	u64 num_bytes;
-
-	/* The expected file extent for the NOCOW write. */
+	/*
+	 * Output fields. Only set when can_nocow_file_extent() returns 1.
+	 * The expected file extent for the NOCOW write.
+	 */
 	struct btrfs_file_extent file_extent;
 };
 
@@ -1894,6 +1888,7 @@  static int can_nocow_file_extent(struct btrfs_path *path,
 	struct btrfs_root *root = inode->root;
 	struct btrfs_file_extent_item *fi;
 	struct btrfs_root *csum_root;
+	u64 io_start;
 	u64 extent_end;
 	u8 extent_type;
 	int can_nocow = 0;
@@ -1906,11 +1901,6 @@  static int can_nocow_file_extent(struct btrfs_path *path,
 	if (extent_type == BTRFS_FILE_EXTENT_INLINE)
 		goto out;
 
-	/* Can't access these fields unless we know it's not an inline extent. */
-	args->disk_bytenr = btrfs_file_extent_disk_bytenr(leaf, fi);
-	args->disk_num_bytes = btrfs_file_extent_disk_num_bytes(leaf, fi);
-	args->extent_offset = btrfs_file_extent_offset(leaf, fi);
-
 	if (!(inode->flags & BTRFS_INODE_NODATACOW) &&
 	    extent_type == BTRFS_FILE_EXTENT_REG)
 		goto out;
@@ -1926,7 +1916,7 @@  static int can_nocow_file_extent(struct btrfs_path *path,
 		goto out;
 
 	/* An explicit hole, must COW. */
-	if (args->disk_bytenr == 0)
+	if (btrfs_file_extent_disk_num_bytes(leaf, fi) == 0)
 		goto out;
 
 	/* Compressed/encrypted/encoded extents must be COWed. */
@@ -1951,8 +1941,8 @@  static int can_nocow_file_extent(struct btrfs_path *path,
 	btrfs_release_path(path);
 
 	ret = btrfs_cross_ref_exist(root, btrfs_ino(inode),
-				    key->offset - args->extent_offset,
-				    args->disk_bytenr, args->strict, path);
+				    key->offset - args->file_extent.offset,
+				    args->file_extent.disk_bytenr, args->strict, path);
 	WARN_ON_ONCE(ret > 0 && is_freespace_inode);
 	if (ret != 0)
 		goto out;
@@ -1973,21 +1963,18 @@  static int can_nocow_file_extent(struct btrfs_path *path,
 	    atomic_read(&root->snapshot_force_cow))
 		goto out;
 
-	args->disk_bytenr += args->extent_offset;
-	args->disk_bytenr += args->start - key->offset;
-	args->num_bytes = min(args->end + 1, extent_end) - args->start;
-
-	args->file_extent.num_bytes = args->num_bytes;
+	args->file_extent.num_bytes = min(args->end + 1, extent_end) - args->start;
 	args->file_extent.offset += args->start - key->offset;
+	io_start = args->file_extent.disk_bytenr + args->file_extent.offset;
 
 	/*
 	 * Force COW if csums exist in the range. This ensures that csums for a
 	 * given extent are either valid or do not exist.
 	 */
 
-	csum_root = btrfs_csum_root(root->fs_info, args->disk_bytenr);
-	ret = btrfs_lookup_csums_list(csum_root, args->disk_bytenr,
-				      args->disk_bytenr + args->num_bytes - 1,
+	csum_root = btrfs_csum_root(root->fs_info, io_start);
+	ret = btrfs_lookup_csums_list(csum_root, io_start,
+				      io_start + args->file_extent.num_bytes - 1,
 				      NULL, nowait);
 	WARN_ON_ONCE(ret > 0 && is_freespace_inode);
 	if (ret != 0)
@@ -2046,7 +2033,6 @@  static noinline int run_delalloc_nocow(struct btrfs_inode *inode,
 		struct extent_buffer *leaf;
 		struct extent_state *cached_state = NULL;
 		u64 extent_end;
-		u64 ram_bytes;
 		u64 nocow_end;
 		int extent_type;
 		bool is_prealloc;
@@ -2125,7 +2111,6 @@  static noinline int run_delalloc_nocow(struct btrfs_inode *inode,
 			ret = -EUCLEAN;
 			goto error;
 		}
-		ram_bytes = btrfs_file_extent_ram_bytes(leaf, fi);
 		extent_end = btrfs_file_extent_end(path);
 
 		/*
@@ -2145,7 +2130,9 @@  static noinline int run_delalloc_nocow(struct btrfs_inode *inode,
 			goto must_cow;
 
 		ret = 0;
-		nocow_bg = btrfs_inc_nocow_writers(fs_info, nocow_args.disk_bytenr);
+		nocow_bg = btrfs_inc_nocow_writers(fs_info,
+				nocow_args.file_extent.disk_bytenr +
+				nocow_args.file_extent.offset);
 		if (!nocow_bg) {
 must_cow:
 			/*
@@ -2181,16 +2168,18 @@  static noinline int run_delalloc_nocow(struct btrfs_inode *inode,
 			}
 		}
 
-		nocow_end = cur_offset + nocow_args.num_bytes - 1;
+		nocow_end = cur_offset + nocow_args.file_extent.num_bytes - 1;
 		lock_extent(&inode->io_tree, cur_offset, nocow_end, &cached_state);
 
 		is_prealloc = extent_type == BTRFS_FILE_EXTENT_PREALLOC;
 		if (is_prealloc) {
 			struct extent_map *em;
 
-			em = create_io_em(inode, cur_offset, nocow_args.num_bytes,
-					  nocow_args.disk_num_bytes, /* orig_block_len */
-					  ram_bytes, BTRFS_COMPRESS_NONE,
+			em = create_io_em(inode, cur_offset,
+					  nocow_args.file_extent.num_bytes,
+					  nocow_args.file_extent.disk_num_bytes,
+					  nocow_args.file_extent.ram_bytes,
+					  BTRFS_COMPRESS_NONE,
 					  &nocow_args.file_extent,
 					  BTRFS_ORDERED_PREALLOC);
 			if (IS_ERR(em)) {
@@ -2203,9 +2192,16 @@  static noinline int run_delalloc_nocow(struct btrfs_inode *inode,
 			free_extent_map(em);
 		}
 
+		/*
+		 * Check btrfs_create_dio_extent() for why we intentionally pass
+		 * incorrect value for NOCOW/PREALLOC OEs.
+		 */
 		ordered = btrfs_alloc_ordered_extent(inode, cur_offset,
-				nocow_args.num_bytes, nocow_args.num_bytes,
-				nocow_args.disk_bytenr, nocow_args.num_bytes, 0,
+				nocow_args.file_extent.num_bytes,
+				nocow_args.file_extent.num_bytes,
+				nocow_args.file_extent.disk_bytenr +
+				nocow_args.file_extent.offset,
+				nocow_args.file_extent.num_bytes, 0,
 				is_prealloc
 				? (1 << BTRFS_ORDERED_PREALLOC)
 				: (1 << BTRFS_ORDERED_NOCOW),
@@ -7144,8 +7140,7 @@  static bool btrfs_extent_readonly(struct btrfs_fs_info *fs_info, u64 bytenr)
  *	 any ordered extents.
  */
 noinline int can_nocow_extent(struct inode *inode, u64 offset, u64 *len,
-			      u64 *orig_block_len,
-			      u64 *ram_bytes, struct btrfs_file_extent *file_extent,
+			      struct btrfs_file_extent *file_extent,
 			      bool nowait, bool strict)
 {
 	struct btrfs_fs_info *fs_info = inode_to_fs_info(inode);
@@ -7196,8 +7191,6 @@  noinline int can_nocow_extent(struct inode *inode, u64 offset, u64 *len,
 
 	fi = btrfs_item_ptr(leaf, path->slots[0], struct btrfs_file_extent_item);
 	found_type = btrfs_file_extent_type(leaf, fi);
-	if (ram_bytes)
-		*ram_bytes = btrfs_file_extent_ram_bytes(leaf, fi);
 
 	nocow_args.start = offset;
 	nocow_args.end = offset + *len - 1;
@@ -7215,14 +7208,15 @@  noinline int can_nocow_extent(struct inode *inode, u64 offset, u64 *len,
 	}
 
 	ret = 0;
-	if (btrfs_extent_readonly(fs_info, nocow_args.disk_bytenr))
+	if (btrfs_extent_readonly(fs_info,
+				nocow_args.file_extent.disk_bytenr + nocow_args.file_extent.offset))
 		goto out;
 
 	if (!(BTRFS_I(inode)->flags & BTRFS_INODE_NODATACOW) &&
 	    found_type == BTRFS_FILE_EXTENT_PREALLOC) {
 		u64 range_end;
 
-		range_end = round_up(offset + nocow_args.num_bytes,
+		range_end = round_up(offset + nocow_args.file_extent.num_bytes,
 				     root->fs_info->sectorsize) - 1;
 		ret = test_range_bit_exists(io_tree, offset, range_end, EXTENT_DELALLOC);
 		if (ret) {
@@ -7231,13 +7225,11 @@  noinline int can_nocow_extent(struct inode *inode, u64 offset, u64 *len,
 		}
 	}
 
-	if (orig_block_len)
-		*orig_block_len = nocow_args.disk_num_bytes;
 	if (file_extent)
 		memcpy(file_extent, &nocow_args.file_extent,
 		       sizeof(struct btrfs_file_extent));
 
-	*len = nocow_args.num_bytes;
+	*len = nocow_args.file_extent.num_bytes;
 	ret = 1;
 out:
 	btrfs_free_path(path);
@@ -7422,7 +7414,7 @@  static int btrfs_get_blocks_direct_write(struct extent_map **map,
 	struct btrfs_file_extent file_extent = { 0 };
 	struct extent_map *em = *map;
 	int type;
-	u64 block_start, orig_block_len, ram_bytes;
+	u64 block_start;
 	struct btrfs_block_group *bg;
 	bool can_nocow = false;
 	bool space_reserved = false;
@@ -7450,7 +7442,6 @@  static int btrfs_get_blocks_direct_write(struct extent_map **map,
 		block_start = extent_map_block_start(em) + (start - em->start);
 
 		if (can_nocow_extent(inode, start, &len,
-				     &orig_block_len, &ram_bytes,
 				     &file_extent, false, false) == 1) {
 			bg = btrfs_inc_nocow_writers(fs_info, block_start);
 			if (bg)
@@ -7477,8 +7468,8 @@  static int btrfs_get_blocks_direct_write(struct extent_map **map,
 		space_reserved = true;
 
 		em2 = btrfs_create_dio_extent(BTRFS_I(inode), dio_data, start, len,
-					      orig_block_len,
-					      ram_bytes, type,
+					      file_extent.disk_num_bytes,
+					      file_extent.ram_bytes, type,
 					      &file_extent);
 		btrfs_dec_nocow_writers(bg);
 		if (type == BTRFS_ORDERED_PREALLOC) {
@@ -10709,7 +10700,7 @@  static int btrfs_swap_activate(struct swap_info_struct *sis, struct file *file,
 		free_extent_map(em);
 		em = NULL;
 
-		ret = can_nocow_extent(inode, start, &len, NULL, NULL, NULL, false, true);
+		ret = can_nocow_extent(inode, start, &len, NULL, false, true);
 		if (ret < 0) {
 			goto out;
 		} else if (ret) {