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

2018-11-05 Thread 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 
> ---
>  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,
> -_started,
> -nr_written, wbc);
> + ret = run_delalloc_range(inode, page, delalloc_start,
> +  delalloc_end, _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.


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

2018-11-01 Thread 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 

Reviewed-by: Josef Bacik 

Thanks,

Josef


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

2018-11-01 Thread 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 
---
 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,
-  _started,
-  nr_written, wbc);
+   ret = run_delalloc_range(inode, page, delalloc_start,
+delalloc_end, _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