[Devel] [PATCH vz10 v2] fs/ext4: do not run test_and_set_bit on every prefetch scan

[Konstantin Khorenko <khorenko@virtuozzo.com>]

From: Alexey Kuznetsov <kuznet@virtuozzo.com>

ext4_mb_prefetch() marks every group it scans with
EXT4_MB_GRP_TEST_AND_SET_READ(), so that a block bitmap is prefetched
only once. The bit is never cleared, yet the atomic test_and_set_bit
is executed on every pass over every group.

A locked read-modify-write takes the bb_state cache line exclusive,
so each allocating thread invalidates it for all the others, and that
same line carries the rest of the hot read-mostly fields of struct
ext4_group_info. The prefetch loop walks s_mb_prefetch groups per
allocation, so the ping-pong scales with the number of allocating
threads: even on just 4 allocating threads ext4 wastes 50% of cpu
time on this test_and_set, none with the patch.

Test the bit first and fall through to the atomic only while it is
still clear. The bit is set-only, so a stale test_bit can only miss a
set bit, and then the atomic decides the race as before. Nothing is
published through the bit, so its full barrier is not needed.

The atomic stays ahead of the EXT4_MB_GRP_NEED_INIT() and free
clusters checks, so a group that turns out not to be worth prefetching
is still marked and not looked at again.

Testing: the change has been in production use as a private patch.

https://virtuozzo.atlassian.net/browse/VSTOR-137147
Feature: ext4 fixes

Signed-off-by: Alexey Kuznetsov <kuznet@virtuozzo.com>
Reviewed-by: Konstantin Khorenko <khorenko@virtuozzo.com>
---
Alexey's patch, resent with the commit message rewritten. The diff is
the same as in v1.

Changes in v2:
- commit message rewritten: what the atomic actually costs and why a
  plain test_bit is safe, plus a note that the atomic has to stay
  ahead of the EXT4_MB_GRP_NEED_INIT() check
- Jira reference in the form used in the tree, Feature tag added
- no code changes
- Link to v1: http://lore.virtuozzo.com/aqlr6A+cDUoielHy@mops2
---
 fs/ext4/ext4.h    | 2 ++
 fs/ext4/mballoc.c | 4 +++-
 2 files changed, 5 insertions(+), 1 deletion(-)

diff --git a/fs/ext4/ext4.h b/fs/ext4/ext4.h
index bf7637785e073..ca6498609e2e1 100644
--- a/fs/ext4/ext4.h
+++ b/fs/ext4/ext4.h
@@ -3514,6 +3514,8 @@ struct ext4_group_info {
 	(clear_bit(EXT4_GROUP_INFO_WAS_TRIMMED_BIT, &((grp)->bb_state)))
 #define EXT4_MB_GRP_TEST_AND_SET_READ(grp)	\
 	(test_and_set_bit(EXT4_GROUP_INFO_BBITMAP_READ_BIT, &((grp)->bb_state)))
+#define EXT4_MB_GRP_TEST_READ(grp)	\
+	(test_bit(EXT4_GROUP_INFO_BBITMAP_READ_BIT, &((grp)->bb_state)))
 
 #define EXT4_MAX_CONTENTION		8
 #define EXT4_CONTENTION_THRESHOLD	2
diff --git a/fs/ext4/mballoc.c b/fs/ext4/mballoc.c
index aaf2c24c514c6..b50aa34057967 100644
--- a/fs/ext4/mballoc.c
+++ b/fs/ext4/mballoc.c
@@ -2836,7 +2836,9 @@ ext4_group_t ext4_mb_prefetch(struct super_block *sb, ext4_group_t group,
 		 * prefetch once, so we avoid getblk() call, which can
 		 * be expensive.
 		 */
-		if (gdp && grp && !EXT4_MB_GRP_TEST_AND_SET_READ(grp) &&
+		if (gdp && grp &&
+		    !EXT4_MB_GRP_TEST_READ(grp) &&
+		    !EXT4_MB_GRP_TEST_AND_SET_READ(grp) &&
 		    EXT4_MB_GRP_NEED_INIT(grp) &&
 		    ext4_free_group_clusters(sb, gdp) > 0 ) {
 			bh = ext4_read_block_bitmap_nowait(sb, group, true);

---
base-commit: c385dabedddacd691cfe0b21a599567c3b72314c
change-id: 20260916-ext4-prefetch-test-bit-a8958ad28ec8

Best regards,
--  
Konstantin Khorenko <khorenko@virtuozzo.com>

_______________________________________________
Devel mailing list
Devel@openvz.org
https://lists.openvz.org/mailman/listinfo/devel