On Tue, Aug 12, 2008 at 09:23:10AM -0700, Mingming Cao wrote:
> This is a rework of journal credits fix patch in the ext4 patch queue.
> The patch series contains
>
> - patch 1: helper funtions for journal credits calculation and fix the
> writepage/write_begin on nonextent files
> - patch 2: journal credit fix wirtepahe/write_begin for extents files,
> and migration
> - patch 3: credit fix for dio, fallocate
> -patch 4: rebase ext4_da_writepages_rework patch
> - patch 5: credit fix for delalloc writepages
> -patch 6: credit fix for defrag
>
>
I see this patch is pushed to the patch queue. Below is the review in
patch form.
commit 679a6fa1de0bc67c0a444b748696a2f2c22428c7
Author: Aneesh Kumar K.V <aneesh.kumar@xxxxxxxxxxxxxxxxxx>
Date: Fri Aug 15 22:13:07 2008 +0530
cleanup
diff --git a/fs/ext4/ext4.h b/fs/ext4/ext4.h
index 258cd1a..164c988 100644
--- a/fs/ext4/ext4.h
+++ b/fs/ext4/ext4.h
@@ -1150,7 +1150,8 @@ extern void ext4_set_inode_flags(struct inode *);
extern void ext4_get_inode_flags(struct ext4_inode_info *);
extern void ext4_set_aops(struct inode *inode);
extern int ext4_writepage_trans_blocks(struct inode *);
-extern int ext4_meta_trans_blocks(struct inode *, int nrblocks, int idxblocks);
+extern int ext4_meta_trans_blocks(struct inode *, int nrblocks,
+ int idxblocks, bool chunk);
extern int ext4_data_trans_blocks(struct inode *, int nrblocks);
extern int ext4_block_truncate_page(handle_t *handle,
struct address_space *mapping, loff_t from);
@@ -1316,7 +1317,7 @@ extern const struct inode_operations ext4_fast_symlink_inode_operations;
/* extents.c */
extern int ext4_ext_tree_init(handle_t *handle, struct inode *);
-extern int ext4_ext_writepage_trans_blocks(struct inode *, int num, int chunk);
+extern int ext4_ext_writeblock_trans_credits(struct inode *, int num, bool chunk);
extern int ext4_ext_get_blocks(handle_t *handle, struct inode *inode,
ext4_lblk_t iblock,
unsigned long max_blocks, struct buffer_head *bh_result,
diff --git a/fs/ext4/extents.c b/fs/ext4/extents.c
index 2fbca0f..b38cc56 100644
--- a/fs/ext4/extents.c
+++ b/fs/ext4/extents.c
@@ -1893,7 +1893,7 @@ static int ext4_ext_rm_idx(handle_t *handle, struct inode *inode,
* When pass the actual path, the caller should calculate credits
* under i_data_sem.
*/
-int ext4_ext_calc_credits_for_single_extent(struct inode *inode, int num,
+int ext4_ext_calc_credits_for_single_extent(struct inode *inode, int nrblocks,
struct ext4_ext_path *path)
{
int depth = ext_depth(inode);
@@ -1905,7 +1905,7 @@ int ext4_ext_calc_credits_for_single_extent(struct inode *inode, int num,
return 2 + EXT4_META_TRANS_BLOCKS(inode->i_sb);
}
- return ext4_ext_writepage_trans_blocks(inode, num, 1);
+ return ext4_ext_writeblock_trans_credits(inode, nrblocks, 1);
}
/*
@@ -1919,7 +1919,7 @@ int ext4_ext_calc_credits_for_single_extent(struct inode *inode, int num,
* If the nrblocks are discontigous, they could cause
* the whole tree split more than once, but this is really rare.
*/
-static int ext4_ext_index_trans_blocks(struct inode *inode, int num, int chunk)
+static int ext4_ext_index_trans_blocks(struct inode *inode, int num, bool chunk)
{
int index;
int depth = ext_depth(inode);
@@ -2993,7 +2993,7 @@ void ext4_ext_truncate(struct inode *inode)
}
/*
- * ext4_ext_writepage_trans_blocks:
+ * ext4_ext_writeblock_trans_credits:
* calculate max number of blocks we could modify
* in order to allocate nrblocks of blocks.
*
@@ -3005,8 +3005,11 @@ void ext4_ext_truncate(struct inode *inode)
* see how many bitmapblocks and block group descriptor groups need to accounted
* At last adds up the superblock, inode, quotao and xattr blocks. These
* all take care of in ext4_meta_trans_blocks()
+ *
+ * This doesn't account for the blocks needed for journaled
+ * data blocks.
*/
-int ext4_ext_writepage_trans_blocks(struct inode *inode, int num, int chunk)
+int ext4_ext_writeblock_trans_credits(struct inode *inode, int nrblocks, bool chunk)
{
int needed;
int index_blocks;
@@ -3016,17 +3019,14 @@ int ext4_ext_writepage_trans_blocks(struct inode *inode, int num, int chunk)
* insert a single extent with num blocks(chunk == 1)
* or @num extents (chunk ==0)
*/
- index_blocks = ext4_ext_index_trans_blocks(inode, num, chunk);
+ index_blocks = ext4_ext_index_trans_blocks(inode, nrblocks, chunk);
/* How many metadat blocks need to modify to modify the @num
* of data blocks and index_blocks? Include, index/leaf blocks,
* bitmaps,block group descriptor block for modifying both data
* and index/leaf blocks, superblock, inode, quota and xattrs
*/
- needed = ext4_meta_trans_blocks(inode, num, index_blocks);
-
- if (ext4_should_journal_data(inode))
- needed += num;
+ needed = ext4_meta_trans_blocks(inode, nrblocks, index_blocks, chunk);
return needed;
}
diff --git a/fs/ext4/inode.c b/fs/ext4/inode.c
index 0b34998..a843cd3 100644
--- a/fs/ext4/inode.c
+++ b/fs/ext4/inode.c
@@ -1568,9 +1568,9 @@ static void ext4_da_release_space(struct inode *inode, int to_free)
* but since this function is called from invalidate
* page, it's harmless to return without any action
*/
- printk(KERN_INFO "ext4 delalloc try to release %d reserved"
- "blocks for inode %lu, but there is no reserved"
- "data blocks\n", inode->i_ino, to_free);
+ printk(KERN_INFO "ext4 delalloc try to release %d reserved "
+ "blocks for inode %lu, but there is no reserved "
+ "data blocks\n", to_free, inode->i_ino);
spin_unlock(&EXT4_I(inode)->i_block_reservation_lock);
return;
}
@@ -2303,13 +2303,13 @@ static int ext4_da_writepage(struct page *page,
* get_block is calculated
*/
-#define EXT4_MAX_WRITEPAGES_SIZE PAGEVEC_SIZE
-static int ext4_writepages_trans_blocks(struct inode *inode)
+static int ext4_da_writepages_trans_blocks(struct inode *inode)
{
- int bpp = ext4_journal_blocks_per_page(inode);
- int max_blocks = EXT4_MAX_WRITEPAGES_SIZE * bpp;
-
- if (max_blocks > EXT4_I(inode)->i_reserved_data_blocks)
+ int max_blocks;
+ if (EXT4_I(inode)->i_reserved_data_blocks >
+ EXT4_BLOCKS_PER_GROUP(inode->i_sb))
+ max_blocks = EXT4_BLOCKS_PER_GROUP(inode->i_sb);
+ else
max_blocks = EXT4_I(inode)->i_reserved_data_blocks;
return ext4_data_trans_blocks(inode, max_blocks);
@@ -2364,7 +2364,7 @@ static int ext4_da_writepages(struct address_space *mapping,
* by delalloc
*/
BUG_ON(ext4_should_journal_data(inode));
- needed_blocks = ext4_writepages_trans_blocks(inode);
+ needed_blocks = ext4_da_writepages_trans_blocks(inode);
/* start a new transaction*/
handle = ext4_journal_start(inode, needed_blocks);
@@ -4459,13 +4459,19 @@ int ext4_getattr(struct vfsmount *mnt, struct dentry *dentry,
* over different block groups
*
* Also account for superblock, inode, quota and xattr blocks
+ * This doesn't account for the blocks needed for journaled
+ * data blocks.
*/
-int ext4_meta_trans_blocks(struct inode* inode, int nrblocks, int idxblocks)
+int ext4_meta_trans_blocks(struct inode* inode,
+ int nrblocks, int idxblocks, bool chunk)
{
int groups, gdpblocks;
int ret = 0;
- groups = nrblocks + idxblocks;
+ if (chunk)
+ groups = 1 + idxblocks;
+ else
+ groups = nrblocks + idxblocks;
gdpblocks = groups;
if (groups > EXT4_SB(inode->i_sb)->s_groups_count)
groups = EXT4_SB(inode->i_sb)->s_groups_count;
@@ -4473,14 +4479,10 @@ int ext4_meta_trans_blocks(struct inode* inode, int nrblocks, int idxblocks)
gdpblocks = EXT4_SB(inode->i_sb)->s_gdb_count;
/* bitmaps and block group descriptor blocks */
- ret += groups + gdpblocks;
+ ret = groups + gdpblocks;
ret += idxblocks;
- /* journalled mode, include buffer to modify data blocks */
- if (ext4_should_journal_data(inode))
- ret += nrblocks;
-
/* Blocks for super block, inode, quota and xattr blocks */
ret += EXT4_META_TRANS_BLOCKS(inode->i_sb);
@@ -4488,14 +4490,14 @@ int ext4_meta_trans_blocks(struct inode* inode, int nrblocks, int idxblocks)
}
static int ext4_indirect_trans_blocks(struct inode *inode, int nrblocks,
- int chunk)
+ bool chunk)
{
int indirects;
- /* if nrblocks are contigous */
+ /* if nrblocks are contiguous */
if (chunk) {
/*
- * With N contigous data blocks, it need at most
+ * With N contiguous data blocks, it need at most
* N/EXT4_ADDR_PER_BLOCK(inode->i_sb) indirect blocks
* 2 dindirect blocks
* 1 tindirect block
@@ -4504,7 +4506,7 @@ static int ext4_indirect_trans_blocks(struct inode *inode, int nrblocks,
return indirects + 3;
}
/*
- * if nrblocks are not contigous, worse case, each block touch
+ * if nrblocks are not contiguous, worse case, each block touch
* a indirect block, and each indirect block touch a double indirect
* block, plus a triple indirect block
*/
@@ -4512,18 +4514,21 @@ static int ext4_indirect_trans_blocks(struct inode *inode, int nrblocks,
return indirects;
}
/*
- * How many journal blocks are need to modify N blocks contigous data()?
+ * How many journal blocks are need to modify N blocks contiguous data()?
*
* It need to account indirect blocks, data blocks, and
* bitmap blocks and group descriptor blocks.
*
* This still overestimates under most circumstances. If we were to pass the
* start and end offsets in here as well we could do block_to_path() on each
- * block and work out the exact number of indirects which are touched. Pah.
+ * block and work out the exact number of indirects which are touched.
+ *
+ * This doesn't account for the blocks needed for journaled
+ * data blocks.
*/
static int ext4_writeblocks_trans_credits_old(struct inode *inode, int nrblocks,
- int chunk)
+ bool chunk)
{
int indirects;
int ret;
@@ -4531,7 +4536,7 @@ static int ext4_writeblocks_trans_credits_old(struct inode *inode, int nrblocks,
/*
* How many index blocks need to touch to modify nrblocks?
* The "Chunk" flag indicating whether the nrblocks is
- * physically contigous on disk
+ * physically contiguous on disk
*
* For Direct IO and fallocate, they calls get_block to allocate
* one single extent at a time, so they could set the "Chunk" flag
@@ -4542,7 +4547,7 @@ static int ext4_writeblocks_trans_credits_old(struct inode *inode, int nrblocks,
* descriptors.Worse case, the nrblocks+indirects blocks spread
* over different block groups
*/
- ret = ext4_meta_trans_blocks(inode, nrblocks, indirects);
+ ret = ext4_meta_trans_blocks(inode, nrblocks, indirects, chunk);
return ret;
}
@@ -4559,11 +4564,17 @@ static int ext4_writeblocks_trans_credits_old(struct inode *inode, int nrblocks,
*/
int ext4_writepage_trans_blocks(struct inode *inode)
{
+ int ret;
int bpp = ext4_journal_blocks_per_page(inode);
if (!(EXT4_I(inode)->i_flags & EXT4_EXTENTS_FL))
- return ext4_writeblocks_trans_credits_old(inode, bpp, 0);
- return ext4_ext_writepage_trans_blocks(inode, bpp, 0);
+ ret = ext4_writeblocks_trans_credits_old(inode, bpp, 0);
+ ret = ext4_ext_writeblock_trans_credits(inode, bpp, 0);
+
+ /* journalled mode, include buffer to modify data blocks */
+ if (ext4_should_journal_data(inode))
+ ret += bpp;
+ return ret;
}
/*
@@ -4575,13 +4586,16 @@ int ext4_writepage_trans_blocks(struct inode *inode)
* chunk of allocation needs.
*
* This is called from DIO, fallocate or whoever calling
- * ext4_get_blocks_wrap() to map/allocate a chunk of contigous disk blocks
+ * ext4_get_blocks_wrap() to map/allocate a chunk of contiguous disk blocks
+ *
+ * This doesn't account for the blocks needed for journaled
+ * data blocks.
*/
int ext4_data_trans_blocks(struct inode *inode, int nrblocks)
{
if (!(EXT4_I(inode)->i_flags & EXT4_EXTENTS_FL))
return ext4_writeblocks_trans_credits_old(inode, nrblocks, 1);
- return ext4_ext_writepage_trans_blocks(inode, nrblocks, 1);
+ return ext4_ext_writeblock_trans_credits(inode, nrblocks, 1);
}
/*
--
To unsubscribe from this list: send the line "unsubscribe linux-ext4" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
