04:18, Josef Bacik wrote:
> We have been creating bitmaps for small extents unconditionally forever.  This
> was great when testing to make sure the bitmap stuff was working, but is
> overkill normally.  So instead of always adding small chunks of free space to
> bitmaps, only start doing it if we go past half of our extent threshold.  This
> will keeps us from creating a bitmap for just one small free extent at the 
> front
> of the block group, and will make the allocator a little faster as a result.
> Thanks,
> 

I was wondering this strategy when reading the code, so this patch
looks good to me.

Just a small nit:

> Signed-off-by: Josef Bacik <[email protected]>
> ---
>  fs/btrfs/free-space-cache.c |   19 ++++++++++++++++---
>  1 files changed, 16 insertions(+), 3 deletions(-)
> 
> diff --git a/fs/btrfs/free-space-cache.c b/fs/btrfs/free-space-cache.c
> index 63776ae..7a808d7 100644
> --- a/fs/btrfs/free-space-cache.c
> +++ b/fs/btrfs/free-space-cache.c
> @@ -1287,9 +1287,22 @@ static int insert_into_bitmap(struct 
> btrfs_block_group_cache *block_group,
>        * If we are below the extents threshold then we can add this as an
>        * extent, and don't have to deal with the bitmap
>        */
> -     if (block_group->free_extents < block_group->extents_thresh &&
> -         info->bytes > block_group->sectorsize * 4)
> -             return 0;
> +     if (block_group->free_extents < block_group->extents_thresh) {
> +             /*
> +              * If this block group has some small extents we don't want to
> +              * use up all of our free slots in the cache with them, we want
> +              * to reserve them to larger extents, however if we have plent
> +              * of cache left then go ahead an dadd them, no sense in adding
> +              * the overhead of a bitmap if we don't have to.
> +              */
> +             if (info->bytes < block_group->sectorsize * 4) {

This also changes how we define a small extent (from 4 sectorsize to 3).
Is it intended?

> +                     if ((block_group->free_extents * 2) <=

The parentheses isn't necessary nor help in readability I think.

> +                         block_group->extents_thresh)
> +                             return 0;
> +             } else {
> +                     return 0;
> +             }
> +     }
>  
>       /*
>        * some block groups are so tiny they can't be enveloped by a bitmap, so

--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to