[PATCH] ext4: limit nrblocks properly with non extent format

[PATCH] ext4: limit nrblocks properly with non extent format

[Aneesh Kumar K.V]

We need to make sure adding new buffer_head to the
contiguous block extent doesn't result in nrblocks
greater than EXT4_MAX_TRANS_DATA. We only have
enought credit to insert EXT4_MAX_TRANS_DATA blocks.

Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.vnet.ibm.com>
---
 fs/ext4/inode.c |   38 +++++++++++++++++++++++++-------------
 1 files changed, 25 insertions(+), 13 deletions(-)

diff --git a/fs/ext4/inode.c b/fs/ext4/inode.c
index dcd1337..a10f8e5 100644
--- a/fs/ext4/inode.c
+++ b/fs/ext4/inode.c
@@ -1909,27 +1909,39 @@ static int  mpage_da_map_blocks(struct mpage_da_data *mpd)
 static void mpage_add_bh_to_extent(struct mpage_da_data *mpd,
 				   sector_t logical, struct buffer_head *bh)
 {
-	struct buffer_head *lbh = &mpd->lbh;
 	sector_t next;
+	size_t b_size = bh->b_size;
+	struct buffer_head *lbh = &mpd->lbh;
 	int nrblocks = lbh->b_size >> mpd->inode->i_blkbits;
 
 	/* check if thereserved journal credits might overflow */
-	if (!(EXT4_I(mpd->inode)->i_flags & EXT4_EXTENTS_FL) &&
-	     (nrblocks >= EXT4_MAX_TRANS_DATA))
-		/*
-		 * With noextent format we are limited by the journal
-		 * credit available. Total credit needed to insert
-		 * nrblocks contiguous blocks is dependent on the
-		 * nrblocks. So limit nrblocks.
-		 */
-		goto flush_it;
-
+	if (!(EXT4_I(mpd->inode)->i_flags & EXT4_EXTENTS_FL)) {
+		if (nrblocks >= EXT4_MAX_TRANS_DATA) {
+			/*
+			 * With noextent format we are limited by the journal
+			 * credit available. Total credit needed to insert
+			 * nrblocks contiguous blocks is dependent on the
+			 * nrblocks. So limit nrblocks.
+			 */
+			goto flush_it;
+		} else if ((nrblocks + (b_size >> mpd->inode->i_blkbits)) >
+				EXT4_MAX_TRANS_DATA) {
+			/*
+			 * Adding the new buffer_head would make it cross the
+			 * allowed limit for which we have journal credit
+			 * reserved. So limit the new bh->b_size
+			 */
+			b_size = (EXT4_MAX_TRANS_DATA - nrblocks) <<
+						mpd->inode->i_blkbits;
+			/* we will do mpage_da_submit_io in the next loop */
+		}
+	}
 	/*
 	 * First block in the extent
 	 */
 	if (lbh->b_size == 0) {
 		lbh->b_blocknr = logical;
-		lbh->b_size = bh->b_size;
+		lbh->b_size = b_size;
 		lbh->b_state = bh->b_state & BH_FLAGS;
 		return;
 	}
@@ -1939,7 +1951,7 @@ static void mpage_add_bh_to_extent(struct mpage_da_data *mpd,
 	 * Can we merge the block to our big extent?
 	 */
 	if (logical == next && (bh->b_state & BH_FLAGS) == lbh->b_state) {
-		lbh->b_size += bh->b_size;
+		lbh->b_size += b_size;
 		return;
 	}
 
-- 
1.6.0.2.g2ebc0

[PATCH] ext4: Code cleanups

[Aneesh Kumar K.V]

a) rename num to nrblocks to clearly indicate we expecting block count as argument.
b) renmae ext4_writepages_trans_blocks to ext4_da_writepages_trans_blocks to
indicate it is the delayed allocation writepages transaction credit.
c) Add some comments. remove some stale comments.

Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.vnet.ibm.com>
---
 fs/ext4/extents.c |    8 ++++----
 fs/ext4/inode.c   |   14 ++++++++------
 2 files changed, 12 insertions(+), 10 deletions(-)

diff --git a/fs/ext4/extents.c b/fs/ext4/extents.c
index 73bb308..bf612a7 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)
 {
 	if (path) {
@@ -1912,12 +1912,12 @@ int ext4_ext_calc_credits_for_single_extent(struct inode *inode, int num,
 			 *  and other metadat blocks still need to be
 			 *  accounted.
 			 */
-			/* 1 one bitmap, 1 block group descriptor */
+			/* 1 bitmap, 1 block group descriptor */
 			ret = 2 + EXT4_META_TRANS_BLOCKS(inode->i_sb);
 		}
 	}
 
-	return ext4_chunk_trans_blocks(inode, num);
+	return ext4_chunk_trans_blocks(inode, nrblocks);
 }
 
 /*
@@ -1931,7 +1931,7 @@ int ext4_ext_calc_credits_for_single_extent(struct inode *inode, int num,
  * If the nrblocks are discontiguous, they could cause
  * the whole tree split more than once, but this is really rare.
  */
-int ext4_ext_index_trans_blocks(struct inode *inode, int num, int chunk)
+int ext4_ext_index_trans_blocks(struct inode *inode, int nrblocks, int chunk)
 {
 	int index;
 	int depth = ext_depth(inode);
diff --git a/fs/ext4/inode.c b/fs/ext4/inode.c
index a10f8e5..b32a723 100644
--- a/fs/ext4/inode.c
+++ b/fs/ext4/inode.c
@@ -2321,16 +2321,18 @@ static int ext4_da_writepage(struct page *page,
  * a single extent allocation into a single transaction,
  * ext4_da_writpeages() will loop calling this before
  * the block allocation.
- *
- * The page vector size limited the max number of pages could
- * be writeout at a time. Based on this, the max blocks to pass to
- * get_block is calculated
  */
 
-static int ext4_writepages_trans_blocks(struct inode *inode)
+static int ext4_da_writepages_trans_blocks(struct inode *inode)
 {
 	int max_blocks = EXT4_I(inode)->i_reserved_data_blocks;
 
+	/*
+	 * With noextent format the journal credit needed to
+	 * insert nrblocks contiguous block is dependent on
+	 * number of contiguous block. So we will limit
+	 * number of contiguous block to a sane value
+	 */
 	if (!(inode->i_flags & EXT4_EXTENTS_FL) &&
 	    (max_blocks > EXT4_MAX_TRANS_DATA))
 		max_blocks = EXT4_MAX_TRANS_DATA;
@@ -2387,7 +2389,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);
-- 
1.6.0.2.g2ebc0

Re: [PATCH] ext4: Code cleanups

[Mingming Cao]

在 2008-08-19二的 12:18 +0530，Aneesh Kumar K.V写道：
> a) rename num to nrblocks to clearly indicate we expecting block count as argument.
> b) renmae ext4_writepages_trans_blocks to ext4_da_writepages_trans_blocks to
> indicate it is the delayed allocation writepages transaction credit.
> c) Add some comments. remove some stale comments.
> 
> Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.vnet.ibm.com>

Acked-by: Mingming Cao <cmm@us.ibm.com>
> ---
>  fs/ext4/extents.c |    8 ++++----
>  fs/ext4/inode.c   |   14 ++++++++------
>  2 files changed, 12 insertions(+), 10 deletions(-)
> 
> diff --git a/fs/ext4/extents.c b/fs/ext4/extents.c
> index 73bb308..bf612a7 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)
>  {
>  	if (path) {
> @@ -1912,12 +1912,12 @@ int ext4_ext_calc_credits_for_single_extent(struct inode *inode, int num,
>  			 *  and other metadat blocks still need to be
>  			 *  accounted.
>  			 */
> -			/* 1 one bitmap, 1 block group descriptor */
> +			/* 1 bitmap, 1 block group descriptor */
>  			ret = 2 + EXT4_META_TRANS_BLOCKS(inode->i_sb);
>  		}
>  	}
> 
> -	return ext4_chunk_trans_blocks(inode, num);
> +	return ext4_chunk_trans_blocks(inode, nrblocks);
>  }
> 
>  /*
> @@ -1931,7 +1931,7 @@ int ext4_ext_calc_credits_for_single_extent(struct inode *inode, int num,
>   * If the nrblocks are discontiguous, they could cause
>   * the whole tree split more than once, but this is really rare.
>   */
> -int ext4_ext_index_trans_blocks(struct inode *inode, int num, int chunk)
> +int ext4_ext_index_trans_blocks(struct inode *inode, int nrblocks, int chunk)
>  {
>  	int index;
>  	int depth = ext_depth(inode);
> diff --git a/fs/ext4/inode.c b/fs/ext4/inode.c
> index a10f8e5..b32a723 100644
> --- a/fs/ext4/inode.c
> +++ b/fs/ext4/inode.c
> @@ -2321,16 +2321,18 @@ static int ext4_da_writepage(struct page *page,
>   * a single extent allocation into a single transaction,
>   * ext4_da_writpeages() will loop calling this before
>   * the block allocation.
> - *
> - * The page vector size limited the max number of pages could
> - * be writeout at a time. Based on this, the max blocks to pass to
> - * get_block is calculated
>   */
> 
> -static int ext4_writepages_trans_blocks(struct inode *inode)
> +static int ext4_da_writepages_trans_blocks(struct inode *inode)
>  {
>  	int max_blocks = EXT4_I(inode)->i_reserved_data_blocks;
> 
> +	/*
> +	 * With noextent format the journal credit needed to
> +	 * insert nrblocks contiguous block is dependent on
> +	 * number of contiguous block. So we will limit
> +	 * number of contiguous block to a sane value
> +	 */
>  	if (!(inode->i_flags & EXT4_EXTENTS_FL) &&
>  	    (max_blocks > EXT4_MAX_TRANS_DATA))
>  		max_blocks = EXT4_MAX_TRANS_DATA;
> @@ -2387,7 +2389,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);

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

Re: [PATCH] ext4: limit nrblocks properly with non extent format

[Mingming Cao]

在 2008-08-19二的 12:18 +0530，Aneesh Kumar K.V写道：
> We need to make sure adding new buffer_head to the
> contiguous block extent doesn't result in nrblocks
> greater than EXT4_MAX_TRANS_DATA. We only have
> enought credit to insert EXT4_MAX_TRANS_DATA blocks.
> 
> Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.vnet.ibm.com>

Reviewed-by: Mingming Cao <cmm@us.ibm.com>
> ---
>  fs/ext4/inode.c |   38 +++++++++++++++++++++++++-------------
>  1 files changed, 25 insertions(+), 13 deletions(-)
> 
> diff --git a/fs/ext4/inode.c b/fs/ext4/inode.c
> index dcd1337..a10f8e5 100644
> --- a/fs/ext4/inode.c
> +++ b/fs/ext4/inode.c
> @@ -1909,27 +1909,39 @@ static int  mpage_da_map_blocks(struct mpage_da_data *mpd)
>  static void mpage_add_bh_to_extent(struct mpage_da_data *mpd,
>  				   sector_t logical, struct buffer_head *bh)
>  {
> -	struct buffer_head *lbh = &mpd->lbh;
>  	sector_t next;
> +	size_t b_size = bh->b_size;
> +	struct buffer_head *lbh = &mpd->lbh;
>  	int nrblocks = lbh->b_size >> mpd->inode->i_blkbits;
> 
>  	/* check if thereserved journal credits might overflow */
> -	if (!(EXT4_I(mpd->inode)->i_flags & EXT4_EXTENTS_FL) &&
> -	     (nrblocks >= EXT4_MAX_TRANS_DATA))
> -		/*
> -		 * With noextent format we are limited by the journal
> -		 * credit available. Total credit needed to insert
> -		 * nrblocks contiguous blocks is dependent on the
> -		 * nrblocks. So limit nrblocks.
> -		 */
> -		goto flush_it;
> -
> +	if (!(EXT4_I(mpd->inode)->i_flags & EXT4_EXTENTS_FL)) {
> +		if (nrblocks >= EXT4_MAX_TRANS_DATA) {
> +			/*
> +			 * With noextent format we are limited by the journal
> +			 * credit available. Total credit needed to insert
> +			 * nrblocks contiguous blocks is dependent on the
> +			 * nrblocks. So limit nrblocks.
> +			 */
> +			goto flush_it;
> +		} else if ((nrblocks + (b_size >> mpd->inode->i_blkbits)) >
> +				EXT4_MAX_TRANS_DATA) {
> +			/*
> +			 * Adding the new buffer_head would make it cross the
> +			 * allowed limit for which we have journal credit
> +			 * reserved. So limit the new bh->b_size
> +			 */
> +			b_size = (EXT4_MAX_TRANS_DATA - nrblocks) <<
> +						mpd->inode->i_blkbits;
> +			/* we will do mpage_da_submit_io in the next loop */
> +		}
> +	}
>  	/*
>  	 * First block in the extent
>  	 */
>  	if (lbh->b_size == 0) {
>  		lbh->b_blocknr = logical;
> -		lbh->b_size = bh->b_size;
> +		lbh->b_size = b_size;
>  		lbh->b_state = bh->b_state & BH_FLAGS;
>  		return;
>  	}
> @@ -1939,7 +1951,7 @@ static void mpage_add_bh_to_extent(struct mpage_da_data *mpd,
>  	 * Can we merge the block to our big extent?
>  	 */
>  	if (logical == next && (bh->b_state & BH_FLAGS) == lbh->b_state) {
> -		lbh->b_size += bh->b_size;
> +		lbh->b_size += b_size;
>  		return;
>  	}
> 

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