Re: [PATCH] New inode allocation for FLEX_BG meta-data groups.

[Andreas Dilger <adilger@sun.com>]

On Jan 11, 2008  11:28 -0600, Jose R. Santos wrote:
> @@ -127,6 +127,8 @@ unsigned ext4_init_block_bitmap(struct super_block *sb, struct buffer_head *bh,
>  		mark_bitmap_end(group_blocks, sb->s_blocksize * 8, bh->b_data);
>  	}
>  
> +	if (sbi->s_log_groups_per_flex)
> +		return free_blocks;
>  	return free_blocks - sbi->s_itb_per_group - 2;

To be honest, I think this is a wart in ext4_init_block_bitmap() that
it returns the number of free blocks in the group.  That value should
really be set at mke2fs or e2fsck time, and if the last group is marked
BLOCK_UNINIT it gets the free blocks count wrong because it always starts
with EXT4_BLOCKS_PER_GROUP().

The above patch may also be incorrect since there may be inode tables or
bitmaps in the above group even in the case of FLEX_BG filesystems.

> +#define free_block_ratio 10
> +
> +static int find_group_flex(struct super_block *sb, struct inode *parent, ext4_group_t *best_group)
> +{
> +	n_fbg_groups = (sbi->s_groups_count + flex_size - 1) / flex_size;

Can be a shift?

I would suggest doing some kind of testing to see how well this allocation
policy is working.  We don't want to force all allocations contiguously at
the start of the filesystem, or we end up with FAT...

> +static int ext4_fill_flex_info(struct super_block *sb)
> +{
> +	sbi->s_log_groups_per_flex = sbi->s_es->s_log_groups_per_flex;

Hmm, I guess no le*_to_cpu() because this is 8 bits?

> +
> +	flex_group_count = (sbi->s_groups_count + groups_per_flex - 1) /
> +		groups_per_flex;
> +	sbi->s_flex_groups = kmalloc(flex_group_count *
> +				     sizeof(struct flex_groups), GFP_KERNEL);
> +	if (sbi->s_flex_groups == NULL) {
> +		printk(KERN_ERR "EXT4-fs: not enough memory\n");

This should report "not enough memory for N flex groups" or something.

> @@ -2105,6 +2154,13 @@ static int ext4_fill_super (struct super_block *sb,
> +	if (EXT4_HAS_INCOMPAT_FEATURE(sb, EXT4_FEATURE_INCOMPAT_FLEX_BG))
> +		if (!ext4_fill_flex_info(sb)) {
> +			printk(KERN_ERR
> +			       "EXT4-fs: unable to initialize flex_bg meta info!\n");
> +			goto failed_mount2;

Should this be considered a fatal error, or could sbi->s_log_groups_per_flex
just be set to 0 and the filesystem be used as-is (maybe with sub-optimal
allocations or something)?  Otherwise this renders the filesystem unusable.

Cheers, Andreas
--
Andreas Dilger
Sr. Staff Engineer, Lustre Group
Sun Microsystems of Canada, Inc.