On Fri, Oct 12, 2018 at 02:18:19PM +0800, Qu Wenruo wrote: > We have a complex loop design for find_free_extent(), that has different > behavior for each loop, some even includes new chunk allocation. > > Instead of putting such a long code into find_free_extent() and makes it > harder to read, just extract them into find_free_extent_update_loop(). > > With all the cleanups, the main find_free_extent() should be pretty > barebone: > > find_free_extent() > |- Iterate through all block groups > | |- Get a valid block group > | |- Try to do clustered allocation in that block group > | |- Try to do unclustered allocation in that block group > | |- Check if the result is valid > | | |- If valid, then exit > | |- Jump to next block group > | > |- Push harder to find free extents > |- If not found, re-iterate all block groups > > Signed-off-by: Qu Wenruo <w...@suse.com> > Reviewed-by: Su Yue <suy.f...@cn.fujitsu.com> > --- > fs/btrfs/extent-tree.c | 217 ++++++++++++++++++++++------------------- > 1 file changed, 117 insertions(+), 100 deletions(-) > > diff --git a/fs/btrfs/extent-tree.c b/fs/btrfs/extent-tree.c > index e6bfa91af41c..938569d2c583 100644 > --- a/fs/btrfs/extent-tree.c > +++ b/fs/btrfs/extent-tree.c > @@ -7236,7 +7236,9 @@ struct find_free_extent_ctl { > /* RAID index, converted from flags */ > int index; > > - /* Current loop number */ > + /* > + * Current loop number, check find_free_extent_update_loop() for details > + */ > int loop; > > /* > @@ -7433,6 +7435,117 @@ static int find_free_extent_unclustered(struct > btrfs_block_group_cache *bg, > return 0; > } > > +/* > + * Return >0 means caller needs to re-search for free extent > + * Return 0 means we have the needed free extent. > + * Return <0 means we failed to locate any free extent. > + */ > +static int find_free_extent_update_loop(struct btrfs_fs_info *fs_info, > + struct btrfs_free_cluster *last_ptr, > + struct btrfs_key *ins, > + struct find_free_extent_ctl *ffe_ctl, > + int full_search, bool use_cluster) > +{ > + struct btrfs_root *root = fs_info->extent_root; > + int ret; > + > + if ((ffe_ctl->loop == LOOP_CACHING_NOWAIT) && > + ffe_ctl->have_caching_bg && !ffe_ctl->orig_have_caching_bg) > + ffe_ctl->orig_have_caching_bg = true; > + > + if (!ins->objectid && ffe_ctl->loop >= LOOP_CACHING_WAIT && > + ffe_ctl->have_caching_bg) > + return 1; > + > + if (!ins->objectid && ++(ffe_ctl->index) < BTRFS_NR_RAID_TYPES) > + return 1; > + > + /* > + * LOOP_CACHING_NOWAIT, search partially cached block groups, kicking > + * caching kthreads as we move along > + * LOOP_CACHING_WAIT, search everything, and wait if our bg is caching > + * LOOP_ALLOC_CHUNK, force a chunk allocation and try again > + * LOOP_NO_EMPTY_SIZE, set empty_size and empty_cluster to 0 and try > + * again > + */ > + if (!ins->objectid && ffe_ctl->loop < LOOP_NO_EMPTY_SIZE) { > + ffe_ctl->index = 0; > + if (ffe_ctl->loop == LOOP_CACHING_NOWAIT) { > + /* > + * We want to skip the LOOP_CACHING_WAIT step if we > + * don't have any uncached bgs and we've already done a > + * full search through. > + */ > + if (ffe_ctl->orig_have_caching_bg || !full_search) > + ffe_ctl->loop = LOOP_CACHING_WAIT; > + else > + ffe_ctl->loop = LOOP_ALLOC_CHUNK; > + } else { > + ffe_ctl->loop++; > + } > + > + if (ffe_ctl->loop == LOOP_ALLOC_CHUNK) { > + struct btrfs_trans_handle *trans; > + int exist = 0; > + > + trans = current->journal_info; > + if (trans) > + exist = 1; > + else > + trans = btrfs_join_transaction(root); > + > + if (IS_ERR(trans)) { > + ret = PTR_ERR(trans); > + return ret; > + } > + > + ret = do_chunk_alloc(trans, ffe_ctl->flags, > + CHUNK_ALLOC_FORCE); > + > + /* > + * If we can't allocate a new chunk we've already looped > + * through at least once, move on to the NO_EMPTY_SIZE > + * case. > + */ > + if (ret == -ENOSPC) > + ffe_ctl->loop = LOOP_NO_EMPTY_SIZE; > + > + /* Do not bail out on ENOSPC since we can do more. */ > + if (ret < 0 && ret != -ENOSPC) > + btrfs_abort_transaction(trans, ret); > + else > + ret = 0; > + if (!exist) > + btrfs_end_transaction(trans); > + if (ret) > + return ret; > + } > + > + if (ffe_ctl->loop == LOOP_NO_EMPTY_SIZE) { > + /* > + * Don't loop again if we already have no empty_size and > + * no empty_cluster. > + */ > + if (ffe_ctl->empty_size == 0 && > + ffe_ctl->empty_cluster == 0) > + return -ENOSPC; > + ffe_ctl->empty_size = 0; > + ffe_ctl->empty_cluster = 0; > + } > + return 1; > + } else if (!ins->objectid) { > + ret = -ENOSPC; > + } else if (ins->objectid) { > + if (!use_cluster && last_ptr) { > + spin_lock(&last_ptr->lock); > + last_ptr->window_start = ins->objectid; > + spin_unlock(&last_ptr->lock); > + } > + ret = 0; > + } > + return ret; > +}
Rework this so the if (ins->objectid) blah is the first thing, that way you don't have to do the if (!ins->objectid && <other things>) for all the other if statements here. The fast path should be the first thing, then we can deal with all of the other crap last. Thanks, Josef