diff options
author | Josef Bacik | 2021-11-18 16:33:14 -0500 |
---|---|---|
committer | David Sterba | 2022-01-03 15:09:46 +0100 |
commit | 950575c023aabfeac506cae02917c32eae1f553e (patch) | |
tree | 2a1d27e91cdfc69dfc39ad3a02afe97af0c5dc37 | |
parent | 83f1b68002c208329412cf9f998c90b3326828d2 (diff) |
btrfs: only use ->max_extent_size if it is set in the bitmap
While adding self tests for my space index change I was hitting a
problem where the space indexed tree wasn't returning the expected
->max_extent_size. This is because we will skip searching any entry
that doesn't have ->bytes >= the amount of bytes we want. However we'll
still set the max_extent_size based on that entry. The problem is if we
don't search the bitmap we won't have ->max_extent_size set properly, so
we can't really trust it.
This doesn't really result in a problem per-se, it can just result in us
not finding contiguous area that may exist. Fix the max_extent_size
helper to return ->bytes if ->max_extent_size isn't set, and add a big
comment explaining why we're doing this.
Signed-off-by: Josef Bacik <josef@toxicpanda.com>
Signed-off-by: David Sterba <dsterba@suse.com>
-rw-r--r-- | fs/btrfs/free-space-cache.c | 26 |
1 files changed, 25 insertions, 1 deletions
diff --git a/fs/btrfs/free-space-cache.c b/fs/btrfs/free-space-cache.c index f3fee88c8ee0..543394acec44 100644 --- a/fs/btrfs/free-space-cache.c +++ b/fs/btrfs/free-space-cache.c @@ -1870,9 +1870,33 @@ static int search_bitmap(struct btrfs_free_space_ctl *ctl, return -1; } +/* + * This is a little subtle. We *only* have ->max_extent_size set if we actually + * searched through the bitmap and figured out the largest ->max_extent_size, + * otherwise it's 0. In the case that it's 0 we don't want to tell the + * allocator the wrong thing, we want to use the actual real max_extent_size + * we've found already if it's larger, or we want to use ->bytes. + * + * This matters because find_free_space() will skip entries who's ->bytes is + * less than the required bytes. So if we didn't search down this bitmap, we + * may pick some previous entry that has a smaller ->max_extent_size than we + * have. For example, assume we have two entries, one that has + * ->max_extent_size set to 4k and ->bytes set to 1M. A second entry hasn't set + * ->max_extent_size yet, has ->bytes set to 8k and it's contiguous. We will + * call into find_free_space(), and return with max_extent_size == 4k, because + * that first bitmap entry had ->max_extent_size set, but the second one did + * not. If instead we returned 8k we'd come in searching for 8k, and find the + * 8k contiguous range. + * + * Consider the other case, we have 2 8k chunks in that second entry and still + * don't have ->max_extent_size set. We'll return 16k, and the next time the + * allocator comes in it'll fully search our second bitmap, and this time it'll + * get an uptodate value of 8k as the maximum chunk size. Then we'll get the + * right allocation the next loop through. + */ static inline u64 get_max_extent_size(struct btrfs_free_space *entry) { - if (entry->bitmap) + if (entry->bitmap && entry->max_extent_size) return entry->max_extent_size; return entry->bytes; } |