Patchwork [2/2] ext4: introduce ext4_get_group_number()

login
register
mail settings
Submitter Lukas Czerner
Date March 25, 2013, 10:24 a.m.
Message ID <1364207051-27037-2-git-send-email-lczerner@redhat.com>
Download mbox | patch
Permalink /patch/230610/
State Superseded
Headers show

Comments

Lukas Czerner - March 25, 2013, 10:24 a.m.
Currently on many places in ext4 we're using
ext4_get_group_no_and_offset() even though we're only interested in
knowing the block group of the particular block, not the offset within
the block group so we can use more efficient way to compute block group.

This patch introduces ext4_get_group_number() which computes block group
for a givem block much more efficiently. Use this function instead of
ext4_get_group_no_and_offset() everywhere where we're only interested in
knowing the block group.

Signed-off-by: Lukas Czerner <lczerner@redhat.com>
---
 fs/ext4/balloc.c  |   14 +++++++++++---
 fs/ext4/ext4.h    |    3 +++
 fs/ext4/mballoc.c |    6 +++---
 fs/ext4/resize.c  |   10 +++++-----
 4 files changed, 22 insertions(+), 11 deletions(-)
Theodore Ts'o - March 27, 2013, 3:12 a.m.
On Mon, Mar 25, 2013 at 11:24:11AM +0100, Lukas Czerner wrote:
> Currently on many places in ext4 we're using
> ext4_get_group_no_and_offset() even though we're only interested in
> knowing the block group of the particular block, not the offset within
> the block group so we can use more efficient way to compute block group.
> 
> This patch introduces ext4_get_group_number() which computes block group
> for a givem block much more efficiently. Use this function instead of
> ext4_get_group_no_and_offset() everywhere where we're only interested in
> knowing the block group.

Again, we have the same problem as the previous patch.  I could
imagine setting a flag which uses the shift-instead-of-div
optimization, but that will reduce the optimization somewhat.  OTOH,
an 64-bit division is pretty expensive, especially on older/simpler
CPU's, so perhaps it's worth it.

   	  	      		    	    - Ted
--
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
Theodore Ts'o - March 27, 2013, 3:44 a.m.
On Tue, Mar 26, 2013 at 11:12:07PM -0400, Theodore Ts'o wrote:
> Again, we have the same problem as the previous patch.  I could
> imagine setting a flag which uses the shift-instead-of-div
> optimization, but that will reduce the optimization somewhat.  

Just to be clear, I was thinking that at mount time we could check to
see whether the number of blocks per group == blocksize * 8, and if
so, then set a per-filesystem flag in the sbi structure which we could
then use to optionally use the shift-right optimization.

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

Patch

diff --git a/fs/ext4/balloc.c b/fs/ext4/balloc.c
index 68368c2..21b43ca 100644
--- a/fs/ext4/balloc.c
+++ b/fs/ext4/balloc.c
@@ -30,6 +30,16 @@  static unsigned ext4_num_base_meta_clusters(struct super_block *sb,
  */
 
 /*
+ * Calculate block group number for a given block number
+ */
+inline ext4_group_t ext4_get_group_number(struct super_block *sb,
+					  ext4_fsblk_t block)
+{
+	return (le32_to_cpu(EXT4_SB(sb)->s_es->s_first_data_block) + block) >>
+	       (EXT4_BLOCK_SIZE_BITS(sb) + EXT4_CLUSTER_BITS(sb) + 3);
+}
+
+/*
  * Calculate the block group number and offset into the block/cluster
  * allocation bitmap, given a block number
  */
@@ -58,9 +68,7 @@  static inline int ext4_block_in_group(struct super_block *sb,
 				      ext4_group_t block_group)
 {
 	ext4_group_t actual_group;
-	actual_group = (le32_to_cpu(EXT4_SB(sb)->s_es->s_first_data_block) +
-			block) >>
-		       (EXT4_BLOCK_SIZE_BITS(sb) + EXT4_CLUSTER_BITS(sb) + 3);
+	actual_group = ext4_get_group_number(sb, block);
 	return (actual_group == block_group) ? 1 : 0;
 }
 
diff --git a/fs/ext4/ext4.h b/fs/ext4/ext4.h
index 167ff56..4df299b 100644
--- a/fs/ext4/ext4.h
+++ b/fs/ext4/ext4.h
@@ -1787,6 +1787,9 @@  ext4_group_first_block_no(struct super_block *sb, ext4_group_t group_no)
 void ext4_get_group_no_and_offset(struct super_block *sb, ext4_fsblk_t blocknr,
 			ext4_group_t *blockgrpp, ext4_grpblk_t *offsetp);
 
+inline ext4_group_t ext4_get_group_number(struct super_block *sb,
+					  ext4_fsblk_t block);
+
 /*
  * Timeout and state flag for lazy initialization inode thread.
  */
diff --git a/fs/ext4/mballoc.c b/fs/ext4/mballoc.c
index ee6614b..dca7f06 100644
--- a/fs/ext4/mballoc.c
+++ b/fs/ext4/mballoc.c
@@ -3342,7 +3342,7 @@  static void ext4_mb_put_pa(struct ext4_allocation_context *ac,
 	if (pa->pa_type == MB_GROUP_PA)
 		grp_blk--;
 
-	ext4_get_group_no_and_offset(sb, grp_blk, &grp, NULL);
+	grp = ext4_get_group_number(sb, grp_blk);
 
 	/*
 	 * possible race:
@@ -3807,7 +3807,7 @@  repeat:
 
 	list_for_each_entry_safe(pa, tmp, &list, u.pa_tmp_list) {
 		BUG_ON(pa->pa_type != MB_INODE_PA);
-		ext4_get_group_no_and_offset(sb, pa->pa_pstart, &group, NULL);
+		group = ext4_get_group_number(sb, pa->pa_pstart);
 
 		err = ext4_mb_load_buddy(sb, group, &e4b);
 		if (err) {
@@ -4069,7 +4069,7 @@  ext4_mb_discard_lg_preallocations(struct super_block *sb,
 
 	list_for_each_entry_safe(pa, tmp, &discard_list, u.pa_tmp_list) {
 
-		ext4_get_group_no_and_offset(sb, pa->pa_pstart, &group, NULL);
+		group = ext4_get_group_number(sb, pa->pa_pstart);
 		if (ext4_mb_load_buddy(sb, group, &e4b)) {
 			ext4_error(sb, "Error loading buddy information for %u",
 					group);
diff --git a/fs/ext4/resize.c b/fs/ext4/resize.c
index c169477..e349853 100644
--- a/fs/ext4/resize.c
+++ b/fs/ext4/resize.c
@@ -272,7 +272,7 @@  next_group:
 		if (start_blk >= last_blk)
 			goto next_group;
 		group_data[bb_index].block_bitmap = start_blk++;
-		ext4_get_group_no_and_offset(sb, start_blk - 1, &group, NULL);
+		group = ext4_get_group_number(sb, start_blk - 1);
 		group -= group_data[0].group;
 		group_data[group].free_blocks_count--;
 		if (flexbg_size > 1)
@@ -284,7 +284,7 @@  next_group:
 		if (start_blk >= last_blk)
 			goto next_group;
 		group_data[ib_index].inode_bitmap = start_blk++;
-		ext4_get_group_no_and_offset(sb, start_blk - 1, &group, NULL);
+		group = ext4_get_group_number(sb, start_blk - 1);
 		group -= group_data[0].group;
 		group_data[group].free_blocks_count--;
 		if (flexbg_size > 1)
@@ -296,7 +296,7 @@  next_group:
 		if (start_blk + EXT4_SB(sb)->s_itb_per_group > last_blk)
 			goto next_group;
 		group_data[it_index].inode_table = start_blk;
-		ext4_get_group_no_and_offset(sb, start_blk, &group, NULL);
+		group = ext4_get_group_number(sb, start_blk - 1);
 		group -= group_data[0].group;
 		group_data[group].free_blocks_count -=
 					EXT4_SB(sb)->s_itb_per_group;
@@ -392,7 +392,7 @@  static int set_flexbg_block_bitmap(struct super_block *sb, handle_t *handle,
 		ext4_group_t group;
 		int err;
 
-		ext4_get_group_no_and_offset(sb, block, &group, NULL);
+		group = ext4_get_group_number(sb, block);
 		start = ext4_group_first_block_no(sb, group);
 		group -= flex_gd->groups[0].group;
 
@@ -1879,7 +1879,7 @@  retry:
 		/* Nothing need to do */
 		return 0;
 
-	ext4_get_group_no_and_offset(sb, n_blocks_count - 1, &n_group, &offset);
+	n_group = ext4_get_group_number(sb, n_blocks_count - 1);
 	ext4_get_group_no_and_offset(sb, o_blocks_count - 1, &o_group, &offset);
 
 	n_desc_blocks = num_desc_blocks(sb, n_group + 1);