diff options
-rw-r--r-- | fs/btrfs/free-space-cache.c | 140 | ||||
-rw-r--r-- | fs/btrfs/tests/free-space-tests.c | 514 |
2 files changed, 653 insertions, 1 deletions
diff --git a/fs/btrfs/free-space-cache.c b/fs/btrfs/free-space-cache.c index 2f0fe1028e51..33848196550e 100644 --- a/fs/btrfs/free-space-cache.c +++ b/fs/btrfs/free-space-cache.c @@ -1997,6 +1997,128 @@ static bool try_merge_free_space(struct btrfs_free_space_ctl *ctl, return merged; } +static bool steal_from_bitmap_to_end(struct btrfs_free_space_ctl *ctl, + struct btrfs_free_space *info, + bool update_stat) +{ + struct btrfs_free_space *bitmap; + unsigned long i; + unsigned long j; + const u64 end = info->offset + info->bytes; + const u64 bitmap_offset = offset_to_bitmap(ctl, end); + u64 bytes; + + bitmap = tree_search_offset(ctl, bitmap_offset, 1, 0); + if (!bitmap) + return false; + + i = offset_to_bit(bitmap->offset, ctl->unit, end); + j = find_next_zero_bit(bitmap->bitmap, BITS_PER_BITMAP, i); + if (j == i) + return false; + bytes = (j - i) * ctl->unit; + info->bytes += bytes; + + if (update_stat) + bitmap_clear_bits(ctl, bitmap, end, bytes); + else + __bitmap_clear_bits(ctl, bitmap, end, bytes); + + if (!bitmap->bytes) + free_bitmap(ctl, bitmap); + + return true; +} + +static bool steal_from_bitmap_to_front(struct btrfs_free_space_ctl *ctl, + struct btrfs_free_space *info, + bool update_stat) +{ + struct btrfs_free_space *bitmap; + u64 bitmap_offset; + unsigned long i; + unsigned long j; + unsigned long prev_j; + u64 bytes; + + bitmap_offset = offset_to_bitmap(ctl, info->offset); + /* If we're on a boundary, try the previous logical bitmap. */ + if (bitmap_offset == info->offset) { + if (info->offset == 0) + return false; + bitmap_offset = offset_to_bitmap(ctl, info->offset - 1); + } + + bitmap = tree_search_offset(ctl, bitmap_offset, 1, 0); + if (!bitmap) + return false; + + i = offset_to_bit(bitmap->offset, ctl->unit, info->offset) - 1; + j = 0; + prev_j = (unsigned long)-1; + for_each_clear_bit_from(j, bitmap->bitmap, BITS_PER_BITMAP) { + if (j > i) + break; + prev_j = j; + } + if (prev_j == i) + return false; + + if (prev_j == (unsigned long)-1) + bytes = (i + 1) * ctl->unit; + else + bytes = (i - prev_j) * ctl->unit; + + info->offset -= bytes; + info->bytes += bytes; + + if (update_stat) + bitmap_clear_bits(ctl, bitmap, info->offset, bytes); + else + __bitmap_clear_bits(ctl, bitmap, info->offset, bytes); + + if (!bitmap->bytes) + free_bitmap(ctl, bitmap); + + return true; +} + +/* + * We prefer always to allocate from extent entries, both for clustered and + * non-clustered allocation requests. So when attempting to add a new extent + * entry, try to see if there's adjacent free space in bitmap entries, and if + * there is, migrate that space from the bitmaps to the extent. + * Like this we get better chances of satisfying space allocation requests + * because we attempt to satisfy them based on a single cache entry, and never + * on 2 or more entries - even if the entries represent a contiguous free space + * region (e.g. 1 extent entry + 1 bitmap entry starting where the extent entry + * ends). + */ +static void steal_from_bitmap(struct btrfs_free_space_ctl *ctl, + struct btrfs_free_space *info, + bool update_stat) +{ + /* + * Only work with disconnected entries, as we can change their offset, + * and must be extent entries. + */ + ASSERT(!info->bitmap); + ASSERT(RB_EMPTY_NODE(&info->offset_index)); + + if (ctl->total_bitmaps > 0) { + bool stole_end; + bool stole_front = false; + + stole_end = steal_from_bitmap_to_end(ctl, info, update_stat); + if (ctl->total_bitmaps > 0) + stole_front = steal_from_bitmap_to_front(ctl, info, + update_stat); + + if (stole_end || stole_front) + try_merge_free_space(ctl, info, update_stat); + } +} + int __btrfs_add_free_space(struct btrfs_free_space_ctl *ctl, u64 offset, u64 bytes) { @@ -2009,6 +2131,7 @@ int __btrfs_add_free_space(struct btrfs_free_space_ctl *ctl, info->offset = offset; info->bytes = bytes; + RB_CLEAR_NODE(&info->offset_index); spin_lock(&ctl->tree_lock); @@ -2028,6 +2151,14 @@ int __btrfs_add_free_space(struct btrfs_free_space_ctl *ctl, goto out; } link: + /* + * Only steal free space from adjacent bitmaps if we're sure we're not + * going to add the new free space to existing bitmap entries - because + * that would mean unnecessary work that would be reverted. Therefore + * attempt to steal space from bitmaps if we're adding an extent entry. + */ + steal_from_bitmap(ctl, info, true); + ret = link_free_space(ctl, info); if (ret) kmem_cache_free(btrfs_free_space_cachep, info); @@ -2204,10 +2335,13 @@ __btrfs_return_cluster_to_free_space( entry = rb_entry(node, struct btrfs_free_space, offset_index); node = rb_next(&entry->offset_index); rb_erase(&entry->offset_index, &cluster->root); + RB_CLEAR_NODE(&entry->offset_index); bitmap = (entry->bitmap != NULL); - if (!bitmap) + if (!bitmap) { try_merge_free_space(ctl, entry, false); + steal_from_bitmap(ctl, entry, false); + } tree_insert_offset(&ctl->free_space_offset, entry->offset, &entry->offset_index, bitmap); } @@ -3175,6 +3309,7 @@ again: map = NULL; add_new_bitmap(ctl, info, offset); bitmap_info = info; + info = NULL; } bytes_added = add_bytes_to_bitmap(ctl, bitmap_info, offset, bytes); @@ -3185,6 +3320,8 @@ again: if (bytes) goto again; + if (info) + kmem_cache_free(btrfs_free_space_cachep, info); if (map) kfree(map); return 0; @@ -3259,6 +3396,7 @@ have_info: goto have_info; } + ret = 0; goto out; } diff --git a/fs/btrfs/tests/free-space-tests.c b/fs/btrfs/tests/free-space-tests.c index c8d9ddf84c69..d78ae10d0446 100644 --- a/fs/btrfs/tests/free-space-tests.c +++ b/fs/btrfs/tests/free-space-tests.c @@ -40,6 +40,7 @@ static struct btrfs_block_group_cache *init_test_block_group(void) cache->key.offset = 1024 * 1024 * 1024; cache->key.type = BTRFS_BLOCK_GROUP_ITEM_KEY; cache->sectorsize = 4096; + cache->full_stripe_len = 4096; spin_lock_init(&cache->lock); INIT_LIST_HEAD(&cache->list); @@ -364,6 +365,517 @@ static int test_bitmaps_and_extents(struct btrfs_block_group_cache *cache) return 0; } +/* Used by test_steal_space_from_bitmap_to_extent(). */ +static bool test_use_bitmap(struct btrfs_free_space_ctl *ctl, + struct btrfs_free_space *info) +{ + return ctl->free_extents > 0; +} + +/* Used by test_steal_space_from_bitmap_to_extent(). */ +static int +check_num_extents_and_bitmaps(const struct btrfs_block_group_cache *cache, + const int num_extents, + const int num_bitmaps) +{ + if (cache->free_space_ctl->free_extents != num_extents) { + test_msg("Incorrect # of extent entries in the cache: %d, expected %d\n", + cache->free_space_ctl->free_extents, num_extents); + return -EINVAL; + } + if (cache->free_space_ctl->total_bitmaps != num_bitmaps) { + test_msg("Incorrect # of extent entries in the cache: %d, expected %d\n", + cache->free_space_ctl->total_bitmaps, num_bitmaps); + return -EINVAL; + } + return 0; +} + +/* Used by test_steal_space_from_bitmap_to_extent(). */ +static int check_cache_empty(struct btrfs_block_group_cache *cache) +{ + u64 offset; + u64 max_extent_size; + + /* + * Now lets confirm that there's absolutely no free space left to + * allocate. + */ + if (cache->free_space_ctl->free_space != 0) { + test_msg("Cache free space is not 0\n"); + return -EINVAL; + } + + /* And any allocation request, no matter how small, should fail now. */ + offset = btrfs_find_space_for_alloc(cache, 0, 4096, 0, + &max_extent_size); + if (offset != 0) { + test_msg("Space allocation did not fail, returned offset: %llu", + offset); + return -EINVAL; + } + + /* And no extent nor bitmap entries in the cache anymore. */ + return check_num_extents_and_bitmaps(cache, 0, 0); +} + +/* + * Before we were able to steal free space from a bitmap entry to an extent + * entry, we could end up with 2 entries representing a contiguous free space. + * One would be an extent entry and the other a bitmap entry. Since in order + * to allocate space to a caller we use only 1 entry, we couldn't return that + * whole range to the caller if it was requested. This forced the caller to + * either assume ENOSPC or perform several smaller space allocations, which + * wasn't optimal as they could be spread all over the block group while under + * concurrency (extra overhead and fragmentation). + * + * This stealing approach is benefical, since we always prefer to allocate from + * extent entries, both for clustered and non-clustered allocation requests. + */ +static int +test_steal_space_from_bitmap_to_extent(struct btrfs_block_group_cache *cache) +{ + int ret; + u64 offset; + u64 max_extent_size; + + bool (*use_bitmap_op)(struct btrfs_free_space_ctl *, + struct btrfs_free_space *); + + test_msg("Running space stealing from bitmap to extent\n"); + + /* + * For this test, we want to ensure we end up with an extent entry + * immediately adjacent to a bitmap entry, where the bitmap starts + * at an offset where the extent entry ends. We keep adding and + * removing free space to reach into this state, but to get there + * we need to reach a point where marking new free space doesn't + * result in adding new extent entries or merging the new space + * with existing extent entries - the space ends up being marked + * in an existing bitmap that covers the new free space range. + * + * To get there, we need to reach the threshold defined set at + * cache->free_space_ctl->extents_thresh, which currently is + * 256 extents on a x86_64 system at least, and a few other + * conditions (check free_space_cache.c). Instead of making the + * test much longer and complicated, use a "use_bitmap" operation + * that forces use of bitmaps as soon as we have at least 1 + * extent entry. + */ + use_bitmap_op = cache->free_space_ctl->op->use_bitmap; + cache->free_space_ctl->op->use_bitmap = test_use_bitmap; + + /* + * Extent entry covering free space range [128Mb - 256Kb, 128Mb - 128Kb[ + */ + ret = test_add_free_space_entry(cache, 128 * 1024 * 1024 - 256 * 1024, + 128 * 1024, 0); + if (ret) { + test_msg("Couldn't add extent entry %d\n", ret); + return ret; + } + + /* Bitmap entry covering free space range [128Mb + 512Kb, 256Mb[ */ + ret = test_add_free_space_entry(cache, 128 * 1024 * 1024 + 512 * 1024, + 128 * 1024 * 1024 - 512 * 1024, 1); + if (ret) { + test_msg("Couldn't add bitmap entry %d\n", ret); + return ret; + } + + ret = check_num_extents_and_bitmaps(cache, 2, 1); + if (ret) + return ret; + + /* + * Now make only the first 256Kb of the bitmap marked as free, so that + * we end up with only the following ranges marked as free space: + * + * [128Mb - 256Kb, 128Mb - 128Kb[ + * [128Mb + 512Kb, 128Mb + 768Kb[ + */ + ret = btrfs_remove_free_space(cache, + 128 * 1024 * 1024 + 768 * 1024, + 128 * 1024 * 1024 - 768 * 1024); + if (ret) { + test_msg("Failed to free part of bitmap space %d\n", ret); + return ret; + } + + /* Confirm that only those 2 ranges are marked as free. */ + if (!test_check_exists(cache, 128 * 1024 * 1024 - 256 * 1024, + 128 * 1024)) { + test_msg("Free space range missing\n"); + return -ENOENT; + } + if (!test_check_exists(cache, 128 * 1024 * 1024 + 512 * 1024, + 256 * 1024)) { + test_msg("Free space range missing\n"); + return -ENOENT; + } + + /* + * Confirm that the bitmap range [128Mb + 768Kb, 256Mb[ isn't marked + * as free anymore. + */ + if (test_check_exists(cache, 128 * 1024 * 1024 + 768 * 1024, + 128 * 1024 * 1024 - 768 * 1024)) { + test_msg("Bitmap region not removed from space cache\n"); + return -EINVAL; + } + + /* + * Confirm that the region [128Mb + 256Kb, 128Mb + 512Kb[, which is + * covered by the bitmap, isn't marked as free. + */ + if (test_check_exists(cache, 128 * 1024 * 1024 + 256 * 1024, + 256 * 1024)) { + test_msg("Invalid bitmap region marked as free\n"); + return -EINVAL; + } + + /* + * Confirm that the region [128Mb, 128Mb + 256Kb[, which is covered + * by the bitmap too, isn't marked as free either. + */ + if (test_check_exists(cache, 128 * 1024 * 1024, + 256 * 1024)) { + test_msg("Invalid bitmap region marked as free\n"); + return -EINVAL; + } + + /* + * Now lets mark the region [128Mb, 128Mb + 512Kb[ as free too. But, + * lets make sure the free space cache marks it as free in the bitmap, + * and doesn't insert a new extent entry to represent this region. + */ + ret = btrfs_add_free_space(cache, 128 * 1024 * 1024, 512 * 1024); + if (ret) { + test_msg("Error adding free space: %d\n", ret); + return ret; + } + /* Confirm the region is marked as free. */ + if (!test_check_exists(cache, 128 * 1024 * 1024, 512 * 1024)) { + test_msg("Bitmap region not marked as free\n"); + return -ENOENT; + } + + /* + * Confirm that no new extent entries or bitmap entries were added to + * the cache after adding that free space region. + */ + ret = check_num_extents_and_bitmaps(cache, 2, 1); + if (ret) + return ret; + + /* + * Now lets add a small free space region to the right of the previous + * one, which is not contiguous with it and is part of the bitmap too. + * The goal is to test that the bitmap entry space stealing doesn't + * steal this space region. + */ + ret = btrfs_add_free_space(cache, 128 * 1024 * 1024 + 16 * 1024 * 1024, + 4096); + if (ret) { + test_msg("Error adding free space: %d\n", ret); + return ret; + } + + /* + * Confirm that no new extent entries or bitmap entries were added to + * the cache after adding that free space region. + */ + ret = check_num_extents_and_bitmaps(cache, 2, 1); + if (ret) + return ret; + + /* + * Now mark the region [128Mb - 128Kb, 128Mb[ as free too. This will + * expand the range covered by the existing extent entry that represents + * the free space [128Mb - 256Kb, 128Mb - 128Kb[. + */ + ret = btrfs_add_free_space(cache, 128 * 1024 * 1024 - 128 * 1024, + 128 * 1024); + if (ret) { + test_msg("Error adding free space: %d\n", ret); + return ret; + } + /* Confirm the region is marked as free. */ + if (!test_check_exists(cache, 128 * 1024 * 1024 - 128 * 1024, + 128 * 1024)) { + test_msg("Extent region not marked as free\n"); + return -ENOENT; + } + + /* + * Confirm that our extent entry didn't stole all free space from the + * bitmap, because of the small 4Kb free space region. + */ + ret = check_num_extents_and_bitmaps(cache, 2, 1); + if (ret) + return ret; + + /* + * So now we have the range [128Mb - 256Kb, 128Mb + 768Kb[ as free + * space. Without stealing bitmap free space into extent entry space, + * we would have all this free space represented by 2 entries in the + * cache: + * + * extent entry covering range: [128Mb - 256Kb, 128Mb[ + * bitmap entry covering range: [128Mb, 128Mb + 768Kb[ + * + * Attempting to allocate the whole free space (1Mb) would fail, because + * we can't allocate from multiple entries. + * With the bitmap free space stealing, we get a single extent entry + * that represents the 1Mb free space, and therefore we're able to + * allocate the whole free space at once. + */ + if (!test_check_exists(cache, 128 * 1024 * 1024 - 256 * 1024, + 1 * 1024 * 1024)) { + test_msg("Expected region not marked as free\n"); + return -ENOENT; + } + + if (cache->free_space_ctl->free_space != (1 * 1024 * 1024 + 4096)) { + test_msg("Cache free space is not 1Mb + 4Kb\n"); + return -EINVAL; + } + + offset = btrfs_find_space_for_alloc(cache, + 0, 1 * 1024 * 1024, 0, + &max_extent_size); + if (offset != (128 * 1024 * 1024 - 256 * 1024)) { + test_msg("Failed to allocate 1Mb from space cache, returned offset is: %llu\n", + offset); + return -EINVAL; + } + + /* All that remains is a 4Kb free space region in a bitmap. Confirm. */ + ret = check_num_extents_and_bitmaps(cache, 1, 1); + if (ret) + return ret; + + if (cache->free_space_ctl->free_space != 4096) { + test_msg("Cache free space is not 4Kb\n"); + return -EINVAL; + } + + offset = btrfs_find_space_for_alloc(cache, + 0, 4096, 0, + &max_extent_size); + if (offset != (128 * 1024 * 1024 + 16 * 1024 * 1024)) { + test_msg("Failed to allocate 4Kb from space cache, returned offset is: %llu\n", + offset); + return -EINVAL; + } + + ret = check_cache_empty(cache); + if (ret) + return ret; + + __btrfs_remove_free_space_cache(cache->free_space_ctl); + + /* + * Now test a similar scenario, but where our extent entry is located + * to the right of the bitmap entry, so that we can check that stealing + * space from a bitmap to the front of an extent entry works. + */ + + /* + * Extent entry covering free space range [128Mb + 128Kb, 128Mb + 256Kb[ + */ + ret = test_add_free_space_entry(cache, 128 * 1024 * 1024 + 128 * 1024, + 128 * 1024, 0); + if (ret) { + test_msg("Couldn't add extent entry %d\n", ret); + return ret; + } + + /* Bitmap entry covering free space range [0, 128Mb - 512Kb[ */ + ret = test_add_free_space_entry(cache, 0, + 128 * 1024 * 1024 - 512 * 1024, 1); + if (ret) { + test_msg("Couldn't add bitmap entry %d\n", ret); + return ret; + } + + ret = check_num_extents_and_bitmaps(cache, 2, 1); + if (ret) + return ret; + + /* + * Now make only the last 256Kb of the bitmap marked as free, so that + * we end up with only the following ranges marked as free space: + * + * [128Mb + 128b, 128Mb + 256Kb[ + * [128Mb - 768Kb, 128Mb - 512Kb[ + */ + ret = btrfs_remove_free_space(cache, + 0, + 128 * 1024 * 1024 - 768 * 1024); + if (ret) { + test_msg("Failed to free part of bitmap space %d\n", ret); + return ret; + } + + /* Confirm that only those 2 ranges are marked as free. */ + if (!test_check_exists(cache, 128 * 1024 * 1024 + 128 * 1024, + 128 * 1024)) { + test_msg("Free space range missing\n"); + return -ENOENT; + } + if (!test_check_exists(cache, 128 * 1024 * 1024 - 768 * 1024, + 256 * 1024)) { + test_msg("Free space range missing\n"); + return -ENOENT; + } + + /* + * Confirm that the bitmap range [0, 128Mb - 768Kb[ isn't marked + * as free anymore. + */ + if (test_check_exists(cache, 0, + 128 * 1024 * 1024 - 768 * 1024)) { + test_msg("Bitmap region not removed from space cache\n"); + return -EINVAL; + } + + /* + * Confirm that the region [128Mb - 512Kb, 128Mb[, which is + * covered by the bitmap, isn't marked as free. + */ + if (test_check_exists(cache, 128 * 1024 * 1024 - 512 * 1024, + 512 * 1024)) { + test_msg("Invalid bitmap region marked as free\n"); + return -EINVAL; + } + + /* + * Now lets mark the region [128Mb - 512Kb, 128Mb[ as free too. But, + * lets make sure the free space cache marks it as free in the bitmap, + * and doesn't insert a new extent entry to represent this region. + */ + ret = btrfs_add_free_space(cache, 128 * 1024 * 1024 - 512 * 1024, + 512 * 1024); + if (ret) { + test_msg("Error adding free space: %d\n", ret); + return ret; + } + /* Confirm the region is marked as free. */ + if (!test_check_exists(cache, 128 * 1024 * 1024 - 512 * 1024, + 512 * 1024)) { + test_msg("Bitmap region not marked as free\n"); + return -ENOENT; + } + + /* + * Confirm that no new extent entries or bitmap entries were added to + * the cache after adding that free space region. + */ + ret = check_num_extents_and_bitmaps(cache, 2, 1); + if (ret) + return ret; + + /* + * Now lets add a small free space region to the left of the previous + * one, which is not contiguous with it and is part of the bitmap too. + * The goal is to test that the bitmap entry space stealing doesn't + * steal this space region. + */ + ret = btrfs_add_free_space(cache, 32 * 1024 * 1024, 8192); + if (ret) { + test_msg("Error adding free space: %d\n", ret); + return ret; + } + + /* + * Now mark the region [128Mb, 128Mb + 128Kb[ as free too. This will + * expand the range covered by the existing extent entry that represents + * the free space [128Mb + 128Kb, 128Mb + 256Kb[. + */ + ret = btrfs_add_free_space(cache, 128 * 1024 * 1024, 128 * 1024); + if (ret) { + test_msg("Error adding free space: %d\n", ret); + return ret; + } + /* Confirm the region is marked as free. */ + if (!test_check_exists(cache, 128 * 1024 * 1024, 128 * 1024)) { + test_msg("Extent region not marked as free\n"); + return -ENOENT; + } + + /* + * Confirm that our extent entry didn't stole all free space from the + * bitmap, because of the small 8Kb free space region. + */ + ret = check_num_extents_and_bitmaps(cache, 2, 1); + if (ret) + return ret; + + /* + * So now we have the range [128Mb - 768Kb, 128Mb + 256Kb[ as free + * space. Without stealing bitmap free space into extent entry space, + * we would have all this free space represented by 2 entries in the + * cache: + * + * extent entry covering range: [128Mb, 128Mb + 256Kb[ + * bitmap entry covering range: [128Mb - 768Kb, 128Mb[ + * + * Attempting to allocate the whole free space (1Mb) would fail, because + * we can't allocate from multiple entries. + * With the bitmap free space stealing, we get a single extent entry + * that represents the 1Mb free space, and therefore we're able to + * allocate the whole free space at once. + */ + if (!test_check_exists(cache, 128 * 1024 * 1024 - 768 * 1024, + 1 * 1024 * 1024)) { + test_msg("Expected region not marked as free\n"); + return -ENOENT; + } + + if (cache->free_space_ctl->free_space != (1 * 1024 * 1024 + 8192)) { + test_msg("Cache free space is not 1Mb + 8Kb\n"); + return -EINVAL; + } + + offset = btrfs_find_space_for_alloc(cache, + 0, 1 * 1024 * 1024, 0, + &max_extent_size); + if (offset != (128 * 1024 * 1024 - 768 * 1024)) { + test_msg("Failed to allocate 1Mb from space cache, returned offset is: %llu\n", + offset); + return -EINVAL; + } + + /* All that remains is a 8Kb free space region in a bitmap. Confirm. */ + ret = check_num_extents_and_bitmaps(cache, 1, 1); + if (ret) + return ret; + + if (cache->free_space_ctl->free_space != 8192) { + test_msg("Cache free space is not 8Kb\n"); + return -EINVAL; + } + + offset = btrfs_find_space_for_alloc(cache, + 0, 8192, 0, + &max_extent_size); + if (offset != (32 * 1024 * 1024)) { + test_msg("Failed to allocate 8Kb from space cache, returned offset is: %llu\n", + offset); + return -EINVAL; + } + + ret = check_cache_empty(cache); + if (ret) + return ret; + + cache->free_space_ctl->op->use_bitmap = use_bitmap_op; + __btrfs_remove_free_space_cache(cache->free_space_ctl); + + return 0; +} + int btrfs_test_free_space_cache(void) { struct btrfs_block_group_cache *cache; @@ -386,6 +898,8 @@ int btrfs_test_free_space_cache(void) ret = test_bitmaps_and_extents(cache); if (ret) goto out; + + ret = test_steal_space_from_bitmap_to_extent(cache); out: __btrfs_remove_free_space_cache(cache->free_space_ctl); kfree(cache->free_space_ctl); |