[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-Id: <1219957396.6384.25.camel@mingming-laptop>
Date: Thu, 28 Aug 2008 14:03:16 -0700
From: Mingming Cao <cmm@...ibm.com>
To: "Aneesh Kumar K.V" <aneesh.kumar@...ux.vnet.ibm.com>
Cc: tytso@....edu, sandeen@...hat.com, linux-ext4@...r.kernel.org
Subject: Re: [PATCH -V3 06/11] ext4: Update meta-data reservation with
delalloc.
在 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@...ux.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@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Powered by blists - more mailing lists