[PATCH][RFC] ext4: avoid taking down_read(&grp->alloc_sem)

["Amir G." <amir73il@users.sourceforge.net>]

Hi Aneesh,

As you are signed off on most of the recent alloc_sem related code changes,
can you please comment on the patch below, which tries to avoid taking
the read lock most of the times on a 4K block fs.

Can anyone tell what performance impact (if any) will be caused by avoiding
the read lock on most allocations? group spin lock will still be taken, but for
much shorter periods of time (cycles).

Any ideas how this patch can be properly tested?

Thanks,
Amir.

grp->alloc_sem is used to synchronize buddy cache users with buddy cache init
of other groups that use the same buddy cache page and with adding blocks to
group on online resize.

When blocks_per_page <= 2, each group has it's own private buddy cache page
so taking the read lock for every allocation is futile and can be avoided for
every group, but the last one.

The write lock is taken in ext4_mb_init_group() and in ext4_add_groupblocks()
to synchronize the buddy cache init of a group on first time allocation after
mount and after extending the last group.

Signed-off-by: Amir Goldstein <amir73il@users.sf.net>
---
 fs/ext4/mballoc.c |   19 +++++++++++++++----
 1 files changed, 15 insertions(+), 4 deletions(-)

diff --git a/fs/ext4/mballoc.c b/fs/ext4/mballoc.c
index 1b3256b..22a5251 100644
--- a/fs/ext4/mballoc.c
+++ b/fs/ext4/mballoc.c
@@ -1160,7 +1160,15 @@ ext4_mb_load_buddy(struct super_block *sb,
ext4_group_t group,
 	e4b->bd_group = group;
 	e4b->bd_buddy_page = NULL;
 	e4b->bd_bitmap_page = NULL;
-	e4b->alloc_semp = &grp->alloc_sem;
+	/*
+	 * We only need to take the read lock if other groups share the buddy
+	 * page with this group or if blocks may be added to this (last) group
+	 * by ext4_group_extend().
+	 */
+	if (blocks_per_page > 2 || group == sbi->s_groups_count - 1)
+		e4b->alloc_semp = &grp->alloc_sem;
+	else
+		e4b->alloc_semp = NULL;

 	/* Take the read lock on the group alloc
 	 * sem. This would make sure a parallel
@@ -1169,7 +1177,8 @@ ext4_mb_load_buddy(struct super_block *sb,
ext4_group_t group,
 	 * till we are done with allocation
 	 */
 repeat_load_buddy:
-	down_read(e4b->alloc_semp);
+	if (e4b->alloc_semp)
+		down_read(e4b->alloc_semp);

 	if (unlikely(EXT4_MB_GRP_NEED_INIT(grp))) {
 		/* we need to check for group need init flag
@@ -1177,7 +1186,8 @@ repeat_load_buddy:
 		 * that new blocks didn't get added to the group
 		 * when we are loading the buddy cache
 		 */
-		up_read(e4b->alloc_semp);
+		if (e4b->alloc_semp)
+			up_read(e4b->alloc_semp);
 		/*
 		 * we need full data about the group
 		 * to make a good selection
@@ -1277,7 +1287,8 @@ err:
 	e4b->bd_bitmap = NULL;

 	/* Done with the buddy cache */
-	up_read(e4b->alloc_semp);
+	if (e4b->alloc_semp)
+		up_read(e4b->alloc_semp);
 	return ret;
 }

-- 
1.7.0.4