[PATCH 1/4] ext4: fold two if statements into one

[PATCH 1/4] ext4: fold two if statements into one

[Yongqiang Yang]

Signed-off-by: Yongqiang Yang <xiaoqiangnk@gmail.com>
---
 fs/ext4/extents.c |   23 +++++++++++------------
 1 files changed, 11 insertions(+), 12 deletions(-)

diff --git a/fs/ext4/extents.c b/fs/ext4/extents.c
index f1ed90b..f887023 100644
--- a/fs/ext4/extents.c
+++ b/fs/ext4/extents.c
@@ -4082,18 +4082,17 @@ got_allocated_blocks:
 		 */
 		reserved_clusters = get_reserved_cluster_alloc(inode,
 						map->m_lblk, allocated);
-		if (map->m_flags & EXT4_MAP_FROM_CLUSTER) {
-			if (reserved_clusters) {
-				/*
-				 * We have clusters reserved for this range.
-				 * But since we are not doing actual allocation
-				 * and are simply using blocks from previously
-				 * allocated cluster, we should release the
-				 * reservation and not claim quota.
-				 */
-				ext4_da_update_reserve_space(inode,
-						reserved_clusters, 0);
-			}
+		if ((map->m_flags & EXT4_MAP_FROM_CLUSTER) &&
+		    reserved_clusters) {
+			/*
+			 * We have clusters reserved for this range.
+			 * But since we are not doing actual allocation
+			 * and are simply using blocks from previously
+			 * allocated cluster, we should release the
+			 * reservation and not claim quota.
+			 */
+			ext4_da_update_reserve_space(inode,
+					reserved_clusters, 0);
 		} else {
 			BUG_ON(allocated_clusters < reserved_clusters);
 			/* We will claim quota for all newly allocated blocks.*/
-- 
1.7.5.1

[PATCH 2/4] ext4: move variable to its scope

[Yongqiang Yang]

Signed-off-by: Yongqiang Yang <xiaoqiangnk@gmail.com>
---
 fs/ext4/extents.c |   15 ++++++++-------
 1 files changed, 8 insertions(+), 7 deletions(-)

diff --git a/fs/ext4/extents.c b/fs/ext4/extents.c
index f887023..8b09a47 100644
--- a/fs/ext4/extents.c
+++ b/fs/ext4/extents.c
@@ -117,10 +117,9 @@ static ext4_fsblk_t ext4_ext_find_goal(struct inode *inode,
 			      struct ext4_ext_path *path,
 			      ext4_lblk_t block)
 {
-	int depth;
-
 	if (path) {
 		struct ext4_extent *ex;
+		int depth;
 		depth = path->p_depth;
 
 		/*
@@ -247,7 +246,7 @@ static inline int ext4_ext_space_root_idx(struct inode *inode, int check)
 int ext4_ext_calc_metadata_amount(struct inode *inode, ext4_lblk_t lblock)
 {
 	struct ext4_inode_info *ei = EXT4_I(inode);
-	int idxs, num = 0;
+	int idxs;
 
 	idxs = ((inode->i_sb->s_blocksize - sizeof(struct ext4_extent_header))
 		/ sizeof(struct ext4_extent_idx));
@@ -262,6 +261,7 @@ int ext4_ext_calc_metadata_amount(struct inode *inode, ext4_lblk_t lblock)
 	 */
 	if (ei->i_da_metadata_calc_len &&
 	    ei->i_da_metadata_calc_last_lblock+1 == lblock) {
+		int num = 0;
 		if ((ei->i_da_metadata_calc_len % idxs) == 0)
 			num++;
 		if ((ei->i_da_metadata_calc_len % (idxs*idxs)) == 0)
@@ -324,8 +324,6 @@ static int ext4_valid_extent_entries(struct inode *inode,
 				struct ext4_extent_header *eh,
 				int depth)
 {
-	struct ext4_extent *ext;
-	struct ext4_extent_idx *ext_idx;
 	unsigned short entries;
 	if (eh->eh_entries == 0)
 		return 1;
@@ -334,6 +332,7 @@ static int ext4_valid_extent_entries(struct inode *inode,
 
 	if (depth == 0) {
 		/* leaf entries */
+		struct ext4_extent *ext;
 		ext = EXT_FIRST_EXTENT(eh);
 		while (entries) {
 			if (!ext4_valid_extent(inode, ext))
@@ -342,6 +341,7 @@ static int ext4_valid_extent_entries(struct inode *inode,
 			entries--;
 		}
 	} else {
+		struct ext4_extent_idx *ext_idx;
 		ext_idx = EXT_FIRST_INDEX(eh);
 		while (entries) {
 			if (!ext4_valid_extent_idx(inode, ext_idx))
@@ -3720,13 +3720,12 @@ int ext4_ext_map_blocks(handle_t *handle, struct inode *inode,
 	ext4_fsblk_t newblock = 0;
 	int free_on_err = 0, err = 0, depth, ret;
 	unsigned int allocated = 0, offset = 0;
-	unsigned int allocated_clusters = 0, reserved_clusters = 0;
+	unsigned int allocated_clusters = 0;
 	unsigned int punched_out = 0;
 	unsigned int result = 0;
 	struct ext4_allocation_request ar;
 	ext4_io_end_t *io = EXT4_I(inode)->cur_aio_dio;
 	ext4_lblk_t cluster_offset;
-	struct ext4_map_blocks punch_map;
 
 	ext_debug("blocks %u/%u requested for inode %lu\n",
 		  map->m_lblk, map->m_len, inode->i_ino);
@@ -3802,6 +3801,7 @@ int ext4_ext_map_blocks(handle_t *handle, struct inode *inode,
 
 		/* if found extent covers block, simply return it */
 		if (in_range(map->m_lblk, ee_block, ee_len)) {
+			struct ext4_map_blocks punch_map;
 			ext4_fsblk_t partial_cluster = 0;
 
 			newblock = map->m_lblk - ee_block + ee_start;
@@ -4077,6 +4077,7 @@ got_allocated_blocks:
 	 * block allocation which had been deferred till now.
 	 */
 	if (flags & EXT4_GET_BLOCKS_DELALLOC_RESERVE) {
+		unsigned int reserved_clusters = 0;
 		/*
 		 * Check how many clusters we had reserved this allocted range.
 		 */
-- 
1.7.5.1

Re: [PATCH 2/4] ext4: move variable to its scope

[Ted Ts'o]

On Wed, Oct 26, 2011 at 03:11:50PM +0800, Yongqiang Yang wrote:
> Signed-off-by: Yongqiang Yang <xiaoqiangnk@gmail.com>

Applied, thanks.

					- Ted

[PATCH 3/4] ext4: let AGGRESSIVE_TEST brace code only related to itself

[Yongqiang Yang]

Signed-off-by: Yongqiang Yang <xiaoqiangnk@gmail.com>
---
 fs/ext4/extents.c |   16 ++++++++--------
 1 files changed, 8 insertions(+), 8 deletions(-)

diff --git a/fs/ext4/extents.c b/fs/ext4/extents.c
index 8b09a47..49b4e28 100644
--- a/fs/ext4/extents.c
+++ b/fs/ext4/extents.c
@@ -182,12 +182,12 @@ static inline int ext4_ext_space_block(struct inode *inode, int check)
 
 	size = (inode->i_sb->s_blocksize - sizeof(struct ext4_extent_header))
 			/ sizeof(struct ext4_extent);
-	if (!check) {
 #ifdef AGGRESSIVE_TEST
+	if (!check) {
 		if (size > 6)
 			size = 6;
-#endif
 	}
+#endif
 	return size;
 }
 
@@ -197,12 +197,12 @@ static inline int ext4_ext_space_block_idx(struct inode *inode, int check)
 
 	size = (inode->i_sb->s_blocksize - sizeof(struct ext4_extent_header))
 			/ sizeof(struct ext4_extent_idx);
-	if (!check) {
 #ifdef AGGRESSIVE_TEST
+	if (!check) {
 		if (size > 5)
 			size = 5;
-#endif
 	}
+#endif
 	return size;
 }
 
@@ -213,12 +213,12 @@ static inline int ext4_ext_space_root(struct inode *inode, int check)
 	size = sizeof(EXT4_I(inode)->i_data);
 	size -= sizeof(struct ext4_extent_header);
 	size /= sizeof(struct ext4_extent);
-	if (!check) {
 #ifdef AGGRESSIVE_TEST
+	if (!check) {
 		if (size > 3)
 			size = 3;
-#endif
 	}
+#endif
 	return size;
 }
 
@@ -229,12 +229,12 @@ static inline int ext4_ext_space_root_idx(struct inode *inode, int check)
 	size = sizeof(EXT4_I(inode)->i_data);
 	size -= sizeof(struct ext4_extent_header);
 	size /= sizeof(struct ext4_extent_idx);
-	if (!check) {
 #ifdef AGGRESSIVE_TEST
+	if (!check) {
 		if (size > 4)
 			size = 4;
-#endif
 	}
+#endif
 	return size;
 }
 
-- 
1.7.5.1

Re: [PATCH 3/4] ext4: let AGGRESSIVE_TEST brace code only related to itself

[Ted Ts'o]

On Wed, Oct 26, 2011 at 03:11:51PM +0800, Yongqiang Yang wrote:
> Signed-off-by: Yongqiang Yang <xiaoqiangnk@gmail.com>

Applied, with a minor tweak --- this *is* a place where folding two
nested if statements is a good idea (and reduces ext4's lines of code
count :-).

					- Ted

commit 2a83a909eab183807ca3ccbb2559e22611131336
Author: Yongqiang Yang <xiaoqiangnk@gmail.com>
Date:   Sat Oct 29 09:29:11 2011 -0400

    ext4: clean up AGGRESSIVE_TEST code
    
    Signed-off-by: Yongqiang Yang <xiaoqiangnk@gmail.com>
    Signed-off-by: "Theodore Ts'o" <tytso@mit.edu>

diff --git a/fs/ext4/extents.c b/fs/ext4/extents.c
index 73823d5..b66cef0 100644
--- a/fs/ext4/extents.c
+++ b/fs/ext4/extents.c
@@ -181,12 +181,10 @@ static inline int ext4_ext_space_block(struct inode *inode, int check)
 
 	size = (inode->i_sb->s_blocksize - sizeof(struct ext4_extent_header))
 			/ sizeof(struct ext4_extent);
-	if (!check) {
 #ifdef AGGRESSIVE_TEST
-		if (size > 6)
-			size = 6;
+	if (!check && size > 6)
+		size = 6;
 #endif
-	}
 	return size;
 }
 
@@ -196,12 +194,10 @@ static inline int ext4_ext_space_block_idx(struct inode *inode, int check)
 
 	size = (inode->i_sb->s_blocksize - sizeof(struct ext4_extent_header))
 			/ sizeof(struct ext4_extent_idx);
-	if (!check) {
 #ifdef AGGRESSIVE_TEST
-		if (size > 5)
-			size = 5;
+	if (!check && size > 5)
+		size = 5;
 #endif
-	}
 	return size;
 }
 
@@ -212,12 +208,10 @@ static inline int ext4_ext_space_root(struct inode *inode, int check)
 	size = sizeof(EXT4_I(inode)->i_data);
 	size -= sizeof(struct ext4_extent_header);
 	size /= sizeof(struct ext4_extent);
-	if (!check) {
 #ifdef AGGRESSIVE_TEST
-		if (size > 3)
-			size = 3;
+	if (!check && size > 3)
+		size = 3;
 #endif
-	}
 	return size;
 }
 
@@ -228,12 +222,10 @@ static inline int ext4_ext_space_root_idx(struct inode *inode, int check)
 	size = sizeof(EXT4_I(inode)->i_data);
 	size -= sizeof(struct ext4_extent_header);
 	size /= sizeof(struct ext4_extent_idx);
-	if (!check) {
 #ifdef AGGRESSIVE_TEST
-		if (size > 4)
-			size = 4;
+	if (!check && size > 4)
+		size = 4;
 #endif
-	}
 	return size;
 }
 

[PATCH 4/4] ext4: trace punch_hole correctly in ext4_ext_map_blocks

[Yongqiang Yang]

When ext4_ext_map_blocks() is called by punch_hole, trace should
trace blocks punched out.

Signed-off-by: Yongqiang Yang <xiaoqiangnk@gmail.com>
---
 fs/ext4/extents.c |    6 +++---
 1 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/fs/ext4/extents.c b/fs/ext4/extents.c
index 49b4e28..58af095 100644
--- a/fs/ext4/extents.c
+++ b/fs/ext4/extents.c
@@ -4173,12 +4173,12 @@ out2:
 		ext4_ext_drop_refs(path);
 		kfree(path);
 	}
-	trace_ext4_ext_map_blocks_exit(inode, map->m_lblk,
-		newblock, map->m_len, err ? err : allocated);

Re: [PATCH 4/4] ext4: trace punch_hole correctly in ext4_ext_map_blocks

[Ted Ts'o]

On Wed, Oct 26, 2011 at 03:11:52PM +0800, Yongqiang Yang wrote:
> When ext4_ext_map_blocks() is called by punch_hole, trace should
> trace blocks punched out.
> 
> Signed-off-by: Yongqiang Yang <xiaoqiangnk@gmail.com>

Thanks for catching this; applied.

						- Ted

Re: [PATCH 1/4] ext4: fold two if statements into one

[Ted Ts'o]

On Wed, Oct 26, 2011 at 03:11:49PM +0800, Yongqiang Yang wrote:
> Signed-off-by: Yongqiang Yang <xiaoqiangnk@gmail.com>
> ---
>  fs/ext4/extents.c |   23 +++++++++++------------
>  1 files changed, 11 insertions(+), 12 deletions(-)

Unfortunately this patch is incorrect since it changes when the else
clause would get executed:

       if (a) {
       	   if (b) {
	       ...
	   }
       } else {
           ...
       }

is not the same as:

       if (a && b) {
	   ...
       } else {
           ...
       }

Regards,

					- Ted