[PATCH 0/8] Removal of optional hooks from struct extent_io_ops

[PATCH 0/8] Removal of optional hooks from struct extent_io_ops

[Nikolay Borisov]

extent_io_ops has a set of 8 optional hooks which are set only for data and 
freespace inodes. The majority of them actually deal with delallocs in one way 
or another. Inspecting the code it transpired that there is actually no need to
have them as function pointers in a structure. Data/freespace inodes can easily
be distinguished from the btree_inode (which is pending removal anyway) by 
inspecting extent_io_tree::private_data. This member is set by all data/freespace
inodes. This series exploits this fact to remove the majority of them. Others, 
such as fill_delalloc, writepage_start_hook and writepage_end_io_hook are always
called from the data writeout path and can be directly called without having to
check whether the respective pointers are set. 

This series has undergone multiple xfstest runs and no regressions were 
identified. Additionally all but run_delalloc_range functions are given more 
descriptive names, related to their actual intent. 

Nikolay Borisov (8):
  btrfs: Remove extent_io_ops::fill_delalloc
  btrfs: Remove extent_io_ops::writepage_start_hook
  btrfs: Remove extent_io_ops::writepage_end_io_hook
  btrfs: Remove extent_io_ops::check_extent_io_range callback
  btrfs: Remove extent_io_ops::set_bit_hook extent_io callback
  btrfs: Remove extent_io_ops::clear_bit_hook callback
  btrfs: Remove extent_io_ops::merge_extent_hook callback
  btrfs: Remove extent_io_ops::split_extent_hook callback

 fs/btrfs/compression.c           |  11 ++--
 fs/btrfs/ctree.h                 |  15 +++++
 fs/btrfs/extent_io.c             | 131 +++++++++++++++++----------------------
 fs/btrfs/extent_io.h             |  24 -------
 fs/btrfs/inode.c                 |  86 +++++++++----------------
 fs/btrfs/tests/extent-io-tests.c |   2 +-
 6 files changed, 105 insertions(+), 164 deletions(-)

-- 
2.7.4

[PATCH 1/8] btrfs: Remove extent_io_ops::fill_delalloc

[Nikolay Borisov]

This callback is called only from writepage_delalloc which in turn
is guaranteed to be called from the data page writeout path. In the end
there is no reason to have the call to this function to be indrected
via the extent_io_ops structure. This patch removes the callback
definition, exports the function and calls it directly. No functional
changes.

Signed-off-by: Nikolay Borisov <nborisov@suse.com>
---
 fs/btrfs/ctree.h     |  3 +++
 fs/btrfs/extent_io.c | 14 ++++++--------
 fs/btrfs/extent_io.h |  5 -----
 fs/btrfs/inode.c     | 10 +++++-----
 4 files changed, 14 insertions(+), 18 deletions(-)

diff --git a/fs/btrfs/ctree.h b/fs/btrfs/ctree.h
index 68ca41dbbef3..dbeb5b2486d5 100644
--- a/fs/btrfs/ctree.h
+++ b/fs/btrfs/ctree.h
@@ -3186,6 +3186,9 @@ int btrfs_prealloc_file_range_trans(struct inode *inode,
 				    struct btrfs_trans_handle *trans, int mode,
 				    u64 start, u64 num_bytes, u64 min_size,
 				    loff_t actual_len, u64 *alloc_hint);
+int run_delalloc_range(void *private_data, struct page *locked_page, u64 start,
+		       u64 end, int *page_started, unsigned long *nr_written,
+		       struct writeback_control *wbc);
 extern const struct dentry_operations btrfs_dentry_operations;
 
 /* ioctl.c */
diff --git a/fs/btrfs/extent_io.c b/fs/btrfs/extent_io.c
index 6877a74c7469..2e6191aa25f3 100644
--- a/fs/btrfs/extent_io.c
+++ b/fs/btrfs/extent_io.c
@@ -3205,7 +3205,7 @@ static void update_nr_written(struct writeback_control *wbc,
 /*
  * helper for __extent_writepage, doing all of the delayed allocation setup.
  *
- * This returns 1 if our fill_delalloc function did all the work required
+ * This returns 1 if run_delalloc_range function did all the work required
  * to write the page (copy into inline extent).  In this case the IO has
  * been started and the page is already unlocked.
  *
@@ -3226,7 +3226,7 @@ static noinline_for_stack int writepage_delalloc(struct inode *inode,
 	int ret;
 	int page_started = 0;
 
-	if (epd->extent_locked || !tree->ops || !tree->ops->fill_delalloc)
+	if (epd->extent_locked)
 		return 0;
 
 	while (delalloc_end < page_end) {
@@ -3239,15 +3239,13 @@ static noinline_for_stack int writepage_delalloc(struct inode *inode,
 			delalloc_start = delalloc_end + 1;
 			continue;
 		}
-		ret = tree->ops->fill_delalloc(inode, page,
-					       delalloc_start,
-					       delalloc_end,
-					       &page_started,
-					       nr_written, wbc);
+		ret = run_delalloc_range(inode, page, delalloc_start,
+					 delalloc_end, &page_started,
+					 nr_written, wbc);
 		/* File system has been set read-only */
 		if (ret) {
 			SetPageError(page);
-			/* fill_delalloc should be return < 0 for error
+			/* run_delalloc_range should return < 0 for error
 			 * but just in case, we use > 0 here meaning the
 			 * IO is started, so we don't want to return > 0
 			 * unless things are going well.
diff --git a/fs/btrfs/extent_io.h b/fs/btrfs/extent_io.h
index 369daa5d4f73..ca48187b86ba 100644
--- a/fs/btrfs/extent_io.h
+++ b/fs/btrfs/extent_io.h
@@ -106,11 +106,6 @@ struct extent_io_ops {
 	/*
 	 * Optional hooks, called if the pointer is not NULL
 	 */
-	int (*fill_delalloc)(void *private_data, struct page *locked_page,
-			     u64 start, u64 end, int *page_started,
-			     unsigned long *nr_written,
-			     struct writeback_control *wbc);
-
 	int (*writepage_start_hook)(struct page *page, u64 start, u64 end);
 	void (*writepage_end_io_hook)(struct page *page, u64 start, u64 end,
 				      struct extent_state *state, int uptodate);
diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c
index f4d31fd62eed..aa6d6b64a70a 100644
--- a/fs/btrfs/inode.c
+++ b/fs/btrfs/inode.c
@@ -109,8 +109,8 @@ static void __endio_write_update_ordered(struct inode *inode,
  * extent_clear_unlock_delalloc() to clear both the bits EXTENT_DO_ACCOUNTING
  * and EXTENT_DELALLOC simultaneously, because that causes the reserved metadata
  * to be released, which we want to happen only when finishing the ordered
- * extent (btrfs_finish_ordered_io()). Also note that the caller of the
- * fill_delalloc() callback already does proper cleanup for the first page of
+ * extent (btrfs_finish_ordered_io()). Also note that the caller of
+ * run_delalloc_range already does proper cleanup for the first page of
  * the range, that is, it invokes the callback writepage_end_io_hook() for the
  * range of the first page.
  */
@@ -1577,9 +1577,10 @@ static inline int need_force_cow(struct inode *inode, u64 start, u64 end)
 }
 
 /*
- * extent_io.c call back to do delayed allocation processing
+ * Function to process delayed allocation (create CoW) for ranges which are
+ * being touched for the first time.
  */
-static int run_delalloc_range(void *private_data, struct page *locked_page,
+int run_delalloc_range(void *private_data, struct page *locked_page,
 			      u64 start, u64 end, int *page_started,
 			      unsigned long *nr_written,
 			      struct writeback_control *wbc)
@@ -10521,7 +10522,6 @@ static const struct extent_io_ops btrfs_extent_io_ops = {
 	.readpage_io_failed_hook = btrfs_readpage_io_failed_hook,
 
 	/* optional callbacks */
-	.fill_delalloc = run_delalloc_range,
 	.writepage_end_io_hook = btrfs_writepage_end_io_hook,
 	.writepage_start_hook = btrfs_writepage_start_hook,
 	.set_bit_hook = btrfs_set_bit_hook,
-- 
2.7.4

Re: [PATCH 1/8] btrfs: Remove extent_io_ops::fill_delalloc

[Josef Bacik]

On Thu, Nov 01, 2018 at 02:09:46PM +0200, Nikolay Borisov wrote:
> This callback is called only from writepage_delalloc which in turn
> is guaranteed to be called from the data page writeout path. In the end
> there is no reason to have the call to this function to be indrected
> via the extent_io_ops structure. This patch removes the callback
> definition, exports the function and calls it directly. No functional
> changes.
> 
> Signed-off-by: Nikolay Borisov <nborisov@suse.com>

Reviewed-by: Josef Bacik <josef@toxicpanda.com>

Thanks,

Josef

Re: [PATCH 1/8] btrfs: Remove extent_io_ops::fill_delalloc

[David Sterba]

On Thu, Nov 01, 2018 at 02:09:46PM +0200, Nikolay Borisov wrote:
> This callback is called only from writepage_delalloc which in turn
> is guaranteed to be called from the data page writeout path. In the end
> there is no reason to have the call to this function to be indrected
> via the extent_io_ops structure. This patch removes the callback
> definition, exports the function and calls it directly. No functional
> changes.
> 
> Signed-off-by: Nikolay Borisov <nborisov@suse.com>
> ---
>  fs/btrfs/ctree.h     |  3 +++
>  fs/btrfs/extent_io.c | 14 ++++++--------
>  fs/btrfs/extent_io.h |  5 -----
>  fs/btrfs/inode.c     | 10 +++++-----
>  4 files changed, 14 insertions(+), 18 deletions(-)
> 
> diff --git a/fs/btrfs/ctree.h b/fs/btrfs/ctree.h
> index 68ca41dbbef3..dbeb5b2486d5 100644
> --- a/fs/btrfs/ctree.h
> +++ b/fs/btrfs/ctree.h
> @@ -3186,6 +3186,9 @@ int btrfs_prealloc_file_range_trans(struct inode *inode,
>  				    struct btrfs_trans_handle *trans, int mode,
>  				    u64 start, u64 num_bytes, u64 min_size,
>  				    loff_t actual_len, u64 *alloc_hint);
> +int run_delalloc_range(void *private_data, struct page *locked_page, u64 start,

Functions exported in .h should have the btrfs_prefix, updated in the
patch.

> +		       u64 end, int *page_started, unsigned long *nr_written,
> +		       struct writeback_control *wbc);
>  extern const struct dentry_operations btrfs_dentry_operations;
>  
>  /* ioctl.c */
> diff --git a/fs/btrfs/extent_io.c b/fs/btrfs/extent_io.c
> index 6877a74c7469..2e6191aa25f3 100644
> --- a/fs/btrfs/extent_io.c
> +++ b/fs/btrfs/extent_io.c
> @@ -3205,7 +3205,7 @@ static void update_nr_written(struct writeback_control *wbc,
>  /*
>   * helper for __extent_writepage, doing all of the delayed allocation setup.
>   *
> - * This returns 1 if our fill_delalloc function did all the work required
> + * This returns 1 if run_delalloc_range function did all the work required
>   * to write the page (copy into inline extent).  In this case the IO has
>   * been started and the page is already unlocked.
>   *
> @@ -3226,7 +3226,7 @@ static noinline_for_stack int writepage_delalloc(struct inode *inode,
>  	int ret;
>  	int page_started = 0;
>  
> -	if (epd->extent_locked || !tree->ops || !tree->ops->fill_delalloc)
> +	if (epd->extent_locked)
>  		return 0;
>  
>  	while (delalloc_end < page_end) {
> @@ -3239,15 +3239,13 @@ static noinline_for_stack int writepage_delalloc(struct inode *inode,
>  			delalloc_start = delalloc_end + 1;
>  			continue;
>  		}
> -		ret = tree->ops->fill_delalloc(inode, page,
> -					       delalloc_start,
> -					       delalloc_end,
> -					       &page_started,
> -					       nr_written, wbc);
> +		ret = run_delalloc_range(inode, page, delalloc_start,
> +					 delalloc_end, &page_started,
> +					 nr_written, wbc);
>  		/* File system has been set read-only */
>  		if (ret) {
>  			SetPageError(page);
> -			/* fill_delalloc should be return < 0 for error
> +			/* run_delalloc_range should return < 0 for error

Please don't use this style of comments, fixed.

[PATCH 2/8] btrfs: Remove extent_io_ops::writepage_start_hook

[Nikolay Borisov]

This hook is called only from __extent_writepage_io which is already
called only from the data page writeout path. So there is no need to
make an indirect call via extent_io_ops. This patch just removes the
callback definition, exports the callback function and calls it
directly at the only call site. Also give the function a more descriptive
name. No functional changes.

Signed-off-by: Nikolay Borisov <nborisov@suse.com>
---
 fs/btrfs/ctree.h     |  1 +
 fs/btrfs/extent_io.c | 23 ++++++++++-------------
 fs/btrfs/extent_io.h |  1 -
 fs/btrfs/inode.c     |  3 +--
 4 files changed, 12 insertions(+), 16 deletions(-)

diff --git a/fs/btrfs/ctree.h b/fs/btrfs/ctree.h
index dbeb5b2486d5..8e73301eaa65 100644
--- a/fs/btrfs/ctree.h
+++ b/fs/btrfs/ctree.h
@@ -3189,6 +3189,7 @@ int btrfs_prealloc_file_range_trans(struct inode *inode,
 int run_delalloc_range(void *private_data, struct page *locked_page, u64 start,
 		       u64 end, int *page_started, unsigned long *nr_written,
 		       struct writeback_control *wbc);
+int btrfs_writepage_cow_fixup(struct page *page, u64 start, u64 end);
 extern const struct dentry_operations btrfs_dentry_operations;
 
 /* ioctl.c */
diff --git a/fs/btrfs/extent_io.c b/fs/btrfs/extent_io.c
index 2e6191aa25f3..c5c713ebb9b1 100644
--- a/fs/btrfs/extent_io.c
+++ b/fs/btrfs/extent_io.c
@@ -3321,20 +3321,17 @@ static noinline_for_stack int __extent_writepage_io(struct inode *inode,
 	int nr = 0;
 	bool compressed;
 
-	if (tree->ops && tree->ops->writepage_start_hook) {
-		ret = tree->ops->writepage_start_hook(page, start,
-						      page_end);
-		if (ret) {
-			/* Fixup worker will requeue */
-			if (ret == -EBUSY)
-				wbc->pages_skipped++;
-			else
-				redirty_page_for_writepage(wbc, page);
+	ret = btrfs_writepage_cow_fixup(page, start, page_end);
+	if (ret) {
+		/* Fixup worker will requeue */
+		if (ret == -EBUSY)
+			wbc->pages_skipped++;
+		else
+			redirty_page_for_writepage(wbc, page);
 
-			update_nr_written(wbc, nr_written);
-			unlock_page(page);
-			return 1;
-		}
+		update_nr_written(wbc, nr_written);
+		unlock_page(page);
+		return 1;
 	}
 
 	/*
diff --git a/fs/btrfs/extent_io.h b/fs/btrfs/extent_io.h
index ca48187b86ba..4275a1061f5a 100644
--- a/fs/btrfs/extent_io.h
+++ b/fs/btrfs/extent_io.h
@@ -106,7 +106,6 @@ struct extent_io_ops {
 	/*
 	 * Optional hooks, called if the pointer is not NULL
 	 */
-	int (*writepage_start_hook)(struct page *page, u64 start, u64 end);
 	void (*writepage_end_io_hook)(struct page *page, u64 start, u64 end,
 				      struct extent_state *state, int uptodate);
 	void (*set_bit_hook)(void *private_data, struct extent_state *state,
diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c
index aa6d6b64a70a..5c07c6f9f7db 100644
--- a/fs/btrfs/inode.c
+++ b/fs/btrfs/inode.c
@@ -2154,7 +2154,7 @@ static void btrfs_writepage_fixup_worker(struct btrfs_work *work)
  * to fix it up.  The async helper will wait for ordered extents, set
  * the delalloc bit and make it safe to write the page.
  */
-static int btrfs_writepage_start_hook(struct page *page, u64 start, u64 end)
+int btrfs_writepage_cow_fixup(struct page *page, u64 start, u64 end)
 {
 	struct inode *inode = page->mapping->host;
 	struct btrfs_fs_info *fs_info = btrfs_sb(inode->i_sb);
@@ -10523,7 +10523,6 @@ static const struct extent_io_ops btrfs_extent_io_ops = {
 
 	/* optional callbacks */
 	.writepage_end_io_hook = btrfs_writepage_end_io_hook,
-	.writepage_start_hook = btrfs_writepage_start_hook,
 	.set_bit_hook = btrfs_set_bit_hook,
 	.clear_bit_hook = btrfs_clear_bit_hook,
 	.merge_extent_hook = btrfs_merge_extent_hook,
-- 
2.7.4

Re: [PATCH 2/8] btrfs: Remove extent_io_ops::writepage_start_hook

[Josef Bacik]

On Thu, Nov 01, 2018 at 02:09:47PM +0200, Nikolay Borisov wrote:
> This hook is called only from __extent_writepage_io which is already
> called only from the data page writeout path. So there is no need to
> make an indirect call via extent_io_ops. This patch just removes the
> callback definition, exports the callback function and calls it
> directly at the only call site. Also give the function a more descriptive
> name. No functional changes.
> 
> Signed-off-by: Nikolay Borisov <nborisov@suse.com>

Reviewed-by: Josef Bacik <josef@toxicpanda.com>

Thanks,

Josef

[PATCH 3/8] btrfs: Remove extent_io_ops::writepage_end_io_hook

[Nikolay Borisov]

This callback is ony ever called for data page writeout so
there is no need to actually abstract it via extent_io_ops. Lets just
export it, remove the definition of the callback and call it directly
in the functions that invoke the callback. Also rename the function to
btrfs_writepage_endio_finish_ordered since what it really does is
account finished io in the ordered extent data structures.
No functional changes.

Signed-off-by: Nikolay Borisov <nborisov@suse.com>
---
 fs/btrfs/compression.c | 11 +++++------
 fs/btrfs/ctree.h       |  3 +++
 fs/btrfs/extent_io.c   | 31 +++++++++++++------------------
 fs/btrfs/extent_io.h   |  2 --
 fs/btrfs/inode.c       |  9 ++++-----
 5 files changed, 25 insertions(+), 31 deletions(-)

diff --git a/fs/btrfs/compression.c b/fs/btrfs/compression.c
index 8703ce68fe9d..2f2b63efe5bb 100644
--- a/fs/btrfs/compression.c
+++ b/fs/btrfs/compression.c
@@ -250,12 +250,11 @@ static void end_compressed_bio_write(struct bio *bio)
 	inode = cb->inode;
 	tree = &BTRFS_I(inode)->io_tree;
 	cb->compressed_pages[0]->mapping = cb->inode->i_mapping;
-	tree->ops->writepage_end_io_hook(cb->compressed_pages[0],
-					 cb->start,
-					 cb->start + cb->len - 1,
-					 NULL,
-					 bio->bi_status ?
-					 BLK_STS_OK : BLK_STS_NOTSUPP);
+	btrfs_writepage_endio_finish_ordered(cb->compressed_pages[0],
+					     cb->start,
+					     cb->start + cb->len - 1, NULL,
+					     bio->bi_status ?
+					     BLK_STS_OK : BLK_STS_NOTSUPP);
 	cb->compressed_pages[0]->mapping = NULL;
 
 	end_compressed_writeback(inode, cb);
diff --git a/fs/btrfs/ctree.h b/fs/btrfs/ctree.h
index 8e73301eaa65..10f4dad8b16d 100644
--- a/fs/btrfs/ctree.h
+++ b/fs/btrfs/ctree.h
@@ -3190,6 +3190,9 @@ int run_delalloc_range(void *private_data, struct page *locked_page, u64 start,
 		       u64 end, int *page_started, unsigned long *nr_written,
 		       struct writeback_control *wbc);
 int btrfs_writepage_cow_fixup(struct page *page, u64 start, u64 end);
+void btrfs_writepage_endio_finish_ordered(struct page *page, u64 start,
+					  u64 end, struct extent_state *state,
+					  int uptodate);
 extern const struct dentry_operations btrfs_dentry_operations;
 
 /* ioctl.c */
diff --git a/fs/btrfs/extent_io.c b/fs/btrfs/extent_io.c
index c5c713ebb9b1..05cbd6b3aeec 100644
--- a/fs/btrfs/extent_io.c
+++ b/fs/btrfs/extent_io.c
@@ -2417,9 +2417,7 @@ void end_extent_writepage(struct page *page, int err, u64 start, u64 end)
 
 	tree = &BTRFS_I(page->mapping->host)->io_tree;
 
-	if (tree->ops && tree->ops->writepage_end_io_hook)
-		tree->ops->writepage_end_io_hook(page, start, end, NULL,
-				uptodate);
+	btrfs_writepage_endio_finish_ordered(page, start, end, NULL, uptodate);
 
 	if (!uptodate) {
 		ClearPageUptodate(page);
@@ -3342,9 +3340,8 @@ static noinline_for_stack int __extent_writepage_io(struct inode *inode,
 
 	end = page_end;
 	if (i_size <= start) {
-		if (tree->ops && tree->ops->writepage_end_io_hook)
-			tree->ops->writepage_end_io_hook(page, start,
-							 page_end, NULL, 1);
+		btrfs_writepage_endio_finish_ordered(page, start, page_end,
+						     NULL, 1);
 		goto done;
 	}
 
@@ -3355,9 +3352,9 @@ static noinline_for_stack int __extent_writepage_io(struct inode *inode,
 		u64 offset;
 
 		if (cur >= i_size) {
-			if (tree->ops && tree->ops->writepage_end_io_hook)
-				tree->ops->writepage_end_io_hook(page, cur,
-							 page_end, NULL, 1);
+			btrfs_writepage_endio_finish_ordered(page, cur,
+							     page_end, NULL,
+							     1);
 			break;
 		}
 		em = btrfs_get_extent(BTRFS_I(inode), page, pg_offset, cur,
@@ -3391,11 +3388,10 @@ static noinline_for_stack int __extent_writepage_io(struct inode *inode,
 			 * end_io notification does not happen here for
 			 * compressed extents
 			 */
-			if (!compressed && tree->ops &&
-			    tree->ops->writepage_end_io_hook)
-				tree->ops->writepage_end_io_hook(page, cur,
-							 cur + iosize - 1,
-							 NULL, 1);
+			if (!compressed)
+				btrfs_writepage_endio_finish_ordered(page, cur,
+							    cur + iosize - 1,
+							    NULL, 1);
 			else if (compressed) {
 				/* we don't want to end_page_writeback on
 				 * a compressed extent.  this happens
@@ -4079,10 +4075,9 @@ int extent_write_locked_range(struct inode *inode, u64 start, u64 end,
 		if (clear_page_dirty_for_io(page))
 			ret = __extent_writepage(page, &wbc_writepages, &epd);
 		else {
-			if (tree->ops && tree->ops->writepage_end_io_hook)
-				tree->ops->writepage_end_io_hook(page, start,
-						 start + PAGE_SIZE - 1,
-						 NULL, 1);
+			btrfs_writepage_endio_finish_ordered(page, start,
+						    start + PAGE_SIZE - 1,
+						    NULL, 1);
 			unlock_page(page);
 		}
 		put_page(page);
diff --git a/fs/btrfs/extent_io.h b/fs/btrfs/extent_io.h
index 4275a1061f5a..415ea7c2b058 100644
--- a/fs/btrfs/extent_io.h
+++ b/fs/btrfs/extent_io.h
@@ -106,8 +106,6 @@ struct extent_io_ops {
 	/*
 	 * Optional hooks, called if the pointer is not NULL
 	 */
-	void (*writepage_end_io_hook)(struct page *page, u64 start, u64 end,
-				      struct extent_state *state, int uptodate);
 	void (*set_bit_hook)(void *private_data, struct extent_state *state,
 			     unsigned *bits);
 	void (*clear_bit_hook)(void *private_data,
diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c
index 5c07c6f9f7db..9e0f2728b9de 100644
--- a/fs/btrfs/inode.c
+++ b/fs/btrfs/inode.c
@@ -847,14 +847,14 @@ static noinline void submit_compressed_extents(struct inode *inode,
 				    ins.offset, async_extent->pages,
 				    async_extent->nr_pages,
 				    async_cow->write_flags)) {
-			struct extent_io_tree *tree = &BTRFS_I(inode)->io_tree;
 			struct page *p = async_extent->pages[0];
 			const u64 start = async_extent->start;
 			const u64 end = start + async_extent->ram_size - 1;
 
 			p->mapping = inode->i_mapping;
-			tree->ops->writepage_end_io_hook(p, start, end,
-							 NULL, 0);
+			btrfs_writepage_endio_finish_ordered(p, start, end,
+							     NULL, 0);
+
 			p->mapping = NULL;
 			extent_clear_unlock_delalloc(inode, start, end, end,
 						     NULL, 0,
@@ -3161,7 +3161,7 @@ static void finish_ordered_fn(struct btrfs_work *work)
 	btrfs_finish_ordered_io(ordered_extent);
 }
 
-static void btrfs_writepage_end_io_hook(struct page *page, u64 start, u64 end,
+void btrfs_writepage_endio_finish_ordered(struct page *page, u64 start, u64 end,
 				struct extent_state *state, int uptodate)
 {
 	struct inode *inode = page->mapping->host;
@@ -10522,7 +10522,6 @@ static const struct extent_io_ops btrfs_extent_io_ops = {
 	.readpage_io_failed_hook = btrfs_readpage_io_failed_hook,
 
 	/* optional callbacks */
-	.writepage_end_io_hook = btrfs_writepage_end_io_hook,
 	.set_bit_hook = btrfs_set_bit_hook,
 	.clear_bit_hook = btrfs_clear_bit_hook,
 	.merge_extent_hook = btrfs_merge_extent_hook,
-- 
2.7.4

Re: [PATCH 3/8] btrfs: Remove extent_io_ops::writepage_end_io_hook

[Josef Bacik]

On Thu, Nov 01, 2018 at 02:09:48PM +0200, Nikolay Borisov wrote:
> This callback is ony ever called for data page writeout so
> there is no need to actually abstract it via extent_io_ops. Lets just
> export it, remove the definition of the callback and call it directly
> in the functions that invoke the callback. Also rename the function to
> btrfs_writepage_endio_finish_ordered since what it really does is
> account finished io in the ordered extent data structures.
> No functional changes.
> 
> Signed-off-by: Nikolay Borisov <nborisov@suse.com>

Could send another cleanup patch to remove the struct extent_state *state from
the arg list as well.

Reviewed-by: Josef Bacik <josef@toxicpanda.com>

Thanks,

Josef

Re: [PATCH 3/8] btrfs: Remove extent_io_ops::writepage_end_io_hook

[Nikolay Borisov]

On 1.11.18 г. 21:03 ч., Josef Bacik wrote:
> On Thu, Nov 01, 2018 at 02:09:48PM +0200, Nikolay Borisov wrote:
>> This callback is ony ever called for data page writeout so
>> there is no need to actually abstract it via extent_io_ops. Lets just
>> export it, remove the definition of the callback and call it directly
>> in the functions that invoke the callback. Also rename the function to
>> btrfs_writepage_endio_finish_ordered since what it really does is
>> account finished io in the ordered extent data structures.
>> No functional changes.
>>
>> Signed-off-by: Nikolay Borisov <nborisov@suse.com>
> 
> Could send another cleanup patch to remove the struct extent_state *state from
> the arg list as well.

Indeed, once this series lands I will send a follow-up cleanup since I
have some other ideas around writepage_delalloc code cleanups as well.
Thanks

> 
> Reviewed-by: Josef Bacik <josef@toxicpanda.com>
> 
> Thanks,
> 
> Josef
> 

[PATCH 4/8] btrfs: Remove extent_io_ops::check_extent_io_range callback

[Nikolay Borisov]

This callback was only used in debug builds by btrfs_leak_debug_check.
A better approach is to move its implementation in
btrfs_leak_debug_check and ensure the latter is only executed for
extent tree which have ->private_data set i.e. relate to a data node and
not the btree one. No functional changes.

Signed-off-by: Nikolay Borisov <nborisov@suse.com>
---
 fs/btrfs/extent_io.c | 15 ++++++++++++---
 fs/btrfs/extent_io.h |  2 --
 fs/btrfs/inode.c     | 15 ---------------
 3 files changed, 12 insertions(+), 20 deletions(-)

diff --git a/fs/btrfs/extent_io.c b/fs/btrfs/extent_io.c
index 05cbd6b3aeec..de171ae2ef20 100644
--- a/fs/btrfs/extent_io.c
+++ b/fs/btrfs/extent_io.c
@@ -89,9 +89,18 @@ void btrfs_leak_debug_check(void)
 static inline void __btrfs_debug_check_extent_io_range(const char *caller,
 		struct extent_io_tree *tree, u64 start, u64 end)
 {
-	if (tree->ops && tree->ops->check_extent_io_range)
-		tree->ops->check_extent_io_range(tree->private_data, caller,
-						 start, end);
+	struct inode *inode = tree->private_data;
+	u64 isize;
+
+	if (!inode)
+		return;
+
+	isize = i_size_read(inode);
+	if (end >= PAGE_SIZE && (end % 2) == 0 && end != isize - 1) {
+		btrfs_debug_rl(BTRFS_I(inode)->root->fs_info,
+		    "%s: ino %llu isize %llu odd range [%llu,%llu]",
+			caller, btrfs_ino(BTRFS_I(inode)), isize, start, end);
+	}
 }
 #else
 #define btrfs_leak_debug_add(new, head)	do {} while (0)
diff --git a/fs/btrfs/extent_io.h b/fs/btrfs/extent_io.h
index 415ea7c2b058..3cb84a0fbaab 100644
--- a/fs/btrfs/extent_io.h
+++ b/fs/btrfs/extent_io.h
@@ -116,8 +116,6 @@ struct extent_io_ops {
 				  struct extent_state *other);
 	void (*split_extent_hook)(void *private_data,
 				  struct extent_state *orig, u64 split);
-	void (*check_extent_io_range)(void *private_data, const char *caller,
-				      u64 start, u64 end);
 };
 
 struct extent_io_tree {
diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c
index 9e0f2728b9de..7110686e9b0e 100644
--- a/fs/btrfs/inode.c
+++ b/fs/btrfs/inode.c
@@ -10447,20 +10447,6 @@ static int btrfs_readpage_io_failed_hook(struct page *page, int failed_mirror)
 	return -EAGAIN;
 }
 
-static void btrfs_check_extent_io_range(void *private_data, const char *caller,
-					u64 start, u64 end)
-{
-	struct inode *inode = private_data;
-	u64 isize;
-
-	isize = i_size_read(inode);
-	if (end >= PAGE_SIZE && (end % 2) == 0 && end != isize - 1) {
-		btrfs_debug_rl(BTRFS_I(inode)->root->fs_info,
-		    "%s: ino %llu isize %llu odd range [%llu,%llu]",
-			caller, btrfs_ino(BTRFS_I(inode)), isize, start, end);
-	}
-}
-
 void btrfs_set_range_writeback(struct extent_io_tree *tree, u64 start, u64 end)
 {
 	struct inode *inode = tree->private_data;
@@ -10526,7 +10512,6 @@ static const struct extent_io_ops btrfs_extent_io_ops = {
 	.clear_bit_hook = btrfs_clear_bit_hook,
 	.merge_extent_hook = btrfs_merge_extent_hook,
 	.split_extent_hook = btrfs_split_extent_hook,
-	.check_extent_io_range = btrfs_check_extent_io_range,
 };
 
 /*
-- 
2.7.4

Re: [PATCH 4/8] btrfs: Remove extent_io_ops::check_extent_io_range callback

[Josef Bacik]

On Thu, Nov 01, 2018 at 02:09:49PM +0200, Nikolay Borisov wrote:
> This callback was only used in debug builds by btrfs_leak_debug_check.
> A better approach is to move its implementation in
> btrfs_leak_debug_check and ensure the latter is only executed for
> extent tree which have ->private_data set i.e. relate to a data node and
> not the btree one. No functional changes.
> 
> Signed-off-by: Nikolay Borisov <nborisov@suse.com>

Reviewed-by: Josef Bacik <josef@toxicpanda.com>

Thanks,

Josef

[PATCH 5/8] btrfs: Remove extent_io_ops::set_bit_hook extent_io callback

[Nikolay Borisov]

This callback is used to properly account delalloc extents for
data inodes (ordinary file inodes and freespace v1 inodes). Those can
be easily identified since they have their extent_io trees
->private_data member point to the inode. Let's exploit this fact to
remove the needless indirection through extent_io_hooks and directly
call the function. Also give the function a name which reflects its
purpose - btrfs_set_delalloc_extent.

This patch also modified test_find_delalloc so that the extent_io_tree
used for testing doesn't have its ->private_data set which would have
caused a crash in btrfs_set_delalloc_extent due to the
btrfs_inode->root member not being initialised. The old version of the
code also didn't call set_bit_hook since the extent_io ops weren't set
for the inode.  No functional changes.

Signed-off-by: Nikolay Borisov <nborisov@suse.com>
---
 fs/btrfs/ctree.h                 |  2 ++
 fs/btrfs/extent_io.c             | 11 +++--------
 fs/btrfs/extent_io.h             |  2 --
 fs/btrfs/inode.c                 | 12 ++++--------
 fs/btrfs/tests/extent-io-tests.c |  2 +-
 5 files changed, 10 insertions(+), 19 deletions(-)

diff --git a/fs/btrfs/ctree.h b/fs/btrfs/ctree.h
index 10f4dad8b16d..8fd9db7aa8de 100644
--- a/fs/btrfs/ctree.h
+++ b/fs/btrfs/ctree.h
@@ -3150,6 +3150,8 @@ int btrfs_create_subvol_root(struct btrfs_trans_handle *trans,
 			     struct btrfs_root *new_root,
 			     struct btrfs_root *parent_root,
 			     u64 new_dirid);
+void btrfs_set_delalloc_extent(struct inode *inode, struct extent_state *state,
+			       unsigned *bits);
 int btrfs_merge_bio_hook(struct page *page, unsigned long offset,
 			 size_t size, struct bio *bio,
 			 unsigned long bio_flags);
diff --git a/fs/btrfs/extent_io.c b/fs/btrfs/extent_io.c
index de171ae2ef20..31bdc596e623 100644
--- a/fs/btrfs/extent_io.c
+++ b/fs/btrfs/extent_io.c
@@ -404,13 +404,6 @@ static void merge_state(struct extent_io_tree *tree,
 	}
 }
 
-static void set_state_cb(struct extent_io_tree *tree,
-			 struct extent_state *state, unsigned *bits)
-{
-	if (tree->ops && tree->ops->set_bit_hook)
-		tree->ops->set_bit_hook(tree->private_data, state, bits);
-}
-
 static void clear_state_cb(struct extent_io_tree *tree,
 			   struct extent_state *state, unsigned *bits)
 {
@@ -809,7 +802,9 @@ static void set_state_bits(struct extent_io_tree *tree,
 	unsigned bits_to_set = *bits & ~EXTENT_CTLBITS;
 	int ret;
 
-	set_state_cb(tree, state, bits);
+	if (tree->private_data)
+		btrfs_set_delalloc_extent(tree->private_data, state, bits);
+
 	if ((bits_to_set & EXTENT_DIRTY) && !(state->state & EXTENT_DIRTY)) {
 		u64 range = state->end - state->start + 1;
 		tree->dirty_bytes += range;
diff --git a/fs/btrfs/extent_io.h b/fs/btrfs/extent_io.h
index 3cb84a0fbaab..b3235d46b5c3 100644
--- a/fs/btrfs/extent_io.h
+++ b/fs/btrfs/extent_io.h
@@ -106,8 +106,6 @@ struct extent_io_ops {
 	/*
 	 * Optional hooks, called if the pointer is not NULL
 	 */
-	void (*set_bit_hook)(void *private_data, struct extent_state *state,
-			     unsigned *bits);
 	void (*clear_bit_hook)(void *private_data,
 			struct extent_state *state,
 			unsigned *bits);
diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c
index 7110686e9b0e..d1c56c56a413 100644
--- a/fs/btrfs/inode.c
+++ b/fs/btrfs/inode.c
@@ -1757,15 +1757,12 @@ static void btrfs_del_delalloc_inode(struct btrfs_root *root,
 }
 
 /*
- * extent_io.c set_bit_hook, used to track delayed allocation
- * bytes in this file, and to maintain the list of inodes that
- * have pending delalloc work to be done.
+ * Properly track delayed allocation bytes in the inode and to maintain the
+ * list of inodes that have pending delalloc work to be done.
  */
-static void btrfs_set_bit_hook(void *private_data,
-			       struct extent_state *state, unsigned *bits)
+void btrfs_set_delalloc_extent(struct inode *inode, struct extent_state *state,
+			       unsigned *bits)
 {
-	struct inode *inode = private_data;
-
 	struct btrfs_fs_info *fs_info = btrfs_sb(inode->i_sb);
 
 	if ((*bits & EXTENT_DEFRAG) && !(*bits & EXTENT_DELALLOC))
@@ -10508,7 +10505,6 @@ static const struct extent_io_ops btrfs_extent_io_ops = {
 	.readpage_io_failed_hook = btrfs_readpage_io_failed_hook,
 
 	/* optional callbacks */
-	.set_bit_hook = btrfs_set_bit_hook,
 	.clear_bit_hook = btrfs_clear_bit_hook,
 	.merge_extent_hook = btrfs_merge_extent_hook,
 	.split_extent_hook = btrfs_split_extent_hook,
diff --git a/fs/btrfs/tests/extent-io-tests.c b/fs/btrfs/tests/extent-io-tests.c
index 9e0f4a01be14..ac8b5e35797d 100644
--- a/fs/btrfs/tests/extent-io-tests.c
+++ b/fs/btrfs/tests/extent-io-tests.c
@@ -76,7 +76,7 @@ static int test_find_delalloc(u32 sectorsize)
 		return -ENOMEM;
 	}
 
-	extent_io_tree_init(&tmp, inode);
+	extent_io_tree_init(&tmp, NULL);
 
 	/*
 	 * First go through and create and mark all of our pages dirty, we pin
-- 
2.7.4

Re: [PATCH 5/8] btrfs: Remove extent_io_ops::set_bit_hook extent_io callback

[Josef Bacik]

On Thu, Nov 01, 2018 at 02:09:50PM +0200, Nikolay Borisov wrote:
> This callback is used to properly account delalloc extents for
> data inodes (ordinary file inodes and freespace v1 inodes). Those can
> be easily identified since they have their extent_io trees
> ->private_data member point to the inode. Let's exploit this fact to
> remove the needless indirection through extent_io_hooks and directly
> call the function. Also give the function a name which reflects its
> purpose - btrfs_set_delalloc_extent.
> 
> This patch also modified test_find_delalloc so that the extent_io_tree
> used for testing doesn't have its ->private_data set which would have
> caused a crash in btrfs_set_delalloc_extent due to the
> btrfs_inode->root member not being initialised. The old version of the
> code also didn't call set_bit_hook since the extent_io ops weren't set
> for the inode.  No functional changes.
> 
> Signed-off-by: Nikolay Borisov <nborisov@suse.com>

Reviewed-by: Josef Bacik <josef@toxicpanda.com>

Thanks,

Josef

[PATCH 6/8] btrfs: Remove extent_io_ops::clear_bit_hook callback

[Nikolay Borisov]

This is the counterpart to ex-set_bit_hook (now btrfs_set_delalloc_extent),
similar to what was done before remove clear_bit_hook and rename the
function. No functional changes.

Signed-off-by: Nikolay Borisov <nborisov@suse.com>
---
 fs/btrfs/ctree.h     |  2 ++
 fs/btrfs/extent_io.c | 12 ++++--------
 fs/btrfs/extent_io.h |  3 ---
 fs/btrfs/inode.c     | 13 ++++++-------
 4 files changed, 12 insertions(+), 18 deletions(-)

diff --git a/fs/btrfs/ctree.h b/fs/btrfs/ctree.h
index 8fd9db7aa8de..49d52180e14c 100644
--- a/fs/btrfs/ctree.h
+++ b/fs/btrfs/ctree.h
@@ -3152,6 +3152,8 @@ int btrfs_create_subvol_root(struct btrfs_trans_handle *trans,
 			     u64 new_dirid);
 void btrfs_set_delalloc_extent(struct inode *inode, struct extent_state *state,
 			       unsigned *bits);
+void btrfs_clear_delalloc_extent(struct inode *inode,
+				 struct extent_state *state, unsigned *bits);
 int btrfs_merge_bio_hook(struct page *page, unsigned long offset,
 			 size_t size, struct bio *bio,
 			 unsigned long bio_flags);
diff --git a/fs/btrfs/extent_io.c b/fs/btrfs/extent_io.c
index 31bdc596e623..a3081c0ca3e3 100644
--- a/fs/btrfs/extent_io.c
+++ b/fs/btrfs/extent_io.c
@@ -404,13 +404,6 @@ static void merge_state(struct extent_io_tree *tree,
 	}
 }
 
-static void clear_state_cb(struct extent_io_tree *tree,
-			   struct extent_state *state, unsigned *bits)
-{
-	if (tree->ops && tree->ops->clear_bit_hook)
-		tree->ops->clear_bit_hook(tree->private_data, state, bits);
-}
-
 static void set_state_bits(struct extent_io_tree *tree,
 			   struct extent_state *state, unsigned *bits,
 			   struct extent_changeset *changeset);
@@ -525,7 +518,10 @@ static struct extent_state *clear_state_bit(struct extent_io_tree *tree,
 		WARN_ON(range > tree->dirty_bytes);
 		tree->dirty_bytes -= range;
 	}
-	clear_state_cb(tree, state, bits);
+
+	if (tree->private_data)
+		btrfs_clear_delalloc_extent(tree->private_data, state, bits);
+
 	ret = add_extent_changeset(state, bits_to_clear, changeset, 0);
 	BUG_ON(ret < 0);
 	state->state &= ~bits_to_clear;
diff --git a/fs/btrfs/extent_io.h b/fs/btrfs/extent_io.h
index b3235d46b5c3..a3a3302f3625 100644
--- a/fs/btrfs/extent_io.h
+++ b/fs/btrfs/extent_io.h
@@ -106,9 +106,6 @@ struct extent_io_ops {
 	/*
 	 * Optional hooks, called if the pointer is not NULL
 	 */
-	void (*clear_bit_hook)(void *private_data,
-			struct extent_state *state,
-			unsigned *bits);
 	void (*merge_extent_hook)(void *private_data,
 				  struct extent_state *new,
 				  struct extent_state *other);
diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c
index d1c56c56a413..41c655cd9d23 100644
--- a/fs/btrfs/inode.c
+++ b/fs/btrfs/inode.c
@@ -1808,14 +1808,14 @@ void btrfs_set_delalloc_extent(struct inode *inode, struct extent_state *state,
 }
 
 /*
- * extent_io.c clear_bit_hook, see set_bit_hook for why
+ * Once a range is no longer delalloc this function ensures proper accounting
+ * happens.
  */
-static void btrfs_clear_bit_hook(void *private_data,
-				 struct extent_state *state,
-				 unsigned *bits)
+void btrfs_clear_delalloc_extent(struct inode *vfs_inode,
+				 struct extent_state *state, unsigned *bits)
 {
-	struct btrfs_inode *inode = BTRFS_I((struct inode *)private_data);
-	struct btrfs_fs_info *fs_info = btrfs_sb(inode->vfs_inode.i_sb);
+	struct btrfs_inode *inode = BTRFS_I(vfs_inode);
+	struct btrfs_fs_info *fs_info = btrfs_sb(vfs_inode->i_sb);
 	u64 len = state->end + 1 - state->start;
 	u32 num_extents = count_max_extents(len);
 
@@ -10505,7 +10505,6 @@ static const struct extent_io_ops btrfs_extent_io_ops = {
 	.readpage_io_failed_hook = btrfs_readpage_io_failed_hook,
 
 	/* optional callbacks */
-	.clear_bit_hook = btrfs_clear_bit_hook,
 	.merge_extent_hook = btrfs_merge_extent_hook,
 	.split_extent_hook = btrfs_split_extent_hook,
 };
-- 
2.7.4

Re: [PATCH 6/8] btrfs: Remove extent_io_ops::clear_bit_hook callback

[Josef Bacik]

On Thu, Nov 01, 2018 at 02:09:51PM +0200, Nikolay Borisov wrote:
> This is the counterpart to ex-set_bit_hook (now btrfs_set_delalloc_extent),
> similar to what was done before remove clear_bit_hook and rename the
> function. No functional changes.
> 
> Signed-off-by: Nikolay Borisov <nborisov@suse.com>

Reviewed-by: Josef Bacik <josef@toxicpanda.com>

Thanks,

Josef

[PATCH 7/8] btrfs: Remove extent_io_ops::merge_extent_hook callback

[Nikolay Borisov]

This callback is used only for data and free space inodes. Such inodes
are guaranteed to have their extent_io_tree::private_data set to the
inode struct. Exploit this fact to directly call the function. Also
give it a more descriptive name. No functional changes.

Signed-off-by: Nikolay Borisov <nborisov@suse.com>
---
 fs/btrfs/ctree.h     |  2 ++
 fs/btrfs/extent_io.c | 15 ++++++---------
 fs/btrfs/extent_io.h |  3 ---
 fs/btrfs/inode.c     | 16 ++++++----------
 4 files changed, 14 insertions(+), 22 deletions(-)

diff --git a/fs/btrfs/ctree.h b/fs/btrfs/ctree.h
index 49d52180e14c..1f0025a0268b 100644
--- a/fs/btrfs/ctree.h
+++ b/fs/btrfs/ctree.h
@@ -3154,6 +3154,8 @@ void btrfs_set_delalloc_extent(struct inode *inode, struct extent_state *state,
 			       unsigned *bits);
 void btrfs_clear_delalloc_extent(struct inode *inode,
 				 struct extent_state *state, unsigned *bits);
+void btrfs_merge_delalloc_extent(struct inode *inode, struct extent_state *new,
+				 struct extent_state *other);
 int btrfs_merge_bio_hook(struct page *page, unsigned long offset,
 			 size_t size, struct bio *bio,
 			 unsigned long bio_flags);
diff --git a/fs/btrfs/extent_io.c b/fs/btrfs/extent_io.c
index a3081c0ca3e3..ee7e6ec3878d 100644
--- a/fs/btrfs/extent_io.c
+++ b/fs/btrfs/extent_io.c
@@ -353,13 +353,6 @@ static inline struct rb_node *tree_search(struct extent_io_tree *tree,
 	return tree_search_for_insert(tree, offset, NULL, NULL);
 }
 
-static void merge_cb(struct extent_io_tree *tree, struct extent_state *new,
-		     struct extent_state *other)
-{
-	if (tree->ops && tree->ops->merge_extent_hook)
-		tree->ops->merge_extent_hook(tree->private_data, new, other);
-}
-
 /*
  * utility function to look for merge candidates inside a given range.
  * Any extents with matching state are merged together into a single
@@ -383,7 +376,9 @@ static void merge_state(struct extent_io_tree *tree,
 		other = rb_entry(other_node, struct extent_state, rb_node);
 		if (other->end == state->start - 1 &&
 		    other->state == state->state) {
-			merge_cb(tree, state, other);
+			if (tree->private_data)
+				btrfs_merge_delalloc_extent(tree->private_data,
+							    state, other);
 			state->start = other->start;
 			rb_erase(&other->rb_node, &tree->state);
 			RB_CLEAR_NODE(&other->rb_node);
@@ -395,7 +390,9 @@ static void merge_state(struct extent_io_tree *tree,
 		other = rb_entry(other_node, struct extent_state, rb_node);
 		if (other->start == state->end + 1 &&
 		    other->state == state->state) {
-			merge_cb(tree, state, other);
+			if (tree->private_data)
+				btrfs_merge_delalloc_extent(tree->private_data,
+							    state, other);
 			state->end = other->end;
 			rb_erase(&other->rb_node, &tree->state);
 			RB_CLEAR_NODE(&other->rb_node);
diff --git a/fs/btrfs/extent_io.h b/fs/btrfs/extent_io.h
index a3a3302f3625..7d181a378d90 100644
--- a/fs/btrfs/extent_io.h
+++ b/fs/btrfs/extent_io.h
@@ -106,9 +106,6 @@ struct extent_io_ops {
 	/*
 	 * Optional hooks, called if the pointer is not NULL
 	 */
-	void (*merge_extent_hook)(void *private_data,
-				  struct extent_state *new,
-				  struct extent_state *other);
 	void (*split_extent_hook)(void *private_data,
 				  struct extent_state *orig, u64 split);
 };
diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c
index 41c655cd9d23..448fad3a8b74 100644
--- a/fs/btrfs/inode.c
+++ b/fs/btrfs/inode.c
@@ -1627,7 +1627,7 @@ static void btrfs_split_extent_hook(void *private_data,
 		u64 new_size;
 
 		/*
-		 * See the explanation in btrfs_merge_extent_hook, the same
+		 * See the explanation in btrfs_merge_delalloc_extent, the same
 		 * applies here, just in reverse.
 		 */
 		new_size = orig->end - split + 1;
@@ -1644,16 +1644,13 @@ static void btrfs_split_extent_hook(void *private_data,
 }
 
 /*
- * extent_io.c merge_extent_hook, used to track merged delayed allocation
- * extents so we can keep track of new extents that are just merged onto old
- * extents, such as when we are doing sequential writes, so we can properly
- * account for the metadata space we'll need.
+ * Handle merged delayed allocation extents so we can keep track of new extents
+ * that are just merged onto old extents, such as when we are doing sequential
+ * writes, so we can properly account for the metadata space we'll need.
  */
-static void btrfs_merge_extent_hook(void *private_data,
-				    struct extent_state *new,
-				    struct extent_state *other)
+void btrfs_merge_delalloc_extent(struct inode *inode, struct extent_state *new,
+				 struct extent_state *other)
 {
-	struct inode *inode = private_data;
 	u64 new_size, old_size;
 	u32 num_extents;
 
@@ -10505,7 +10502,6 @@ static const struct extent_io_ops btrfs_extent_io_ops = {
 	.readpage_io_failed_hook = btrfs_readpage_io_failed_hook,
 
 	/* optional callbacks */
-	.merge_extent_hook = btrfs_merge_extent_hook,
 	.split_extent_hook = btrfs_split_extent_hook,
 };
 
-- 
2.7.4

Re: [PATCH 7/8] btrfs: Remove extent_io_ops::merge_extent_hook callback

[Josef Bacik]

On Thu, Nov 01, 2018 at 02:09:52PM +0200, Nikolay Borisov wrote:
> This callback is used only for data and free space inodes. Such inodes
> are guaranteed to have their extent_io_tree::private_data set to the
> inode struct. Exploit this fact to directly call the function. Also
> give it a more descriptive name. No functional changes.
> 
> Signed-off-by: Nikolay Borisov <nborisov@suse.com>
> ---

Reviewed-by: Josef Bacik <josef@toxicpanda.com>

Thanks,

Josef

[PATCH 8/8] btrfs: Remove extent_io_ops::split_extent_hook callback

[Nikolay Borisov]

This is the counterpart to merge_extent_hook, similarly, it's used only
for data/freespace inodes so let's remove it, rename it and call it
directly where necessary. No functional changes.

Signed-off-by: Nikolay Borisov <nborisov@suse.com>
---
 fs/btrfs/ctree.h     |  2 ++
 fs/btrfs/extent_io.c | 10 ++--------
 fs/btrfs/extent_io.h |  6 ------
 fs/btrfs/inode.c     |  8 ++------
 4 files changed, 6 insertions(+), 20 deletions(-)

diff --git a/fs/btrfs/ctree.h b/fs/btrfs/ctree.h
index 1f0025a0268b..c20ef001f9af 100644
--- a/fs/btrfs/ctree.h
+++ b/fs/btrfs/ctree.h
@@ -3156,6 +3156,8 @@ void btrfs_clear_delalloc_extent(struct inode *inode,
 				 struct extent_state *state, unsigned *bits);
 void btrfs_merge_delalloc_extent(struct inode *inode, struct extent_state *new,
 				 struct extent_state *other);
+void btrfs_split_delalloc_extent(struct inode *inode,
+				 struct extent_state *orig, u64 split);
 int btrfs_merge_bio_hook(struct page *page, unsigned long offset,
 			 size_t size, struct bio *bio,
 			 unsigned long bio_flags);
diff --git a/fs/btrfs/extent_io.c b/fs/btrfs/extent_io.c
index ee7e6ec3878d..8c9073f28f6f 100644
--- a/fs/btrfs/extent_io.c
+++ b/fs/btrfs/extent_io.c
@@ -443,13 +443,6 @@ static int insert_state(struct extent_io_tree *tree,
 	return 0;
 }
 
-static void split_cb(struct extent_io_tree *tree, struct extent_state *orig,
-		     u64 split)
-{
-	if (tree->ops && tree->ops->split_extent_hook)
-		tree->ops->split_extent_hook(tree->private_data, orig, split);
-}
-
 /*
  * split a given extent state struct in two, inserting the preallocated
  * struct 'prealloc' as the newly created second half.  'split' indicates an
@@ -469,7 +462,8 @@ static int split_state(struct extent_io_tree *tree, struct extent_state *orig,
 {
 	struct rb_node *node;
 
-	split_cb(tree, orig, split);
+	if (tree->private_data)
+		btrfs_split_delalloc_extent(tree->private_data, orig, split);
 
 	prealloc->start = orig->start;
 	prealloc->end = split - 1;
diff --git a/fs/btrfs/extent_io.h b/fs/btrfs/extent_io.h
index 7d181a378d90..d96fd534f144 100644
--- a/fs/btrfs/extent_io.h
+++ b/fs/btrfs/extent_io.h
@@ -102,12 +102,6 @@ struct extent_io_ops {
 				    struct page *page, u64 start, u64 end,
 				    int mirror);
 	int (*readpage_io_failed_hook)(struct page *page, int failed_mirror);
-
-	/*
-	 * Optional hooks, called if the pointer is not NULL
-	 */
-	void (*split_extent_hook)(void *private_data,
-				  struct extent_state *orig, u64 split);
 };
 
 struct extent_io_tree {
diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c
index 448fad3a8b74..c38733c44b98 100644
--- a/fs/btrfs/inode.c
+++ b/fs/btrfs/inode.c
@@ -1611,10 +1611,9 @@ int run_delalloc_range(void *private_data, struct page *locked_page,
 	return ret;
 }
 
-static void btrfs_split_extent_hook(void *private_data,
-				    struct extent_state *orig, u64 split)
+void btrfs_split_delalloc_extent(struct inode *inode,
+				 struct extent_state *orig, u64 split)
 {
-	struct inode *inode = private_data;
 	u64 size;
 
 	/* not delalloc, ignore it */
@@ -10500,9 +10499,6 @@ static const struct extent_io_ops btrfs_extent_io_ops = {
 	.submit_bio_hook = btrfs_submit_bio_hook,
 	.readpage_end_io_hook = btrfs_readpage_end_io_hook,
 	.readpage_io_failed_hook = btrfs_readpage_io_failed_hook,
-
-	/* optional callbacks */
-	.split_extent_hook = btrfs_split_extent_hook,
 };
 
 /*
-- 
2.7.4

Re: [PATCH 8/8] btrfs: Remove extent_io_ops::split_extent_hook callback

[Josef Bacik]

On Thu, Nov 01, 2018 at 02:09:53PM +0200, Nikolay Borisov wrote:
> This is the counterpart to merge_extent_hook, similarly, it's used only
> for data/freespace inodes so let's remove it, rename it and call it
> directly where necessary. No functional changes.
> 
> Signed-off-by: Nikolay Borisov <nborisov@suse.com>
> ---

Reviewed-by: Josef Bacik <josef@toxicpanda.com>

Thanks,

Josef

Re: [PATCH 0/8] Removal of optional hooks from struct extent_io_ops

[David Sterba]

On Thu, Nov 01, 2018 at 02:09:45PM +0200, Nikolay Borisov wrote:
> extent_io_ops has a set of 8 optional hooks which are set only for data and 
> freespace inodes. The majority of them actually deal with delallocs in one way 
> or another. Inspecting the code it transpired that there is actually no need to
> have them as function pointers in a structure. Data/freespace inodes can easily
> be distinguished from the btree_inode (which is pending removal anyway) by 
> inspecting extent_io_tree::private_data. This member is set by all data/freespace
> inodes. This series exploits this fact to remove the majority of them. Others, 
> such as fill_delalloc, writepage_start_hook and writepage_end_io_hook are always
> called from the data writeout path and can be directly called without having to
> check whether the respective pointers are set. 
> 
> This series has undergone multiple xfstest runs and no regressions were 
> identified. Additionally all but run_delalloc_range functions are given more 
> descriptive names, related to their actual intent. 
> 
> Nikolay Borisov (8):
>   btrfs: Remove extent_io_ops::fill_delalloc
>   btrfs: Remove extent_io_ops::writepage_start_hook
>   btrfs: Remove extent_io_ops::writepage_end_io_hook
>   btrfs: Remove extent_io_ops::check_extent_io_range callback
>   btrfs: Remove extent_io_ops::set_bit_hook extent_io callback
>   btrfs: Remove extent_io_ops::clear_bit_hook callback
>   btrfs: Remove extent_io_ops::merge_extent_hook callback
>   btrfs: Remove extent_io_ops::split_extent_hook callback

Added to misc-next, thanks.