| Message ID | 20181203152459.21630-8-josef@toxicpanda.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Enospc cleanups and fixeS | expand |
On Mon, Dec 03, 2018 at 10:24:58AM -0500, Josef Bacik wrote: > For FLUSH_LIMIT flushers (think evict, truncate) we can deadlock when > running delalloc because we may be holding a tree lock. We can also > deadlock with delayed refs rsv's that are running via the committing > mechanism. The only safe operations for FLUSH_LIMIT is to run the > delayed operations and to allocate chunks, everything else has the > potential to deadlock. Future proof this by explicitly specifying the > states that FLUSH_LIMIT is allowed to use. This will keep us from > introducing bugs later on when adding new flush states. > > Signed-off-by: Josef Bacik <josef@toxicpanda.com> Reviewed-by: David Sterba <dsterba@suse.com>
On 3.12.18 г. 17:24 ч., Josef Bacik wrote: > For FLUSH_LIMIT flushers (think evict, truncate) we can deadlock when > running delalloc because we may be holding a tree lock. We can also > deadlock with delayed refs rsv's that are running via the committing > mechanism. The only safe operations for FLUSH_LIMIT is to run the > delayed operations and to allocate chunks, everything else has the > potential to deadlock. Future proof this by explicitly specifying the > states that FLUSH_LIMIT is allowed to use. This will keep us from > introducing bugs later on when adding new flush states. > > Signed-off-by: Josef Bacik <josef@toxicpanda.com> Reviewed-by: Nikolay Borisov <nborisov@suse.com> > --- > fs/btrfs/extent-tree.c | 21 ++++++++++----------- > 1 file changed, 10 insertions(+), 11 deletions(-) > > diff --git a/fs/btrfs/extent-tree.c b/fs/btrfs/extent-tree.c > index 0e1a499035ac..ab9d915d9289 100644 > --- a/fs/btrfs/extent-tree.c > +++ b/fs/btrfs/extent-tree.c > @@ -5123,12 +5123,18 @@ void btrfs_init_async_reclaim_work(struct work_struct *work) > INIT_WORK(work, btrfs_async_reclaim_metadata_space); > } > > +static const enum btrfs_flush_state priority_flush_states[] = { > + FLUSH_DELAYED_ITEMS_NR, > + FLUSH_DELAYED_ITEMS, > + ALLOC_CHUNK, > +}; > + > static void priority_reclaim_metadata_space(struct btrfs_fs_info *fs_info, > struct btrfs_space_info *space_info, > struct reserve_ticket *ticket) > { > u64 to_reclaim; > - int flush_state = FLUSH_DELAYED_ITEMS_NR; > + int flush_state = 0; > > spin_lock(&space_info->lock); > to_reclaim = btrfs_calc_reclaim_metadata_size(fs_info, space_info, > @@ -5140,7 +5146,8 @@ static void priority_reclaim_metadata_space(struct btrfs_fs_info *fs_info, > spin_unlock(&space_info->lock); > > do { > - flush_space(fs_info, space_info, to_reclaim, flush_state); > + flush_space(fs_info, space_info, to_reclaim, > + priority_flush_states[flush_state]); > flush_state++; > spin_lock(&space_info->lock); > if (ticket->bytes == 0) { > @@ -5148,15 +5155,7 @@ static void priority_reclaim_metadata_space(struct btrfs_fs_info *fs_info, > return; > } > spin_unlock(&space_info->lock); > - > - /* > - * Priority flushers can't wait on delalloc without > - * deadlocking. > - */ > - if (flush_state == FLUSH_DELALLOC || > - flush_state == FLUSH_DELALLOC_WAIT) > - flush_state = ALLOC_CHUNK; > - } while (flush_state < COMMIT_TRANS); > + } while (flush_state < ARRAY_SIZE(priority_flush_states)); > } > > static int wait_reserve_ticket(struct btrfs_fs_info *fs_info, >
diff --git a/fs/btrfs/extent-tree.c b/fs/btrfs/extent-tree.c index 0e1a499035ac..ab9d915d9289 100644 --- a/fs/btrfs/extent-tree.c +++ b/fs/btrfs/extent-tree.c @@ -5123,12 +5123,18 @@ void btrfs_init_async_reclaim_work(struct work_struct *work) INIT_WORK(work, btrfs_async_reclaim_metadata_space); } +static const enum btrfs_flush_state priority_flush_states[] = { + FLUSH_DELAYED_ITEMS_NR, + FLUSH_DELAYED_ITEMS, + ALLOC_CHUNK, +}; + static void priority_reclaim_metadata_space(struct btrfs_fs_info *fs_info, struct btrfs_space_info *space_info, struct reserve_ticket *ticket) { u64 to_reclaim; - int flush_state = FLUSH_DELAYED_ITEMS_NR; + int flush_state = 0; spin_lock(&space_info->lock); to_reclaim = btrfs_calc_reclaim_metadata_size(fs_info, space_info, @@ -5140,7 +5146,8 @@ static void priority_reclaim_metadata_space(struct btrfs_fs_info *fs_info, spin_unlock(&space_info->lock); do { - flush_space(fs_info, space_info, to_reclaim, flush_state); + flush_space(fs_info, space_info, to_reclaim, + priority_flush_states[flush_state]); flush_state++; spin_lock(&space_info->lock); if (ticket->bytes == 0) { @@ -5148,15 +5155,7 @@ static void priority_reclaim_metadata_space(struct btrfs_fs_info *fs_info, return; } spin_unlock(&space_info->lock); - - /* - * Priority flushers can't wait on delalloc without - * deadlocking. - */ - if (flush_state == FLUSH_DELALLOC || - flush_state == FLUSH_DELALLOC_WAIT) - flush_state = ALLOC_CHUNK; - } while (flush_state < COMMIT_TRANS); + } while (flush_state < ARRAY_SIZE(priority_flush_states)); } static int wait_reserve_ticket(struct btrfs_fs_info *fs_info,
For FLUSH_LIMIT flushers (think evict, truncate) we can deadlock when running delalloc because we may be holding a tree lock. We can also deadlock with delayed refs rsv's that are running via the committing mechanism. The only safe operations for FLUSH_LIMIT is to run the delayed operations and to allocate chunks, everything else has the potential to deadlock. Future proof this by explicitly specifying the states that FLUSH_LIMIT is allowed to use. This will keep us from introducing bugs later on when adding new flush states. Signed-off-by: Josef Bacik <josef@toxicpanda.com> --- fs/btrfs/extent-tree.c | 21 ++++++++++----------- 1 file changed, 10 insertions(+), 11 deletions(-)