Re: [PATCH -V3 06/11] ext4: Update meta-data reservation with delalloc.

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

在 2008-08-27三的 20:58 +0530，Aneesh Kumar K.V写道：
> This makes the meta-data reservation simpler. The logic
> followed is simpler. After each block allocation request
> if we have allocated some meta-data blocks subtract the
> same from the reserved meta-data blocks. If the total
> reserved data blocks after allocation is zero, free the
> remaining meta-data blocks reserved. 

With this change ext4 keeps unnecessary blocks reserved for metadata
blocks for a longer time (untilall dirty data have been flushed), I am
concerned this will leads to early ENOSPC.

The current metadata reservation logic is a little complex, but it's not
that bad. It's there to make sure we don't over-reserve the metadata.
I'd say it worth the effort. 

> During reservation
> if the total reserved blocks need more meta-data blocks
> add the extra meta-data blocks needed to the reserve_meta_blocks
> 
> Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.vnet.ibm.com>
> ---
>  fs/ext4/inode.c |   75 +++++++++++++++++++++++++++----------------------------
>  1 files changed, 37 insertions(+), 38 deletions(-)
> 
> diff --git a/fs/ext4/inode.c b/fs/ext4/inode.c
> index a45121f..3ef0822 100644
> --- a/fs/ext4/inode.c
> +++ b/fs/ext4/inode.c
> @@ -1019,31 +1019,34 @@ static int ext4_calc_metadata_amount(struct inode *inode, int blocks)
>  static void ext4_da_update_reserve_space(struct inode *inode, int used)
>  {
>  	struct ext4_sb_info *sbi = EXT4_SB(inode->i_sb);
> -	int total, mdb, mdb_free;
> 
>  	spin_lock(&EXT4_I(inode)->i_block_reservation_lock);
> -	/* recalculate the number of metablocks still need to be reserved */
> -	total = EXT4_I(inode)->i_reserved_data_blocks - used;
> -	mdb = ext4_calc_metadata_amount(inode, total);
> -
> -	/* figure out how many metablocks to release */
> -	BUG_ON(mdb > EXT4_I(inode)->i_reserved_meta_blocks);
> -	mdb_free = EXT4_I(inode)->i_reserved_meta_blocks - mdb;
> -
> -	if (mdb_free) {
> -		/* Account for allocated meta_blocks */
> -		mdb_free -= EXT4_I(inode)->i_allocated_meta_blocks;
> -
> -		/* update fs dirty blocks counter */
> -		percpu_counter_sub(&sbi->s_dirtyblocks_counter, mdb_free);
> +	if (EXT4_I(inode)->i_allocated_meta_blocks) {
> +		/* update the reseved meta- blocks */
> +		BUG_ON(EXT4_I(inode)->i_allocated_meta_blocks >
> +			EXT4_I(inode)->i_reserved_meta_blocks);
> +		EXT4_I(inode)->i_reserved_meta_blocks -=
> +			EXT4_I(inode)->i_allocated_meta_blocks;
>  		EXT4_I(inode)->i_allocated_meta_blocks = 0;
> -		EXT4_I(inode)->i_reserved_meta_blocks = mdb;
> +		/*
> +		 * We already updated the percpu dirty
> +		 * block counter in the block allocation path.
> +		 */
>  	}
> -
>  	/* update per-inode reservations */
>  	BUG_ON(used  > EXT4_I(inode)->i_reserved_data_blocks);
>  	EXT4_I(inode)->i_reserved_data_blocks -= used;
> +	if (!EXT4_I(inode)->i_reserved_data_blocks) {
> +		/*
> +		 * If we don't have any reserved data blocks
> +		 * release all the resered meta data blocks
> +		 * update percpu dirty block counter also.
> +		 */
> +		percpu_counter_sub(&sbi->s_dirtyblocks_counter,
> +				EXT4_I(inode)->i_reserved_meta_blocks);
> +		EXT4_I(inode)->i_reserved_meta_blocks = 0;
> 
> +	}
>  	spin_unlock(&EXT4_I(inode)->i_block_reservation_lock);
>  }
> 
> @@ -1524,7 +1527,7 @@ static int ext4_da_reserve_space(struct inode *inode, int nrblocks)
>  {
>  	int retries = 0;
>         struct ext4_sb_info *sbi = EXT4_SB(inode->i_sb);
> -       unsigned long md_needed, mdblocks, total = 0;
> +       unsigned long md_needed = 0, mdblocks, total = 0;
> 
>  	/*
>  	 * recalculate the amount of metadata blocks to reserve
> @@ -1535,9 +1538,9 @@ static int ext4_da_reserve_space(struct inode *inode, int nrblocks)
>  	spin_lock(&EXT4_I(inode)->i_block_reservation_lock);
>  	total = EXT4_I(inode)->i_reserved_data_blocks + nrblocks;
>  	mdblocks = ext4_calc_metadata_amount(inode, total);
> -	BUG_ON(mdblocks < EXT4_I(inode)->i_reserved_meta_blocks);
> +	if (mdblocks > EXT4_I(inode)->i_reserved_meta_blocks)
> +		md_needed = mdblocks - EXT4_I(inode)->i_reserved_meta_blocks;
> 
> -	md_needed = mdblocks - EXT4_I(inode)->i_reserved_meta_blocks;
>  	total = md_needed + nrblocks;
> 
>  	if (ext4_claim_free_blocks(sbi, total)) {
> @@ -1549,7 +1552,7 @@ static int ext4_da_reserve_space(struct inode *inode, int nrblocks)
>  		return -ENOSPC;
>  	}
>  	EXT4_I(inode)->i_reserved_data_blocks += nrblocks;
> -	EXT4_I(inode)->i_reserved_meta_blocks = mdblocks;
> +	EXT4_I(inode)->i_reserved_meta_blocks += md_needed;
> 
>  	spin_unlock(&EXT4_I(inode)->i_block_reservation_lock);
>  	return 0;       /* success */
> @@ -1558,13 +1561,12 @@ static int ext4_da_reserve_space(struct inode *inode, int nrblocks)
>  static void ext4_da_release_space(struct inode *inode, int to_free)
>  {
>  	struct ext4_sb_info *sbi = EXT4_SB(inode->i_sb);
> -	int total, mdb, mdb_free, release;
> +	int release, mdb_free = 0;
> 
>  	if (!to_free)
>  		return;		/* Nothing to release, exit */
> 
>  	spin_lock(&EXT4_I(inode)->i_block_reservation_lock);
> -
>  	if (!EXT4_I(inode)->i_reserved_data_blocks) {
>  		/*
>  		 * if there is no reserved blocks, but we try to free some
> @@ -1578,26 +1580,23 @@ static void ext4_da_release_space(struct inode *inode, int to_free)
>  		spin_unlock(&EXT4_I(inode)->i_block_reservation_lock);
>  		return;
>  	}
> +	/* update per-inode reservations */
> +	BUG_ON(to_free > EXT4_I(inode)->i_reserved_data_blocks);
> +	EXT4_I(inode)->i_reserved_data_blocks -= to_free;
> +	if (!EXT4_I(inode)->i_reserved_data_blocks) {
> +		/*
> +		 * If we don't have any reserved data blocks
> +		 * release all the resered meta data blocks
> +		 * update percpu dirty block counter also.
> +		 */
> +		mdb_free = EXT4_I(inode)->i_reserved_meta_blocks;
> +		EXT4_I(inode)->i_reserved_meta_blocks = 0;
> 
> -	/* recalculate the number of metablocks still need to be reserved */
> -	total = EXT4_I(inode)->i_reserved_data_blocks - to_free;
> -	mdb = ext4_calc_metadata_amount(inode, total);
> -
> -	/* figure out how many metablocks to release */
> -	BUG_ON(mdb > EXT4_I(inode)->i_reserved_meta_blocks);
> -	mdb_free = EXT4_I(inode)->i_reserved_meta_blocks - mdb;
> -
> +	}
>  	release = to_free + mdb_free;
> 
>  	/* update fs dirty blocks counter for truncate case */
>  	percpu_counter_sub(&sbi->s_dirtyblocks_counter, release);
> -
> -	/* update per-inode reservations */
> -	BUG_ON(to_free > EXT4_I(inode)->i_reserved_data_blocks);
> -	EXT4_I(inode)->i_reserved_data_blocks -= to_free;
> -
> -	BUG_ON(mdb > EXT4_I(inode)->i_reserved_meta_blocks);
> -	EXT4_I(inode)->i_reserved_meta_blocks = mdb;
>  	spin_unlock(&EXT4_I(inode)->i_block_reservation_lock);
>  }
> 

--
To unsubscribe from this list: send the line "unsubscribe linux-ext4" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html