Re: [PATCH 04/17] btrfs: make btrfs_fs_info::buffer_radix to take sector size devided values

Nikolay Borisov
2020-09-11T10:12:01Z

On 8.09.20 г. 10:52 ч., Qu Wenruo wrote:
> For subpage size sector size support, one page can contain mutliple tree
> blocks, thus we can no longer use (eb->start >> PAGE_SHIFT) any more, or
> we can easily get extent buffer doesn't belongs to us.
>
> This patch will use (extent_buffer::start / sectorsize) as index for radix
> tree so that we can get correct extent buffer for subpage size support.
>
> Signed-off-by: Qu Wenruo <wqu@suse.com>
That's fine, however now that we are moving towards sectorsize I wonder
if it would make more sense to fs_info->sector_bits which would be

log2(fs_info->sectorsize) and have expressions such as:

start >> fs_info->sector_bits. I * think* we can rely on the compiler
doing the right thing given fs_info->sectorsize is guaranteed to be a
power of 2 value.

Reviewed-by: Nikolay Borisov <nborisov@suse.com>

> ---
> fs/btrfs/extent_io.c | 8 ++++----
> 1 file changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/fs/btrfs/extent_io.c b/fs/btrfs/extent_io.c
> index 6def411b2eba..5d969340275e 100644
> --- a/fs/btrfs/extent_io.c
> +++ b/fs/btrfs/extent_io.c
> @@ -5142,7 +5142,7 @@ struct extent_buffer *find_extent_buffer(struct btrfs_fs_info *fs_info,
>
> rcu_read_lock();
> eb = radix_tree_lookup(&fs_info->buffer_radix,
> - start >> PAGE_SHIFT);
> + start / fs_info->sectorsize);
> if (eb && atomic_inc_not_zero(&eb->refs)) {
> rcu_read_unlock();
> /*
> @@ -5194,7 +5194,7 @@ struct extent_buffer *alloc_test_extent_buffer(struct btrfs_fs_info *fs_info,
> }
> spin_lock(&fs_info->buffer_lock);
> ret = radix_tree_insert(&fs_info->buffer_radix,
> - start >> PAGE_SHIFT, eb);
> + start / fs_info->sectorsize, eb);
> spin_unlock(&fs_info->buffer_lock);
> radix_tree_preload_end();
> if (ret == -EEXIST) {
> @@ -5302,7 +5302,7 @@ struct extent_buffer *alloc_extent_buffer(struct btrfs_fs_info *fs_info,
>
> spin_lock(&fs_info->buffer_lock);
> ret = radix_tree_insert(&fs_info->buffer_radix,
> - start >> PAGE_SHIFT, eb);
> + start / fs_info->sectorsize, eb);
> spin_unlock(&fs_info->buffer_lock);
> radix_tree_preload_end();
> if (ret == -EEXIST) {
> @@ -5358,7 +5358,7 @@ static int release_extent_buffer(struct extent_buffer *eb)
>
> spin_lock(&fs_info->buffer_lock);
> radix_tree_delete(&fs_info->buffer_radix,
> - eb->start >> PAGE_SHIFT);
> + eb->start / fs_info->sectorsize);
> spin_unlock(&fs_info->buffer_lock);
> } else {
> spin_unlock(&eb->refs_lock);
>