diff mbox series

[4/7] btrfs: Don't opencode is_data_inode in end_bio_extent_readpage

Message ID 20200918133439.23187-5-nborisov@suse.com (mailing list archive)
State New, archived
Headers show
Series Remove struct extent_io_ops | expand

Commit Message

Nikolay Borisov Sept. 18, 2020, 1:34 p.m. UTC
Use the is_data_inode helper.

Signed-off-by: Nikolay Borisov <nborisov@suse.com>
---
 fs/btrfs/extent_io.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

Comments

David Sterba Sept. 21, 2020, 8:29 p.m. UTC | #1
On Fri, Sep 18, 2020 at 04:34:36PM +0300, Nikolay Borisov wrote:
> Use the is_data_inode helper.
> 
> Signed-off-by: Nikolay Borisov <nborisov@suse.com>
> ---
>  fs/btrfs/extent_io.c | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
> 
> diff --git a/fs/btrfs/extent_io.c b/fs/btrfs/extent_io.c
> index 6e976bd86600..26b002e2f3b3 100644
> --- a/fs/btrfs/extent_io.c
> +++ b/fs/btrfs/extent_io.c
> @@ -2816,8 +2816,7 @@ static void end_bio_extent_readpage(struct bio *bio)
>  		struct page *page = bvec->bv_page;
>  		struct inode *inode = page->mapping->host;
>  		struct btrfs_fs_info *fs_info = btrfs_sb(inode->i_sb);
> -		bool data_inode = btrfs_ino(BTRFS_I(inode))
> -			!= BTRFS_BTREE_INODE_OBJECTID;
> +		bool data_inode = is_data_inode(inode);

I think you can remove the temporary variable and call is_data_inode
directly in the later code, there's only one use.
Nikolay Borisov Sept. 23, 2020, 6:23 a.m. UTC | #2
On 21.09.20 г. 23:29 ч., David Sterba wrote:
> On Fri, Sep 18, 2020 at 04:34:36PM +0300, Nikolay Borisov wrote:
>> Use the is_data_inode helper.
>>
>> Signed-off-by: Nikolay Borisov <nborisov@suse.com>
>> ---
>>  fs/btrfs/extent_io.c | 3 +--
>>  1 file changed, 1 insertion(+), 2 deletions(-)
>>
>> diff --git a/fs/btrfs/extent_io.c b/fs/btrfs/extent_io.c
>> index 6e976bd86600..26b002e2f3b3 100644
>> --- a/fs/btrfs/extent_io.c
>> +++ b/fs/btrfs/extent_io.c
>> @@ -2816,8 +2816,7 @@ static void end_bio_extent_readpage(struct bio *bio)
>>  		struct page *page = bvec->bv_page;
>>  		struct inode *inode = page->mapping->host;
>>  		struct btrfs_fs_info *fs_info = btrfs_sb(inode->i_sb);
>> -		bool data_inode = btrfs_ino(BTRFS_I(inode))
>> -			!= BTRFS_BTREE_INODE_OBJECTID;
>> +		bool data_inode = is_data_inode(inode);
> 
> I think you can remove the temporary variable and call is_data_inode
> directly in the later code, there's only one use.
> 

Actually it's used twice, yet I did an experiment to remove it and
bloat-o-meter indicates it's a win to call the inline function twice:


add/remove: 0/0 grow/shrink: 0/2 up/down: 0/-161 (-161)
Function                                     old     new   delta
end_bio_extent_readpage.cold                 117     104     -13
end_bio_extent_readpage                     1614    1466    -148
Total: Before=45527, After=45366, chg -0.35%


diff --git a/fs/btrfs/extent_io.c b/fs/btrfs/extent_io.c
index b5e00b62bcb1..0994fb56e39a 100644
--- a/fs/btrfs/extent_io.c
+++ b/fs/btrfs/extent_io.c
@@ -2819,7 +2819,6 @@ static void end_bio_extent_readpage(struct bio *bio)
                struct page *page = bvec->bv_page;
                struct inode *inode = page->mapping->host;
                struct btrfs_fs_info *fs_info = btrfs_sb(inode->i_sb);
-               bool data_inode = is_data_inode(inode);

                btrfs_debug(fs_info,
                        "end_bio_extent_readpage: bi_sector=%llu,
err=%d, mirror=%u",
@@ -2850,7 +2849,7 @@ static void end_bio_extent_readpage(struct bio *bio)

                mirror = io_bio->mirror_num;
                if (likely(uptodate)) {
-                       if (data_inode)
+                       if (is_data_inode(inode))
                                ret = btrfs_check_csum(io_bio, offset, page,
                                                       start, end, mirror);
                        else
@@ -2868,7 +2867,7 @@ static void end_bio_extent_readpage(struct bio *bio)
                if (likely(uptodate))
                        goto readpage_ok;

-               if (data_inode) {
+               if (is_data_inode(inode)) {

                        /*
                         * The generic bio_readpage_error handles errors the



So I will fold this into the next version.
David Sterba Sept. 23, 2020, 2:11 p.m. UTC | #3
On Wed, Sep 23, 2020 at 09:23:48AM +0300, Nikolay Borisov wrote:
> 
> 
> On 21.09.20 г. 23:29 ч., David Sterba wrote:
> > On Fri, Sep 18, 2020 at 04:34:36PM +0300, Nikolay Borisov wrote:
> >> Use the is_data_inode helper.
> >>
> >> Signed-off-by: Nikolay Borisov <nborisov@suse.com>
> >> ---
> >>  fs/btrfs/extent_io.c | 3 +--
> >>  1 file changed, 1 insertion(+), 2 deletions(-)
> >>
> >> diff --git a/fs/btrfs/extent_io.c b/fs/btrfs/extent_io.c
> >> index 6e976bd86600..26b002e2f3b3 100644
> >> --- a/fs/btrfs/extent_io.c
> >> +++ b/fs/btrfs/extent_io.c
> >> @@ -2816,8 +2816,7 @@ static void end_bio_extent_readpage(struct bio *bio)
> >>  		struct page *page = bvec->bv_page;
> >>  		struct inode *inode = page->mapping->host;
> >>  		struct btrfs_fs_info *fs_info = btrfs_sb(inode->i_sb);
> >> -		bool data_inode = btrfs_ino(BTRFS_I(inode))
> >> -			!= BTRFS_BTREE_INODE_OBJECTID;
> >> +		bool data_inode = is_data_inode(inode);
> > 
> > I think you can remove the temporary variable and call is_data_inode
> > directly in the later code, there's only one use.
> > 
> 
> Actually it's used twice, yet I did an experiment to remove it and
> bloat-o-meter indicates it's a win to call the inline function twice:

My oversight, sorry.

> add/remove: 0/0 grow/shrink: 0/2 up/down: 0/-161 (-161)
> Function                                     old     new   delta
> end_bio_extent_readpage.cold                 117     104     -13
> end_bio_extent_readpage                     1614    1466    -148
> Total: Before=45527, After=45366, chg -0.35%

Ok, I'll update it to call the helper.
diff mbox series

Patch

diff --git a/fs/btrfs/extent_io.c b/fs/btrfs/extent_io.c
index 6e976bd86600..26b002e2f3b3 100644
--- a/fs/btrfs/extent_io.c
+++ b/fs/btrfs/extent_io.c
@@ -2816,8 +2816,7 @@  static void end_bio_extent_readpage(struct bio *bio)
 		struct page *page = bvec->bv_page;
 		struct inode *inode = page->mapping->host;
 		struct btrfs_fs_info *fs_info = btrfs_sb(inode->i_sb);
-		bool data_inode = btrfs_ino(BTRFS_I(inode))
-			!= BTRFS_BTREE_INODE_OBJECTID;
+		bool data_inode = is_data_inode(inode);
 
 		btrfs_debug(fs_info,
 			"end_bio_extent_readpage: bi_sector=%llu, err=%d, mirror=%u",