* [PATCH 0/7] A few patches to improve ext4 balloc
@ 2023-02-21 11:59 Kemeng Shi
2023-02-21 11:59 ` [PATCH 1/7] ext4: properly handle error of ext4_init_block_bitmap in ext4_read_block_bitmap_nowait Kemeng Shi
` (7 more replies)
0 siblings, 8 replies; 17+ messages in thread
From: Kemeng Shi @ 2023-02-21 11:59 UTC (permalink / raw)
To: tytso, adilger.kernel; +Cc: linux-ext4, linux-kernel, shikemeng
Hi, this series contain patches to properly handle error in
ext4_read_block_bitmap_nowait to avoid potential data
inconsistent, correct validation check of inode table in
ext4_valid_block_bitmap and other random cleanup patches.
More details can be found in respective log.
Thanks!
Kemeng Shi (7):
ext4: properly handle error of ext4_init_block_bitmap in
ext4_read_block_bitmap_nowait
ext4: correct validation check of inode table in
ext4_valid_block_bitmap
ext4: call ext4_bg_num_gdb_[no]meta directly in
ext4_num_base_meta_clusters
ext4: remove unnecessary check in ext4_bg_num_gdb_nometa
ext4: remove stale comment in ext4_init_block_bitmap
ext4: stop trying to verify just initialized bitmap in
ext4_read_block_bitmap_nowait
ext4: improve inode table blocks counting in
ext4_num_overhead_clusters
fs/ext4/balloc.c | 121 ++++++++++++++++++++++++-----------------------
1 file changed, 61 insertions(+), 60 deletions(-)
--
2.30.0
^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH 1/7] ext4: properly handle error of ext4_init_block_bitmap in ext4_read_block_bitmap_nowait
2023-02-21 11:59 [PATCH 0/7] A few patches to improve ext4 balloc Kemeng Shi
@ 2023-02-21 11:59 ` Kemeng Shi
2023-02-21 11:59 ` [PATCH 2/7] ext4: correct validation check of inode table in ext4_valid_block_bitmap Kemeng Shi
` (6 subsequent siblings)
7 siblings, 0 replies; 17+ messages in thread
From: Kemeng Shi @ 2023-02-21 11:59 UTC (permalink / raw)
To: tytso, adilger.kernel; +Cc: linux-ext4, linux-kernel, shikemeng
We mark buffer_head of bitmap successfully initialized even error occurs
in ext4_init_block_bitmap. Although we will return error, we will get a
invalid buffer_head of bitmap from next ext4_read_block_bitmap_nowait
which is marked buffer_verified but not successfully initialized actually
in previous ext4_read_block_bitmap_nowait.
Fix this by only marking buffer_head successfully initialized if
ext4_init_block_bitmap successes.
Signed-off-by: Kemeng Shi <shikemeng@huaweicloud.com>
---
fs/ext4/balloc.c | 12 +++++++-----
1 file changed, 7 insertions(+), 5 deletions(-)
diff --git a/fs/ext4/balloc.c b/fs/ext4/balloc.c
index 8ff4b9192a9f..10cab743e313 100644
--- a/fs/ext4/balloc.c
+++ b/fs/ext4/balloc.c
@@ -474,16 +474,18 @@ ext4_read_block_bitmap_nowait(struct super_block *sb, ext4_group_t block_group,
goto out;
}
err = ext4_init_block_bitmap(sb, bh, block_group, desc);
- set_bitmap_uptodate(bh);
- set_buffer_uptodate(bh);
- set_buffer_verified(bh);
- ext4_unlock_group(sb, block_group);
- unlock_buffer(bh);
if (err) {
+ ext4_unlock_group(sb, block_group);
+ unlock_buffer(bh);
ext4_error(sb, "Failed to init block bitmap for group "
"%u: %d", block_group, err);
goto out;
}
+ set_bitmap_uptodate(bh);
+ set_buffer_uptodate(bh);
+ set_buffer_verified(bh);
+ ext4_unlock_group(sb, block_group);
+ unlock_buffer(bh);
goto verify;
}
ext4_unlock_group(sb, block_group);
--
2.30.0
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [PATCH 2/7] ext4: correct validation check of inode table in ext4_valid_block_bitmap
2023-02-21 11:59 [PATCH 0/7] A few patches to improve ext4 balloc Kemeng Shi
2023-02-21 11:59 ` [PATCH 1/7] ext4: properly handle error of ext4_init_block_bitmap in ext4_read_block_bitmap_nowait Kemeng Shi
@ 2023-02-21 11:59 ` Kemeng Shi
2023-02-21 11:59 ` [PATCH 3/7] ext4: call ext4_bg_num_gdb_[no]meta directly in ext4_num_base_meta_clusters Kemeng Shi
` (5 subsequent siblings)
7 siblings, 0 replies; 17+ messages in thread
From: Kemeng Shi @ 2023-02-21 11:59 UTC (permalink / raw)
To: tytso, adilger.kernel; +Cc: linux-ext4, linux-kernel, shikemeng
1.Last valid cluster of inode table is EXT4_B2C(sbi, offset +
sbi->s_itb_per_group - 1). We should make sure last valid cluster is <
max_bit, i.e., EXT4_B2C(sbi, offset + sbi->s_itb_per_group - 1) is <
max_bit rather than EXT4_B2C(sbi, offset + sbi->s_itb_per_group) is
< max_bit.
2.Bit search length should be last valid cluster plus 1, i.e.,
EXT4_B2C(sbi, offset + sbi->s_itb_per_group - 1) + 1 rather than
EXT4_B2C(sbi, offset + sbi->s_itb_per_group).
Signed-off-by: Kemeng Shi <shikemeng@huaweicloud.com>
---
fs/ext4/balloc.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/fs/ext4/balloc.c b/fs/ext4/balloc.c
index 10cab743e313..22be5cd70505 100644
--- a/fs/ext4/balloc.c
+++ b/fs/ext4/balloc.c
@@ -350,13 +350,13 @@ static ext4_fsblk_t ext4_valid_block_bitmap(struct super_block *sb,
blk = ext4_inode_table(sb, desc);
offset = blk - group_first_block;
if (offset < 0 || EXT4_B2C(sbi, offset) >= max_bit ||
- EXT4_B2C(sbi, offset + sbi->s_itb_per_group) >= max_bit)
+ EXT4_B2C(sbi, offset + sbi->s_itb_per_group - 1) >= max_bit)
return blk;
next_zero_bit = ext4_find_next_zero_bit(bh->b_data,
- EXT4_B2C(sbi, offset + sbi->s_itb_per_group),
+ EXT4_B2C(sbi, offset + sbi->s_itb_per_group - 1) + 1,
EXT4_B2C(sbi, offset));
if (next_zero_bit <
- EXT4_B2C(sbi, offset + sbi->s_itb_per_group))
+ EXT4_B2C(sbi, offset + sbi->s_itb_per_group - 1) + 1)
/* bad bitmap for inode tables */
return blk;
return 0;
--
2.30.0
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [PATCH 3/7] ext4: call ext4_bg_num_gdb_[no]meta directly in ext4_num_base_meta_clusters
2023-02-21 11:59 [PATCH 0/7] A few patches to improve ext4 balloc Kemeng Shi
2023-02-21 11:59 ` [PATCH 1/7] ext4: properly handle error of ext4_init_block_bitmap in ext4_read_block_bitmap_nowait Kemeng Shi
2023-02-21 11:59 ` [PATCH 2/7] ext4: correct validation check of inode table in ext4_valid_block_bitmap Kemeng Shi
@ 2023-02-21 11:59 ` Kemeng Shi
2023-02-21 11:59 ` [PATCH 4/7] ext4: remove unnecessary check in ext4_bg_num_gdb_nometa Kemeng Shi
` (4 subsequent siblings)
7 siblings, 0 replies; 17+ messages in thread
From: Kemeng Shi @ 2023-02-21 11:59 UTC (permalink / raw)
To: tytso, adilger.kernel; +Cc: linux-ext4, linux-kernel, shikemeng
ext4_num_base_meta_clusters is already aware of meta_bg feature and test
if block_group is inside real meta block groups before calling
ext4_bg_num_gdb. Then ext4_bg_num_gdb will check if block group is inside
a real meta block groups again to decide either ext4_bg_num_gdb_meta or
ext4_bg_num_gdb_nometa is needed.
Call ext4_bg_num_gdb_meta or ext4_bg_num_gdb_nometa directly after we
check if block_group is inside a meta block groups in
ext4_num_base_meta_clusters to remove redundant check of meta block
groups in ext4_bg_num_gdb.
Signed-off-by: Kemeng Shi <shikemeng@huaweicloud.com>
---
fs/ext4/balloc.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/fs/ext4/balloc.c b/fs/ext4/balloc.c
index 22be5cd70505..9b8a32b90ddc 100644
--- a/fs/ext4/balloc.c
+++ b/fs/ext4/balloc.c
@@ -889,11 +889,11 @@ static unsigned ext4_num_base_meta_clusters(struct super_block *sb,
block_group < le32_to_cpu(sbi->s_es->s_first_meta_bg) *
sbi->s_desc_per_block) {
if (num) {
- num += ext4_bg_num_gdb(sb, block_group);
+ num += ext4_bg_num_gdb_nometa(sb, block_group);
num += le16_to_cpu(sbi->s_es->s_reserved_gdt_blocks);
}
} else { /* For META_BG_BLOCK_GROUPS */
- num += ext4_bg_num_gdb(sb, block_group);
+ num += ext4_bg_num_gdb_meta(sb, block_group);
}
return EXT4_NUM_B2C(sbi, num);
}
--
2.30.0
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [PATCH 4/7] ext4: remove unnecessary check in ext4_bg_num_gdb_nometa
2023-02-21 11:59 [PATCH 0/7] A few patches to improve ext4 balloc Kemeng Shi
` (2 preceding siblings ...)
2023-02-21 11:59 ` [PATCH 3/7] ext4: call ext4_bg_num_gdb_[no]meta directly in ext4_num_base_meta_clusters Kemeng Shi
@ 2023-02-21 11:59 ` Kemeng Shi
[not found] ` <20230613131507.0ce55666@mir>
2023-02-21 11:59 ` [PATCH 5/7] ext4: remove stale comment in ext4_init_block_bitmap Kemeng Shi
` (3 subsequent siblings)
7 siblings, 1 reply; 17+ messages in thread
From: Kemeng Shi @ 2023-02-21 11:59 UTC (permalink / raw)
To: tytso, adilger.kernel; +Cc: linux-ext4, linux-kernel, shikemeng
We only call ext4_bg_num_gdb_nometa if there is no meta_bg feature or group
does not reach first_meta_bg, so group must not reside in meta group. Then
group has a global gdt if superblock exists. Just remove confusing branch
that meta_bg feature exists.
Signed-off-by: Kemeng Shi <shikemeng@huaweicloud.com>
---
fs/ext4/balloc.c | 5 +----
1 file changed, 1 insertion(+), 4 deletions(-)
diff --git a/fs/ext4/balloc.c b/fs/ext4/balloc.c
index 9b8a32b90ddc..08f1692f7d2f 100644
--- a/fs/ext4/balloc.c
+++ b/fs/ext4/balloc.c
@@ -844,10 +844,7 @@ static unsigned long ext4_bg_num_gdb_nometa(struct super_block *sb,
if (!ext4_bg_has_super(sb, group))
return 0;
- if (ext4_has_feature_meta_bg(sb))
- return le32_to_cpu(EXT4_SB(sb)->s_es->s_first_meta_bg);
- else
- return EXT4_SB(sb)->s_gdb_count;
+ return EXT4_SB(sb)->s_gdb_count;
}
/**
--
2.30.0
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [PATCH 5/7] ext4: remove stale comment in ext4_init_block_bitmap
2023-02-21 11:59 [PATCH 0/7] A few patches to improve ext4 balloc Kemeng Shi
` (3 preceding siblings ...)
2023-02-21 11:59 ` [PATCH 4/7] ext4: remove unnecessary check in ext4_bg_num_gdb_nometa Kemeng Shi
@ 2023-02-21 11:59 ` Kemeng Shi
2023-02-21 11:59 ` [PATCH 6/7] ext4: stop trying to verify just initialized bitmap in ext4_read_block_bitmap_nowait Kemeng Shi
` (2 subsequent siblings)
7 siblings, 0 replies; 17+ messages in thread
From: Kemeng Shi @ 2023-02-21 11:59 UTC (permalink / raw)
To: tytso, adilger.kernel; +Cc: linux-ext4, linux-kernel, shikemeng
Commit bdfb6ff4a255d ("ext4: mark group corrupt on group descriptor
checksum") added flag to indicate corruption of group instead of
marking all blocks used. Just remove stale comment.
Signed-off-by: Kemeng Shi <shikemeng@huaweicloud.com>
---
fs/ext4/balloc.c | 2 --
1 file changed, 2 deletions(-)
diff --git a/fs/ext4/balloc.c b/fs/ext4/balloc.c
index 08f1692f7d2f..4de06d68e9e7 100644
--- a/fs/ext4/balloc.c
+++ b/fs/ext4/balloc.c
@@ -187,8 +187,6 @@ static int ext4_init_block_bitmap(struct super_block *sb,
ASSERT(buffer_locked(bh));
- /* If checksum is bad mark all blocks used to prevent allocation
- * essentially implementing a per-group read-only flag. */
if (!ext4_group_desc_csum_verify(sb, block_group, gdp)) {
ext4_mark_group_bitmap_corrupted(sb, block_group,
EXT4_GROUP_INFO_BBITMAP_CORRUPT |
--
2.30.0
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [PATCH 6/7] ext4: stop trying to verify just initialized bitmap in ext4_read_block_bitmap_nowait
2023-02-21 11:59 [PATCH 0/7] A few patches to improve ext4 balloc Kemeng Shi
` (4 preceding siblings ...)
2023-02-21 11:59 ` [PATCH 5/7] ext4: remove stale comment in ext4_init_block_bitmap Kemeng Shi
@ 2023-02-21 11:59 ` Kemeng Shi
2023-02-21 11:59 ` [PATCH 7/7] ext4: improve inode table blocks counting in ext4_num_overhead_clusters Kemeng Shi
2023-03-17 1:52 ` [PATCH 0/7] A few patches to improve ext4 balloc Theodore Ts'o
7 siblings, 0 replies; 17+ messages in thread
From: Kemeng Shi @ 2023-02-21 11:59 UTC (permalink / raw)
To: tytso, adilger.kernel; +Cc: linux-ext4, linux-kernel, shikemeng
For case we initialize a bitmap bh, we will set bitmap bh verified.
We can return immediately instead of goto verify to remove unnecessary
work for trying to verify bitmap bh.
Signed-off-by: Kemeng Shi <shikemeng@huaweicloud.com>
---
fs/ext4/balloc.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/fs/ext4/balloc.c b/fs/ext4/balloc.c
index 4de06d68e9e7..dab46274d591 100644
--- a/fs/ext4/balloc.c
+++ b/fs/ext4/balloc.c
@@ -484,7 +484,7 @@ ext4_read_block_bitmap_nowait(struct super_block *sb, ext4_group_t block_group,
set_buffer_verified(bh);
ext4_unlock_group(sb, block_group);
unlock_buffer(bh);
- goto verify;
+ return bh;
}
ext4_unlock_group(sb, block_group);
if (buffer_uptodate(bh)) {
--
2.30.0
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [PATCH 7/7] ext4: improve inode table blocks counting in ext4_num_overhead_clusters
2023-02-21 11:59 [PATCH 0/7] A few patches to improve ext4 balloc Kemeng Shi
` (5 preceding siblings ...)
2023-02-21 11:59 ` [PATCH 6/7] ext4: stop trying to verify just initialized bitmap in ext4_read_block_bitmap_nowait Kemeng Shi
@ 2023-02-21 11:59 ` Kemeng Shi
2023-03-20 12:44 ` Jan Kara
2023-03-17 1:52 ` [PATCH 0/7] A few patches to improve ext4 balloc Theodore Ts'o
7 siblings, 1 reply; 17+ messages in thread
From: Kemeng Shi @ 2023-02-21 11:59 UTC (permalink / raw)
To: tytso, adilger.kernel; +Cc: linux-ext4, linux-kernel, shikemeng
As inode table blocks are contiguous, inode table blocks inside the
block_group can be represented as range [itbl_cluster_start,
itbl_cluster_last]. Then we can simply account inode table cluters and
check cluster overlap with [itbl_cluster_start, itbl_cluster_last] instead
of traverse each block of inode table.
By the way, this patch fixes code style problem of comment for
ext4_num_overhead_clusters.
Signed-off-by: Kemeng Shi <shikemeng@huaweicloud.com>
---
fs/ext4/balloc.c | 90 +++++++++++++++++++++++++-----------------------
1 file changed, 47 insertions(+), 43 deletions(-)
diff --git a/fs/ext4/balloc.c b/fs/ext4/balloc.c
index dab46274d591..689edfed231a 100644
--- a/fs/ext4/balloc.c
+++ b/fs/ext4/balloc.c
@@ -80,32 +80,56 @@ static inline int ext4_block_in_group(struct super_block *sb,
return (actual_group == block_group) ? 1 : 0;
}
-/* Return the number of clusters used for file system metadata; this
+/*
+ * Return the number of clusters used for file system metadata; this
* represents the overhead needed by the file system.
*/
static unsigned ext4_num_overhead_clusters(struct super_block *sb,
ext4_group_t block_group,
struct ext4_group_desc *gdp)
{
- unsigned num_clusters;
- int block_cluster = -1, inode_cluster = -1, itbl_cluster = -1, i, c;
+ unsigned base_clusters, num_clusters;
+ int block_cluster, inode_cluster;
+ int itbl_cluster_start = -1, itbl_cluster_end = -1;
ext4_fsblk_t start = ext4_group_first_block_no(sb, block_group);
- ext4_fsblk_t itbl_blk;
+ ext4_fsblk_t end = start + EXT4_BLOCKS_PER_GROUP(sb) - 1;
+ ext4_fsblk_t itbl_blk_start, itbl_blk_end;
struct ext4_sb_info *sbi = EXT4_SB(sb);
/* This is the number of clusters used by the superblock,
* block group descriptors, and reserved block group
* descriptor blocks */
- num_clusters = ext4_num_base_meta_clusters(sb, block_group);
+ base_clusters = ext4_num_base_meta_clusters(sb, block_group);
+ num_clusters = base_clusters;
/*
- * For the allocation bitmaps and inode table, we first need
- * to check to see if the block is in the block group. If it
- * is, then check to see if the cluster is already accounted
- * for in the clusters used for the base metadata cluster, or
- * if we can increment the base metadata cluster to include
- * that block. Otherwise, we will have to track the cluster
- * used for the allocation bitmap or inode table explicitly.
+ * Account and record inode table clusters if any cluster
+ * is in the block group, or inode table cluster range is
+ * [-1, -1] and won't overlap with block/inode bitmap cluster
+ * accounted below.
+ */
+ itbl_blk_start = ext4_inode_table(sb, gdp);
+ itbl_blk_end = itbl_blk_start + sbi->s_itb_per_group - 1;
+ if (itbl_blk_start <= end && itbl_blk_end >= start) {
+ itbl_blk_start = itbl_blk_start >= start ?
+ itbl_blk_start : start;
+ itbl_blk_end = itbl_blk_end <= end ?
+ itbl_blk_end : end;
+
+ itbl_cluster_start = EXT4_B2C(sbi, itbl_blk_start - start);
+ itbl_cluster_end = EXT4_B2C(sbi, itbl_blk_end - start);
+
+ num_clusters += itbl_cluster_end - itbl_cluster_start + 1;
+ /* check if border cluster is overlapped */
+ if (itbl_cluster_start == base_clusters - 1)
+ num_clusters--;
+ }
+
+ /*
+ * For the allocation bitmaps, we first need to check to see
+ * if the block is in the block group. If it is, then check
+ * to see if the cluster is already accounted for in the clusters
+ * used for the base metadata cluster and inode tables cluster.
* Normally all of these blocks are contiguous, so the special
* case handling shouldn't be necessary except for *very*
* unusual file system layouts.
@@ -113,46 +137,26 @@ static unsigned ext4_num_overhead_clusters(struct super_block *sb,
if (ext4_block_in_group(sb, ext4_block_bitmap(sb, gdp), block_group)) {
block_cluster = EXT4_B2C(sbi,
ext4_block_bitmap(sb, gdp) - start);
- if (block_cluster < num_clusters)
- block_cluster = -1;
- else if (block_cluster == num_clusters) {
+ if (block_cluster >= base_clusters &&
+ (block_cluster < itbl_cluster_start ||
+ block_cluster > itbl_cluster_end))
num_clusters++;
- block_cluster = -1;
- }
}
if (ext4_block_in_group(sb, ext4_inode_bitmap(sb, gdp), block_group)) {
inode_cluster = EXT4_B2C(sbi,
ext4_inode_bitmap(sb, gdp) - start);
- if (inode_cluster < num_clusters)
- inode_cluster = -1;
- else if (inode_cluster == num_clusters) {
- num_clusters++;
- inode_cluster = -1;
- }
- }
-
- itbl_blk = ext4_inode_table(sb, gdp);
- for (i = 0; i < sbi->s_itb_per_group; i++) {
- if (ext4_block_in_group(sb, itbl_blk + i, block_group)) {
- c = EXT4_B2C(sbi, itbl_blk + i - start);
- if ((c < num_clusters) || (c == inode_cluster) ||
- (c == block_cluster) || (c == itbl_cluster))
- continue;
- if (c == num_clusters) {
- num_clusters++;
- continue;
- }
+ /*
+ * Additional check if inode bitmap is in just accounted
+ * block_cluster
+ */
+ if (inode_cluster != block_cluster &&
+ inode_cluster >= base_clusters &&
+ (inode_cluster < itbl_cluster_start ||
+ inode_cluster > itbl_cluster_end))
num_clusters++;
- itbl_cluster = c;
- }
}
- if (block_cluster != -1)
- num_clusters++;
- if (inode_cluster != -1)
- num_clusters++;
-
return num_clusters;
}
--
2.30.0
^ permalink raw reply related [flat|nested] 17+ messages in thread
* Re: [PATCH 7/7] ext4: improve inode table blocks counting in ext4_num_overhead_clusters
2023-02-21 11:59 ` [PATCH 7/7] ext4: improve inode table blocks counting in ext4_num_overhead_clusters Kemeng Shi
@ 2023-02-22 15:13 ` Dan Carpenter
0 siblings, 0 replies; 17+ messages in thread
From: kernel test robot @ 2023-02-22 15:05 UTC (permalink / raw)
To: oe-kbuild; +Cc: lkp, Dan Carpenter
BCC: lkp@intel.com
CC: oe-kbuild-all@lists.linux.dev
In-Reply-To: <20230221115919.1918161-8-shikemeng@huaweicloud.com>
References: <20230221115919.1918161-8-shikemeng@huaweicloud.com>
TO: Kemeng Shi <shikemeng@huaweicloud.com>
TO: tytso@mit.edu
TO: adilger.kernel@dilger.ca
CC: linux-ext4@vger.kernel.org
CC: linux-kernel@vger.kernel.org
CC: shikemeng@huaweicloud.com
Hi Kemeng,
Thank you for the patch! Perhaps something to improve:
[auto build test WARNING on tytso-ext4/dev]
[also build test WARNING on linus/master v6.2 next-20230222]
[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-properly-handle-error-of-ext4_init_block_bitmap-in-ext4_read_block_bitmap_nowait/20230221-115830
base: https://git.kernel.org/pub/scm/linux/kernel/git/tytso/ext4.git dev
patch link: https://lore.kernel.org/r/20230221115919.1918161-8-shikemeng%40huaweicloud.com
patch subject: [PATCH 7/7] ext4: improve inode table blocks counting in ext4_num_overhead_clusters
:::::: branch date: 35 hours ago
:::::: commit date: 35 hours ago
config: riscv-randconfig-m031-20230219 (https://download.01.org/0day-ci/archive/20230222/202302222219.u328sqfs-lkp@intel.com/config)
compiler: riscv32-linux-gcc (GCC) 12.1.0
If you fix the issue, kindly add following tag where applicable
| Reported-by: kernel test robot <lkp@intel.com>
| Reported-by: Dan Carpenter <error27@gmail.com>
| Link: https://lore.kernel.org/r/202302222219.u328sqfs-lkp@intel.com/
New smatch warnings:
fs/ext4/balloc.c:153 ext4_num_overhead_clusters() error: uninitialized symbol 'block_cluster'.
Old smatch warnings:
arch/riscv/include/asm/atomic.h:204 arch_atomic_fetch_add_unless() warn: inconsistent indenting
vim +/block_cluster +153 fs/ext4/balloc.c
0bf7e8379ce7e0 Jose R. Santos 2008-06-03 82
2b59a2fd93873a Kemeng Shi 2023-02-21 83 /*
2b59a2fd93873a Kemeng Shi 2023-02-21 84 * Return the number of clusters used for file system metadata; this
d5b8f31007a937 Theodore Ts'o 2011-09-09 85 * represents the overhead needed by the file system.
d5b8f31007a937 Theodore Ts'o 2011-09-09 86 */
c197855ea14175 Stephen Hemminger 2014-05-12 87 static unsigned ext4_num_overhead_clusters(struct super_block *sb,
e187c6588d6ef3 Theodore Ts'o 2009-02-06 88 ext4_group_t block_group,
e187c6588d6ef3 Theodore Ts'o 2009-02-06 89 struct ext4_group_desc *gdp)
0bf7e8379ce7e0 Jose R. Santos 2008-06-03 90 {
2b59a2fd93873a Kemeng Shi 2023-02-21 91 unsigned base_clusters, num_clusters;
2b59a2fd93873a Kemeng Shi 2023-02-21 92 int block_cluster, inode_cluster;
2b59a2fd93873a Kemeng Shi 2023-02-21 93 int itbl_cluster_start = -1, itbl_cluster_end = -1;
d5b8f31007a937 Theodore Ts'o 2011-09-09 94 ext4_fsblk_t start = ext4_group_first_block_no(sb, block_group);
2b59a2fd93873a Kemeng Shi 2023-02-21 95 ext4_fsblk_t end = start + EXT4_BLOCKS_PER_GROUP(sb) - 1;
2b59a2fd93873a Kemeng Shi 2023-02-21 96 ext4_fsblk_t itbl_blk_start, itbl_blk_end;
0bf7e8379ce7e0 Jose R. Santos 2008-06-03 97 struct ext4_sb_info *sbi = EXT4_SB(sb);
0bf7e8379ce7e0 Jose R. Santos 2008-06-03 98
d5b8f31007a937 Theodore Ts'o 2011-09-09 99 /* This is the number of clusters used by the superblock,
d5b8f31007a937 Theodore Ts'o 2011-09-09 100 * block group descriptors, and reserved block group
d5b8f31007a937 Theodore Ts'o 2011-09-09 101 * descriptor blocks */
2b59a2fd93873a Kemeng Shi 2023-02-21 102 base_clusters = ext4_num_base_meta_clusters(sb, block_group);
2b59a2fd93873a Kemeng Shi 2023-02-21 103 num_clusters = base_clusters;
2b59a2fd93873a Kemeng Shi 2023-02-21 104
2b59a2fd93873a Kemeng Shi 2023-02-21 105 /*
2b59a2fd93873a Kemeng Shi 2023-02-21 106 * Account and record inode table clusters if any cluster
2b59a2fd93873a Kemeng Shi 2023-02-21 107 * is in the block group, or inode table cluster range is
2b59a2fd93873a Kemeng Shi 2023-02-21 108 * [-1, -1] and won't overlap with block/inode bitmap cluster
2b59a2fd93873a Kemeng Shi 2023-02-21 109 * accounted below.
2b59a2fd93873a Kemeng Shi 2023-02-21 110 */
2b59a2fd93873a Kemeng Shi 2023-02-21 111 itbl_blk_start = ext4_inode_table(sb, gdp);
2b59a2fd93873a Kemeng Shi 2023-02-21 112 itbl_blk_end = itbl_blk_start + sbi->s_itb_per_group - 1;
2b59a2fd93873a Kemeng Shi 2023-02-21 113 if (itbl_blk_start <= end && itbl_blk_end >= start) {
2b59a2fd93873a Kemeng Shi 2023-02-21 114 itbl_blk_start = itbl_blk_start >= start ?
2b59a2fd93873a Kemeng Shi 2023-02-21 115 itbl_blk_start : start;
2b59a2fd93873a Kemeng Shi 2023-02-21 116 itbl_blk_end = itbl_blk_end <= end ?
2b59a2fd93873a Kemeng Shi 2023-02-21 117 itbl_blk_end : end;
2b59a2fd93873a Kemeng Shi 2023-02-21 118
2b59a2fd93873a Kemeng Shi 2023-02-21 119 itbl_cluster_start = EXT4_B2C(sbi, itbl_blk_start - start);
2b59a2fd93873a Kemeng Shi 2023-02-21 120 itbl_cluster_end = EXT4_B2C(sbi, itbl_blk_end - start);
2b59a2fd93873a Kemeng Shi 2023-02-21 121
2b59a2fd93873a Kemeng Shi 2023-02-21 122 num_clusters += itbl_cluster_end - itbl_cluster_start + 1;
2b59a2fd93873a Kemeng Shi 2023-02-21 123 /* check if border cluster is overlapped */
2b59a2fd93873a Kemeng Shi 2023-02-21 124 if (itbl_cluster_start == base_clusters - 1)
2b59a2fd93873a Kemeng Shi 2023-02-21 125 num_clusters--;
2b59a2fd93873a Kemeng Shi 2023-02-21 126 }
0bf7e8379ce7e0 Jose R. Santos 2008-06-03 127
d5b8f31007a937 Theodore Ts'o 2011-09-09 128 /*
2b59a2fd93873a Kemeng Shi 2023-02-21 129 * For the allocation bitmaps, we first need to check to see
2b59a2fd93873a Kemeng Shi 2023-02-21 130 * if the block is in the block group. If it is, then check
2b59a2fd93873a Kemeng Shi 2023-02-21 131 * to see if the cluster is already accounted for in the clusters
2b59a2fd93873a Kemeng Shi 2023-02-21 132 * used for the base metadata cluster and inode tables cluster.
d5b8f31007a937 Theodore Ts'o 2011-09-09 133 * Normally all of these blocks are contiguous, so the special
d5b8f31007a937 Theodore Ts'o 2011-09-09 134 * case handling shouldn't be necessary except for *very*
d5b8f31007a937 Theodore Ts'o 2011-09-09 135 * unusual file system layouts.
d5b8f31007a937 Theodore Ts'o 2011-09-09 136 */
d5b8f31007a937 Theodore Ts'o 2011-09-09 137 if (ext4_block_in_group(sb, ext4_block_bitmap(sb, gdp), block_group)) {
b0dd6b70f0fda1 Theodore Ts'o 2012-06-07 138 block_cluster = EXT4_B2C(sbi,
b0dd6b70f0fda1 Theodore Ts'o 2012-06-07 139 ext4_block_bitmap(sb, gdp) - start);
2b59a2fd93873a Kemeng Shi 2023-02-21 140 if (block_cluster >= base_clusters &&
2b59a2fd93873a Kemeng Shi 2023-02-21 141 (block_cluster < itbl_cluster_start ||
2b59a2fd93873a Kemeng Shi 2023-02-21 142 block_cluster > itbl_cluster_end))
d5b8f31007a937 Theodore Ts'o 2011-09-09 143 num_clusters++;
d5b8f31007a937 Theodore Ts'o 2011-09-09 144 }
d5b8f31007a937 Theodore Ts'o 2011-09-09 145
d5b8f31007a937 Theodore Ts'o 2011-09-09 146 if (ext4_block_in_group(sb, ext4_inode_bitmap(sb, gdp), block_group)) {
d5b8f31007a937 Theodore Ts'o 2011-09-09 147 inode_cluster = EXT4_B2C(sbi,
b0dd6b70f0fda1 Theodore Ts'o 2012-06-07 148 ext4_inode_bitmap(sb, gdp) - start);
2b59a2fd93873a Kemeng Shi 2023-02-21 149 /*
2b59a2fd93873a Kemeng Shi 2023-02-21 150 * Additional check if inode bitmap is in just accounted
2b59a2fd93873a Kemeng Shi 2023-02-21 151 * block_cluster
2b59a2fd93873a Kemeng Shi 2023-02-21 152 */
2b59a2fd93873a Kemeng Shi 2023-02-21 @153 if (inode_cluster != block_cluster &&
2b59a2fd93873a Kemeng Shi 2023-02-21 154 inode_cluster >= base_clusters &&
2b59a2fd93873a Kemeng Shi 2023-02-21 155 (inode_cluster < itbl_cluster_start ||
2b59a2fd93873a Kemeng Shi 2023-02-21 156 inode_cluster > itbl_cluster_end))
d5b8f31007a937 Theodore Ts'o 2011-09-09 157 num_clusters++;
0bf7e8379ce7e0 Jose R. Santos 2008-06-03 158 }
d5b8f31007a937 Theodore Ts'o 2011-09-09 159
d5b8f31007a937 Theodore Ts'o 2011-09-09 160 return num_clusters;
0bf7e8379ce7e0 Jose R. Santos 2008-06-03 161 }
c2ea3fde61f1df Theodore Ts'o 2008-10-10 162
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 7/7] ext4: improve inode table blocks counting in ext4_num_overhead_clusters
@ 2023-02-22 15:13 ` Dan Carpenter
0 siblings, 0 replies; 17+ messages in thread
From: Dan Carpenter @ 2023-02-22 15:13 UTC (permalink / raw)
To: oe-kbuild, Kemeng Shi, tytso, adilger.kernel
Cc: lkp, oe-kbuild-all, linux-ext4, linux-kernel, shikemeng
Hi Kemeng,
https://git-scm.com/docs/git-format-patch#_base_tree_information]
url: https://github.com/intel-lab-lkp/linux/commits/Kemeng-Shi/ext4-properly-handle-error-of-ext4_init_block_bitmap-in-ext4_read_block_bitmap_nowait/20230221-115830
base: https://git.kernel.org/pub/scm/linux/kernel/git/tytso/ext4.git dev
patch link: https://lore.kernel.org/r/20230221115919.1918161-8-shikemeng%40huaweicloud.com
patch subject: [PATCH 7/7] ext4: improve inode table blocks counting in ext4_num_overhead_clusters
config: riscv-randconfig-m031-20230219 (https://download.01.org/0day-ci/archive/20230222/202302222219.u328sqfs-lkp@intel.com/config)
compiler: riscv32-linux-gcc (GCC) 12.1.0
If you fix the issue, kindly add following tag where applicable
| Reported-by: kernel test robot <lkp@intel.com>
| Reported-by: Dan Carpenter <error27@gmail.com>
| Link: https://lore.kernel.org/r/202302222219.u328sqfs-lkp@intel.com/
New smatch warnings:
fs/ext4/balloc.c:153 ext4_num_overhead_clusters() error: uninitialized symbol 'block_cluster'.
vim +/block_cluster +153 fs/ext4/balloc.c
c197855ea14175 Stephen Hemminger 2014-05-12 87 static unsigned ext4_num_overhead_clusters(struct super_block *sb,
e187c6588d6ef3 Theodore Ts'o 2009-02-06 88 ext4_group_t block_group,
e187c6588d6ef3 Theodore Ts'o 2009-02-06 89 struct ext4_group_desc *gdp)
0bf7e8379ce7e0 Jose R. Santos 2008-06-03 90 {
2b59a2fd93873a Kemeng Shi 2023-02-21 91 unsigned base_clusters, num_clusters;
2b59a2fd93873a Kemeng Shi 2023-02-21 92 int block_cluster, inode_cluster;
2b59a2fd93873a Kemeng Shi 2023-02-21 93 int itbl_cluster_start = -1, itbl_cluster_end = -1;
d5b8f31007a937 Theodore Ts'o 2011-09-09 94 ext4_fsblk_t start = ext4_group_first_block_no(sb, block_group);
2b59a2fd93873a Kemeng Shi 2023-02-21 95 ext4_fsblk_t end = start + EXT4_BLOCKS_PER_GROUP(sb) - 1;
2b59a2fd93873a Kemeng Shi 2023-02-21 96 ext4_fsblk_t itbl_blk_start, itbl_blk_end;
0bf7e8379ce7e0 Jose R. Santos 2008-06-03 97 struct ext4_sb_info *sbi = EXT4_SB(sb);
0bf7e8379ce7e0 Jose R. Santos 2008-06-03 98
d5b8f31007a937 Theodore Ts'o 2011-09-09 99 /* This is the number of clusters used by the superblock,
d5b8f31007a937 Theodore Ts'o 2011-09-09 100 * block group descriptors, and reserved block group
d5b8f31007a937 Theodore Ts'o 2011-09-09 101 * descriptor blocks */
2b59a2fd93873a Kemeng Shi 2023-02-21 102 base_clusters = ext4_num_base_meta_clusters(sb, block_group);
2b59a2fd93873a Kemeng Shi 2023-02-21 103 num_clusters = base_clusters;
2b59a2fd93873a Kemeng Shi 2023-02-21 104
2b59a2fd93873a Kemeng Shi 2023-02-21 105 /*
2b59a2fd93873a Kemeng Shi 2023-02-21 106 * Account and record inode table clusters if any cluster
2b59a2fd93873a Kemeng Shi 2023-02-21 107 * is in the block group, or inode table cluster range is
2b59a2fd93873a Kemeng Shi 2023-02-21 108 * [-1, -1] and won't overlap with block/inode bitmap cluster
2b59a2fd93873a Kemeng Shi 2023-02-21 109 * accounted below.
2b59a2fd93873a Kemeng Shi 2023-02-21 110 */
2b59a2fd93873a Kemeng Shi 2023-02-21 111 itbl_blk_start = ext4_inode_table(sb, gdp);
2b59a2fd93873a Kemeng Shi 2023-02-21 112 itbl_blk_end = itbl_blk_start + sbi->s_itb_per_group - 1;
2b59a2fd93873a Kemeng Shi 2023-02-21 113 if (itbl_blk_start <= end && itbl_blk_end >= start) {
2b59a2fd93873a Kemeng Shi 2023-02-21 114 itbl_blk_start = itbl_blk_start >= start ?
2b59a2fd93873a Kemeng Shi 2023-02-21 115 itbl_blk_start : start;
2b59a2fd93873a Kemeng Shi 2023-02-21 116 itbl_blk_end = itbl_blk_end <= end ?
2b59a2fd93873a Kemeng Shi 2023-02-21 117 itbl_blk_end : end;
2b59a2fd93873a Kemeng Shi 2023-02-21 118
2b59a2fd93873a Kemeng Shi 2023-02-21 119 itbl_cluster_start = EXT4_B2C(sbi, itbl_blk_start - start);
2b59a2fd93873a Kemeng Shi 2023-02-21 120 itbl_cluster_end = EXT4_B2C(sbi, itbl_blk_end - start);
2b59a2fd93873a Kemeng Shi 2023-02-21 121
2b59a2fd93873a Kemeng Shi 2023-02-21 122 num_clusters += itbl_cluster_end - itbl_cluster_start + 1;
2b59a2fd93873a Kemeng Shi 2023-02-21 123 /* check if border cluster is overlapped */
2b59a2fd93873a Kemeng Shi 2023-02-21 124 if (itbl_cluster_start == base_clusters - 1)
2b59a2fd93873a Kemeng Shi 2023-02-21 125 num_clusters--;
2b59a2fd93873a Kemeng Shi 2023-02-21 126 }
0bf7e8379ce7e0 Jose R. Santos 2008-06-03 127
d5b8f31007a937 Theodore Ts'o 2011-09-09 128 /*
2b59a2fd93873a Kemeng Shi 2023-02-21 129 * For the allocation bitmaps, we first need to check to see
2b59a2fd93873a Kemeng Shi 2023-02-21 130 * if the block is in the block group. If it is, then check
2b59a2fd93873a Kemeng Shi 2023-02-21 131 * to see if the cluster is already accounted for in the clusters
2b59a2fd93873a Kemeng Shi 2023-02-21 132 * used for the base metadata cluster and inode tables cluster.
d5b8f31007a937 Theodore Ts'o 2011-09-09 133 * Normally all of these blocks are contiguous, so the special
d5b8f31007a937 Theodore Ts'o 2011-09-09 134 * case handling shouldn't be necessary except for *very*
d5b8f31007a937 Theodore Ts'o 2011-09-09 135 * unusual file system layouts.
d5b8f31007a937 Theodore Ts'o 2011-09-09 136 */
d5b8f31007a937 Theodore Ts'o 2011-09-09 137 if (ext4_block_in_group(sb, ext4_block_bitmap(sb, gdp), block_group)) {
b0dd6b70f0fda1 Theodore Ts'o 2012-06-07 138 block_cluster = EXT4_B2C(sbi,
b0dd6b70f0fda1 Theodore Ts'o 2012-06-07 139 ext4_block_bitmap(sb, gdp) - start);
2b59a2fd93873a Kemeng Shi 2023-02-21 140 if (block_cluster >= base_clusters &&
2b59a2fd93873a Kemeng Shi 2023-02-21 141 (block_cluster < itbl_cluster_start ||
2b59a2fd93873a Kemeng Shi 2023-02-21 142 block_cluster > itbl_cluster_end))
d5b8f31007a937 Theodore Ts'o 2011-09-09 143 num_clusters++;
d5b8f31007a937 Theodore Ts'o 2011-09-09 144 }
d5b8f31007a937 Theodore Ts'o 2011-09-09 145
d5b8f31007a937 Theodore Ts'o 2011-09-09 146 if (ext4_block_in_group(sb, ext4_inode_bitmap(sb, gdp), block_group)) {
These two conditions are exactly the same so I don't see why they can't
be combined into one condition. I have read the comment, but I guess I
don't understand ext4 well enough to really understand it.
d5b8f31007a937 Theodore Ts'o 2011-09-09 147 inode_cluster = EXT4_B2C(sbi,
b0dd6b70f0fda1 Theodore Ts'o 2012-06-07 148 ext4_inode_bitmap(sb, gdp) - start);
2b59a2fd93873a Kemeng Shi 2023-02-21 149 /*
2b59a2fd93873a Kemeng Shi 2023-02-21 150 * Additional check if inode bitmap is in just accounted
2b59a2fd93873a Kemeng Shi 2023-02-21 151 * block_cluster
2b59a2fd93873a Kemeng Shi 2023-02-21 152 */
2b59a2fd93873a Kemeng Shi 2023-02-21 @153 if (inode_cluster != block_cluster &&
So this seems like a false positive to me. But the code is puzzling for
a human or for a static checker.
2b59a2fd93873a Kemeng Shi 2023-02-21 154 inode_cluster >= base_clusters &&
2b59a2fd93873a Kemeng Shi 2023-02-21 155 (inode_cluster < itbl_cluster_start ||
2b59a2fd93873a Kemeng Shi 2023-02-21 156 inode_cluster > itbl_cluster_end))
d5b8f31007a937 Theodore Ts'o 2011-09-09 157 num_clusters++;
0bf7e8379ce7e0 Jose R. Santos 2008-06-03 158 }
d5b8f31007a937 Theodore Ts'o 2011-09-09 159
d5b8f31007a937 Theodore Ts'o 2011-09-09 160 return num_clusters;
0bf7e8379ce7e0 Jose R. Santos 2008-06-03 161 }
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 7/7] ext4: improve inode table blocks counting in ext4_num_overhead_clusters
2023-02-22 15:13 ` Dan Carpenter
(?)
@ 2023-02-23 1:31 ` Kemeng Shi
2023-02-23 4:04 ` Dan Carpenter
-1 siblings, 1 reply; 17+ messages in thread
From: Kemeng Shi @ 2023-02-23 1:31 UTC (permalink / raw)
To: Dan Carpenter, oe-kbuild, tytso, adilger.kernel
Cc: lkp, oe-kbuild-all, linux-ext4, linux-kernel
on 2/22/2023 11:13 PM, Dan Carpenter wrote:
> Hi Kemeng,
>
> https://git-scm.com/docs/git-format-patch#_base_tree_information]
>
> url: https://github.com/intel-lab-lkp/linux/commits/Kemeng-Shi/ext4-properly-handle-error-of-ext4_init_block_bitmap-in-ext4_read_block_bitmap_nowait/20230221-115830
> base: https://git.kernel.org/pub/scm/linux/kernel/git/tytso/ext4.git dev
> patch link: https://lore.kernel.org/r/20230221115919.1918161-8-shikemeng%40huaweicloud.com
> patch subject: [PATCH 7/7] ext4: improve inode table blocks counting in ext4_num_overhead_clusters
> config: riscv-randconfig-m031-20230219 (https://download.01.org/0day-ci/archive/20230222/202302222219.u328sqfs-lkp@intel.com/config)
> compiler: riscv32-linux-gcc (GCC) 12.1.0
>
> If you fix the issue, kindly add following tag where applicable
> | Reported-by: kernel test robot <lkp@intel.com>
> | Reported-by: Dan Carpenter <error27@gmail.com>
> | Link: https://lore.kernel.org/r/202302222219.u328sqfs-lkp@intel.com/
>
> New smatch warnings:
> fs/ext4/balloc.c:153 ext4_num_overhead_clusters() error: uninitialized symbol 'block_cluster'.
>
> vim +/block_cluster +153 fs/ext4/balloc.c
[...]
> d5b8f31007a937 Theodore Ts'o 2011-09-09 128 /*
> 2b59a2fd93873a Kemeng Shi 2023-02-21 129 * For the allocation bitmaps, we first need to check to see
> 2b59a2fd93873a Kemeng Shi 2023-02-21 130 * if the block is in the block group. If it is, then check
> 2b59a2fd93873a Kemeng Shi 2023-02-21 131 * to see if the cluster is already accounted for in the clusters
> 2b59a2fd93873a Kemeng Shi 2023-02-21 132 * used for the base metadata cluster and inode tables cluster.
> d5b8f31007a937 Theodore Ts'o 2011-09-09 133 * Normally all of these blocks are contiguous, so the special
> d5b8f31007a937 Theodore Ts'o 2011-09-09 134 * case handling shouldn't be necessary except for *very*
> d5b8f31007a937 Theodore Ts'o 2011-09-09 135 * unusual file system layouts.
> d5b8f31007a937 Theodore Ts'o 2011-09-09 136 */
> d5b8f31007a937 Theodore Ts'o 2011-09-09 137 if (ext4_block_in_group(sb, ext4_block_bitmap(sb, gdp), block_group)) {
> b0dd6b70f0fda1 Theodore Ts'o 2012-06-07 138 block_cluster = EXT4_B2C(sbi,
> b0dd6b70f0fda1 Theodore Ts'o 2012-06-07 139 ext4_block_bitmap(sb, gdp) - start);
> 2b59a2fd93873a Kemeng Shi 2023-02-21 140 if (block_cluster >= base_clusters &&
> 2b59a2fd93873a Kemeng Shi 2023-02-21 141 (block_cluster < itbl_cluster_start ||
> 2b59a2fd93873a Kemeng Shi 2023-02-21 142 block_cluster > itbl_cluster_end))
> d5b8f31007a937 Theodore Ts'o 2011-09-09 143 num_clusters++;
> d5b8f31007a937 Theodore Ts'o 2011-09-09 144 }
> d5b8f31007a937 Theodore Ts'o 2011-09-09 145
> d5b8f31007a937 Theodore Ts'o 2011-09-09 146 if (ext4_block_in_group(sb, ext4_inode_bitmap(sb, gdp), block_group)) {
>
> These two conditions are exactly the same so I don't see why they can't
> be combined into one condition. I have read the comment, but I guess I
> don't understand ext4 well enough to really understand it.
These two conditions check two kinds of bitmap block: *block* bitmap block
and *inode* bitmap block. For case that block bitmap in the block group
while inode bitmap in a different group, there is a risk to access
uninitialized block_cluster.
I will fix this in next version, Thanks!
--
Best wishes
Kemeng Shi
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 7/7] ext4: improve inode table blocks counting in ext4_num_overhead_clusters
2023-02-23 1:31 ` Kemeng Shi
@ 2023-02-23 4:04 ` Dan Carpenter
0 siblings, 0 replies; 17+ messages in thread
From: Dan Carpenter @ 2023-02-23 4:04 UTC (permalink / raw)
To: Kemeng Shi
Cc: oe-kbuild, tytso, adilger.kernel, lkp, oe-kbuild-all, linux-ext4,
linux-kernel
On Thu, Feb 23, 2023 at 09:31:54AM +0800, Kemeng Shi wrote:
>
>
> on 2/22/2023 11:13 PM, Dan Carpenter wrote:
> > Hi Kemeng,
> >
> > https://git-scm.com/docs/git-format-patch#_base_tree_information]
> >
> > url: https://github.com/intel-lab-lkp/linux/commits/Kemeng-Shi/ext4-properly-handle-error-of-ext4_init_block_bitmap-in-ext4_read_block_bitmap_nowait/20230221-115830
> > base: https://git.kernel.org/pub/scm/linux/kernel/git/tytso/ext4.git dev
> > patch link: https://lore.kernel.org/r/20230221115919.1918161-8-shikemeng%40huaweicloud.com
> > patch subject: [PATCH 7/7] ext4: improve inode table blocks counting in ext4_num_overhead_clusters
> > config: riscv-randconfig-m031-20230219 (https://download.01.org/0day-ci/archive/20230222/202302222219.u328sqfs-lkp@intel.com/config)
> > compiler: riscv32-linux-gcc (GCC) 12.1.0
> >
> > If you fix the issue, kindly add following tag where applicable
> > | Reported-by: kernel test robot <lkp@intel.com>
> > | Reported-by: Dan Carpenter <error27@gmail.com>
> > | Link: https://lore.kernel.org/r/202302222219.u328sqfs-lkp@intel.com/
> >
> > New smatch warnings:
> > fs/ext4/balloc.c:153 ext4_num_overhead_clusters() error: uninitialized symbol 'block_cluster'.
> >
> > vim +/block_cluster +153 fs/ext4/balloc.c
> [...]
> > d5b8f31007a937 Theodore Ts'o 2011-09-09 128 /*
> > 2b59a2fd93873a Kemeng Shi 2023-02-21 129 * For the allocation bitmaps, we first need to check to see
> > 2b59a2fd93873a Kemeng Shi 2023-02-21 130 * if the block is in the block group. If it is, then check
> > 2b59a2fd93873a Kemeng Shi 2023-02-21 131 * to see if the cluster is already accounted for in the clusters
> > 2b59a2fd93873a Kemeng Shi 2023-02-21 132 * used for the base metadata cluster and inode tables cluster.
> > d5b8f31007a937 Theodore Ts'o 2011-09-09 133 * Normally all of these blocks are contiguous, so the special
> > d5b8f31007a937 Theodore Ts'o 2011-09-09 134 * case handling shouldn't be necessary except for *very*
> > d5b8f31007a937 Theodore Ts'o 2011-09-09 135 * unusual file system layouts.
> > d5b8f31007a937 Theodore Ts'o 2011-09-09 136 */
> > d5b8f31007a937 Theodore Ts'o 2011-09-09 137 if (ext4_block_in_group(sb, ext4_block_bitmap(sb, gdp), block_group)) {
> > b0dd6b70f0fda1 Theodore Ts'o 2012-06-07 138 block_cluster = EXT4_B2C(sbi,
> > b0dd6b70f0fda1 Theodore Ts'o 2012-06-07 139 ext4_block_bitmap(sb, gdp) - start);
> > 2b59a2fd93873a Kemeng Shi 2023-02-21 140 if (block_cluster >= base_clusters &&
> > 2b59a2fd93873a Kemeng Shi 2023-02-21 141 (block_cluster < itbl_cluster_start ||
> > 2b59a2fd93873a Kemeng Shi 2023-02-21 142 block_cluster > itbl_cluster_end))
> > d5b8f31007a937 Theodore Ts'o 2011-09-09 143 num_clusters++;
> > d5b8f31007a937 Theodore Ts'o 2011-09-09 144 }
> > d5b8f31007a937 Theodore Ts'o 2011-09-09 145
> > d5b8f31007a937 Theodore Ts'o 2011-09-09 146 if (ext4_block_in_group(sb, ext4_inode_bitmap(sb, gdp), block_group)) {
> >
> > These two conditions are exactly the same so I don't see why they can't
> > be combined into one condition. I have read the comment, but I guess I
> > don't understand ext4 well enough to really understand it.
> These two conditions check two kinds of bitmap block: *block* bitmap block
> and *inode* bitmap block.
Ah right. When I was reviewing this code, I copy and pasted the if
conditions so they were exactly lined up with each other and I still
didn't see the block vs inode difference until you pointed it out. :P
Thanks!
regards,
dan carpenter
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 0/7] A few patches to improve ext4 balloc
2023-02-21 11:59 [PATCH 0/7] A few patches to improve ext4 balloc Kemeng Shi
` (6 preceding siblings ...)
2023-02-21 11:59 ` [PATCH 7/7] ext4: improve inode table blocks counting in ext4_num_overhead_clusters Kemeng Shi
@ 2023-03-17 1:52 ` Theodore Ts'o
7 siblings, 0 replies; 17+ messages in thread
From: Theodore Ts'o @ 2023-03-17 1:52 UTC (permalink / raw)
To: Kemeng Shi, adilger.kernel; +Cc: Theodore Ts'o, linux-ext4, linux-kernel
On Tue, 21 Feb 2023 19:59:12 +0800, Kemeng Shi wrote:
> ext4_read_block_bitmap_nowait to avoid potential data
> inconsistent, correct validation check of inode table in
> ext4_valid_block_bitmap and other random cleanup patches.
> More details can be found in respective log.
> Thanks!
>
> Kemeng Shi (7):
> ext4: properly handle error of ext4_init_block_bitmap in
> ext4_read_block_bitmap_nowait
> ext4: correct validation check of inode table in
> ext4_valid_block_bitmap
> ext4: call ext4_bg_num_gdb_[no]meta directly in
> ext4_num_base_meta_clusters
> ext4: remove unnecessary check in ext4_bg_num_gdb_nometa
> ext4: remove stale comment in ext4_init_block_bitmap
> ext4: stop trying to verify just initialized bitmap in
> ext4_read_block_bitmap_nowait
> ext4: improve inode table blocks counting in
> ext4_num_overhead_clusters
>
> [...]
Applied, thanks!
[1/7] ext4: properly handle error of ext4_init_block_bitmap in ext4_read_block_bitmap_nowait
commit: 68070432da0de5e346f5f75a76f76858000c8e53
[2/7] ext4: correct validation check of inode table in ext4_valid_block_bitmap
commit: 1d4ef2264e0f18275d8176fa3083a7988117de40
[3/7] ext4: call ext4_bg_num_gdb_[no]meta directly in ext4_num_base_meta_clusters
commit: 8bd501d2ef66bb06765aecfd3a69a5ac3213e28f
[4/7] ext4: remove unnecessary check in ext4_bg_num_gdb_nometa
commit: 6b8948ef40e97de05fa6329eb985c9cc1168aefa
[5/7] ext4: remove stale comment in ext4_init_block_bitmap
commit: 35690178606526d33a8ae254660a8a2239a08d6d
[6/7] ext4: stop trying to verify just initialized bitmap in ext4_read_block_bitmap_nowait
commit: 23a7838694dc48315b38d0cf111afaaedfc5081f
[7/7] ext4: improve inode table blocks counting in ext4_num_overhead_clusters
commit: e3c70113e2cbeb3dadb3768964920337eff290f6
Best regards,
--
Theodore Ts'o <tytso@mit.edu>
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 7/7] ext4: improve inode table blocks counting in ext4_num_overhead_clusters
2023-02-21 11:59 ` [PATCH 7/7] ext4: improve inode table blocks counting in ext4_num_overhead_clusters Kemeng Shi
@ 2023-03-20 12:44 ` Jan Kara
2023-03-20 13:19 ` Kemeng Shi
0 siblings, 1 reply; 17+ messages in thread
From: Jan Kara @ 2023-03-20 12:44 UTC (permalink / raw)
To: Kemeng Shi; +Cc: tytso, adilger.kernel, linux-ext4, linux-kernel
On Tue 21-02-23 19:59:19, Kemeng Shi wrote:
> As inode table blocks are contiguous, inode table blocks inside the
> block_group can be represented as range [itbl_cluster_start,
> itbl_cluster_last]. Then we can simply account inode table cluters and
> check cluster overlap with [itbl_cluster_start, itbl_cluster_last] instead
> of traverse each block of inode table.
> By the way, this patch fixes code style problem of comment for
> ext4_num_overhead_clusters.
>
> Signed-off-by: Kemeng Shi <shikemeng@huaweicloud.com>
FWIW this is triggering Coverity warning:
*** CID 1536792: Uninitialized variables (UNINIT)
/fs/ext4/balloc.c: 153 in ext4_num_overhead_clusters()
147 inode_cluster = EXT4_B2C(sbi,
148 ext4_inode_bitmap(sb, gdp) - st
149 /*
150 * Additional check if inode bitmap is in just accounted
151 * block_cluster
152 */
>>> CID 1536792: Uninitialized variables (UNINIT)
>>> Using uninitialized value "block_cluster".
153 if (inode_cluster != block_cluster &&
154 inode_cluster >= base_clusters &&
155 (inode_cluster < itbl_cluster_start ||
156 inode_cluster > itbl_cluster_end))
157 num_clusters++;
158 }
which actually looks valid AFAICT.
Honza
--
Jan Kara <jack@suse.com>
SUSE Labs, CR
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 7/7] ext4: improve inode table blocks counting in ext4_num_overhead_clusters
2023-03-20 12:44 ` Jan Kara
@ 2023-03-20 13:19 ` Kemeng Shi
0 siblings, 0 replies; 17+ messages in thread
From: Kemeng Shi @ 2023-03-20 13:19 UTC (permalink / raw)
To: Jan Kara; +Cc: tytso, adilger.kernel, linux-ext4, linux-kernel
on 3/20/2023 8:44 PM, Jan Kara wrote:
> On Tue 21-02-23 19:59:19, Kemeng Shi wrote:
>> As inode table blocks are contiguous, inode table blocks inside the
>> block_group can be represented as range [itbl_cluster_start,
>> itbl_cluster_last]. Then we can simply account inode table cluters and
>> check cluster overlap with [itbl_cluster_start, itbl_cluster_last] instead
>> of traverse each block of inode table.
>> By the way, this patch fixes code style problem of comment for
>> ext4_num_overhead_clusters.
>>
>> Signed-off-by: Kemeng Shi <shikemeng@huaweicloud.com>
>
> FWIW this is triggering Coverity warning:
>
> *** CID 1536792: Uninitialized variables (UNINIT)
> /fs/ext4/balloc.c: 153 in ext4_num_overhead_clusters()
> 147 inode_cluster = EXT4_B2C(sbi,
> 148 ext4_inode_bitmap(sb, gdp) - st
> 149 /*
> 150 * Additional check if inode bitmap is in just accounted
> 151 * block_cluster
> 152 */
>>>> CID 1536792: Uninitialized variables (UNINIT)
>>>> Using uninitialized value "block_cluster".
> 153 if (inode_cluster != block_cluster &&
> 154 inode_cluster >= base_clusters &&
> 155 (inode_cluster < itbl_cluster_start ||
> 156 inode_cluster > itbl_cluster_end))
> 157 num_clusters++;
> 158 }
>
> which actually looks valid AFAICT.
Yes, there is a risk to access uninitialized block_cluster if block bitmap block
and inode bitmap block are in different groups. Patch to fix is just sent. Thanks!
--
Best wishes
Kemeng Shi
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 4/7] ext4: remove unnecessary check in ext4_bg_num_gdb_nometa
[not found] ` <20230613131507.0ce55666@mir>
@ 2023-06-13 12:04 ` Linux regression tracking (Thorsten Leemhuis)
2023-06-13 14:46 ` Kemeng Shi
1 sibling, 0 replies; 17+ messages in thread
From: Linux regression tracking (Thorsten Leemhuis) @ 2023-06-13 12:04 UTC (permalink / raw)
To: Stefan Lippers-Hollmann, Kemeng Shi
Cc: tytso, adilger.kernel, linux-ext4, linux-kernel,
Linux kernel regressions list
[just resend while CCing the regression list, as the report apparently
didn't make it to the lists]
On 13.06.23 13:15, Stefan Lippers-Hollmann wrote:
> On 2023-02-21, Kemeng Shi wrote:
>> We only call ext4_bg_num_gdb_nometa if there is no meta_bg feature or group
>> does not reach first_meta_bg, so group must not reside in meta group. Then
>> group has a global gdt if superblock exists. Just remove confusing branch
>> that meta_bg feature exists.
>>
>> Signed-off-by: Kemeng Shi <shikemeng@huaweicloud.com>
>> ---
>> fs/ext4/balloc.c | 5 +----
>> 1 file changed, 1 insertion(+), 4 deletions(-)
>>
>> diff --git a/fs/ext4/balloc.c b/fs/ext4/balloc.c
>> index 9b8a32b90ddc..08f1692f7d2f 100644
>> --- a/fs/ext4/balloc.c
>> +++ b/fs/ext4/balloc.c
>> @@ -844,10 +844,7 @@ static unsigned long ext4_bg_num_gdb_nometa(struct super_block *sb,
>> if (!ext4_bg_has_super(sb, group))
>> return 0;
>>
>> - if (ext4_has_feature_meta_bg(sb))
>> - return le32_to_cpu(EXT4_SB(sb)->s_es->s_first_meta_bg);
>> - else
>> - return EXT4_SB(sb)->s_gdb_count;
>> + return EXT4_SB(sb)->s_gdb_count;
>> }
>>
>> /**
>
> This change as part of v6.4-rc6-26-gfb054096aea0 seems to cause a
> regression for mounting a (large, lvm2 backed, grown several times,
> probably also shrunk at least once) ext4 filesystem for me (other,
> smaller/ simpler ext4 filesystems mount fine).
>
> # LANG= mount /dev/vg-trident/storage /mnt/
> mount: /mnt: mount(2) system call failed: Structure needs cleaning.
> dmesg(1) may have more information after failed mount system call.
>
> [ 569.576241] EXT4-fs (dm-6): ext4_check_descriptors: Inode table for group 2 overlaps block group descriptors
> [ 569.576251] EXT4-fs (dm-6): group descriptors corrupted!
>
> e2fsck (1.47.0-2, Debian/unstable, amd64) does not find any problems
> about this fs.
>
> # fsck -f /dev/vg-trident/storage
> fsck from util-linux 2.38.1
> e2fsck 1.47.0 (5-Feb-2023)
> Pass 1: Checking inodes, blocks, and sizes
> Pass 2: Checking directory structure
> Pass 3: Checking directory connectivity
> Pass 4: Checking reference counts
> Pass 5: Checking group summary information
> storage: 1274324/595369984 files (0.4% non-contiguous), 4194120223/4762936320 blocks
>
> Bisecting leads me to this patch:
>
> $ git bisect log
> git bisect start
> # Status: warte auf guten und schlechten Commit
> # good: [457391b0380335d5e9a5babdec90ac53928b23b4] Linux 6.3
> git bisect good 457391b0380335d5e9a5babdec90ac53928b23b4
> # Status: warte auf schlechten Commit, 1 guter Commit bekannt
> # bad: [fb054096aea0576f0c0a61c598e5e9676443ee86] Merge tag 'mm-hotfixes-stable-2023-06-12-12-22' of git://git.kernel.org/pub/scm/linux/kernel/git/akpm/mm
> git bisect bad fb054096aea0576f0c0a61c598e5e9676443ee86
> # bad: [6e98b09da931a00bf4e0477d0fa52748bf28fcce] Merge tag 'net-next-6.4' of git://git.kernel.org/pub/scm/linux/kernel/git/netdev/net-next
> git bisect bad 6e98b09da931a00bf4e0477d0fa52748bf28fcce
> # good: [088e0c188513b58a0056a488cf5b7df094a8a48a] Merge tag 'platform-drivers-x86-v6.4-1' of git://git.kernel.org/pub/scm/linux/kernel/git/pdx86/platform-drivers-x86
> git bisect good 088e0c188513b58a0056a488cf5b7df094a8a48a
> # good: [ca288965801572fe41386560d4e6c5cc0e5cc56d] Merge tag 'wireless-next-2023-04-21' of git://git.kernel.org/pub/scm/linux/kernel/git/wireless/wireless-next
> git bisect good ca288965801572fe41386560d4e6c5cc0e5cc56d
> # bad: [94fc0792661a96d64a4bb79cf10d0793ecadf76e] Merge tag 'fs_for_v6.4-rc1' of git://git.kernel.org/pub/scm/linux/kernel/git/jack/linux-fs
> git bisect bad 94fc0792661a96d64a4bb79cf10d0793ecadf76e
> # good: [4173cf6fb6b7d1b4569cca08af318c4561356fb5] Merge tag 'hwmon-for-v6.4' of git://git.kernel.org/pub/scm/linux/kernel/git/groeck/linux-staging
> git bisect good 4173cf6fb6b7d1b4569cca08af318c4561356fb5
> # good: [98f99e67a1dc456e9a542584819b2aa265ffc737] Merge tag 'flex-array-transformations-6.4-rc1' of git://git.kernel.org/pub/scm/linux/kernel/git/gustavoars/linux
> git bisect good 98f99e67a1dc456e9a542584819b2aa265ffc737
> # good: [ba24b8eb3ef676cb7d6cef4a2a53f3624f880d42] crypto: testmgr - Add some test vectors for cmac(camellia)
> git bisect good ba24b8eb3ef676cb7d6cef4a2a53f3624f880d42
> # bad: [4d934a5e6caa6dcdd3fbee7b96fe512a455863b6] ext4: Convert ext4_write_begin() to use a folio
> git bisect bad 4d934a5e6caa6dcdd3fbee7b96fe512a455863b6
> # bad: [78dc9f844f4ec999a30517313040948a4c4bbc00] ext4: use best found when complex scan of group finishs
> git bisect bad 78dc9f844f4ec999a30517313040948a4c4bbc00
> # bad: [b83acc77718644b2c19832226ca284ce01efc550] ext4: remove unused group parameter in ext4_inode_bitmap_csum_verify
> git bisect bad b83acc77718644b2c19832226ca284ce01efc550
> # good: [e6c28a26b799c7640b77daff3e4a67808c74381c] ext4: Fix warnings when freezing filesystem with journaled data
> git bisect good e6c28a26b799c7640b77daff3e4a67808c74381c
> # good: [a38627f14356f505f621b31197fd872b99a10563] ext4: call ext4_bg_num_gdb_[no]meta directly in ext4_num_base_meta_clusters
> git bisect good a38627f14356f505f621b31197fd872b99a10563
> # bad: [f567ea7843562b7d7dfe1e6cfea455a1e9623208] ext4: remove stale comment in ext4_init_block_bitmap
> git bisect bad f567ea7843562b7d7dfe1e6cfea455a1e9623208
>
> # tune2fs -l /dev/vg-trident/storage
> tune2fs 1.47.0 (5-Feb-2023)
> Filesystem volume name: storage
> Last mounted on: /srv/storage
> Filesystem UUID: ad4e7e8d-f375-491f-a5dd-6251d32229e6
> Filesystem magic number: 0xEF53
> Filesystem revision #: 1 (dynamic)
> Filesystem features: has_journal ext_attr dir_index filetype meta_bg extent 64bit flex_bg sparse_super large_file huge_file dir_nlink extra_isize metadata_csum
> Filesystem flags: signed_directory_hash
> Default mount options: user_xattr acl
> Filesystem state: clean
> Errors behavior: Continue
> Filesystem OS type: Linux
> Inode count: 595369984
> Block count: 4762936320
> Reserved block count: 0
> Overhead clusters: 37571453
> Free blocks: 568816097
> Free inodes: 594095660
> First block: 0
> Block size: 4096
> Fragment size: 4096
> Group descriptor size: 64
> Blocks per group: 32768
> Fragments per group: 32768
> Inodes per group: 4096
> Inode blocks per group: 256
> First meta block group: 1453
> Flex block group size: 16
> Filesystem created: Tue Dec 16 03:26:32 2014
> Last mount time: Tue Jun 13 08:54:33 2023
> Last write time: Tue Jun 13 08:55:57 2023
> Mount count: 0
> Maximum mount count: -1
> Last checked: Tue Jun 13 08:55:57 2023
> Check interval: 0 (<none>)
> Lifetime writes: 145 TB
> Reserved blocks uid: 0 (user root)
> Reserved blocks gid: 0 (group root)
> First inode: 11
> Inode size: 256
> Required extra isize: 28
> Desired extra isize: 28
> Journal inode: 8
> Default directory hash: half_md4
> Directory Hash Seed: 3e89594a-1c09-4c46-bb6e-402ad3d697b8
> Journal backup: inode blocks
> Checksum type: crc32c
> Checksum: 0xa57a2ce7
>
> -- dumpe2fs --
>
> dumpe2fs 1.47.0 (5-Feb-2023)
> Filesystem volume name: storage
> Last mounted on: /srv/storage
> Filesystem UUID: ad4e7e8d-f375-491f-a5dd-6251d32229e6
> Filesystem magic number: 0xEF53
> Filesystem revision #: 1 (dynamic)
> Filesystem features: has_journal ext_attr dir_index filetype meta_bg extent 64bit flex_bg sparse_super large_file huge_file dir_nlink extra_isize metadata_csum
> Filesystem flags: signed_directory_hash
> Default mount options: user_xattr acl
> Filesystem state: clean
> Errors behavior: Continue
> Filesystem OS type: Linux
> Inode count: 595369984
> Block count: 4762936320
> Reserved block count: 0
> Overhead clusters: 37571453
> Free blocks: 568816097
> Free inodes: 594095660
> First block: 0
> Block size: 4096
> Fragment size: 4096
> Group descriptor size: 64
> Blocks per group: 32768
> Fragments per group: 32768
> Inodes per group: 4096
> Inode blocks per group: 256
> First meta block group: 1453
> Flex block group size: 16
> Filesystem created: Tue Dec 16 03:26:32 2014
> Last mount time: Tue Jun 13 08:54:33 2023
> Last write time: Tue Jun 13 09:07:45 2023
> Mount count: 0
> Maximum mount count: -1
> Last checked: Tue Jun 13 09:07:45 2023
> Check interval: 0 (<none>)
> Lifetime writes: 145 TB
> Reserved blocks uid: 0 (user root)
> Reserved blocks gid: 0 (group root)
> First inode: 11
> Inode size: 256
> Required extra isize: 28
> Desired extra isize: 28
> Journal inode: 8
> Default directory hash: half_md4
> Directory Hash Seed: 3e89594a-1c09-4c46-bb6e-402ad3d697b8
> Journal backup: inode blocks
> Checksum type: crc32c
> Checksum: 0xf32a54d5
> Journal features: journal_incompat_revoke journal_64bit journal_checksum_v3
> Total journal size: 128M
> Total journal blocks: 32768
> Max transaction length: 32768
> Fast commit length: 0
> Journal sequence: 0x0074096a
> Journal start: 0
> Journal checksum type: crc32c
> Journal checksum: 0x478ec5ac
>
> [... cut, the full dumpe2fs is ~60 MB, and even xz -9 compressed
> still ~6 MB large ...]
>
> Reverting (just) ad3f09be6cfe332be8ff46c78e6ec0f8839107aa from
> v6.4-rc6-27-ge328c473340c allows me to mount this filesystem again.
>
> This seems to be the same issue as:
> https://bugzilla.kernel.org/show_bug.cgi?id=217534
> https://lkml.kernel.org/r/<17d7e7f8-ad8d-1696-32b7-3ff9fd4548c1@gmail.com>
>
> Regards
> Stefan Lippers-Hollmann
>
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 4/7] ext4: remove unnecessary check in ext4_bg_num_gdb_nometa
[not found] ` <20230613131507.0ce55666@mir>
2023-06-13 12:04 ` Linux regression tracking (Thorsten Leemhuis)
@ 2023-06-13 14:46 ` Kemeng Shi
1 sibling, 0 replies; 17+ messages in thread
From: Kemeng Shi @ 2023-06-13 14:46 UTC (permalink / raw)
To: Stefan Lippers-Hollmann
Cc: tytso, adilger.kernel, linux-ext4, linux-kernel, regressions
on 6/13/2023 7:15 PM, Stefan Lippers-Hollmann wrote:
> Hi
>
> On 2023-02-21, Kemeng Shi wrote:
>> We only call ext4_bg_num_gdb_nometa if there is no meta_bg feature or group
>> does not reach first_meta_bg, so group must not reside in meta group. Then
>> group has a global gdt if superblock exists. Just remove confusing branch
>> that meta_bg feature exists.
>>
>> Signed-off-by: Kemeng Shi <shikemeng@huaweicloud.com>
>> ---
>> fs/ext4/balloc.c | 5 +----
>> 1 file changed, 1 insertion(+), 4 deletions(-)
>>
>> diff --git a/fs/ext4/balloc.c b/fs/ext4/balloc.c
>> index 9b8a32b90ddc..08f1692f7d2f 100644
>> --- a/fs/ext4/balloc.c
>> +++ b/fs/ext4/balloc.c
>> @@ -844,10 +844,7 @@ static unsigned long ext4_bg_num_gdb_nometa(struct super_block *sb,
>> if (!ext4_bg_has_super(sb, group))
>> return 0;
>>
>> - if (ext4_has_feature_meta_bg(sb))
>> - return le32_to_cpu(EXT4_SB(sb)->s_es->s_first_meta_bg);
>> - else
>> - return EXT4_SB(sb)->s_gdb_count;
>> + return EXT4_SB(sb)->s_gdb_count;
>> }
>>
>> /**
>
> This change as part of v6.4-rc6-26-gfb054096aea0 seems to cause a
> regression for mounting a (large, lvm2 backed, grown several times,
> probably also shrunk at least once) ext4 filesystem for me (other,
> smaller/ simpler ext4 filesystems mount fine).
>
Hi Stefan, thank you for let me know the issue. It seems that s_gdb_count
is block number used for both non-meta and meta block group descriptors.
However we should return block number used for non-meta block group in
ext4_bg_num_gdb_nometa.
Sorry for this problem, I will send a revert patch to fix this.
> # LANG= mount /dev/vg-trident/storage /mnt/
> mount: /mnt: mount(2) system call failed: Structure needs cleaning.
> dmesg(1) may have more information after failed mount system call.
>
> [ 569.576241] EXT4-fs (dm-6): ext4_check_descriptors: Inode table for group 2 overlaps block group descriptors
> [ 569.576251] EXT4-fs (dm-6): group descriptors corrupted!
>
> e2fsck (1.47.0-2, Debian/unstable, amd64) does not find any problems
> about this fs.
>
> # fsck -f /dev/vg-trident/storage
> fsck from util-linux 2.38.1
> e2fsck 1.47.0 (5-Feb-2023)
> Pass 1: Checking inodes, blocks, and sizes
> Pass 2: Checking directory structure
> Pass 3: Checking directory connectivity
> Pass 4: Checking reference counts
> Pass 5: Checking group summary information
> storage: 1274324/595369984 files (0.4% non-contiguous), 4194120223/4762936320 blocks
>
> Bisecting leads me to this patch:
>
> $ git bisect log
> git bisect start
> # Status: warte auf guten und schlechten Commit
> # good: [457391b0380335d5e9a5babdec90ac53928b23b4] Linux 6.3
> git bisect good 457391b0380335d5e9a5babdec90ac53928b23b4
> # Status: warte auf schlechten Commit, 1 guter Commit bekannt
> # bad: [fb054096aea0576f0c0a61c598e5e9676443ee86] Merge tag 'mm-hotfixes-stable-2023-06-12-12-22' of git://git.kernel.org/pub/scm/linux/kernel/git/akpm/mm
> git bisect bad fb054096aea0576f0c0a61c598e5e9676443ee86
> # bad: [6e98b09da931a00bf4e0477d0fa52748bf28fcce] Merge tag 'net-next-6.4' of git://git.kernel.org/pub/scm/linux/kernel/git/netdev/net-next
> git bisect bad 6e98b09da931a00bf4e0477d0fa52748bf28fcce
> # good: [088e0c188513b58a0056a488cf5b7df094a8a48a] Merge tag 'platform-drivers-x86-v6.4-1' of git://git.kernel.org/pub/scm/linux/kernel/git/pdx86/platform-drivers-x86
> git bisect good 088e0c188513b58a0056a488cf5b7df094a8a48a
> # good: [ca288965801572fe41386560d4e6c5cc0e5cc56d] Merge tag 'wireless-next-2023-04-21' of git://git.kernel.org/pub/scm/linux/kernel/git/wireless/wireless-next
> git bisect good ca288965801572fe41386560d4e6c5cc0e5cc56d
> # bad: [94fc0792661a96d64a4bb79cf10d0793ecadf76e] Merge tag 'fs_for_v6.4-rc1' of git://git.kernel.org/pub/scm/linux/kernel/git/jack/linux-fs
> git bisect bad 94fc0792661a96d64a4bb79cf10d0793ecadf76e
> # good: [4173cf6fb6b7d1b4569cca08af318c4561356fb5] Merge tag 'hwmon-for-v6.4' of git://git.kernel.org/pub/scm/linux/kernel/git/groeck/linux-staging
> git bisect good 4173cf6fb6b7d1b4569cca08af318c4561356fb5
> # good: [98f99e67a1dc456e9a542584819b2aa265ffc737] Merge tag 'flex-array-transformations-6.4-rc1' of git://git.kernel.org/pub/scm/linux/kernel/git/gustavoars/linux
> git bisect good 98f99e67a1dc456e9a542584819b2aa265ffc737
> # good: [ba24b8eb3ef676cb7d6cef4a2a53f3624f880d42] crypto: testmgr - Add some test vectors for cmac(camellia)
> git bisect good ba24b8eb3ef676cb7d6cef4a2a53f3624f880d42
> # bad: [4d934a5e6caa6dcdd3fbee7b96fe512a455863b6] ext4: Convert ext4_write_begin() to use a folio
> git bisect bad 4d934a5e6caa6dcdd3fbee7b96fe512a455863b6
> # bad: [78dc9f844f4ec999a30517313040948a4c4bbc00] ext4: use best found when complex scan of group finishs
> git bisect bad 78dc9f844f4ec999a30517313040948a4c4bbc00
> # bad: [b83acc77718644b2c19832226ca284ce01efc550] ext4: remove unused group parameter in ext4_inode_bitmap_csum_verify
> git bisect bad b83acc77718644b2c19832226ca284ce01efc550
> # good: [e6c28a26b799c7640b77daff3e4a67808c74381c] ext4: Fix warnings when freezing filesystem with journaled data
> git bisect good e6c28a26b799c7640b77daff3e4a67808c74381c
> # good: [a38627f14356f505f621b31197fd872b99a10563] ext4: call ext4_bg_num_gdb_[no]meta directly in ext4_num_base_meta_clusters
> git bisect good a38627f14356f505f621b31197fd872b99a10563
> # bad: [f567ea7843562b7d7dfe1e6cfea455a1e9623208] ext4: remove stale comment in ext4_init_block_bitmap
> git bisect bad f567ea7843562b7d7dfe1e6cfea455a1e9623208
>
> # tune2fs -l /dev/vg-trident/storage
> tune2fs 1.47.0 (5-Feb-2023)
> Filesystem volume name: storage
> Last mounted on: /srv/storage
> Filesystem UUID: ad4e7e8d-f375-491f-a5dd-6251d32229e6
> Filesystem magic number: 0xEF53
> Filesystem revision #: 1 (dynamic)
> Filesystem features: has_journal ext_attr dir_index filetype meta_bg extent 64bit flex_bg sparse_super large_file huge_file dir_nlink extra_isize metadata_csum
> Filesystem flags: signed_directory_hash
> Default mount options: user_xattr acl
> Filesystem state: clean
> Errors behavior: Continue
> Filesystem OS type: Linux
> Inode count: 595369984
> Block count: 4762936320
> Reserved block count: 0
> Overhead clusters: 37571453
> Free blocks: 568816097
> Free inodes: 594095660
> First block: 0
> Block size: 4096
> Fragment size: 4096
> Group descriptor size: 64
> Blocks per group: 32768
> Fragments per group: 32768
> Inodes per group: 4096
> Inode blocks per group: 256
> First meta block group: 1453
> Flex block group size: 16
> Filesystem created: Tue Dec 16 03:26:32 2014
> Last mount time: Tue Jun 13 08:54:33 2023
> Last write time: Tue Jun 13 08:55:57 2023
> Mount count: 0
> Maximum mount count: -1
> Last checked: Tue Jun 13 08:55:57 2023
> Check interval: 0 (<none>)
> Lifetime writes: 145 TB
> Reserved blocks uid: 0 (user root)
> Reserved blocks gid: 0 (group root)
> First inode: 11
> Inode size: 256
> Required extra isize: 28
> Desired extra isize: 28
> Journal inode: 8
> Default directory hash: half_md4
> Directory Hash Seed: 3e89594a-1c09-4c46-bb6e-402ad3d697b8
> Journal backup: inode blocks
> Checksum type: crc32c
> Checksum: 0xa57a2ce7
>
> -- dumpe2fs --
>
> dumpe2fs 1.47.0 (5-Feb-2023)
> Filesystem volume name: storage
> Last mounted on: /srv/storage
> Filesystem UUID: ad4e7e8d-f375-491f-a5dd-6251d32229e6
> Filesystem magic number: 0xEF53
> Filesystem revision #: 1 (dynamic)
> Filesystem features: has_journal ext_attr dir_index filetype meta_bg extent 64bit flex_bg sparse_super large_file huge_file dir_nlink extra_isize metadata_csum
> Filesystem flags: signed_directory_hash
> Default mount options: user_xattr acl
> Filesystem state: clean
> Errors behavior: Continue
> Filesystem OS type: Linux
> Inode count: 595369984
> Block count: 4762936320
> Reserved block count: 0
> Overhead clusters: 37571453
> Free blocks: 568816097
> Free inodes: 594095660
> First block: 0
> Block size: 4096
> Fragment size: 4096
> Group descriptor size: 64
> Blocks per group: 32768
> Fragments per group: 32768
> Inodes per group: 4096
> Inode blocks per group: 256
> First meta block group: 1453
> Flex block group size: 16
> Filesystem created: Tue Dec 16 03:26:32 2014
> Last mount time: Tue Jun 13 08:54:33 2023
> Last write time: Tue Jun 13 09:07:45 2023
> Mount count: 0
> Maximum mount count: -1
> Last checked: Tue Jun 13 09:07:45 2023
> Check interval: 0 (<none>)
> Lifetime writes: 145 TB
> Reserved blocks uid: 0 (user root)
> Reserved blocks gid: 0 (group root)
> First inode: 11
> Inode size: 256
> Required extra isize: 28
> Desired extra isize: 28
> Journal inode: 8
> Default directory hash: half_md4
> Directory Hash Seed: 3e89594a-1c09-4c46-bb6e-402ad3d697b8
> Journal backup: inode blocks
> Checksum type: crc32c
> Checksum: 0xf32a54d5
> Journal features: journal_incompat_revoke journal_64bit journal_checksum_v3
> Total journal size: 128M
> Total journal blocks: 32768
> Max transaction length: 32768
> Fast commit length: 0
> Journal sequence: 0x0074096a
> Journal start: 0
> Journal checksum type: crc32c
> Journal checksum: 0x478ec5ac
>
> [... cut, the full dumpe2fs is ~60 MB, and even xz -9 compressed
> still ~6 MB large ...]
>
> Reverting (just) ad3f09be6cfe332be8ff46c78e6ec0f8839107aa from
> v6.4-rc6-27-ge328c473340c allows me to mount this filesystem again.
>
> This seems to be the same issue as:
> https://bugzilla.kernel.org/show_bug.cgi?id=217534
> https://lkml.kernel.org/r/<17d7e7f8-ad8d-1696-32b7-3ff9fd4548c1@gmail.com>
>
> Regards
> Stefan Lippers-Hollmann
>
--
Best wishes
Kemeng Shi
^ permalink raw reply [flat|nested] 17+ messages in thread
end of thread, other threads:[~2023-06-13 14:47 UTC | newest]
Thread overview: 17+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-02-22 15:05 [PATCH 7/7] ext4: improve inode table blocks counting in ext4_num_overhead_clusters kernel test robot
2023-02-22 15:13 ` Dan Carpenter
2023-02-23 1:31 ` Kemeng Shi
2023-02-23 4:04 ` Dan Carpenter
-- strict thread matches above, loose matches on Subject: below --
2023-02-21 11:59 [PATCH 0/7] A few patches to improve ext4 balloc Kemeng Shi
2023-02-21 11:59 ` [PATCH 1/7] ext4: properly handle error of ext4_init_block_bitmap in ext4_read_block_bitmap_nowait Kemeng Shi
2023-02-21 11:59 ` [PATCH 2/7] ext4: correct validation check of inode table in ext4_valid_block_bitmap Kemeng Shi
2023-02-21 11:59 ` [PATCH 3/7] ext4: call ext4_bg_num_gdb_[no]meta directly in ext4_num_base_meta_clusters Kemeng Shi
2023-02-21 11:59 ` [PATCH 4/7] ext4: remove unnecessary check in ext4_bg_num_gdb_nometa Kemeng Shi
[not found] ` <20230613131507.0ce55666@mir>
2023-06-13 12:04 ` Linux regression tracking (Thorsten Leemhuis)
2023-06-13 14:46 ` Kemeng Shi
2023-02-21 11:59 ` [PATCH 5/7] ext4: remove stale comment in ext4_init_block_bitmap Kemeng Shi
2023-02-21 11:59 ` [PATCH 6/7] ext4: stop trying to verify just initialized bitmap in ext4_read_block_bitmap_nowait Kemeng Shi
2023-02-21 11:59 ` [PATCH 7/7] ext4: improve inode table blocks counting in ext4_num_overhead_clusters Kemeng Shi
2023-03-20 12:44 ` Jan Kara
2023-03-20 13:19 ` Kemeng Shi
2023-03-17 1:52 ` [PATCH 0/7] A few patches to improve ext4 balloc Theodore Ts'o
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.