| Message ID | 1226461390-5502-11-git-send-email-vaurora@redhat.com |
|---|---|
| State | Superseded, archived |
| Headers | show |
On Nov 11, 2008 19:43 -0800, Valerie Aurora Henson wrote: > diff --git a/lib/ext2fs/alloc_tables.c b/lib/ext2fs/alloc_tables.c > index 7235f7d..71ad445 100644 > --- a/lib/ext2fs/alloc_tables.c > +++ b/lib/ext2fs/alloc_tables.c > @@ -147,7 +147,7 @@ errcode_t ext2fs_allocate_group_table(ext2_filsys fs, dgrp_t group, > - int prev_block = 0; > + blk64_t prev_block = 0; > @@ -178,7 +178,7 @@ errcode_t ext2fs_allocate_group_table(ext2_filsys fs, dgrp_t group, > if (flexbg_size) { > - int prev_block = 0; > + blk64_t prev_block = 0; These appear to be defects in the base code and should be landed ASAP (as int -> blk_t) independently of this patch series. Cheers, Andreas -- Andreas Dilger Sr. Staff Engineer, Lustre Group Sun Microsystems of Canada, Inc. -- 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 Thu, Nov 13, 2008 at 12:57:42PM -0700, Andreas Dilger wrote: > On Nov 11, 2008 19:43 -0800, Valerie Aurora Henson wrote: > > diff --git a/lib/ext2fs/alloc_tables.c b/lib/ext2fs/alloc_tables.c > > index 7235f7d..71ad445 100644 > > --- a/lib/ext2fs/alloc_tables.c > > +++ b/lib/ext2fs/alloc_tables.c > > @@ -147,7 +147,7 @@ errcode_t ext2fs_allocate_group_table(ext2_filsys fs, dgrp_t group, > > - int prev_block = 0; > > + blk64_t prev_block = 0; > > > @@ -178,7 +178,7 @@ errcode_t ext2fs_allocate_group_table(ext2_filsys fs, dgrp_t group, > > if (flexbg_size) { > > - int prev_block = 0; > > + blk64_t prev_block = 0; > > These appear to be defects in the base code and should be landed ASAP > (as int -> blk_t) independently of this patch series. Agreed. Ted, is this a good format for you or do you want me to regenerate against something? -VAL -- 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
Valerie Aurora Henson wrote: > On Thu, Nov 13, 2008 at 12:57:42PM -0700, Andreas Dilger wrote: >> On Nov 11, 2008 19:43 -0800, Valerie Aurora Henson wrote: >>> diff --git a/lib/ext2fs/alloc_tables.c b/lib/ext2fs/alloc_tables.c >>> index 7235f7d..71ad445 100644 >>> --- a/lib/ext2fs/alloc_tables.c >>> +++ b/lib/ext2fs/alloc_tables.c >>> @@ -147,7 +147,7 @@ errcode_t ext2fs_allocate_group_table(ext2_filsys fs, dgrp_t group, >>> - int prev_block = 0; >>> + blk64_t prev_block = 0; >>> @@ -178,7 +178,7 @@ errcode_t ext2fs_allocate_group_table(ext2_filsys fs, dgrp_t group, >>> if (flexbg_size) { >>> - int prev_block = 0; >>> + blk64_t prev_block = 0; >> These appear to be defects in the base code and should be landed ASAP >> (as int -> blk_t) independently of this patch series. > > Agreed. Ted, is this a good format for you or do you want me to > regenerate against something? Is it? if (flexbg_size) { int prev_block = 0; if (group && fs->group_desc[group-1].bg_block_bitmap) prev_block = fs->group_desc[group-1].bg_block_bitmap; start_blk = flexbg_offset(fs, group, prev_block, bmap, 0, rem_grps, 1); last_blk = ext2fs_group_last_block(fs, last_grp); } bg_block_bitmap is only a __u32, and that's what we assign to prev_block. Just a quick scan, but isn't this just a relative block in the group? -Eric > -VAL > -- > 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 -- 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 Thu, Nov 13, 2008 at 09:42:52PM -0600, Eric Sandeen wrote: > Valerie Aurora Henson wrote: > > On Thu, Nov 13, 2008 at 12:57:42PM -0700, Andreas Dilger wrote: > >> On Nov 11, 2008 19:43 -0800, Valerie Aurora Henson wrote: > >>> diff --git a/lib/ext2fs/alloc_tables.c b/lib/ext2fs/alloc_tables.c > >>> index 7235f7d..71ad445 100644 > >>> --- a/lib/ext2fs/alloc_tables.c > >>> +++ b/lib/ext2fs/alloc_tables.c > >>> @@ -147,7 +147,7 @@ errcode_t ext2fs_allocate_group_table(ext2_filsys fs, dgrp_t group, > >>> - int prev_block = 0; > >>> + blk64_t prev_block = 0; > >>> @@ -178,7 +178,7 @@ errcode_t ext2fs_allocate_group_table(ext2_filsys fs, dgrp_t group, > >>> if (flexbg_size) { > >>> - int prev_block = 0; > >>> + blk64_t prev_block = 0; > >> These appear to be defects in the base code and should be landed ASAP > >> (as int -> blk_t) independently of this patch series. > > > > Agreed. Ted, is this a good format for you or do you want me to > > regenerate against something? > > Is it? > > if (flexbg_size) { > int prev_block = 0; > if (group && fs->group_desc[group-1].bg_block_bitmap) > prev_block = > fs->group_desc[group-1].bg_block_bitmap; > start_blk = flexbg_offset(fs, group, prev_block, bmap, > 0, rem_grps, 1); > last_blk = ext2fs_group_last_block(fs, last_grp); > } > > bg_block_bitmap is only a __u32, and that's what we assign to prev_block. It's a signed/unsigned bug (int vs. blk_t) - it shows up at > 2^31 blocks. > Just a quick scan, but isn't this just a relative block in the group? I double-checked and it's relative to the beginning of the file system. -VAL -- 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
Valerie Aurora Henson wrote: > On Thu, Nov 13, 2008 at 09:42:52PM -0600, Eric Sandeen wrote: >> Valerie Aurora Henson wrote: >>> On Thu, Nov 13, 2008 at 12:57:42PM -0700, Andreas Dilger wrote: >>>> On Nov 11, 2008 19:43 -0800, Valerie Aurora Henson wrote: >>>>> diff --git a/lib/ext2fs/alloc_tables.c b/lib/ext2fs/alloc_tables.c >>>>> index 7235f7d..71ad445 100644 >>>>> --- a/lib/ext2fs/alloc_tables.c >>>>> +++ b/lib/ext2fs/alloc_tables.c >>>>> @@ -147,7 +147,7 @@ errcode_t ext2fs_allocate_group_table(ext2_filsys fs, dgrp_t group, >>>>> - int prev_block = 0; >>>>> + blk64_t prev_block = 0; >>>>> @@ -178,7 +178,7 @@ errcode_t ext2fs_allocate_group_table(ext2_filsys fs, dgrp_t group, >>>>> if (flexbg_size) { >>>>> - int prev_block = 0; >>>>> + blk64_t prev_block = 0; >>>> These appear to be defects in the base code and should be landed ASAP >>>> (as int -> blk_t) independently of this patch series. >>> Agreed. Ted, is this a good format for you or do you want me to >>> regenerate against something? >> Is it? >> >> if (flexbg_size) { >> int prev_block = 0; >> if (group && fs->group_desc[group-1].bg_block_bitmap) >> prev_block = >> fs->group_desc[group-1].bg_block_bitmap; >> start_blk = flexbg_offset(fs, group, prev_block, bmap, >> 0, rem_grps, 1); >> last_blk = ext2fs_group_last_block(fs, last_grp); >> } >> >> bg_block_bitmap is only a __u32, and that's what we assign to prev_block. > > It's a signed/unsigned bug (int vs. blk_t) - it shows up at > 2^31 > blocks. oh, sigh, I was thinking of blk_t as a 64 bit here already :) -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
On Thu, Nov 13, 2008 at 09:38:12PM -0500, Valerie Aurora Henson wrote: > > Agreed. Ted, is this a good format for you or do you want me to > regenerate against something? > No, this is fine. I'll take the patch and merge it into maint. Note, what I'm going to try to do is to take entire patches of yours at once, although in some cases they will be applied to different branches (or split into different patches). The goal will be to not disturb things too badly if you do any further work on the patch series, since git-rebase should be smart enough. However, it might be a good idea for you to set a tag on what you've pushed so far, and to save everything past that point as diffs in case git rebase doesn't work out for whatever reason. Hopefully while I'm in Japan and/or over Thanksgiving weekend I'll have enough time to merge all of these patches into mainline. (I'll probably miss next week's conference call as well since I'll be in Tokyo, BTW. If someone can take notes during the call, or at least for anything that needs my attention, I'd appreciate it.) - 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 Nov 14, 2008 09:24 -0500, Theodore Ts'o wrote: > I'll probably miss next week's conference call as well since I'll be in > Tokyo, BTW. Speaking of which, I'll also miss next week's call due to travelling. Cheers, Andreas -- Andreas Dilger Sr. Staff Engineer, Lustre Group Sun Microsystems of Canada, Inc. -- 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/alloc_tables.c b/lib/ext2fs/alloc_tables.c index 7235f7d..71ad445 100644 --- a/lib/ext2fs/alloc_tables.c +++ b/lib/ext2fs/alloc_tables.c @@ -147,7 +147,7 @@ errcode_t ext2fs_allocate_group_table(ext2_filsys fs, dgrp_t group, } if (flexbg_size) { - int prev_block = 0; + blk64_t prev_block = 0; if (group && ext2fs_inode_bitmap_loc(fs, group - 1)) prev_block = ext2fs_inode_bitmap_loc(fs, group - 1); start_blk = flexbg_offset(fs, group, prev_block, bmap, @@ -178,7 +178,7 @@ errcode_t ext2fs_allocate_group_table(ext2_filsys fs, dgrp_t group, * Allocate the inode table */ if (flexbg_size) { - int prev_block = 0; + blk64_t prev_block = 0; if (group && ext2fs_inode_table_loc(fs, group - 1)) prev_block = ext2fs_inode_table_loc(fs, group - 1); group_blk = flexbg_offset(fs, group, prev_block, bmap,
Signed-off-by: Valerie Aurora Henson <vaurora@redhat.com> --- lib/ext2fs/alloc_tables.c | 4 ++-- 1 files changed, 2 insertions(+), 2 deletions(-)