linux-ext4 - Re: [PATCH 2/2] ext4: Support large files

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] [day] [month] [year] [list]

Date:	Fri, 12 Oct 2007 12:42:41 +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 2/2] ext4: Support large files



Andreas Dilger wrote:
> On Oct 12, 2007  10:06 +0530, Aneesh Kumar K.V wrote:
>> We add a RO_COMPAT feature to the super
>> block to indicate that some of the inode have i_blocks
>> represented as file system block size units. Super block
>> with this feature set cannot be mounted read write on a kernel
>> with CONFIG_LSF disabled.
>>
>> Super block flag EXT4_FEATURE_RO_COMPAT_HUGE_FILE
>> inode flag  EXT4_HUGE_FILE_FL
> 
> I was wondering where this part of the patch went...


I fogot to update the commit message properly. right now it reads as below 

ext4: Support large files
    
    This patch converts ext4_inode i_blocks to represent total
    blocks occupied by the inode in file system block size.
    Earlier the variable used to represent this in 512 byte
    block size. This actually limited the total size of the file.
    
    The feature is enabled transparently when we write an inode
    whose i_blocks cannot be represnted as 512 byte units in a
    48 bit variable.
    
    inode flag  EXT4_HUGE_FILE_FL



EXT4_FEATURE_RO_COMPAT_HUGE_FILE is set when we start using the higher order 16 bit
it is done in the previous patch. To enable that we need the CONFIG_LSF support.
With CONFIG_LSF support enabled and if the inode have EXT4_HUGE_FILE_FL set that means
i_blocks is represented in terms of file system block size.

> 
>> @@ -2905,10 +2912,32 @@ static int ext4_inode_blocks_set(handle_t *handle,
>>  		/* i_block is stored in the split  48 bit fields */
>>  		raw_inode->i_blocks_lo   = cpu_to_le32((u32)i_blocks);
>>  		raw_inode->i_blocks_high = cpu_to_le16(i_blocks >> 32);
> 
> I don't think we need to cast (u32) here, since cpu_to_le32() should do
> that already?
> 

yes. will remove the same

>> +	} else {
>> +		/*
>> +		 * i_blocks should be represented in a 48 bit variable
>> +		 * as multiple of  file system block size
>> +		 */
>> +		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);
>> +		}
>> +		ei->i_flags |= EXT4_HUGE_FILE_FL;
>> +		/* i_block is stored in file system block size */
>> +		i_blocks = i_blocks >> (inode->i_blkbits - 9);
>> +		raw_inode->i_blocks_lo   = cpu_to_le32((u32)i_blocks);
>> +		raw_inode->i_blocks_high = cpu_to_le16(i_blocks >> 32);
>>  	}
> 
> This "else" clause is a LOT like the previous case, maybe they can be
> merges?  Having the feature helper I suggested will reduce that a lot,
> but it still seems like most of it is the same except for the shift.
> 

Yes. ext4_update_feature will simplify the above.


Thanks a lot for all your reviews.

-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