All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v2 00/19] Fixes, cleanups and unit test for mballoc
@ 2023-04-12 17:28 Kemeng Shi
  2023-04-12 17:28 ` [PATCH v2 01/19] ext4: fix wrong unit use in ext4_mb_normalize_request Kemeng Shi
                   ` (18 more replies)
  0 siblings, 19 replies; 25+ messages in thread
From: Kemeng Shi @ 2023-04-12 17:28 UTC (permalink / raw)
  To: tytso, adilger.kernel, ojaswin; +Cc: linux-ext4, linux-kernel, shikemeng

There are three parts in this patchset:
Part1: Patch 1-7 is v2 of sent series
v1->v2:
1. collect reviewed-by from Ojaswin. Only "ext4: add
EXT4_MB_HINT_GOAL_ONLY test in ext4_mb_use_preallocated" needs futher
review. See [1] for previous comments.
2. drop "ext4: fix wrong unit use in ext4_mb_new_inode_pa" which is
already done in [2].

Part2: Patch 8-17 are more fixes and cleanups to mballoc
Some patches in this part will be conflict with patches in part1, so
append new patches in this series instead of creating a new one.
Patch 8-11 are some random fixes and cleanups, see respective log
message for detail.
Patch 12-17 factor out codes to mark bit in group is used or free
which will update on disk block bitmap and group descriptor. Several
reasons to do this:
1. pair behavior of alloc/free bits. For example,
ext4_mb_new_blocks_simple will update free_clusters in struct flex_groups
in ext4_mb_mark_bb while ext4_free_blocks_simple forgets this.
2. remove repeat code to read from disk, update and write back to disk.
3. reduce future unit test mocks to avoid real IO to update structure
on disk.

Part3: Patch 18-19 add one unit test for mballoc
Patch 18 add mocks to functions which will issue IO to disk.
Patch 19 add unit test for ext4_mb_new_blocks_simple in mballoc.
Details can be found in respective log message.

Before add more unit test, there are something should be discussed:
1. How to test static function in mballoc.c
Currently, include mballoc-test.c in mballoc.c to test static function
in mballoc.c from mballoc-test.c.Not sure if there is any more elegant
way to test static function without touch mballoc.c.
2. How add mocks to function in mballoc.c which may issue IO to disk
Currently, KUNIT_STATIC_STUB_REDIRECT is added to functions as suggested
in kunit document [3]. However, this will trigger warnning "ISO C90
forbids mixed declarations and code".
3. How to simulate a block bitmap.
Currently, a fake buffer_head with bitmap data is returned, then no
futher change is needed.
If we simulate a block bitmap with an array of data structure like:
struct test_bitmap {
       unsigned int	start;
       unsigned int	len;
}
which is suggested by Theodore in [4], then we need to add mocks to
function which expected bitmap from bitmap_bh->b_data, like
mb_find_next_bit, mb_find_next_zero_bit and maybe more.

Would like to hear any suggestion! Thanks!

[1]
https://lore.kernel.org/linux-ext4/ZC3MoWn2UO6p+Swp@li-bb2b2a4c-3307-11b2-a85c-8fa5c3a69313.ibm.com/
[2]
https://lore.kernel.org/linux-ext4/9b35f3955a1d7b66bbd713eca1e63026e01f78c1.1679731817.git.ojaswin@linux.ibm.com
[3]
https://docs.kernel.org/dev-tools/kunit/api/functionredirection.html#c.KUNIT_STATIC_STUB_REDIRECT
[4]
https://lore.kernel.org/linux-ext4/20230317155047.GB3270589@mit.edu/

By the way, the "xfstest somke" passes. Please let me know if any more
test is needed.

Kemeng Shi (19):
  ext4: fix wrong unit use in ext4_mb_normalize_request
  ext4: fix unit mismatch in ext4_mb_new_blocks_simple
  ext4: fix wrong unit use in ext4_mb_find_by_goal
  ext4: treat stripe in block unit
  ext4: add EXT4_MB_HINT_GOAL_ONLY test in ext4_mb_use_preallocated
  ext4: remove ext4_block_group and ext4_block_group_offset declaration
  ext4: try all groups in ext4_mb_new_blocks_simple
  ext4: get block from bh before pass it to ext4_free_blocks_simple
    in ext4_free_blocks
  ext4: remove unsed parameter and unnecessary forward declaration of
    ext4_mb_new_blocks_simple
  ext4: fix wrong unit use in ext4_mb_clear_bb
  ext4: fix wrong unit use in ext4_mb_new_blocks
  ext4: factor out codes to update block bitmap and group descriptor on
    disk from ext4_mb_mark_bb
  ext4: call ext4_mb_mark_group_bb in ext4_free_blocks_simple
  ext4: extent ext4_mb_mark_group_bb to support allocation under journal
  ext4: call ext4_mb_mark_group_bb in ext4_mb_mark_diskspace_used
  ext4: call ext4_mb_mark_group_bb in ext4_mb_clear_bb
  ext4: call ext4_mb_mark_group_bb in ext4_group_add_blocks
  ext4: add some kunit stub for mballoc kunit test
  ext4: add first unit test for ext4_mb_new_blocks_simple in mballoc

 fs/ext4/balloc.c       |  13 +
 fs/ext4/ext4.h         |   4 -
 fs/ext4/mballoc-test.c | 319 +++++++++++++++++++
 fs/ext4/mballoc.c      | 703 +++++++++++++++++++----------------------
 fs/ext4/super.c        |  13 +
 5 files changed, 663 insertions(+), 389 deletions(-)
 create mode 100644 fs/ext4/mballoc-test.c

-- 
2.30.0


^ permalink raw reply	[flat|nested] 25+ messages in thread

* [PATCH v2 01/19] ext4: fix wrong unit use in ext4_mb_normalize_request
  2023-04-12 17:28 [PATCH v2 00/19] Fixes, cleanups and unit test for mballoc Kemeng Shi
@ 2023-04-12 17:28 ` Kemeng Shi
  2023-04-12 17:28 ` [PATCH v2 02/19] ext4: fix unit mismatch in ext4_mb_new_blocks_simple Kemeng Shi
                   ` (17 subsequent siblings)
  18 siblings, 0 replies; 25+ messages in thread
From: Kemeng Shi @ 2023-04-12 17:28 UTC (permalink / raw)
  To: tytso, adilger.kernel, ojaswin; +Cc: linux-ext4, linux-kernel, shikemeng

NRL_CHECK_SIZE will compare input req and size, so req and size should
be in same unit. Input req "fe_len" is in cluster unit while input
size "(8<<20)>>bsbits" is in block unit. Convert "fe_len" to block
unit to fix the mismatch.

Signed-off-by: Kemeng Shi <shikemeng@huaweicloud.com>
Reviewed-by: Ojaswin Mujoo <ojaswin@linux.ibm.com>
---
 fs/ext4/mballoc.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/fs/ext4/mballoc.c b/fs/ext4/mballoc.c
index 63a68cee36c6..6318c763a239 100644
--- a/fs/ext4/mballoc.c
+++ b/fs/ext4/mballoc.c
@@ -4056,7 +4056,7 @@ ext4_mb_normalize_request(struct ext4_allocation_context *ac,
 		start_off = ((loff_t)ac->ac_o_ex.fe_logical >>
 							(22 - bsbits)) << 22;
 		size = 4 * 1024 * 1024;
-	} else if (NRL_CHECK_SIZE(ac->ac_o_ex.fe_len,
+	} else if (NRL_CHECK_SIZE(EXT4_C2B(sbi, ac->ac_o_ex.fe_len),
 					(8<<20)>>bsbits, max, 8 * 1024)) {
 		start_off = ((loff_t)ac->ac_o_ex.fe_logical >>
 							(23 - bsbits)) << 23;
-- 
2.30.0


^ permalink raw reply related	[flat|nested] 25+ messages in thread

* [PATCH v2 02/19] ext4: fix unit mismatch in ext4_mb_new_blocks_simple
  2023-04-12 17:28 [PATCH v2 00/19] Fixes, cleanups and unit test for mballoc Kemeng Shi
  2023-04-12 17:28 ` [PATCH v2 01/19] ext4: fix wrong unit use in ext4_mb_normalize_request Kemeng Shi
@ 2023-04-12 17:28 ` Kemeng Shi
  2023-04-12 17:28 ` [PATCH v2 03/19] ext4: fix wrong unit use in ext4_mb_find_by_goal Kemeng Shi
                   ` (16 subsequent siblings)
  18 siblings, 0 replies; 25+ messages in thread
From: Kemeng Shi @ 2023-04-12 17:28 UTC (permalink / raw)
  To: tytso, adilger.kernel, ojaswin; +Cc: linux-ext4, linux-kernel, shikemeng

The "i" returned from mb_find_next_zero_bit is in cluster unit and we
need offset "block" corresponding to "i" in block unit. Convert "i" to
block unit to fix the unit mismatch.

Signed-off-by: Kemeng Shi <shikemeng@huaweicloud.com>
Reviewed-by: Ojaswin Mujoo <ojaswin@linux.ibm.com>
---
 fs/ext4/mballoc.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/fs/ext4/mballoc.c b/fs/ext4/mballoc.c
index 6318c763a239..7f695830621a 100644
--- a/fs/ext4/mballoc.c
+++ b/fs/ext4/mballoc.c
@@ -5761,6 +5761,7 @@ static ext4_fsblk_t ext4_mb_new_blocks_simple(handle_t *handle,
 {
 	struct buffer_head *bitmap_bh;
 	struct super_block *sb = ar->inode->i_sb;
+	struct ext4_sb_info *sbi = EXT4_SB(sb);
 	ext4_group_t group;
 	ext4_grpblk_t blkoff;
 	ext4_grpblk_t max = EXT4_CLUSTERS_PER_GROUP(sb);
@@ -5789,7 +5790,8 @@ static ext4_fsblk_t ext4_mb_new_blocks_simple(handle_t *handle,
 			if (i >= max)
 				break;
 			if (ext4_fc_replay_check_excluded(sb,
-				ext4_group_first_block_no(sb, group) + i)) {
+				ext4_group_first_block_no(sb, group) +
+				EXT4_C2B(sbi, i))) {
 				blkoff = i + 1;
 			} else
 				break;
@@ -5806,7 +5808,7 @@ static ext4_fsblk_t ext4_mb_new_blocks_simple(handle_t *handle,
 		return 0;
 	}
 
-	block = ext4_group_first_block_no(sb, group) + i;
+	block = ext4_group_first_block_no(sb, group) + EXT4_C2B(sbi, i);
 	ext4_mb_mark_bb(sb, block, 1, 1);
 	ar->len = 1;
 
-- 
2.30.0


^ permalink raw reply related	[flat|nested] 25+ messages in thread

* [PATCH v2 03/19] ext4: fix wrong unit use in ext4_mb_find_by_goal
  2023-04-12 17:28 [PATCH v2 00/19] Fixes, cleanups and unit test for mballoc Kemeng Shi
  2023-04-12 17:28 ` [PATCH v2 01/19] ext4: fix wrong unit use in ext4_mb_normalize_request Kemeng Shi
  2023-04-12 17:28 ` [PATCH v2 02/19] ext4: fix unit mismatch in ext4_mb_new_blocks_simple Kemeng Shi
@ 2023-04-12 17:28 ` Kemeng Shi
  2023-04-12 17:28 ` [PATCH v2 04/19] ext4: treat stripe in block unit Kemeng Shi
                   ` (15 subsequent siblings)
  18 siblings, 0 replies; 25+ messages in thread
From: Kemeng Shi @ 2023-04-12 17:28 UTC (permalink / raw)
  To: tytso, adilger.kernel, ojaswin; +Cc: linux-ext4, linux-kernel, shikemeng

We need start in block unit while fe_start is in cluster unit. Use
ext4_grp_offs_to_block helper to convert fe_start to get start in
block unit.

Signed-off-by: Kemeng Shi <shikemeng@huaweicloud.com>
Reviewed-by: Ojaswin Mujoo <ojaswin@linux.ibm.com>
---
 fs/ext4/mballoc.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/fs/ext4/mballoc.c b/fs/ext4/mballoc.c
index 7f695830621a..d6a4f6b8b8a4 100644
--- a/fs/ext4/mballoc.c
+++ b/fs/ext4/mballoc.c
@@ -2181,8 +2181,7 @@ int ext4_mb_find_by_goal(struct ext4_allocation_context *ac,
 	if (max >= ac->ac_g_ex.fe_len && ac->ac_g_ex.fe_len == sbi->s_stripe) {
 		ext4_fsblk_t start;
 
-		start = ext4_group_first_block_no(ac->ac_sb, e4b->bd_group) +
-			ex.fe_start;
+		start = ext4_grp_offs_to_block(ac->ac_sb, &ex);
 		/* use do_div to get remainder (would be 64-bit modulo) */
 		if (do_div(start, sbi->s_stripe) == 0) {
 			ac->ac_found++;
-- 
2.30.0


^ permalink raw reply related	[flat|nested] 25+ messages in thread

* [PATCH v2 04/19] ext4: treat stripe in block unit
  2023-04-12 17:28 [PATCH v2 00/19] Fixes, cleanups and unit test for mballoc Kemeng Shi
                   ` (2 preceding siblings ...)
  2023-04-12 17:28 ` [PATCH v2 03/19] ext4: fix wrong unit use in ext4_mb_find_by_goal Kemeng Shi
@ 2023-04-12 17:28 ` Kemeng Shi
  2023-04-12 17:28 ` [PATCH v2 05/19] ext4: add EXT4_MB_HINT_GOAL_ONLY test in ext4_mb_use_preallocated Kemeng Shi
                   ` (14 subsequent siblings)
  18 siblings, 0 replies; 25+ messages in thread
From: Kemeng Shi @ 2023-04-12 17:28 UTC (permalink / raw)
  To: tytso, adilger.kernel, ojaswin; +Cc: linux-ext4, linux-kernel, shikemeng

Stripe is misused in block unit and in cluster unit in different code
paths. User awared of stripe maybe not awared of bigalloc feature, so
treat stripe only in block unit to fix this.
Besides, it's hard to get stripe aligned blocks (start and length are both
aligned with stripe) if stripe is not aligned with cluster, just disable
stripe and alert user in this case to simpfy the code and avoid
unnecessary work to get stripe aligned blocks which likely to be failed.

Signed-off-by: Kemeng Shi <shikemeng@huaweicloud.com>
Reviewed-by: Ojaswin Mujoo <ojaswin@linux.ibm.com>
---
 fs/ext4/mballoc.c | 18 +++++++++++-------
 fs/ext4/super.c   | 13 +++++++++++++
 2 files changed, 24 insertions(+), 7 deletions(-)

diff --git a/fs/ext4/mballoc.c b/fs/ext4/mballoc.c
index d6a4f6b8b8a4..b75f16d2fdfc 100644
--- a/fs/ext4/mballoc.c
+++ b/fs/ext4/mballoc.c
@@ -2178,7 +2178,8 @@ int ext4_mb_find_by_goal(struct ext4_allocation_context *ac,
 			     ac->ac_g_ex.fe_len, &ex);
 	ex.fe_logical = 0xDEADFA11; /* debug value */
 
-	if (max >= ac->ac_g_ex.fe_len && ac->ac_g_ex.fe_len == sbi->s_stripe) {
+	if (max >= ac->ac_g_ex.fe_len &&
+	    ac->ac_g_ex.fe_len == EXT4_B2C(sbi, sbi->s_stripe)) {
 		ext4_fsblk_t start;
 
 		start = ext4_grp_offs_to_block(ac->ac_sb, &ex);
@@ -2343,7 +2344,7 @@ void ext4_mb_scan_aligned(struct ext4_allocation_context *ac,
 	struct ext4_free_extent ex;
 	ext4_fsblk_t first_group_block;
 	ext4_fsblk_t a;
-	ext4_grpblk_t i;
+	ext4_grpblk_t i, stripe;
 	int max;
 
 	BUG_ON(sbi->s_stripe == 0);
@@ -2355,10 +2356,12 @@ void ext4_mb_scan_aligned(struct ext4_allocation_context *ac,
 	do_div(a, sbi->s_stripe);
 	i = (a * sbi->s_stripe) - first_group_block;
 
+	stripe = EXT4_B2C(sbi, sbi->s_stripe);
+	i = EXT4_B2C(sbi, i);
 	while (i < EXT4_CLUSTERS_PER_GROUP(sb)) {
 		if (!mb_test_bit(i, bitmap)) {
-			max = mb_find_extent(e4b, i, sbi->s_stripe, &ex);
-			if (max >= sbi->s_stripe) {
+			max = mb_find_extent(e4b, i, stripe, &ex);
+			if (max >= stripe) {
 				ac->ac_found++;
 				ex.fe_logical = 0xDEADF00D; /* debug value */
 				ac->ac_b_ex = ex;
@@ -2366,7 +2369,7 @@ void ext4_mb_scan_aligned(struct ext4_allocation_context *ac,
 				break;
 			}
 		}
-		i += sbi->s_stripe;
+		i += stripe;
 	}
 }
 
@@ -2727,7 +2730,8 @@ ext4_mb_regular_allocator(struct ext4_allocation_context *ac)
 			if (cr == 0)
 				ext4_mb_simple_scan_group(ac, &e4b);
 			else if (cr == 1 && sbi->s_stripe &&
-					!(ac->ac_g_ex.fe_len % sbi->s_stripe))
+				 !(ac->ac_g_ex.fe_len %
+				 EXT4_B2C(sbi, sbi->s_stripe)))
 				ext4_mb_scan_aligned(ac, &e4b);
 			else
 				ext4_mb_complex_scan_group(ac, &e4b);
@@ -3441,7 +3445,7 @@ int ext4_mb_init(struct super_block *sb)
 	 */
 	if (sbi->s_stripe > 1) {
 		sbi->s_mb_group_prealloc = roundup(
-			sbi->s_mb_group_prealloc, sbi->s_stripe);
+			sbi->s_mb_group_prealloc, EXT4_B2C(sbi, sbi->s_stripe));
 	}
 
 	sbi->s_locality_groups = alloc_percpu(struct ext4_locality_group);
diff --git a/fs/ext4/super.c b/fs/ext4/super.c
index f226f8ab469b..0a5bf375df5c 100644
--- a/fs/ext4/super.c
+++ b/fs/ext4/super.c
@@ -5231,6 +5231,19 @@ static int __ext4_fill_super(struct fs_context *fc, struct super_block *sb)
 		goto failed_mount3;
 
 	sbi->s_stripe = ext4_get_stripe_size(sbi);
+	/*
+	 * It's hard to get stripe aligned blocks if stripe is not aligned with
+	 * cluster, just disable stripe and alert user to simpfy code and avoid
+	 * stripe aligned allocation which will rarely successes.
+	 */
+	if (sbi->s_stripe > 0 && sbi->s_cluster_ratio > 1 &&
+	    sbi->s_stripe % sbi->s_cluster_ratio != 0) {
+		ext4_msg(sb, KERN_WARNING,
+			 "stripe (%lu) is not aligned with cluster size (%u), "
+			 "stripe is disabled",
+			 sbi->s_stripe, sbi->s_cluster_ratio);
+		sbi->s_stripe = 0;
+	}
 	sbi->s_extent_max_zeroout_kb = 32;
 
 	/*
-- 
2.30.0


^ permalink raw reply related	[flat|nested] 25+ messages in thread

* [PATCH v2 05/19] ext4: add EXT4_MB_HINT_GOAL_ONLY test in ext4_mb_use_preallocated
  2023-04-12 17:28 [PATCH v2 00/19] Fixes, cleanups and unit test for mballoc Kemeng Shi
                   ` (3 preceding siblings ...)
  2023-04-12 17:28 ` [PATCH v2 04/19] ext4: treat stripe in block unit Kemeng Shi
@ 2023-04-12 17:28 ` Kemeng Shi
  2023-04-12 17:28 ` [PATCH v2 06/19] ext4: remove ext4_block_group and ext4_block_group_offset declaration Kemeng Shi
                   ` (13 subsequent siblings)
  18 siblings, 0 replies; 25+ messages in thread
From: Kemeng Shi @ 2023-04-12 17:28 UTC (permalink / raw)
  To: tytso, adilger.kernel, ojaswin; +Cc: linux-ext4, linux-kernel, shikemeng

ext4_mb_use_preallocated will ignore the demand to alloc goal blocks,
although the EXT4_MB_HINT_GOAL_ONLY is requested.
For group pa, ext4_mb_group_or_file will not set EXT4_MB_HINT_GROUP_ALLOC
if EXT4_MB_HINT_GOAL_ONLY is set. So we will not alloc goal blocks from
group pa if EXT4_MB_HINT_GOAL_ONLY is set.
For inode pa, ext4_mb_pa_goal_check is added to check if free extent in
found inode pa meets goal blocks when EXT4_MB_HINT_GOAL_ONLY is set.

Signed-off-by: Kemeng Shi <shikemeng@huaweicloud.com>
Suggested-by: Ojaswin Mujoo <ojaswin@linux.ibm.com>
---
 fs/ext4/mballoc.c | 34 +++++++++++++++++++++++++++++++++-
 1 file changed, 33 insertions(+), 1 deletion(-)

diff --git a/fs/ext4/mballoc.c b/fs/ext4/mballoc.c
index b75f16d2fdfc..be1115ea1800 100644
--- a/fs/ext4/mballoc.c
+++ b/fs/ext4/mballoc.c
@@ -4351,6 +4351,37 @@ ext4_mb_check_group_pa(ext4_fsblk_t goal_block,
 	return pa;
 }
 
+/*
+ * check if found pa meets EXT4_MB_HINT_GOAL_ONLY
+ */
+static bool
+ext4_mb_pa_goal_check(struct ext4_allocation_context *ac,
+		      struct ext4_prealloc_space *pa)
+{
+	struct ext4_sb_info *sbi = EXT4_SB(ac->ac_sb);
+	ext4_fsblk_t start;
+
+	if (likely(!(ac->ac_flags & EXT4_MB_HINT_GOAL_ONLY)))
+		return true;
+
+	/*
+	 * If EXT4_MB_HINT_GOAL_ONLY is set, ac_g_ex will not be adjusted
+	 * in ext4_mb_normalize_request and will keep same with ac_o_ex
+	 * from ext4_mb_initialize_context. Choose ac_g_ex here to keep
+	 * consistent with ext4_mb_find_by_goal.
+	 */
+	start = pa->pa_pstart +
+		(ac->ac_g_ex.fe_logical - pa->pa_lstart);
+	if (ext4_grp_offs_to_block(ac->ac_sb, &ac->ac_g_ex) != start)
+		return false;
+
+	if (ac->ac_g_ex.fe_len > pa->pa_len -
+	    EXT4_B2C(sbi, ac->ac_g_ex.fe_logical - pa->pa_lstart))
+		return false;
+
+	return true;
+}
+
 /*
  * search goal blocks in preallocated space
  */
@@ -4387,7 +4418,8 @@ ext4_mb_use_preallocated(struct ext4_allocation_context *ac)
 
 		/* found preallocated blocks, use them */
 		spin_lock(&pa->pa_lock);
-		if (pa->pa_deleted == 0 && pa->pa_free) {
+		if (pa->pa_deleted == 0 && pa->pa_free &&
+		    likely(ext4_mb_pa_goal_check(ac, pa))) {
 			atomic_inc(&pa->pa_count);
 			ext4_mb_use_inode_pa(ac, pa);
 			spin_unlock(&pa->pa_lock);
-- 
2.30.0


^ permalink raw reply related	[flat|nested] 25+ messages in thread

* [PATCH v2 06/19] ext4: remove ext4_block_group and ext4_block_group_offset declaration
  2023-04-12 17:28 [PATCH v2 00/19] Fixes, cleanups and unit test for mballoc Kemeng Shi
                   ` (4 preceding siblings ...)
  2023-04-12 17:28 ` [PATCH v2 05/19] ext4: add EXT4_MB_HINT_GOAL_ONLY test in ext4_mb_use_preallocated Kemeng Shi
@ 2023-04-12 17:28 ` Kemeng Shi
  2023-04-12 17:28 ` [PATCH v2 07/19] ext4: try all groups in ext4_mb_new_blocks_simple Kemeng Shi
                   ` (12 subsequent siblings)
  18 siblings, 0 replies; 25+ messages in thread
From: Kemeng Shi @ 2023-04-12 17:28 UTC (permalink / raw)
  To: tytso, adilger.kernel, ojaswin; +Cc: linux-ext4, linux-kernel, shikemeng

For ext4_block_group and ext4_block_group_offset, there are only
declaration without definition. Just remove them.

Signed-off-by: Kemeng Shi <shikemeng@huaweicloud.com>
Reviewed-by: Ojaswin Mujoo <ojaswin@linux.ibm.com>
---
 fs/ext4/ext4.h | 4 ----
 1 file changed, 4 deletions(-)

diff --git a/fs/ext4/ext4.h b/fs/ext4/ext4.h
index 9b2cfc32cf78..f25f13a357de 100644
--- a/fs/ext4/ext4.h
+++ b/fs/ext4/ext4.h
@@ -2697,10 +2697,6 @@ extern void ext4_get_group_no_and_offset(struct super_block *sb,
 extern ext4_group_t ext4_get_group_number(struct super_block *sb,
 					  ext4_fsblk_t block);
 
-extern unsigned int ext4_block_group(struct super_block *sb,
-			ext4_fsblk_t blocknr);
-extern ext4_grpblk_t ext4_block_group_offset(struct super_block *sb,
-			ext4_fsblk_t blocknr);
 extern int ext4_bg_has_super(struct super_block *sb, ext4_group_t group);
 extern unsigned long ext4_bg_num_gdb(struct super_block *sb,
 			ext4_group_t group);
-- 
2.30.0


^ permalink raw reply related	[flat|nested] 25+ messages in thread

* [PATCH v2 07/19] ext4: try all groups in ext4_mb_new_blocks_simple
  2023-04-12 17:28 [PATCH v2 00/19] Fixes, cleanups and unit test for mballoc Kemeng Shi
                   ` (5 preceding siblings ...)
  2023-04-12 17:28 ` [PATCH v2 06/19] ext4: remove ext4_block_group and ext4_block_group_offset declaration Kemeng Shi
@ 2023-04-12 17:28 ` Kemeng Shi
  2023-04-12 17:28 ` [PATCH v2 08/19] ext4: get block from bh before pass it to ext4_free_blocks_simple in ext4_free_blocks Kemeng Shi
                   ` (11 subsequent siblings)
  18 siblings, 0 replies; 25+ messages in thread
From: Kemeng Shi @ 2023-04-12 17:28 UTC (permalink / raw)
  To: tytso, adilger.kernel, ojaswin; +Cc: linux-ext4, linux-kernel, shikemeng

ext4_mb_new_blocks_simple ignores the group before goal, so it will fail
if free blocks reside in group before goal. Try all groups to avoid
unexpected failure.
Search finishes either if any free block is found or if no available
blocks are found. Simpliy check "i >= max" to distinguish the above
cases.

Signed-off-by: Kemeng Shi <shikemeng@huaweicloud.com>
Suggested-by: Theodore Ts'o <tytso@mit.edu>
Reviewed-by: Ojaswin Mujoo <ojaswin@linux.ibm.com>
---
 fs/ext4/mballoc.c | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/fs/ext4/mballoc.c b/fs/ext4/mballoc.c
index be1115ea1800..b9a7b669b97d 100644
--- a/fs/ext4/mballoc.c
+++ b/fs/ext4/mballoc.c
@@ -5797,7 +5797,7 @@ static ext4_fsblk_t ext4_mb_new_blocks_simple(handle_t *handle,
 	struct buffer_head *bitmap_bh;
 	struct super_block *sb = ar->inode->i_sb;
 	struct ext4_sb_info *sbi = EXT4_SB(sb);
-	ext4_group_t group;
+	ext4_group_t group, nr;
 	ext4_grpblk_t blkoff;
 	ext4_grpblk_t max = EXT4_CLUSTERS_PER_GROUP(sb);
 	ext4_grpblk_t i = 0;
@@ -5811,7 +5811,7 @@ static ext4_fsblk_t ext4_mb_new_blocks_simple(handle_t *handle,
 
 	ar->len = 0;
 	ext4_get_group_no_and_offset(sb, goal, &group, &blkoff);
-	for (; group < ext4_get_groups_count(sb); group++) {
+	for (nr = ext4_get_groups_count(sb); nr > 0; nr--) {
 		bitmap_bh = ext4_read_block_bitmap(sb, group);
 		if (IS_ERR(bitmap_bh)) {
 			*errp = PTR_ERR(bitmap_bh);
@@ -5835,10 +5835,13 @@ static ext4_fsblk_t ext4_mb_new_blocks_simple(handle_t *handle,
 		if (i < max)
 			break;
 
+		if (++group >= ext4_get_groups_count(sb))
+			group = 0;
+
 		blkoff = 0;
 	}
 
-	if (group >= ext4_get_groups_count(sb) || i >= max) {
+	if (i >= max) {
 		*errp = -ENOSPC;
 		return 0;
 	}
-- 
2.30.0


^ permalink raw reply related	[flat|nested] 25+ messages in thread

* [PATCH v2 08/19] ext4: get block from bh before pass it to ext4_free_blocks_simple in ext4_free_blocks
  2023-04-12 17:28 [PATCH v2 00/19] Fixes, cleanups and unit test for mballoc Kemeng Shi
                   ` (6 preceding siblings ...)
  2023-04-12 17:28 ` [PATCH v2 07/19] ext4: try all groups in ext4_mb_new_blocks_simple Kemeng Shi
@ 2023-04-12 17:28 ` Kemeng Shi
  2023-04-12 17:28 ` [PATCH v2 09/19] ext4: remove unsed parameter and unnecessary forward declaration of ext4_mb_new_blocks_simple Kemeng Shi
                   ` (10 subsequent siblings)
  18 siblings, 0 replies; 25+ messages in thread
From: Kemeng Shi @ 2023-04-12 17:28 UTC (permalink / raw)
  To: tytso, adilger.kernel, ojaswin; +Cc: linux-ext4, linux-kernel, shikemeng

ext4_free_blocks will retrieve block from bh if block parameter is zero.
Retrieve block before ext4_free_blocks_simple to avoid potentially
passing wrong block to ext4_free_blocks_simple.

Signed-off-by: Kemeng Shi <shikemeng@huaweicloud.com>
---
 fs/ext4/mballoc.c | 13 +++++++------
 1 file changed, 7 insertions(+), 6 deletions(-)

diff --git a/fs/ext4/mballoc.c b/fs/ext4/mballoc.c
index b9a7b669b97d..ca11d0aa8a59 100644
--- a/fs/ext4/mballoc.c
+++ b/fs/ext4/mballoc.c
@@ -6117,12 +6117,6 @@ void ext4_free_blocks(handle_t *handle, struct inode *inode,
 
 	sbi = EXT4_SB(sb);
 
-	if (sbi->s_mount_state & EXT4_FC_REPLAY) {
-		ext4_free_blocks_simple(inode, block, count);
-		return;
-	}
-
-	might_sleep();
 	if (bh) {
 		if (block)
 			BUG_ON(block != bh->b_blocknr);
@@ -6130,6 +6124,13 @@ void ext4_free_blocks(handle_t *handle, struct inode *inode,
 			block = bh->b_blocknr;
 	}
 
+	if (sbi->s_mount_state & EXT4_FC_REPLAY) {
+		ext4_free_blocks_simple(inode, block, count);
+		return;
+	}
+
+	might_sleep();
+
 	if (!(flags & EXT4_FREE_BLOCKS_VALIDATED) &&
 	    !ext4_inode_block_valid(inode, block, count)) {
 		ext4_error(sb, "Freeing blocks not in datazone - "
-- 
2.30.0


^ permalink raw reply related	[flat|nested] 25+ messages in thread

* [PATCH v2 09/19] ext4: remove unsed parameter and unnecessary forward declaration of ext4_mb_new_blocks_simple
  2023-04-12 17:28 [PATCH v2 00/19] Fixes, cleanups and unit test for mballoc Kemeng Shi
                   ` (7 preceding siblings ...)
  2023-04-12 17:28 ` [PATCH v2 08/19] ext4: get block from bh before pass it to ext4_free_blocks_simple in ext4_free_blocks Kemeng Shi
@ 2023-04-12 17:28 ` Kemeng Shi
  2023-04-12 17:28 ` [PATCH v2 10/19] ext4: fix wrong unit use in ext4_mb_clear_bb Kemeng Shi
                   ` (9 subsequent siblings)
  18 siblings, 0 replies; 25+ messages in thread
From: Kemeng Shi @ 2023-04-12 17:28 UTC (permalink / raw)
  To: tytso, adilger.kernel, ojaswin; +Cc: linux-ext4, linux-kernel, shikemeng

Two cleanups for ext4_mb_new_blocks_simple:
Remove unsed parameter handle of ext4_mb_new_blocks_simple.
Move ext4_mb_new_blocks_simple definition before ext4_mb_new_blocks to
remove unnecessary forward declaration of ext4_mb_new_blocks_simple.

Signed-off-by: Kemeng Shi <shikemeng@huaweicloud.com>
---
 fs/ext4/mballoc.c | 137 +++++++++++++++++++++++-----------------------
 1 file changed, 67 insertions(+), 70 deletions(-)

diff --git a/fs/ext4/mballoc.c b/fs/ext4/mballoc.c
index ca11d0aa8a59..2bbfded78093 100644
--- a/fs/ext4/mballoc.c
+++ b/fs/ext4/mballoc.c
@@ -5536,8 +5536,72 @@ static bool ext4_mb_discard_preallocations_should_retry(struct super_block *sb,
 	return ret;
 }
 
-static ext4_fsblk_t ext4_mb_new_blocks_simple(handle_t *handle,
-				struct ext4_allocation_request *ar, int *errp);
+/*
+ * Simple allocator for Ext4 fast commit replay path. It searches for blocks
+ * linearly starting at the goal block and also excludes the blocks which
+ * are going to be in use after fast commit replay.
+ */
+static ext4_fsblk_t
+ext4_mb_new_blocks_simple(struct ext4_allocation_request *ar, int *errp)
+{
+	struct buffer_head *bitmap_bh;
+	struct super_block *sb = ar->inode->i_sb;
+	struct ext4_sb_info *sbi = EXT4_SB(sb);
+	ext4_group_t group, nr;
+	ext4_grpblk_t blkoff;
+	ext4_grpblk_t max = EXT4_CLUSTERS_PER_GROUP(sb);
+	ext4_grpblk_t i = 0;
+	ext4_fsblk_t goal, block;
+	struct ext4_super_block *es = EXT4_SB(sb)->s_es;
+
+	goal = ar->goal;
+	if (goal < le32_to_cpu(es->s_first_data_block) ||
+			goal >= ext4_blocks_count(es))
+		goal = le32_to_cpu(es->s_first_data_block);
+
+	ar->len = 0;
+	ext4_get_group_no_and_offset(sb, goal, &group, &blkoff);
+	for (nr = ext4_get_groups_count(sb); nr > 0; nr--) {
+		bitmap_bh = ext4_read_block_bitmap(sb, group);
+		if (IS_ERR(bitmap_bh)) {
+			*errp = PTR_ERR(bitmap_bh);
+			pr_warn("Failed to read block bitmap\n");
+			return 0;
+		}
+
+		while (1) {
+			i = mb_find_next_zero_bit(bitmap_bh->b_data, max,
+						blkoff);
+			if (i >= max)
+				break;
+			if (ext4_fc_replay_check_excluded(sb,
+				ext4_group_first_block_no(sb, group) +
+				EXT4_C2B(sbi, i))) {
+				blkoff = i + 1;
+			} else
+				break;
+		}
+		brelse(bitmap_bh);
+		if (i < max)
+			break;
+
+		if (++group >= ext4_get_groups_count(sb))
+			group = 0;
+
+		blkoff = 0;
+	}
+
+	if (i >= max) {
+		*errp = -ENOSPC;
+		return 0;
+	}
+
+	block = ext4_group_first_block_no(sb, group) + EXT4_C2B(sbi, i);
+	ext4_mb_mark_bb(sb, block, 1, 1);
+	ar->len = 1;
+
+	return block;
+}
 
 /*
  * Main entry point into mballoc to allocate blocks
@@ -5562,7 +5626,7 @@ ext4_fsblk_t ext4_mb_new_blocks(handle_t *handle,
 
 	trace_ext4_request_blocks(ar);
 	if (sbi->s_mount_state & EXT4_FC_REPLAY)
-		return ext4_mb_new_blocks_simple(handle, ar, errp);
+		return ext4_mb_new_blocks_simple(ar, errp);
 
 	/* Allow to use superuser reservation for quota file */
 	if (ext4_is_quota_file(ar->inode))
@@ -5786,73 +5850,6 @@ ext4_mb_free_metadata(handle_t *handle, struct ext4_buddy *e4b,
 	spin_unlock(&sbi->s_md_lock);
 }
 
-/*
- * Simple allocator for Ext4 fast commit replay path. It searches for blocks
- * linearly starting at the goal block and also excludes the blocks which
- * are going to be in use after fast commit replay.
- */
-static ext4_fsblk_t ext4_mb_new_blocks_simple(handle_t *handle,
-				struct ext4_allocation_request *ar, int *errp)
-{
-	struct buffer_head *bitmap_bh;
-	struct super_block *sb = ar->inode->i_sb;
-	struct ext4_sb_info *sbi = EXT4_SB(sb);
-	ext4_group_t group, nr;
-	ext4_grpblk_t blkoff;
-	ext4_grpblk_t max = EXT4_CLUSTERS_PER_GROUP(sb);
-	ext4_grpblk_t i = 0;
-	ext4_fsblk_t goal, block;
-	struct ext4_super_block *es = EXT4_SB(sb)->s_es;
-
-	goal = ar->goal;
-	if (goal < le32_to_cpu(es->s_first_data_block) ||
-			goal >= ext4_blocks_count(es))
-		goal = le32_to_cpu(es->s_first_data_block);
-
-	ar->len = 0;
-	ext4_get_group_no_and_offset(sb, goal, &group, &blkoff);
-	for (nr = ext4_get_groups_count(sb); nr > 0; nr--) {
-		bitmap_bh = ext4_read_block_bitmap(sb, group);
-		if (IS_ERR(bitmap_bh)) {
-			*errp = PTR_ERR(bitmap_bh);
-			pr_warn("Failed to read block bitmap\n");
-			return 0;
-		}
-
-		while (1) {
-			i = mb_find_next_zero_bit(bitmap_bh->b_data, max,
-						blkoff);
-			if (i >= max)
-				break;
-			if (ext4_fc_replay_check_excluded(sb,
-				ext4_group_first_block_no(sb, group) +
-				EXT4_C2B(sbi, i))) {
-				blkoff = i + 1;
-			} else
-				break;
-		}
-		brelse(bitmap_bh);
-		if (i < max)
-			break;
-
-		if (++group >= ext4_get_groups_count(sb))
-			group = 0;
-
-		blkoff = 0;
-	}
-
-	if (i >= max) {
-		*errp = -ENOSPC;
-		return 0;
-	}
-
-	block = ext4_group_first_block_no(sb, group) + EXT4_C2B(sbi, i);
-	ext4_mb_mark_bb(sb, block, 1, 1);
-	ar->len = 1;
-
-	return block;
-}
-
 static void ext4_free_blocks_simple(struct inode *inode, ext4_fsblk_t block,
 					unsigned long count)
 {
-- 
2.30.0


^ permalink raw reply related	[flat|nested] 25+ messages in thread

* [PATCH v2 10/19] ext4: fix wrong unit use in ext4_mb_clear_bb
  2023-04-12 17:28 [PATCH v2 00/19] Fixes, cleanups and unit test for mballoc Kemeng Shi
                   ` (8 preceding siblings ...)
  2023-04-12 17:28 ` [PATCH v2 09/19] ext4: remove unsed parameter and unnecessary forward declaration of ext4_mb_new_blocks_simple Kemeng Shi
@ 2023-04-12 17:28 ` Kemeng Shi
  2023-04-12 17:28 ` [PATCH v2 11/19] ext4: fix wrong unit use in ext4_mb_new_blocks Kemeng Shi
                   ` (8 subsequent siblings)
  18 siblings, 0 replies; 25+ messages in thread
From: Kemeng Shi @ 2023-04-12 17:28 UTC (permalink / raw)
  To: tytso, adilger.kernel, ojaswin; +Cc: linux-ext4, linux-kernel, shikemeng

Function ext4_issue_discard need count in cluster. Pass count_clusters
instead of count to fix the mismatch.

Signed-off-by: Kemeng Shi <shikemeng@huaweicloud.com>
---
 fs/ext4/mballoc.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/fs/ext4/mballoc.c b/fs/ext4/mballoc.c
index 2bbfded78093..eac91b2e3aa3 100644
--- a/fs/ext4/mballoc.c
+++ b/fs/ext4/mballoc.c
@@ -6029,8 +6029,8 @@ static void ext4_mb_clear_bb(handle_t *handle, struct inode *inode,
 		 * them with group lock_held
 		 */
 		if (test_opt(sb, DISCARD)) {
-			err = ext4_issue_discard(sb, block_group, bit, count,
-						 NULL);
+			err = ext4_issue_discard(sb, block_group, bit,
+						 count_clusters, NULL);
 			if (err && err != -EOPNOTSUPP)
 				ext4_msg(sb, KERN_WARNING, "discard request in"
 					 " group:%u block:%d count:%lu failed"
-- 
2.30.0


^ permalink raw reply related	[flat|nested] 25+ messages in thread

* [PATCH v2 11/19] ext4: fix wrong unit use in ext4_mb_new_blocks
  2023-04-12 17:28 [PATCH v2 00/19] Fixes, cleanups and unit test for mballoc Kemeng Shi
                   ` (9 preceding siblings ...)
  2023-04-12 17:28 ` [PATCH v2 10/19] ext4: fix wrong unit use in ext4_mb_clear_bb Kemeng Shi
@ 2023-04-12 17:28 ` Kemeng Shi
  2023-04-12 17:28 ` [PATCH v2 12/19] ext4: factor out codes to update block bitmap and group descriptor on disk from ext4_mb_mark_bb Kemeng Shi
                   ` (7 subsequent siblings)
  18 siblings, 0 replies; 25+ messages in thread
From: Kemeng Shi @ 2023-04-12 17:28 UTC (permalink / raw)
  To: tytso, adilger.kernel, ojaswin; +Cc: linux-ext4, linux-kernel, shikemeng

Function ext4_mb_new_blocks_simple needs count in cluster. Function
ext4_mb_new_blocks accepts count in block. Convert count to cluster
to fix the mismatch.

Signed-off-by: Kemeng Shi <shikemeng@huaweicloud.com>
---
 fs/ext4/mballoc.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/fs/ext4/mballoc.c b/fs/ext4/mballoc.c
index eac91b2e3aa3..11ea5feb6a0d 100644
--- a/fs/ext4/mballoc.c
+++ b/fs/ext4/mballoc.c
@@ -6122,7 +6122,7 @@ void ext4_free_blocks(handle_t *handle, struct inode *inode,
 	}
 
 	if (sbi->s_mount_state & EXT4_FC_REPLAY) {
-		ext4_free_blocks_simple(inode, block, count);
+		ext4_free_blocks_simple(inode, block, EXT4_NUM_B2C(sbi, count));
 		return;
 	}
 
-- 
2.30.0


^ permalink raw reply related	[flat|nested] 25+ messages in thread

* [PATCH v2 12/19] ext4: factor out codes to update block bitmap and group descriptor on disk from ext4_mb_mark_bb
  2023-04-12 17:28 [PATCH v2 00/19] Fixes, cleanups and unit test for mballoc Kemeng Shi
                   ` (10 preceding siblings ...)
  2023-04-12 17:28 ` [PATCH v2 11/19] ext4: fix wrong unit use in ext4_mb_new_blocks Kemeng Shi
@ 2023-04-12 17:28 ` Kemeng Shi
  2023-04-12 17:28 ` [PATCH v2 13/19] ext4: call ext4_mb_mark_group_bb in ext4_free_blocks_simple Kemeng Shi
                   ` (6 subsequent siblings)
  18 siblings, 0 replies; 25+ messages in thread
From: Kemeng Shi @ 2023-04-12 17:28 UTC (permalink / raw)
  To: tytso, adilger.kernel, ojaswin; +Cc: linux-ext4, linux-kernel, shikemeng

There are several reasons to add a general function to update block
bitmap and group descriptor on disk:
1. pair behavior of alloc/free bits. For example,
ext4_mb_new_blocks_simple will update free_clusters in struct flex_groups
in ext4_mb_mark_bb while ext4_free_blocks_simple forgets this.
2. remove repeat code to read from disk, update and write back to disk.
3. reduce future unit test mocks to catch real IO to update structure
on disk.

Signed-off-by: Kemeng Shi <shikemeng@huaweicloud.com>
---
 fs/ext4/mballoc.c | 157 +++++++++++++++++++++++++---------------------
 1 file changed, 87 insertions(+), 70 deletions(-)

diff --git a/fs/ext4/mballoc.c b/fs/ext4/mballoc.c
index 11ea5feb6a0d..b93260eaad92 100644
--- a/fs/ext4/mballoc.c
+++ b/fs/ext4/mballoc.c
@@ -3731,6 +3731,86 @@ void ext4_exit_mballoc(void)
 	ext4_groupinfo_destroy_slabs();
 }
 
+struct ext4_mark_context {
+	struct super_block *sb;
+	int state;
+};
+
+static int
+ext4_mb_mark_group_bb(struct ext4_mark_context *mc, ext4_group_t group,
+		      ext4_grpblk_t blkoff, ext4_grpblk_t len)
+{
+	struct super_block *sb = mc->sb;
+	struct ext4_sb_info *sbi = EXT4_SB(sb);
+	struct buffer_head *bitmap_bh = NULL;
+	struct ext4_group_desc *gdp;
+	struct buffer_head *gdp_bh;
+	int err;
+	unsigned int i, already, changed;
+
+	bitmap_bh = ext4_read_block_bitmap(sb, group);
+	if (IS_ERR(bitmap_bh))
+		return PTR_ERR(bitmap_bh);
+
+	err = -EIO;
+	gdp = ext4_get_group_desc(sb, group, &gdp_bh);
+	if (!gdp)
+		goto out_err;
+
+	ext4_lock_group(sb, group);
+	if (ext4_has_group_desc_csum(sb) &&
+	    (gdp->bg_flags & cpu_to_le16(EXT4_BG_BLOCK_UNINIT))) {
+		gdp->bg_flags &= cpu_to_le16(~EXT4_BG_BLOCK_UNINIT);
+		ext4_free_group_clusters_set(sb, gdp,
+			ext4_free_clusters_after_init(sb, group, gdp));
+	}
+
+	already = 0;
+	for (i = 0; i < len; i++)
+		if (mb_test_bit(blkoff + i, bitmap_bh->b_data) ==
+				mc->state)
+			already++;
+	changed = len - already;
+
+	if (mc->state) {
+		mb_set_bits(bitmap_bh->b_data, blkoff, len);
+		ext4_free_group_clusters_set(sb, gdp,
+			ext4_free_group_clusters(sb, gdp) - changed);
+	} else {
+		mb_clear_bits(bitmap_bh->b_data, blkoff, len);
+		ext4_free_group_clusters_set(sb, gdp,
+			ext4_free_group_clusters(sb, gdp) + changed);
+	}
+
+	ext4_block_bitmap_csum_set(sb, gdp, bitmap_bh);
+	ext4_group_desc_csum_set(sb, group, gdp);
+	ext4_unlock_group(sb, group);
+
+	if (sbi->s_log_groups_per_flex) {
+		ext4_group_t flex_group = ext4_flex_group(sbi, group);
+		struct flex_groups *fg = sbi_array_rcu_deref(sbi,
+					   s_flex_groups, flex_group);
+
+		if (mc->state)
+			atomic64_sub(changed, &fg->free_clusters);
+		else
+			atomic64_add(changed, &fg->free_clusters);
+	}
+
+	err = ext4_handle_dirty_metadata(NULL, NULL, bitmap_bh);
+	if (err)
+		goto out_err;
+	err = ext4_handle_dirty_metadata(NULL, NULL, gdp_bh);
+	if (err)
+		goto out_err;
+
+	sync_dirty_buffer(bitmap_bh);
+	sync_dirty_buffer(gdp_bh);
+
+out_err:
+	brelse(bitmap_bh);
+	return err;
+}
 
 /*
  * Check quota and mark chosen space (ac->ac_b_ex) non-free in bitmaps
@@ -3857,15 +3937,15 @@ ext4_mb_mark_diskspace_used(struct ext4_allocation_context *ac,
 void ext4_mb_mark_bb(struct super_block *sb, ext4_fsblk_t block,
 			int len, int state)
 {
-	struct buffer_head *bitmap_bh = NULL;
-	struct ext4_group_desc *gdp;
-	struct buffer_head *gdp_bh;
+	struct ext4_mark_context mc = {
+		.sb = sb,
+		.state = state,
+	};
 	struct ext4_sb_info *sbi = EXT4_SB(sb);
 	ext4_group_t group;
 	ext4_grpblk_t blkoff;
-	int i, err;
-	int already;
-	unsigned int clen, clen_changed, thisgrp_len;
+	int err;
+	unsigned int clen, thisgrp_len;
 
 	while (len > 0) {
 		ext4_get_group_no_and_offset(sb, block, &group, &blkoff);
@@ -3886,80 +3966,17 @@ void ext4_mb_mark_bb(struct super_block *sb, ext4_fsblk_t block,
 			ext4_error(sb, "Marking blocks in system zone - "
 				   "Block = %llu, len = %u",
 				   block, thisgrp_len);
-			bitmap_bh = NULL;
 			break;
 		}
 
-		bitmap_bh = ext4_read_block_bitmap(sb, group);
-		if (IS_ERR(bitmap_bh)) {
-			err = PTR_ERR(bitmap_bh);
-			bitmap_bh = NULL;
-			break;
-		}
-
-		err = -EIO;
-		gdp = ext4_get_group_desc(sb, group, &gdp_bh);
-		if (!gdp)
-			break;
-
-		ext4_lock_group(sb, group);
-		already = 0;
-		for (i = 0; i < clen; i++)
-			if (!mb_test_bit(blkoff + i, bitmap_bh->b_data) ==
-					 !state)
-				already++;
-
-		clen_changed = clen - already;
-		if (state)
-			mb_set_bits(bitmap_bh->b_data, blkoff, clen);
-		else
-			mb_clear_bits(bitmap_bh->b_data, blkoff, clen);
-		if (ext4_has_group_desc_csum(sb) &&
-		    (gdp->bg_flags & cpu_to_le16(EXT4_BG_BLOCK_UNINIT))) {
-			gdp->bg_flags &= cpu_to_le16(~EXT4_BG_BLOCK_UNINIT);
-			ext4_free_group_clusters_set(sb, gdp,
-			     ext4_free_clusters_after_init(sb, group, gdp));
-		}
-		if (state)
-			clen = ext4_free_group_clusters(sb, gdp) - clen_changed;
-		else
-			clen = ext4_free_group_clusters(sb, gdp) + clen_changed;
-
-		ext4_free_group_clusters_set(sb, gdp, clen);
-		ext4_block_bitmap_csum_set(sb, gdp, bitmap_bh);
-		ext4_group_desc_csum_set(sb, group, gdp);
-
-		ext4_unlock_group(sb, group);
-
-		if (sbi->s_log_groups_per_flex) {
-			ext4_group_t flex_group = ext4_flex_group(sbi, group);
-			struct flex_groups *fg = sbi_array_rcu_deref(sbi,
-						   s_flex_groups, flex_group);
-
-			if (state)
-				atomic64_sub(clen_changed, &fg->free_clusters);
-			else
-				atomic64_add(clen_changed, &fg->free_clusters);
-
-		}
-
-		err = ext4_handle_dirty_metadata(NULL, NULL, bitmap_bh);
-		if (err)
-			break;
-		sync_dirty_buffer(bitmap_bh);
-		err = ext4_handle_dirty_metadata(NULL, NULL, gdp_bh);
-		sync_dirty_buffer(gdp_bh);
+		err = ext4_mb_mark_group_bb(&mc, group, blkoff, clen);
 		if (err)
 			break;
 
 		block += thisgrp_len;
 		len -= thisgrp_len;
-		brelse(bitmap_bh);
 		BUG_ON(len < 0);
 	}
-
-	if (err)
-		brelse(bitmap_bh);
 }
 
 /*
-- 
2.30.0


^ permalink raw reply related	[flat|nested] 25+ messages in thread

* [PATCH v2 13/19] ext4: call ext4_mb_mark_group_bb in ext4_free_blocks_simple
  2023-04-12 17:28 [PATCH v2 00/19] Fixes, cleanups and unit test for mballoc Kemeng Shi
                   ` (11 preceding siblings ...)
  2023-04-12 17:28 ` [PATCH v2 12/19] ext4: factor out codes to update block bitmap and group descriptor on disk from ext4_mb_mark_bb Kemeng Shi
@ 2023-04-12 17:28 ` Kemeng Shi
  2023-04-12 17:28 ` [PATCH v2 14/19] ext4: extent ext4_mb_mark_group_bb to support allocation under journal Kemeng Shi
                   ` (5 subsequent siblings)
  18 siblings, 0 replies; 25+ messages in thread
From: Kemeng Shi @ 2023-04-12 17:28 UTC (permalink / raw)
  To: tytso, adilger.kernel, ojaswin; +Cc: linux-ext4, linux-kernel, shikemeng

call ext4_mb_mark_group_bb in ext4_free_blocks_simple to:
1. remove repeat code
2. pair update of free_clusters in ext4_mb_new_blocks_simple.
3. add missing ext4_lock_group/ext4_unlock_group protection.

Signed-off-by: Kemeng Shi <shikemeng@huaweicloud.com>
---
 fs/ext4/mballoc.c | 37 +++++--------------------------------
 1 file changed, 5 insertions(+), 32 deletions(-)

diff --git a/fs/ext4/mballoc.c b/fs/ext4/mballoc.c
index b93260eaad92..9a76d5a8bb73 100644
--- a/fs/ext4/mballoc.c
+++ b/fs/ext4/mballoc.c
@@ -5870,43 +5870,16 @@ ext4_mb_free_metadata(handle_t *handle, struct ext4_buddy *e4b,
 static void ext4_free_blocks_simple(struct inode *inode, ext4_fsblk_t block,
 					unsigned long count)
 {
-	struct buffer_head *bitmap_bh;
+	struct ext4_mark_context mc = {
+		.sb = inode->i_sb,
+		.state = 0,
+	};
 	struct super_block *sb = inode->i_sb;
-	struct ext4_group_desc *gdp;
-	struct buffer_head *gdp_bh;
 	ext4_group_t group;
 	ext4_grpblk_t blkoff;
-	int already_freed = 0, err, i;
 
 	ext4_get_group_no_and_offset(sb, block, &group, &blkoff);
-	bitmap_bh = ext4_read_block_bitmap(sb, group);
-	if (IS_ERR(bitmap_bh)) {
-		pr_warn("Failed to read block bitmap\n");
-		return;
-	}
-	gdp = ext4_get_group_desc(sb, group, &gdp_bh);
-	if (!gdp)
-		goto err_out;
-
-	for (i = 0; i < count; i++) {
-		if (!mb_test_bit(blkoff + i, bitmap_bh->b_data))
-			already_freed++;
-	}
-	mb_clear_bits(bitmap_bh->b_data, blkoff, count);
-	err = ext4_handle_dirty_metadata(NULL, NULL, bitmap_bh);
-	if (err)
-		goto err_out;
-	ext4_free_group_clusters_set(
-		sb, gdp, ext4_free_group_clusters(sb, gdp) +
-		count - already_freed);
-	ext4_block_bitmap_csum_set(sb, gdp, bitmap_bh);
-	ext4_group_desc_csum_set(sb, group, gdp);
-	ext4_handle_dirty_metadata(NULL, NULL, gdp_bh);
-	sync_dirty_buffer(bitmap_bh);
-	sync_dirty_buffer(gdp_bh);
-
-err_out:
-	brelse(bitmap_bh);
+	ext4_mb_mark_group_bb(&mc, group, blkoff, count);
 }
 
 /**
-- 
2.30.0


^ permalink raw reply related	[flat|nested] 25+ messages in thread

* [PATCH v2 14/19] ext4: extent ext4_mb_mark_group_bb to support allocation under journal
  2023-04-12 17:28 [PATCH v2 00/19] Fixes, cleanups and unit test for mballoc Kemeng Shi
                   ` (12 preceding siblings ...)
  2023-04-12 17:28 ` [PATCH v2 13/19] ext4: call ext4_mb_mark_group_bb in ext4_free_blocks_simple Kemeng Shi
@ 2023-04-12 17:28 ` Kemeng Shi
  2023-04-12 17:28 ` [PATCH v2 15/19] ext4: call ext4_mb_mark_group_bb in ext4_mb_mark_diskspace_used Kemeng Shi
                   ` (4 subsequent siblings)
  18 siblings, 0 replies; 25+ messages in thread
From: Kemeng Shi @ 2023-04-12 17:28 UTC (permalink / raw)
  To: tytso, adilger.kernel, ojaswin; +Cc: linux-ext4, linux-kernel, shikemeng

Previously, ext4_mb_mark_group_bb is only called under fast commit
replay path, so there is no valid handle when we update block bitmap
and group descriptor. This patch try to extent ext4_mb_mark_group_bb
to be used by code under journal. There are several improves:
1. add "handle_t *handle" to struct ext4_mark_context to accept handle
to journal block bitmap and group descriptor update inside
ext4_mb_mark_group_bb (the added journal caode is based on journal
code in ext4_mb_mark_diskspace_used where ext4_mb_mark_group_bb
is going to be used.)
2. add EXT4_MB_BITMAP_MARKED_CHECK flag to control check if bits in block
bitmap are already marked as allocation code under journal asserts that
all bits to be changed are not marked before.
3. add "ext4_grpblk_t changed" to struct ext4_mark_context to notify number
of bits in block bitmap has changed.

Signed-off-by: Kemeng Shi <shikemeng@huaweicloud.com>
---
 fs/ext4/mballoc.c | 59 ++++++++++++++++++++++++++++++++++++-----------
 1 file changed, 45 insertions(+), 14 deletions(-)

diff --git a/fs/ext4/mballoc.c b/fs/ext4/mballoc.c
index 9a76d5a8bb73..fb163b61ac02 100644
--- a/fs/ext4/mballoc.c
+++ b/fs/ext4/mballoc.c
@@ -3731,32 +3731,54 @@ void ext4_exit_mballoc(void)
 	ext4_groupinfo_destroy_slabs();
 }
 
+#define EXT4_MB_BITMAP_MARKED_CHECK 0x0001
+#define EXT4_MB_SYNC_UPDATE 0x0002
 struct ext4_mark_context {
+	handle_t *handle;
 	struct super_block *sb;
 	int state;
+	ext4_grpblk_t changed;
 };
 
 static int
 ext4_mb_mark_group_bb(struct ext4_mark_context *mc, ext4_group_t group,
-		      ext4_grpblk_t blkoff, ext4_grpblk_t len)
+		      ext4_grpblk_t blkoff, ext4_grpblk_t len, int flags)
 {
+	handle_t *handle = mc->handle;
 	struct super_block *sb = mc->sb;
 	struct ext4_sb_info *sbi = EXT4_SB(sb);
 	struct buffer_head *bitmap_bh = NULL;
 	struct ext4_group_desc *gdp;
 	struct buffer_head *gdp_bh;
 	int err;
-	unsigned int i, already, changed;
+	unsigned int i, already, changed = len;
 
+	mc->changed = 0;
 	bitmap_bh = ext4_read_block_bitmap(sb, group);
 	if (IS_ERR(bitmap_bh))
 		return PTR_ERR(bitmap_bh);
 
+	if (handle) {
+		BUFFER_TRACE(bitmap_bh, "getting write access");
+		err = ext4_journal_get_write_access(handle, sb, bitmap_bh,
+						    EXT4_JTR_NONE);
+		if (err)
+			goto out_err;
+	}
+
 	err = -EIO;
 	gdp = ext4_get_group_desc(sb, group, &gdp_bh);
 	if (!gdp)
 		goto out_err;
 
+	if (handle) {
+		BUFFER_TRACE(gdp_bh, "get_write_access");
+		err = ext4_journal_get_write_access(handle, sb, gdp_bh,
+						    EXT4_JTR_NONE);
+		if (err)
+			goto out_err;
+	}
+
 	ext4_lock_group(sb, group);
 	if (ext4_has_group_desc_csum(sb) &&
 	    (gdp->bg_flags & cpu_to_le16(EXT4_BG_BLOCK_UNINIT))) {
@@ -3765,12 +3787,14 @@ ext4_mb_mark_group_bb(struct ext4_mark_context *mc, ext4_group_t group,
 			ext4_free_clusters_after_init(sb, group, gdp));
 	}
 
-	already = 0;
-	for (i = 0; i < len; i++)
-		if (mb_test_bit(blkoff + i, bitmap_bh->b_data) ==
-				mc->state)
-			already++;
-	changed = len - already;
+	if (flags & EXT4_MB_BITMAP_MARKED_CHECK) {
+		already = 0;
+		for (i = 0; i < len; i++)
+			if (mb_test_bit(blkoff + i, bitmap_bh->b_data) ==
+					mc->state)
+				already++;
+		changed = len - already;
+	}
 
 	if (mc->state) {
 		mb_set_bits(bitmap_bh->b_data, blkoff, len);
@@ -3785,6 +3809,7 @@ ext4_mb_mark_group_bb(struct ext4_mark_context *mc, ext4_group_t group,
 	ext4_block_bitmap_csum_set(sb, gdp, bitmap_bh);
 	ext4_group_desc_csum_set(sb, group, gdp);
 	ext4_unlock_group(sb, group);
+	mc->changed = changed;
 
 	if (sbi->s_log_groups_per_flex) {
 		ext4_group_t flex_group = ext4_flex_group(sbi, group);
@@ -3797,15 +3822,17 @@ ext4_mb_mark_group_bb(struct ext4_mark_context *mc, ext4_group_t group,
 			atomic64_add(changed, &fg->free_clusters);
 	}
 
-	err = ext4_handle_dirty_metadata(NULL, NULL, bitmap_bh);
+	err = ext4_handle_dirty_metadata(handle, NULL, bitmap_bh);
 	if (err)
 		goto out_err;
-	err = ext4_handle_dirty_metadata(NULL, NULL, gdp_bh);
+	err = ext4_handle_dirty_metadata(handle, NULL, gdp_bh);
 	if (err)
 		goto out_err;
 
-	sync_dirty_buffer(bitmap_bh);
-	sync_dirty_buffer(gdp_bh);
+	if (flags & EXT4_MB_SYNC_UPDATE) {
+		sync_dirty_buffer(bitmap_bh);
+		sync_dirty_buffer(gdp_bh);
+	}
 
 out_err:
 	brelse(bitmap_bh);
@@ -3969,7 +3996,9 @@ void ext4_mb_mark_bb(struct super_block *sb, ext4_fsblk_t block,
 			break;
 		}
 
-		err = ext4_mb_mark_group_bb(&mc, group, blkoff, clen);
+		err = ext4_mb_mark_group_bb(&mc, group, blkoff, clen,
+					    EXT4_MB_BITMAP_MARKED_CHECK |
+					    EXT4_MB_SYNC_UPDATE);
 		if (err)
 			break;
 
@@ -5879,7 +5908,9 @@ static void ext4_free_blocks_simple(struct inode *inode, ext4_fsblk_t block,
 	ext4_grpblk_t blkoff;
 
 	ext4_get_group_no_and_offset(sb, block, &group, &blkoff);
-	ext4_mb_mark_group_bb(&mc, group, blkoff, count);
+	ext4_mb_mark_group_bb(&mc, group, blkoff, count,
+			      EXT4_MB_BITMAP_MARKED_CHECK |
+			      EXT4_MB_SYNC_UPDATE);
 }
 
 /**
-- 
2.30.0


^ permalink raw reply related	[flat|nested] 25+ messages in thread

* [PATCH v2 15/19] ext4: call ext4_mb_mark_group_bb in ext4_mb_mark_diskspace_used
  2023-04-12 17:28 [PATCH v2 00/19] Fixes, cleanups and unit test for mballoc Kemeng Shi
                   ` (13 preceding siblings ...)
  2023-04-12 17:28 ` [PATCH v2 14/19] ext4: extent ext4_mb_mark_group_bb to support allocation under journal Kemeng Shi
@ 2023-04-12 17:28 ` Kemeng Shi
  2023-04-12 17:28 ` [PATCH v2 16/19] ext4: call ext4_mb_mark_group_bb in ext4_mb_clear_bb Kemeng Shi
                   ` (3 subsequent siblings)
  18 siblings, 0 replies; 25+ messages in thread
From: Kemeng Shi @ 2023-04-12 17:28 UTC (permalink / raw)
  To: tytso, adilger.kernel, ojaswin; +Cc: linux-ext4, linux-kernel, shikemeng

call ext4_mb_mark_group_bb in ext4_mb_mark_diskspace_used to:
1. remove repeat code to normally update bitmap and group descriptor
on disk.
2. call ext4_mb_mark_group_bb instead of only setting bits in block bitmap
to fix the bitmap. Function ext4_mb_mark_group_bb will also update
checksum of bitmap and other counter along with the bit change to keep
the cosistent with bit change or block bitmap will be marked corrupted as
checksum of bitmap is in inconsistent state.

Signed-off-by: Kemeng Shi <shikemeng@huaweicloud.com>
---
 fs/ext4/mballoc.c | 90 +++++++++++++----------------------------------
 1 file changed, 24 insertions(+), 66 deletions(-)

diff --git a/fs/ext4/mballoc.c b/fs/ext4/mballoc.c
index fb163b61ac02..3d31ad486a0c 100644
--- a/fs/ext4/mballoc.c
+++ b/fs/ext4/mballoc.c
@@ -3847,9 +3847,12 @@ static noinline_for_stack int
 ext4_mb_mark_diskspace_used(struct ext4_allocation_context *ac,
 				handle_t *handle, unsigned int reserv_clstrs)
 {
-	struct buffer_head *bitmap_bh = NULL;
+	struct ext4_mark_context mc = {
+		.handle = handle,
+		.sb = ac->ac_sb,
+		.state = 1,
+	};
 	struct ext4_group_desc *gdp;
-	struct buffer_head *gdp_bh;
 	struct ext4_sb_info *sbi;
 	struct super_block *sb;
 	ext4_fsblk_t block;
@@ -3861,32 +3864,13 @@ ext4_mb_mark_diskspace_used(struct ext4_allocation_context *ac,
 	sb = ac->ac_sb;
 	sbi = EXT4_SB(sb);
 
-	bitmap_bh = ext4_read_block_bitmap(sb, ac->ac_b_ex.fe_group);
-	if (IS_ERR(bitmap_bh)) {
-		return PTR_ERR(bitmap_bh);
-	}
-
-	BUFFER_TRACE(bitmap_bh, "getting write access");
-	err = ext4_journal_get_write_access(handle, sb, bitmap_bh,
-					    EXT4_JTR_NONE);
-	if (err)
-		goto out_err;
-
-	err = -EIO;
-	gdp = ext4_get_group_desc(sb, ac->ac_b_ex.fe_group, &gdp_bh);
+	gdp = ext4_get_group_desc(sb, ac->ac_b_ex.fe_group, NULL);
 	if (!gdp)
-		goto out_err;
-
+		return -EIO;
 	ext4_debug("using block group %u(%d)\n", ac->ac_b_ex.fe_group,
 			ext4_free_group_clusters(sb, gdp));
 
-	BUFFER_TRACE(gdp_bh, "get_write_access");
-	err = ext4_journal_get_write_access(handle, sb, gdp_bh, EXT4_JTR_NONE);
-	if (err)
-		goto out_err;
-
 	block = ext4_grp_offs_to_block(sb, &ac->ac_b_ex);
-
 	len = EXT4_C2B(sbi, ac->ac_b_ex.fe_len);
 	if (!ext4_inode_block_valid(ac->ac_inode, block, len)) {
 		ext4_error(sb, "Allocating blocks %llu-%llu which overlap "
@@ -3895,41 +3879,30 @@ ext4_mb_mark_diskspace_used(struct ext4_allocation_context *ac,
 		 * Fix the bitmap and return EFSCORRUPTED
 		 * We leak some of the blocks here.
 		 */
-		ext4_lock_group(sb, ac->ac_b_ex.fe_group);
-		mb_set_bits(bitmap_bh->b_data, ac->ac_b_ex.fe_start,
-			      ac->ac_b_ex.fe_len);
-		ext4_unlock_group(sb, ac->ac_b_ex.fe_group);
-		err = ext4_handle_dirty_metadata(handle, NULL, bitmap_bh);
+		err = ext4_mb_mark_group_bb(&mc, ac->ac_b_ex.fe_group,
+					    ac->ac_b_ex.fe_start,
+					    ac->ac_b_ex.fe_len,
+					    0);
 		if (!err)
 			err = -EFSCORRUPTED;
-		goto out_err;
+		return err;
 	}
 
-	ext4_lock_group(sb, ac->ac_b_ex.fe_group);
 #ifdef AGGRESSIVE_CHECK
-	{
-		int i;
-		for (i = 0; i < ac->ac_b_ex.fe_len; i++) {
-			BUG_ON(mb_test_bit(ac->ac_b_ex.fe_start + i,
-						bitmap_bh->b_data));
-		}
-	}
+	err = ext4_mb_mark_group_bb(&mc, ac->ac_b_ex.fe_group,
+				    ac->ac_b_ex.fe_start, ac->ac_b_ex.fe_len,
+				    EXT4_MB_BITMAP_MARKED_CHECK);
+#else
+	err = ext4_mb_mark_group_bb(&mc, ac->ac_b_ex.fe_group,
+				    ac->ac_b_ex.fe_start, ac->ac_b_ex.fe_len,
+				    0);
 #endif
-	mb_set_bits(bitmap_bh->b_data, ac->ac_b_ex.fe_start,
-		      ac->ac_b_ex.fe_len);
-	if (ext4_has_group_desc_csum(sb) &&
-	    (gdp->bg_flags & cpu_to_le16(EXT4_BG_BLOCK_UNINIT))) {
-		gdp->bg_flags &= cpu_to_le16(~EXT4_BG_BLOCK_UNINIT);
-		ext4_free_group_clusters_set(sb, gdp,
-					     ext4_free_clusters_after_init(sb,
-						ac->ac_b_ex.fe_group, gdp));
-	}
-	len = ext4_free_group_clusters(sb, gdp) - ac->ac_b_ex.fe_len;
-	ext4_free_group_clusters_set(sb, gdp, len);
-	ext4_block_bitmap_csum_set(sb, gdp, bitmap_bh);
-	ext4_group_desc_csum_set(sb, ac->ac_b_ex.fe_group, gdp);
+	if (err && mc.changed == 0)
+		return err;
 
-	ext4_unlock_group(sb, ac->ac_b_ex.fe_group);
+#ifdef AGGRESSIVE_CHECK
+	BUG_ON(mc.changed != ac->ac_b_ex.fe_len);
+#endif
 	percpu_counter_sub(&sbi->s_freeclusters_counter, ac->ac_b_ex.fe_len);
 	/*
 	 * Now reduce the dirty block count also. Should not go negative
@@ -3939,21 +3912,6 @@ ext4_mb_mark_diskspace_used(struct ext4_allocation_context *ac,
 		percpu_counter_sub(&sbi->s_dirtyclusters_counter,
 				   reserv_clstrs);
 
-	if (sbi->s_log_groups_per_flex) {
-		ext4_group_t flex_group = ext4_flex_group(sbi,
-							  ac->ac_b_ex.fe_group);
-		atomic64_sub(ac->ac_b_ex.fe_len,
-			     &sbi_array_rcu_deref(sbi, s_flex_groups,
-						  flex_group)->free_clusters);
-	}
-
-	err = ext4_handle_dirty_metadata(handle, NULL, bitmap_bh);
-	if (err)
-		goto out_err;
-	err = ext4_handle_dirty_metadata(handle, NULL, gdp_bh);
-
-out_err:
-	brelse(bitmap_bh);
 	return err;
 }
 
-- 
2.30.0


^ permalink raw reply related	[flat|nested] 25+ messages in thread

* [PATCH v2 16/19] ext4: call ext4_mb_mark_group_bb in ext4_mb_clear_bb
  2023-04-12 17:28 [PATCH v2 00/19] Fixes, cleanups and unit test for mballoc Kemeng Shi
                   ` (14 preceding siblings ...)
  2023-04-12 17:28 ` [PATCH v2 15/19] ext4: call ext4_mb_mark_group_bb in ext4_mb_mark_diskspace_used Kemeng Shi
@ 2023-04-12 17:28 ` Kemeng Shi
  2023-04-12 17:28 ` [PATCH v2 17/19] ext4: call ext4_mb_mark_group_bb in ext4_group_add_blocks Kemeng Shi
                   ` (2 subsequent siblings)
  18 siblings, 0 replies; 25+ messages in thread
From: Kemeng Shi @ 2023-04-12 17:28 UTC (permalink / raw)
  To: tytso, adilger.kernel, ojaswin; +Cc: linux-ext4, linux-kernel, shikemeng

call ext4_mb_mark_group_bb in ext4_mb_clear_bb to remove repeat code
to update block bitmap and group descriptor on disk.

Signed-off-by: Kemeng Shi <shikemeng@huaweicloud.com>
---
 fs/ext4/mballoc.c | 86 +++++++++++++----------------------------------
 1 file changed, 23 insertions(+), 63 deletions(-)

diff --git a/fs/ext4/mballoc.c b/fs/ext4/mballoc.c
index 3d31ad486a0c..c3a4c7eae0b1 100644
--- a/fs/ext4/mballoc.c
+++ b/fs/ext4/mballoc.c
@@ -5884,18 +5884,19 @@ static void ext4_mb_clear_bb(handle_t *handle, struct inode *inode,
 			       ext4_fsblk_t block, unsigned long count,
 			       int flags)
 {
-	struct buffer_head *bitmap_bh = NULL;
+	struct ext4_mark_context mc = {
+		.handle = handle,
+		.sb = inode->i_sb,
+		.state = 0,
+	};
 	struct super_block *sb = inode->i_sb;
-	struct ext4_group_desc *gdp;
 	unsigned int overflow;
 	ext4_grpblk_t bit;
-	struct buffer_head *gd_bh;
 	ext4_group_t block_group;
 	struct ext4_sb_info *sbi;
 	struct ext4_buddy e4b;
 	unsigned int count_clusters;
 	int err = 0;
-	int ret;
 
 	sbi = EXT4_SB(sb);
 
@@ -5927,18 +5928,6 @@ static void ext4_mb_clear_bb(handle_t *handle, struct inode *inode,
 		/* The range changed so it's no longer validated */
 		flags &= ~EXT4_FREE_BLOCKS_VALIDATED;
 	}
-	count_clusters = EXT4_NUM_B2C(sbi, count);
-	bitmap_bh = ext4_read_block_bitmap(sb, block_group);
-	if (IS_ERR(bitmap_bh)) {
-		err = PTR_ERR(bitmap_bh);
-		bitmap_bh = NULL;
-		goto error_return;
-	}
-	gdp = ext4_get_group_desc(sb, block_group, &gd_bh);
-	if (!gdp) {
-		err = -EIO;
-		goto error_return;
-	}
 
 	if (!(flags & EXT4_FREE_BLOCKS_VALIDATED) &&
 	    !ext4_inode_block_valid(inode, block, count)) {
@@ -5948,28 +5937,7 @@ static void ext4_mb_clear_bb(handle_t *handle, struct inode *inode,
 		goto error_return;
 	}
 
-	BUFFER_TRACE(bitmap_bh, "getting write access");
-	err = ext4_journal_get_write_access(handle, sb, bitmap_bh,
-					    EXT4_JTR_NONE);
-	if (err)
-		goto error_return;
-
-	/*
-	 * We are about to modify some metadata.  Call the journal APIs
-	 * to unshare ->b_data if a currently-committing transaction is
-	 * using it
-	 */
-	BUFFER_TRACE(gd_bh, "get_write_access");
-	err = ext4_journal_get_write_access(handle, sb, gd_bh, EXT4_JTR_NONE);
-	if (err)
-		goto error_return;
-#ifdef AGGRESSIVE_CHECK
-	{
-		int i;
-		for (i = 0; i < count_clusters; i++)
-			BUG_ON(!mb_test_bit(bit + i, bitmap_bh->b_data));
-	}
-#endif
+	count_clusters = EXT4_NUM_B2C(sbi, count);
 	trace_ext4_mballoc_free(sb, inode, block_group, bit, count_clusters);
 
 	/* __GFP_NOFAIL: retry infinitely, ignore TIF_MEMDIE and memcg limit. */
@@ -5978,6 +5946,23 @@ static void ext4_mb_clear_bb(handle_t *handle, struct inode *inode,
 	if (err)
 		goto error_return;
 
+#ifdef AGGRESSIVE_CHECK
+	err = ext4_mb_mark_group_bb(&mc, block_group, bit, count_clusters,
+				    EXT4_MB_BITMAP_MARKED_CHECK);
+#else
+	err = ext4_mb_mark_group_bb(&mc, block_group, bit, count_clusters,
+				    0);
+#endif
+
+	if (err && mc.changed == 0) {
+		ext4_mb_unload_buddy(&e4b);
+		goto error_return;
+	}
+
+#ifdef AGGRESSIVE_CHECK
+	BUG_ON(mc.changed != count_clusters);
+#endif
+
 	/*
 	 * We need to make sure we don't reuse the freed block until after the
 	 * transaction is committed. We make an exception if the inode is to be
@@ -6000,7 +5985,6 @@ static void ext4_mb_clear_bb(handle_t *handle, struct inode *inode,
 		new_entry->efd_tid = handle->h_transaction->t_tid;
 
 		ext4_lock_group(sb, block_group);
-		mb_clear_bits(bitmap_bh->b_data, bit, count_clusters);
 		ext4_mb_free_metadata(handle, &e4b, new_entry);
 	} else {
 		/* need to update group_info->bb_free and bitmap
@@ -6019,23 +6003,11 @@ static void ext4_mb_clear_bb(handle_t *handle, struct inode *inode,
 			EXT4_MB_GRP_CLEAR_TRIMMED(e4b.bd_info);
 
 		ext4_lock_group(sb, block_group);
-		mb_clear_bits(bitmap_bh->b_data, bit, count_clusters);
 		mb_free_blocks(inode, &e4b, bit, count_clusters);
 	}
 
-	ret = ext4_free_group_clusters(sb, gdp) + count_clusters;
-	ext4_free_group_clusters_set(sb, gdp, ret);
-	ext4_block_bitmap_csum_set(sb, gdp, bitmap_bh);
-	ext4_group_desc_csum_set(sb, block_group, gdp);
 	ext4_unlock_group(sb, block_group);
 
-	if (sbi->s_log_groups_per_flex) {
-		ext4_group_t flex_group = ext4_flex_group(sbi, block_group);
-		atomic64_add(count_clusters,
-			     &sbi_array_rcu_deref(sbi, s_flex_groups,
-						  flex_group)->free_clusters);
-	}
-
 	/*
 	 * on a bigalloc file system, defer the s_freeclusters_counter
 	 * update to the caller (ext4_remove_space and friends) so they
@@ -6050,26 +6022,14 @@ static void ext4_mb_clear_bb(handle_t *handle, struct inode *inode,
 
 	ext4_mb_unload_buddy(&e4b);
 
-	/* We dirtied the bitmap block */
-	BUFFER_TRACE(bitmap_bh, "dirtied bitmap block");
-	err = ext4_handle_dirty_metadata(handle, NULL, bitmap_bh);
-
-	/* And the group descriptor block */
-	BUFFER_TRACE(gd_bh, "dirtied group descriptor block");
-	ret = ext4_handle_dirty_metadata(handle, NULL, gd_bh);
-	if (!err)
-		err = ret;
-
 	if (overflow && !err) {
 		block += count;
 		count = overflow;
-		put_bh(bitmap_bh);
 		/* The range changed so it's no longer validated */
 		flags &= ~EXT4_FREE_BLOCKS_VALIDATED;
 		goto do_more;
 	}
 error_return:
-	brelse(bitmap_bh);
 	ext4_std_error(sb, err);
 	return;
 }
-- 
2.30.0


^ permalink raw reply related	[flat|nested] 25+ messages in thread

* [PATCH v2 17/19] ext4: call ext4_mb_mark_group_bb in ext4_group_add_blocks
  2023-04-12 17:28 [PATCH v2 00/19] Fixes, cleanups and unit test for mballoc Kemeng Shi
                   ` (15 preceding siblings ...)
  2023-04-12 17:28 ` [PATCH v2 16/19] ext4: call ext4_mb_mark_group_bb in ext4_mb_clear_bb Kemeng Shi
@ 2023-04-12 17:28 ` Kemeng Shi
  2023-04-12 17:28 ` [PATCH v2 18/19] ext4: add some kunit stub for mballoc kunit test Kemeng Shi
  2023-04-12 17:28 ` [PATCH v2 19/19] ext4: add first unit test for ext4_mb_new_blocks_simple in mballoc Kemeng Shi
  18 siblings, 0 replies; 25+ messages in thread
From: Kemeng Shi @ 2023-04-12 17:28 UTC (permalink / raw)
  To: tytso, adilger.kernel, ojaswin; +Cc: linux-ext4, linux-kernel, shikemeng

call ext4_mb_mark_group_bb in ext4_group_add_blocks to remove repeat code
to update block bitmap and group descriptor on disk.

Signed-off-by: Kemeng Shi <shikemeng@huaweicloud.com>
---
 fs/ext4/mballoc.c | 87 +++++++++--------------------------------------
 1 file changed, 17 insertions(+), 70 deletions(-)

diff --git a/fs/ext4/mballoc.c b/fs/ext4/mballoc.c
index c3a4c7eae0b1..64860341ef2d 100644
--- a/fs/ext4/mballoc.c
+++ b/fs/ext4/mballoc.c
@@ -6149,23 +6149,23 @@ void ext4_free_blocks(handle_t *handle, struct inode *inode,
 int ext4_group_add_blocks(handle_t *handle, struct super_block *sb,
 			 ext4_fsblk_t block, unsigned long count)
 {
-	struct buffer_head *bitmap_bh = NULL;
-	struct buffer_head *gd_bh;
+	struct ext4_mark_context mc = {
+		.handle = handle,
+		.sb = sb,
+		.state = 0,
+	};
 	ext4_group_t block_group;
 	ext4_grpblk_t bit;
-	unsigned int i;
-	struct ext4_group_desc *desc;
 	struct ext4_sb_info *sbi = EXT4_SB(sb);
 	struct ext4_buddy e4b;
-	int err = 0, ret, free_clusters_count;
-	ext4_grpblk_t clusters_freed;
+	int err = 0;
 	ext4_fsblk_t first_cluster = EXT4_B2C(sbi, block);
 	ext4_fsblk_t last_cluster = EXT4_B2C(sbi, block + count - 1);
 	unsigned long cluster_count = last_cluster - first_cluster + 1;
 
 	ext4_debug("Adding block(s) %llu-%llu\n", block, block + count - 1);
 
-	if (count == 0)
+	if (cluster_count == 0)
 		return 0;
 
 	ext4_get_group_no_and_offset(sb, block, &block_group, &bit);
@@ -6180,19 +6180,6 @@ int ext4_group_add_blocks(handle_t *handle, struct super_block *sb,
 		goto error_return;
 	}
 
-	bitmap_bh = ext4_read_block_bitmap(sb, block_group);
-	if (IS_ERR(bitmap_bh)) {
-		err = PTR_ERR(bitmap_bh);
-		bitmap_bh = NULL;
-		goto error_return;
-	}
-
-	desc = ext4_get_group_desc(sb, block_group, &gd_bh);
-	if (!desc) {
-		err = -EIO;
-		goto error_return;
-	}
-
 	if (!ext4_sb_block_valid(sb, NULL, block, count)) {
 		ext4_error(sb, "Adding blocks in system zones - "
 			   "Block = %llu, count = %lu",
@@ -6201,75 +6188,35 @@ int ext4_group_add_blocks(handle_t *handle, struct super_block *sb,
 		goto error_return;
 	}
 
-	BUFFER_TRACE(bitmap_bh, "getting write access");
-	err = ext4_journal_get_write_access(handle, sb, bitmap_bh,
-					    EXT4_JTR_NONE);
-	if (err)
-		goto error_return;
-
-	/*
-	 * We are about to modify some metadata.  Call the journal APIs
-	 * to unshare ->b_data if a currently-committing transaction is
-	 * using it
-	 */
-	BUFFER_TRACE(gd_bh, "get_write_access");
-	err = ext4_journal_get_write_access(handle, sb, gd_bh, EXT4_JTR_NONE);
+	err = ext4_mb_load_buddy(sb, block_group, &e4b);
 	if (err)
 		goto error_return;
 
-	for (i = 0, clusters_freed = 0; i < cluster_count; i++) {
-		BUFFER_TRACE(bitmap_bh, "clear bit");
-		if (!mb_test_bit(bit + i, bitmap_bh->b_data)) {
-			ext4_error(sb, "bit already cleared for block %llu",
-				   (ext4_fsblk_t)(block + i));
-			BUFFER_TRACE(bitmap_bh, "bit already cleared");
-		} else {
-			clusters_freed++;
-		}
-	}
+	err = ext4_mb_mark_group_bb(&mc, block_group, bit, cluster_count,
+				    EXT4_MB_BITMAP_MARKED_CHECK);
 
-	err = ext4_mb_load_buddy(sb, block_group, &e4b);
-	if (err)
+	if (err && mc.changed == 0) {
+		ext4_mb_unload_buddy(&e4b);
 		goto error_return;
+	}
 
+	if (mc.changed != cluster_count)
+		ext4_error(sb, "bit already cleared in group %u",
+			   block_group);
 	/*
 	 * need to update group_info->bb_free and bitmap
 	 * with group lock held. generate_buddy look at
 	 * them with group lock_held
 	 */
 	ext4_lock_group(sb, block_group);
-	mb_clear_bits(bitmap_bh->b_data, bit, cluster_count);
 	mb_free_blocks(NULL, &e4b, bit, cluster_count);
-	free_clusters_count = clusters_freed +
-		ext4_free_group_clusters(sb, desc);
-	ext4_free_group_clusters_set(sb, desc, free_clusters_count);
-	ext4_block_bitmap_csum_set(sb, desc, bitmap_bh);
-	ext4_group_desc_csum_set(sb, block_group, desc);
 	ext4_unlock_group(sb, block_group);
 	percpu_counter_add(&sbi->s_freeclusters_counter,
-			   clusters_freed);
-
-	if (sbi->s_log_groups_per_flex) {
-		ext4_group_t flex_group = ext4_flex_group(sbi, block_group);
-		atomic64_add(clusters_freed,
-			     &sbi_array_rcu_deref(sbi, s_flex_groups,
-						  flex_group)->free_clusters);
-	}
+			   mc.changed);
 
 	ext4_mb_unload_buddy(&e4b);
 
-	/* We dirtied the bitmap block */
-	BUFFER_TRACE(bitmap_bh, "dirtied bitmap block");
-	err = ext4_handle_dirty_metadata(handle, NULL, bitmap_bh);
-
-	/* And the group descriptor block */
-	BUFFER_TRACE(gd_bh, "dirtied group descriptor block");
-	ret = ext4_handle_dirty_metadata(handle, NULL, gd_bh);
-	if (!err)
-		err = ret;
-
 error_return:
-	brelse(bitmap_bh);
 	ext4_std_error(sb, err);
 	return err;
 }
-- 
2.30.0


^ permalink raw reply related	[flat|nested] 25+ messages in thread

* [PATCH v2 18/19] ext4: add some kunit stub for mballoc kunit test
  2023-04-12 17:28 [PATCH v2 00/19] Fixes, cleanups and unit test for mballoc Kemeng Shi
                   ` (16 preceding siblings ...)
  2023-04-12 17:28 ` [PATCH v2 17/19] ext4: call ext4_mb_mark_group_bb in ext4_group_add_blocks Kemeng Shi
@ 2023-04-12 17:28 ` Kemeng Shi
  2023-04-12 17:45   ` kernel test robot
  2023-04-12 18:16   ` kernel test robot
  2023-04-12 17:28 ` [PATCH v2 19/19] ext4: add first unit test for ext4_mb_new_blocks_simple in mballoc Kemeng Shi
  18 siblings, 2 replies; 25+ messages in thread
From: Kemeng Shi @ 2023-04-12 17:28 UTC (permalink / raw)
  To: tytso, adilger.kernel, ojaswin; +Cc: linux-ext4, linux-kernel, shikemeng

Multiblocks allocation will read and write block bitmap and group
descriptor which reside on disk. Add kunit stub to function
ext4_get_group_desc, ext4_read_block_bitmap_nowait, ext4_wait_block_bitmap
and ext4_mb_mark_group_bb to avoid real IO to disk.

Signed-off-by: Kemeng Shi <shikemeng@huaweicloud.com>
---
 fs/ext4/balloc.c  | 13 +++++++++++++
 fs/ext4/mballoc.c |  5 +++++
 2 files changed, 18 insertions(+)

diff --git a/fs/ext4/balloc.c b/fs/ext4/balloc.c
index 094269488183..682336d3dac1 100644
--- a/fs/ext4/balloc.c
+++ b/fs/ext4/balloc.c
@@ -22,6 +22,7 @@
 #include "mballoc.h"
 
 #include <trace/events/ext4.h>
+#include <kunit/static_stub.h>
 
 static unsigned ext4_num_base_meta_clusters(struct super_block *sb,
 					    ext4_group_t block_group);
@@ -267,6 +268,10 @@ struct ext4_group_desc * ext4_get_group_desc(struct super_block *sb,
 					     ext4_group_t block_group,
 					     struct buffer_head **bh)
 {
+#ifdef CONFIG_EXT4_KUNIT_TESTS
+	KUNIT_STATIC_STUB_REDIRECT(ext4_get_group_desc,
+				   sb, block_group, bh);
+#endif
 	unsigned int group_desc;
 	unsigned int offset;
 	ext4_group_t ngroups = ext4_get_groups_count(sb);
@@ -423,6 +428,10 @@ struct buffer_head *
 ext4_read_block_bitmap_nowait(struct super_block *sb, ext4_group_t block_group,
 			      bool ignore_locked)
 {
+#ifdef CONFIG_EXT4_KUNIT_TESTS
+	KUNIT_STATIC_STUB_REDIRECT(ext4_read_block_bitmap_nowait,
+				   sb, block_group, ignore_locked);
+#endif
 	struct ext4_group_desc *desc;
 	struct ext4_sb_info *sbi = EXT4_SB(sb);
 	struct buffer_head *bh;
@@ -522,6 +531,10 @@ ext4_read_block_bitmap_nowait(struct super_block *sb, ext4_group_t block_group,
 int ext4_wait_block_bitmap(struct super_block *sb, ext4_group_t block_group,
 			   struct buffer_head *bh)
 {
+#ifdef CONFIG_EXT4_KUNIT_TESTS
+	KUNIT_STATIC_STUB_REDIRECT(ext4_wait_block_bitmap,
+				   sb, block_group, bh);
+#endif
 	struct ext4_group_desc *desc;
 
 	if (!buffer_new(bh))
diff --git a/fs/ext4/mballoc.c b/fs/ext4/mballoc.c
index 64860341ef2d..f95a48bc8e31 100644
--- a/fs/ext4/mballoc.c
+++ b/fs/ext4/mballoc.c
@@ -17,6 +17,7 @@
 #include <linux/nospec.h>
 #include <linux/backing-dev.h>
 #include <trace/events/ext4.h>
+#include <kunit/static_stub.h>
 
 /*
  * MUSTDO:
@@ -3744,6 +3745,10 @@ static int
 ext4_mb_mark_group_bb(struct ext4_mark_context *mc, ext4_group_t group,
 		      ext4_grpblk_t blkoff, ext4_grpblk_t len, int flags)
 {
+#ifdef CONFIG_EXT4_KUNIT_TESTS
+	KUNIT_STATIC_STUB_REDIRECT(ext4_mb_mark_group_bb,
+				   mc, group, blkoff, len, flags);
+#endif
 	handle_t *handle = mc->handle;
 	struct super_block *sb = mc->sb;
 	struct ext4_sb_info *sbi = EXT4_SB(sb);
-- 
2.30.0


^ permalink raw reply related	[flat|nested] 25+ messages in thread

* [PATCH v2 19/19] ext4: add first unit test for ext4_mb_new_blocks_simple in mballoc
  2023-04-12 17:28 [PATCH v2 00/19] Fixes, cleanups and unit test for mballoc Kemeng Shi
                   ` (17 preceding siblings ...)
  2023-04-12 17:28 ` [PATCH v2 18/19] ext4: add some kunit stub for mballoc kunit test Kemeng Shi
@ 2023-04-12 17:28 ` Kemeng Shi
  2023-04-12 18:16   ` kernel test robot
                     ` (2 more replies)
  18 siblings, 3 replies; 25+ messages in thread
From: Kemeng Shi @ 2023-04-12 17:28 UTC (permalink / raw)
  To: tytso, adilger.kernel, ojaswin; +Cc: linux-ext4, linux-kernel, shikemeng

Here are prepared work:
1. Include mballoc-test.c to mballoc.c to be able test static function
in mballoc.c.
2. Implement static stub to avoid read IO to disk.
3. Construct fake super_block. Only partial members are set, more members
will be set when more functions are tested.
Then unit test for ext4_mb_new_blocks_simple is added.

Signed-off-by: Kemeng Shi <shikemeng@huaweicloud.com>
---
 fs/ext4/mballoc-test.c | 319 +++++++++++++++++++++++++++++++++++++++++
 fs/ext4/mballoc.c      |   4 +
 2 files changed, 323 insertions(+)
 create mode 100644 fs/ext4/mballoc-test.c

diff --git a/fs/ext4/mballoc-test.c b/fs/ext4/mballoc-test.c
new file mode 100644
index 000000000000..2ec3efe45b3f
--- /dev/null
+++ b/fs/ext4/mballoc-test.c
@@ -0,0 +1,319 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * KUnit test of ext4 multiblocks allocation.
+ */
+
+#include <kunit/test.h>
+#include <kunit/static_stub.h>
+
+#include "ext4.h"
+
+struct mb_grp_ctx {
+	struct buffer_head bitmap_bh;
+	struct ext4_group_desc desc;
+	/* one group descriptor for each group descriptor for simplicity */
+	struct buffer_head gd_bh;
+};
+
+struct mb_ctx {
+	struct mb_grp_ctx *grp_ctx;
+};
+
+#define MB_CTX(sb) ((struct mb_ctx *)((struct super_block *)sb + 1))
+#define MB_GRP_CTX(sb, group) (&MB_CTX(sb)->grp_ctx[group])
+
+static struct super_block *alloc_fake_super_block(void)
+{
+	struct ext4_super_block *es = kzalloc(sizeof(*es), GFP_KERNEL);
+	struct ext4_sb_info *sbi = kzalloc(sizeof(*sbi), GFP_KERNEL);
+	struct super_block *sb = kzalloc(sizeof(*sb) +
+					 sizeof(struct mb_ctx),
+					 GFP_KERNEL);
+
+	if (sb == NULL || sbi == NULL || es == NULL)
+		goto out;
+
+	sbi->s_es = es;
+	sb->s_fs_info = sbi;
+	return sb;
+
+out:
+	kfree(sb);
+	kfree(sbi);
+	kfree(es);
+	return NULL;
+}
+
+static void free_fake_super_block(struct super_block *sb)
+{
+	struct ext4_sb_info *sbi = EXT4_SB(sb);
+
+	kfree(sbi->s_es);
+	kfree(sbi);
+	kfree(sb);
+}
+
+struct ext4_block_layout {
+	unsigned char blocksize_bits;
+	unsigned int cluster_bits;
+	unsigned long blocks_per_group;
+	ext4_group_t group_count;
+	unsigned long desc_size;
+};
+
+static void init_sb_layout(struct super_block *sb,
+			  struct ext4_block_layout *layout)
+{
+	struct ext4_sb_info *sbi = EXT4_SB(sb);
+	struct ext4_super_block *es = sbi->s_es;
+
+	sb->s_blocksize = 1UL << layout->blocksize_bits;
+	sb->s_blocksize_bits = layout->blocksize_bits;
+
+	sbi->s_groups_count = layout->group_count;
+	sbi->s_blocks_per_group = layout->blocks_per_group;
+	sbi->s_cluster_bits = layout->cluster_bits;
+	sbi->s_cluster_ratio = 1U << layout->cluster_bits;
+	sbi->s_clusters_per_group = layout->blocks_per_group >>
+				    layout->cluster_bits;
+	sbi->s_desc_size = layout->desc_size;
+
+	es->s_first_data_block = cpu_to_le32(0);
+	es->s_blocks_count_lo = cpu_to_le32(layout->blocks_per_group *
+					    layout->group_count);
+}
+
+static int mb_grp_ctx_init(struct super_block *sb,
+			   struct mb_grp_ctx *grp_ctx)
+{
+	grp_ctx->bitmap_bh.b_data = kzalloc(EXT4_BLOCK_SIZE(sb), GFP_KERNEL);
+	if (grp_ctx->bitmap_bh.b_data == NULL)
+		return -ENOMEM;
+
+	get_bh(&grp_ctx->bitmap_bh);
+	get_bh(&grp_ctx->gd_bh);
+	return 0;
+}
+
+static void mb_grp_ctx_release(struct mb_grp_ctx *grp_ctx)
+{
+	kfree(grp_ctx->bitmap_bh.b_data);
+	grp_ctx->bitmap_bh.b_data = NULL;
+}
+
+static void mb_ctx_mark_used(struct super_block *sb, ext4_group_t group,
+			     unsigned int start, unsigned int len)
+{
+	struct mb_grp_ctx *grp_ctx = MB_GRP_CTX(sb, group);
+
+	mb_set_bits(grp_ctx->bitmap_bh.b_data, start, len);
+}
+
+/* called after init_sb_layout */
+static int mb_ctx_init(struct super_block *sb)
+{
+	struct mb_ctx *ctx = MB_CTX(sb);
+	ext4_group_t i, ngroups = ext4_get_groups_count(sb);
+
+	ctx->grp_ctx = kcalloc(ngroups, sizeof(struct mb_grp_ctx),
+			       GFP_KERNEL);
+	if (ctx->grp_ctx == NULL)
+		return -ENOMEM;
+
+	for (i = 0; i < ngroups; i++)
+		if (mb_grp_ctx_init(sb, &ctx->grp_ctx[i]))
+			goto out;
+
+	/*
+	 * first data block(first cluster in first group) is used by
+	 * metadata, mark it used to avoid to alloc data block at first
+	 * block which will fail ext4_sb_block_valid check.
+	 */
+	mb_set_bits(ctx->grp_ctx[0].bitmap_bh.b_data, 0, 1);
+
+	return 0;
+out:
+	while (i-- > 0)
+		mb_grp_ctx_release(&ctx->grp_ctx[i]);
+	kfree(ctx->grp_ctx);
+	return -ENOMEM;
+}
+
+static void mb_ctx_release(struct super_block *sb)
+{
+	struct mb_ctx *ctx = MB_CTX(sb);
+	ext4_group_t i, ngroups = ext4_get_groups_count(sb);
+
+	for (i = 0; i < ngroups; i++)
+		mb_grp_ctx_release(&ctx->grp_ctx[i]);
+	kfree(ctx->grp_ctx);
+}
+
+struct buffer_head *
+ext4_read_block_bitmap_nowait_stub(struct super_block *sb, ext4_group_t block_group,
+				   bool ignore_locked)
+{
+	struct mb_grp_ctx *grp_ctx = MB_GRP_CTX(sb, block_group);
+
+	get_bh(&grp_ctx->bitmap_bh);
+	return &grp_ctx->bitmap_bh;
+}
+
+int ext4_wait_block_bitmap_stub(struct super_block *sb,
+				ext4_group_t block_group,
+				struct buffer_head *bh)
+{
+	return 0;
+}
+
+struct ext4_group_desc *ext4_get_group_desc_stub(struct super_block *sb,
+					     ext4_group_t block_group,
+					     struct buffer_head **bh)
+{
+	struct mb_grp_ctx *grp_ctx = MB_GRP_CTX(sb, block_group);
+
+	if (bh != NULL)
+		*bh = &grp_ctx->gd_bh;
+
+	return &grp_ctx->desc;
+}
+
+int ext4_mb_mark_group_bb_stub(struct ext4_mark_context *mc,
+			       ext4_group_t group, ext4_grpblk_t blkoff,
+			       ext4_grpblk_t len, int flags)
+{
+	struct mb_grp_ctx *grp_ctx = MB_GRP_CTX(mc->sb, group);
+	struct buffer_head *bitmap_bh = &grp_ctx->bitmap_bh;
+
+	if (mc->state)
+		mb_set_bits(bitmap_bh->b_data, blkoff, len);
+	else
+		mb_clear_bits(bitmap_bh->b_data, blkoff, len);
+
+	return 0;
+}
+
+#define TEST_BLOCKSIZE_BITS 10
+#define TEST_CLUSTER_BITS 3
+#define TEST_BLOCKS_PER_GROUP 8192
+#define TEST_GROUP_COUNT 4
+#define TEST_DESC_SIZE 64
+#define TEST_GOAL_GROUP 1
+static int mballoc_test_init(struct kunit *test)
+{
+	struct ext4_block_layout layout = {
+		.blocksize_bits = TEST_BLOCKSIZE_BITS,
+		.cluster_bits = TEST_CLUSTER_BITS,
+		.blocks_per_group = TEST_BLOCKS_PER_GROUP,
+		.group_count = TEST_GROUP_COUNT,
+		.desc_size = TEST_DESC_SIZE,
+	};
+	struct super_block *sb;
+	int ret;
+
+	sb = alloc_fake_super_block();
+	if (sb == NULL)
+		return -ENOMEM;
+
+	init_sb_layout(sb, &layout);
+
+	ret = mb_ctx_init(sb);
+	if (ret != 0) {
+		free_fake_super_block(sb);
+		return ret;
+	}
+
+	test->priv = sb;
+	kunit_activate_static_stub(test,
+				   ext4_read_block_bitmap_nowait,
+				   ext4_read_block_bitmap_nowait_stub);
+	kunit_activate_static_stub(test,
+				   ext4_wait_block_bitmap,
+				   ext4_wait_block_bitmap_stub);
+	kunit_activate_static_stub(test,
+				   ext4_get_group_desc,
+				   ext4_get_group_desc_stub);
+	kunit_activate_static_stub(test,
+				   ext4_mb_mark_group_bb,
+				   ext4_mb_mark_group_bb_stub);
+	return 0;
+}
+
+static void mballoc_test_exit(struct kunit *test)
+{
+	struct super_block *sb = (struct super_block *)test->priv;
+
+	mb_ctx_release(sb);
+	free_fake_super_block(sb);
+}
+
+static void test_new_blocks_simple(struct kunit *test)
+{
+	struct super_block *sb = (struct super_block *)test->priv;
+	struct inode inode = { .i_sb = sb, };
+	struct ext4_allocation_request ar;
+	ext4_group_t i, goal_group = TEST_GOAL_GROUP;
+	int err = 0;
+	ext4_fsblk_t found;
+	struct ext4_sb_info *sbi = EXT4_SB(sb);
+
+	ar.inode = &inode;
+
+	/* get block at goal */
+	ar.goal = ext4_group_first_block_no(sb, goal_group);
+	found = ext4_mb_new_blocks_simple(&ar, &err);
+	KUNIT_ASSERT_EQ_MSG(test, ar.goal, found,
+		"failed to alloc block at goal, expected %llu found %llu",
+		ar.goal, found);
+
+	/* get block after goal in goal group */
+	ar.goal = ext4_group_first_block_no(sb, goal_group);
+	found = ext4_mb_new_blocks_simple(&ar, &err);
+	KUNIT_ASSERT_EQ_MSG(test, ar.goal + EXT4_C2B(sbi, 1), found,
+		"failed to alloc block after goal in goal group, expected %llu found %llu",
+		ar.goal + 1, found);
+
+	/* get block after goal group */
+	mb_ctx_mark_used(sb, goal_group, 0, EXT4_CLUSTERS_PER_GROUP(sb));
+	ar.goal = ext4_group_first_block_no(sb, goal_group);
+	found = ext4_mb_new_blocks_simple(&ar, &err);
+	KUNIT_ASSERT_EQ_MSG(test,
+		ext4_group_first_block_no(sb, goal_group + 1), found,
+		"failed to alloc block after goal group, expected %llu found %llu",
+		ext4_group_first_block_no(sb, goal_group + 1), found);
+
+	/* get block before goal group */
+	for (i = goal_group; i < ext4_get_groups_count(sb); i++)
+		mb_ctx_mark_used(sb, i, 0, EXT4_CLUSTERS_PER_GROUP(sb));
+	ar.goal = ext4_group_first_block_no(sb, goal_group);
+	found = ext4_mb_new_blocks_simple(&ar, &err);
+	KUNIT_ASSERT_EQ_MSG(test,
+		ext4_group_first_block_no(sb, 0) + EXT4_C2B(sbi, 1), found,
+		"failed to alloc block before goal group, expected %llu found %llu",
+		ext4_group_first_block_no(sb, 0 + EXT4_C2B(sbi, 1)), found);
+
+	/* no block available, fail to allocate block */
+	for (i = 0; i < ext4_get_groups_count(sb); i++)
+		mb_ctx_mark_used(sb, i, 0, EXT4_CLUSTERS_PER_GROUP(sb));
+	ar.goal = ext4_group_first_block_no(sb, goal_group);
+	found = ext4_mb_new_blocks_simple(&ar, &err);
+	KUNIT_ASSERT_NE_MSG(test, err, 0,
+		"unexpectedly get block when no block is available");
+}
+
+
+static struct kunit_case ext4_mballoc_test_cases[] = {
+	KUNIT_CASE(test_new_blocks_simple),
+	{}
+};
+
+static struct kunit_suite ext4_mballoc_test_suite = {
+	.name = "ext4_mballoc_test",
+	.init = mballoc_test_init,
+	.exit = mballoc_test_exit,
+	.test_cases = ext4_mballoc_test_cases,
+};
+
+kunit_test_suites(&ext4_mballoc_test_suite);
+
+MODULE_LICENSE("GPL v2");
diff --git a/fs/ext4/mballoc.c b/fs/ext4/mballoc.c
index f95a48bc8e31..ff1249673233 100644
--- a/fs/ext4/mballoc.c
+++ b/fs/ext4/mballoc.c
@@ -6518,3 +6518,7 @@ ext4_mballoc_query_range(
 
 	return error;
 }
+
+#ifdef CONFIG_EXT4_KUNIT_TESTS
+#include "mballoc-test.c"
+#endif
-- 
2.30.0


^ permalink raw reply related	[flat|nested] 25+ messages in thread

* Re: [PATCH v2 18/19] ext4: add some kunit stub for mballoc kunit test
  2023-04-12 17:28 ` [PATCH v2 18/19] ext4: add some kunit stub for mballoc kunit test Kemeng Shi
@ 2023-04-12 17:45   ` kernel test robot
  2023-04-12 18:16   ` kernel test robot
  1 sibling, 0 replies; 25+ messages in thread
From: kernel test robot @ 2023-04-12 17:45 UTC (permalink / raw)
  To: Kemeng Shi, tytso, adilger.kernel, ojaswin
  Cc: oe-kbuild-all, linux-ext4, linux-kernel, shikemeng

Hi Kemeng,

kernel test robot noticed the following build warnings:

[auto build test WARNING on tytso-ext4/dev]
[also build test WARNING on next-20230412]
[cannot apply to linus/master v6.3-rc6]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Kemeng-Shi/ext4-fix-wrong-unit-use-in-ext4_mb_normalize_request/20230412-172757
base:   https://git.kernel.org/pub/scm/linux/kernel/git/tytso/ext4.git dev
patch link:    https://lore.kernel.org/r/20230412172833.2317696-19-shikemeng%40huaweicloud.com
patch subject: [PATCH v2 18/19] ext4: add some kunit stub for mballoc kunit test
config: loongarch-randconfig-r004-20230409 (https://download.01.org/0day-ci/archive/20230413/202304130140.2kslXTgi-lkp@intel.com/config)
compiler: loongarch64-linux-gcc (GCC) 12.1.0
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # https://github.com/intel-lab-lkp/linux/commit/357d528a1ead868fa038c4bfe426744ac7c34ea6
        git remote add linux-review https://github.com/intel-lab-lkp/linux
        git fetch --no-tags linux-review Kemeng-Shi/ext4-fix-wrong-unit-use-in-ext4_mb_normalize_request/20230412-172757
        git checkout 357d528a1ead868fa038c4bfe426744ac7c34ea6
        # save the config file
        mkdir build_dir && cp config build_dir/.config
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.1.0 make.cross W=1 O=build_dir ARCH=loongarch olddefconfig
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.1.0 make.cross W=1 O=build_dir ARCH=loongarch SHELL=/bin/bash fs/

If you fix the issue, kindly add following tag where applicable
| Reported-by: kernel test robot <lkp@intel.com>
| Link: https://lore.kernel.org/oe-kbuild-all/202304130140.2kslXTgi-lkp@intel.com/

All warnings (new ones prefixed by >>):

   fs/ext4/mballoc.c: In function 'ext4_mb_mark_group_bb':
>> fs/ext4/mballoc.c:3752:9: warning: ISO C90 forbids mixed declarations and code [-Wdeclaration-after-statement]
    3752 |         handle_t *handle = mc->handle;
         |         ^~~~~~~~
--
   fs/ext4/balloc.c: In function 'ext4_get_group_desc':
>> fs/ext4/balloc.c:275:9: warning: ISO C90 forbids mixed declarations and code [-Wdeclaration-after-statement]
     275 |         unsigned int group_desc;
         |         ^~~~~~~~
   fs/ext4/balloc.c: In function 'ext4_read_block_bitmap_nowait':
   fs/ext4/balloc.c:435:9: warning: ISO C90 forbids mixed declarations and code [-Wdeclaration-after-statement]
     435 |         struct ext4_group_desc *desc;
         |         ^~~~~~
   fs/ext4/balloc.c: In function 'ext4_wait_block_bitmap':
   fs/ext4/balloc.c:538:9: warning: ISO C90 forbids mixed declarations and code [-Wdeclaration-after-statement]
     538 |         struct ext4_group_desc *desc;
         |         ^~~~~~


vim +3752 fs/ext4/mballoc.c

80cb2f14af5f5f Kemeng Shi 2023-04-13  3743  
80cb2f14af5f5f Kemeng Shi 2023-04-13  3744  static int
80cb2f14af5f5f Kemeng Shi 2023-04-13  3745  ext4_mb_mark_group_bb(struct ext4_mark_context *mc, ext4_group_t group,
a760807c4967d7 Kemeng Shi 2023-04-13  3746  		      ext4_grpblk_t blkoff, ext4_grpblk_t len, int flags)
80cb2f14af5f5f Kemeng Shi 2023-04-13  3747  {
357d528a1ead86 Kemeng Shi 2023-04-13  3748  #ifdef CONFIG_EXT4_KUNIT_TESTS
357d528a1ead86 Kemeng Shi 2023-04-13  3749  	KUNIT_STATIC_STUB_REDIRECT(ext4_mb_mark_group_bb,
357d528a1ead86 Kemeng Shi 2023-04-13  3750  				   mc, group, blkoff, len, flags);
357d528a1ead86 Kemeng Shi 2023-04-13  3751  #endif
a760807c4967d7 Kemeng Shi 2023-04-13 @3752  	handle_t *handle = mc->handle;
80cb2f14af5f5f Kemeng Shi 2023-04-13  3753  	struct super_block *sb = mc->sb;
80cb2f14af5f5f Kemeng Shi 2023-04-13  3754  	struct ext4_sb_info *sbi = EXT4_SB(sb);
80cb2f14af5f5f Kemeng Shi 2023-04-13  3755  	struct buffer_head *bitmap_bh = NULL;
80cb2f14af5f5f Kemeng Shi 2023-04-13  3756  	struct ext4_group_desc *gdp;
80cb2f14af5f5f Kemeng Shi 2023-04-13  3757  	struct buffer_head *gdp_bh;
80cb2f14af5f5f Kemeng Shi 2023-04-13  3758  	int err;
a760807c4967d7 Kemeng Shi 2023-04-13  3759  	unsigned int i, already, changed = len;
80cb2f14af5f5f Kemeng Shi 2023-04-13  3760  
a760807c4967d7 Kemeng Shi 2023-04-13  3761  	mc->changed = 0;
80cb2f14af5f5f Kemeng Shi 2023-04-13  3762  	bitmap_bh = ext4_read_block_bitmap(sb, group);
80cb2f14af5f5f Kemeng Shi 2023-04-13  3763  	if (IS_ERR(bitmap_bh))
80cb2f14af5f5f Kemeng Shi 2023-04-13  3764  		return PTR_ERR(bitmap_bh);
80cb2f14af5f5f Kemeng Shi 2023-04-13  3765  
a760807c4967d7 Kemeng Shi 2023-04-13  3766  	if (handle) {
a760807c4967d7 Kemeng Shi 2023-04-13  3767  		BUFFER_TRACE(bitmap_bh, "getting write access");
a760807c4967d7 Kemeng Shi 2023-04-13  3768  		err = ext4_journal_get_write_access(handle, sb, bitmap_bh,
a760807c4967d7 Kemeng Shi 2023-04-13  3769  						    EXT4_JTR_NONE);
a760807c4967d7 Kemeng Shi 2023-04-13  3770  		if (err)
a760807c4967d7 Kemeng Shi 2023-04-13  3771  			goto out_err;
a760807c4967d7 Kemeng Shi 2023-04-13  3772  	}
a760807c4967d7 Kemeng Shi 2023-04-13  3773  
80cb2f14af5f5f Kemeng Shi 2023-04-13  3774  	err = -EIO;
80cb2f14af5f5f Kemeng Shi 2023-04-13  3775  	gdp = ext4_get_group_desc(sb, group, &gdp_bh);
80cb2f14af5f5f Kemeng Shi 2023-04-13  3776  	if (!gdp)
80cb2f14af5f5f Kemeng Shi 2023-04-13  3777  		goto out_err;
80cb2f14af5f5f Kemeng Shi 2023-04-13  3778  
a760807c4967d7 Kemeng Shi 2023-04-13  3779  	if (handle) {
a760807c4967d7 Kemeng Shi 2023-04-13  3780  		BUFFER_TRACE(gdp_bh, "get_write_access");
a760807c4967d7 Kemeng Shi 2023-04-13  3781  		err = ext4_journal_get_write_access(handle, sb, gdp_bh,
a760807c4967d7 Kemeng Shi 2023-04-13  3782  						    EXT4_JTR_NONE);
a760807c4967d7 Kemeng Shi 2023-04-13  3783  		if (err)
a760807c4967d7 Kemeng Shi 2023-04-13  3784  			goto out_err;
a760807c4967d7 Kemeng Shi 2023-04-13  3785  	}
a760807c4967d7 Kemeng Shi 2023-04-13  3786  
80cb2f14af5f5f Kemeng Shi 2023-04-13  3787  	ext4_lock_group(sb, group);
80cb2f14af5f5f Kemeng Shi 2023-04-13  3788  	if (ext4_has_group_desc_csum(sb) &&
80cb2f14af5f5f Kemeng Shi 2023-04-13  3789  	    (gdp->bg_flags & cpu_to_le16(EXT4_BG_BLOCK_UNINIT))) {
80cb2f14af5f5f Kemeng Shi 2023-04-13  3790  		gdp->bg_flags &= cpu_to_le16(~EXT4_BG_BLOCK_UNINIT);
80cb2f14af5f5f Kemeng Shi 2023-04-13  3791  		ext4_free_group_clusters_set(sb, gdp,
80cb2f14af5f5f Kemeng Shi 2023-04-13  3792  			ext4_free_clusters_after_init(sb, group, gdp));
80cb2f14af5f5f Kemeng Shi 2023-04-13  3793  	}
80cb2f14af5f5f Kemeng Shi 2023-04-13  3794  
a760807c4967d7 Kemeng Shi 2023-04-13  3795  	if (flags & EXT4_MB_BITMAP_MARKED_CHECK) {
80cb2f14af5f5f Kemeng Shi 2023-04-13  3796  		already = 0;
80cb2f14af5f5f Kemeng Shi 2023-04-13  3797  		for (i = 0; i < len; i++)
80cb2f14af5f5f Kemeng Shi 2023-04-13  3798  			if (mb_test_bit(blkoff + i, bitmap_bh->b_data) ==
80cb2f14af5f5f Kemeng Shi 2023-04-13  3799  					mc->state)
80cb2f14af5f5f Kemeng Shi 2023-04-13  3800  				already++;
80cb2f14af5f5f Kemeng Shi 2023-04-13  3801  		changed = len - already;
a760807c4967d7 Kemeng Shi 2023-04-13  3802  	}
80cb2f14af5f5f Kemeng Shi 2023-04-13  3803  
80cb2f14af5f5f Kemeng Shi 2023-04-13  3804  	if (mc->state) {
80cb2f14af5f5f Kemeng Shi 2023-04-13  3805  		mb_set_bits(bitmap_bh->b_data, blkoff, len);
80cb2f14af5f5f Kemeng Shi 2023-04-13  3806  		ext4_free_group_clusters_set(sb, gdp,
80cb2f14af5f5f Kemeng Shi 2023-04-13  3807  			ext4_free_group_clusters(sb, gdp) - changed);
80cb2f14af5f5f Kemeng Shi 2023-04-13  3808  	} else {
80cb2f14af5f5f Kemeng Shi 2023-04-13  3809  		mb_clear_bits(bitmap_bh->b_data, blkoff, len);
80cb2f14af5f5f Kemeng Shi 2023-04-13  3810  		ext4_free_group_clusters_set(sb, gdp,
80cb2f14af5f5f Kemeng Shi 2023-04-13  3811  			ext4_free_group_clusters(sb, gdp) + changed);
80cb2f14af5f5f Kemeng Shi 2023-04-13  3812  	}
80cb2f14af5f5f Kemeng Shi 2023-04-13  3813  
80cb2f14af5f5f Kemeng Shi 2023-04-13  3814  	ext4_block_bitmap_csum_set(sb, gdp, bitmap_bh);
80cb2f14af5f5f Kemeng Shi 2023-04-13  3815  	ext4_group_desc_csum_set(sb, group, gdp);
80cb2f14af5f5f Kemeng Shi 2023-04-13  3816  	ext4_unlock_group(sb, group);
a760807c4967d7 Kemeng Shi 2023-04-13  3817  	mc->changed = changed;
80cb2f14af5f5f Kemeng Shi 2023-04-13  3818  
80cb2f14af5f5f Kemeng Shi 2023-04-13  3819  	if (sbi->s_log_groups_per_flex) {
80cb2f14af5f5f Kemeng Shi 2023-04-13  3820  		ext4_group_t flex_group = ext4_flex_group(sbi, group);
80cb2f14af5f5f Kemeng Shi 2023-04-13  3821  		struct flex_groups *fg = sbi_array_rcu_deref(sbi,
80cb2f14af5f5f Kemeng Shi 2023-04-13  3822  					   s_flex_groups, flex_group);
80cb2f14af5f5f Kemeng Shi 2023-04-13  3823  
80cb2f14af5f5f Kemeng Shi 2023-04-13  3824  		if (mc->state)
80cb2f14af5f5f Kemeng Shi 2023-04-13  3825  			atomic64_sub(changed, &fg->free_clusters);
80cb2f14af5f5f Kemeng Shi 2023-04-13  3826  		else
80cb2f14af5f5f Kemeng Shi 2023-04-13  3827  			atomic64_add(changed, &fg->free_clusters);
80cb2f14af5f5f Kemeng Shi 2023-04-13  3828  	}
80cb2f14af5f5f Kemeng Shi 2023-04-13  3829  
a760807c4967d7 Kemeng Shi 2023-04-13  3830  	err = ext4_handle_dirty_metadata(handle, NULL, bitmap_bh);
80cb2f14af5f5f Kemeng Shi 2023-04-13  3831  	if (err)
80cb2f14af5f5f Kemeng Shi 2023-04-13  3832  		goto out_err;
a760807c4967d7 Kemeng Shi 2023-04-13  3833  	err = ext4_handle_dirty_metadata(handle, NULL, gdp_bh);
80cb2f14af5f5f Kemeng Shi 2023-04-13  3834  	if (err)
80cb2f14af5f5f Kemeng Shi 2023-04-13  3835  		goto out_err;
80cb2f14af5f5f Kemeng Shi 2023-04-13  3836  
a760807c4967d7 Kemeng Shi 2023-04-13  3837  	if (flags & EXT4_MB_SYNC_UPDATE) {
80cb2f14af5f5f Kemeng Shi 2023-04-13  3838  		sync_dirty_buffer(bitmap_bh);
80cb2f14af5f5f Kemeng Shi 2023-04-13  3839  		sync_dirty_buffer(gdp_bh);
a760807c4967d7 Kemeng Shi 2023-04-13  3840  	}
80cb2f14af5f5f Kemeng Shi 2023-04-13  3841  
80cb2f14af5f5f Kemeng Shi 2023-04-13  3842  out_err:
80cb2f14af5f5f Kemeng Shi 2023-04-13  3843  	brelse(bitmap_bh);
80cb2f14af5f5f Kemeng Shi 2023-04-13  3844  	return err;
80cb2f14af5f5f Kemeng Shi 2023-04-13  3845  }
c9de560ded61fa Alex Tomas 2008-01-29  3846  

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH v2 19/19] ext4: add first unit test for ext4_mb_new_blocks_simple in mballoc
  2023-04-12 17:28 ` [PATCH v2 19/19] ext4: add first unit test for ext4_mb_new_blocks_simple in mballoc Kemeng Shi
@ 2023-04-12 18:16   ` kernel test robot
  2023-04-12 18:58   ` kernel test robot
  2023-04-12 19:49   ` kernel test robot
  2 siblings, 0 replies; 25+ messages in thread
From: kernel test robot @ 2023-04-12 18:16 UTC (permalink / raw)
  To: Kemeng Shi, tytso, adilger.kernel, ojaswin
  Cc: llvm, oe-kbuild-all, linux-ext4, linux-kernel, shikemeng

Hi Kemeng,

kernel test robot noticed the following build errors:

[auto build test ERROR on tytso-ext4/dev]
[also build test ERROR on next-20230412]
[cannot apply to linus/master v6.3-rc6]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Kemeng-Shi/ext4-fix-wrong-unit-use-in-ext4_mb_normalize_request/20230412-172757
base:   https://git.kernel.org/pub/scm/linux/kernel/git/tytso/ext4.git dev
patch link:    https://lore.kernel.org/r/20230412172833.2317696-20-shikemeng%40huaweicloud.com
patch subject: [PATCH v2 19/19] ext4: add first unit test for ext4_mb_new_blocks_simple in mballoc
config: hexagon-randconfig-r013-20230410 (https://download.01.org/0day-ci/archive/20230413/202304130218.GUkZVdgG-lkp@intel.com/config)
compiler: clang version 17.0.0 (https://github.com/llvm/llvm-project 2c57868e2e877f73c339796c3374ae660bb77f0d)
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # https://github.com/intel-lab-lkp/linux/commit/3ceb1daf83fdf578c28ede1a10e55f05ef7642d5
        git remote add linux-review https://github.com/intel-lab-lkp/linux
        git fetch --no-tags linux-review Kemeng-Shi/ext4-fix-wrong-unit-use-in-ext4_mb_normalize_request/20230412-172757
        git checkout 3ceb1daf83fdf578c28ede1a10e55f05ef7642d5
        # save the config file
        mkdir build_dir && cp config build_dir/.config
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=hexagon olddefconfig
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=hexagon SHELL=/bin/bash fs/

If you fix the issue, kindly add following tag where applicable
| Reported-by: kernel test robot <lkp@intel.com>
| Link: https://lore.kernel.org/oe-kbuild-all/202304130218.GUkZVdgG-lkp@intel.com/

All errors (new ones prefixed by >>):

   In file included from fs/ext4/mballoc.c:12:
   In file included from fs/ext4/ext4_jbd2.h:16:
   In file included from include/linux/jbd2.h:23:
   In file included from include/linux/buffer_head.h:12:
   In file included from include/linux/blk_types.h:10:
   In file included from include/linux/bvec.h:10:
   In file included from include/linux/highmem.h:12:
   In file included from include/linux/hardirq.h:11:
   In file included from ./arch/hexagon/include/generated/asm/hardirq.h:1:
   In file included from include/asm-generic/hardirq.h:17:
   In file included from include/linux/irq.h:20:
   In file included from include/linux/io.h:13:
   In file included from arch/hexagon/include/asm/io.h:334:
   include/asm-generic/io.h:547:31: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
           val = __raw_readb(PCI_IOBASE + addr);
                             ~~~~~~~~~~ ^
   include/asm-generic/io.h:560:61: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
           val = __le16_to_cpu((__le16 __force)__raw_readw(PCI_IOBASE + addr));
                                                           ~~~~~~~~~~ ^
   include/uapi/linux/byteorder/little_endian.h:37:51: note: expanded from macro '__le16_to_cpu'
   #define __le16_to_cpu(x) ((__force __u16)(__le16)(x))
                                                     ^
   In file included from fs/ext4/mballoc.c:12:
   In file included from fs/ext4/ext4_jbd2.h:16:
   In file included from include/linux/jbd2.h:23:
   In file included from include/linux/buffer_head.h:12:
   In file included from include/linux/blk_types.h:10:
   In file included from include/linux/bvec.h:10:
   In file included from include/linux/highmem.h:12:
   In file included from include/linux/hardirq.h:11:
   In file included from ./arch/hexagon/include/generated/asm/hardirq.h:1:
   In file included from include/asm-generic/hardirq.h:17:
   In file included from include/linux/irq.h:20:
   In file included from include/linux/io.h:13:
   In file included from arch/hexagon/include/asm/io.h:334:
   include/asm-generic/io.h:573:61: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
           val = __le32_to_cpu((__le32 __force)__raw_readl(PCI_IOBASE + addr));
                                                           ~~~~~~~~~~ ^
   include/uapi/linux/byteorder/little_endian.h:35:51: note: expanded from macro '__le32_to_cpu'
   #define __le32_to_cpu(x) ((__force __u32)(__le32)(x))
                                                     ^
   In file included from fs/ext4/mballoc.c:12:
   In file included from fs/ext4/ext4_jbd2.h:16:
   In file included from include/linux/jbd2.h:23:
   In file included from include/linux/buffer_head.h:12:
   In file included from include/linux/blk_types.h:10:
   In file included from include/linux/bvec.h:10:
   In file included from include/linux/highmem.h:12:
   In file included from include/linux/hardirq.h:11:
   In file included from ./arch/hexagon/include/generated/asm/hardirq.h:1:
   In file included from include/asm-generic/hardirq.h:17:
   In file included from include/linux/irq.h:20:
   In file included from include/linux/io.h:13:
   In file included from arch/hexagon/include/asm/io.h:334:
   include/asm-generic/io.h:584:33: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
           __raw_writeb(value, PCI_IOBASE + addr);
                               ~~~~~~~~~~ ^
   include/asm-generic/io.h:594:59: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
           __raw_writew((u16 __force)cpu_to_le16(value), PCI_IOBASE + addr);
                                                         ~~~~~~~~~~ ^
   include/asm-generic/io.h:604:59: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
           __raw_writel((u32 __force)cpu_to_le32(value), PCI_IOBASE + addr);
                                                         ~~~~~~~~~~ ^
   fs/ext4/mballoc.c:3752:12: warning: mixing declarations and code is incompatible with standards before C99 [-Wdeclaration-after-statement]
           handle_t *handle = mc->handle;
                     ^
   In file included from fs/ext4/mballoc.c:6523:
>> fs/ext4/mballoc-test.c:107:31: error: casting from randomized structure pointer type 'struct super_block *' to 'struct mb_ctx *'
           struct mb_grp_ctx *grp_ctx = MB_GRP_CTX(sb, group);
                                        ^
   fs/ext4/mballoc-test.c:23:33: note: expanded from macro 'MB_GRP_CTX'
   #define MB_GRP_CTX(sb, group) (&MB_CTX(sb)->grp_ctx[group])
                                   ^
   fs/ext4/mballoc-test.c:22:21: note: expanded from macro 'MB_CTX'
   #define MB_CTX(sb) ((struct mb_ctx *)((struct super_block *)sb + 1))
                       ^
   fs/ext4/mballoc-test.c:115:23: error: casting from randomized structure pointer type 'struct super_block *' to 'struct mb_ctx *'
           struct mb_ctx *ctx = MB_CTX(sb);
                                ^
   fs/ext4/mballoc-test.c:22:21: note: expanded from macro 'MB_CTX'
   #define MB_CTX(sb) ((struct mb_ctx *)((struct super_block *)sb + 1))
                       ^
   fs/ext4/mballoc-test.c:144:23: error: casting from randomized structure pointer type 'struct super_block *' to 'struct mb_ctx *'
           struct mb_ctx *ctx = MB_CTX(sb);
                                ^
   fs/ext4/mballoc-test.c:22:21: note: expanded from macro 'MB_CTX'
   #define MB_CTX(sb) ((struct mb_ctx *)((struct super_block *)sb + 1))
                       ^
   fs/ext4/mballoc-test.c:156:31: error: casting from randomized structure pointer type 'struct super_block *' to 'struct mb_ctx *'
           struct mb_grp_ctx *grp_ctx = MB_GRP_CTX(sb, block_group);
                                        ^
   fs/ext4/mballoc-test.c:23:33: note: expanded from macro 'MB_GRP_CTX'
   #define MB_GRP_CTX(sb, group) (&MB_CTX(sb)->grp_ctx[group])
                                   ^
   fs/ext4/mballoc-test.c:22:21: note: expanded from macro 'MB_CTX'
   #define MB_CTX(sb) ((struct mb_ctx *)((struct super_block *)sb + 1))
                       ^
   fs/ext4/mballoc-test.c:153:1: warning: no previous prototype for function 'ext4_read_block_bitmap_nowait_stub' [-Wmissing-prototypes]
   ext4_read_block_bitmap_nowait_stub(struct super_block *sb, ext4_group_t block_group,
   ^
   fs/ext4/mballoc-test.c:152:1: note: declare 'static' if the function is not intended to be used outside of this translation unit
   struct buffer_head *
   ^
   static 
   fs/ext4/mballoc-test.c:162:5: warning: no previous prototype for function 'ext4_wait_block_bitmap_stub' [-Wmissing-prototypes]
   int ext4_wait_block_bitmap_stub(struct super_block *sb,
       ^
   fs/ext4/mballoc-test.c:162:1: note: declare 'static' if the function is not intended to be used outside of this translation unit
   int ext4_wait_block_bitmap_stub(struct super_block *sb,
   ^
   static 
   fs/ext4/mballoc-test.c:173:31: error: casting from randomized structure pointer type 'struct super_block *' to 'struct mb_ctx *'
           struct mb_grp_ctx *grp_ctx = MB_GRP_CTX(sb, block_group);
                                        ^
   fs/ext4/mballoc-test.c:23:33: note: expanded from macro 'MB_GRP_CTX'
   #define MB_GRP_CTX(sb, group) (&MB_CTX(sb)->grp_ctx[group])
                                   ^
   fs/ext4/mballoc-test.c:22:21: note: expanded from macro 'MB_CTX'
   #define MB_CTX(sb) ((struct mb_ctx *)((struct super_block *)sb + 1))
                       ^
   fs/ext4/mballoc-test.c:169:25: warning: no previous prototype for function 'ext4_get_group_desc_stub' [-Wmissing-prototypes]
   struct ext4_group_desc *ext4_get_group_desc_stub(struct super_block *sb,
                           ^
   fs/ext4/mballoc-test.c:169:1: note: declare 'static' if the function is not intended to be used outside of this translation unit
   struct ext4_group_desc *ext4_get_group_desc_stub(struct super_block *sb,
   ^
   static 
   fs/ext4/mballoc-test.c:185:31: error: casting from randomized structure pointer type 'struct super_block *' to 'struct mb_ctx *'
           struct mb_grp_ctx *grp_ctx = MB_GRP_CTX(mc->sb, group);
                                        ^
   fs/ext4/mballoc-test.c:23:33: note: expanded from macro 'MB_GRP_CTX'
   #define MB_GRP_CTX(sb, group) (&MB_CTX(sb)->grp_ctx[group])
                                   ^
   fs/ext4/mballoc-test.c:22:21: note: expanded from macro 'MB_CTX'
   #define MB_CTX(sb) ((struct mb_ctx *)((struct super_block *)sb + 1))
                       ^
   fs/ext4/mballoc-test.c:181:5: warning: no previous prototype for function 'ext4_mb_mark_group_bb_stub' [-Wmissing-prototypes]
   int ext4_mb_mark_group_bb_stub(struct ext4_mark_context *mc,
       ^
   fs/ext4/mballoc-test.c:181:1: note: declare 'static' if the function is not intended to be used outside of this translation unit
   int ext4_mb_mark_group_bb_stub(struct ext4_mark_context *mc,
   ^
   static 
   11 warnings and 6 errors generated.


vim +107 fs/ext4/mballoc-test.c

   103	
   104	static void mb_ctx_mark_used(struct super_block *sb, ext4_group_t group,
   105				     unsigned int start, unsigned int len)
   106	{
 > 107		struct mb_grp_ctx *grp_ctx = MB_GRP_CTX(sb, group);
   108	
   109		mb_set_bits(grp_ctx->bitmap_bh.b_data, start, len);
   110	}
   111	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH v2 18/19] ext4: add some kunit stub for mballoc kunit test
  2023-04-12 17:28 ` [PATCH v2 18/19] ext4: add some kunit stub for mballoc kunit test Kemeng Shi
  2023-04-12 17:45   ` kernel test robot
@ 2023-04-12 18:16   ` kernel test robot
  1 sibling, 0 replies; 25+ messages in thread
From: kernel test robot @ 2023-04-12 18:16 UTC (permalink / raw)
  To: Kemeng Shi, tytso, adilger.kernel, ojaswin
  Cc: llvm, oe-kbuild-all, linux-ext4, linux-kernel, shikemeng

Hi Kemeng,

kernel test robot noticed the following build warnings:

[auto build test WARNING on tytso-ext4/dev]
[also build test WARNING on next-20230412]
[cannot apply to linus/master v6.3-rc6]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Kemeng-Shi/ext4-fix-wrong-unit-use-in-ext4_mb_normalize_request/20230412-172757
base:   https://git.kernel.org/pub/scm/linux/kernel/git/tytso/ext4.git dev
patch link:    https://lore.kernel.org/r/20230412172833.2317696-19-shikemeng%40huaweicloud.com
patch subject: [PATCH v2 18/19] ext4: add some kunit stub for mballoc kunit test
config: powerpc-randconfig-r006-20230409 (https://download.01.org/0day-ci/archive/20230413/202304130244.S0jqbqkn-lkp@intel.com/config)
compiler: clang version 17.0.0 (https://github.com/llvm/llvm-project 2c57868e2e877f73c339796c3374ae660bb77f0d)
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # install powerpc cross compiling tool for clang build
        # apt-get install binutils-powerpc-linux-gnu
        # https://github.com/intel-lab-lkp/linux/commit/357d528a1ead868fa038c4bfe426744ac7c34ea6
        git remote add linux-review https://github.com/intel-lab-lkp/linux
        git fetch --no-tags linux-review Kemeng-Shi/ext4-fix-wrong-unit-use-in-ext4_mb_normalize_request/20230412-172757
        git checkout 357d528a1ead868fa038c4bfe426744ac7c34ea6
        # save the config file
        mkdir build_dir && cp config build_dir/.config
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=powerpc olddefconfig
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=powerpc SHELL=/bin/bash fs/

If you fix the issue, kindly add following tag where applicable
| Reported-by: kernel test robot <lkp@intel.com>
| Link: https://lore.kernel.org/oe-kbuild-all/202304130244.S0jqbqkn-lkp@intel.com/

All warnings (new ones prefixed by >>):

>> fs/ext4/balloc.c:275:15: warning: mixing declarations and code is incompatible with standards before C99 [-Wdeclaration-after-statement]
           unsigned int group_desc;
                        ^
   fs/ext4/balloc.c:435:26: warning: mixing declarations and code is incompatible with standards before C99 [-Wdeclaration-after-statement]
           struct ext4_group_desc *desc;
                                   ^
   fs/ext4/balloc.c:538:26: warning: mixing declarations and code is incompatible with standards before C99 [-Wdeclaration-after-statement]
           struct ext4_group_desc *desc;
                                   ^
   3 warnings generated.
--
>> fs/ext4/mballoc.c:3752:12: warning: mixing declarations and code is incompatible with standards before C99 [-Wdeclaration-after-statement]
           handle_t *handle = mc->handle;
                     ^
   1 warning generated.


vim +275 fs/ext4/balloc.c

717d50e4971b81 Andreas Dilger    2007-10-16  248  
ac27a0ec112a08 Dave Kleikamp     2006-10-11  249  /*
ac27a0ec112a08 Dave Kleikamp     2006-10-11  250   * The free blocks are managed by bitmaps.  A file system contains several
ac27a0ec112a08 Dave Kleikamp     2006-10-11  251   * blocks groups.  Each group contains 1 bitmap block for blocks, 1 bitmap
ac27a0ec112a08 Dave Kleikamp     2006-10-11  252   * block for inodes, N blocks for the inode table and data blocks.
ac27a0ec112a08 Dave Kleikamp     2006-10-11  253   *
ac27a0ec112a08 Dave Kleikamp     2006-10-11  254   * The file system contains group descriptors which are located after the
ac27a0ec112a08 Dave Kleikamp     2006-10-11  255   * super block.  Each descriptor contains the number of the bitmap block and
ac27a0ec112a08 Dave Kleikamp     2006-10-11  256   * the free blocks count in the block.  The descriptors are loaded in memory
e627432c2948d5 Aneesh Kumar K.V  2007-02-20  257   * when a file system is mounted (see ext4_fill_super).
ac27a0ec112a08 Dave Kleikamp     2006-10-11  258   */
ac27a0ec112a08 Dave Kleikamp     2006-10-11  259  
ac27a0ec112a08 Dave Kleikamp     2006-10-11  260  /**
617ba13b31fbf5 Mingming Cao      2006-10-11  261   * ext4_get_group_desc() -- load group descriptor from disk
ac27a0ec112a08 Dave Kleikamp     2006-10-11  262   * @sb:			super block
ac27a0ec112a08 Dave Kleikamp     2006-10-11  263   * @block_group:	given block group
ac27a0ec112a08 Dave Kleikamp     2006-10-11  264   * @bh:			pointer to the buffer head to store the block
ac27a0ec112a08 Dave Kleikamp     2006-10-11  265   *			group descriptor
ac27a0ec112a08 Dave Kleikamp     2006-10-11  266   */
617ba13b31fbf5 Mingming Cao      2006-10-11  267  struct ext4_group_desc * ext4_get_group_desc(struct super_block *sb,
fd2d42912f9f09 Avantika Mathur   2008-01-28  268  					     ext4_group_t block_group,
ac27a0ec112a08 Dave Kleikamp     2006-10-11  269  					     struct buffer_head **bh)
ac27a0ec112a08 Dave Kleikamp     2006-10-11  270  {
357d528a1ead86 Kemeng Shi        2023-04-13  271  #ifdef CONFIG_EXT4_KUNIT_TESTS
357d528a1ead86 Kemeng Shi        2023-04-13  272  	KUNIT_STATIC_STUB_REDIRECT(ext4_get_group_desc,
357d528a1ead86 Kemeng Shi        2023-04-13  273  				   sb, block_group, bh);
357d528a1ead86 Kemeng Shi        2023-04-13  274  #endif
498e5f24158da7 Theodore Ts'o     2008-11-05 @275  	unsigned int group_desc;
498e5f24158da7 Theodore Ts'o     2008-11-05  276  	unsigned int offset;
8df9675f8b498d Theodore Ts'o     2009-05-01  277  	ext4_group_t ngroups = ext4_get_groups_count(sb);
617ba13b31fbf5 Mingming Cao      2006-10-11  278  	struct ext4_group_desc *desc;
617ba13b31fbf5 Mingming Cao      2006-10-11  279  	struct ext4_sb_info *sbi = EXT4_SB(sb);
1d0c3924a92e69 Theodore Ts'o     2020-02-15  280  	struct buffer_head *bh_p;
ac27a0ec112a08 Dave Kleikamp     2006-10-11  281  
8df9675f8b498d Theodore Ts'o     2009-05-01  282  	if (block_group >= ngroups) {
12062dddda4509 Eric Sandeen      2010-02-15  283  		ext4_error(sb, "block_group >= groups_count - block_group = %u,"
12062dddda4509 Eric Sandeen      2010-02-15  284  			   " groups_count = %u", block_group, ngroups);
ac27a0ec112a08 Dave Kleikamp     2006-10-11  285  
ac27a0ec112a08 Dave Kleikamp     2006-10-11  286  		return NULL;
ac27a0ec112a08 Dave Kleikamp     2006-10-11  287  	}
ac27a0ec112a08 Dave Kleikamp     2006-10-11  288  
617ba13b31fbf5 Mingming Cao      2006-10-11  289  	group_desc = block_group >> EXT4_DESC_PER_BLOCK_BITS(sb);
617ba13b31fbf5 Mingming Cao      2006-10-11  290  	offset = block_group & (EXT4_DESC_PER_BLOCK(sb) - 1);
1d0c3924a92e69 Theodore Ts'o     2020-02-15  291  	bh_p = sbi_array_rcu_deref(sbi, s_group_desc, group_desc);
1d0c3924a92e69 Theodore Ts'o     2020-02-15  292  	/*
1d0c3924a92e69 Theodore Ts'o     2020-02-15  293  	 * sbi_array_rcu_deref returns with rcu unlocked, this is ok since
1d0c3924a92e69 Theodore Ts'o     2020-02-15  294  	 * the pointer being dereferenced won't be dereferenced again. By
1d0c3924a92e69 Theodore Ts'o     2020-02-15  295  	 * looking at the usage in add_new_gdb() the value isn't modified,
1d0c3924a92e69 Theodore Ts'o     2020-02-15  296  	 * just the pointer, and so it remains valid.
1d0c3924a92e69 Theodore Ts'o     2020-02-15  297  	 */
1d0c3924a92e69 Theodore Ts'o     2020-02-15  298  	if (!bh_p) {
12062dddda4509 Eric Sandeen      2010-02-15  299  		ext4_error(sb, "Group descriptor not loaded - "
498e5f24158da7 Theodore Ts'o     2008-11-05  300  			   "block_group = %u, group_desc = %u, desc = %u",
ac27a0ec112a08 Dave Kleikamp     2006-10-11  301  			   block_group, group_desc, offset);
ac27a0ec112a08 Dave Kleikamp     2006-10-11  302  		return NULL;
ac27a0ec112a08 Dave Kleikamp     2006-10-11  303  	}
ac27a0ec112a08 Dave Kleikamp     2006-10-11  304  
0d1ee42f27d30e Alexandre Ratchov 2006-10-11  305  	desc = (struct ext4_group_desc *)(
1d0c3924a92e69 Theodore Ts'o     2020-02-15  306  		(__u8 *)bh_p->b_data +
0d1ee42f27d30e Alexandre Ratchov 2006-10-11  307  		offset * EXT4_DESC_SIZE(sb));
ac27a0ec112a08 Dave Kleikamp     2006-10-11  308  	if (bh)
1d0c3924a92e69 Theodore Ts'o     2020-02-15  309  		*bh = bh_p;
0d1ee42f27d30e Alexandre Ratchov 2006-10-11  310  	return desc;
ac27a0ec112a08 Dave Kleikamp     2006-10-11  311  }
ac27a0ec112a08 Dave Kleikamp     2006-10-11  312  

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH v2 19/19] ext4: add first unit test for ext4_mb_new_blocks_simple in mballoc
  2023-04-12 17:28 ` [PATCH v2 19/19] ext4: add first unit test for ext4_mb_new_blocks_simple in mballoc Kemeng Shi
  2023-04-12 18:16   ` kernel test robot
@ 2023-04-12 18:58   ` kernel test robot
  2023-04-12 19:49   ` kernel test robot
  2 siblings, 0 replies; 25+ messages in thread
From: kernel test robot @ 2023-04-12 18:58 UTC (permalink / raw)
  To: Kemeng Shi, tytso, adilger.kernel, ojaswin
  Cc: oe-kbuild-all, linux-ext4, linux-kernel, shikemeng

Hi Kemeng,

kernel test robot noticed the following build warnings:

[auto build test WARNING on tytso-ext4/dev]
[also build test WARNING on next-20230412]
[cannot apply to linus/master v6.3-rc6]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Kemeng-Shi/ext4-fix-wrong-unit-use-in-ext4_mb_normalize_request/20230412-172757
base:   https://git.kernel.org/pub/scm/linux/kernel/git/tytso/ext4.git dev
patch link:    https://lore.kernel.org/r/20230412172833.2317696-20-shikemeng%40huaweicloud.com
patch subject: [PATCH v2 19/19] ext4: add first unit test for ext4_mb_new_blocks_simple in mballoc
config: loongarch-randconfig-r004-20230409 (https://download.01.org/0day-ci/archive/20230413/202304130200.eNNpj54h-lkp@intel.com/config)
compiler: loongarch64-linux-gcc (GCC) 12.1.0
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # https://github.com/intel-lab-lkp/linux/commit/3ceb1daf83fdf578c28ede1a10e55f05ef7642d5
        git remote add linux-review https://github.com/intel-lab-lkp/linux
        git fetch --no-tags linux-review Kemeng-Shi/ext4-fix-wrong-unit-use-in-ext4_mb_normalize_request/20230412-172757
        git checkout 3ceb1daf83fdf578c28ede1a10e55f05ef7642d5
        # save the config file
        mkdir build_dir && cp config build_dir/.config
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.1.0 make.cross W=1 O=build_dir ARCH=loongarch olddefconfig
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.1.0 make.cross W=1 O=build_dir ARCH=loongarch SHELL=/bin/bash fs/

If you fix the issue, kindly add following tag where applicable
| Reported-by: kernel test robot <lkp@intel.com>
| Link: https://lore.kernel.org/oe-kbuild-all/202304130200.eNNpj54h-lkp@intel.com/

All warnings (new ones prefixed by >>):

   fs/ext4/mballoc.c: In function 'ext4_mb_mark_group_bb':
   fs/ext4/mballoc.c:3752:9: warning: ISO C90 forbids mixed declarations and code [-Wdeclaration-after-statement]
    3752 |         handle_t *handle = mc->handle;
         |         ^~~~~~~~
   In file included from fs/ext4/mballoc.c:6523:
   fs/ext4/mballoc-test.c: At top level:
>> fs/ext4/mballoc-test.c:153:1: warning: no previous prototype for 'ext4_read_block_bitmap_nowait_stub' [-Wmissing-prototypes]
     153 | ext4_read_block_bitmap_nowait_stub(struct super_block *sb, ext4_group_t block_group,
         | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>> fs/ext4/mballoc-test.c:162:5: warning: no previous prototype for 'ext4_wait_block_bitmap_stub' [-Wmissing-prototypes]
     162 | int ext4_wait_block_bitmap_stub(struct super_block *sb,
         |     ^~~~~~~~~~~~~~~~~~~~~~~~~~~
>> fs/ext4/mballoc-test.c:169:25: warning: no previous prototype for 'ext4_get_group_desc_stub' [-Wmissing-prototypes]
     169 | struct ext4_group_desc *ext4_get_group_desc_stub(struct super_block *sb,
         |                         ^~~~~~~~~~~~~~~~~~~~~~~~
>> fs/ext4/mballoc-test.c:181:5: warning: no previous prototype for 'ext4_mb_mark_group_bb_stub' [-Wmissing-prototypes]
     181 | int ext4_mb_mark_group_bb_stub(struct ext4_mark_context *mc,
         |     ^~~~~~~~~~~~~~~~~~~~~~~~~~


vim +/ext4_read_block_bitmap_nowait_stub +153 fs/ext4/mballoc-test.c

   151	
   152	struct buffer_head *
 > 153	ext4_read_block_bitmap_nowait_stub(struct super_block *sb, ext4_group_t block_group,
   154					   bool ignore_locked)
   155	{
   156		struct mb_grp_ctx *grp_ctx = MB_GRP_CTX(sb, block_group);
   157	
   158		get_bh(&grp_ctx->bitmap_bh);
   159		return &grp_ctx->bitmap_bh;
   160	}
   161	
 > 162	int ext4_wait_block_bitmap_stub(struct super_block *sb,
   163					ext4_group_t block_group,
   164					struct buffer_head *bh)
   165	{
   166		return 0;
   167	}
   168	
 > 169	struct ext4_group_desc *ext4_get_group_desc_stub(struct super_block *sb,
   170						     ext4_group_t block_group,
   171						     struct buffer_head **bh)
   172	{
   173		struct mb_grp_ctx *grp_ctx = MB_GRP_CTX(sb, block_group);
   174	
   175		if (bh != NULL)
   176			*bh = &grp_ctx->gd_bh;
   177	
   178		return &grp_ctx->desc;
   179	}
   180	
 > 181	int ext4_mb_mark_group_bb_stub(struct ext4_mark_context *mc,
   182				       ext4_group_t group, ext4_grpblk_t blkoff,
   183				       ext4_grpblk_t len, int flags)
   184	{
   185		struct mb_grp_ctx *grp_ctx = MB_GRP_CTX(mc->sb, group);
   186		struct buffer_head *bitmap_bh = &grp_ctx->bitmap_bh;
   187	
   188		if (mc->state)
   189			mb_set_bits(bitmap_bh->b_data, blkoff, len);
   190		else
   191			mb_clear_bits(bitmap_bh->b_data, blkoff, len);
   192	
   193		return 0;
   194	}
   195	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH v2 19/19] ext4: add first unit test for ext4_mb_new_blocks_simple in mballoc
  2023-04-12 17:28 ` [PATCH v2 19/19] ext4: add first unit test for ext4_mb_new_blocks_simple in mballoc Kemeng Shi
  2023-04-12 18:16   ` kernel test robot
  2023-04-12 18:58   ` kernel test robot
@ 2023-04-12 19:49   ` kernel test robot
  2 siblings, 0 replies; 25+ messages in thread
From: kernel test robot @ 2023-04-12 19:49 UTC (permalink / raw)
  To: Kemeng Shi, tytso, adilger.kernel, ojaswin
  Cc: llvm, oe-kbuild-all, linux-ext4, linux-kernel, shikemeng

Hi Kemeng,

kernel test robot noticed the following build errors:

[auto build test ERROR on tytso-ext4/dev]
[also build test ERROR on next-20230412]
[cannot apply to linus/master v6.3-rc6]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Kemeng-Shi/ext4-fix-wrong-unit-use-in-ext4_mb_normalize_request/20230412-172757
base:   https://git.kernel.org/pub/scm/linux/kernel/git/tytso/ext4.git dev
patch link:    https://lore.kernel.org/r/20230412172833.2317696-20-shikemeng%40huaweicloud.com
patch subject: [PATCH v2 19/19] ext4: add first unit test for ext4_mb_new_blocks_simple in mballoc
config: powerpc-randconfig-r006-20230409 (https://download.01.org/0day-ci/archive/20230413/202304130309.erdFVvYL-lkp@intel.com/config)
compiler: clang version 17.0.0 (https://github.com/llvm/llvm-project 2c57868e2e877f73c339796c3374ae660bb77f0d)
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # install powerpc cross compiling tool for clang build
        # apt-get install binutils-powerpc-linux-gnu
        # https://github.com/intel-lab-lkp/linux/commit/3ceb1daf83fdf578c28ede1a10e55f05ef7642d5
        git remote add linux-review https://github.com/intel-lab-lkp/linux
        git fetch --no-tags linux-review Kemeng-Shi/ext4-fix-wrong-unit-use-in-ext4_mb_normalize_request/20230412-172757
        git checkout 3ceb1daf83fdf578c28ede1a10e55f05ef7642d5
        # save the config file
        mkdir build_dir && cp config build_dir/.config
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=powerpc olddefconfig
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=powerpc SHELL=/bin/bash fs/

If you fix the issue, kindly add following tag where applicable
| Reported-by: kernel test robot <lkp@intel.com>
| Link: https://lore.kernel.org/oe-kbuild-all/202304130309.erdFVvYL-lkp@intel.com/

All error/warnings (new ones prefixed by >>):

   fs/ext4/mballoc.c:3752:12: warning: mixing declarations and code is incompatible with standards before C99 [-Wdeclaration-after-statement]
           handle_t *handle = mc->handle;
                     ^
   In file included from fs/ext4/mballoc.c:6523:
>> fs/ext4/mballoc-test.c:107:31: error: casting from randomized structure pointer type 'struct super_block *' to 'struct mb_ctx *'
           struct mb_grp_ctx *grp_ctx = MB_GRP_CTX(sb, group);
                                        ^
   fs/ext4/mballoc-test.c:23:33: note: expanded from macro 'MB_GRP_CTX'
   #define MB_GRP_CTX(sb, group) (&MB_CTX(sb)->grp_ctx[group])
                                   ^
   fs/ext4/mballoc-test.c:22:21: note: expanded from macro 'MB_CTX'
   #define MB_CTX(sb) ((struct mb_ctx *)((struct super_block *)sb + 1))
                       ^
   fs/ext4/mballoc-test.c:115:23: error: casting from randomized structure pointer type 'struct super_block *' to 'struct mb_ctx *'
           struct mb_ctx *ctx = MB_CTX(sb);
                                ^
   fs/ext4/mballoc-test.c:22:21: note: expanded from macro 'MB_CTX'
   #define MB_CTX(sb) ((struct mb_ctx *)((struct super_block *)sb + 1))
                       ^
   fs/ext4/mballoc-test.c:144:23: error: casting from randomized structure pointer type 'struct super_block *' to 'struct mb_ctx *'
           struct mb_ctx *ctx = MB_CTX(sb);
                                ^
   fs/ext4/mballoc-test.c:22:21: note: expanded from macro 'MB_CTX'
   #define MB_CTX(sb) ((struct mb_ctx *)((struct super_block *)sb + 1))
                       ^
   fs/ext4/mballoc-test.c:156:31: error: casting from randomized structure pointer type 'struct super_block *' to 'struct mb_ctx *'
           struct mb_grp_ctx *grp_ctx = MB_GRP_CTX(sb, block_group);
                                        ^
   fs/ext4/mballoc-test.c:23:33: note: expanded from macro 'MB_GRP_CTX'
   #define MB_GRP_CTX(sb, group) (&MB_CTX(sb)->grp_ctx[group])
                                   ^
   fs/ext4/mballoc-test.c:22:21: note: expanded from macro 'MB_CTX'
   #define MB_CTX(sb) ((struct mb_ctx *)((struct super_block *)sb + 1))
                       ^
>> fs/ext4/mballoc-test.c:153:1: warning: no previous prototype for function 'ext4_read_block_bitmap_nowait_stub' [-Wmissing-prototypes]
   ext4_read_block_bitmap_nowait_stub(struct super_block *sb, ext4_group_t block_group,
   ^
   fs/ext4/mballoc-test.c:152:1: note: declare 'static' if the function is not intended to be used outside of this translation unit
   struct buffer_head *
   ^
   static 
>> fs/ext4/mballoc-test.c:162:5: warning: no previous prototype for function 'ext4_wait_block_bitmap_stub' [-Wmissing-prototypes]
   int ext4_wait_block_bitmap_stub(struct super_block *sb,
       ^
   fs/ext4/mballoc-test.c:162:1: note: declare 'static' if the function is not intended to be used outside of this translation unit
   int ext4_wait_block_bitmap_stub(struct super_block *sb,
   ^
   static 
   fs/ext4/mballoc-test.c:173:31: error: casting from randomized structure pointer type 'struct super_block *' to 'struct mb_ctx *'
           struct mb_grp_ctx *grp_ctx = MB_GRP_CTX(sb, block_group);
                                        ^
   fs/ext4/mballoc-test.c:23:33: note: expanded from macro 'MB_GRP_CTX'
   #define MB_GRP_CTX(sb, group) (&MB_CTX(sb)->grp_ctx[group])
                                   ^
   fs/ext4/mballoc-test.c:22:21: note: expanded from macro 'MB_CTX'
   #define MB_CTX(sb) ((struct mb_ctx *)((struct super_block *)sb + 1))
                       ^
>> fs/ext4/mballoc-test.c:169:25: warning: no previous prototype for function 'ext4_get_group_desc_stub' [-Wmissing-prototypes]
   struct ext4_group_desc *ext4_get_group_desc_stub(struct super_block *sb,
                           ^
   fs/ext4/mballoc-test.c:169:1: note: declare 'static' if the function is not intended to be used outside of this translation unit
   struct ext4_group_desc *ext4_get_group_desc_stub(struct super_block *sb,
   ^
   static 
   fs/ext4/mballoc-test.c:185:31: error: casting from randomized structure pointer type 'struct super_block *' to 'struct mb_ctx *'
           struct mb_grp_ctx *grp_ctx = MB_GRP_CTX(mc->sb, group);
                                        ^
   fs/ext4/mballoc-test.c:23:33: note: expanded from macro 'MB_GRP_CTX'
   #define MB_GRP_CTX(sb, group) (&MB_CTX(sb)->grp_ctx[group])
                                   ^
   fs/ext4/mballoc-test.c:22:21: note: expanded from macro 'MB_CTX'
   #define MB_CTX(sb) ((struct mb_ctx *)((struct super_block *)sb + 1))
                       ^
>> fs/ext4/mballoc-test.c:181:5: warning: no previous prototype for function 'ext4_mb_mark_group_bb_stub' [-Wmissing-prototypes]
   int ext4_mb_mark_group_bb_stub(struct ext4_mark_context *mc,
       ^
   fs/ext4/mballoc-test.c:181:1: note: declare 'static' if the function is not intended to be used outside of this translation unit
   int ext4_mb_mark_group_bb_stub(struct ext4_mark_context *mc,
   ^
   static 
   5 warnings and 6 errors generated.


vim +107 fs/ext4/mballoc-test.c

   103	
   104	static void mb_ctx_mark_used(struct super_block *sb, ext4_group_t group,
   105				     unsigned int start, unsigned int len)
   106	{
 > 107		struct mb_grp_ctx *grp_ctx = MB_GRP_CTX(sb, group);
   108	
   109		mb_set_bits(grp_ctx->bitmap_bh.b_data, start, len);
   110	}
   111	
   112	/* called after init_sb_layout */
   113	static int mb_ctx_init(struct super_block *sb)
   114	{
   115		struct mb_ctx *ctx = MB_CTX(sb);
   116		ext4_group_t i, ngroups = ext4_get_groups_count(sb);
   117	
   118		ctx->grp_ctx = kcalloc(ngroups, sizeof(struct mb_grp_ctx),
   119				       GFP_KERNEL);
   120		if (ctx->grp_ctx == NULL)
   121			return -ENOMEM;
   122	
   123		for (i = 0; i < ngroups; i++)
   124			if (mb_grp_ctx_init(sb, &ctx->grp_ctx[i]))
   125				goto out;
   126	
   127		/*
   128		 * first data block(first cluster in first group) is used by
   129		 * metadata, mark it used to avoid to alloc data block at first
   130		 * block which will fail ext4_sb_block_valid check.
   131		 */
   132		mb_set_bits(ctx->grp_ctx[0].bitmap_bh.b_data, 0, 1);
   133	
   134		return 0;
   135	out:
   136		while (i-- > 0)
   137			mb_grp_ctx_release(&ctx->grp_ctx[i]);
   138		kfree(ctx->grp_ctx);
   139		return -ENOMEM;
   140	}
   141	
   142	static void mb_ctx_release(struct super_block *sb)
   143	{
   144		struct mb_ctx *ctx = MB_CTX(sb);
   145		ext4_group_t i, ngroups = ext4_get_groups_count(sb);
   146	
   147		for (i = 0; i < ngroups; i++)
   148			mb_grp_ctx_release(&ctx->grp_ctx[i]);
   149		kfree(ctx->grp_ctx);
   150	}
   151	
   152	struct buffer_head *
 > 153	ext4_read_block_bitmap_nowait_stub(struct super_block *sb, ext4_group_t block_group,
   154					   bool ignore_locked)
   155	{
   156		struct mb_grp_ctx *grp_ctx = MB_GRP_CTX(sb, block_group);
   157	
   158		get_bh(&grp_ctx->bitmap_bh);
   159		return &grp_ctx->bitmap_bh;
   160	}
   161	
 > 162	int ext4_wait_block_bitmap_stub(struct super_block *sb,
   163					ext4_group_t block_group,
   164					struct buffer_head *bh)
   165	{
   166		return 0;
   167	}
   168	
 > 169	struct ext4_group_desc *ext4_get_group_desc_stub(struct super_block *sb,
   170						     ext4_group_t block_group,
   171						     struct buffer_head **bh)
   172	{
   173		struct mb_grp_ctx *grp_ctx = MB_GRP_CTX(sb, block_group);
   174	
   175		if (bh != NULL)
   176			*bh = &grp_ctx->gd_bh;
   177	
   178		return &grp_ctx->desc;
   179	}
   180	
 > 181	int ext4_mb_mark_group_bb_stub(struct ext4_mark_context *mc,
   182				       ext4_group_t group, ext4_grpblk_t blkoff,
   183				       ext4_grpblk_t len, int flags)
   184	{
   185		struct mb_grp_ctx *grp_ctx = MB_GRP_CTX(mc->sb, group);
   186		struct buffer_head *bitmap_bh = &grp_ctx->bitmap_bh;
   187	
   188		if (mc->state)
   189			mb_set_bits(bitmap_bh->b_data, blkoff, len);
   190		else
   191			mb_clear_bits(bitmap_bh->b_data, blkoff, len);
   192	
   193		return 0;
   194	}
   195	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests

^ permalink raw reply	[flat|nested] 25+ messages in thread

end of thread, other threads:[~2023-04-12 19:50 UTC | newest]

Thread overview: 25+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-04-12 17:28 [PATCH v2 00/19] Fixes, cleanups and unit test for mballoc Kemeng Shi
2023-04-12 17:28 ` [PATCH v2 01/19] ext4: fix wrong unit use in ext4_mb_normalize_request Kemeng Shi
2023-04-12 17:28 ` [PATCH v2 02/19] ext4: fix unit mismatch in ext4_mb_new_blocks_simple Kemeng Shi
2023-04-12 17:28 ` [PATCH v2 03/19] ext4: fix wrong unit use in ext4_mb_find_by_goal Kemeng Shi
2023-04-12 17:28 ` [PATCH v2 04/19] ext4: treat stripe in block unit Kemeng Shi
2023-04-12 17:28 ` [PATCH v2 05/19] ext4: add EXT4_MB_HINT_GOAL_ONLY test in ext4_mb_use_preallocated Kemeng Shi
2023-04-12 17:28 ` [PATCH v2 06/19] ext4: remove ext4_block_group and ext4_block_group_offset declaration Kemeng Shi
2023-04-12 17:28 ` [PATCH v2 07/19] ext4: try all groups in ext4_mb_new_blocks_simple Kemeng Shi
2023-04-12 17:28 ` [PATCH v2 08/19] ext4: get block from bh before pass it to ext4_free_blocks_simple in ext4_free_blocks Kemeng Shi
2023-04-12 17:28 ` [PATCH v2 09/19] ext4: remove unsed parameter and unnecessary forward declaration of ext4_mb_new_blocks_simple Kemeng Shi
2023-04-12 17:28 ` [PATCH v2 10/19] ext4: fix wrong unit use in ext4_mb_clear_bb Kemeng Shi
2023-04-12 17:28 ` [PATCH v2 11/19] ext4: fix wrong unit use in ext4_mb_new_blocks Kemeng Shi
2023-04-12 17:28 ` [PATCH v2 12/19] ext4: factor out codes to update block bitmap and group descriptor on disk from ext4_mb_mark_bb Kemeng Shi
2023-04-12 17:28 ` [PATCH v2 13/19] ext4: call ext4_mb_mark_group_bb in ext4_free_blocks_simple Kemeng Shi
2023-04-12 17:28 ` [PATCH v2 14/19] ext4: extent ext4_mb_mark_group_bb to support allocation under journal Kemeng Shi
2023-04-12 17:28 ` [PATCH v2 15/19] ext4: call ext4_mb_mark_group_bb in ext4_mb_mark_diskspace_used Kemeng Shi
2023-04-12 17:28 ` [PATCH v2 16/19] ext4: call ext4_mb_mark_group_bb in ext4_mb_clear_bb Kemeng Shi
2023-04-12 17:28 ` [PATCH v2 17/19] ext4: call ext4_mb_mark_group_bb in ext4_group_add_blocks Kemeng Shi
2023-04-12 17:28 ` [PATCH v2 18/19] ext4: add some kunit stub for mballoc kunit test Kemeng Shi
2023-04-12 17:45   ` kernel test robot
2023-04-12 18:16   ` kernel test robot
2023-04-12 17:28 ` [PATCH v2 19/19] ext4: add first unit test for ext4_mb_new_blocks_simple in mballoc Kemeng Shi
2023-04-12 18:16   ` kernel test robot
2023-04-12 18:58   ` kernel test robot
2023-04-12 19:49   ` kernel test robot

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.