| Message ID | 20200908075230.86856-5-wqu@suse.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | btrfs: add read-only support for subpage sector size | expand |
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); >
On 2020/9/11 下午6:11, Nikolay Borisov wrote: > > > 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 Exactly what david is doing. IIRC he mentioned such shift bits simplification in IRC, and I'm just waiting for that bit to land and use that to further simplify the code. Thanks, Qu > > 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); >> >
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);
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> --- fs/btrfs/extent_io.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-)