linux-ext4 - Re: [PATCH 10/37] mke2fs: Allow metadata checksums to be turned on at mkfs time

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: Windows password security audit tool. GUI, reports in PDF.

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

Message-Id: <4764BABB-0FFB-4AE5-B6A3-EF804AA59FC8@dilger.ca>
Date:	Sun, 4 Sep 2011 12:28:24 -0600
From:	Andreas Dilger <adilger@...ger.ca>
To:	"Darrick J. Wong" <djwong@...ibm.com>
Cc:	Andreas Dilger <adilger.kernel@...ger.ca>,
	Theodore Tso <tytso@....edu>,
	"Darrick J. Wong" <djwong@...ibm.com>,
	Sunil Mushran <sunil.mushran@...cle.com>,
	Amir Goldstein <amir73il@...il.com>,
	Andi Kleen <andi@...stfloor.org>,
	Mingming Cao <cmm@...ibm.com>,
	Joel Becker <jlbec@...lplan.org>,
	"linux-ext4@...r.kernel.org" <linux-ext4@...r.kernel.org>,
	Coly Li <colyli@...il.com>
Subject: Re: [PATCH 10/37] mke2fs: Allow metadata checksums to be turned on at mkfs time

On 2011-08-31, at 6:36 PM, "Darrick J. Wong" <djwong@...ibm.com> wrote:
> Write out checksummed inodes even when writing out a zeroed table.
> 
> Signed-off-by: Darrick J. Wong <djwong@...ibm.com>
> ---
> misc/mke2fs.c |   37 ++++++++++++++++++++++++++++++-------
> 1 files changed, 30 insertions(+), 7 deletions(-)
> 
> 
> diff --git a/misc/mke2fs.c b/misc/mke2fs.c
> index 2d57d09..bbc0533 100644
> --- a/misc/mke2fs.c
> +++ b/misc/mke2fs.c
> @@ -309,6 +309,8 @@ static void write_inode_tables(ext2_filsys fs, int lazy_flag, int itable_zeroed)
>    dgrp_t        i;
>    int        num;
>    struct ext2fs_numeric_progress_struct progress;
> +    ext2_ino_t    ino;
> +    struct ext2_inode_large inode;
> 
>    ext2fs_numeric_progress_init(fs, &progress,
>                     _("Writing inode tables: "),
> @@ -330,12 +332,32 @@ static void write_inode_tables(ext2_filsys fs, int lazy_flag, int itable_zeroed)
>            ext2fs_bg_flags_set(fs, i, EXT2_BG_INODE_ZEROED);
>            ext2fs_group_desc_csum_set(fs, i);
>        }
> -        retval = ext2fs_zero_blocks2(fs, blk, num, &blk, &num);
> -        if (retval) {
> -            fprintf(stderr, _("\nCould not write %d "
> -                  "blocks in inode table starting at %llu: %s\n"),
> -                num, blk, error_message(retval));
> -            exit(1);
> +        if (fs->super->s_creator_os == EXT2_OS_LINUX &&
> +            fs->super->s_feature_ro_compat &
> +            EXT4_FEATURE_RO_COMPAT_METADATA_CSUM) {

Somehow it doesn't look like this is skipping the zeroing of the inode table blocks if lazy itable zeroing is set. 

Any measurements on how much this slows down inode table writing (which is already the slowest part of mke2fs)?

> +            bzero(&inode, sizeof(inode));
> +            for (ino = fs->super->s_inodes_per_group * i;
> +                 ino < fs->super->s_inodes_per_group * (i + 1);
> +                 ino++) {

Why recompute "ino" each time through this loop?  It should be enough to simply initialize it at 1 and then increment it for each inode written. 

> +                if (!ino)
> +                    continue;
> +                retval = ext2fs_write_inode(fs, ino, &inode);
> +                if (retval) {
> +                    com_err("inode_init", retval,
> +                        "while writing inode %d\n",
> +                        ino);
> +                    exit(1);
> +                }
> +            }
> +        } else {
> +            retval = ext2fs_zero_blocks2(fs, blk, num, &blk, &num);
> +            if (retval) {
> +                fprintf(stderr, _("\nCould not write %d "
> +                    "blocks in inode table starting "
> +                    "at %llu: %s\n"),
> +                    num, blk, error_message(retval));
> +                exit(1);
> +            }
>        }
>        if (sync_kludge) {
>            if (sync_kludge == 1)
> @@ -829,7 +851,8 @@ static __u32 ok_features[3] = {
>        EXT4_FEATURE_RO_COMPAT_EXTRA_ISIZE|
>        EXT2_FEATURE_RO_COMPAT_SPARSE_SUPER|
>        EXT4_FEATURE_RO_COMPAT_GDT_CSUM|
> -        EXT4_FEATURE_RO_COMPAT_BIGALLOC
> +        EXT4_FEATURE_RO_COMPAT_BIGALLOC|
> +        EXT4_FEATURE_RO_COMPAT_METADATA_CSUM
> };
> 
> 
> 
--
To unsubscribe from this list: send the line "unsubscribe linux-ext4" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html