| Message ID | 20200908075230.86856-6-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: > As a preparation for subpage sector size support (allow sector size > smaller than page size to be mounted), if the sector size is smaller nit: (allowing filesystem with sector size smaller than page size to be mounted) > than page size, we don't allow tree block to be read if it cross page > boundary (normally 64K). Why normally 64k ? I suspect you assume readers are familiar with the motivation for this work which they don't necessarily need to be. Please make your sentences explicit. Correct me if I'm wrong but you are ensuring that an eb doesn't cross the largest possible sectorsize? > > This ensures that, tree blocks are always contained in one page for 64K > system, which can greatly simplify the handling. "For system with 64k page size" the term "64k system" is ambiguous, heed this when rewording other patches as well since I've seen this used in multiple places. > Or we need to do complex multi-page handling for tree blocks. > > Currently the only way to create such tree blocks crossing 64K boundary > is by btrfs-convert, which will get fixed soon and doesn't get > wide-spread usage. > > Signed-off-by: Qu Wenruo <wqu@suse.com> > --- > fs/btrfs/extent_io.c | 7 +++++++ > 1 file changed, 7 insertions(+) > > diff --git a/fs/btrfs/extent_io.c b/fs/btrfs/extent_io.c > index 5d969340275e..119193166cec 100644 > --- a/fs/btrfs/extent_io.c > +++ b/fs/btrfs/extent_io.c > @@ -5232,6 +5232,13 @@ struct extent_buffer *alloc_extent_buffer(struct btrfs_fs_info *fs_info, > btrfs_err(fs_info, "bad tree block start %llu", start); > return ERR_PTR(-EINVAL); > } > + if (fs_info->sectorsize < PAGE_SIZE && round_down(start, PAGE_SIZE) != > + round_down(start + len - 1, PAGE_SIZE)) { > + btrfs_err(fs_info, > + "tree block crosses page boundary, start %llu nodesize %lu", > + start, len); > + return ERR_PTR(-EINVAL); > + } > > eb = find_extent_buffer(fs_info, start); > if (eb) >
On 2020/9/11 下午6:26, Nikolay Borisov wrote: > > > On 8.09.20 г. 10:52 ч., Qu Wenruo wrote: >> As a preparation for subpage sector size support (allow sector size >> smaller than page size to be mounted), if the sector size is smaller > > nit: (allowing filesystem with sector size smaller than page size to be > mounted) > >> than page size, we don't allow tree block to be read if it cross page >> boundary (normally 64K). > > Why normally 64k ? Because there are only two major arches supporting non-4k page size, ppc64 which uses 64K page size, and arm which can support 4K, 16K and 64K page size. Currently our plan is only to support 64K page size and 4K page size. Furthermore, that 64K magic number matches with 64K stripe length of btrfs, thus we choose 64K as the boundary. > I suspect you assume readers are familiar with the > motivation for this work which they don't necessarily need to be. Please > make your sentences explicit. Correct me if I'm wrong but you are > ensuring that an eb doesn't cross the largest possible sectorsize? That's correct, and by incident, it's also the stripe length of btrfs RAID, and scrub stripe length too. (And scrub can't handle such tree block either) Thanks, Qu >> >> This ensures that, tree blocks are always contained in one page for 64K >> system, which can greatly simplify the handling. > > "For system with 64k page size" the term "64k system" is ambiguous, heed > this when rewording other patches as well since I've seen this used in > multiple places. > > >> Or we need to do complex multi-page handling for tree blocks. >> >> Currently the only way to create such tree blocks crossing 64K boundary >> is by btrfs-convert, which will get fixed soon and doesn't get >> wide-spread usage. >> >> Signed-off-by: Qu Wenruo <wqu@suse.com> >> --- >> fs/btrfs/extent_io.c | 7 +++++++ >> 1 file changed, 7 insertions(+) >> >> diff --git a/fs/btrfs/extent_io.c b/fs/btrfs/extent_io.c >> index 5d969340275e..119193166cec 100644 >> --- a/fs/btrfs/extent_io.c >> +++ b/fs/btrfs/extent_io.c >> @@ -5232,6 +5232,13 @@ struct extent_buffer *alloc_extent_buffer(struct btrfs_fs_info *fs_info, >> btrfs_err(fs_info, "bad tree block start %llu", start); >> return ERR_PTR(-EINVAL); >> } >> + if (fs_info->sectorsize < PAGE_SIZE && round_down(start, PAGE_SIZE) != >> + round_down(start + len - 1, PAGE_SIZE)) { >> + btrfs_err(fs_info, >> + "tree block crosses page boundary, start %llu nodesize %lu", >> + start, len); >> + return ERR_PTR(-EINVAL); >> + } >> >> eb = find_extent_buffer(fs_info, start); >> if (eb) >> >
On 11.09.20 г. 14:36 ч., Qu Wenruo wrote: > > > On 2020/9/11 下午6:26, Nikolay Borisov wrote: >> >> >> On 8.09.20 г. 10:52 ч., Qu Wenruo wrote: >>> As a preparation for subpage sector size support (allow sector size >>> smaller than page size to be mounted), if the sector size is smaller >> >> nit: (allowing filesystem with sector size smaller than page size to be >> mounted) >> >>> than page size, we don't allow tree block to be read if it cross page >>> boundary (normally 64K). >> >> Why normally 64k ? > > Because there are only two major arches supporting non-4k page size, > ppc64 which uses 64K page size, and arm which can support 4K, 16K and > 64K page size. > > Currently our plan is only to support 64K page size and 4K page size. I really rather make the support generic rather than boxing ourselves in some "artificial" constraints. > > Furthermore, that 64K magic number matches with 64K stripe length of > btrfs, thus we choose 64K as the boundary. > >> I suspect you assume readers are familiar with the >> motivation for this work which they don't necessarily need to be. Please >> make your sentences explicit. Correct me if I'm wrong but you are >> ensuring that an eb doesn't cross the largest possible sectorsize? > > That's correct, and by incident, it's also the stripe length of btrfs > RAID, and scrub stripe length too. > > (And scrub can't handle such tree block either) And that by itself is a good reason why we may want to support just 64k. So state that explicitly. > > Thanks, > Qu > <snip>
diff --git a/fs/btrfs/extent_io.c b/fs/btrfs/extent_io.c index 5d969340275e..119193166cec 100644 --- a/fs/btrfs/extent_io.c +++ b/fs/btrfs/extent_io.c @@ -5232,6 +5232,13 @@ struct extent_buffer *alloc_extent_buffer(struct btrfs_fs_info *fs_info, btrfs_err(fs_info, "bad tree block start %llu", start); return ERR_PTR(-EINVAL); } + if (fs_info->sectorsize < PAGE_SIZE && round_down(start, PAGE_SIZE) != + round_down(start + len - 1, PAGE_SIZE)) { + btrfs_err(fs_info, + "tree block crosses page boundary, start %llu nodesize %lu", + start, len); + return ERR_PTR(-EINVAL); + } eb = find_extent_buffer(fs_info, start); if (eb)
As a preparation for subpage sector size support (allow sector size smaller than page size to be mounted), if the sector size is smaller than page size, we don't allow tree block to be read if it cross page boundary (normally 64K). This ensures that, tree blocks are always contained in one page for 64K system, which can greatly simplify the handling. Or we need to do complex multi-page handling for tree blocks. Currently the only way to create such tree blocks crossing 64K boundary is by btrfs-convert, which will get fixed soon and doesn't get wide-spread usage. Signed-off-by: Qu Wenruo <wqu@suse.com> --- fs/btrfs/extent_io.c | 7 +++++++ 1 file changed, 7 insertions(+)