linux-ext4 - Re: [PATCH 1/2] ext4: Add support for 48 bit inode i

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: <20071012064859.GC8122@schatzie.adilger.int>
Date:	Fri, 12 Oct 2007 00:48:59 -0600
From:	Andreas Dilger <adilger@...sterfs.com>
To:	"Aneesh Kumar K.V" <aneesh.kumar@...ux.vnet.ibm.com>
Cc:	linux-ext4@...r.kernel.org
Subject: Re: [PATCH 1/2] ext4: Add support for 48 bit inode i_blocks.

On Oct 12, 2007  10:06 +0530, Aneesh Kumar K.V wrote:
> use the __le16  l_i_reserved1 field of the linux2
> struct of ext4_inode to represet the higher 16
> bits for i_blocks. With this change max_file size becomes
> (2**48 -1 )* 512 bytes.
> 
> +static int ext4_inode_blocks_set(handle_t *handle,
> +					struct ext4_inode *raw_inode,
> +					struct ext4_inode_info *ei)

CodingStyle would suggest aligning the continued lines with the '('.

> +{
> +	} else if (i_blocks <= 0xffffffffffffULL) {
> +		if (!EXT4_HAS_RO_COMPAT_FEATURE(sb,
> +					EXT4_FEATURE_RO_COMPAT_HUGE_FILE)) {
> +
> +			err = ext4_journal_get_write_access(handle,
> +					EXT4_SB(sb)->s_sbh);
> +			if (err)
> +				goto  err_out;
> +			ext4_update_dynamic_rev(sb);
> +			EXT4_SET_RO_COMPAT_FEATURE(sb,
> +					EXT4_FEATURE_RO_COMPAT_HUGE_FILE);
> +			sb->s_dirt = 1;
> +			handle->h_sync = 1;
> +			err = ext4_journal_dirty_metadata(handle,
> +					EXT4_SB(sb)->s_sbh);
> +		}

Can you please make a helper function for this, like:

    int ext4_update_feature(sb, __u32 compat, __u32 rocompat, __u32 incompat)

as we have similar code in a few places already (HTREE, LARGE_FILE, EXTENTS).
That could be done in a prerequisite patch.

> +	if (EXT4_HAS_RO_COMPAT_FEATURE(sb, EXT4_FEATURE_RO_COMPAT_HUGE_FILE)) {
> +		/*
> +		 * Large file size enabled file system can only be
> +		 * mount if kernel is build with CONFIG_LSF
> +		 */
> +		if (sizeof(root->i_blocks) < sizeof(u64) &&
> +				!(sb->s_flags & MS_RDONLY)) {
> +			printk(KERN_ERR "EXT4-fs: %s: Having huge file with "
> +					"LSF off, you must mount filesystem "
> +					"read-only.\n", sb->s_id);

What do you think about changing the wording:

"Filesystem with huge files cannot mount read-write without CONFIG_LSF."

>  #define EXT4_FEATURE_INCOMPAT_EXTENTS		0x0040 /* extents support */
>  #define EXT4_FEATURE_INCOMPAT_64BIT		0x0080
> +#define EXT4_FEATURE_INCOMPAT_MMP               0x0100
>  #define EXT4_FEATURE_INCOMPAT_FLEX_BG		0x0200

Note that it is fine to add the #define for this flag.

>  					 EXT4_FEATURE_INCOMPAT_EXTENTS| \
>  					 EXT4_FEATURE_INCOMPAT_64BIT| \
> +					 EXT4_FEATURE_INCOMPAT_MMP|\
>  					 EXT4_FEATURE_INCOMPAT_FLEX_BG)

Note it is NOT OK to add INCOMPAT_MMP to the INCOMPAT_SUPP flags, or you
defeat the entire purpose of having the feature flag.

Cheers, Andreas
--
Andreas Dilger
Principal Software Engineer
Cluster File Systems, Inc.

-
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