linux-ext4 - Re: [PATCH] e2fsck: add optimization for heavily hard-linked file systems

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [thread-next>] [day] [month] [year] [list]

Message-ID: <20170826153313.le7biqxzg7mz6f32@thunk.org>
Date:   Sat, 26 Aug 2017 11:33:14 -0400
From:   Theodore Ts'o <tytso@....edu>
To:     Jaco Kroon <jaco@....co.za>
Cc:     Doug Porter <dsp@...com>, linux-ext4@...r.kernel.org,
        Omar Sandoval <osandov@...com>
Subject: Re: [PATCH] e2fsck: add optimization for heavily hard-linked file
 systems

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