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: <470F1965.9090401@linux.vnet.ibm.com>
Date:	Fri, 12 Oct 2007 12:21:17 +0530
From:	"Aneesh Kumar K.V" <aneesh.kumar@...ux.vnet.ibm.com>
To:	Andreas Dilger <adilger@...sterfs.com>
CC:	linux-ext4@...r.kernel.org
Subject: Re: [PATCH 1/2] ext4: Add support for 48 bit inode i_blocks.



Andreas Dilger wrote:
> 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 '('.
> 

Will fix

>> +{
>> +	} 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.


Will do


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

Will fix.

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