* [PATCH] e2fsck: fix error in computing blocks of the ending group @ 2011-07-19 15:46 Yongqiang Yang 2011-07-19 16:18 ` Eric Sandeen 2011-07-22 4:18 ` [PATCH V2] " Eric Sandeen 0 siblings, 2 replies; 13+ messages in thread From: Yongqiang Yang @ 2011-07-19 15:46 UTC (permalink / raw) To: linux-ext4; +Cc: tytso, Yongqiang Yang If the blocks of a filesystem is a multiple of blocks_per_group, blocks of the ending group is computed wrongly. This patch computes it by substracting blocks processed from blocks of the filesystem. Signed-off-by: Yongqiang Yang <xiaoqiangnk@gmail.com> --- e2fsck/pass5.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/e2fsck/pass5.c b/e2fsck/pass5.c index f9d746c..7c4e336 100644 --- a/e2fsck/pass5.c +++ b/e2fsck/pass5.c @@ -227,7 +227,7 @@ redo_counts: if (group == (int)fs->group_desc_count - 1) cmp_block = EXT2FS_NUM_B2C(fs, - ext2fs_blocks_count(fs->super) % fs->super->s_blocks_per_group); + ext2fs_blocks_count(fs->super) - i); } bitmap = 0; -- 1.7.5.1 ^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [PATCH] e2fsck: fix error in computing blocks of the ending group 2011-07-19 15:46 [PATCH] e2fsck: fix error in computing blocks of the ending group Yongqiang Yang @ 2011-07-19 16:18 ` Eric Sandeen 2011-07-20 21:46 ` [PATCH] e2fsprogs: add ext2fs_group_blocks_count helper Eric Sandeen 2011-07-21 17:40 ` [PATCH] e2fsck: fix error in computing blocks of the ending group Eric Sandeen 2011-07-22 4:18 ` [PATCH V2] " Eric Sandeen 1 sibling, 2 replies; 13+ messages in thread From: Eric Sandeen @ 2011-07-19 16:18 UTC (permalink / raw) To: Yongqiang Yang; +Cc: linux-ext4, tytso On 07/19/2011 10:46 AM, Yongqiang Yang wrote: > If the blocks of a filesystem is a multiple of blocks_per_group, > blocks of the ending group is computed wrongly. This patch computes > it by substracting blocks processed from blocks of the filesystem. > > Signed-off-by: Yongqiang Yang <xiaoqiangnk@gmail.com> > --- > e2fsck/pass5.c | 2 +- > 1 files changed, 1 insertions(+), 1 deletions(-) > > diff --git a/e2fsck/pass5.c b/e2fsck/pass5.c > index f9d746c..7c4e336 100644 > --- a/e2fsck/pass5.c > +++ b/e2fsck/pass5.c > @@ -227,7 +227,7 @@ redo_counts: > if (group == (int)fs->group_desc_count - 1) > cmp_block = > EXT2FS_NUM_B2C(fs, > - ext2fs_blocks_count(fs->super) % fs->super->s_blocks_per_group); > + ext2fs_blocks_count(fs->super) - i); > } > > bitmap = 0; I think it would be clearer to just get the last group block count from superblock information, rather than from using the loop counter. For example resize does: if (i == fs->group_desc_count-1) { numblocks = (ext2fs_blocks_count(fs->super) - fs->super->s_first_data_block) % fs->super->s_blocks_per_group; if (!numblocks) numblocks = fs->super->s_blocks_per_group; same as this function in alloc_sb.c: if (group == fs->group_desc_count-1) { num_blocks = (ext2fs_blocks_count(fs->super) - fs->super->s_first_data_block) % fs->super->s_blocks_per_group; if (!num_blocks) num_blocks = fs->super->s_blocks_per_group; Hm, seems maybe an ext2fs_blocks_in_group() helper function might be nice, since this is a little verbose, and is used fairly often. -Eric ^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH] e2fsprogs: add ext2fs_group_blocks_count helper 2011-07-19 16:18 ` Eric Sandeen @ 2011-07-20 21:46 ` Eric Sandeen 2011-07-20 21:57 ` [PATCH V2] " Eric Sandeen 2011-07-21 14:54 ` [PATCH] " Yongqiang Yang 2011-07-21 17:40 ` [PATCH] e2fsck: fix error in computing blocks of the ending group Eric Sandeen 1 sibling, 2 replies; 13+ messages in thread From: Eric Sandeen @ 2011-07-20 21:46 UTC (permalink / raw) To: Yongqiang Yang; +Cc: linux-ext4, tytso Code to count the number of blocks in the last partial group is cut and pasted around the e2fsprogs codebase, and is wrong in at least one instancem as pointed out by Yongqiang Yang (but not fixed in this patch). Making this a helper function should improve matters. Signed-off-by: Eric Sandeen <sandeen@redhat.com> --- (I have not fixed up the spot Yongqiang Yang discovered, but this helper should now make that fix easy to do.) e2fsck/pass5.c | 6 +----- lib/ext2fs/alloc_sb.c | 10 +--------- lib/ext2fs/blknum.c | 19 +++++++++++++++++++ lib/ext2fs/closefs.c | 9 +-------- lib/ext2fs/ext2fs.h | 1 + resize/online.c | 7 +------ resize/resize2fs.c | 14 ++------------ 7 files changed, 26 insertions(+), 40 deletions(-) diff --git a/e2fsck/pass5.c b/e2fsck/pass5.c index b423d28..73db4e8 100644 --- a/e2fsck/pass5.c +++ b/e2fsck/pass5.c @@ -218,11 +218,7 @@ redo_counts: fs->super->s_reserved_gdt_blocks; count = 0; - cmp_block = fs->super->s_blocks_per_group; - if (group == (int)fs->group_desc_count - 1) - cmp_block = - ext2fs_blocks_count(fs->super) % - fs->super->s_blocks_per_group; + cmp_block = ext2fs_group_blocks_count(fs, group); } bitmap = 0; diff --git a/lib/ext2fs/alloc_sb.c b/lib/ext2fs/alloc_sb.c index d5fca3b..98aec52 100644 --- a/lib/ext2fs/alloc_sb.c +++ b/lib/ext2fs/alloc_sb.c @@ -71,15 +71,7 @@ int ext2fs_reserve_super_and_bgd(ext2_filsys fs, if (new_desc_blk) ext2fs_mark_block_bitmap2(bmap, new_desc_blk); - if (group == fs->group_desc_count-1) { - num_blocks = (ext2fs_blocks_count(fs->super) - - fs->super->s_first_data_block) % - fs->super->s_blocks_per_group; - if (!num_blocks) - num_blocks = fs->super->s_blocks_per_group; - } else - num_blocks = fs->super->s_blocks_per_group; - + num_blocks = ext2fs_group_blocks_count(fs, group); num_blocks -= 2 + fs->inode_blocks_per_group + used_blks; return num_blocks ; diff --git a/lib/ext2fs/blknum.c b/lib/ext2fs/blknum.c index b3e6dca..f1bbe9b 100644 --- a/lib/ext2fs/blknum.c +++ b/lib/ext2fs/blknum.c @@ -43,6 +43,25 @@ blk64_t ext2fs_group_last_block2(ext2_filsys fs, dgrp_t group) } /* + * Return the number of blocks in a group + */ +int ext2fs_group_blocks_count(ext2_filsys fs, dgrp_t group) +{ + int num_blocks; + + if (group == fs->group_desc_count-1) { + num_blocks = (ext2fs_blocks_count(fs->super) - + fs->super->s_first_data_block) % + fs->super->s_blocks_per_group; + if (!num_blocks) + num_blocks = fs->super->s_blocks_per_group; + } else + num_blocks = fs->super->s_blocks_per_group; + + return num_blocks; +} + +/* * Return the inode data block count */ blk64_t ext2fs_inode_data_blocks2(ext2_filsys fs, diff --git a/lib/ext2fs/closefs.c b/lib/ext2fs/closefs.c index 7a23e46..b13a0c9 100644 --- a/lib/ext2fs/closefs.c +++ b/lib/ext2fs/closefs.c @@ -150,14 +150,7 @@ int ext2fs_super_and_bgd_loc(ext2_filsys fs, &ret_new_desc_blk2, &ret_used_blks); - if (group == fs->group_desc_count-1) { - numblocks = (ext2fs_blocks_count(fs->super) - - (blk64_t) fs->super->s_first_data_block) % - (blk64_t) fs->super->s_blocks_per_group; - if (!numblocks) - numblocks = fs->super->s_blocks_per_group; - } else - numblocks = fs->super->s_blocks_per_group; + numblocks = ext2fs_group_blocks_count(fs, group); if (ret_super_blk) *ret_super_blk = (blk_t)ret_super_blk2; diff --git a/lib/ext2fs/ext2fs.h b/lib/ext2fs/ext2fs.h index d3eb31d..5983adc 100644 --- a/lib/ext2fs/ext2fs.h +++ b/lib/ext2fs/ext2fs.h @@ -731,6 +731,7 @@ extern errcode_t ext2fs_get_block_bitmap_range2(ext2fs_block_bitmap bmap, extern dgrp_t ext2fs_group_of_blk2(ext2_filsys fs, blk64_t); extern blk64_t ext2fs_group_first_block2(ext2_filsys fs, dgrp_t group); extern blk64_t ext2fs_group_last_block2(ext2_filsys fs, dgrp_t group); +extern int ext2fs_group_blocks_count(ext2_filsys fs, dgrp_t group); extern blk64_t ext2fs_inode_data_blocks2(ext2_filsys fs, struct ext2_inode *inode); extern blk64_t ext2fs_inode_i_blocks(ext2_filsys fs, diff --git a/resize/online.c b/resize/online.c index 1d8d4ec..c2b98c6 100644 --- a/resize/online.c +++ b/resize/online.c @@ -135,12 +135,7 @@ errcode_t online_resize_fs(ext2_filsys fs, const char *mtpt, input.block_bitmap = ext2fs_block_bitmap_loc(new_fs, i); input.inode_bitmap = ext2fs_inode_bitmap_loc(new_fs, i); input.inode_table = ext2fs_inode_table_loc(new_fs, i); - input.blocks_count = sb->s_blocks_per_group; - if (i == new_fs->group_desc_count-1) { - input.blocks_count = ext2fs_blocks_count(new_fs->super) - - sb->s_first_data_block - - (i * sb->s_blocks_per_group); - } + input.blocks_count = ext2fs_group_blocks_count(new_fs, i); input.reserved_blocks = (blk_t) (percent * input.blocks_count / 100.0); diff --git a/resize/resize2fs.c b/resize/resize2fs.c index 216a626..4b83ca9 100644 --- a/resize/resize2fs.c +++ b/resize/resize2fs.c @@ -499,18 +499,8 @@ retry: ext2fs_bg_flags_zap(fs, i); if (csum_flag) ext2fs_bg_flags_set(fs, i, EXT2_BG_INODE_UNINIT | EXT2_BG_INODE_ZEROED); - if (i == fs->group_desc_count-1) { - numblocks = (ext2fs_blocks_count(fs->super) - - fs->super->s_first_data_block) % - fs->super->s_blocks_per_group; - if (!numblocks) - numblocks = fs->super->s_blocks_per_group; - } else { - numblocks = fs->super->s_blocks_per_group; - if (csum_flag) - ext2fs_bg_flags_set(fs, i, - EXT2_BG_BLOCK_UNINIT); - } + + numblocks = ext2fs_group_blocks_count(fs, i); has_super = ext2fs_bg_has_super(fs, i); if (has_super) { ^ permalink raw reply related [flat|nested] 13+ messages in thread
* [PATCH V2] e2fsprogs: add ext2fs_group_blocks_count helper 2011-07-20 21:46 ` [PATCH] e2fsprogs: add ext2fs_group_blocks_count helper Eric Sandeen @ 2011-07-20 21:57 ` Eric Sandeen 2011-07-21 13:55 ` Eric Sandeen 2011-07-21 18:15 ` [PATCH V3] " Eric Sandeen 2011-07-21 14:54 ` [PATCH] " Yongqiang Yang 1 sibling, 2 replies; 13+ messages in thread From: Eric Sandeen @ 2011-07-20 21:57 UTC (permalink / raw) To: Yongqiang Yang; +Cc: linux-ext4, tytso Code to count the number of blocks in the last partial group is cut and pasted around the e2fsprogs codebase, and is wrong in at least one instancem as pointed out by Yongqiang Yang (but not fixed in this patch). Making this a helper function should improve matters. Signed-off-by: Eric Sandeen <sandeen@redhat.com> --- (I have not fixed up the spot Yongqiang Yang discovered, but this helper should now make that fix easy to do.) V2: Sorry, first patch was against an old tree. lib/ext2fs/alloc_sb.c | 10 +--------- lib/ext2fs/blknum.c | 19 +++++++++++++++++++ lib/ext2fs/closefs.c | 9 +-------- lib/ext2fs/ext2fs.h | 1 + resize/online.c | 7 +------ resize/resize2fs.c | 14 ++------------ 6 files changed, 25 insertions(+), 35 deletions(-) diff --git a/lib/ext2fs/alloc_sb.c b/lib/ext2fs/alloc_sb.c index 09786a7..2e32389 100644 --- a/lib/ext2fs/alloc_sb.c +++ b/lib/ext2fs/alloc_sb.c @@ -74,15 +74,7 @@ int ext2fs_reserve_super_and_bgd(ext2_filsys fs, if (new_desc_blk) ext2fs_mark_block_bitmap2(bmap, new_desc_blk); - if (group == fs->group_desc_count-1) { - num_blocks = (ext2fs_blocks_count(fs->super) - - fs->super->s_first_data_block) % - fs->super->s_blocks_per_group; - if (!num_blocks) - num_blocks = fs->super->s_blocks_per_group; - } else - num_blocks = fs->super->s_blocks_per_group; - + num_blocks = ext2fs_group_blocks_count(fs, group); num_blocks -= 2 + fs->inode_blocks_per_group + used_blks; return num_blocks ; diff --git a/lib/ext2fs/blknum.c b/lib/ext2fs/blknum.c index b3e6dca..f1bbe9b 100644 --- a/lib/ext2fs/blknum.c +++ b/lib/ext2fs/blknum.c @@ -43,6 +43,25 @@ blk64_t ext2fs_group_last_block2(ext2_filsys fs, dgrp_t group) } /* + * Return the number of blocks in a group + */ +int ext2fs_group_blocks_count(ext2_filsys fs, dgrp_t group) +{ + int num_blocks; + + if (group == fs->group_desc_count-1) { + num_blocks = (ext2fs_blocks_count(fs->super) - + fs->super->s_first_data_block) % + fs->super->s_blocks_per_group; + if (!num_blocks) + num_blocks = fs->super->s_blocks_per_group; + } else + num_blocks = fs->super->s_blocks_per_group; + + return num_blocks; +} + +/* * Return the inode data block count */ blk64_t ext2fs_inode_data_blocks2(ext2_filsys fs, diff --git a/lib/ext2fs/closefs.c b/lib/ext2fs/closefs.c index 952f496..51ef63c 100644 --- a/lib/ext2fs/closefs.c +++ b/lib/ext2fs/closefs.c @@ -152,14 +152,7 @@ int ext2fs_super_and_bgd_loc(ext2_filsys fs, &ret_new_desc_blk2, &ret_used_blks); - if (group == fs->group_desc_count-1) { - numblocks = (ext2fs_blocks_count(fs->super) - - (blk64_t) fs->super->s_first_data_block) % - (blk64_t) fs->super->s_blocks_per_group; - if (!numblocks) - numblocks = fs->super->s_blocks_per_group; - } else - numblocks = fs->super->s_blocks_per_group; + numblocks = ext2fs_group_blocks_count(fs, group); if (ret_super_blk) *ret_super_blk = (blk_t)ret_super_blk2; diff --git a/lib/ext2fs/ext2fs.h b/lib/ext2fs/ext2fs.h index dc83fb0..60a903a 100644 --- a/lib/ext2fs/ext2fs.h +++ b/lib/ext2fs/ext2fs.h @@ -757,6 +757,7 @@ extern errcode_t ext2fs_get_block_bitmap_range2(ext2fs_block_bitmap bmap, extern dgrp_t ext2fs_group_of_blk2(ext2_filsys fs, blk64_t); extern blk64_t ext2fs_group_first_block2(ext2_filsys fs, dgrp_t group); extern blk64_t ext2fs_group_last_block2(ext2_filsys fs, dgrp_t group); +extern int ext2fs_group_blocks_count(ext2_filsys fs, dgrp_t group); extern blk64_t ext2fs_inode_data_blocks2(ext2_filsys fs, struct ext2_inode *inode); extern blk64_t ext2fs_inode_i_blocks(ext2_filsys fs, diff --git a/resize/online.c b/resize/online.c index 1d8d4ec..c2b98c6 100644 --- a/resize/online.c +++ b/resize/online.c @@ -135,12 +135,7 @@ errcode_t online_resize_fs(ext2_filsys fs, const char *mtpt, input.block_bitmap = ext2fs_block_bitmap_loc(new_fs, i); input.inode_bitmap = ext2fs_inode_bitmap_loc(new_fs, i); input.inode_table = ext2fs_inode_table_loc(new_fs, i); - input.blocks_count = sb->s_blocks_per_group; - if (i == new_fs->group_desc_count-1) { - input.blocks_count = ext2fs_blocks_count(new_fs->super) - - sb->s_first_data_block - - (i * sb->s_blocks_per_group); - } + input.blocks_count = ext2fs_group_blocks_count(new_fs, i); input.reserved_blocks = (blk_t) (percent * input.blocks_count / 100.0); diff --git a/resize/resize2fs.c b/resize/resize2fs.c index 45ea5f4..a8e7d51 100644 --- a/resize/resize2fs.c +++ b/resize/resize2fs.c @@ -499,18 +499,8 @@ retry: ext2fs_bg_flags_zap(fs, i); if (csum_flag) ext2fs_bg_flags_set(fs, i, EXT2_BG_INODE_UNINIT | EXT2_BG_INODE_ZEROED); - if (i == fs->group_desc_count-1) { - numblocks = (ext2fs_blocks_count(fs->super) - - fs->super->s_first_data_block) % - fs->super->s_blocks_per_group; - if (!numblocks) - numblocks = fs->super->s_blocks_per_group; - } else { - numblocks = fs->super->s_blocks_per_group; - if (csum_flag) - ext2fs_bg_flags_set(fs, i, - EXT2_BG_BLOCK_UNINIT); - } + + numblocks = ext2fs_group_blocks_count(fs, i); has_super = ext2fs_bg_has_super(fs, i); if (has_super) { ^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [PATCH V2] e2fsprogs: add ext2fs_group_blocks_count helper 2011-07-20 21:57 ` [PATCH V2] " Eric Sandeen @ 2011-07-21 13:55 ` Eric Sandeen 2011-07-21 14:57 ` Yongqiang Yang 2011-07-21 18:15 ` [PATCH V3] " Eric Sandeen 1 sibling, 1 reply; 13+ messages in thread From: Eric Sandeen @ 2011-07-21 13:55 UTC (permalink / raw) To: Yongqiang Yang; +Cc: linux-ext4, tytso On 7/20/11 4:57 PM, Eric Sandeen wrote: > Code to count the number of blocks in the last partial > group is cut and pasted around the e2fsprogs codebase, and > is wrong in at least one instancem as pointed out by > Yongqiang Yang (but not fixed in this patch). > > Making this a helper function should improve matters. > > Signed-off-by: Eric Sandeen <sandeen@redhat.com> > --- Argh V3 coming today, I got a little too fast and loose sending this one at the end of the day yesterday didn't I: > @@ -499,18 +499,8 @@ retry: > ext2fs_bg_flags_zap(fs, i); > if (csum_flag) > ext2fs_bg_flags_set(fs, i, EXT2_BG_INODE_UNINIT | EXT2_BG_INODE_ZEROED); > - if (i == fs->group_desc_count-1) { > - numblocks = (ext2fs_blocks_count(fs->super) - > - fs->super->s_first_data_block) % > - fs->super->s_blocks_per_group; > - if (!numblocks) > - numblocks = fs->super->s_blocks_per_group; > - } else { > - numblocks = fs->super->s_blocks_per_group; > - if (csum_flag) > - ext2fs_bg_flags_set(fs, i, > - EXT2_BG_BLOCK_UNINIT); still need to do that flag set with the new helper. - Eric > - } > + > + numblocks = ext2fs_group_blocks_count(fs, i); > > has_super = ext2fs_bg_has_super(fs, i); > if (has_super) { > > -- > 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 ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH V2] e2fsprogs: add ext2fs_group_blocks_count helper 2011-07-21 13:55 ` Eric Sandeen @ 2011-07-21 14:57 ` Yongqiang Yang 2011-07-21 15:06 ` Eric Sandeen 0 siblings, 1 reply; 13+ messages in thread From: Yongqiang Yang @ 2011-07-21 14:57 UTC (permalink / raw) To: Eric Sandeen; +Cc: linux-ext4, tytso On Thu, Jul 21, 2011 at 9:55 PM, Eric Sandeen <sandeen@redhat.com> wrote: > On 7/20/11 4:57 PM, Eric Sandeen wrote: >> Code to count the number of blocks in the last partial >> group is cut and pasted around the e2fsprogs codebase, and >> is wrong in at least one instancem as pointed out by >> Yongqiang Yang (but not fixed in this patch). >> >> Making this a helper function should improve matters. >> >> Signed-off-by: Eric Sandeen <sandeen@redhat.com> >> --- > > Argh V3 coming today, I got a little too fast and loose sending this one > at the end of the day yesterday didn't I: Hi Eric, It seemed you did not send out the v3 patch. You just pointed out what to be added in the V3 patch. Yongqiang. > >> @@ -499,18 +499,8 @@ retry: >> ext2fs_bg_flags_zap(fs, i); >> if (csum_flag) >> ext2fs_bg_flags_set(fs, i, EXT2_BG_INODE_UNINIT | EXT2_BG_INODE_ZEROED); >> - if (i == fs->group_desc_count-1) { >> - numblocks = (ext2fs_blocks_count(fs->super) - >> - fs->super->s_first_data_block) % >> - fs->super->s_blocks_per_group; >> - if (!numblocks) >> - numblocks = fs->super->s_blocks_per_group; >> - } else { >> - numblocks = fs->super->s_blocks_per_group; >> - if (csum_flag) >> - ext2fs_bg_flags_set(fs, i, >> - EXT2_BG_BLOCK_UNINIT); > > still need to do that flag set with the new helper. > > - Eric > >> - } >> + >> + numblocks = ext2fs_group_blocks_count(fs, i); >> >> has_super = ext2fs_bg_has_super(fs, i); >> if (has_super) { >> >> -- >> 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 > > -- Best Wishes Yongqiang Yang -- 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 ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH V2] e2fsprogs: add ext2fs_group_blocks_count helper 2011-07-21 14:57 ` Yongqiang Yang @ 2011-07-21 15:06 ` Eric Sandeen 0 siblings, 0 replies; 13+ messages in thread From: Eric Sandeen @ 2011-07-21 15:06 UTC (permalink / raw) To: Yongqiang Yang; +Cc: linux-ext4, tytso On 7/21/11 9:57 AM, Yongqiang Yang wrote: > On Thu, Jul 21, 2011 at 9:55 PM, Eric Sandeen <sandeen@redhat.com> wrote: >> On 7/20/11 4:57 PM, Eric Sandeen wrote: >>> Code to count the number of blocks in the last partial >>> group is cut and pasted around the e2fsprogs codebase, and >>> is wrong in at least one instancem as pointed out by >>> Yongqiang Yang (but not fixed in this patch). >>> >>> Making this a helper function should improve matters. >>> >>> Signed-off-by: Eric Sandeen <sandeen@redhat.com> >>> --- >> >> Argh V3 coming today, I got a little too fast and loose sending this one >> at the end of the day yesterday didn't I: > Hi Eric, > > It seemed you did not send out the v3 patch. You just pointed out > what to be added in the V3 patch. That is correct. :) I'll send it out a bit later after I carefully review this time :( -Eric > Yongqiang. >> >>> @@ -499,18 +499,8 @@ retry: >>> ext2fs_bg_flags_zap(fs, i); >>> if (csum_flag) >>> ext2fs_bg_flags_set(fs, i, EXT2_BG_INODE_UNINIT | EXT2_BG_INODE_ZEROED); >>> - if (i == fs->group_desc_count-1) { >>> - numblocks = (ext2fs_blocks_count(fs->super) - >>> - fs->super->s_first_data_block) % >>> - fs->super->s_blocks_per_group; >>> - if (!numblocks) >>> - numblocks = fs->super->s_blocks_per_group; >>> - } else { >>> - numblocks = fs->super->s_blocks_per_group; >>> - if (csum_flag) >>> - ext2fs_bg_flags_set(fs, i, >>> - EXT2_BG_BLOCK_UNINIT); >> >> still need to do that flag set with the new helper. >> >> - Eric >> >>> - } >>> + >>> + numblocks = ext2fs_group_blocks_count(fs, i); >>> >>> has_super = ext2fs_bg_has_super(fs, i); >>> if (has_super) { >>> >>> -- >>> 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 >> >> > > > ^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH V3] e2fsprogs: add ext2fs_group_blocks_count helper 2011-07-20 21:57 ` [PATCH V2] " Eric Sandeen 2011-07-21 13:55 ` Eric Sandeen @ 2011-07-21 18:15 ` Eric Sandeen 2011-09-16 13:22 ` [V3] " Ted Ts'o 1 sibling, 1 reply; 13+ messages in thread From: Eric Sandeen @ 2011-07-21 18:15 UTC (permalink / raw) To: Yongqiang Yang; +Cc: linux-ext4, tytso Code to count the number of blocks in the last partial group is cut and pasted around the e2fsprogs codebase a few times. Making this a helper function should improve matters. Signed-off-by: Eric Sandeen <sandeen@redhat.com> --- V2: Sorry, first patch was against an old tree. V3: Restore lost EXT2_BG_BLOCK_UNINIT in resize2fs.c diff --git a/lib/ext2fs/alloc_sb.c b/lib/ext2fs/alloc_sb.c index 09786a7..2e32389 100644 --- a/lib/ext2fs/alloc_sb.c +++ b/lib/ext2fs/alloc_sb.c @@ -74,15 +74,7 @@ int ext2fs_reserve_super_and_bgd(ext2_filsys fs, if (new_desc_blk) ext2fs_mark_block_bitmap2(bmap, new_desc_blk); - if (group == fs->group_desc_count-1) { - num_blocks = (ext2fs_blocks_count(fs->super) - - fs->super->s_first_data_block) % - fs->super->s_blocks_per_group; - if (!num_blocks) - num_blocks = fs->super->s_blocks_per_group; - } else - num_blocks = fs->super->s_blocks_per_group; - + num_blocks = ext2fs_group_blocks_count(fs, group); num_blocks -= 2 + fs->inode_blocks_per_group + used_blks; return num_blocks ; diff --git a/lib/ext2fs/blknum.c b/lib/ext2fs/blknum.c index b3e6dca..b9ac36c 100644 --- a/lib/ext2fs/blknum.c +++ b/lib/ext2fs/blknum.c @@ -43,6 +43,25 @@ blk64_t ext2fs_group_last_block2(ext2_filsys fs, dgrp_t group) } /* + * Return the number of blocks in a group + */ +int ext2fs_group_blocks_count(ext2_filsys fs, dgrp_t group) +{ + int num_blocks; + + if (group == fs->group_desc_count - 1) { + num_blocks = (ext2fs_blocks_count(fs->super) - + fs->super->s_first_data_block) % + fs->super->s_blocks_per_group; + if (!num_blocks) + num_blocks = fs->super->s_blocks_per_group; + } else + num_blocks = fs->super->s_blocks_per_group; + + return num_blocks; +} + +/* * Return the inode data block count */ blk64_t ext2fs_inode_data_blocks2(ext2_filsys fs, diff --git a/lib/ext2fs/closefs.c b/lib/ext2fs/closefs.c index 952f496..51ef63c 100644 --- a/lib/ext2fs/closefs.c +++ b/lib/ext2fs/closefs.c @@ -152,14 +152,7 @@ int ext2fs_super_and_bgd_loc(ext2_filsys fs, &ret_new_desc_blk2, &ret_used_blks); - if (group == fs->group_desc_count-1) { - numblocks = (ext2fs_blocks_count(fs->super) - - (blk64_t) fs->super->s_first_data_block) % - (blk64_t) fs->super->s_blocks_per_group; - if (!numblocks) - numblocks = fs->super->s_blocks_per_group; - } else - numblocks = fs->super->s_blocks_per_group; + numblocks = ext2fs_group_blocks_count(fs, group); if (ret_super_blk) *ret_super_blk = (blk_t)ret_super_blk2; diff --git a/lib/ext2fs/ext2fs.h b/lib/ext2fs/ext2fs.h index dc83fb0..60a903a 100644 --- a/lib/ext2fs/ext2fs.h +++ b/lib/ext2fs/ext2fs.h @@ -757,6 +757,7 @@ extern errcode_t ext2fs_get_block_bitmap_range2(ext2fs_block_bitmap bmap, extern dgrp_t ext2fs_group_of_blk2(ext2_filsys fs, blk64_t); extern blk64_t ext2fs_group_first_block2(ext2_filsys fs, dgrp_t group); extern blk64_t ext2fs_group_last_block2(ext2_filsys fs, dgrp_t group); +extern int ext2fs_group_blocks_count(ext2_filsys fs, dgrp_t group); extern blk64_t ext2fs_inode_data_blocks2(ext2_filsys fs, struct ext2_inode *inode); extern blk64_t ext2fs_inode_i_blocks(ext2_filsys fs, diff --git a/resize/online.c b/resize/online.c index 1d8d4ec..c2b98c6 100644 --- a/resize/online.c +++ b/resize/online.c @@ -135,12 +135,7 @@ errcode_t online_resize_fs(ext2_filsys fs, const char *mtpt, input.block_bitmap = ext2fs_block_bitmap_loc(new_fs, i); input.inode_bitmap = ext2fs_inode_bitmap_loc(new_fs, i); input.inode_table = ext2fs_inode_table_loc(new_fs, i); - input.blocks_count = sb->s_blocks_per_group; - if (i == new_fs->group_desc_count-1) { - input.blocks_count = ext2fs_blocks_count(new_fs->super) - - sb->s_first_data_block - - (i * sb->s_blocks_per_group); - } + input.blocks_count = ext2fs_group_blocks_count(new_fs, i); input.reserved_blocks = (blk_t) (percent * input.blocks_count / 100.0); diff --git a/resize/resize2fs.c b/resize/resize2fs.c index 45ea5f4..59beb35 100644 --- a/resize/resize2fs.c +++ b/resize/resize2fs.c @@ -499,18 +499,10 @@ retry: ext2fs_bg_flags_zap(fs, i); if (csum_flag) ext2fs_bg_flags_set(fs, i, EXT2_BG_INODE_UNINIT | EXT2_BG_INODE_ZEROED); - if (i == fs->group_desc_count-1) { - numblocks = (ext2fs_blocks_count(fs->super) - - fs->super->s_first_data_block) % - fs->super->s_blocks_per_group; - if (!numblocks) - numblocks = fs->super->s_blocks_per_group; - } else { - numblocks = fs->super->s_blocks_per_group; - if (csum_flag) - ext2fs_bg_flags_set(fs, i, - EXT2_BG_BLOCK_UNINIT); - } + + numblocks = ext2fs_group_blocks_count(fs, i); + if ((i < fs->group_desc_count - 1) && csum_flag) + ext2fs_bg_flags_set(fs, i, EXT2_BG_BLOCK_UNINIT); has_super = ext2fs_bg_has_super(fs, i); if (has_super) { ^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [V3] e2fsprogs: add ext2fs_group_blocks_count helper 2011-07-21 18:15 ` [PATCH V3] " Eric Sandeen @ 2011-09-16 13:22 ` Ted Ts'o 0 siblings, 0 replies; 13+ messages in thread From: Ted Ts'o @ 2011-09-16 13:22 UTC (permalink / raw) To: Eric Sandeen; +Cc: Yongqiang Yang, linux-ext4 On Thu, Jul 21, 2011 at 08:15:50AM -0000, Eric Sandeen wrote: > Code to count the number of blocks in the last partial > group is cut and pasted around the e2fsprogs codebase > a few times. > > Making this a helper function should improve matters. > > Signed-off-by: Eric Sandeen <sandeen@redhat.com> Thanks, applied. Apologies for the delay. - Ted ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH] e2fsprogs: add ext2fs_group_blocks_count helper 2011-07-20 21:46 ` [PATCH] e2fsprogs: add ext2fs_group_blocks_count helper Eric Sandeen 2011-07-20 21:57 ` [PATCH V2] " Eric Sandeen @ 2011-07-21 14:54 ` Yongqiang Yang 1 sibling, 0 replies; 13+ messages in thread From: Yongqiang Yang @ 2011-07-21 14:54 UTC (permalink / raw) To: Eric Sandeen; +Cc: linux-ext4, tytso On Thu, Jul 21, 2011 at 5:46 AM, Eric Sandeen <sandeen@redhat.com> wrote: > Code to count the number of blocks in the last partial > group is cut and pasted around the e2fsprogs codebase, and > is wrong in at least one instancem as pointed out by > Yongqiang Yang (but not fixed in this patch). > > Making this a helper function should improve matters. > > Signed-off-by: Eric Sandeen <sandeen@redhat.com> > --- > > (I have not fixed up the spot Yongqiang Yang discovered, > but this helper should now make that fix easy to do.) > > e2fsck/pass5.c | 6 +----- > lib/ext2fs/alloc_sb.c | 10 +--------- > lib/ext2fs/blknum.c | 19 +++++++++++++++++++ > lib/ext2fs/closefs.c | 9 +-------- > lib/ext2fs/ext2fs.h | 1 + > resize/online.c | 7 +------ > resize/resize2fs.c | 14 ++------------ > 7 files changed, 26 insertions(+), 40 deletions(-) > > > diff --git a/e2fsck/pass5.c b/e2fsck/pass5.c > index b423d28..73db4e8 100644 > --- a/e2fsck/pass5.c > +++ b/e2fsck/pass5.c > @@ -218,11 +218,7 @@ redo_counts: > fs->super->s_reserved_gdt_blocks; > > count = 0; > - cmp_block = fs->super->s_blocks_per_group; > - if (group == (int)fs->group_desc_count - 1) > - cmp_block = > - ext2fs_blocks_count(fs->super) % > - fs->super->s_blocks_per_group; > + cmp_block = ext2fs_group_blocks_count(fs, group); > } Hi Eric, You fixed the bug I pointed out here. but your commit log said it is not fixed. Yongqiang. > > bitmap = 0; > diff --git a/lib/ext2fs/alloc_sb.c b/lib/ext2fs/alloc_sb.c > index d5fca3b..98aec52 100644 > --- a/lib/ext2fs/alloc_sb.c > +++ b/lib/ext2fs/alloc_sb.c > @@ -71,15 +71,7 @@ int ext2fs_reserve_super_and_bgd(ext2_filsys fs, > if (new_desc_blk) > ext2fs_mark_block_bitmap2(bmap, new_desc_blk); > > - if (group == fs->group_desc_count-1) { > - num_blocks = (ext2fs_blocks_count(fs->super) - > - fs->super->s_first_data_block) % > - fs->super->s_blocks_per_group; > - if (!num_blocks) > - num_blocks = fs->super->s_blocks_per_group; > - } else > - num_blocks = fs->super->s_blocks_per_group; > - > + num_blocks = ext2fs_group_blocks_count(fs, group); > num_blocks -= 2 + fs->inode_blocks_per_group + used_blks; > > return num_blocks ; > diff --git a/lib/ext2fs/blknum.c b/lib/ext2fs/blknum.c > index b3e6dca..f1bbe9b 100644 > --- a/lib/ext2fs/blknum.c > +++ b/lib/ext2fs/blknum.c > @@ -43,6 +43,25 @@ blk64_t ext2fs_group_last_block2(ext2_filsys fs, dgrp_t group) > } > > /* > + * Return the number of blocks in a group > + */ > +int ext2fs_group_blocks_count(ext2_filsys fs, dgrp_t group) > +{ > + int num_blocks; > + > + if (group == fs->group_desc_count-1) { > + num_blocks = (ext2fs_blocks_count(fs->super) - > + fs->super->s_first_data_block) % > + fs->super->s_blocks_per_group; > + if (!num_blocks) > + num_blocks = fs->super->s_blocks_per_group; > + } else > + num_blocks = fs->super->s_blocks_per_group; > + > + return num_blocks; > +} > + > +/* > * Return the inode data block count > */ > blk64_t ext2fs_inode_data_blocks2(ext2_filsys fs, > diff --git a/lib/ext2fs/closefs.c b/lib/ext2fs/closefs.c > index 7a23e46..b13a0c9 100644 > --- a/lib/ext2fs/closefs.c > +++ b/lib/ext2fs/closefs.c > @@ -150,14 +150,7 @@ int ext2fs_super_and_bgd_loc(ext2_filsys fs, > &ret_new_desc_blk2, > &ret_used_blks); > > - if (group == fs->group_desc_count-1) { > - numblocks = (ext2fs_blocks_count(fs->super) - > - (blk64_t) fs->super->s_first_data_block) % > - (blk64_t) fs->super->s_blocks_per_group; > - if (!numblocks) > - numblocks = fs->super->s_blocks_per_group; > - } else > - numblocks = fs->super->s_blocks_per_group; > + numblocks = ext2fs_group_blocks_count(fs, group); > > if (ret_super_blk) > *ret_super_blk = (blk_t)ret_super_blk2; > diff --git a/lib/ext2fs/ext2fs.h b/lib/ext2fs/ext2fs.h > index d3eb31d..5983adc 100644 > --- a/lib/ext2fs/ext2fs.h > +++ b/lib/ext2fs/ext2fs.h > @@ -731,6 +731,7 @@ extern errcode_t ext2fs_get_block_bitmap_range2(ext2fs_block_bitmap bmap, > extern dgrp_t ext2fs_group_of_blk2(ext2_filsys fs, blk64_t); > extern blk64_t ext2fs_group_first_block2(ext2_filsys fs, dgrp_t group); > extern blk64_t ext2fs_group_last_block2(ext2_filsys fs, dgrp_t group); > +extern int ext2fs_group_blocks_count(ext2_filsys fs, dgrp_t group); > extern blk64_t ext2fs_inode_data_blocks2(ext2_filsys fs, > struct ext2_inode *inode); > extern blk64_t ext2fs_inode_i_blocks(ext2_filsys fs, > diff --git a/resize/online.c b/resize/online.c > index 1d8d4ec..c2b98c6 100644 > --- a/resize/online.c > +++ b/resize/online.c > @@ -135,12 +135,7 @@ errcode_t online_resize_fs(ext2_filsys fs, const char *mtpt, > input.block_bitmap = ext2fs_block_bitmap_loc(new_fs, i); > input.inode_bitmap = ext2fs_inode_bitmap_loc(new_fs, i); > input.inode_table = ext2fs_inode_table_loc(new_fs, i); > - input.blocks_count = sb->s_blocks_per_group; > - if (i == new_fs->group_desc_count-1) { > - input.blocks_count = ext2fs_blocks_count(new_fs->super) - > - sb->s_first_data_block - > - (i * sb->s_blocks_per_group); > - } > + input.blocks_count = ext2fs_group_blocks_count(new_fs, i); > input.reserved_blocks = (blk_t) (percent * input.blocks_count > / 100.0); > > diff --git a/resize/resize2fs.c b/resize/resize2fs.c > index 216a626..4b83ca9 100644 > --- a/resize/resize2fs.c > +++ b/resize/resize2fs.c > @@ -499,18 +499,8 @@ retry: > ext2fs_bg_flags_zap(fs, i); > if (csum_flag) > ext2fs_bg_flags_set(fs, i, EXT2_BG_INODE_UNINIT | EXT2_BG_INODE_ZEROED); > - if (i == fs->group_desc_count-1) { > - numblocks = (ext2fs_blocks_count(fs->super) - > - fs->super->s_first_data_block) % > - fs->super->s_blocks_per_group; > - if (!numblocks) > - numblocks = fs->super->s_blocks_per_group; > - } else { > - numblocks = fs->super->s_blocks_per_group; > - if (csum_flag) > - ext2fs_bg_flags_set(fs, i, > - EXT2_BG_BLOCK_UNINIT); > - } > + > + numblocks = ext2fs_group_blocks_count(fs, i); > > has_super = ext2fs_bg_has_super(fs, i); > if (has_super) { > > -- Best Wishes Yongqiang Yang -- 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 ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH] e2fsck: fix error in computing blocks of the ending group 2011-07-19 16:18 ` Eric Sandeen 2011-07-20 21:46 ` [PATCH] e2fsprogs: add ext2fs_group_blocks_count helper Eric Sandeen @ 2011-07-21 17:40 ` Eric Sandeen 1 sibling, 0 replies; 13+ messages in thread From: Eric Sandeen @ 2011-07-21 17:40 UTC (permalink / raw) To: Yongqiang Yang; +Cc: linux-ext4, tytso On 7/19/11 11:18 AM, Eric Sandeen wrote: > On 07/19/2011 10:46 AM, Yongqiang Yang wrote: >> If the blocks of a filesystem is a multiple of blocks_per_group, >> blocks of the ending group is computed wrongly. This patch computes >> it by substracting blocks processed from blocks of the filesystem. >> >> Signed-off-by: Yongqiang Yang <xiaoqiangnk@gmail.com> >> --- >> e2fsck/pass5.c | 2 +- >> 1 files changed, 1 insertions(+), 1 deletions(-) >> >> diff --git a/e2fsck/pass5.c b/e2fsck/pass5.c >> index f9d746c..7c4e336 100644 >> --- a/e2fsck/pass5.c >> +++ b/e2fsck/pass5.c >> @@ -227,7 +227,7 @@ redo_counts: >> if (group == (int)fs->group_desc_count - 1) >> cmp_block = >> EXT2FS_NUM_B2C(fs, >> - ext2fs_blocks_count(fs->super) % fs->super->s_blocks_per_group); >> + ext2fs_blocks_count(fs->super) - i); >> } >> >> bitmap = 0; > > I think it would be clearer to just get the last group block count from > superblock information, rather than from using the loop counter. Actually I see that I misread this... here we are dealing with clusters per group and my proposed blocks per group helper won't help. Sorry for the noise, I think your patch should go in as proposed. I'll still send my helper, but I don't think it will help in this location, and your patch is correct. -Eric ^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH V2] e2fsck: fix error in computing blocks of the ending group 2011-07-19 15:46 [PATCH] e2fsck: fix error in computing blocks of the ending group Yongqiang Yang 2011-07-19 16:18 ` Eric Sandeen @ 2011-07-22 4:18 ` Eric Sandeen 2011-09-16 13:29 ` [V2] " Ted Ts'o 1 sibling, 1 reply; 13+ messages in thread From: Eric Sandeen @ 2011-07-22 4:18 UTC (permalink / raw) To: Yongqiang Yang; +Cc: linux-ext4, tytso From: Yongqiang Yang <xiaoqiangnk@gmail.com> If the blocks of a filesystem is a multiple of blocks_per_group, blocks of the ending group is computed wrongly. Use the new ext2fs_group_blocks_count() helper instead. Eric Sandeen: Converted to use new blocks per group helper Signed-off-by: Yongqiang Yang <xiaoqiangnk@gmail.com> --- diff --git a/e2fsck/pass5.c b/e2fsck/pass5.c index f9d746c..2a28c7a 100644 --- a/e2fsck/pass5.c +++ b/e2fsck/pass5.c @@ -225,9 +225,8 @@ redo_counts: count = 0; cmp_block = fs->super->s_clusters_per_group; if (group == (int)fs->group_desc_count - 1) - cmp_block = - EXT2FS_NUM_B2C(fs, - ext2fs_blocks_count(fs->super) % fs->super->s_blocks_per_group); + cmp_block = EXT2FS_NUM_B2C(fs, + ext2fs_group_blocks_count(fs, group)); } bitmap = 0; ^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [V2] e2fsck: fix error in computing blocks of the ending group 2011-07-22 4:18 ` [PATCH V2] " Eric Sandeen @ 2011-09-16 13:29 ` Ted Ts'o 0 siblings, 0 replies; 13+ messages in thread From: Ted Ts'o @ 2011-09-16 13:29 UTC (permalink / raw) To: Eric Sandeen; +Cc: Yongqiang Yang, linux-ext4 On Thu, Jul 21, 2011 at 06:18:57PM -0000, Eric Sandeen wrote: > From: Yongqiang Yang <xiaoqiangnk@gmail.com> > > If the blocks of a filesystem is a multiple of blocks_per_group, > blocks of the ending group is computed wrongly. Use the > new ext2fs_group_blocks_count() helper instead. > > Eric Sandeen: Converted to use new blocks per group helper > > Signed-off-by: Yongqiang Yang <xiaoqiangnk@gmail.com> Thanks applied to the next branch. Apologies for the delay. - Ted ^ permalink raw reply [flat|nested] 13+ messages in thread
end of thread, other threads:[~2011-09-16 13:29 UTC | newest] Thread overview: 13+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2011-07-19 15:46 [PATCH] e2fsck: fix error in computing blocks of the ending group Yongqiang Yang 2011-07-19 16:18 ` Eric Sandeen 2011-07-20 21:46 ` [PATCH] e2fsprogs: add ext2fs_group_blocks_count helper Eric Sandeen 2011-07-20 21:57 ` [PATCH V2] " Eric Sandeen 2011-07-21 13:55 ` Eric Sandeen 2011-07-21 14:57 ` Yongqiang Yang 2011-07-21 15:06 ` Eric Sandeen 2011-07-21 18:15 ` [PATCH V3] " Eric Sandeen 2011-09-16 13:22 ` [V3] " Ted Ts'o 2011-07-21 14:54 ` [PATCH] " Yongqiang Yang 2011-07-21 17:40 ` [PATCH] e2fsck: fix error in computing blocks of the ending group Eric Sandeen 2011-07-22 4:18 ` [PATCH V2] " Eric Sandeen 2011-09-16 13:29 ` [V2] " Ted Ts'o
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).