| Message ID | 1317702420-31085-2-git-send-email-tytso@mit.edu |
|---|---|
| State | Accepted, archived |
| Headers | show |
On 10/3/11 11:27 PM, Theodore Ts'o wrote: > If the number of block groups exceeds 2**32, a bad cast would lead to > a bogus "Not enough space to build proposed filesystem while setting > up superblock" failure. It's the proper cast now, but I don't think it fixes the problem, since they are both __u32... But in any case, for the actual change at least: Reviewed-by: Eric Sandeen <sandeen@redhat.com> > Signed-off-by: "Theodore Ts'o" <tytso@mit.edu> > --- > lib/ext2fs/initialize.c | 2 +- > 1 files changed, 1 insertions(+), 1 deletions(-) > > diff --git a/lib/ext2fs/initialize.c b/lib/ext2fs/initialize.c > index 2875f97..b050a0a 100644 > --- a/lib/ext2fs/initialize.c > +++ b/lib/ext2fs/initialize.c > @@ -248,7 +248,7 @@ errcode_t ext2fs_initialize(const char *name, int flags, > } > > retry: > - fs->group_desc_count = (blk_t) ext2fs_div64_ceil( > + fs->group_desc_count = (dgrp_t) ext2fs_div64_ceil( > ext2fs_blocks_count(super) - super->s_first_data_block, > EXT2_BLOCKS_PER_GROUP(super)); > if (fs->group_desc_count == 0) { -- To unsubscribe from this list: send the line "unsubscribe linux-ext4" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Tue, Oct 04, 2011 at 06:47:12AM -0500, Eric Sandeen wrote: > On 10/3/11 11:27 PM, Theodore Ts'o wrote: > > If the number of block groups exceeds 2**32, a bad cast would lead to > > a bogus "Not enough space to build proposed filesystem while setting > > up superblock" failure. > > It's the proper cast now, but I don't think it fixes the problem, since they > are both __u32... Hmm, yes. And to be quite honest I'm not sure it's worth fixing. 2**32 block groups gets us up to 2**59 bytes assuming 4k blocks. The theoretical maximum given the current extent tree format is 2**60 assuming 4k blocks. So changing dgrp_t to be 64-bits just to get that last power of two (i.e., from 512EB to a full PB) doesn't seem worth it. Simply using a bigalloc cluster size of 8k would make the problem go away (and arguably we'd probably want a large cluster size if someone wanted to create a file system that big anyway). So maybe we should just check to see if the required number of block groups is greater than 2**32, and if so, give an error. - Ted -- To unsubscribe from this list: send the line "unsubscribe linux-ext4" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 10/4/11 1:05 PM, Ted Ts'o wrote: > On Tue, Oct 04, 2011 at 06:47:12AM -0500, Eric Sandeen wrote: >> On 10/3/11 11:27 PM, Theodore Ts'o wrote: >>> If the number of block groups exceeds 2**32, a bad cast would lead to >>> a bogus "Not enough space to build proposed filesystem while setting >>> up superblock" failure. >> >> It's the proper cast now, but I don't think it fixes the problem, since they >> are both __u32... > > Hmm, yes. > > And to be quite honest I'm not sure it's worth fixing. 2**32 block > groups gets us up to 2**59 bytes assuming 4k blocks. The theoretical > maximum given the current extent tree format is 2**60 assuming 4k > blocks. So changing dgrp_t to be 64-bits just to get that last power > of two (i.e., from 512EB to a full PB) doesn't seem worth it. Simply > using a bigalloc cluster size of 8k would make the problem go away > (and arguably we'd probably want a large cluster size if someone > wanted to create a file system that big anyway). > > So maybe we should just check to see if the required number of block > groups is greater than 2**32, and if so, give an error. > > - Ted > As long as we have a consistent, predictable, well-designed and well-understood maximum (theoretical) size for the fs, I'm all for documenting & enforcing it. TBH I'm still trying to get all the moving parts together in my head, between meta_bg & bigalloc & whatnot, at these sizes. The initialization functions are looking pretty ad-hoc to me right now. :) -Eric -- To unsubscribe from this list: send the line "unsubscribe linux-ext4" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/lib/ext2fs/initialize.c b/lib/ext2fs/initialize.c index 2875f97..b050a0a 100644 --- a/lib/ext2fs/initialize.c +++ b/lib/ext2fs/initialize.c @@ -248,7 +248,7 @@ errcode_t ext2fs_initialize(const char *name, int flags, } retry: - fs->group_desc_count = (blk_t) ext2fs_div64_ceil( + fs->group_desc_count = (dgrp_t) ext2fs_div64_ceil( ext2fs_blocks_count(super) - super->s_first_data_block, EXT2_BLOCKS_PER_GROUP(super)); if (fs->group_desc_count == 0) {
If the number of block groups exceeds 2**32, a bad cast would lead to a bogus "Not enough space to build proposed filesystem while setting up superblock" failure. Signed-off-by: "Theodore Ts'o" <tytso@mit.edu> --- lib/ext2fs/initialize.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-)