linux-btrfs.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/4] Minor helper and struct member cleanups
@ 2018-11-22 16:16 David Sterba
  2018-11-22 16:16 ` [PATCH 1/4] btrfs: merge btrfs_submit_bio_done to its caller David Sterba
                   ` (3 more replies)
  0 siblings, 4 replies; 13+ messages in thread
From: David Sterba @ 2018-11-22 16:16 UTC (permalink / raw)
  To: linux-btrfs; +Cc: 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


^ permalink raw reply	[flat|nested] 13+ messages in thread

* [PATCH 1/4] btrfs: merge btrfs_submit_bio_done to its caller
  2018-11-22 16:16 [PATCH 0/4] Minor helper and struct member cleanups David Sterba
@ 2018-11-22 16:16 ` David Sterba
  2018-11-22 16:18   ` Nikolay Borisov
  2018-11-23  8:27   ` Johannes Thumshirn
  2018-11-22 16:16 ` [PATCH 2/4] btrfs: replace async_cow::root with fs_info David Sterba
                   ` (2 subsequent siblings)
  3 siblings, 2 replies; 13+ messages in thread
From: David Sterba @ 2018-11-22 16:16 UTC (permalink / raw)
  To: linux-btrfs; +Cc: 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


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH 2/4] btrfs: replace async_cow::root with fs_info
  2018-11-22 16:16 [PATCH 0/4] Minor helper and struct member cleanups David Sterba
  2018-11-22 16:16 ` [PATCH 1/4] btrfs: merge btrfs_submit_bio_done to its caller David Sterba
@ 2018-11-22 16:16 ` David Sterba
  2018-11-22 16:19   ` Nikolay Borisov
  2018-11-23  8:25   ` Johannes Thumshirn
  2018-11-22 16:16 ` [PATCH 3/4] btrfs: remove redundant csum buffer in btrfs_io_bio David Sterba
  2018-11-22 16:16 ` [PATCH 4/4] btrfs: replace btrfs_io_bio::end_io with a simple helper David Sterba
  3 siblings, 2 replies; 13+ messages in thread
From: David Sterba @ 2018-11-22 16:16 UTC (permalink / raw)
  To: linux-btrfs; +Cc: 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


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH 3/4] btrfs: remove redundant csum buffer in btrfs_io_bio
  2018-11-22 16:16 [PATCH 0/4] Minor helper and struct member cleanups David Sterba
  2018-11-22 16:16 ` [PATCH 1/4] btrfs: merge btrfs_submit_bio_done to its caller David Sterba
  2018-11-22 16:16 ` [PATCH 2/4] btrfs: replace async_cow::root with fs_info David Sterba
@ 2018-11-22 16:16 ` David Sterba
  2018-11-22 16:20   ` Nikolay Borisov
  2018-11-23  8:23   ` Johannes Thumshirn
  2018-11-22 16:16 ` [PATCH 4/4] btrfs: replace btrfs_io_bio::end_io with a simple helper David Sterba
  3 siblings, 2 replies; 13+ messages in thread
From: David Sterba @ 2018-11-22 16:16 UTC (permalink / raw)
  To: linux-btrfs; +Cc: 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


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH 4/4] btrfs: replace btrfs_io_bio::end_io with a simple helper
  2018-11-22 16:16 [PATCH 0/4] Minor helper and struct member cleanups David Sterba
                   ` (2 preceding siblings ...)
  2018-11-22 16:16 ` [PATCH 3/4] btrfs: remove redundant csum buffer in btrfs_io_bio David Sterba
@ 2018-11-22 16:16 ` David Sterba
  2018-11-22 16:22   ` Nikolay Borisov
  2018-11-23  8:19   ` Johannes Thumshirn
  3 siblings, 2 replies; 13+ messages in thread
From: David Sterba @ 2018-11-22 16:16 UTC (permalink / raw)
  To: linux-btrfs; +Cc: 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


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* Re: [PATCH 1/4] btrfs: merge btrfs_submit_bio_done to its caller
  2018-11-22 16:16 ` [PATCH 1/4] btrfs: merge btrfs_submit_bio_done to its caller David Sterba
@ 2018-11-22 16:18   ` Nikolay Borisov
  2018-11-23  8:27   ` Johannes Thumshirn
  1 sibling, 0 replies; 13+ messages in thread
From: Nikolay Borisov @ 2018-11-22 16:18 UTC (permalink / raw)
  To: David Sterba, linux-btrfs



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.
> 

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH 2/4] btrfs: replace async_cow::root with fs_info
  2018-11-22 16:16 ` [PATCH 2/4] btrfs: replace async_cow::root with fs_info David Sterba
@ 2018-11-22 16:19   ` Nikolay Borisov
  2018-11-23  8:25   ` Johannes Thumshirn
  1 sibling, 0 replies; 13+ messages in thread
From: Nikolay Borisov @ 2018-11-22 16:19 UTC (permalink / raw)
  To: David Sterba, linux-btrfs



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;
> 

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH 3/4] btrfs: remove redundant csum buffer in btrfs_io_bio
  2018-11-22 16:16 ` [PATCH 3/4] btrfs: remove redundant csum buffer in btrfs_io_bio David Sterba
@ 2018-11-22 16:20   ` Nikolay Borisov
  2018-11-23  8:23   ` Johannes Thumshirn
  1 sibling, 0 replies; 13+ messages in thread
From: Nikolay Borisov @ 2018-11-22 16:20 UTC (permalink / raw)
  To: David Sterba, linux-btrfs



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;
>  	/*
> 

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH 4/4] btrfs: replace btrfs_io_bio::end_io with a simple helper
  2018-11-22 16:16 ` [PATCH 4/4] btrfs: replace btrfs_io_bio::end_io with a simple helper David Sterba
@ 2018-11-22 16:22   ` Nikolay Borisov
  2018-11-23  8:19   ` Johannes Thumshirn
  1 sibling, 0 replies; 13+ messages in thread
From: Nikolay Borisov @ 2018-11-22 16:22 UTC (permalink / raw)
  To: David Sterba, linux-btrfs



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;
> 

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH 4/4] btrfs: replace btrfs_io_bio::end_io with a simple helper
  2018-11-22 16:16 ` [PATCH 4/4] btrfs: replace btrfs_io_bio::end_io with a simple helper David Sterba
  2018-11-22 16:22   ` Nikolay Borisov
@ 2018-11-23  8:19   ` Johannes Thumshirn
  1 sibling, 0 replies; 13+ messages in thread
From: Johannes Thumshirn @ 2018-11-23  8:19 UTC (permalink / raw)
  To: David Sterba, linux-btrfs

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

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH 3/4] btrfs: remove redundant csum buffer in btrfs_io_bio
  2018-11-22 16:16 ` [PATCH 3/4] btrfs: remove redundant csum buffer in btrfs_io_bio David Sterba
  2018-11-22 16:20   ` Nikolay Borisov
@ 2018-11-23  8:23   ` Johannes Thumshirn
  1 sibling, 0 replies; 13+ messages in thread
From: Johannes Thumshirn @ 2018-11-23  8:23 UTC (permalink / raw)
  To: David Sterba, linux-btrfs

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

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH 2/4] btrfs: replace async_cow::root with fs_info
  2018-11-22 16:16 ` [PATCH 2/4] btrfs: replace async_cow::root with fs_info David Sterba
  2018-11-22 16:19   ` Nikolay Borisov
@ 2018-11-23  8:25   ` Johannes Thumshirn
  1 sibling, 0 replies; 13+ messages in thread
From: Johannes Thumshirn @ 2018-11-23  8:25 UTC (permalink / raw)
  To: David Sterba, linux-btrfs

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

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH 1/4] btrfs: merge btrfs_submit_bio_done to its caller
  2018-11-22 16:16 ` [PATCH 1/4] btrfs: merge btrfs_submit_bio_done to its caller David Sterba
  2018-11-22 16:18   ` Nikolay Borisov
@ 2018-11-23  8:27   ` Johannes Thumshirn
  1 sibling, 0 replies; 13+ messages in thread
From: Johannes Thumshirn @ 2018-11-23  8:27 UTC (permalink / raw)
  To: David Sterba, linux-btrfs

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

^ permalink raw reply	[flat|nested] 13+ messages in thread

end of thread, other threads:[~2018-11-23  8:27 UTC | newest]

Thread overview: 13+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2018-11-22 16:16 [PATCH 0/4] Minor helper and struct member cleanups David Sterba
2018-11-22 16:16 ` [PATCH 1/4] btrfs: merge btrfs_submit_bio_done to its caller David Sterba
2018-11-22 16:18   ` Nikolay Borisov
2018-11-23  8:27   ` Johannes Thumshirn
2018-11-22 16:16 ` [PATCH 2/4] btrfs: replace async_cow::root with fs_info David Sterba
2018-11-22 16:19   ` Nikolay Borisov
2018-11-23  8:25   ` Johannes Thumshirn
2018-11-22 16:16 ` [PATCH 3/4] btrfs: remove redundant csum buffer in btrfs_io_bio David Sterba
2018-11-22 16:20   ` Nikolay Borisov
2018-11-23  8:23   ` Johannes Thumshirn
2018-11-22 16:16 ` [PATCH 4/4] btrfs: replace btrfs_io_bio::end_io with a simple helper David Sterba
2018-11-22 16:22   ` Nikolay Borisov
2018-11-23  8:19   ` Johannes Thumshirn

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).