diff options
author | Wang Xiaoguang <wangxg.fnst@cn.fujitsu.com> | 2016-09-01 22:58:46 -0400 |
---|---|---|
committer | David Sterba <dsterba@suse.com> | 2016-09-06 10:31:43 -0400 |
commit | ce129655c9d9aaa7b3bcc46529db1b36693575ed (patch) | |
tree | 3e653a3c55e2393f04ebc881712921f35cf3eb2b /fs | |
parent | ed7a6948394305b810d0c6203268648715e5006f (diff) |
btrfs: introduce tickets_id to determine whether asynchronous metadata reclaim work makes progress
In btrfs_async_reclaim_metadata_space(), we use ticket's address to
determine whether asynchronous metadata reclaim work is making progress.
ticket = list_first_entry(&space_info->tickets,
struct reserve_ticket, list);
if (last_ticket == ticket) {
flush_state++;
} else {
last_ticket = ticket;
flush_state = FLUSH_DELAYED_ITEMS_NR;
if (commit_cycles)
commit_cycles--;
}
But indeed it's wrong, we should not rely on local variable's address to
do this check, because addresses may be same. In my test environment, I
dd one 168MB file in a 256MB fs, found that for this file, every time
wait_reserve_ticket() called, local variable ticket's address is same,
For above codes, assume a previous ticket's address is addrA, last_ticket
is addrA. Btrfs_async_reclaim_metadata_space() finished this ticket and
wake up it, then another ticket is added, but with the same address addrA,
now last_ticket will be same to current ticket, then current ticket's flush
work will start from current flush_state, not initial FLUSH_DELAYED_ITEMS_NR,
which may result in some enospc issues(I have seen this in my test machine).
Signed-off-by: Wang Xiaoguang <wangxg.fnst@cn.fujitsu.com>
Reviewed-by: Josef Bacik <jbacik@fb.com>
Signed-off-by: David Sterba <dsterba@suse.com>
Diffstat (limited to 'fs')
-rw-r--r-- | fs/btrfs/ctree.h | 1 | ||||
-rw-r--r-- | fs/btrfs/extent-tree.c | 11 |
2 files changed, 7 insertions, 5 deletions
diff --git a/fs/btrfs/ctree.h b/fs/btrfs/ctree.h index ec4154faab61..146d1c7078ed 100644 --- a/fs/btrfs/ctree.h +++ b/fs/btrfs/ctree.h | |||
@@ -427,6 +427,7 @@ struct btrfs_space_info { | |||
427 | struct list_head ro_bgs; | 427 | struct list_head ro_bgs; |
428 | struct list_head priority_tickets; | 428 | struct list_head priority_tickets; |
429 | struct list_head tickets; | 429 | struct list_head tickets; |
430 | u64 tickets_id; | ||
430 | 431 | ||
431 | struct rw_semaphore groups_sem; | 432 | struct rw_semaphore groups_sem; |
432 | /* for block groups in our same type */ | 433 | /* for block groups in our same type */ |
diff --git a/fs/btrfs/extent-tree.c b/fs/btrfs/extent-tree.c index 4483487ef021..d09cf7aa083b 100644 --- a/fs/btrfs/extent-tree.c +++ b/fs/btrfs/extent-tree.c | |||
@@ -4966,12 +4966,12 @@ static void wake_all_tickets(struct list_head *head) | |||
4966 | */ | 4966 | */ |
4967 | static void btrfs_async_reclaim_metadata_space(struct work_struct *work) | 4967 | static void btrfs_async_reclaim_metadata_space(struct work_struct *work) |
4968 | { | 4968 | { |
4969 | struct reserve_ticket *last_ticket = NULL; | ||
4970 | struct btrfs_fs_info *fs_info; | 4969 | struct btrfs_fs_info *fs_info; |
4971 | struct btrfs_space_info *space_info; | 4970 | struct btrfs_space_info *space_info; |
4972 | u64 to_reclaim; | 4971 | u64 to_reclaim; |
4973 | int flush_state; | 4972 | int flush_state; |
4974 | int commit_cycles = 0; | 4973 | int commit_cycles = 0; |
4974 | u64 last_tickets_id; | ||
4975 | 4975 | ||
4976 | fs_info = container_of(work, struct btrfs_fs_info, async_reclaim_work); | 4976 | fs_info = container_of(work, struct btrfs_fs_info, async_reclaim_work); |
4977 | space_info = __find_space_info(fs_info, BTRFS_BLOCK_GROUP_METADATA); | 4977 | space_info = __find_space_info(fs_info, BTRFS_BLOCK_GROUP_METADATA); |
@@ -4984,8 +4984,7 @@ static void btrfs_async_reclaim_metadata_space(struct work_struct *work) | |||
4984 | spin_unlock(&space_info->lock); | 4984 | spin_unlock(&space_info->lock); |
4985 | return; | 4985 | return; |
4986 | } | 4986 | } |
4987 | last_ticket = list_first_entry(&space_info->tickets, | 4987 | last_tickets_id = space_info->tickets_id; |
4988 | struct reserve_ticket, list); | ||
4989 | spin_unlock(&space_info->lock); | 4988 | spin_unlock(&space_info->lock); |
4990 | 4989 | ||
4991 | flush_state = FLUSH_DELAYED_ITEMS_NR; | 4990 | flush_state = FLUSH_DELAYED_ITEMS_NR; |
@@ -5005,10 +5004,10 @@ static void btrfs_async_reclaim_metadata_space(struct work_struct *work) | |||
5005 | space_info); | 5004 | space_info); |
5006 | ticket = list_first_entry(&space_info->tickets, | 5005 | ticket = list_first_entry(&space_info->tickets, |
5007 | struct reserve_ticket, list); | 5006 | struct reserve_ticket, list); |
5008 | if (last_ticket == ticket) { | 5007 | if (last_tickets_id == space_info->tickets_id) { |
5009 | flush_state++; | 5008 | flush_state++; |
5010 | } else { | 5009 | } else { |
5011 | last_ticket = ticket; | 5010 | last_tickets_id = space_info->tickets_id; |
5012 | flush_state = FLUSH_DELAYED_ITEMS_NR; | 5011 | flush_state = FLUSH_DELAYED_ITEMS_NR; |
5013 | if (commit_cycles) | 5012 | if (commit_cycles) |
5014 | commit_cycles--; | 5013 | commit_cycles--; |
@@ -5384,6 +5383,7 @@ again: | |||
5384 | list_del_init(&ticket->list); | 5383 | list_del_init(&ticket->list); |
5385 | num_bytes -= ticket->bytes; | 5384 | num_bytes -= ticket->bytes; |
5386 | ticket->bytes = 0; | 5385 | ticket->bytes = 0; |
5386 | space_info->tickets_id++; | ||
5387 | wake_up(&ticket->wait); | 5387 | wake_up(&ticket->wait); |
5388 | } else { | 5388 | } else { |
5389 | ticket->bytes -= num_bytes; | 5389 | ticket->bytes -= num_bytes; |
@@ -5426,6 +5426,7 @@ again: | |||
5426 | num_bytes -= ticket->bytes; | 5426 | num_bytes -= ticket->bytes; |
5427 | space_info->bytes_may_use += ticket->bytes; | 5427 | space_info->bytes_may_use += ticket->bytes; |
5428 | ticket->bytes = 0; | 5428 | ticket->bytes = 0; |
5429 | space_info->tickets_id++; | ||
5429 | wake_up(&ticket->wait); | 5430 | wake_up(&ticket->wait); |
5430 | } else { | 5431 | } else { |
5431 | trace_btrfs_space_reservation(fs_info, "space_info", | 5432 | trace_btrfs_space_reservation(fs_info, "space_info", |