[PATCH 0/4] Minor helper and struct member cleanups

[PATCH 0/4] Minor helper and struct member cleanups

[David Sterba]

Assorted updates to code vaguely related to the bio callbacks and
structures. Remove call indirection, remove redundant struct members,
etc.

David Sterba (4):
  btrfs: merge btrfs_submit_bio_done to its caller
  btrfs: replace async_cow::root with fs_info
  btrfs: remove redundant csum buffer in btrfs_io_bio
  btrfs: replace btrfs_io_bio::end_io with a simple helper

 fs/btrfs/disk-io.c   | 18 +++++++++++++++++-
 fs/btrfs/extent_io.c |  3 +--
 fs/btrfs/file-item.c | 13 +++----------
 fs/btrfs/inode.c     | 39 +++++----------------------------------
 fs/btrfs/volumes.h   | 11 ++++++++---
 5 files changed, 34 insertions(+), 50 deletions(-)

-- 
2.19.1

[PATCH 1/4] btrfs: merge btrfs_submit_bio_done to its caller

[David Sterba]

There's one caller and its code is simple, we can open code it in
run_one_async_done. The errors are passed through bio.

Signed-off-by: David Sterba <dsterba@suse.com>
---
 fs/btrfs/disk-io.c | 18 +++++++++++++++++-
 fs/btrfs/inode.c   | 23 -----------------------
 2 files changed, 17 insertions(+), 24 deletions(-)

diff --git a/fs/btrfs/disk-io.c b/fs/btrfs/disk-io.c
index feb67dfd663d..2f5c5442cf00 100644
--- a/fs/btrfs/disk-io.c
+++ b/fs/btrfs/disk-io.c
@@ -764,11 +764,22 @@ static void run_one_async_start(struct btrfs_work *work)
 		async->status = ret;
 }
 
+/*
+ * In order to insert checksums into the metadata in large chunks, we wait
+ * until bio submission time.   All the pages in the bio are checksummed and
+ * sums are attached onto the ordered extent record.
+ *
+ * At IO completion time the csums attached on the ordered extent record are
+ * inserted into the tree.
+ */
 static void run_one_async_done(struct btrfs_work *work)
 {
 	struct async_submit_bio *async;
+	struct inode *inode;
+	blk_status_t ret;
 
 	async = container_of(work, struct  async_submit_bio, work);
+	inode = async->private_data;
 
 	/* If an error occurred we just want to clean up the bio and move on */
 	if (async->status) {
@@ -777,7 +788,12 @@ static void run_one_async_done(struct btrfs_work *work)
 		return;
 	}
 
-	btrfs_submit_bio_done(async->private_data, async->bio, async->mirror_num);
+	ret = btrfs_map_bio(btrfs_sb(inode->i_sb), async->bio,
+			async->mirror_num, 1);
+	if (ret) {
+		async->bio->bi_status = ret;
+		bio_endio(async->bio);
+	}
 }
 
 static void run_one_async_free(struct btrfs_work *work)
diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c
index 9becf8543489..a88122b89f50 100644
--- a/fs/btrfs/inode.c
+++ b/fs/btrfs/inode.c
@@ -1924,29 +1924,6 @@ static blk_status_t btrfs_submit_bio_start(void *private_data, struct bio *bio,
 	return 0;
 }
 
-/*
- * in order to insert checksums into the metadata in large chunks,
- * we wait until bio submission time.   All the pages in the bio are
- * checksummed and sums are attached onto the ordered extent record.
- *
- * At IO completion time the cums attached on the ordered extent record
- * are inserted into the btree
- */
-blk_status_t btrfs_submit_bio_done(void *private_data, struct bio *bio,
-			  int mirror_num)
-{
-	struct inode *inode = private_data;
-	struct btrfs_fs_info *fs_info = btrfs_sb(inode->i_sb);
-	blk_status_t ret;
-
-	ret = btrfs_map_bio(fs_info, bio, mirror_num, 1);
-	if (ret) {
-		bio->bi_status = ret;
-		bio_endio(bio);
-	}
-	return ret;
-}
-
 /*
  * extent_io.c submission hook. This does the right thing for csum calculation
  * on write, or reading the csums from the tree before a read.
-- 
2.19.1

Re: [PATCH 1/4] btrfs: merge btrfs_submit_bio_done to its caller

[Nikolay Borisov]

On 22.11.18 г. 18:16 ч., David Sterba wrote:
> There's one caller and its code is simple, we can open code it in
> run_one_async_done. The errors are passed through bio.
> 
> Signed-off-by: David Sterba <dsterba@suse.com>

Reviewed-by: Nikolay Borisov <nborisov@suse.com>
> ---
>  fs/btrfs/disk-io.c | 18 +++++++++++++++++-
>  fs/btrfs/inode.c   | 23 -----------------------
>  2 files changed, 17 insertions(+), 24 deletions(-)
> 
> diff --git a/fs/btrfs/disk-io.c b/fs/btrfs/disk-io.c
> index feb67dfd663d..2f5c5442cf00 100644
> --- a/fs/btrfs/disk-io.c
> +++ b/fs/btrfs/disk-io.c
> @@ -764,11 +764,22 @@ static void run_one_async_start(struct btrfs_work *work)
>  		async->status = ret;
>  }
>  
> +/*
> + * In order to insert checksums into the metadata in large chunks, we wait
> + * until bio submission time.   All the pages in the bio are checksummed and
> + * sums are attached onto the ordered extent record.
> + *
> + * At IO completion time the csums attached on the ordered extent record are
> + * inserted into the tree.
> + */
>  static void run_one_async_done(struct btrfs_work *work)
>  {
>  	struct async_submit_bio *async;
> +	struct inode *inode;
> +	blk_status_t ret;
>  
>  	async = container_of(work, struct  async_submit_bio, work);
> +	inode = async->private_data;
>  
>  	/* If an error occurred we just want to clean up the bio and move on */
>  	if (async->status) {
> @@ -777,7 +788,12 @@ static void run_one_async_done(struct btrfs_work *work)
>  		return;
>  	}
>  
> -	btrfs_submit_bio_done(async->private_data, async->bio, async->mirror_num);
> +	ret = btrfs_map_bio(btrfs_sb(inode->i_sb), async->bio,
> +			async->mirror_num, 1);
> +	if (ret) {
> +		async->bio->bi_status = ret;
> +		bio_endio(async->bio);
> +	}
>  }
>  
>  static void run_one_async_free(struct btrfs_work *work)
> diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c
> index 9becf8543489..a88122b89f50 100644
> --- a/fs/btrfs/inode.c
> +++ b/fs/btrfs/inode.c
> @@ -1924,29 +1924,6 @@ static blk_status_t btrfs_submit_bio_start(void *private_data, struct bio *bio,
>  	return 0;
>  }
>  
> -/*
> - * in order to insert checksums into the metadata in large chunks,
> - * we wait until bio submission time.   All the pages in the bio are
> - * checksummed and sums are attached onto the ordered extent record.
> - *
> - * At IO completion time the cums attached on the ordered extent record
> - * are inserted into the btree
> - */
> -blk_status_t btrfs_submit_bio_done(void *private_data, struct bio *bio,
> -			  int mirror_num)
> -{
> -	struct inode *inode = private_data;
> -	struct btrfs_fs_info *fs_info = btrfs_sb(inode->i_sb);
> -	blk_status_t ret;
> -
> -	ret = btrfs_map_bio(fs_info, bio, mirror_num, 1);
> -	if (ret) {
> -		bio->bi_status = ret;
> -		bio_endio(bio);
> -	}
> -	return ret;
> -}
> -
>  /*
>   * extent_io.c submission hook. This does the right thing for csum calculation
>   * on write, or reading the csums from the tree before a read.
> 

Re: [PATCH 1/4] btrfs: merge btrfs_submit_bio_done to its caller

[Johannes Thumshirn]

Looks good,
Reviewed-by: Johannes Thumshirn <jthumshirn@suse.de>
-- 
Johannes Thumshirn                                        SUSE Labs
jthumshirn@suse.de                                +49 911 74053 689
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: Felix Imendörffer, Jane Smithard, Graham Norton
HRB 21284 (AG Nürnberg)
Key fingerprint = EC38 9CAB C2C4 F25D 8600 D0D0 0393 969D 2D76 0850

[PATCH 2/4] btrfs: replace async_cow::root with fs_info

[David Sterba]

The async_cow::root is used to propagate fs_info to async_cow_submit.
We can't use inode to reach it because it could become NULL after
write without compression in async_cow_start.

Signed-off-by: David Sterba <dsterba@suse.com>
---
 fs/btrfs/inode.c | 9 +++------
 1 file changed, 3 insertions(+), 6 deletions(-)

diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c
index a88122b89f50..26b8bec7c2dc 100644
--- a/fs/btrfs/inode.c
+++ b/fs/btrfs/inode.c
@@ -358,7 +358,7 @@ struct async_extent {
 
 struct async_cow {
 	struct inode *inode;
-	struct btrfs_root *root;
+	struct btrfs_fs_info *fs_info;
 	struct page *locked_page;
 	u64 start;
 	u64 end;
@@ -1144,13 +1144,11 @@ static noinline void async_cow_submit(struct btrfs_work *work)
 {
 	struct btrfs_fs_info *fs_info;
 	struct async_cow *async_cow;
-	struct btrfs_root *root;
 	unsigned long nr_pages;
 
 	async_cow = container_of(work, struct async_cow, work);
 
-	root = async_cow->root;
-	fs_info = root->fs_info;
+	fs_info = async_cow->fs_info;
 	nr_pages = (async_cow->end - async_cow->start + PAGE_SIZE) >>
 		PAGE_SHIFT;
 
@@ -1179,7 +1177,6 @@ static int cow_file_range_async(struct inode *inode, struct page *locked_page,
 {
 	struct btrfs_fs_info *fs_info = btrfs_sb(inode->i_sb);
 	struct async_cow *async_cow;
-	struct btrfs_root *root = BTRFS_I(inode)->root;
 	unsigned long nr_pages;
 	u64 cur_end;
 
@@ -1189,7 +1186,7 @@ static int cow_file_range_async(struct inode *inode, struct page *locked_page,
 		async_cow = kmalloc(sizeof(*async_cow), GFP_NOFS);
 		BUG_ON(!async_cow); /* -ENOMEM */
 		async_cow->inode = igrab(inode);
-		async_cow->root = root;
+		async_cow->fs_info = fs_info;
 		async_cow->locked_page = locked_page;
 		async_cow->start = start;
 		async_cow->write_flags = write_flags;
-- 
2.19.1

Re: [PATCH 2/4] btrfs: replace async_cow::root with fs_info

[Nikolay Borisov]

On 22.11.18 г. 18:16 ч., David Sterba wrote:
> The async_cow::root is used to propagate fs_info to async_cow_submit.
> We can't use inode to reach it because it could become NULL after
> write without compression in async_cow_start.
> 
> Signed-off-by: David Sterba <dsterba@suse.com>

Reviewed-by: Nikolay Borisov <nborisov@suse.com>

> ---
>  fs/btrfs/inode.c | 9 +++------
>  1 file changed, 3 insertions(+), 6 deletions(-)
> 
> diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c
> index a88122b89f50..26b8bec7c2dc 100644
> --- a/fs/btrfs/inode.c
> +++ b/fs/btrfs/inode.c
> @@ -358,7 +358,7 @@ struct async_extent {
>  
>  struct async_cow {
>  	struct inode *inode;
> -	struct btrfs_root *root;
> +	struct btrfs_fs_info *fs_info;
>  	struct page *locked_page;
>  	u64 start;
>  	u64 end;
> @@ -1144,13 +1144,11 @@ static noinline void async_cow_submit(struct btrfs_work *work)
>  {
>  	struct btrfs_fs_info *fs_info;
>  	struct async_cow *async_cow;
> -	struct btrfs_root *root;
>  	unsigned long nr_pages;
>  
>  	async_cow = container_of(work, struct async_cow, work);
>  
> -	root = async_cow->root;
> -	fs_info = root->fs_info;
> +	fs_info = async_cow->fs_info;
>  	nr_pages = (async_cow->end - async_cow->start + PAGE_SIZE) >>
>  		PAGE_SHIFT;
>  
> @@ -1179,7 +1177,6 @@ static int cow_file_range_async(struct inode *inode, struct page *locked_page,
>  {
>  	struct btrfs_fs_info *fs_info = btrfs_sb(inode->i_sb);
>  	struct async_cow *async_cow;
> -	struct btrfs_root *root = BTRFS_I(inode)->root;
>  	unsigned long nr_pages;
>  	u64 cur_end;
>  
> @@ -1189,7 +1186,7 @@ static int cow_file_range_async(struct inode *inode, struct page *locked_page,
>  		async_cow = kmalloc(sizeof(*async_cow), GFP_NOFS);
>  		BUG_ON(!async_cow); /* -ENOMEM */
>  		async_cow->inode = igrab(inode);
> -		async_cow->root = root;
> +		async_cow->fs_info = fs_info;
>  		async_cow->locked_page = locked_page;
>  		async_cow->start = start;
>  		async_cow->write_flags = write_flags;
> 

Re: [PATCH 2/4] btrfs: replace async_cow::root with fs_info

[Johannes Thumshirn]

Looks good,
Reviewed-by: Johannes Thumshirn <jthumshirn@suse.de>
-- 
Johannes Thumshirn                                        SUSE Labs
jthumshirn@suse.de                                +49 911 74053 689
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: Felix Imendörffer, Jane Smithard, Graham Norton
HRB 21284 (AG Nürnberg)
Key fingerprint = EC38 9CAB C2C4 F25D 8600 D0D0 0393 969D 2D76 0850

[PATCH 3/4] btrfs: remove redundant csum buffer in btrfs_io_bio

[David Sterba]

The io_bio tracks checksums and has an inline buffer or an allocated
one. And there's a third member that points to the right one, but we
don't need to use an extra pointer for that. Let btrfs_io_bio::csum
point to the right buffer and check that the inline buffer is not
accidentally freed.

This shrinks struct btrfs_io_bio by 8 bytes.

Signed-off-by: David Sterba <dsterba@suse.com>
---
 fs/btrfs/file-item.c | 12 +++++++-----
 fs/btrfs/volumes.h   |  1 -
 2 files changed, 7 insertions(+), 6 deletions(-)

diff --git a/fs/btrfs/file-item.c b/fs/btrfs/file-item.c
index ba74827beb32..1f2d0a6ab634 100644
--- a/fs/btrfs/file-item.c
+++ b/fs/btrfs/file-item.c
@@ -144,7 +144,10 @@ int btrfs_lookup_file_extent(struct btrfs_trans_handle *trans,
 
 static void btrfs_io_bio_endio_readpage(struct btrfs_io_bio *bio, int err)
 {
-	kfree(bio->csum_allocated);
+	if (bio->csum != bio->csum_inline) {
+		kfree(bio->csum);
+		bio->csum = NULL;
+	}
 }
 
 static blk_status_t __btrfs_lookup_bio_sums(struct inode *inode, struct bio *bio,
@@ -175,13 +178,12 @@ static blk_status_t __btrfs_lookup_bio_sums(struct inode *inode, struct bio *bio
 	nblocks = bio->bi_iter.bi_size >> inode->i_sb->s_blocksize_bits;
 	if (!dst) {
 		if (nblocks * csum_size > BTRFS_BIO_INLINE_CSUM_SIZE) {
-			btrfs_bio->csum_allocated = kmalloc_array(nblocks,
-					csum_size, GFP_NOFS);
-			if (!btrfs_bio->csum_allocated) {
+			btrfs_bio->csum = kmalloc_array(nblocks, csum_size,
+							GFP_NOFS);
+			if (!btrfs_bio->csum) {
 				btrfs_free_path(path);
 				return BLK_STS_RESOURCE;
 			}
-			btrfs_bio->csum = btrfs_bio->csum_allocated;
 			btrfs_bio->end_io = btrfs_io_bio_endio_readpage;
 		} else {
 			btrfs_bio->csum = btrfs_bio->csum_inline;
diff --git a/fs/btrfs/volumes.h b/fs/btrfs/volumes.h
index 1d936ce282c3..9a764f2d462e 100644
--- a/fs/btrfs/volumes.h
+++ b/fs/btrfs/volumes.h
@@ -274,7 +274,6 @@ struct btrfs_io_bio {
 	u64 logical;
 	u8 *csum;
 	u8 csum_inline[BTRFS_BIO_INLINE_CSUM_SIZE];
-	u8 *csum_allocated;
 	btrfs_io_bio_end_io_t *end_io;
 	struct bvec_iter iter;
 	/*
-- 
2.19.1

Re: [PATCH 3/4] btrfs: remove redundant csum buffer in btrfs_io_bio

[Nikolay Borisov]

On 22.11.18 г. 18:16 ч., David Sterba wrote:
> The io_bio tracks checksums and has an inline buffer or an allocated
> one. And there's a third member that points to the right one, but we
> don't need to use an extra pointer for that. Let btrfs_io_bio::csum
> point to the right buffer and check that the inline buffer is not
> accidentally freed.
> 
> This shrinks struct btrfs_io_bio by 8 bytes.
> 
> Signed-off-by: David Sterba <dsterba@suse.com>

Reviewed-by: Nikolay Borisov <nborisov@suse.com>

> ---
>  fs/btrfs/file-item.c | 12 +++++++-----
>  fs/btrfs/volumes.h   |  1 -
>  2 files changed, 7 insertions(+), 6 deletions(-)
> 
> diff --git a/fs/btrfs/file-item.c b/fs/btrfs/file-item.c
> index ba74827beb32..1f2d0a6ab634 100644
> --- a/fs/btrfs/file-item.c
> +++ b/fs/btrfs/file-item.c
> @@ -144,7 +144,10 @@ int btrfs_lookup_file_extent(struct btrfs_trans_handle *trans,
>  
>  static void btrfs_io_bio_endio_readpage(struct btrfs_io_bio *bio, int err)
>  {
> -	kfree(bio->csum_allocated);
> +	if (bio->csum != bio->csum_inline) {
> +		kfree(bio->csum);
> +		bio->csum = NULL;
> +	}
>  }
>  
>  static blk_status_t __btrfs_lookup_bio_sums(struct inode *inode, struct bio *bio,
> @@ -175,13 +178,12 @@ static blk_status_t __btrfs_lookup_bio_sums(struct inode *inode, struct bio *bio
>  	nblocks = bio->bi_iter.bi_size >> inode->i_sb->s_blocksize_bits;
>  	if (!dst) {
>  		if (nblocks * csum_size > BTRFS_BIO_INLINE_CSUM_SIZE) {
> -			btrfs_bio->csum_allocated = kmalloc_array(nblocks,
> -					csum_size, GFP_NOFS);
> -			if (!btrfs_bio->csum_allocated) {
> +			btrfs_bio->csum = kmalloc_array(nblocks, csum_size,
> +							GFP_NOFS);
> +			if (!btrfs_bio->csum) {
>  				btrfs_free_path(path);
>  				return BLK_STS_RESOURCE;
>  			}
> -			btrfs_bio->csum = btrfs_bio->csum_allocated;
>  			btrfs_bio->end_io = btrfs_io_bio_endio_readpage;
>  		} else {
>  			btrfs_bio->csum = btrfs_bio->csum_inline;
> diff --git a/fs/btrfs/volumes.h b/fs/btrfs/volumes.h
> index 1d936ce282c3..9a764f2d462e 100644
> --- a/fs/btrfs/volumes.h
> +++ b/fs/btrfs/volumes.h
> @@ -274,7 +274,6 @@ struct btrfs_io_bio {
>  	u64 logical;
>  	u8 *csum;
>  	u8 csum_inline[BTRFS_BIO_INLINE_CSUM_SIZE];
> -	u8 *csum_allocated;
>  	btrfs_io_bio_end_io_t *end_io;
>  	struct bvec_iter iter;
>  	/*
> 

Re: [PATCH 3/4] btrfs: remove redundant csum buffer in btrfs_io_bio

[Johannes Thumshirn]

Looks good,
Reviewed-by: Johannes Thumshirn <jthumshirn@suse.de>
-- 
Johannes Thumshirn                                        SUSE Labs
jthumshirn@suse.de                                +49 911 74053 689
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: Felix Imendörffer, Jane Smithard, Graham Norton
HRB 21284 (AG Nürnberg)
Key fingerprint = EC38 9CAB C2C4 F25D 8600 D0D0 0393 969D 2D76 0850

[PATCH 4/4] btrfs: replace btrfs_io_bio::end_io with a simple helper

[David Sterba]

The end_io callback implemented as btrfs_io_bio_endio_readpage only
calls kfree. Also the callback is set only in case the csum buffer is
allocated and not pointing to the inline buffer. We can use that
information to drop the indirection and call a helper that will free the
csums only in the right case.

This shrinks struct btrfs_io_bio by 8 bytes.

Signed-off-by: David Sterba <dsterba@suse.com>
---
 fs/btrfs/extent_io.c |  3 +--
 fs/btrfs/file-item.c |  9 ---------
 fs/btrfs/inode.c     |  7 ++-----
 fs/btrfs/volumes.h   | 10 ++++++++--
 4 files changed, 11 insertions(+), 18 deletions(-)

diff --git a/fs/btrfs/extent_io.c b/fs/btrfs/extent_io.c
index 4ea808d6cfbc..aef3c9866ff0 100644
--- a/fs/btrfs/extent_io.c
+++ b/fs/btrfs/extent_io.c
@@ -2623,8 +2623,7 @@ static void end_bio_extent_readpage(struct bio *bio)
 	if (extent_len)
 		endio_readpage_release_extent(tree, extent_start, extent_len,
 					      uptodate);
-	if (io_bio->end_io)
-		io_bio->end_io(io_bio, blk_status_to_errno(bio->bi_status));
+	btrfs_io_bio_free_csum(io_bio);
 	bio_put(bio);
 }
 
diff --git a/fs/btrfs/file-item.c b/fs/btrfs/file-item.c
index 1f2d0a6ab634..920bf3b4b0ef 100644
--- a/fs/btrfs/file-item.c
+++ b/fs/btrfs/file-item.c
@@ -142,14 +142,6 @@ int btrfs_lookup_file_extent(struct btrfs_trans_handle *trans,
 	return ret;
 }
 
-static void btrfs_io_bio_endio_readpage(struct btrfs_io_bio *bio, int err)
-{
-	if (bio->csum != bio->csum_inline) {
-		kfree(bio->csum);
-		bio->csum = NULL;
-	}
-}
-
 static blk_status_t __btrfs_lookup_bio_sums(struct inode *inode, struct bio *bio,
 				   u64 logical_offset, u32 *dst, int dio)
 {
@@ -184,7 +176,6 @@ static blk_status_t __btrfs_lookup_bio_sums(struct inode *inode, struct bio *bio
 				btrfs_free_path(path);
 				return BLK_STS_RESOURCE;
 			}
-			btrfs_bio->end_io = btrfs_io_bio_endio_readpage;
 		} else {
 			btrfs_bio->csum = btrfs_bio->csum_inline;
 		}
diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c
index 26b8bec7c2dc..6bfd37e58924 100644
--- a/fs/btrfs/inode.c
+++ b/fs/btrfs/inode.c
@@ -8017,9 +8017,7 @@ static void btrfs_endio_direct_read(struct bio *bio)
 
 	dio_bio->bi_status = err;
 	dio_end_io(dio_bio);
-
-	if (io_bio->end_io)
-		io_bio->end_io(io_bio, blk_status_to_errno(err));
+	btrfs_io_bio_free_csum(io_bio);
 	bio_put(bio);
 }
 
@@ -8372,8 +8370,7 @@ static void btrfs_submit_direct(struct bio *dio_bio, struct inode *inode,
 	if (!ret)
 		return;
 
-	if (io_bio->end_io)
-		io_bio->end_io(io_bio, ret);
+	btrfs_io_bio_free_csum(io_bio);
 
 free_ordered:
 	/*
diff --git a/fs/btrfs/volumes.h b/fs/btrfs/volumes.h
index 9a764f2d462e..a13045fcfc45 100644
--- a/fs/btrfs/volumes.h
+++ b/fs/btrfs/volumes.h
@@ -267,14 +267,12 @@ struct btrfs_fs_devices {
  * we allocate are actually btrfs_io_bios.  We'll cram as much of
  * struct btrfs_bio as we can into this over time.
  */
-typedef void (btrfs_io_bio_end_io_t) (struct btrfs_io_bio *bio, int err);
 struct btrfs_io_bio {
 	unsigned int mirror_num;
 	unsigned int stripe_index;
 	u64 logical;
 	u8 *csum;
 	u8 csum_inline[BTRFS_BIO_INLINE_CSUM_SIZE];
-	btrfs_io_bio_end_io_t *end_io;
 	struct bvec_iter iter;
 	/*
 	 * This member must come last, bio_alloc_bioset will allocate enough
@@ -288,6 +286,14 @@ static inline struct btrfs_io_bio *btrfs_io_bio(struct bio *bio)
 	return container_of(bio, struct btrfs_io_bio, bio);
 }
 
+static inline void btrfs_io_bio_free_csum(struct btrfs_io_bio *io_bio)
+{
+	if (io_bio->csum != io_bio->csum_inline) {
+		kfree(io_bio->csum);
+		io_bio->csum = NULL;
+	}
+}
+
 struct btrfs_bio_stripe {
 	struct btrfs_device *dev;
 	u64 physical;
-- 
2.19.1

Re: [PATCH 4/4] btrfs: replace btrfs_io_bio::end_io with a simple helper

[Nikolay Borisov]

On 22.11.18 г. 18:16 ч., David Sterba wrote:
> The end_io callback implemented as btrfs_io_bio_endio_readpage only
> calls kfree. Also the callback is set only in case the csum buffer is
> allocated and not pointing to the inline buffer. We can use that
> information to drop the indirection and call a helper that will free the
> csums only in the right case.
> 
> This shrinks struct btrfs_io_bio by 8 bytes.
> 
> Signed-off-by: David Sterba <dsterba@suse.com>

Reviewed-by: Nikolay Borisov <nborisov@suse.com>

> ---
>  fs/btrfs/extent_io.c |  3 +--
>  fs/btrfs/file-item.c |  9 ---------
>  fs/btrfs/inode.c     |  7 ++-----
>  fs/btrfs/volumes.h   | 10 ++++++++--
>  4 files changed, 11 insertions(+), 18 deletions(-)
> 
> diff --git a/fs/btrfs/extent_io.c b/fs/btrfs/extent_io.c
> index 4ea808d6cfbc..aef3c9866ff0 100644
> --- a/fs/btrfs/extent_io.c
> +++ b/fs/btrfs/extent_io.c
> @@ -2623,8 +2623,7 @@ static void end_bio_extent_readpage(struct bio *bio)
>  	if (extent_len)
>  		endio_readpage_release_extent(tree, extent_start, extent_len,
>  					      uptodate);
> -	if (io_bio->end_io)
> -		io_bio->end_io(io_bio, blk_status_to_errno(bio->bi_status));
> +	btrfs_io_bio_free_csum(io_bio);
>  	bio_put(bio);
>  }
>  
> diff --git a/fs/btrfs/file-item.c b/fs/btrfs/file-item.c
> index 1f2d0a6ab634..920bf3b4b0ef 100644
> --- a/fs/btrfs/file-item.c
> +++ b/fs/btrfs/file-item.c
> @@ -142,14 +142,6 @@ int btrfs_lookup_file_extent(struct btrfs_trans_handle *trans,
>  	return ret;
>  }
>  
> -static void btrfs_io_bio_endio_readpage(struct btrfs_io_bio *bio, int err)
> -{
> -	if (bio->csum != bio->csum_inline) {
> -		kfree(bio->csum);
> -		bio->csum = NULL;
> -	}
> -}
> -
>  static blk_status_t __btrfs_lookup_bio_sums(struct inode *inode, struct bio *bio,
>  				   u64 logical_offset, u32 *dst, int dio)
>  {
> @@ -184,7 +176,6 @@ static blk_status_t __btrfs_lookup_bio_sums(struct inode *inode, struct bio *bio
>  				btrfs_free_path(path);
>  				return BLK_STS_RESOURCE;
>  			}
> -			btrfs_bio->end_io = btrfs_io_bio_endio_readpage;
>  		} else {
>  			btrfs_bio->csum = btrfs_bio->csum_inline;
>  		}
> diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c
> index 26b8bec7c2dc..6bfd37e58924 100644
> --- a/fs/btrfs/inode.c
> +++ b/fs/btrfs/inode.c
> @@ -8017,9 +8017,7 @@ static void btrfs_endio_direct_read(struct bio *bio)
>  
>  	dio_bio->bi_status = err;
>  	dio_end_io(dio_bio);
> -
> -	if (io_bio->end_io)
> -		io_bio->end_io(io_bio, blk_status_to_errno(err));
> +	btrfs_io_bio_free_csum(io_bio);
>  	bio_put(bio);
>  }
>  
> @@ -8372,8 +8370,7 @@ static void btrfs_submit_direct(struct bio *dio_bio, struct inode *inode,
>  	if (!ret)
>  		return;
>  
> -	if (io_bio->end_io)
> -		io_bio->end_io(io_bio, ret);
> +	btrfs_io_bio_free_csum(io_bio);
>  
>  free_ordered:
>  	/*
> diff --git a/fs/btrfs/volumes.h b/fs/btrfs/volumes.h
> index 9a764f2d462e..a13045fcfc45 100644
> --- a/fs/btrfs/volumes.h
> +++ b/fs/btrfs/volumes.h
> @@ -267,14 +267,12 @@ struct btrfs_fs_devices {
>   * we allocate are actually btrfs_io_bios.  We'll cram as much of
>   * struct btrfs_bio as we can into this over time.
>   */
> -typedef void (btrfs_io_bio_end_io_t) (struct btrfs_io_bio *bio, int err);
>  struct btrfs_io_bio {
>  	unsigned int mirror_num;
>  	unsigned int stripe_index;
>  	u64 logical;
>  	u8 *csum;
>  	u8 csum_inline[BTRFS_BIO_INLINE_CSUM_SIZE];
> -	btrfs_io_bio_end_io_t *end_io;
>  	struct bvec_iter iter;
>  	/*
>  	 * This member must come last, bio_alloc_bioset will allocate enough
> @@ -288,6 +286,14 @@ static inline struct btrfs_io_bio *btrfs_io_bio(struct bio *bio)
>  	return container_of(bio, struct btrfs_io_bio, bio);
>  }
>  
> +static inline void btrfs_io_bio_free_csum(struct btrfs_io_bio *io_bio)
> +{
> +	if (io_bio->csum != io_bio->csum_inline) {
> +		kfree(io_bio->csum);
> +		io_bio->csum = NULL;
> +	}
> +}
> +
>  struct btrfs_bio_stripe {
>  	struct btrfs_device *dev;
>  	u64 physical;
> 

Re: [PATCH 4/4] btrfs: replace btrfs_io_bio::end_io with a simple helper

[Johannes Thumshirn]

Looks good,
Reviewed-by: Johannes Thumshirn <jthumshirn@suse.de>
-- 
Johannes Thumshirn                                        SUSE Labs
jthumshirn@suse.de                                +49 911 74053 689
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: Felix Imendörffer, Jane Smithard, Graham Norton
HRB 21284 (AG Nürnberg)
Key fingerprint = EC38 9CAB C2C4 F25D 8600 D0D0 0393 969D 2D76 0850