On Thu, Aug 24, 2017 at 11:08:52AM +0200, Jaco Kroon wrote:
> > +#define E2F_OPT_ICOUNT_FULLMAP 0x20000 /* don't optimize extents */
> description here seems wrong? Perhaps "use fullmap for inode link count" ?
Oops, fixed.
> > --- a/e2fsck/pass1.c
> > +++ b/e2fsck/pass1.c
> > @@ -711,6 +711,7 @@ extern errcode_t e2fsck_setup_icount(e2fsck_t ctx, const char *icount_name,
> > errcode_t retval;
> > char *tdb_dir;
> > int enable;
> > + int full_map;
> > *ret = 0;
> > @@ -734,6 +735,8 @@ extern errcode_t e2fsck_setup_icount(e2fsck_t ctx, const char *icount_name,
> > }
> > e2fsck_set_bitmap_type(ctx->fs, EXT2FS_BMAP64_RBTREE, icount_name,
> > &save_type);
> > + if (ctx->options & E2F_OPT_ICOUNT_FULLMAP)
> > + flags |= EXT2_ICOUNT_OPT_FULLMAP;
> > retval = ext2fs_create_icount2(ctx->fs, flags, 0, hint, ret);
> > ctx->fs->default_bitmap_type = save_type;
> > return retval;
> This isn't used for pass1 (I did originally use full_map either way then
> decided to only use it for pass2) - and we to receive flags coming into this
> function - shouldn't this perhaps rather go into pass2.c (e2fsck_pass2
> function to be more specific)? This really is cosmetic imho.
The e2fsck_setup_icount() function is used for both pass1 and pass2.
> > diff --git a/e2fsck/unix.c b/e2fsck/unix.c
> > index ff961483b..939862f1a 100644
> > --- a/e2fsck/unix.c
> > +++ b/e2fsck/unix.c
> > @@ -709,9 +709,18 @@ static void parse_extended_opts(e2fsck_t ctx, const char *opts)
> > } else if (strcmp(token, "nodiscard") == 0) {
> > ctx->options &= ~E2F_OPT_DISCARD;
> > continue;
> > + } else if (strcmp(token, "optimize_extents") == 0) {
> > + ctx->options &= ~E2F_OPT_NOOPT_EXTENTS;
> > + continue;
> unrelated, but makes sense :).
Yes, this was a cleanup I was doing as I weant.
> > + if (!retval) {
> > + memset(icount->fullmap, 0,
> > + *sizeof(__u32)* * fs->super->s_inodes_count); <-------
> this of course should be sizeof(__u16), or sizeof(*count->fullmap) which
> will track the type pointed to. It might be better practice to store
> "sizeof(*icount->fullmap) * fs->super->s_inodes_count" to a variable and
> just re-use it.
Good point. Fixed.
> > goto errout;
> > +
> Extra line can be removed.
> > if (flags & EXT2_ICOUNT_OPT_INCREMENT) {
Removed.
Thanks for taking a look at the patch!
- Ted
