Re: [PATCH 03/15] mke2fs: simplify inode table block counting

[Namhyung Kim <namhyung@gmail.com>]

2010-11-30 (화), 13:01 +0100, Lukas Czerner:
> On Mon, 29 Nov 2010, Namhyung Kim wrote:
> 
> > Simplify counting of inode table blocks in a block group using
> > local variable 'ipb'. Otherwise the variable could be removed
> > because there was no user of it.
> > 
> > Signed-off-by: Namhyung Kim <namhyung@gmail.com>
> > ---
> >  misc/mke2fs.c |    7 ++-----
> >  1 files changed, 2 insertions(+), 5 deletions(-)
> > 
> > diff --git a/misc/mke2fs.c b/misc/mke2fs.c
> > index 0980045..9fb5d5f 100644
> > --- a/misc/mke2fs.c
> > +++ b/misc/mke2fs.c
> > @@ -320,11 +320,8 @@ static void write_inode_tables(ext2_filsys fs, int lazy_flag, int itable_zeroed)
> >  
> >  		if (lazy_flag) {
> >  			ipb = fs->blocksize / EXT2_INODE_SIZE(fs->super);
> > -			num = ((((fs->super->s_inodes_per_group -
> > -				  ext2fs_bg_itable_unused(fs, i)) *
> > -				 EXT2_INODE_SIZE(fs->super)) +
> > -				EXT2_BLOCK_SIZE(fs->super) - 1) /
> > -			       EXT2_BLOCK_SIZE(fs->super));
> > +			num = (fs->super->s_inodes_per_group -
> > +			       ext2fs_bg_itable_unused(fs, i) + ipb - 1) / ipb;
> >  		}
> >  		if (!lazy_flag || itable_zeroed) {
> >  			/* The kernel doesn't need to zero the itable blocks */
> > 
> 
> Hi,
> 
> I would rather add this macro into header file (lib/ext2fs/ext2fs.h
> maybe?)
> 
> #define DIV_ROUND_UP(n,d) (((n) + (d) - 1) / (d))
> 
> remoevd ipb variable because it is rather useless. And do something like
> this ?
> 
> num = DIV_ROUND_UP((fs->super->s_inodes_per_group -
> 		    ext2fs_bg_itable_unused(fs, i)) *
> 		   EXT2_INODE_SIZE(fs->super), 
> 		   EXT2_BLOCK_SIZE(fs->super));
> 
> IMO it improves readability a lot and I am sure that there are other
> places which may take advantage of the DIV_ROUND_UP macro.
> 
> Thanks!
> 
> -Lukas

Hi, Lukas.
Thanks for the review.

We have ext2fs_div[64]_ceil() for that. So I can use it like

ipb = fs->blocksize / EXT2_INODE_SIZE(fs->super);
num = ext2_fs_div_ceil(fs->super->s_inodes_per_group -
			ext2fs_bg_itable_unused(fs, i), ipb);

or

num = ext2_fs_div_ceil((fs->super->s_inodes_per_group -
			ext2fs_bg_itable_unused(fs, i)) *
		       EXT2_INODE_SIZE(fs->super),
		       EXT2_BLOCK_SIZE(fs->super));

Either is fine to me. Ted, what's your preference?

-- 
Regards,
Namhyung Kim


--
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