[PATCH] btrfs: Remove unused variables from btrfs_async_reclaim_metadata_space

[PATCH] btrfs: Remove unused variables from btrfs_async_reclaim_metadata_space

[Nikolay Borisov]

The ret from flush_space is ignored so no point in assigning it to a variable.
The ticket variable was used in the initial version of the ticketed ENOSPC
work, however Wang Xiaoguang detected a problem with this and fixed it in
ce129655c9d9 ("btrfs: introduce tickets_id to determine whether asynchronous
metadata reclaim work makes progress").

Signed-off-by: Nikolay Borisov <nborisov@suse.com>
---
 fs/btrfs/extent-tree.c | 7 +------
 1 file changed, 1 insertion(+), 6 deletions(-)

diff --git a/fs/btrfs/extent-tree.c b/fs/btrfs/extent-tree.c
index d57f1f161fa2..31924c834049 100644
--- a/fs/btrfs/extent-tree.c
+++ b/fs/btrfs/extent-tree.c
@@ -5057,10 +5057,7 @@ static void btrfs_async_reclaim_metadata_space(struct work_struct *work)
 
 	flush_state = FLUSH_DELAYED_ITEMS_NR;
 	do {
-		struct reserve_ticket *ticket;
-		int ret;
-
-		ret = flush_space(fs_info, space_info, to_reclaim, to_reclaim,
+		flush_space(fs_info, space_info, to_reclaim, to_reclaim,
 				  flush_state);
 		spin_lock(&space_info->lock);
 		if (list_empty(&space_info->tickets)) {
@@ -5071,8 +5068,6 @@ static void btrfs_async_reclaim_metadata_space(struct work_struct *work)
 		to_reclaim = btrfs_calc_reclaim_metadata_size(fs_info,
 							      space_info,
 							      false);
-		ticket = list_first_entry(&space_info->tickets,
-					  struct reserve_ticket, list);
 		if (last_tickets_id == space_info->tickets_id) {
 			flush_state++;
 		} else {
-- 
2.7.4

Re: [PATCH] btrfs: Remove unused variables from btrfs_async_reclaim_metadata_space

[David Sterba]

On Fri, Jul 14, 2017 at 11:46:24AM +0300, Nikolay Borisov wrote:
> The ret from flush_space is ignored so no point in assigning it to a variable.

It would be good to document why it's safe to ignore return value of
flush_spaces along with removing it.

[PATCH] btrfs: Make flush_space return void

[Nikolay Borisov]

The return value of flush_space was used to have significance in the early days
when the code was first introduced and before the ticketed enospc rework. Since
the latter got introduced the return value lost any significance whatsoever to
its callers. So let's remove it. While at it also remove the unused ticket
variable in btrfs_async_reclaim_metadata_space. It was used in the initial
version of the ticketed ENOSPC work, however Wang Xiaoguang detected a problem
with this and fixed it in ce129655c9d9 ("btrfs: introduce tickets_id to
determine whether asynchronous metadata reclaim work makes progress").

Signed-off-by: Nikolay Borisov <nborisov@suse.com>
---
 fs/btrfs/extent-tree.c | 11 +++--------
 1 file changed, 3 insertions(+), 8 deletions(-)

diff --git a/fs/btrfs/extent-tree.c b/fs/btrfs/extent-tree.c
index a53441ffce71..463063fb4f07 100644
--- a/fs/btrfs/extent-tree.c
+++ b/fs/btrfs/extent-tree.c
@@ -4901,7 +4901,7 @@ struct reserve_ticket {
 	wait_queue_head_t wait;
 };
 
-static int flush_space(struct btrfs_fs_info *fs_info,
+static void flush_space(struct btrfs_fs_info *fs_info,
 		       struct btrfs_space_info *space_info, u64 num_bytes,
 		       u64 orig_bytes, int state)
 {
@@ -4955,7 +4955,7 @@ static int flush_space(struct btrfs_fs_info *fs_info,
 
 	trace_btrfs_flush_space(fs_info, space_info->flags, num_bytes,
 				orig_bytes, state, ret);
-	return ret;
+	return;
 }
 
 static inline u64
@@ -5057,10 +5057,7 @@ static void btrfs_async_reclaim_metadata_space(struct work_struct *work)
 
 	flush_state = FLUSH_DELAYED_ITEMS_NR;
 	do {
-		struct reserve_ticket *ticket;
-		int ret;
-
-		ret = flush_space(fs_info, space_info, to_reclaim, to_reclaim,
+		flush_space(fs_info, space_info, to_reclaim, to_reclaim,
 				  flush_state);
 		spin_lock(&space_info->lock);
 		if (list_empty(&space_info->tickets)) {
@@ -5071,8 +5068,6 @@ static void btrfs_async_reclaim_metadata_space(struct work_struct *work)
 		to_reclaim = btrfs_calc_reclaim_metadata_size(fs_info,
 							      space_info,
 							      false);
-		ticket = list_first_entry(&space_info->tickets,
-					  struct reserve_ticket, list);
 		if (last_tickets_id == space_info->tickets_id) {
 			flush_state++;
 		} else {
-- 
2.7.4

Re: [PATCH] btrfs: Make flush_space return void

[David Sterba]

On Tue, Jul 25, 2017 at 05:48:28PM +0300, Nikolay Borisov wrote:
> The return value of flush_space was used to have significance in the early days
> when the code was first introduced and before the ticketed enospc rework. Since
> the latter got introduced the return value lost any significance whatsoever to
> its callers. So let's remove it. While at it also remove the unused ticket
> variable in btrfs_async_reclaim_metadata_space. It was used in the initial
> version of the ticketed ENOSPC work, however Wang Xiaoguang detected a problem
> with this and fixed it in ce129655c9d9 ("btrfs: introduce tickets_id to
> determine whether asynchronous metadata reclaim work makes progress").
> 
> Signed-off-by: Nikolay Borisov <nborisov@suse.com>

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

I've added a comment to the function, as it's not obvious why all the
error conditions inside get ignored in the end.