diff mbox series

[v2,11/11] btrfs: cleanup duplicated parameters related to btrfs_create_dio_extent()

Message ID 8bdb1c42be16e32919b5e3e80aa6576c3a688d0d.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 3 parameters can be cleaned up using btrfs_file_extent
structure:

- len
  btrfs_file_extent::num_bytes

- orig_block_len
  btrfs_file_extent::disk_num_bytes

- ram_bytes
  btrfs_file_extent::ram_bytes

Signed-off-by: Qu Wenruo <wqu@suse.com>
---
 fs/btrfs/inode.c | 22 ++++++++--------------
 1 file changed, 8 insertions(+), 14 deletions(-)

Comments

Filipe Manana May 20, 2024, 4:48 p.m. UTC | #1 
On Fri, May 3, 2024 at 7:03 AM Qu Wenruo <wqu@suse.com> wrote:
>
> The following 3 parameters can be cleaned up using btrfs_file_extent
> structure:
>
> - len
>   btrfs_file_extent::num_bytes
>
> - orig_block_len
>   btrfs_file_extent::disk_num_bytes
>
> - ram_bytes
>   btrfs_file_extent::ram_bytes
>
> Signed-off-by: Qu Wenruo <wqu@suse.com>
> ---
>  fs/btrfs/inode.c | 22 ++++++++--------------
>  1 file changed, 8 insertions(+), 14 deletions(-)
>
> diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c
> index a95dc2333972..09974c86d3d1 100644
> --- a/fs/btrfs/inode.c
> +++ b/fs/btrfs/inode.c
> @@ -6969,11 +6969,8 @@ struct extent_map *btrfs_get_extent(struct btrfs_inode *inode,
>  static struct extent_map *btrfs_create_dio_extent(struct btrfs_inode *inode,
>                                                   struct btrfs_dio_data *dio_data,
>                                                   const u64 start,
> -                                                 const u64 len,
> -                                                 const u64 orig_block_len,
> -                                                 const u64 ram_bytes,
> -                                                 const int type,
> -                                                 struct btrfs_file_extent *file_extent)
> +                                                 struct btrfs_file_extent *file_extent,
> +                                                 const int type)
>  {
>         struct extent_map *em = NULL;
>         struct btrfs_ordered_extent *ordered;
> @@ -6991,7 +6988,7 @@ static struct extent_map *btrfs_create_dio_extent(struct btrfs_inode *inode,
>                 if (em) {
>                         free_extent_map(em);
>                         btrfs_drop_extent_map_range(inode, start,
> -                                                   start + len - 1, false);
> +                                       start + file_extent->num_bytes - 1, false);
>                 }
>                 em = ERR_CAST(ordered);
>         } else {
> @@ -7034,10 +7031,9 @@ static struct extent_map *btrfs_new_extent_direct(struct btrfs_inode *inode,
>         file_extent.ram_bytes = ins.offset;
>         file_extent.offset = 0;
>         file_extent.compression = BTRFS_COMPRESS_NONE;
> -       em = btrfs_create_dio_extent(inode, dio_data, start, ins.offset,
> -                                    ins.offset,
> -                                    ins.offset, BTRFS_ORDERED_REGULAR,
> -                                    &file_extent);
> +       em = btrfs_create_dio_extent(inode, dio_data, start,
> +                                    &file_extent,
> +                                    BTRFS_ORDERED_REGULAR);

As we're changing this, we can leave this in a single line as it fits.

>         btrfs_dec_block_group_reservations(fs_info, ins.objectid);
>         if (IS_ERR(em))
>                 btrfs_free_reserved_extent(fs_info, ins.objectid, ins.offset,
> @@ -7404,10 +7400,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,
> -                                             file_extent.disk_num_bytes,
> -                                             file_extent.ram_bytes, type,
> -                                             &file_extent);
> +               em2 = btrfs_create_dio_extent(BTRFS_I(inode), dio_data, start,
> +                                             &file_extent, type);

Same here.

The rest looks good, thanks.

>                 btrfs_dec_nocow_writers(bg);
>                 if (type == BTRFS_ORDERED_PREALLOC) {
>                         free_extent_map(em);
> --
> 2.45.0
>
>
Qu Wenruo May 23, 2024, 4:03 a.m. UTC | #2 
在 2024/5/21 02:18, Filipe Manana 写道:
> On Fri, May 3, 2024 at 7:03 AM Qu Wenruo <wqu@suse.com> wrote:
>>
>> The following 3 parameters can be cleaned up using btrfs_file_extent
>> structure:
>>
>> - len
>>    btrfs_file_extent::num_bytes
>>
>> - orig_block_len
>>    btrfs_file_extent::disk_num_bytes
>>
>> - ram_bytes
>>    btrfs_file_extent::ram_bytes
>>
>> Signed-off-by: Qu Wenruo <wqu@suse.com>
>> ---
>>   fs/btrfs/inode.c | 22 ++++++++--------------
>>   1 file changed, 8 insertions(+), 14 deletions(-)
>>
>> diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c
>> index a95dc2333972..09974c86d3d1 100644
>> --- a/fs/btrfs/inode.c
>> +++ b/fs/btrfs/inode.c
>> @@ -6969,11 +6969,8 @@ struct extent_map *btrfs_get_extent(struct btrfs_inode *inode,
>>   static struct extent_map *btrfs_create_dio_extent(struct btrfs_inode *inode,
>>                                                    struct btrfs_dio_data *dio_data,
>>                                                    const u64 start,
>> -                                                 const u64 len,
>> -                                                 const u64 orig_block_len,
>> -                                                 const u64 ram_bytes,
>> -                                                 const int type,
>> -                                                 struct btrfs_file_extent *file_extent)
>> +                                                 struct btrfs_file_extent *file_extent,
>> +                                                 const int type)
>>   {
>>          struct extent_map *em = NULL;
>>          struct btrfs_ordered_extent *ordered;
>> @@ -6991,7 +6988,7 @@ static struct extent_map *btrfs_create_dio_extent(struct btrfs_inode *inode,
>>                  if (em) {
>>                          free_extent_map(em);
>>                          btrfs_drop_extent_map_range(inode, start,
>> -                                                   start + len - 1, false);
>> +                                       start + file_extent->num_bytes - 1, false);
>>                  }
>>                  em = ERR_CAST(ordered);
>>          } else {
>> @@ -7034,10 +7031,9 @@ static struct extent_map *btrfs_new_extent_direct(struct btrfs_inode *inode,
>>          file_extent.ram_bytes = ins.offset;
>>          file_extent.offset = 0;
>>          file_extent.compression = BTRFS_COMPRESS_NONE;
>> -       em = btrfs_create_dio_extent(inode, dio_data, start, ins.offset,
>> -                                    ins.offset,
>> -                                    ins.offset, BTRFS_ORDERED_REGULAR,
>> -                                    &file_extent);
>> +       em = btrfs_create_dio_extent(inode, dio_data, start,
>> +                                    &file_extent,
>> +                                    BTRFS_ORDERED_REGULAR);
>
> As we're changing this, we can leave this in a single line as it fits.
>
>>          btrfs_dec_block_group_reservations(fs_info, ins.objectid);
>>          if (IS_ERR(em))
>>                  btrfs_free_reserved_extent(fs_info, ins.objectid, ins.offset,
>> @@ -7404,10 +7400,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,
>> -                                             file_extent.disk_num_bytes,
>> -                                             file_extent.ram_bytes, type,
>> -                                             &file_extent);
>> +               em2 = btrfs_create_dio_extent(BTRFS_I(inode), dio_data, start,
>> +                                             &file_extent, type);
>
> Same here.

Just a small question related to the single line one.

The parameter @start with its tailing ',' is already at 80 chars,
do we still need to follow the old 80 chars width recommendation?

With previous several patches, I re-checked the lines, some can indeed
be improved a little, but some BTRFS_ORDERED_* flags can not be merged
without exceeding the 80 chars limits.

Thanks,
Qu
>
> The rest looks good, thanks.
>
>>                  btrfs_dec_nocow_writers(bg);
>>                  if (type == BTRFS_ORDERED_PREALLOC) {
>>                          free_extent_map(em);
>> --
>> 2.45.0
>>
>>
>
diff mbox series

Patch

diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c
index a95dc2333972..09974c86d3d1 100644
--- a/fs/btrfs/inode.c
+++ b/fs/btrfs/inode.c
@@ -6969,11 +6969,8 @@  struct extent_map *btrfs_get_extent(struct btrfs_inode *inode,
 static struct extent_map *btrfs_create_dio_extent(struct btrfs_inode *inode,
 						  struct btrfs_dio_data *dio_data,
 						  const u64 start,
-						  const u64 len,
-						  const u64 orig_block_len,
-						  const u64 ram_bytes,
-						  const int type,
-						  struct btrfs_file_extent *file_extent)
+						  struct btrfs_file_extent *file_extent,
+						  const int type)
 {
 	struct extent_map *em = NULL;
 	struct btrfs_ordered_extent *ordered;
@@ -6991,7 +6988,7 @@  static struct extent_map *btrfs_create_dio_extent(struct btrfs_inode *inode,
 		if (em) {
 			free_extent_map(em);
 			btrfs_drop_extent_map_range(inode, start,
-						    start + len - 1, false);
+					start + file_extent->num_bytes - 1, false);
 		}
 		em = ERR_CAST(ordered);
 	} else {
@@ -7034,10 +7031,9 @@  static struct extent_map *btrfs_new_extent_direct(struct btrfs_inode *inode,
 	file_extent.ram_bytes = ins.offset;
 	file_extent.offset = 0;
 	file_extent.compression = BTRFS_COMPRESS_NONE;
-	em = btrfs_create_dio_extent(inode, dio_data, start, ins.offset,
-				     ins.offset,
-				     ins.offset, BTRFS_ORDERED_REGULAR,
-				     &file_extent);
+	em = btrfs_create_dio_extent(inode, dio_data, start,
+				     &file_extent,
+				     BTRFS_ORDERED_REGULAR);
 	btrfs_dec_block_group_reservations(fs_info, ins.objectid);
 	if (IS_ERR(em))
 		btrfs_free_reserved_extent(fs_info, ins.objectid, ins.offset,
@@ -7404,10 +7400,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,
-					      file_extent.disk_num_bytes,
-					      file_extent.ram_bytes, type,
-					      &file_extent);
+		em2 = btrfs_create_dio_extent(BTRFS_I(inode), dio_data, start,
+					      &file_extent, type);
 		btrfs_dec_nocow_writers(bg);
 		if (type == BTRFS_ORDERED_PREALLOC) {
 			free_extent_map(em);