| 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
| ||
|
Message-ID: <4E035410.1050702@redhat.com>
Date: Thu, 23 Jun 2011 09:56:16 -0500
From: Eric Sandeen <sandeen@...hat.com>
To: Andreas Dilger <adilger@...ger.ca>
CC: ext4 development <linux-ext4@...r.kernel.org>
Subject: Re: [PATCH] ext4: refactor duplicated block placement code
On 6/22/11 10:16 PM, Andreas Dilger wrote:
> On 2011-06-22, at 12:37 PM, Eric Sandeen wrote:
>> I found that ext4_ext_find_goal() and ext4_find_near()
>> share the same code for returning a coloured start block
>> based on i_block_group.
>>
>> We can refactor this into a common function so that they
>> don't diverge in the future.
>>
>> Signed-off-by: Eric Sandeen <sandeen@...hat.com>
>> ---
>>
>> ext4.h | 1
>> extents.c | 37 ---------------------------------
>> inode.c | 69 +++++++++++++++++++++++++++++++++++++-------------------------
>> 3 files changed, 44 insertions(+), 63 deletions(-)
>>
>> I don't know that I like the new function name too much,
>> I'm open to suggestions...
>
> What about ext4_goal_block_from_inode() or ext4_start_block_from_inode() or
> ext4_inode_to_{goal,start}_block()? One of last ones might be good since
> it makes it clear this function is related to other ext4_inode_*() functions.
ext4_inode_to_goal_block sounds good to me, I'll resend, thanks.
-Eric
>> diff --git a/fs/ext4/ext4.h b/fs/ext4/ext4.h
>> index 1921392..1d79fbc 100644
>> --- a/fs/ext4/ext4.h
>> +++ b/fs/ext4/ext4.h
>> @@ -1812,6 +1812,7 @@ extern int ext4_getattr(struct vfsmount *mnt, struct dentry *dentry,
>> struct kstat *stat);
>> extern void ext4_evict_inode(struct inode *);
>> extern void ext4_clear_inode(struct inode *);
>> +ext4_fsblk_t ext4_inode_group_start(struct inode *);
>> extern int ext4_sync_inode(handle_t *, struct inode *);
>> extern void ext4_dirty_inode(struct inode *, int);
>> extern int ext4_change_inode_journal_flag(struct inode *, int);
>> diff --git a/fs/ext4/extents.c b/fs/ext4/extents.c
>> index 5199bac..23cf7dd 100644
>> --- a/fs/ext4/extents.c
>> +++ b/fs/ext4/extents.c
>> @@ -114,12 +114,6 @@ static ext4_fsblk_t ext4_ext_find_goal(struct inode *inode,
>> struct ext4_ext_path *path,
>> ext4_lblk_t block)
>> {
>> - struct ext4_inode_info *ei = EXT4_I(inode);
>> - ext4_fsblk_t bg_start;
>> - ext4_fsblk_t last_block;
>> - ext4_grpblk_t colour;
>> - ext4_group_t block_group;
>> - int flex_size = ext4_flex_bg_size(EXT4_SB(inode->i_sb));
>> int depth;
>>
>> if (path) {
>> @@ -161,36 +155,7 @@ static ext4_fsblk_t ext4_ext_find_goal(struct inode *inode,
>> }
>>
>> /* OK. use inode's group */
>> - block_group = ei->i_block_group;
>> - if (flex_size >= EXT4_FLEX_SIZE_DIR_ALLOC_SCHEME) {
>> - /*
>> - * If there are at least EXT4_FLEX_SIZE_DIR_ALLOC_SCHEME
>> - * block groups per flexgroup, reserve the first block
>> - * group for directories and special files. Regular
>> - * files will start at the second block group. This
>> - * tends to speed up directory access and improves
>> - * fsck times.
>> - */
>> - block_group &= ~(flex_size-1);
>> - if (S_ISREG(inode->i_mode))
>> - block_group++;
>> - }
>> - bg_start = ext4_group_first_block_no(inode->i_sb, block_group);
>> - last_block = ext4_blocks_count(EXT4_SB(inode->i_sb)->s_es) - 1;
>> -
>> - /*
>> - * If we are doing delayed allocation, we don't need take
>> - * colour into account.
>> - */
>> - if (test_opt(inode->i_sb, DELALLOC))
>> - return bg_start;
>> -
>> - if (bg_start + EXT4_BLOCKS_PER_GROUP(inode->i_sb) <= last_block)
>> - colour = (current->pid % 16) *
>> - (EXT4_BLOCKS_PER_GROUP(inode->i_sb) / 16);
>> - else
>> - colour = (current->pid % 16) * ((last_block - bg_start) / 16);
>> - return bg_start + colour + block;
>> + return ext4_inode_group_start(inode);
>> }
>>
>> /*
>> diff --git a/fs/ext4/inode.c b/fs/ext4/inode.c
>> index a5763e3..5784809 100644
>> --- a/fs/ext4/inode.c
>> +++ b/fs/ext4/inode.c
>> @@ -466,6 +466,47 @@ no_block:
>> return p;
>> }
>>
>> +ext4_fsblk_t ext4_inode_group_start(struct inode *inode)
>> +{
>> + struct ext4_inode_info *ei = EXT4_I(inode);
>> + ext4_group_t block_group;
>> + ext4_grpblk_t colour;
>> + int flex_size = ext4_flex_bg_size(EXT4_SB(inode->i_sb));
>> + ext4_fsblk_t bg_start;
>> + ext4_fsblk_t last_block;
>> +
>> + block_group = ei->i_block_group;
>> + if (flex_size >= EXT4_FLEX_SIZE_DIR_ALLOC_SCHEME) {
>> + /*
>> + * If there are at least EXT4_FLEX_SIZE_DIR_ALLOC_SCHEME
>> + * block groups per flexgroup, reserve the first block
>> + * group for directories and special files. Regular
>> + * files will start at the second block group. This
>> + * tends to speed up directory access and improves
>> + * fsck times.
>> + */
>> + block_group &= ~(flex_size-1);
>> + if (S_ISREG(inode->i_mode))
>> + block_group++;
>> + }
>> + bg_start = ext4_group_first_block_no(inode->i_sb, block_group);
>> + last_block = ext4_blocks_count(EXT4_SB(inode->i_sb)->s_es) - 1;
>> +
>> + /*
>> + * If we are doing delayed allocation, we don't need take
>> + * colour into account.
>> + */
>> + if (test_opt(inode->i_sb, DELALLOC))
>> + return bg_start;
>> +
>> + if (bg_start + EXT4_BLOCKS_PER_GROUP(inode->i_sb) <= last_block)
>> + colour = (current->pid % 16) *
>> + (EXT4_BLOCKS_PER_GROUP(inode->i_sb) / 16);
>> + else
>> + colour = (current->pid % 16) * ((last_block - bg_start) / 16);
>> + return bg_start + colour;
>> +}
>> +
>> /**
>> * ext4_find_near - find a place for allocation with sufficient locality
>> * @inode: owner
>> @@ -491,11 +532,6 @@ static ext4_fsblk_t ext4_find_near(struct inode *inode, Indirect *ind)
>> struct ext4_inode_info *ei = EXT4_I(inode);
>> __le32 *start = ind->bh ? (__le32 *) ind->bh->b_data : ei->i_data;
>> __le32 *p;
>> - ext4_fsblk_t bg_start;
>> - ext4_fsblk_t last_block;
>> - ext4_grpblk_t colour;
>> - ext4_group_t block_group;
>> - int flex_size = ext4_flex_bg_size(EXT4_SB(inode->i_sb));
>>
>> /* Try to find previous block */
>> for (p = ind->p - 1; p >= start; p--) {
>> @@ -511,28 +547,7 @@ static ext4_fsblk_t ext4_find_near(struct inode *inode, Indirect *ind)
>> * It is going to be referred to from the inode itself? OK, just put it
>> * into the same cylinder group then.
>> */
>> - block_group = ei->i_block_group;
>> - if (flex_size >= EXT4_FLEX_SIZE_DIR_ALLOC_SCHEME) {
>> - block_group &= ~(flex_size-1);
>> - if (S_ISREG(inode->i_mode))
>> - block_group++;
>> - }
>> - bg_start = ext4_group_first_block_no(inode->i_sb, block_group);
>> - last_block = ext4_blocks_count(EXT4_SB(inode->i_sb)->s_es) - 1;
>> -
>> - /*
>> - * If we are doing delayed allocation, we don't need take
>> - * colour into account.
>> - */
>> - if (test_opt(inode->i_sb, DELALLOC))
>> - return bg_start;
>> -
>> - if (bg_start + EXT4_BLOCKS_PER_GROUP(inode->i_sb) <= last_block)
>> - colour = (current->pid % 16) *
>> - (EXT4_BLOCKS_PER_GROUP(inode->i_sb) / 16);
>> - else
>> - colour = (current->pid % 16) * ((last_block - bg_start) / 16);
>> - return bg_start + colour;
>> + return ext4_inode_group_start(inode);
>> }
>>
>> /**
>> --
>> 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
>
>
> Cheers, Andreas
>
>
>
>
>
--
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
Powered by blists - more mailing lists