[PATCH 0/5] Remove delay_iput parameter when running delalloc work

[PATCH 0/5] Remove delay_iput parameter when running delalloc work

[Nikolay Borisov]

While trying to make sense of the lifecycle of delayed iputs it became apparent
that the delay_iput parameter of btrfs_start_delalloc_roots/
btrfs_start_delalloc_inodes is always set to 0. (Patch 1 contains historical 
information of why this parameter was needed and when it became obsolete). Now
that the code has changed sufficiently it's no longer required to have it so
this series gradually removes it. 

Nikolay Borisov (5):
  btrfs: Remove delayed_iput parameter of btrfs_start_delalloc_roots
  btrfs: Remove delayed_iput parameter from btrfs_start_delalloc_inodes
  btrfs: Remove delay_iput parameter from __start_delalloc_inodes
  btrfs: Remove delayed_iput member from btrfs_delalloc_work
  btrfs: Unexport btrfs_alloc_delalloc_work

 fs/btrfs/ctree.h       |  9 ++-------
 fs/btrfs/dev-replace.c |  2 +-
 fs/btrfs/extent-tree.c |  4 ++--
 fs/btrfs/inode.c       | 28 +++++++++-------------------
 fs/btrfs/ioctl.c       |  4 ++--
 5 files changed, 16 insertions(+), 31 deletions(-)

-- 
2.7.4

[PATCH 1/5] btrfs: Remove delayed_iput parameter of btrfs_start_delalloc_roots

[Nikolay Borisov]

This parameter was introduced alongside the function in
eb73c1b7cea7 ("Btrfs: introduce per-subvolume delalloc inode list") to
avoid deadlocks since this function was used in the transaction commit
path. However, commit 8d875f95da43 ("btrfs: disable strict file flushes
for renames and truncates") removed that usage, rendering the parameter
obsolete.

Signed-off-by: Nikolay Borisov <nborisov@suse.com>
---
 fs/btrfs/ctree.h       | 3 +--
 fs/btrfs/dev-replace.c | 2 +-
 fs/btrfs/extent-tree.c | 4 ++--
 fs/btrfs/inode.c       | 5 ++---
 fs/btrfs/ioctl.c       | 2 +-
 5 files changed, 7 insertions(+), 9 deletions(-)

diff --git a/fs/btrfs/ctree.h b/fs/btrfs/ctree.h
index aac3a8bfab51..8c5ab06ccc0a 100644
--- a/fs/btrfs/ctree.h
+++ b/fs/btrfs/ctree.h
@@ -3199,8 +3199,7 @@ int btrfs_truncate_inode_items(struct btrfs_trans_handle *trans,
 			       u32 min_type);
 
 int btrfs_start_delalloc_inodes(struct btrfs_root *root, int delay_iput);
-int btrfs_start_delalloc_roots(struct btrfs_fs_info *fs_info, int delay_iput,
-			       int nr);
+int btrfs_start_delalloc_roots(struct btrfs_fs_info *fs_info, int nr);
 int btrfs_set_extent_delalloc(struct inode *inode, u64 start, u64 end,
 			      unsigned int extra_bits,
 			      struct extent_state **cached_state, int dedupe);
diff --git a/fs/btrfs/dev-replace.c b/fs/btrfs/dev-replace.c
index f82be266ba4b..a08b30052e06 100644
--- a/fs/btrfs/dev-replace.c
+++ b/fs/btrfs/dev-replace.c
@@ -503,7 +503,7 @@ static int btrfs_dev_replace_finishing(struct btrfs_fs_info *fs_info,
 	 * flush all outstanding I/O and inode extent mappings before the
 	 * copy operation is declared as being finished
 	 */
-	ret = btrfs_start_delalloc_roots(fs_info, 0, -1);
+	ret = btrfs_start_delalloc_roots(fs_info, -1);
 	if (ret) {
 		mutex_unlock(&dev_replace->lock_finishing_cancel_unmount);
 		return ret;
diff --git a/fs/btrfs/extent-tree.c b/fs/btrfs/extent-tree.c
index 22ba5d500836..cf3cd23f274d 100644
--- a/fs/btrfs/extent-tree.c
+++ b/fs/btrfs/extent-tree.c
@@ -4337,7 +4337,7 @@ int btrfs_alloc_data_chunk_ondemand(struct btrfs_inode *inode, u64 bytes)
 			need_commit--;
 
 			if (need_commit > 0) {
-				btrfs_start_delalloc_roots(fs_info, 0, -1);
+				btrfs_start_delalloc_roots(fs_info, -1);
 				btrfs_wait_ordered_roots(fs_info, U64_MAX, 0,
 							 (u64)-1);
 			}
@@ -4790,7 +4790,7 @@ static void btrfs_writeback_inodes_sb_nr(struct btrfs_fs_info *fs_info,
 		 * the filesystem is readonly(all dirty pages are written to
 		 * the disk).
 		 */
-		btrfs_start_delalloc_roots(fs_info, 0, nr_items);
+		btrfs_start_delalloc_roots(fs_info, nr_items);
 		if (!current->journal_info)
 			btrfs_wait_ordered_roots(fs_info, nr_items, 0, (u64)-1);
 	}
diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c
index c0a38917539d..74ea63159b93 100644
--- a/fs/btrfs/inode.c
+++ b/fs/btrfs/inode.c
@@ -10268,8 +10268,7 @@ int btrfs_start_delalloc_inodes(struct btrfs_root *root, int delay_iput)
 	return ret;
 }
 
-int btrfs_start_delalloc_roots(struct btrfs_fs_info *fs_info, int delay_iput,
-			       int nr)
+int btrfs_start_delalloc_roots(struct btrfs_fs_info *fs_info, int nr)
 {
 	struct btrfs_root *root;
 	struct list_head splice;
@@ -10292,7 +10291,7 @@ int btrfs_start_delalloc_roots(struct btrfs_fs_info *fs_info, int delay_iput,
 			       &fs_info->delalloc_roots);
 		spin_unlock(&fs_info->delalloc_root_lock);
 
-		ret = __start_delalloc_inodes(root, delay_iput, nr);
+		ret = __start_delalloc_inodes(root, 0, nr);
 		btrfs_put_fs_root(root);
 		if (ret < 0)
 			goto out;
diff --git a/fs/btrfs/ioctl.c b/fs/btrfs/ioctl.c
index de5dcdb87e97..ea192378cb3b 100644
--- a/fs/btrfs/ioctl.c
+++ b/fs/btrfs/ioctl.c
@@ -5316,7 +5316,7 @@ long btrfs_ioctl(struct file *file, unsigned int
 	case BTRFS_IOC_SYNC: {
 		int ret;
 
-		ret = btrfs_start_delalloc_roots(fs_info, 0, -1);
+		ret = btrfs_start_delalloc_roots(fs_info, -1);
 		if (ret)
 			return ret;
 		ret = btrfs_sync_fs(inode->i_sb, 1);
-- 
2.7.4

[PATCH 2/5] btrfs: Remove delayed_iput parameter from btrfs_start_delalloc_inodes

[Nikolay Borisov]

It's always set to 0, so just remove it and collapse the constant value
to the only function we are passing it.

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

diff --git a/fs/btrfs/ctree.h b/fs/btrfs/ctree.h
index 8c5ab06ccc0a..1ac983270bb7 100644
--- a/fs/btrfs/ctree.h
+++ b/fs/btrfs/ctree.h
@@ -3198,7 +3198,7 @@ int btrfs_truncate_inode_items(struct btrfs_trans_handle *trans,
 			       struct inode *inode, u64 new_size,
 			       u32 min_type);
 
-int btrfs_start_delalloc_inodes(struct btrfs_root *root, int delay_iput);
+int btrfs_start_delalloc_inodes(struct btrfs_root *root);
 int btrfs_start_delalloc_roots(struct btrfs_fs_info *fs_info, int nr);
 int btrfs_set_extent_delalloc(struct inode *inode, u64 start, u64 end,
 			      unsigned int extra_bits,
diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c
index 74ea63159b93..855237737acb 100644
--- a/fs/btrfs/inode.c
+++ b/fs/btrfs/inode.c
@@ -10254,7 +10254,7 @@ static int __start_delalloc_inodes(struct btrfs_root *root, int delay_iput,
 	return ret;
 }
 
-int btrfs_start_delalloc_inodes(struct btrfs_root *root, int delay_iput)
+int btrfs_start_delalloc_inodes(struct btrfs_root *root)
 {
 	struct btrfs_fs_info *fs_info = root->fs_info;
 	int ret;
@@ -10262,7 +10262,7 @@ int btrfs_start_delalloc_inodes(struct btrfs_root *root, int delay_iput)
 	if (test_bit(BTRFS_FS_STATE_ERROR, &fs_info->fs_state))
 		return -EROFS;
 
-	ret = __start_delalloc_inodes(root, delay_iput, -1);
+	ret = __start_delalloc_inodes(root, 0, -1);
 	if (ret > 0)
 		ret = 0;
 	return ret;
diff --git a/fs/btrfs/ioctl.c b/fs/btrfs/ioctl.c
index ea192378cb3b..f94b70e3525d 100644
--- a/fs/btrfs/ioctl.c
+++ b/fs/btrfs/ioctl.c
@@ -640,7 +640,7 @@ static int create_snapshot(struct btrfs_root *root, struct inode *dir,
 	wait_event(root->subv_writers->wait,
 		   percpu_counter_sum(&root->subv_writers->counter) == 0);
 
-	ret = btrfs_start_delalloc_inodes(root, 0);
+	ret = btrfs_start_delalloc_inodes(root);
 	if (ret)
 		goto dec_and_free;
 
-- 
2.7.4

[PATCH 3/5] btrfs: Remove delay_iput parameter from __start_delalloc_inodes

[Nikolay Borisov]

It's always set to 0 so remove it

Signed-off-by: Nikolay Borisov <nborisov@suse.com>
---
 fs/btrfs/inode.c | 14 +++++---------
 1 file changed, 5 insertions(+), 9 deletions(-)

diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c
index 855237737acb..42a2590559df 100644
--- a/fs/btrfs/inode.c
+++ b/fs/btrfs/inode.c
@@ -10189,8 +10189,7 @@ struct btrfs_delalloc_work *btrfs_alloc_delalloc_work(struct inode *inode,
  * some fairly slow code that needs optimization. This walks the list
  * of all the inodes with pending delalloc and forces them to disk.
  */
-static int __start_delalloc_inodes(struct btrfs_root *root, int delay_iput,
-				   int nr)
+static int __start_delalloc_inodes(struct btrfs_root *root, int nr)
 {
 	struct btrfs_inode *binode;
 	struct inode *inode;
@@ -10218,12 +10217,9 @@ static int __start_delalloc_inodes(struct btrfs_root *root, int delay_iput,
 		}
 		spin_unlock(&root->delalloc_lock);
 
-		work = btrfs_alloc_delalloc_work(inode, delay_iput);
+		work = btrfs_alloc_delalloc_work(inode, 0);
 		if (!work) {
-			if (delay_iput)
-				btrfs_add_delayed_iput(inode);
-			else
-				iput(inode);
+			iput(inode);
 			ret = -ENOMEM;
 			goto out;
 		}
@@ -10262,7 +10258,7 @@ int btrfs_start_delalloc_inodes(struct btrfs_root *root)
 	if (test_bit(BTRFS_FS_STATE_ERROR, &fs_info->fs_state))
 		return -EROFS;
 
-	ret = __start_delalloc_inodes(root, 0, -1);
+	ret = __start_delalloc_inodes(root, -1);
 	if (ret > 0)
 		ret = 0;
 	return ret;
@@ -10291,7 +10287,7 @@ int btrfs_start_delalloc_roots(struct btrfs_fs_info *fs_info, int nr)
 			       &fs_info->delalloc_roots);
 		spin_unlock(&fs_info->delalloc_root_lock);
 
-		ret = __start_delalloc_inodes(root, 0, nr);
+		ret = __start_delalloc_inodes(root, nr);
 		btrfs_put_fs_root(root);
 		if (ret < 0)
 			goto out;
-- 
2.7.4

Re: [PATCH 3/5] btrfs: Remove delay_iput parameter from __start_delalloc_inodes

[David Sterba]

On Mon, Apr 23, 2018 at 10:54:15AM +0300, Nikolay Borisov wrote:
> It's always set to 0 so remove it
> 
> Signed-off-by: Nikolay Borisov <nborisov@suse.com>
> ---
>  fs/btrfs/inode.c | 14 +++++---------
>  1 file changed, 5 insertions(+), 9 deletions(-)
> 
> diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c
> index 855237737acb..42a2590559df 100644
> --- a/fs/btrfs/inode.c
> +++ b/fs/btrfs/inode.c
> @@ -10189,8 +10189,7 @@ struct btrfs_delalloc_work *btrfs_alloc_delalloc_work(struct inode *inode,
>   * some fairly slow code that needs optimization. This walks the list
>   * of all the inodes with pending delalloc and forces them to disk.
>   */
> -static int __start_delalloc_inodes(struct btrfs_root *root, int delay_iput,
> -				   int nr)
> +static int __start_delalloc_inodes(struct btrfs_root *root, int nr)

As the prototype and all callsites are changed, renaming the function
(dropping __ in this case) is an accepted change. Better than having
that as separate patch that just pollutes the git history.

>  {
>  	struct btrfs_inode *binode;
>  	struct inode *inode;
> @@ -10218,12 +10217,9 @@ static int __start_delalloc_inodes(struct btrfs_root *root, int delay_iput,
>  		}
>  		spin_unlock(&root->delalloc_lock);
>  
> -		work = btrfs_alloc_delalloc_work(inode, delay_iput);
> +		work = btrfs_alloc_delalloc_work(inode, 0);
>  		if (!work) {
> -			if (delay_iput)
> -				btrfs_add_delayed_iput(inode);
> -			else
> -				iput(inode);
> +			iput(inode);
>  			ret = -ENOMEM;
>  			goto out;
>  		}
> @@ -10262,7 +10258,7 @@ int btrfs_start_delalloc_inodes(struct btrfs_root *root)
>  	if (test_bit(BTRFS_FS_STATE_ERROR, &fs_info->fs_state))
>  		return -EROFS;
>  
> -	ret = __start_delalloc_inodes(root, 0, -1);
> +	ret = __start_delalloc_inodes(root, -1);
>  	if (ret > 0)
>  		ret = 0;
>  	return ret;
> @@ -10291,7 +10287,7 @@ int btrfs_start_delalloc_roots(struct btrfs_fs_info *fs_info, int nr)
>  			       &fs_info->delalloc_roots);
>  		spin_unlock(&fs_info->delalloc_root_lock);
>  
> -		ret = __start_delalloc_inodes(root, 0, nr);
> +		ret = __start_delalloc_inodes(root, nr);
>  		btrfs_put_fs_root(root);
>  		if (ret < 0)
>  			goto out;
> -- 
> 2.7.4
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 4/5] btrfs: Remove delayed_iput member from btrfs_delalloc_work

[Nikolay Borisov]

When allocating a delalloc work we are always setting the delayed_iput
to 0. So remove the delay_iput member of btrfs_delalloc_work, as a
result also remove it as a parameter from btrfs_alloc_delalloc_work
since it's not used anymore.

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

diff --git a/fs/btrfs/ctree.h b/fs/btrfs/ctree.h
index 1ac983270bb7..91f51a80334f 100644
--- a/fs/btrfs/ctree.h
+++ b/fs/btrfs/ctree.h
@@ -3165,14 +3165,12 @@ void btrfs_extent_item_to_extent_map(struct btrfs_inode *inode,
 /* inode.c */
 struct btrfs_delalloc_work {
 	struct inode *inode;
-	int delay_iput;
 	struct completion completion;
 	struct list_head list;
 	struct btrfs_work work;
 };
 
-struct btrfs_delalloc_work *btrfs_alloc_delalloc_work(struct inode *inode,
-						    int delay_iput);
+struct btrfs_delalloc_work *btrfs_alloc_delalloc_work(struct inode *inode);
 
 struct extent_map *btrfs_get_extent_fiemap(struct btrfs_inode *inode,
 		struct page *page, size_t pg_offset, u64 start,
diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c
index 42a2590559df..9b8f2904ad55 100644
--- a/fs/btrfs/inode.c
+++ b/fs/btrfs/inode.c
@@ -10158,15 +10158,11 @@ static void btrfs_run_delalloc_work(struct btrfs_work *work)
 				&BTRFS_I(inode)->runtime_flags))
 		filemap_flush(inode->i_mapping);
 
-	if (delalloc_work->delay_iput)
-		btrfs_add_delayed_iput(inode);
-	else
-		iput(inode);
+	iput(inode);
 	complete(&delalloc_work->completion);
 }
 
-struct btrfs_delalloc_work *btrfs_alloc_delalloc_work(struct inode *inode,
-						    int delay_iput)
+struct btrfs_delalloc_work *btrfs_alloc_delalloc_work(struct inode *inode)
 {
 	struct btrfs_delalloc_work *work;
 
@@ -10177,7 +10173,6 @@ struct btrfs_delalloc_work *btrfs_alloc_delalloc_work(struct inode *inode,
 	init_completion(&work->completion);
 	INIT_LIST_HEAD(&work->list);
 	work->inode = inode;
-	work->delay_iput = delay_iput;
 	WARN_ON_ONCE(!inode);
 	btrfs_init_work(&work->work, btrfs_flush_delalloc_helper,
 			btrfs_run_delalloc_work, NULL, NULL);
@@ -10217,7 +10212,7 @@ static int __start_delalloc_inodes(struct btrfs_root *root, int nr)
 		}
 		spin_unlock(&root->delalloc_lock);
 
-		work = btrfs_alloc_delalloc_work(inode, 0);
+		work = btrfs_alloc_delalloc_work(inode);
 		if (!work) {
 			iput(inode);
 			ret = -ENOMEM;
-- 
2.7.4

[PATCH 5/5] btrfs: Unexport btrfs_alloc_delalloc_work

[Nikolay Borisov]

It's used only in inode.c so makes no sense to have it exported.

Signed-off-by: Nikolay Borisov <nborisov@suse.com>
---
 fs/btrfs/ctree.h | 2 --
 1 file changed, 2 deletions(-)

diff --git a/fs/btrfs/ctree.h b/fs/btrfs/ctree.h
index 91f51a80334f..e5c24d86fdfa 100644
--- a/fs/btrfs/ctree.h
+++ b/fs/btrfs/ctree.h
@@ -3170,8 +3170,6 @@ struct btrfs_delalloc_work {
 	struct btrfs_work work;
 };
 
-struct btrfs_delalloc_work *btrfs_alloc_delalloc_work(struct inode *inode);
-
 struct extent_map *btrfs_get_extent_fiemap(struct btrfs_inode *inode,
 		struct page *page, size_t pg_offset, u64 start,
 		u64 len, int create);
-- 
2.7.4

Re: [PATCH 5/5] btrfs: Unexport btrfs_alloc_delalloc_work

[David Sterba]

On Mon, Apr 23, 2018 at 10:54:17AM +0300, Nikolay Borisov wrote:
> It's used only in inode.c so makes no sense to have it exported.
> 
> Signed-off-by: Nikolay Borisov <nborisov@suse.com>
> ---
>  fs/btrfs/ctree.h | 2 --
>  1 file changed, 2 deletions(-)
> 
> diff --git a/fs/btrfs/ctree.h b/fs/btrfs/ctree.h
> index 91f51a80334f..e5c24d86fdfa 100644
> --- a/fs/btrfs/ctree.h
> +++ b/fs/btrfs/ctree.h
> @@ -3170,8 +3170,6 @@ struct btrfs_delalloc_work {
>  	struct btrfs_work work;
>  };
>  
> -struct btrfs_delalloc_work *btrfs_alloc_delalloc_work(struct inode *inode);

So thic can be also made static, I can fix that.

Re: [PATCH 5/5] btrfs: Unexport btrfs_alloc_delalloc_work

[Nikolay Borisov]

On 24.04.2018 16:22, David Sterba wrote:
> On Mon, Apr 23, 2018 at 10:54:17AM +0300, Nikolay Borisov wrote:
>> It's used only in inode.c so makes no sense to have it exported.
>>
>> Signed-off-by: Nikolay Borisov <nborisov@suse.com>
>> ---
>>  fs/btrfs/ctree.h | 2 --
>>  1 file changed, 2 deletions(-)
>>
>> diff --git a/fs/btrfs/ctree.h b/fs/btrfs/ctree.h
>> index 91f51a80334f..e5c24d86fdfa 100644
>> --- a/fs/btrfs/ctree.h
>> +++ b/fs/btrfs/ctree.h
>> @@ -3170,8 +3170,6 @@ struct btrfs_delalloc_work {
>>  	struct btrfs_work work;
>>  };
>>  
>> -struct btrfs_delalloc_work *btrfs_alloc_delalloc_work(struct inode *inode);
> 
> So thic can be also made static, I can fix that.


I forgot, my bad.

> 

[PATCH v2] btrfs: Unexport btrfs_alloc_delalloc_work

[Nikolay Borisov]

It's used only in inode.c so makes no sense to have it exported. Also
move the definition of btrfs_delalloc_work to inode.c since it's used
only this file.

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

diff --git a/fs/btrfs/ctree.h b/fs/btrfs/ctree.h
index 91f51a80334f..fd7ad63d51cc 100644
--- a/fs/btrfs/ctree.h
+++ b/fs/btrfs/ctree.h
@@ -3163,15 +3163,6 @@ void btrfs_extent_item_to_extent_map(struct btrfs_inode *inode,
 				     struct extent_map *em);
 
 /* inode.c */
-struct btrfs_delalloc_work {
-	struct inode *inode;
-	struct completion completion;
-	struct list_head list;
-	struct btrfs_work work;
-};
-
-struct btrfs_delalloc_work *btrfs_alloc_delalloc_work(struct inode *inode);
-
 struct extent_map *btrfs_get_extent_fiemap(struct btrfs_inode *inode,
 		struct page *page, size_t pg_offset, u64 start,
 		u64 len, int create);
diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c
index 9b8f2904ad55..078e02c21825 100644
--- a/fs/btrfs/inode.c
+++ b/fs/btrfs/inode.c
@@ -10145,6 +10145,13 @@ static int btrfs_rename2(struct inode *old_dir, struct dentry *old_dentry,
 	return btrfs_rename(old_dir, old_dentry, new_dir, new_dentry, flags);
 }
 
+struct btrfs_delalloc_work {
+	struct inode *inode;
+	struct completion completion;
+	struct list_head list;
+	struct btrfs_work work;
+};
+
 static void btrfs_run_delalloc_work(struct btrfs_work *work)
 {
 	struct btrfs_delalloc_work *delalloc_work;
@@ -10162,6 +10169,7 @@ static void btrfs_run_delalloc_work(struct btrfs_work *work)
 	complete(&delalloc_work->completion);
 }
 
+static
 struct btrfs_delalloc_work *btrfs_alloc_delalloc_work(struct inode *inode)
 {
 	struct btrfs_delalloc_work *work;
-- 
2.7.4

Re: [PATCH v2] btrfs: Unexport btrfs_alloc_delalloc_work

[David Sterba]

On Tue, Apr 24, 2018 at 05:23:59PM +0300, Nikolay Borisov wrote:
> It's used only in inode.c so makes no sense to have it exported. Also
> move the definition of btrfs_delalloc_work to inode.c since it's used
> only this file.
> 
> Signed-off-by: Nikolay Borisov <nborisov@suse.com>

Reviewed-by: David Sterba <dsterba@suse.com>

Re: [PATCH 0/5] Remove delay_iput parameter when running delalloc work

[Qu Wenruo]

On 2018年04月23日 15:54, Nikolay Borisov wrote:
> While trying to make sense of the lifecycle of delayed iputs it became apparent
> that the delay_iput parameter of btrfs_start_delalloc_roots/
> btrfs_start_delalloc_inodes is always set to 0. (Patch 1 contains historical 
> information of why this parameter was needed and when it became obsolete). Now
> that the code has changed sufficiently it's no longer required to have it so
> this series gradually removes it. 
> 
> Nikolay Borisov (5):
>   btrfs: Remove delayed_iput parameter of btrfs_start_delalloc_roots
>   btrfs: Remove delayed_iput parameter from btrfs_start_delalloc_inodes
>   btrfs: Remove delay_iput parameter from __start_delalloc_inodes
>   btrfs: Remove delayed_iput member from btrfs_delalloc_work
>   btrfs: Unexport btrfs_alloc_delalloc_work

Solid cleanup.

Reviewed-by: Qu Wenruo <wqu@suse.com>

Just a little nitpick about the 3rd and 4th patch.
It would be nicer the merge them, as in the the 3rd patch
btrfs_alloc_delalloc_work() call still takes 0 as @delay_iput, but in
next patch the @delay_iput just get removed.

Thanks,
Qu

> 
>  fs/btrfs/ctree.h       |  9 ++-------
>  fs/btrfs/dev-replace.c |  2 +-
>  fs/btrfs/extent-tree.c |  4 ++--
>  fs/btrfs/inode.c       | 28 +++++++++-------------------
>  fs/btrfs/ioctl.c       |  4 ++--
>  5 files changed, 16 insertions(+), 31 deletions(-)
> 

Re: [PATCH 0/5] Remove delay_iput parameter when running delalloc work

[Nikolay Borisov]

On 23.04.2018 12:27, Qu Wenruo wrote:
> 
> 
> On 2018年04月23日 15:54, Nikolay Borisov wrote:
>> While trying to make sense of the lifecycle of delayed iputs it became apparent
>> that the delay_iput parameter of btrfs_start_delalloc_roots/
>> btrfs_start_delalloc_inodes is always set to 0. (Patch 1 contains historical 
>> information of why this parameter was needed and when it became obsolete). Now
>> that the code has changed sufficiently it's no longer required to have it so
>> this series gradually removes it. 
>>
>> Nikolay Borisov (5):
>>   btrfs: Remove delayed_iput parameter of btrfs_start_delalloc_roots
>>   btrfs: Remove delayed_iput parameter from btrfs_start_delalloc_inodes
>>   btrfs: Remove delay_iput parameter from __start_delalloc_inodes
>>   btrfs: Remove delayed_iput member from btrfs_delalloc_work
>>   btrfs: Unexport btrfs_alloc_delalloc_work
> 
> Solid cleanup.
> 
> Reviewed-by: Qu Wenruo <wqu@suse.com>
> 
> Just a little nitpick about the 3rd and 4th patch.
> It would be nicer the merge them, as in the the 3rd patch
> btrfs_alloc_delalloc_work() call still takes 0 as @delay_iput, but in
> next patch the @delay_iput just get removed.

I'm fine with that, I guess David if you deem it more reasonable you
could squash the 2 patches. I just did it for the sake of bisectability
and review purposes.

> 
> Thanks,
> Qu
> 
>>
>>  fs/btrfs/ctree.h       |  9 ++-------
>>  fs/btrfs/dev-replace.c |  2 +-
>>  fs/btrfs/extent-tree.c |  4 ++--
>>  fs/btrfs/inode.c       | 28 +++++++++-------------------
>>  fs/btrfs/ioctl.c       |  4 ++--
>>  5 files changed, 16 insertions(+), 31 deletions(-)
>>
> 

Re: [PATCH 0/5] Remove delay_iput parameter when running delalloc work

[David Sterba]

On Mon, Apr 23, 2018 at 12:31:17PM +0300, Nikolay Borisov wrote:
> 
> 
> On 23.04.2018 12:27, Qu Wenruo wrote:
> > 
> > 
> > On 2018年04月23日 15:54, Nikolay Borisov wrote:
> >> While trying to make sense of the lifecycle of delayed iputs it became apparent
> >> that the delay_iput parameter of btrfs_start_delalloc_roots/
> >> btrfs_start_delalloc_inodes is always set to 0. (Patch 1 contains historical 
> >> information of why this parameter was needed and when it became obsolete). Now
> >> that the code has changed sufficiently it's no longer required to have it so
> >> this series gradually removes it. 
> >>
> >> Nikolay Borisov (5):
> >>   btrfs: Remove delayed_iput parameter of btrfs_start_delalloc_roots
> >>   btrfs: Remove delayed_iput parameter from btrfs_start_delalloc_inodes
> >>   btrfs: Remove delay_iput parameter from __start_delalloc_inodes
> >>   btrfs: Remove delayed_iput member from btrfs_delalloc_work
> >>   btrfs: Unexport btrfs_alloc_delalloc_work
> > 
> > Solid cleanup.
> > 
> > Reviewed-by: Qu Wenruo <wqu@suse.com>
> > 
> > Just a little nitpick about the 3rd and 4th patch.
> > It would be nicer the merge them, as in the the 3rd patch
> > btrfs_alloc_delalloc_work() call still takes 0 as @delay_iput, but in
> > next patch the @delay_iput just get removed.
> 
> I'm fine with that, I guess David if you deem it more reasonable you
> could squash the 2 patches. I just did it for the sake of bisectability
> and review purposes.

Unless the change is too fine-grained, the separate patches are easier
to review. In this case, one is removing the member and the other the
function arugment, this can be considered 2 logical changes.