Re: [RFC PATCH -v2] ext4: Retry block reservation

Mingming Cao
2008-08-25T21:06:38Z

在 2008-08-25一的 16:50 +0530，Aneesh Kumar K.V写道：
> During block reservation if we don't have enough
> blocks left, retry block reservation with smaller
> block count. This make sure we try fallocate
> and DIO with smaller request size and don't fail early.
> The delayed allocation reservation cannot try with smaller
> block count. So retry block reservation to handle temporary
> disk full conditions. Also print free blocks details if we
> fail block allocation during writepages.
>
> Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.vnet.ibm.com>
> ---
> fs/ext4/balloc.c | 8 +++++++-
> fs/ext4/inode.c | 33 ++++++++++++++++++++++++++++++---
> fs/ext4/mballoc.c | 7 ++++++-
> 3 files changed, 43 insertions(+), 5 deletions(-)
>
> diff --git a/fs/ext4/balloc.c b/fs/ext4/balloc.c
> index 4a53541..b7d1347 100644
> --- a/fs/ext4/balloc.c
> +++ b/fs/ext4/balloc.c
> @@ -1760,10 +1760,16 @@ ext4_fsblk_t ext4_old_new_blocks(handle_t *handle, struct inode *inode,
> /*
> * With delalloc we already reserved the blocks
> */
> - if (ext4_claim_free_blocks(sbi, *count)) {
> + while (*count && ext4_claim_free_blocks(sbi, *count)) {
> + /* let others to free the space */
> + yield();
> + *count = *count >> 1;
> + }
> + if (!*count) {

Could ext4_claim_free_blocks() returns the total number of blocks
shortage, so that we don't need to "guess" how much to attempt to
allocate?

> *errp = -ENOSPC;
> return 0; /*return with ENOSPC error */
> }
> + num = *count;
> }
> /*
> * Check quota for allocation of this block.
> diff --git a/fs/ext4/inode.c b/fs/ext4/inode.c
> index d965a05..72a4a71 100644
> --- a/fs/ext4/inode.c
> +++ b/fs/ext4/inode.c
> @@ -1521,6 +1521,7 @@ static int ext4_journalled_write_end(struct file *file,
>
> 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;
>
> @@ -1529,6 +1530,7 @@ static int ext4_da_reserve_space(struct inode *inode, int nrblocks)
> * in order to allocate nrblocks
> * worse case is one extent per block
> */
> +repeat:
> 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);
> @@ -1539,6 +1541,10 @@ static int ext4_da_reserve_space(struct inode *inode, int nrblocks)
>
> if (ext4_claim_free_blocks(sbi, total)) {
> spin_unlock(&EXT4_I(inode)->i_block_reservation_lock);
> + if (ext4_should_retry_alloc(inode->i_sb, &retries)) {
> + yield();
> + goto repeat;
> + }
> return -ENOSPC;
> }
> EXT4_I(inode)->i_reserved_data_blocks += nrblocks;
> @@ -1813,6 +1819,26 @@ static void ext4_da_block_invalidatepages(struct mpage_da_data *mpd,
> return;
> }
>
> +static void ext4_print_free_blocks(struct super_block *sb)
> +{
> +#ifdef EXT4FS_DEBUG
> + ext4_group_t i;
> + struct ext4_group_desc *gdp;
> + ext4_group_t ngroups = EXT4_SB(sb)->s_groups_count;
> + printk(KERN_DEBUG "Free blocks details\n");
> + for (i = 0; i < ngroups; i++) {
> + gdp = ext4_get_group_desc(sb, i, NULL);
> + if (!gdp)
> + continue;
> + printk(KERN_DEBUG "Free blocks in group %lu is %d\n",
> + i, le16_to_cpu(gdp->bg_free_blocks_count));
> + }
> +#endif
> + printk(KERN_CRIT "Total free blocks count %lld\n",
> + ext4_count_free_blocks(sb));
> + return;
> +}
> +

Ext4_count_free_blocks() has the extra debugging code too. I think you
could calling ext4_count_free_blocks() directly and don't need the new
function ext4_print_free_blocks().
> /*
> * mpage_da_map_blocks - go through given space
> *
> @@ -1825,20 +1851,18 @@ static void ext4_da_block_invalidatepages(struct mpage_da_data *mpd,
> static int mpage_da_map_blocks(struct mpage_da_data *mpd)
> {
> int err = 0;
> + struct buffer_head new;
> struct buffer_head *lbh = &mpd->lbh;
> sector_t next = lbh->b_blocknr;
> - struct buffer_head new;
>
> /*
> * We consider only non-mapped and non-allocated blocks
> */
> if (buffer_mapped(lbh) && !buffer_delay(lbh))
> return 0;
> -
> new.b_state = lbh->b_state;
> new.b_blocknr = 0;
> new.b_size = lbh->b_size;
> -
> /*
> * If we didn't accumulate anything
> * to write simply return
> @@ -1871,6 +1895,9 @@ static int mpage_da_map_blocks(struct mpage_da_data *mpd)
> lbh->b_size >> mpd->inode->i_blkbits, err);
> printk(KERN_EMERG "This should not happen.!! "
> "Data will be lost\n");

could we print out the error code with the warning message here?

> + if (err == -ENOSPC) {
> + ext4_print_free_blocks(mpd->inode->i_sb);
> + }
> /* invlaidate all the pages */
> ext4_da_block_invalidatepages(mpd, next,
> lbh->b_size >> mpd->inode->i_blkbits);
> diff --git a/fs/ext4/mballoc.c b/fs/ext4/mballoc.c
> index 4404b46..419009f 100644
> --- a/fs/ext4/mballoc.c
> +++ b/fs/ext4/mballoc.c
> @@ -4397,7 +4397,12 @@ ext4_fsblk_t ext4_mb_new_blocks(handle_t *handle,
> /*
> * With delalloc we already reserved the blocks
> */
> - if (ext4_claim_free_blocks(sbi, ar->len)) {
> + while (ar->len && ext4_claim_free_blocks(sbi, ar->len)) {
> + /* let others to free the space */
> + yield();
> + ar->len = ar->len >> 1;
> + }
> + if (!ar->len) {
> *errp = -ENOSPC;
> return 0;
> }

--
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