Patchwork [1/2,v2] ext4: Make ext4_block_in_group() much more efficient

login
register
mail settings
Submitter Lukas Czerner
Date March 27, 2013, 11:04 a.m.
Message ID <1364382245-32510-1-git-send-email-lczerner@redhat.com>
Download mbox | patch
Permalink /patch/231657/
State Superseded
Headers show

Comments

Lukas Czerner - March 27, 2013, 11:04 a.m.
Currently in when getting the block group number for a particular block
in ext4_block_in_group() we're using ext4_get_group_no_and_offset()
which uses do_div() to get the block group and the remainer which is
offset within the group.

We don't need all of that in ext4_block_in_group() as we only need to
figure out the group number.

This commit changes ext4_block_in_group() to calculate group number
directly. This shows as a big improvement with regards to cpu
utilization. Measuring fallocate -l 15T on fresh file system with perf
showed that 23% of cpu time was spend in the
ext4_get_group_no_and_offset(). With this change it completely
disappears from the list only bumping the occurrence of
ext4_init_block_bitmap() which is the biggest user of
ext4_block_in_group() by 4%. As the result of this change on my system
the fallocate call was approx. 10% faster.

However since there is '-g' option in mkfs which allow us setting
different groups size (mostly for developers) I've introduced new per
file system flag whether we have a standard block group size or not. The
flag is used to determine whether we can use the bit shift optimization
or not.

Signed-off-by: Lukas Czerner <lczerner@redhat.com>
---
v2: Add sbi flag if there is standard group size

 fs/ext4/balloc.c |   22 ++++++++++++++++------
 fs/ext4/ext4.h   |    3 +++
 fs/ext4/super.c  |    4 ++++
 3 files changed, 23 insertions(+), 6 deletions(-)
Theodore Ts'o - March 27, 2013, 1:34 p.m.
On Wed, Mar 27, 2013 at 12:04:04PM +0100, Lukas Czerner wrote:
>  #define EXT4_MOUNT2_EXPLICIT_DELALLOC	0x00000001 /* User explicitly
>  						      specified delalloc */
> +#define EXT4_MOUNT_STD_GROUP_SIZE	0x00000002 /* We have standard group
> +						      size of blocksize * 8
> +						      blocks */

This should be EXT4_MOUNT2_STD_GROUP_SIZE, and then all calls to
set_opt, test_opt, etc. should be set_opt2, test_opt2, etc.  As it
turns out bit 0x0002 isn't used for flags in s_mount_opt family, so
the other fix would be to move this definition to before
EXT4_MOUNT_GRPID in ext4.

However, the EXT4_MOUNT_* options should be reserved for options that
are set via mount options, since we have support for setting bits in
s_mount_opt via a table-driven approach (see how the mount_opt field
in the struct mount_opts in fs/ext4/super.c is used).  So for things
that are set automatically, such as in this case, we should use a bit
assignment for s_mount_opt2.

Cheers,

					- 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 92e68b3..a4d2e73 100644
--- a/fs/ext4/balloc.c
+++ b/fs/ext4/balloc.c
@@ -49,14 +49,24 @@  void ext4_get_group_no_and_offset(struct super_block *sb, ext4_fsblk_t blocknr,
 
 }
 
-static int ext4_block_in_group(struct super_block *sb, ext4_fsblk_t block,
-			ext4_group_t block_group)
+/*
+ * Check whether the 'block' lives within the 'block_group'. Returns 1 if so
+ * and 0 otherwise.
+ */
+static inline int ext4_block_in_group(struct super_block *sb,
+				      ext4_fsblk_t block,
+				      ext4_group_t block_group)
 {
 	ext4_group_t actual_group;
-	ext4_get_group_no_and_offset(sb, block, &actual_group, NULL);
-	if (actual_group == block_group)
-		return 1;
-	return 0;
+
+	if (test_opt(sb, STD_GROUP_SIZE))
+		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);
+	else
+		ext4_get_group_no_and_offset(sb, block, &actual_group, NULL);
+	return (actual_group == block_group) ? 1 : 0;
 }
 
 /* Return the number of clusters used for file system metadata; this
diff --git a/fs/ext4/ext4.h b/fs/ext4/ext4.h
index 167ff56..2ee8d76 100644
--- a/fs/ext4/ext4.h
+++ b/fs/ext4/ext4.h
@@ -983,6 +983,9 @@  struct ext4_inode_info {
 
 #define EXT4_MOUNT2_EXPLICIT_DELALLOC	0x00000001 /* User explicitly
 						      specified delalloc */
+#define EXT4_MOUNT_STD_GROUP_SIZE	0x00000002 /* We have standard group
+						      size of blocksize * 8
+						      blocks */
 
 #define clear_opt(sb, opt)		EXT4_SB(sb)->s_mount_opt &= \
 						~EXT4_MOUNT_##opt
diff --git a/fs/ext4/super.c b/fs/ext4/super.c
index d1ee6a8..28ea887 100644
--- a/fs/ext4/super.c
+++ b/fs/ext4/super.c
@@ -3522,6 +3522,10 @@  static int ext4_fill_super(struct super_block *sb, void *data, int silent)
 	sbi->s_addr_per_block_bits = ilog2(EXT4_ADDR_PER_BLOCK(sb));
 	sbi->s_desc_per_block_bits = ilog2(EXT4_DESC_PER_BLOCK(sb));
 
+	/* Do we have standard group size of blocksize * 8 blocks ? */
+	if (sbi->s_blocks_per_group == blocksize << 8)
+		set_opt(sb, STD_GROUP_SIZE);
+
 	for (i = 0; i < 4; i++)
 		sbi->s_hash_seed[i] = le32_to_cpu(es->s_hash_seed[i]);
 	sbi->s_def_hash_version = es->s_def_hash_version;