Re: [PATCH] ext4: Fix the oldallocator block reservation accounting with delalloc.

[Mingming Cao <cmm@us.ibm.com>]

On Thu, 2008-06-12 at 22:39 +0530, Aneesh Kumar K.V wrote:
> Update old block allocator to not decrement s_freeblocks_counter
> for allocated blocks with delalloc. Also account for the
> actual meta-data allocated. We should not check for freespace
> again in allocator with delalloc since we already did block
> reservation.
> 
> Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.vnet.ibm.com>
> ---
>  fs/ext4/balloc.c |   16 ++++++++++++----
>  1 files changed, 12 insertions(+), 4 deletions(-)
> 
> diff --git a/fs/ext4/balloc.c b/fs/ext4/balloc.c
> index 4331577..eca8c0e 100644
> --- a/fs/ext4/balloc.c
> +++ b/fs/ext4/balloc.c
> @@ -1706,7 +1706,12 @@ ext4_fsblk_t ext4_old_new_blocks(handle_t *handle, struct inode *inode,
>  	}
> 
>  	sbi = EXT4_SB(sb);
> -	*count = ext4_has_free_blocks(sbi, *count);
> +	if (!EXT4_I(inode)->i_delalloc_reserved_flag) {
> +		/*
> +		 * With delalloc we already reserved the blocks
> +		 */
> +		*count = ext4_has_free_blocks(sbi, *count);
> +	}
>  	if (*count == 0) {
>  		*errp = -ENOSPC;
>  		return 0;	/*return with ENOSPC error */
> @@ -1907,7 +1912,8 @@ ext4_fsblk_t ext4_old_new_blocks(handle_t *handle, struct inode *inode,
>  	le16_add_cpu(&gdp->bg_free_blocks_count, -num);
>  	gdp->bg_checksum = ext4_group_desc_csum(sbi, group_no, gdp);
>  	spin_unlock(sb_bgl_lock(sbi, group_no));
> -	percpu_counter_sub(&sbi->s_freeblocks_counter, num);
> +	if (!EXT4_I(inode)->i_delalloc_reserved_flag)
> +		percpu_counter_sub(&sbi->s_freeblocks_counter, num);
> 
>  	if (sbi->s_log_groups_per_flex) {
>  		ext4_group_t flex_group = ext4_flex_group(sbi, group_no);
> @@ -1957,7 +1963,8 @@ static ext4_fsblk_t do_blk_alloc(handle_t *handle, struct inode *inode,
>  	ext4_fsblk_t ret;
> 
>  	if (!test_opt(inode->i_sb, MBALLOC)) {
> -		return ext4_old_new_blocks(handle, inode, goal, count, errp);
> +		ret = ext4_old_new_blocks(handle, inode, goal, count, errp);
> +		goto out;
>  	}
> 
>  	memset(&ar, 0, sizeof(ar));
> @@ -1990,12 +1997,13 @@ static ext4_fsblk_t do_blk_alloc(handle_t *handle, struct inode *inode,
> 
>  	ret = ext4_mb_new_blocks(handle, &ar, errp);
>  	*count = ar.len;
> +out:
>  	/*
>  	 * Account for the allocated meta blocks
>  	 */
>  	if (!(*errp) && (flags & EXT4_META_BLOCK)) {
>  		spin_lock(&EXT4_I(inode)->i_block_reservation_lock);
> -		EXT4_I(inode)->i_allocated_meta_blocks += ar.len;
> +		EXT4_I(inode)->i_allocated_meta_blocks += *count;
>  		spin_unlock(&EXT4_I(inode)->i_block_reservation_lock);
>  	}
>  	return ret;

The changes looks correct, but I think it would be cleaner to put the
changes about metadata allocation related accounting for delayed
allocation  to ext4_new_meta_blocks() , so it doesn't need to check the
EXT4_META_BLOCK flags in common code do_blk_alloc all the time.

Mingming