[PATCH 0/2] misc cleanup patches

[PATCH 0/2] misc cleanup patches

[Anand Jain]

These patches are independent, not related.
They are miscellaneous cleanup patches.

Anand Jain (2):
  btrfs: cleanup fs_devices pointer usage in btrfs_trim_fs
  btrfs: change btrfs_io_bio_init inline function to macro

 fs/btrfs/extent-tree.c | 10 +++++-----
 fs/btrfs/extent_io.c   |  5 +----
 2 files changed, 6 insertions(+), 9 deletions(-)

-- 
2.31.1

[PATCH 1/2] btrfs: cleanup fs_devices pointer usage in btrfs_trim_fs

[Anand Jain]

Drop variable
  struct list_head *devices
and add new variable
  struct btrfs_fs_devices *fs_devices = fs_info->fs_devices;

so that fs_devices is used at two locations within btrfs_trim_fs() function
and also helps to access fs_devices->devices.

Signed-off-by: Anand Jain <anand.jain@oracle.com>
---
 fs/btrfs/extent-tree.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/fs/btrfs/extent-tree.c b/fs/btrfs/extent-tree.c
index 268ce58d4569..d5925bebd379 100644
--- a/fs/btrfs/extent-tree.c
+++ b/fs/btrfs/extent-tree.c
@@ -5950,9 +5950,9 @@ static int btrfs_trim_free_extents(struct btrfs_device *device, u64 *trimmed)
  */
 int btrfs_trim_fs(struct btrfs_fs_info *fs_info, struct fstrim_range *range)
 {
+	struct btrfs_fs_devices *fs_devices = fs_info->fs_devices;
 	struct btrfs_block_group *cache = NULL;
 	struct btrfs_device *device;
-	struct list_head *devices;
 	u64 group_trimmed;
 	u64 range_end = U64_MAX;
 	u64 start;
@@ -6016,9 +6016,9 @@ int btrfs_trim_fs(struct btrfs_fs_info *fs_info, struct fstrim_range *range)
 		btrfs_warn(fs_info,
 			"failed to trim %llu block group(s), last error %d",
 			bg_failed, bg_ret);
-	mutex_lock(&fs_info->fs_devices->device_list_mutex);
-	devices = &fs_info->fs_devices->devices;
-	list_for_each_entry(device, devices, dev_list) {
+
+	mutex_lock(&fs_devices->device_list_mutex);
+	list_for_each_entry(device, &fs_devices->devices, dev_list) {
 		if (test_bit(BTRFS_DEV_STATE_MISSING, &device->dev_state))
 			continue;
 
@@ -6031,7 +6031,7 @@ int btrfs_trim_fs(struct btrfs_fs_info *fs_info, struct fstrim_range *range)
 
 		trimmed += group_trimmed;
 	}
-	mutex_unlock(&fs_info->fs_devices->device_list_mutex);
+	mutex_unlock(&fs_devices->device_list_mutex);
 
 	if (dev_failed)
 		btrfs_warn(fs_info,
-- 
2.31.1

Re: [PATCH 1/2] btrfs: cleanup fs_devices pointer usage in btrfs_trim_fs

[David Sterba]

On Sun, Jul 04, 2021 at 08:04:57PM +0800, Anand Jain wrote:
> Drop variable
>   struct list_head *devices
> and add new variable
>   struct btrfs_fs_devices *fs_devices = fs_info->fs_devices;
> 
> so that fs_devices is used at two locations within btrfs_trim_fs() function
> and also helps to access fs_devices->devices.
> 
> Signed-off-by: Anand Jain <anand.jain@oracle.com>

Added to misc-next, thanks.

[PATCH 2/2] btrfs: change btrfs_io_bio_init inline function to macro

[Anand Jain]

btrfs_io_bio_init() is a single line static inline function and initializes
part of allocated struct btrfs_io_bio. Make it macro so that preprocessor
handles it and preserve the original comments of the function.

Signed-off-by: Anand Jain <anand.jain@oracle.com>
---
 fs/btrfs/extent_io.c | 5 +----
 1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/fs/btrfs/extent_io.c b/fs/btrfs/extent_io.c
index 9e81d25dea70..8ed07cffb4a4 100644
--- a/fs/btrfs/extent_io.c
+++ b/fs/btrfs/extent_io.c
@@ -3110,10 +3110,7 @@ static void end_bio_extent_readpage(struct bio *bio)
  * new bio by bio_alloc_bioset as it does not initialize the bytes outside of
  * 'bio' because use of __GFP_ZERO is not supported.
  */
-static inline void btrfs_io_bio_init(struct btrfs_io_bio *btrfs_bio)
-{
-	memset(btrfs_bio, 0, offsetof(struct btrfs_io_bio, bio));
-}
+#define btrfs_io_bio_init(bbio)	memset(bbio, 0, offsetof(struct btrfs_io_bio, bio))
 
 /*
  * The following helpers allocate a bio. As it's backed by a bioset, it'll
-- 
2.31.1

Re: [PATCH 2/2] btrfs: change btrfs_io_bio_init inline function to macro

[Nikolay Borisov]

On 4.07.21 г. 15:04, Anand Jain wrote:
> btrfs_io_bio_init() is a single line static inline function and initializes
> part of allocated struct btrfs_io_bio. Make it macro so that preprocessor
> handles it and preserve the original comments of the function.
> 
> Signed-off-by: Anand Jain <anand.jain@oracle.com>
> ---
>  fs/btrfs/extent_io.c | 5 +----
>  1 file changed, 1 insertion(+), 4 deletions(-)
> 
> diff --git a/fs/btrfs/extent_io.c b/fs/btrfs/extent_io.c
> index 9e81d25dea70..8ed07cffb4a4 100644
> --- a/fs/btrfs/extent_io.c
> +++ b/fs/btrfs/extent_io.c
> @@ -3110,10 +3110,7 @@ static void end_bio_extent_readpage(struct bio *bio)
>   * new bio by bio_alloc_bioset as it does not initialize the bytes outside of
>   * 'bio' because use of __GFP_ZERO is not supported.
>   */
> -static inline void btrfs_io_bio_init(struct btrfs_io_bio *btrfs_bio)
> -{
> -	memset(btrfs_bio, 0, offsetof(struct btrfs_io_bio, bio));
> -}
> +#define btrfs_io_bio_init(bbio)	memset(bbio, 0, offsetof(struct btrfs_io_bio, bio))
>  
>  /*
>   * The following helpers allocate a bio. As it's backed by a bioset, it'll
> 


What do we gain by this change? The compiler is perfectly able to inline
btrfs_io_bio_init.

Re: [PATCH 2/2] btrfs: change btrfs_io_bio_init inline function to macro

[Anand Jain]

On 5/7/21 10:05 pm, Nikolay Borisov wrote:
> 
> 
> On 4.07.21 г. 15:04, Anand Jain wrote:
>> btrfs_io_bio_init() is a single line static inline function and initializes
>> part of allocated struct btrfs_io_bio. Make it macro so that preprocessor
>> handles it and preserve the original comments of the function.
>>
>> Signed-off-by: Anand Jain <anand.jain@oracle.com>
>> ---
>>   fs/btrfs/extent_io.c | 5 +----
>>   1 file changed, 1 insertion(+), 4 deletions(-)
>>
>> diff --git a/fs/btrfs/extent_io.c b/fs/btrfs/extent_io.c
>> index 9e81d25dea70..8ed07cffb4a4 100644
>> --- a/fs/btrfs/extent_io.c
>> +++ b/fs/btrfs/extent_io.c
>> @@ -3110,10 +3110,7 @@ static void end_bio_extent_readpage(struct bio *bio)
>>    * new bio by bio_alloc_bioset as it does not initialize the bytes outside of
>>    * 'bio' because use of __GFP_ZERO is not supported.
>>    */
>> -static inline void btrfs_io_bio_init(struct btrfs_io_bio *btrfs_bio)
>> -{
>> -	memset(btrfs_bio, 0, offsetof(struct btrfs_io_bio, bio));
>> -}
>> +#define btrfs_io_bio_init(bbio)	memset(bbio, 0, offsetof(struct btrfs_io_bio, bio))
>>   
>>   /*
>>    * The following helpers allocate a bio. As it's backed by a bioset, it'll
>>
> 
> 
> What do we gain by this change? The compiler is perfectly able to inline
> btrfs_io_bio_init.
> 

The gain is macro is guaranteed to be inline-ed. A function with the 
inline prefix isn't.

Re: [PATCH 2/2] btrfs: change btrfs_io_bio_init inline function to macro

[Nikolay Borisov]

On 6.07.21 г. 7:34, Anand Jain wrote:
> 
> 
> On 5/7/21 10:05 pm, Nikolay Borisov wrote:
>>
>>
>> On 4.07.21 г. 15:04, Anand Jain wrote:
>>> btrfs_io_bio_init() is a single line static inline function and
>>> initializes
>>> part of allocated struct btrfs_io_bio. Make it macro so that
>>> preprocessor
>>> handles it and preserve the original comments of the function.
>>>
>>> Signed-off-by: Anand Jain <anand.jain@oracle.com>
>>> ---
>>>   fs/btrfs/extent_io.c | 5 +----
>>>   1 file changed, 1 insertion(+), 4 deletions(-)
>>>
>>> diff --git a/fs/btrfs/extent_io.c b/fs/btrfs/extent_io.c
>>> index 9e81d25dea70..8ed07cffb4a4 100644
>>> --- a/fs/btrfs/extent_io.c
>>> +++ b/fs/btrfs/extent_io.c
>>> @@ -3110,10 +3110,7 @@ static void end_bio_extent_readpage(struct bio
>>> *bio)
>>>    * new bio by bio_alloc_bioset as it does not initialize the bytes
>>> outside of
>>>    * 'bio' because use of __GFP_ZERO is not supported.
>>>    */
>>> -static inline void btrfs_io_bio_init(struct btrfs_io_bio *btrfs_bio)
>>> -{
>>> -    memset(btrfs_bio, 0, offsetof(struct btrfs_io_bio, bio));
>>> -}
>>> +#define btrfs_io_bio_init(bbio)    memset(bbio, 0, offsetof(struct
>>> btrfs_io_bio, bio))
>>>     /*
>>>    * The following helpers allocate a bio. As it's backed by a
>>> bioset, it'll
>>>
>>
>>
>> What do we gain by this change? The compiler is perfectly able to inline
>> btrfs_io_bio_init.
>>
> 
> The gain is macro is guaranteed to be inline-ed. A function with the
> inline prefix isn't.
> 

In this particular case it's guaranteed that the function will be inlined.

Re: [PATCH 2/2] btrfs: change btrfs_io_bio_init inline function to macro

[Johannes Thumshirn]

On 06/07/2021 07:29, Nikolay Borisov wrote:
>> The gain is macro is guaranteed to be inline-ed. A function with the
>> inline prefix isn't.
>>
> In this particular case it's guaranteed that the function will be inlined.
> 

And we also get additional type safety from the function, which the macro
doesn't provide.

Re: [PATCH 2/2] btrfs: change btrfs_io_bio_init inline function to macro

[David Sterba]

On Tue, Jul 06, 2021 at 06:14:38AM +0000, Johannes Thumshirn wrote:
> On 06/07/2021 07:29, Nikolay Borisov wrote:
> >> The gain is macro is guaranteed to be inline-ed. A function with the
> >> inline prefix isn't.
> >>
> > In this particular case it's guaranteed that the function will be inlined.
> > 
> 
> And we also get additional type safety from the function, which the macro
> doesn't provide.

Yeah, using static inlines instead of macros is preferred when they're
reasonably simple, like in this case.